Fix shader debugging and pipeline viewer with constant buffer arrays

With SM5.1 and D3D12, constant buffer arrays can be declared which
are treated as an array of resources on the shader side, and a simple
collection of descriptors on the D3D12 side. DXBCDebug::GlobalState
handles the bridge between these by storing each array resource as a
nested ShaderVariable struct. Accessing the data for instructions
traverses the structure similarly. The shader viewer handles nested
resources and displays them in the appropriate tree view. In the
pipeline viewer, descriptors for CB arrays now indicate the array
index and correctly handle buffer offsets for viewing.

qrenderdoc/Windows/ConstantBufferPreviewer.cpp

@@ -144,8 +144,11 @@ void ConstantBufferPreviewer::OnEventChanged(uint32_t eventId)
         setVariables(vars);
         if(wasEmpty)
         {
+          // Expand before resizing so that collapsed data will already be visible when expanded
+          ui->variables->expandAll();
           for(int i = 0; i < 3; i++)
             ui->variables->resizeColumnToContents(i);
+          ui->variables->collapseAll();
         }
         ui->variables->applyExpansion(state, 0);
       });
@@ -169,8 +172,11 @@ void ConstantBufferPreviewer::OnEventChanged(uint32_t eventId)
             setVariables(vars);
             if(wasEmpty)
             {
+              // Expand before resizing so that collapsed data will already be visible when expanded
+              ui->variables->expandAll();
               for(int i = 0; i < 3; i++)
                 ui->variables->resizeColumnToContents(i);
+              ui->variables->collapseAll();
             }
 
             // if we have saved expansion state for the new shader, apply it, otherwise apply the

qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp

@@ -59,21 +59,22 @@ struct D3D12CBufTag
   D3D12CBufTag()
   {
     idx = ~0U;
-    space = reg = 0;
+    space = reg = arrayIdx = 0;
   }
   D3D12CBufTag(uint32_t s, uint32_t r)
   {
     idx = ~0U;
     space = s;
     reg = r;
+    arrayIdx = 0;
   }
   D3D12CBufTag(uint32_t i)
   {
     idx = i;
-    space = reg = 0;
+    space = reg = arrayIdx = 0;
   }
 
-  uint32_t idx, space, reg;
+  uint32_t idx, space, reg, arrayIdx;
 };
 
 Q_DECLARE_METATYPE(D3D12CBufTag);
@@ -1169,7 +1170,9 @@ void D3D12PipelineStateViewer::setShaderState(const D3D12Pipe::Shader &stage, RD
           {
             bind = &bm;
             shaderCBuf = &res;
-            tag = QVariant::fromValue(D3D12CBufTag(i));
+            D3D12CBufTag cbufTag(i);
+            cbufTag.arrayIdx = reg - bm.bind;
+            tag = QVariant::fromValue(cbufTag);
             break;
           }
         }
@@ -1213,6 +1216,9 @@ void D3D12PipelineStateViewer::setShaderState(const D3D12Pipe::Shader &stage, RD
         if(shaderCBuf && !shaderCBuf->name.empty())
           regname += lit(": ") + shaderCBuf->name;
 
+        if(bind != NULL && bind->arraySize > 1)
+          regname += tr("[%1]").arg(reg - bind->bind);
+
         QString sizestr;
         if(bytesize == (uint32_t)length)
           sizestr = tr("%1 Variables, %2 bytes").arg(numvars).arg(length);
@@ -2018,7 +2024,7 @@ void D3D12PipelineStateViewer::cbuffer_itemActivated(RDTreeWidgetItem *item, int
     return;
   }
 
-  IConstantBufferPreviewer *prev = m_Ctx.ViewConstantBuffer(stage->stage, cb.idx, 0);
+  IConstantBufferPreviewer *prev = m_Ctx.ViewConstantBuffer(stage->stage, cb.idx, cb.arrayIdx);
 
   m_Ctx.AddDockWindow(prev->Widget(), DockReference::TransientPopupArea, this, 0.3f);
 }

qrenderdoc/Windows/ShaderViewer.cpp

@@ -1765,13 +1765,17 @@ void ShaderViewer::combineStructures(RDTreeWidgetItem *root, int skipPrefixLengt
       continue;
     }
 
