diff --git a/renderdoc/driver/d3d11/d3d11_replay.cpp b/renderdoc/driver/d3d11/d3d11_replay.cpp index 306baf571..88af70484 100644 --- a/renderdoc/driver/d3d11/d3d11_replay.cpp +++ b/renderdoc/driver/d3d11/d3d11_replay.cpp @@ -2582,7 +2582,6 @@ uint32_t D3D11Replay::PickVertex(uint32_t eventId, int32_t width, int32_t height } ID3D11Buffer *vb = NULL, *ib = NULL; - DXGI_FORMAT ifmt = cfg.position.indexByteStride == 4 ? DXGI_FORMAT_R32_UINT : DXGI_FORMAT_R16_UINT; { auto it = WrappedID3D11Buffer::m_BufferList.find(cfg.position.vertexResourceId); @@ -2598,27 +2597,34 @@ uint32_t D3D11Replay::PickVertex(uint32_t eventId, int32_t width, int32_t height HRESULT hr = S_OK; - // most IB/VBs will not be available as SRVs. So, we copy into our own buffers. - // In the case of VB we also tightly pack and unpack the data. IB can just be - // read as R16 or R32 via the SRV so it is just a straight copy + // most IB/VBs will not be available as SRVs. So, we copy into our own buffers. In the case of VB + // we also tightly pack and unpack the data. IB is upcast to R32 so it we can apply baseVertex + // without risking overflow. + + uint32_t minIndex = 0; + uint32_t maxIndex = cfg.position.numIndices; + + uint32_t idxclamp = 0; + if(cfg.position.baseVertex < 0) + idxclamp = uint32_t(-cfg.position.baseVertex); if(cfg.position.indexByteStride) { // resize up on demand if(m_VertexPick.PickIBBuf == NULL || - m_VertexPick.PickIBSize < cfg.position.numIndices * cfg.position.indexByteStride) + m_VertexPick.PickIBSize < cfg.position.numIndices * sizeof(uint32_t)) { SAFE_RELEASE(m_VertexPick.PickIBBuf); SAFE_RELEASE(m_VertexPick.PickIBSRV); - D3D11_BUFFER_DESC desc = {cfg.position.numIndices * cfg.position.indexByteStride, + D3D11_BUFFER_DESC desc = {cfg.position.numIndices * sizeof(uint32_t), D3D11_USAGE_DEFAULT, D3D11_BIND_SHADER_RESOURCE, 0, 0, 0}; - m_VertexPick.PickIBSize = cfg.position.numIndices * cfg.position.indexByteStride; + m_VertexPick.PickIBSize = cfg.position.numIndices * sizeof(uint32_t); hr = m_pDevice->CreateBuffer(&desc, NULL, &m_VertexPick.PickIBBuf); @@ -2630,7 +2636,7 @@ uint32_t D3D11Replay::PickVertex(uint32_t eventId, int32_t width, int32_t height D3D11_SHADER_RESOURCE_VIEW_DESC sdesc; sdesc.ViewDimension = D3D11_SRV_DIMENSION_BUFFER; - sdesc.Format = ifmt; + sdesc.Format = DXGI_FORMAT_R32_UINT; sdesc.Buffer.FirstElement = 0; sdesc.Buffer.NumElements = cfg.position.numIndices; @@ -2645,100 +2651,141 @@ uint32_t D3D11Replay::PickVertex(uint32_t eventId, int32_t width, int32_t height } } - // copy index data as-is, the view format will take care of the rest - RDCASSERT(cfg.position.indexByteOffset < 0xffffffff); if(ib) { - D3D11_BUFFER_DESC ibdesc; - ib->GetDesc(&ibdesc); + bytebuf idxs; + GetBufferData(cfg.position.indexResourceId, cfg.position.indexByteOffset, 0, idxs); + + std::vector outidxs; + outidxs.resize(cfg.position.numIndices); + + uint16_t *idxs16 = (uint16_t *)&idxs[0]; + uint32_t *idxs32 = (uint32_t *)&idxs[0]; + + if(cfg.position.indexByteStride == 2) + { + size_t bufsize = idxs.size() / 2; + + for(uint32_t i = 0; i < bufsize && i < cfg.position.numIndices; i++) + { + uint32_t idx = idxs16[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + if(i == 0) + { + minIndex = maxIndex = idx; + } + else + { + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + } + + outidxs[i] = idx; + } + } + else + { + uint32_t bufsize = uint32_t(idxs.size() / 4); + + minIndex = maxIndex = idxs32[0]; + + for(uint32_t i = 0; i < RDCMIN(bufsize, cfg.position.numIndices); i++) + { + uint32_t idx = idxs32[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + + outidxs[i] = idx; + } + } D3D11_BOX box; - box.front = 0; - box.back = 1; - box.left = (uint32_t)cfg.position.indexByteOffset; - box.right = (uint32_t)cfg.position.indexByteOffset + - cfg.position.numIndices * cfg.position.indexByteStride; box.top = 0; box.bottom = 1; + box.front = 0; + box.back = 1; + box.left = 0; + box.right = UINT(outidxs.size() * sizeof(uint32_t)); - box.right = RDCMIN(box.right, ibdesc.ByteWidth - (uint32_t)cfg.position.indexByteOffset); - - m_pImmediateContext->CopySubresourceRegion(m_VertexPick.PickIBBuf, 0, 0, 0, 0, ib, 0, &box); - } - } - - if(m_VertexPick.PickVBBuf == NULL || - m_VertexPick.PickVBSize < cfg.position.numIndices * sizeof(Vec4f)) - { - SAFE_RELEASE(m_VertexPick.PickVBBuf); - SAFE_RELEASE(m_VertexPick.PickVBSRV); - - D3D11_BUFFER_DESC desc = {cfg.position.numIndices * sizeof(Vec4f), - D3D11_USAGE_DEFAULT, - D3D11_BIND_SHADER_RESOURCE, - 0, - 0, - 0}; - - m_VertexPick.PickVBSize = cfg.position.numIndices * sizeof(Vec4f); - - hr = m_pDevice->CreateBuffer(&desc, NULL, &m_VertexPick.PickVBBuf); - - if(FAILED(hr)) - { - RDCERR("Failed to create PickVBBuf HRESULT: %s", ToStr(hr).c_str()); - return ~0U; - } - - D3D11_SHADER_RESOURCE_VIEW_DESC sdesc; - sdesc.ViewDimension = D3D11_SRV_DIMENSION_BUFFER; - sdesc.Format = DXGI_FORMAT_R32G32B32A32_FLOAT; - sdesc.Buffer.FirstElement = 0; - sdesc.Buffer.NumElements = cfg.position.numIndices; - - hr = m_pDevice->CreateShaderResourceView(m_VertexPick.PickVBBuf, &sdesc, &m_VertexPick.PickVBSRV); - - if(FAILED(hr)) - { - SAFE_RELEASE(m_VertexPick.PickVBBuf); - RDCERR("Failed to create PickVBSRV HRESULT: %s", ToStr(hr).c_str()); - return ~0U; + m_pImmediateContext->UpdateSubresource(m_VertexPick.PickIBBuf, 0, &box, outidxs.data(), 0, 0); } } // unpack and linearise the data if(vb) { - FloatVector *vbData = new FloatVector[cfg.position.numIndices]; - bytebuf oldData; GetDebugManager()->GetBufferData(vb, cfg.position.vertexByteOffset, 0, oldData); + // clamp maxIndex to upper bound in case we got invalid indices or primitive restart indices + maxIndex = RDCMIN(maxIndex, uint32_t(oldData.size() / cfg.position.vertexByteStride)); + + if(m_VertexPick.PickVBBuf == NULL || m_VertexPick.PickVBSize < (maxIndex + 1) * sizeof(Vec4f)) + { + SAFE_RELEASE(m_VertexPick.PickVBBuf); + SAFE_RELEASE(m_VertexPick.PickVBSRV); + + D3D11_BUFFER_DESC desc = { + (maxIndex + 1) * sizeof(Vec4f), D3D11_USAGE_DEFAULT, D3D11_BIND_SHADER_RESOURCE, 0, 0, 0}; + + m_VertexPick.PickVBSize = (maxIndex + 1) * sizeof(Vec4f); + + hr = m_pDevice->CreateBuffer(&desc, NULL, &m_VertexPick.PickVBBuf); + + if(FAILED(hr)) + { + RDCERR("Failed to create PickVBBuf HRESULT: %s", ToStr(hr).c_str()); + return ~0U; + } + + D3D11_SHADER_RESOURCE_VIEW_DESC sdesc; + sdesc.ViewDimension = D3D11_SRV_DIMENSION_BUFFER; + sdesc.Format = DXGI_FORMAT_R32G32B32A32_FLOAT; + sdesc.Buffer.FirstElement = 0; + sdesc.Buffer.NumElements = (maxIndex + 1); + + hr = m_pDevice->CreateShaderResourceView(m_VertexPick.PickVBBuf, &sdesc, + &m_VertexPick.PickVBSRV); + + if(FAILED(hr)) + { + SAFE_RELEASE(m_VertexPick.PickVBBuf); + RDCERR("Failed to create PickVBSRV HRESULT: %s", ToStr(hr).c_str()); + return ~0U; + } + } + + std::vector vbData; + vbData.resize(maxIndex + 1); + byte *data = &oldData[0]; byte *dataEnd = data + oldData.size(); bool valid; - uint32_t idxclamp = 0; - if(cfg.position.baseVertex < 0) - idxclamp = uint32_t(-cfg.position.baseVertex); - - for(uint32_t i = 0; i < cfg.position.numIndices; i++) - { - uint32_t idx = i; - - // apply baseVertex but clamp to 0 (don't allow index to become negative) - if(idx < idxclamp) - idx = 0; - else if(cfg.position.baseVertex < 0) - idx -= idxclamp; - else if(cfg.position.baseVertex > 0) - idx += cfg.position.baseVertex; - - vbData[i] = HighlightCache::InterpretVertex(data, idx, cfg, dataEnd, valid); - } + // the index buffer may refer to vertices past the start of the vertex buffer, so we can't just + // conver the first N vertices we'll need. + // Instead we grab min and max above, and convert every vertex in that range. This might + // slightly over-estimate but not as bad as 0-max or the whole buffer. + for(uint32_t idx = minIndex; idx <= maxIndex; idx++) + vbData[idx] = HighlightCache::InterpretVertex(data, idx, cfg, dataEnd, valid); D3D11_BOX box; box.top = 0; @@ -2746,12 +2793,10 @@ uint32_t D3D11Replay::PickVertex(uint32_t eventId, int32_t width, int32_t height box.front = 0; box.back = 1; box.left = 0; - box.right = cfg.position.numIndices * sizeof(Vec4f); + box.right = (maxIndex + 1) * sizeof(Vec4f); - m_pImmediateContext->UpdateSubresource(m_VertexPick.PickVBBuf, 0, &box, vbData, sizeof(Vec4f), - sizeof(Vec4f)); - - delete[] vbData; + m_pImmediateContext->UpdateSubresource(m_VertexPick.PickVBBuf, 0, &box, vbData.data(), + sizeof(Vec4f), sizeof(Vec4f)); } ID3D11ShaderResourceView *srvs[2] = {m_VertexPick.PickIBSRV, m_VertexPick.PickVBSRV}; diff --git a/renderdoc/driver/d3d12/d3d12_debug.cpp b/renderdoc/driver/d3d12/d3d12_debug.cpp index d85474489..cc95e41dd 100644 --- a/renderdoc/driver/d3d12/d3d12_debug.cpp +++ b/renderdoc/driver/d3d12/d3d12_debug.cpp @@ -1427,6 +1427,7 @@ void D3D12Replay::VertexPicking::Init(WrappedID3D12Device *device, D3D12DebugMan void D3D12Replay::VertexPicking::Release() { + SAFE_RELEASE(IB); SAFE_RELEASE(VB); SAFE_RELEASE(ResultBuf); SAFE_RELEASE(RootSig); diff --git a/renderdoc/driver/d3d12/d3d12_replay.cpp b/renderdoc/driver/d3d12/d3d12_replay.cpp index b548b19ed..017911d91 100644 --- a/renderdoc/driver/d3d12/d3d12_replay.cpp +++ b/renderdoc/driver/d3d12/d3d12_replay.cpp @@ -1603,7 +1603,6 @@ uint32_t D3D12Replay::PickVertex(uint32_t eventId, int32_t width, int32_t height } ID3D12Resource *vb = NULL, *ib = NULL; - DXGI_FORMAT ifmt = cfg.position.indexByteStride == 4 ? DXGI_FORMAT_R32_UINT : DXGI_FORMAT_R16_UINT; if(cfg.position.vertexResourceId != ResourceId()) vb = m_pDevice->GetResourceManager()->GetCurrentAs(cfg.position.vertexResourceId); @@ -1613,20 +1612,145 @@ uint32_t D3D12Replay::PickVertex(uint32_t eventId, int32_t width, int32_t height HRESULT hr = S_OK; - // most IB/VBs will not be available as SRVs. So, we copy into our own buffers. - // In the case of VB we also tightly pack and unpack the data. IB can just be - // read as R16 or R32 via the SRV so it is just a straight copy + // most IB/VBs will not be available as SRVs. So, we copy into our own buffers. In the case of VB + // we also tightly pack and unpack the data. IB is upcast to R32 so it we can apply baseVertex + // without risking overflow. + + uint32_t minIndex = 0; + uint32_t maxIndex = cfg.position.numIndices; + + uint32_t idxclamp = 0; + if(cfg.position.baseVertex < 0) + idxclamp = uint32_t(-cfg.position.baseVertex); D3D12_SHADER_RESOURCE_VIEW_DESC sdesc = {}; sdesc.ViewDimension = D3D12_SRV_DIMENSION_BUFFER; sdesc.Shader4ComponentMapping = D3D12_DEFAULT_SHADER_4_COMPONENT_MAPPING; - sdesc.Format = ifmt; + sdesc.Format = DXGI_FORMAT_R32_UINT; if(cfg.position.indexByteStride && ib) { - sdesc.Buffer.FirstElement = cfg.position.indexByteOffset / (cfg.position.indexByteStride); - sdesc.Buffer.NumElements = cfg.position.numIndices; - m_pDevice->CreateShaderResourceView(ib, &sdesc, GetDebugManager()->GetCPUHandle(PICK_IB_SRV)); + // resize up on demand + if(m_VertexPick.IB == NULL || m_VertexPick.IBSize < cfg.position.numIndices * sizeof(uint32_t)) + { + SAFE_RELEASE(m_VertexPick.IB); + + m_VertexPick.IBSize = cfg.position.numIndices * sizeof(uint32_t); + + D3D12_HEAP_PROPERTIES heapProps; + heapProps.Type = D3D12_HEAP_TYPE_UPLOAD; + heapProps.CPUPageProperty = D3D12_CPU_PAGE_PROPERTY_UNKNOWN; + heapProps.MemoryPoolPreference = D3D12_MEMORY_POOL_UNKNOWN; + heapProps.CreationNodeMask = 1; + heapProps.VisibleNodeMask = 1; + + D3D12_RESOURCE_DESC ibDesc; + ibDesc.Alignment = 0; + ibDesc.DepthOrArraySize = 1; + ibDesc.Dimension = D3D12_RESOURCE_DIMENSION_BUFFER; + ibDesc.Flags = D3D12_RESOURCE_FLAG_NONE; + ibDesc.Format = DXGI_FORMAT_UNKNOWN; + ibDesc.Height = 1; + ibDesc.Layout = D3D12_TEXTURE_LAYOUT_ROW_MAJOR; + ibDesc.MipLevels = 1; + ibDesc.SampleDesc.Count = 1; + ibDesc.SampleDesc.Quality = 0; + ibDesc.Width = m_VertexPick.IBSize; + + hr = m_pDevice->CreateCommittedResource(&heapProps, D3D12_HEAP_FLAG_NONE, &ibDesc, + D3D12_RESOURCE_STATE_GENERIC_READ, NULL, + __uuidof(ID3D12Resource), (void **)&m_VertexPick.IB); + + if(FAILED(hr)) + { + RDCERR("Couldn't create pick index buffer: HRESULT: %s", ToStr(hr).c_str()); + return ~0U; + } + + m_VertexPick.IB->SetName(L"m_PickIB"); + + sdesc.Buffer.FirstElement = 0; + sdesc.Buffer.NumElements = cfg.position.numIndices; + m_pDevice->CreateShaderResourceView(m_VertexPick.IB, &sdesc, + GetDebugManager()->GetCPUHandle(PICK_IB_SRV)); + } + + RDCASSERT(cfg.position.indexByteOffset < 0xffffffff); + + if(m_VertexPick.IB) + { + bytebuf idxs; + GetBufferData(cfg.position.indexResourceId, cfg.position.indexByteOffset, 0, idxs); + + std::vector outidxs; + outidxs.resize(cfg.position.numIndices); + + uint16_t *idxs16 = (uint16_t *)&idxs[0]; + uint32_t *idxs32 = (uint32_t *)&idxs[0]; + + if(cfg.position.indexByteStride == 2) + { + size_t bufsize = idxs.size() / 2; + + for(uint32_t i = 0; i < bufsize && i < cfg.position.numIndices; i++) + { + uint32_t idx = idxs16[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + if(i == 0) + { + minIndex = maxIndex = idx; + } + else + { + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + } + + outidxs[i] = idx; + } + } + else + { + uint32_t bufsize = uint32_t(idxs.size() / 4); + + minIndex = maxIndex = idxs32[0]; + + for(uint32_t i = 0; i < RDCMIN(bufsize, cfg.position.numIndices); i++) + { + uint32_t idx = idxs32[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + + outidxs[i] = idx; + } + } + + D3D11_BOX box; + box.top = 0; + box.bottom = 1; + box.front = 0; + box.back = 1; + box.left = 0; + box.right = UINT(outidxs.size() * sizeof(uint32_t)); + + GetDebugManager()->FillBuffer(m_VertexPick.IB, 0, outidxs.data(), + sizeof(uint32_t) * outidxs.size()); + } } else { @@ -1637,92 +1761,81 @@ uint32_t D3D12Replay::PickVertex(uint32_t eventId, int32_t width, int32_t height sdesc.Buffer.FirstElement = 0; sdesc.Format = DXGI_FORMAT_R32G32B32A32_FLOAT; - if(vb) - { - if(m_VertexPick.VB == NULL || m_VertexPick.VBSize < cfg.position.numIndices) - { - SAFE_RELEASE(m_VertexPick.VB); - - m_VertexPick.VBSize = cfg.position.numIndices; - - D3D12_HEAP_PROPERTIES heapProps; - heapProps.Type = D3D12_HEAP_TYPE_UPLOAD; - heapProps.CPUPageProperty = D3D12_CPU_PAGE_PROPERTY_UNKNOWN; - heapProps.MemoryPoolPreference = D3D12_MEMORY_POOL_UNKNOWN; - heapProps.CreationNodeMask = 1; - heapProps.VisibleNodeMask = 1; - - D3D12_RESOURCE_DESC vbDesc; - vbDesc.Alignment = 0; - vbDesc.DepthOrArraySize = 1; - vbDesc.Dimension = D3D12_RESOURCE_DIMENSION_BUFFER; - vbDesc.Flags = D3D12_RESOURCE_FLAG_NONE; - vbDesc.Format = DXGI_FORMAT_UNKNOWN; - vbDesc.Height = 1; - vbDesc.Layout = D3D12_TEXTURE_LAYOUT_ROW_MAJOR; - vbDesc.MipLevels = 1; - vbDesc.SampleDesc.Count = 1; - vbDesc.SampleDesc.Quality = 0; - vbDesc.Width = sizeof(Vec4f) * cfg.position.numIndices; - - hr = m_pDevice->CreateCommittedResource(&heapProps, D3D12_HEAP_FLAG_NONE, &vbDesc, - D3D12_RESOURCE_STATE_GENERIC_READ, NULL, - __uuidof(ID3D12Resource), (void **)&m_VertexPick.VB); - - if(FAILED(hr)) - { - RDCERR("Couldn't create pick vertex buffer: HRESULT: %s", ToStr(hr).c_str()); - return ~0U; - } - - m_VertexPick.VB->SetName(L"m_PickVB"); - - sdesc.Buffer.NumElements = cfg.position.numIndices; - m_pDevice->CreateShaderResourceView(m_VertexPick.VB, &sdesc, - GetDebugManager()->GetCPUHandle(PICK_VB_SRV)); - } - } - else - { - sdesc.Buffer.NumElements = 4; - m_pDevice->CreateShaderResourceView(NULL, &sdesc, GetDebugManager()->GetCPUHandle(PICK_VB_SRV)); - } - // unpack and linearise the data { - FloatVector *vbData = new FloatVector[cfg.position.numIndices]; - bytebuf oldData; GetDebugManager()->GetBufferData(vb, cfg.position.vertexByteOffset, 0, oldData); + // clamp maxIndex to upper bound in case we got invalid indices or primitive restart indices + maxIndex = RDCMIN(maxIndex, uint32_t(oldData.size() / cfg.position.vertexByteStride)); + + if(vb) + { + if(m_VertexPick.VB == NULL || m_VertexPick.VBSize < (maxIndex + 1) * sizeof(Vec4f)) + { + SAFE_RELEASE(m_VertexPick.VB); + + m_VertexPick.VBSize = (maxIndex + 1) * sizeof(Vec4f); + + D3D12_HEAP_PROPERTIES heapProps; + heapProps.Type = D3D12_HEAP_TYPE_UPLOAD; + heapProps.CPUPageProperty = D3D12_CPU_PAGE_PROPERTY_UNKNOWN; + heapProps.MemoryPoolPreference = D3D12_MEMORY_POOL_UNKNOWN; + heapProps.CreationNodeMask = 1; + heapProps.VisibleNodeMask = 1; + + D3D12_RESOURCE_DESC vbDesc; + vbDesc.Alignment = 0; + vbDesc.DepthOrArraySize = 1; + vbDesc.Dimension = D3D12_RESOURCE_DIMENSION_BUFFER; + vbDesc.Flags = D3D12_RESOURCE_FLAG_NONE; + vbDesc.Format = DXGI_FORMAT_UNKNOWN; + vbDesc.Height = 1; + vbDesc.Layout = D3D12_TEXTURE_LAYOUT_ROW_MAJOR; + vbDesc.MipLevels = 1; + vbDesc.SampleDesc.Count = 1; + vbDesc.SampleDesc.Quality = 0; + vbDesc.Width = m_VertexPick.VBSize; + + hr = m_pDevice->CreateCommittedResource(&heapProps, D3D12_HEAP_FLAG_NONE, &vbDesc, + D3D12_RESOURCE_STATE_GENERIC_READ, NULL, + __uuidof(ID3D12Resource), (void **)&m_VertexPick.VB); + + if(FAILED(hr)) + { + RDCERR("Couldn't create pick vertex buffer: HRESULT: %s", ToStr(hr).c_str()); + return ~0U; + } + + m_VertexPick.VB->SetName(L"m_PickVB"); + + sdesc.Buffer.NumElements = (maxIndex + 1); + m_pDevice->CreateShaderResourceView(m_VertexPick.VB, &sdesc, + GetDebugManager()->GetCPUHandle(PICK_VB_SRV)); + } + } + else + { + sdesc.Buffer.NumElements = 4; + m_pDevice->CreateShaderResourceView(NULL, &sdesc, GetDebugManager()->GetCPUHandle(PICK_VB_SRV)); + } + + std::vector vbData; + vbData.resize(maxIndex + 1); + byte *data = &oldData[0]; byte *dataEnd = data + oldData.size(); bool valid = true; - uint32_t idxclamp = 0; - if(cfg.position.baseVertex < 0) - idxclamp = uint32_t(-cfg.position.baseVertex); + // the index buffer may refer to vertices past the start of the vertex buffer, so we can't just + // conver the first N vertices we'll need. + // Instead we grab min and max above, and convert every vertex in that range. This might + // slightly over-estimate but not as bad as 0-max or the whole buffer. + for(uint32_t idx = minIndex; idx <= maxIndex; idx++) + vbData[idx] = HighlightCache::InterpretVertex(data, idx, cfg, dataEnd, valid); - for(uint32_t i = 0; i < cfg.position.numIndices; i++) - { - uint32_t idx = i; - - // apply baseVertex but clamp to 0 (don't allow index to become negative) - if(idx < idxclamp) - idx = 0; - else if(cfg.position.baseVertex < 0) - idx -= idxclamp; - else if(cfg.position.baseVertex > 0) - idx += cfg.position.baseVertex; - - vbData[i] = HighlightCache::InterpretVertex(data, idx, cfg, dataEnd, valid); - } - - GetDebugManager()->FillBuffer(m_VertexPick.VB, 0, vbData, - sizeof(Vec4f) * cfg.position.numIndices); - - delete[] vbData; + GetDebugManager()->FillBuffer(m_VertexPick.VB, 0, vbData.data(), sizeof(Vec4f) * (maxIndex + 1)); } ID3D12GraphicsCommandList *list = m_pDevice->GetNewList(); diff --git a/renderdoc/driver/d3d12/d3d12_replay.h b/renderdoc/driver/d3d12/d3d12_replay.h index 5bf6c955f..6705fd984 100644 --- a/renderdoc/driver/d3d12/d3d12_replay.h +++ b/renderdoc/driver/d3d12/d3d12_replay.h @@ -346,7 +346,9 @@ private: static const uint32_t MaxMeshPicks = 500; ID3D12Resource *VB = NULL; + ID3D12Resource *IB = NULL; uint32_t VBSize = 0; + uint32_t IBSize = 0; ID3D12Resource *ResultBuf = NULL; ID3D12RootSignature *RootSig = NULL; ID3D12PipelineState *Pipe = NULL; diff --git a/renderdoc/driver/gl/gl_debug.cpp b/renderdoc/driver/gl/gl_debug.cpp index 389f7a292..1a91d13c3 100644 --- a/renderdoc/driver/gl/gl_debug.cpp +++ b/renderdoc/driver/gl/gl_debug.cpp @@ -1302,11 +1302,18 @@ uint32_t GLReplay::PickVertex(uint32_t eventId, int32_t width, int32_t height, GLuint ib = 0; + uint32_t minIndex = 0; + uint32_t maxIndex = cfg.position.numIndices; + + uint32_t idxclamp = 0; + if(cfg.position.baseVertex < 0) + idxclamp = uint32_t(-cfg.position.baseVertex); + if(cfg.position.indexByteStride && cfg.position.indexResourceId != ResourceId()) ib = m_pDriver->GetResourceManager()->GetCurrentResource(cfg.position.indexResourceId).name; - // We copy into our own buffers to promote to the target type (uint32) that the - // shader expects. Most IBs will be 16-bit indices, most VBs will not be float4. + // We copy into our own buffers to promote to the target type (uint32) that the shader expects. + // Most IBs will be 16-bit indices, most VBs will not be float4. We also apply baseVertex here if(ib) { @@ -1325,10 +1332,9 @@ uint32_t GLReplay::PickVertex(uint32_t eventId, int32_t width, int32_t height, byte *idxs = new byte[cfg.position.numIndices * cfg.position.indexByteStride]; memset(idxs, 0, cfg.position.numIndices * cfg.position.indexByteStride); - uint32_t *outidxs = NULL; - if(cfg.position.indexByteStride < 4) - outidxs = new uint32_t[cfg.position.numIndices]; + std::vector outidxs; + outidxs.resize(cfg.position.numIndices); gl.glBindBuffer(eGL_COPY_READ_BUFFER, ib); @@ -1340,83 +1346,138 @@ uint32_t GLReplay::PickVertex(uint32_t eventId, int32_t width, int32_t height, cfg.position.numIndices * cfg.position.indexByteStride), idxs); + uint8_t *idxs8 = (uint8_t *)idxs; uint16_t *idxs16 = (uint16_t *)idxs; if(cfg.position.indexByteStride == 1) { for(uint32_t i = 0; i < cfg.position.numIndices; i++) - outidxs[i] = idxs[i]; + { + uint32_t idx = idxs8[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + if(i == 0) + { + minIndex = maxIndex = idx; + } + else + { + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + } + + outidxs[i] = idx; + } gl.glBindBuffer(eGL_SHADER_STORAGE_BUFFER, DebugData.pickIBBuf); gl.glBufferSubData(eGL_SHADER_STORAGE_BUFFER, 0, cfg.position.numIndices * sizeof(uint32_t), - outidxs); + outidxs.data()); } else if(cfg.position.indexByteStride == 2) { for(uint32_t i = 0; i < cfg.position.numIndices; i++) - outidxs[i] = idxs16[i]; + { + uint32_t idx = idxs16[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + if(i == 0) + { + minIndex = maxIndex = idx; + } + else + { + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + } + + outidxs[i] = idx; + } gl.glBindBuffer(eGL_SHADER_STORAGE_BUFFER, DebugData.pickIBBuf); gl.glBufferSubData(eGL_SHADER_STORAGE_BUFFER, 0, cfg.position.numIndices * sizeof(uint32_t), - outidxs); + outidxs.data()); } else { + for(uint32_t i = 0; i < cfg.position.numIndices; i++) + { + uint32_t idx = idxs[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + if(i == 0) + { + minIndex = maxIndex = idx; + } + else + { + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + } + + outidxs[i] = idx; + } + gl.glBindBuffer(eGL_SHADER_STORAGE_BUFFER, DebugData.pickIBBuf); gl.glBufferSubData(eGL_SHADER_STORAGE_BUFFER, 0, cfg.position.numIndices * sizeof(uint32_t), - idxs); + outidxs.data()); } - - SAFE_DELETE_ARRAY(outidxs); - } - - if(DebugData.pickVBBuf == 0 || DebugData.pickVBSize < cfg.position.numIndices * sizeof(Vec4f)) - { - gl.glDeleteBuffers(1, &DebugData.pickVBBuf); - - gl.glGenBuffers(1, &DebugData.pickVBBuf); - gl.glBindBuffer(eGL_SHADER_STORAGE_BUFFER, DebugData.pickVBBuf); - gl.glNamedBufferDataEXT(DebugData.pickVBBuf, cfg.position.numIndices * sizeof(Vec4f), NULL, - eGL_DYNAMIC_DRAW); - - DebugData.pickVBSize = cfg.position.numIndices * sizeof(Vec4f); } // unpack and linearise the data { - FloatVector *vbData = new FloatVector[cfg.position.numIndices]; - bytebuf oldData; GetBufferData(cfg.position.vertexResourceId, cfg.position.vertexByteOffset, 0, oldData); + // clamp maxIndex to upper bound in case we got invalid indices or primitive restart indices + maxIndex = RDCMIN(maxIndex, uint32_t(oldData.size() / cfg.position.vertexByteStride)); + + if(DebugData.pickVBBuf == 0 || DebugData.pickVBSize < (maxIndex + 1) * sizeof(Vec4f)) + { + gl.glDeleteBuffers(1, &DebugData.pickVBBuf); + + gl.glGenBuffers(1, &DebugData.pickVBBuf); + gl.glBindBuffer(eGL_SHADER_STORAGE_BUFFER, DebugData.pickVBBuf); + gl.glNamedBufferDataEXT(DebugData.pickVBBuf, (maxIndex + 1) * sizeof(Vec4f), NULL, + eGL_DYNAMIC_DRAW); + + DebugData.pickVBSize = (maxIndex + 1) * sizeof(Vec4f); + } + + std::vector vbData; + vbData.resize(maxIndex + 1); + byte *data = &oldData[0]; byte *dataEnd = data + oldData.size(); bool valid; - uint32_t idxclamp = 0; - if(cfg.position.baseVertex < 0) - idxclamp = uint32_t(-cfg.position.baseVertex); - - for(uint32_t i = 0; i < cfg.position.numIndices; i++) - { - uint32_t idx = i; - - // apply baseVertex but clamp to 0 (don't allow index to become negative) - if(idx < idxclamp) - idx = 0; - else if(cfg.position.baseVertex < 0) - idx -= idxclamp; - else if(cfg.position.baseVertex > 0) - idx += cfg.position.baseVertex; - - vbData[i] = HighlightCache::InterpretVertex(data, idx, cfg, dataEnd, valid); - } + // the index buffer may refer to vertices past the start of the vertex buffer, so we can't just + // conver the first N vertices we'll need. + // Instead we grab min and max above, and convert every vertex in that range. This might + // slightly over-estimate but not as bad as 0-max or the whole buffer. + for(uint32_t idx = minIndex; idx <= maxIndex; idx++) + vbData[idx] = HighlightCache::InterpretVertex(data, idx, cfg, dataEnd, valid); gl.glBindBuffer(eGL_SHADER_STORAGE_BUFFER, DebugData.pickVBBuf); - gl.glBufferSubData(eGL_SHADER_STORAGE_BUFFER, 0, cfg.position.numIndices * sizeof(Vec4f), vbData); - - delete[] vbData; + gl.glBufferSubData(eGL_SHADER_STORAGE_BUFFER, 0, (maxIndex + 1) * sizeof(Vec4f), vbData.data()); } uint32_t reset[4] = {}; diff --git a/renderdoc/driver/vulkan/vk_debug.cpp b/renderdoc/driver/vulkan/vk_debug.cpp index fb5b08d0d..6dfc7f8bd 100644 --- a/renderdoc/driver/vulkan/vk_debug.cpp +++ b/renderdoc/driver/vulkan/vk_debug.cpp @@ -887,11 +887,18 @@ uint32_t VulkanReplay::PickVertex(uint32_t eventId, int32_t w, int32_t h, const bytebuf idxs; + uint32_t minIndex = 0; + uint32_t maxIndex = cfg.position.numIndices; + if(cfg.position.indexByteStride && cfg.position.indexResourceId != ResourceId()) GetBufferData(cfg.position.indexResourceId, cfg.position.indexByteOffset, 0, idxs); - // We copy into our own buffers to promote to the target type (uint32) that the - // shader expects. Most IBs will be 16-bit indices, most VBs will not be float4. + uint32_t idxclamp = 0; + if(cfg.position.baseVertex < 0) + idxclamp = uint32_t(-cfg.position.baseVertex); + + // We copy into our own buffers to promote to the target type (uint32) that the shader expects. + // Most IBs will be 16-bit indices, most VBs will not be float4. We also apply baseVertex here if(!idxs.empty()) { @@ -918,45 +925,84 @@ uint32_t VulkanReplay::PickVertex(uint32_t eventId, int32_t w, int32_t h, const uint16_t *idxs16 = (uint16_t *)&idxs[0]; uint32_t *idxs32 = (uint32_t *)&idxs[0]; - // if indices are 16-bit, manually upcast them so the shader only - // has to deal with one type if(cfg.position.indexByteStride == 2) { size_t bufsize = idxs.size() / 2; for(uint32_t i = 0; i < bufsize && i < cfg.position.numIndices; i++) - outidxs[i] = idxs16[i]; + { + uint32_t idx = idxs16[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + if(i == 0) + { + minIndex = maxIndex = idx; + } + else + { + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + } + + outidxs[i] = idx; + } } else { - size_t bufsize = idxs.size() / 4; + uint32_t bufsize = uint32_t(idxs.size() / 4); - memcpy(outidxs, idxs32, RDCMIN(bufsize, cfg.position.numIndices * sizeof(uint32_t))); + minIndex = maxIndex = idxs32[0]; + + for(uint32_t i = 0; i < RDCMIN(bufsize, cfg.position.numIndices); i++) + { + uint32_t idx = idxs32[i]; + + if(idx < idxclamp) + idx = 0; + else if(cfg.position.baseVertex < 0) + idx -= idxclamp; + else if(cfg.position.baseVertex > 0) + idx += cfg.position.baseVertex; + + minIndex = RDCMIN(idx, minIndex); + maxIndex = RDCMAX(idx, maxIndex); + + outidxs[i] = idx; + } } m_VertexPick.IBUpload.Unmap(); } - if(m_VertexPick.VBSize < cfg.position.numIndices * sizeof(FloatVector)) - { - if(m_VertexPick.VBSize > 0) - { - m_VertexPick.VB.Destroy(); - m_VertexPick.VBUpload.Destroy(); - } - - m_VertexPick.VBSize = cfg.position.numIndices * sizeof(FloatVector); - - m_VertexPick.VB.Create(m_pDriver, dev, m_VertexPick.VBSize, 1, - GPUBuffer::eGPUBufferGPULocal | GPUBuffer::eGPUBufferSSBO); - m_VertexPick.VBUpload.Create(m_pDriver, dev, m_VertexPick.VBSize, 1, 0); - } - // unpack and linearise the data { bytebuf oldData; GetBufferData(cfg.position.vertexResourceId, cfg.position.vertexByteOffset, 0, oldData); + // clamp maxIndex to upper bound in case we got invalid indices or primitive restart indices + maxIndex = RDCMIN(maxIndex, uint32_t(oldData.size() / cfg.position.vertexByteStride)); + + if(m_VertexPick.VBSize < (maxIndex + 1) * sizeof(FloatVector)) + { + if(m_VertexPick.VBSize > 0) + { + m_VertexPick.VB.Destroy(); + m_VertexPick.VBUpload.Destroy(); + } + + m_VertexPick.VBSize = (maxIndex + 1) * sizeof(FloatVector); + + m_VertexPick.VB.Create(m_pDriver, dev, m_VertexPick.VBSize, 1, + GPUBuffer::eGPUBufferGPULocal | GPUBuffer::eGPUBufferSSBO); + m_VertexPick.VBUpload.Create(m_pDriver, dev, m_VertexPick.VBSize, 1, 0); + } + byte *data = &oldData[0]; byte *dataEnd = data + oldData.size(); @@ -964,24 +1010,12 @@ uint32_t VulkanReplay::PickVertex(uint32_t eventId, int32_t w, int32_t h, const FloatVector *vbData = (FloatVector *)m_VertexPick.VBUpload.Map(); - uint32_t idxclamp = 0; - if(cfg.position.baseVertex < 0) - idxclamp = uint32_t(-cfg.position.baseVertex); - - for(uint32_t i = 0; i < cfg.position.numIndices; i++) - { - uint32_t idx = i; - - // apply baseVertex but clamp to 0 (don't allow index to become negative) - if(idx < idxclamp) - idx = 0; - else if(cfg.position.baseVertex < 0) - idx -= idxclamp; - else if(cfg.position.baseVertex > 0) - idx += cfg.position.baseVertex; - - vbData[i] = HighlightCache::InterpretVertex(data, idx, cfg, dataEnd, valid); - } + // the index buffer may refer to vertices past the start of the vertex buffer, so we can't just + // conver the first N vertices we'll need. + // Instead we grab min and max above, and convert every vertex in that range. This might + // slightly over-estimate but not as bad as 0-max or the whole buffer. + for(uint32_t idx = minIndex; idx <= maxIndex; idx++) + vbData[idx] = HighlightCache::InterpretVertex(data, idx, cfg, dataEnd, valid); m_VertexPick.VBUpload.Unmap(); }