Fix mesh picking in cases with index/vertex offsets

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<uint32_t> 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<FloatVector> 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};

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);

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<ID3D12Resource>(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<uint32_t> 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<FloatVector> 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();

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;

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<uint32_t> 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<FloatVector> 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] = {};

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();
   }