-    // sort the children by offset, then name (in case all offsets are empty)
+    // Sort the children by offset, then global source var index, then by text.
+    // Using the global source var index allows resource arrays to be presented in index order
+    // rather than by name, so for example arr[2] comes before arr[10]
     std::sort(matches.begin(), matches.end(),
               [](const RDTreeWidgetItem *a, const RDTreeWidgetItem *b) {
                 VariableTag at = a->tag().value<VariableTag>();
                 VariableTag bt = b->tag().value<VariableTag>();
                 if(at.offset != bt.offset)
                   return at.offset < bt.offset;
+                if(at.globalSourceVar != bt.globalSourceVar)
+                  return at.globalSourceVar < bt.globalSourceVar;
                 return a->text(0) < b->text(0);
               });
 
@@ -2188,6 +2192,26 @@ void ShaderViewer::updateDebugState()
             node->addChild(child);
           }
         }
+        else
+        {
+          // Check if this is a constant buffer array
+          int arrayCount = m_Trace->constantBlocks[i].members[j].members.count();
+          for(int k = 0; k < arrayCount; k++)
+          {
+            if(m_Trace->constantBlocks[i].members[j].members[k].rows > 0 ||
+               m_Trace->constantBlocks[i].members[j].members[k].columns > 0)
+            {
+              name = m_Trace->constantBlocks[i].members[j].members[k].name;
+              RDTreeWidgetItem *child =
+                  new RDTreeWidgetItem({name, name, lit("Constant"),
+                                        stringRep(m_Trace->constantBlocks[i].members[j].members[k])});
+              node->setTag(QVariant::fromValue(
+                  VariableTag(DebugVariableReference(DebugVariableType::Constant, name))));
+
+              node->addChild(child);
+            }
+          }
+        }
       }
 
       if(node->childCount() == 0)
@@ -2871,13 +2895,22 @@ const ShaderVariable *ShaderViewer::GetRegisterVariable(const DebugVariableRefer
   {
     for(int i = 0; i < m_Trace->constantBlocks.count(); i++)
     {
+      // Root constant
       if(m_Trace->constantBlocks[i].name == r.name)
         return &m_Trace->constantBlocks[i];
 
       for(int j = 0; j < m_Trace->constantBlocks[i].members.count(); j++)
       {
+        // Variable in constant buffer
         if(m_Trace->constantBlocks[i].members[j].name == r.name)
           return &m_Trace->constantBlocks[i].members[j];
+
+        for(int k = 0; k < m_Trace->constantBlocks[i].members[j].members.count(); k++)
+        {
+          // Variable in constant buffer array
+          if(m_Trace->constantBlocks[i].members[j].members[k].name == r.name)
+            return &m_Trace->constantBlocks[i].members[j].members[k];
+        }
       }
     }
 

renderdoc/api/replay/pipestate.h

@@ -141,7 +141,10 @@ public:
 :return: A boolean indicating if binding arrays of resources is supported.
 :rtype: ``bool``
 )");
-  bool SupportsResourceArrays() const { return IsCaptureLoaded() && IsCaptureVK(); }
+  bool SupportsResourceArrays() const
+  {
+    return IsCaptureLoaded() && (IsCaptureVK() || IsCaptureD3D12());
+  }
   DOCUMENT(R"(Determines whether or not the current capture uses explicit barriers.
 
 :return: A boolean indicating if explicit barriers are used.

renderdoc/api/replay/pipestate.inl

@@ -928,10 +928,11 @@ BoundCBuffer PipeState::GetConstantBuffer(ShaderStage stage, uint32_t BufIdx, ui
         if(space == -1)
           return BoundCBuffer();
 
-        if(space >= s.spaces.count() || bind.bind >= s.spaces[space].constantBuffers.count())
+        int32_t shaderReg = bind.bind + (int32_t)ArrayIdx;
+        if(space >= s.spaces.count() || shaderReg >= s.spaces[space].constantBuffers.count())
           return BoundCBuffer();
 
-        const D3D12Pipe::ConstantBuffer &descriptor = s.spaces[space].constantBuffers[bind.bind];
+        const D3D12Pipe::ConstantBuffer &descriptor = s.spaces[space].constantBuffers[shaderReg];
 
         buf = descriptor.resourceId;
         ByteOffset = descriptor.byteOffset;

renderdoc/driver/d3d12/d3d12_shaderdebug.cpp

@@ -1123,9 +1123,13 @@ void GatherConstantBuffers(WrappedID3D12Device *pDevice, const DXBCBytecode::Pro
               ID3D12Resource *pCbvResource =
                   pDevice->GetResourceManager()->GetCurrentAs<ID3D12Resource>(resId);
               cbufData.clear();
-              pDevice->GetDebugManager()->GetBufferData(pCbvResource, element.offset, 0, cbufData);
+
+              pDevice->GetDebugManager()->GetBufferData(pCbvResource, element.offset + byteOffset,
+                                                        0, cbufData);
               AddCBufferToGlobalState(program, global, sourceVars, refl, mapping, slot, cbufData);
             }
+
+            desc++;
           }
         }
       }

