Drop legacy ShaderRegister struct, store byte offset per constant

* For D3D11 byte offsets are always uint32 aligned, but for other APIs that's
  not guaranteed. Storing a byte offset is strictly more expressive and a lot
  simpler to reason about.

qrenderdoc/Windows/PipelineState/PipelineStateViewer.cpp

@@ -570,16 +570,24 @@ void PipelineStateViewer::MakeShaderVariablesHLSL(bool cbufferContents,
 
     struct_contents += lit("\t%1 %2").arg(v.type.descriptor.name).arg(v.name);
 
+    if((v.byteOffset % 4) != 0)
+      qWarning() << "Variable " << QString(v.name) << " is not DWORD aligned";
+
+    uint32_t dwordOffset = v.byteOffset / 4;
+
+    uint32_t vectorIndex = dwordOffset / 4;
+    uint32_t vectorComponent = dwordOffset % 4;
+
     char comp = 'x';
-    if(v.reg.comp == 1)
+    if(vectorComponent == 1)
       comp = 'y';
-    if(v.reg.comp == 2)
+    if(vectorComponent == 2)
       comp = 'z';
-    if(v.reg.comp == 3)
+    if(vectorComponent == 3)
       comp = 'w';
 
     if(cbufferContents)
-      struct_contents += lit(" : packoffset(c%1.%2);").arg(v.reg.vec).arg(QLatin1Char(comp));
+      struct_contents += lit(" : packoffset(c%1.%2);").arg(vectorIndex).arg(QLatin1Char(comp));
     else
       struct_contents += lit(";");
 

renderdoc/api/replay/shader_types.h

@@ -680,30 +680,6 @@ struct ShaderVariableType
 
 DECLARE_REFLECTION_STRUCT(ShaderVariableType);
 
-DOCUMENT("Describes the offset of a constant in memory in terms of 16 byte vectors.");
-struct ShaderRegister
-{
-  DOCUMENT("");
-  ShaderRegister() = default;
-  ShaderRegister(const ShaderRegister &) = default;
-
-  bool operator==(const ShaderRegister &o) const { return vec == o.vec && comp == o.comp; }
-  bool operator<(const ShaderRegister &o) const
-  {
-    if(!(vec == o.vec))
-      return vec < o.vec;
-    if(!(comp == o.comp))
-      return comp < o.comp;
-    return false;
-  }
-  DOCUMENT("The index of the 16 byte vector where this register begins");
-  uint32_t vec;
-  DOCUMENT("The 4 byte component within that vector where this register begins");
-  uint32_t comp;
-};
-
-DECLARE_REFLECTION_STRUCT(ShaderRegister);
-
 DOCUMENT("Contains the detail of a constant within a :class:`ConstantBlock` in memory.");
 struct ShaderConstant
 {
@@ -713,14 +689,15 @@ struct ShaderConstant
 
   bool operator==(const ShaderConstant &o) const
   {
-    return name == o.name && reg == o.reg && defaultValue == o.defaultValue && type == o.type;
+    return name == o.name && byteOffset == o.byteOffset && defaultValue == o.defaultValue &&
+           type == o.type;
   }
   bool operator<(const ShaderConstant &o) const
   {
     if(!(name == o.name))
       return name < o.name;
-    if(!(reg == o.reg))
-      return reg < o.reg;
+    if(!(byteOffset == o.byteOffset))
+      return byteOffset < o.byteOffset;
     if(!(defaultValue == o.defaultValue))
       return defaultValue < o.defaultValue;
     if(!(type == o.type))
@@ -729,10 +706,8 @@ struct ShaderConstant
   }
   DOCUMENT("The name of this constant");
   rdcstr name;
-  DOCUMENT(
-      "A :class:`ShaderRegister` describing where this constant is offset from the start of the "
-      "block");
-  ShaderRegister reg;
+  DOCUMENT("The byte offset of this constant relative to the start of the block");
+  uint32_t byteOffset;
   DOCUMENT("If this constant is no larger than a 64-bit constant, gives a default value for it.");
   uint64_t defaultValue;
   DOCUMENT(

renderdoc/driver/gl/gl_shader_refl.cpp

@@ -33,18 +33,9 @@ void sort(rdcarray<ShaderConstant> &vars)
   if(vars.empty())
     return;
 
-  struct offset_sort
-  {
-    bool operator()(const ShaderConstant &a, const ShaderConstant &b)
-    {
-      if(a.reg.vec == b.reg.vec)
-        return a.reg.comp < b.reg.comp;
-      else
-        return a.reg.vec < b.reg.vec;
-    }
-  };
-
-  std::sort(vars.begin(), vars.end(), offset_sort());
+  std::sort(vars.begin(), vars.end(), [](const ShaderConstant &a, const ShaderConstant &b) {
+    return a.byteOffset < b.byteOffset;
+  });
 
   for(size_t i = 0; i < vars.size(); i++)
     sort(vars[i].type.members);
@@ -706,19 +697,15 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
 
   if(values[5] == -1 && values[2] >= 0)
   {
-    var.reg.vec = values[2];
-    var.reg.comp = 0;
+    var.byteOffset = values[2];
   }
   else if(values[5] >= 0)
   {
-    var.reg.vec = values[5] / 16;
-    var.reg.comp = (values[5] / 4) % 4;
-
-    RDCASSERT((values[5] % 4) == 0);
+    var.byteOffset = values[5];
   }
   else
   {
-    var.reg.vec = var.reg.comp = ~0U;
+    var.byteOffset = ~0U;
   }
 
   var.type.descriptor.rowMajorStorage = (values[6] > 0);
@@ -807,8 +794,7 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
     // construct a parent variable
     ShaderConstant parentVar;
     parentVar.name = base;
-    parentVar.reg.vec = var.reg.vec;
-    parentVar.reg.comp = 0;
+    parentVar.byteOffset = var.byteOffset;
     parentVar.type.descriptor.name = "struct";
     parentVar.type.descriptor.rows = 0;
     parentVar.type.descriptor.columns = 0;
@@ -831,7 +817,7 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
         // just for sorting
         (*parentmembers)[i].type.descriptor.elements =
             RDCMAX((*parentmembers)[i].type.descriptor.elements, parentVar.type.descriptor.elements);
-        (*parentmembers)[i].reg.vec = RDCMIN((*parentmembers)[i].reg.vec, parentVar.reg.vec);
+        (*parentmembers)[i].byteOffset = RDCMIN((*parentmembers)[i].byteOffset, parentVar.byteOffset);
 
         parentmembers = &((*parentmembers)[i].type.members);
         found = true;
@@ -1572,7 +1558,7 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref
           last = &last->type.members.back();
 
         // start from the offset
-        uint32_t stride = last->reg.vec * 16 + last->reg.comp * 4;
+        uint32_t stride = last->byteOffset;
 
         // add its size
         uint32_t size = last->type.descriptor.rows * last->type.descriptor.columns * 4;
@@ -1594,8 +1580,7 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref
 
           ShaderConstant paddingVar;
           paddingVar.name = "__padding";
-          paddingVar.reg.vec = last->reg.vec + (size / 16);
-          paddingVar.reg.comp = (last->reg.comp + size / 4) % 16;
+          paddingVar.byteOffset = last->byteOffset + size;
           paddingVar.type.descriptor.type = VarType::UInt;
           paddingVar.type.descriptor.rows = 1;
           paddingVar.type.descriptor.columns = (uint8_t)RDCMIN(padding, 255U);

renderdoc/driver/shaders/dxbc/dxbc_reflect.cpp

@@ -87,15 +87,14 @@ static ShaderConstant MakeConstantBufferVariable(const DXBC::CBufferVariable &va
   ShaderConstant ret;
 
   ret.name = var.name;
-  ret.reg.vec = offset + var.descriptor.offset / 16;
-  ret.reg.comp = (var.descriptor.offset - (var.descriptor.offset & ~0xf)) / 4;
+  ret.byteOffset = offset + var.descriptor.offset;
   ret.defaultValue = 0;
 
-  offset = ret.reg.vec;
+  offset = ret.byteOffset;
 
   ret.type = MakeShaderVariableType(var.type, offset);
 
-  offset = ret.reg.vec + RDCMAX(1U, var.type.descriptor.bytesize / 16);
+  offset = ret.byteOffset + RDCMAX(1U, var.type.descriptor.bytesize);
 
   return ret;
 }

renderdoc/driver/shaders/spirv/spirv_common.cpp

@@ -75,8 +75,7 @@ void SPIRVFillCBufferVariables(const rdcarray<ShaderConstant> &invars,
     bool rowMajor = invars[v].type.descriptor.rowMajorStorage != 0;
     bool isArray = elems > 1;
 
-    size_t dataOffset =
-        baseOffset + invars[v].reg.vec * sizeof(float) * 4 + invars[v].reg.comp * sizeof(float);
+    size_t dataOffset = baseOffset + invars[v].byteOffset;
 
     if(!invars[v].type.members.empty() || (rows == 0 && cols == 0))
     {
@@ -278,7 +277,7 @@ void FillSpecConstantVariables(const rdcarray<ShaderConstant> &invars,
   {
     for(size_t v = 0; v < invars.size(); v++)
     {
-      if(specInfo[i].specID == invars[v].reg.vec)
+      if(specInfo[i].specID == invars[v].byteOffset)
       {
         memcpy(outvars[v].value.uv, specInfo[i].data.data(),
                RDCMIN(specInfo[i].data.size(), sizeof(outvars[v].value.uv)));

renderdoc/driver/shaders/spirv/spirv_disassemble.cpp

@@ -3503,11 +3503,7 @@ void MakeConstantBlockVariable(ShaderConstant &outConst, SPVTypeData *type, cons
   {
     if(decorations[d].decoration == spv::DecorationOffset)
     {
-      uint32_t byteOffset = decorations[d].val;
-      RDCASSERT(byteOffset % 4 == 0);    // assume uint32_t aligned
-      byteOffset /= 4;
-      outConst.reg.vec = byteOffset / 4;
-      outConst.reg.comp = byteOffset % 4;
+      outConst.byteOffset = decorations[d].val;
       break;
     }
   }
@@ -3655,11 +3651,7 @@ void MakeConstantBlockVariables(SPVTypeData *structType, uint32_t arraySize,
       MakeConstantBlockVariable(cblock[i], structType, StringFormat::Fmt("[%u]", i),
                                 structType->decorations ? *structType->decorations : empty);
 
-      uint32_t byteOffset = relativeOffset;
-      RDCASSERT(byteOffset % 4 == 0);    // assume uint32_t aligned
-      byteOffset /= 4;
-      cblock[i].reg.vec = byteOffset / 4;
-      cblock[i].reg.comp = byteOffset % 4;
+      cblock[i].byteOffset = relativeOffset;
 
       relativeOffset += arrayByteStride;
     }
@@ -3687,7 +3679,7 @@ uint32_t CalculateMinimumByteSize(const rdcarray<ShaderConstant> &variables)
   const ShaderConstant &last = variables.back();
 
   // find its offset
-  uint32_t byteOffset = last.reg.vec * sizeof(Vec4f) + last.reg.comp * sizeof(float);
+  uint32_t byteOffset = last.byteOffset;
 
   // arrays are easy
   if(last.type.descriptor.arrayByteStride > 0)
@@ -4273,11 +4265,9 @@ void SPVModule::MakeReflection(GraphicsAPI sourceAPI, ShaderStage stage, const s
         for(size_t d = 0; d < inst->decorations.size(); d++)
         {
           if(inst->decorations[d].decoration == spv::DecorationLocation)
-            constant.reg.vec = (int32_t)inst->decorations[d].val;
+            constant.byteOffset = (int32_t)inst->decorations[d].val;
         }
 
-        constant.reg.comp = 0;
-
         globalsblock.variables.push_back(constant);
       }
       else if(type->type == SPVTypeData::eStruct)
@@ -4577,7 +4567,7 @@ void SPVModule::MakeReflection(GraphicsAPI sourceAPI, ShaderStage stage, const s
         RDCERR("Couldn't find specialisation index for spec constant");
 
       // put the specId in here since we don't have an actual offset for specialization constants.
-      cblock.variables[i].reg.vec = specId;
+      cblock.variables[i].byteOffset = specId;
       cblock.variables[i].defaultValue = specConstants[i]->constant->u64;
 
       RDCASSERTEQUAL(cblock.variables[i].type.members.size(),

renderdoc/replay/renderdoc_serialise.inl

@@ -194,20 +194,11 @@ void DoSerialise(SerialiserType &ser, ShaderVariableType &el)
   SIZE_CHECK(48);
 }
 
-template <typename SerialiserType>
-void DoSerialise(SerialiserType &ser, ShaderRegister &el)
-{
-  SERIALISE_MEMBER(vec);
-  SERIALISE_MEMBER(comp);
-
-  SIZE_CHECK(8);
-}
-
 template <typename SerialiserType>
 void DoSerialise(SerialiserType &ser, ShaderConstant &el)
 {
   SERIALISE_MEMBER(name);
-  SERIALISE_MEMBER(reg);
+  SERIALISE_MEMBER(byteOffset);
   SERIALISE_MEMBER(defaultValue);
   SERIALISE_MEMBER(type);