renderdoc/driver/shaders/dxbc/dxbc_debug.cpp

@@ -1452,13 +1452,17 @@ ShaderVariable ThreadState::GetSrc(const Operand &oper, const Operation &op, boo
       // For this operand, index 0 is always the logical identifier that we can use to
       // lookup the correct cbuffer. The location of the access entry differs for SM5.1,
       // as index 1 stores the shader register.
+      bool isCBArray = false;
       uint32_t cbIdentifier = indices[0];
+      uint32_t cbArrayIndex = ~0U;
       uint32_t cbLookup = program->IsShaderModel51() ? indices[2] : indices[1];
       for(size_t i = 0; i < reflection->CBuffers.size(); i++)
       {
         if(reflection->CBuffers[i].identifier == cbIdentifier)
         {
           cb = (int)i;
+          cbArrayIndex = indices[1] - reflection->CBuffers[i].reg;
+          isCBArray = reflection->CBuffers[i].bindCount > 1;
           break;
         }
       }
@@ -1468,14 +1472,27 @@ ShaderVariable ThreadState::GetSrc(const Operand &oper, const Operation &op, boo
 
       if(cb >= 0 && cb < global.constantBlocks.count())
       {
-        RDCASSERTMSG("Out of bounds cbuffer lookup",
-                     cbLookup < (uint32_t)global.constantBlocks[cb].members.count(), cbLookup,
-                     global.constantBlocks[cb].members.count());
+        RDCASSERTMSG("Invalid cbuffer array index",
+                     !isCBArray || cbArrayIndex < (uint32_t)global.constantBlocks[cb].members.count(),
+                     isCBArray, cb, global.constantBlocks[cb].members.count());
 
-        if(cbLookup < (uint32_t)global.constantBlocks[cb].members.count())
-          v = s = global.constantBlocks[cb].members[cbLookup];
+        if(!isCBArray || cbArrayIndex < (uint32_t)global.constantBlocks[cb].members.count())
+        {
+          const rdcarray<ShaderVariable> &targetVars =
+              isCBArray ? global.constantBlocks[cb].members[cbArrayIndex].members
+                        : global.constantBlocks[cb].members;
+          RDCASSERTMSG("Out of bounds cbuffer lookup", cbLookup < (uint32_t)targetVars.count(),
+                       cbLookup, targetVars.count());
+
+          if(cbLookup < (uint32_t)targetVars.count())
+            v = s = targetVars[cbLookup];
+          else
+            v = s = ShaderVariable("", 0U, 0U, 0U, 0U);
+        }
         else
+        {
           v = s = ShaderVariable("", 0U, 0U, 0U, 0U);
+        }
       }
       else
       {
@@ -4185,10 +4202,15 @@ void AddCBufferToGlobalState(const DXBCBytecode::Program &program, GlobalState &
   size_t numCBs = mapping.constantBlocks.size();
   for(size_t i = 0; i < numCBs; ++i)
   {
-    if((uint32_t)mapping.constantBlocks[i].bindset == slot.registerSpace &&
-       (uint32_t)mapping.constantBlocks[i].bind == slot.shaderRegister)
+    const Bindpoint &bp = mapping.constantBlocks[i];
+    if(slot.registerSpace == (uint32_t)bp.bindset && slot.shaderRegister >= (uint32_t)bp.bind &&
+       slot.shaderRegister < (uint32_t)(bp.bind + bp.arraySize))
     {
-      RDCASSERTMSG("Reassigning previously filled cbuffer", global.constantBlocks[i].members.empty());
+      uint32_t arrayIndex = slot.shaderRegister - bp.bind;
+      rdcarray<ShaderVariable> &targetVars =
+          bp.arraySize > 1 ? global.constantBlocks[i].members[arrayIndex].members
+                           : global.constantBlocks[i].members;
+      RDCASSERTMSG("Reassigning previously filled cbuffer", targetVars.empty());
 
       uint32_t cbufferIndex = program.IsShaderModel51() ? (uint32_t)i : slot.shaderRegister;
 
@@ -4201,16 +4223,36 @@ void AddCBufferToGlobalState(const DXBCBytecode::Program &program, GlobalState &
           DebugVariableReference(DebugVariableType::Constant, global.constantBlocks[i].name));
       sourceVars.push_back(cbSourceMapping);
 
+      rdcstr identifierPrefix = global.constantBlocks[i].name;
+      rdcstr variablePrefix = refl.constantBlocks[i].name;
+      if(bp.arraySize > 1)
+      {
+        identifierPrefix =
+            StringFormat::Fmt("%s[%u]", global.constantBlocks[i].name.c_str(), arrayIndex);
+        variablePrefix = StringFormat::Fmt("%s[%u]", refl.constantBlocks[i].name.c_str(), arrayIndex);
+
+        // The above sourceVar is for the logical identifier, and FlattenVariables adds the
+        // individual elements of the constant buffer. For CB arrays, add an extra source
+        // var for the CB array index
+        SourceVariableMapping cbArrayMapping;
+        global.constantBlocks[i].members[arrayIndex].name = identifierPrefix;
+        cbArrayMapping.name = variablePrefix;
+        cbArrayMapping.variables.push_back(
+            DebugVariableReference(DebugVariableType::Constant, identifierPrefix));
+        sourceVars.push_back(cbArrayMapping);
+      }
+      const rdcarray<ShaderConstant> &constants =
+          (bp.arraySize > 1) ? refl.constantBlocks[i].variables[0].type.members
+                             : refl.constantBlocks[i].variables;
+
       rdcarray<ShaderVariable> vars;
-      StandardFillCBufferVariables(refl.resourceId, refl.constantBlocks[i].variables, vars, cbufData);
-      FlattenVariables(global.constantBlocks[i].name, refl.constantBlocks[i].variables, vars,
-                       global.constantBlocks[i].members, refl.constantBlocks[i].name + ".", 0,
+      StandardFillCBufferVariables(refl.resourceId, constants, vars, cbufData);
+      FlattenVariables(identifierPrefix, constants, vars, targetVars, variablePrefix + ".", 0,
                        sourceVars);
 
-      for(size_t c = 0; c < global.constantBlocks[i].members.size(); c++)
+      for(size_t c = 0; c < targetVars.size(); c++)
       {
-        global.constantBlocks[i].members[c].name =
-            StringFormat::Fmt("%s[%u]", global.constantBlocks[i].name.c_str(), (uint32_t)c);
+        targetVars[c].name = StringFormat::Fmt("%s[%u]", identifierPrefix.c_str(), (uint32_t)c);
       }
 
       return;
@@ -5003,7 +5045,17 @@ ShaderDebugTrace *InterpretDebugger::BeginDebug(const DXBC::DXBCContainer *dxbcC
   }
 
   // Set the number of constant buffers in the trace, but assignment happens later
-  global.constantBlocks.resize(refl.constantBlocks.size());
+  size_t numCBuffers = dxbc->GetReflection()->CBuffers.size();
+  global.constantBlocks.resize(numCBuffers);
+  for(size_t i = 0; i < numCBuffers; ++i)
+  {
+    uint32_t bindCount = dxbc->GetReflection()->CBuffers[i].bindCount;
+    if(bindCount > 1)
+    {
+      // With a CB array, we use a nesting structure for the trace
+      global.constantBlocks[i].members.resize(bindCount);
+    }
+  }
 
   struct ResList
   {

renderdoc/driver/shaders/dxbc/dxbc_reflect.cpp

@@ -269,7 +269,7 @@ void MakeShaderReflection(DXBC::DXBCContainer *dxbc, ShaderReflection *refl,
     cb.bindPoint = (int32_t)i;
 
     Bindpoint map;
-    map.arraySize = 1;
+    map.arraySize = dxbc->GetReflection()->CBuffers[i].bindCount;
     map.bindset = dxbc->GetReflection()->CBuffers[i].space;
     map.bind = dxbc->GetReflection()->CBuffers[i].reg;
     map.used = true;