Improve consistency in shader constant reflection

* This reports the struct vartype now, ensures that struct sizes are always
  reported and alignments are respected, along with some other fixes to more
  accurately report memory layouts of structs.

renderdoc/data/glsl_shaders.cpp

@@ -1011,8 +1011,8 @@ void main() {
             INFO("UBO member: " << member.name.c_str());
 
             CHECK(member.byteOffset == 192);
-            // this doesn't reflect in native introspection, so we skip it
-            // CHECK(member.type.descriptor.elements == 3);
+            CHECK(member.type.descriptor.type == VarType::Struct);
+            CHECK(member.type.descriptor.arrayByteStride == 48);
 
             REQUIRE_ARRAY_SIZE(member.type.members.size(), 3);
             {
@@ -1257,9 +1257,9 @@ void main() {
             INFO("SSBO member: " << member.name.c_str());
 
             CHECK(member.byteOffset == 40);
-            // this doesn't reflect in native introspection, so we skip it
-            // CHECK(member.type.descriptor.elements == 3);
+            CHECK(member.type.descriptor.type == VarType::Struct);
             CHECK(member.type.descriptor.arrayByteStride == 24);
+            CHECK(member.type.descriptor.elements == 3);
 
             REQUIRE_ARRAY_SIZE(member.type.members.size(), 3);
             {
@@ -1335,6 +1335,7 @@ void main() {
             INFO("SSBO member: " << member.name.c_str());
 
             CHECK(member.byteOffset == 0);
+            CHECK(member.type.descriptor.type == VarType::Struct);
             CHECK(member.type.descriptor.arrayByteStride == 48);
             CHECK(member.type.descriptor.elements == ~0U);
 
@@ -1346,6 +1347,8 @@ void main() {
                 INFO("SSBO submember: " << submember.name.c_str());
 
                 CHECK(submember.byteOffset == 0);
+                CHECK(submember.type.descriptor.type == VarType::Struct);
+                CHECK(submember.type.descriptor.arrayByteStride == 24);
 
                 REQUIRE_ARRAY_SIZE(submember.type.members.size(), 3);
                 {
@@ -1396,6 +1399,8 @@ void main() {
                 INFO("SSBO submember: " << submember.name.c_str());
 
                 CHECK(submember.byteOffset == 24);
+                CHECK(submember.type.descriptor.type == VarType::Struct);
+                CHECK(submember.type.descriptor.arrayByteStride == 24);
 
                 REQUIRE_ARRAY_SIZE(submember.type.members.size(), 3);
                 {

renderdoc/driver/gl/gl_replay.cpp

@@ -2101,7 +2101,7 @@ void GLReplay::OpenGLFillCBufferVariables(ResourceId shader, GLuint prog, bool b
 
     if(!variables[i].type.members.empty())
     {
-      if(desc.elements == 0)
+      if(desc.elements <= 1)
       {
         OpenGLFillCBufferVariables(shader, prog, bufferBacked, prefix + var.name.c_str() + ".",
                                    variables[i].type.members, var.members, data);
@@ -2151,7 +2151,7 @@ void GLReplay::OpenGLFillCBufferVariables(ResourceId shader, GLuint prog, bool b
           RDCERR("Uniform is buffer backed - index expected");
 
         // if this is an array, generate empty members
-        if(desc.elements > 0)
+        if(desc.elements > 1)
         {
           rdcarray<ShaderVariable> elems;
           for(uint32_t a = 0; a < desc.elements; a++)
@@ -2185,7 +2185,7 @@ void GLReplay::OpenGLFillCBufferVariables(ResourceId shader, GLuint prog, bool b
         if(!bufferBacked)
           offset = 0;
 
-        if(desc.elements == 0)
+        if(desc.elements <= 1)
         {
           if(!bufferBacked)
           {

renderdoc/driver/gl/gl_shader_refl.cpp

@@ -804,13 +804,17 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
   if(var.name[c - 3] == '[' && var.name[c - 2] == '0' && var.name[c - 1] == ']')
     var.name.resize(c - 3);
   else
-    var.type.descriptor.elements = 0;
+    var.type.descriptor.elements = 1;
 
   GLint topLevelStride = 0;
   if(query == eGL_BUFFER_VARIABLE)
   {
     GLenum propName = eGL_TOP_LEVEL_ARRAY_STRIDE;
     GL.glGetProgramResourceiv(sepProg, query, varIdx, 1, &propName, 1, NULL, &topLevelStride);
+
+    // if ARRAY_SIZE is 0 this is an unbounded array
+    if(values[4] == 0)
+      var.type.descriptor.elements = ~0U;
   }
 
   rdcarray<ShaderConstant> *parentmembers = defaultBlock;
@@ -832,6 +836,7 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
   int arrayIdx = 0;
 
   bool blockLevel = true;
+  int level = 0;
 
   // reverse figure out structures and structure arrays
   while(strchr(nm, '.') || strchr(nm, '['))
@@ -883,21 +888,35 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
     parentVar.type.descriptor.name = "struct";
     parentVar.type.descriptor.rows = 0;
     parentVar.type.descriptor.columns = 0;
-    parentVar.type.descriptor.type = var.type.descriptor.type;
+    parentVar.type.descriptor.type = VarType::Struct;
     parentVar.type.descriptor.elements =
-        isarray && !multiDimArray ? RDCMAX(1U, uint32_t(arrayIdx + 1)) : 0;
+        isarray && !multiDimArray ? RDCMAX(1U, uint32_t(arrayIdx + 1)) : 1;
     parentVar.type.descriptor.matrixByteStride = 0;
 
     RDCASSERTMSG("Stride is too large for uint16_t", topLevelStride <= 0xffff);
     parentVar.type.descriptor.arrayByteStride = RDCMIN((uint32_t)topLevelStride, 0xffffu) & 0xffff;
 
     // consider all block-level SSBO structs to have infinite elements if they are an array at all
+    // for structs that aren't the last struct in a block which can't be infinite, this will be
+    // fixup'd later by looking at the offset of subsequent elements
     if(blockLevel && topLevelStride && isarray)
       parentVar.type.descriptor.elements = ~0U;
 
     if(!blockLevel)
       topLevelStride = 0;
 
+    // this is no longer block level after the first array, or the first struct member.
+    //
+    // this logic is because whether or not a block has a name affects what comes back. E.g.
+    // buffer ssbo { float ssbo_foo } ; will just be "ssbo_foo", whereas
+    // buffer ssbo { float ssbo_foo } root; will be "root.ssbo_foo"
+    //
+    // we only use blocklevel for the check above, to see if we are on a block-level array to mark
+    // it as unknown size (to be fixed later), so this check can be a little fuzzy as long as it
+    // doesn't have false positives.
+    if(isarray || level >= 1)
+      blockLevel = false;
+
     bool found = false;
 
     // if we can find the base variable already, we recurse into its members
@@ -915,8 +934,6 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
         parentmembers = &((*parentmembers)[i].type.members);
         found = true;
 
-        blockLevel = false;
-
         break;
       }
     }
@@ -996,6 +1013,8 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
       parentmembers = NULL;
       break;
     }
+
+    level++;
   }
 
   if(parentmembers)
@@ -1045,13 +1064,108 @@ void ReconstructVarTree(GLenum query, GLuint sepProg, GLuint varIdx, GLint numPa
   }
 }
 
-void MakeChildByteOffsetsRelative(ShaderConstant &member)
+static uint32_t GetVarAlignment(bool std140, const ShaderConstant &c)
+{
+  if(!c.type.members.empty())
+  {
+    uint32_t ret = 4;
+    for(const ShaderConstant &m : c.type.members)
+      ret = RDCMAX(ret, GetVarAlignment(std140, m));
+
+    if(std140)
+      ret = AlignUp16(ret);
+    return ret;
+  }
+
+  uint8_t vecSize = c.type.descriptor.columns;
+
+  if(c.type.descriptor.rows > 1 && c.type.descriptor.ColMajor())
+    vecSize = c.type.descriptor.rows;
+
+  if(vecSize <= 1)
+    return 4;
+  if(vecSize == 2)
+    return 8;
+  return 16;
+}
+
+static uint32_t GetVarArrayStride(bool std140, const ShaderConstant &c)
+{
+  uint32_t stride;
+  if(!c.type.members.empty())
+  {
+    const ShaderConstant &lastChild = c.type.members.back();
+    stride = GetVarArrayStride(std140, lastChild);
+    if(lastChild.type.descriptor.elements > 1 && lastChild.type.descriptor.elements != ~0U)
+      stride *= lastChild.type.descriptor.elements;
+    stride = AlignUp(lastChild.byteOffset + stride, GetVarAlignment(std140, c));
+  }
+  else
+  {
+    if(c.type.descriptor.elements > 1)
+    {
+      stride = c.type.descriptor.arrayByteStride;
+    }
+    else
+    {
+      stride = VarTypeByteSize(c.type.descriptor.type);
+
+      if(c.type.descriptor.rows > 1)
+      {
+        if(std140)
+        {
+          stride *= 4;
+
+          if(c.type.descriptor.ColMajor())
+            stride *= RDCMAX((uint8_t)1, c.type.descriptor.columns);
+          else
+            stride *= RDCMAX((uint8_t)1, c.type.descriptor.rows);
+        }
+        else
+        {
+          if(c.type.descriptor.ColMajor())
+          {
+            stride *= RDCMAX((uint8_t)1, c.type.descriptor.columns);
+            if(c.type.descriptor.rows == 3)
+              stride *= 4;
+            else
+              stride *= RDCMAX((uint8_t)1, c.type.descriptor.rows);
+          }
+          else
+          {
+            stride *= RDCMAX((uint8_t)1, c.type.descriptor.rows);
+            if(c.type.descriptor.columns == 3)
+              stride *= 4;
+            else
+              stride *= RDCMAX((uint8_t)1, c.type.descriptor.columns);
+          }
+        }
+      }
+      else
+      {
+        if(c.type.descriptor.columns == 3 && std140)
+          stride *= 4;
+        else
+          stride *= RDCMAX((uint8_t)1, c.type.descriptor.columns);
+      }
+    }
+  }
+
+  return stride;
+}
+
+void FixupStructOffsetsAndSize(bool std140, ShaderConstant &member)
 {
   for(ShaderConstant &child : member.type.members)
   {
-    MakeChildByteOffsetsRelative(child);
+    FixupStructOffsetsAndSize(std140, child);
     child.byteOffset -= member.byteOffset;
   }
+
+  if(!member.type.members.empty())
+  {
+    member.type.descriptor.arrayByteStride = GetVarArrayStride(std140, member);
+  }
 }
 
 int ParseVersionStatement(const char *version)
@@ -1155,7 +1269,7 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref
     res.isTexture = true;
     res.variableType.descriptor.rows = 1;
     res.variableType.descriptor.columns = 4;
-    res.variableType.descriptor.elements = 0;
+    res.variableType.descriptor.elements = 1;
     res.variableType.descriptor.arrayByteStride = 0;
     res.variableType.descriptor.matrixByteStride = 0;
 
@@ -1700,7 +1814,7 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref
       res.resType = TextureType::Buffer;
       res.variableType.descriptor.rows = 0;
       res.variableType.descriptor.columns = 0;
-      res.variableType.descriptor.elements = 0;
+      res.variableType.descriptor.elements = 1;
       res.variableType.descriptor.arrayByteStride = 0;
       res.variableType.descriptor.matrixByteStride = 0;
       res.variableType.descriptor.name = "buffer";
@@ -1801,6 +1915,11 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref
     for(size_t ssbo = 0; ssbo < ssbos.size(); ssbo++)
     {
       rdcarray<ShaderConstant> &ssboVars = rwresources[ssbos[ssbo]].variableType.members;
+
+      // can't make perfect guesses of struct alignment but assume std430 for ssbos
+      for(ShaderConstant &member : ssboVars)
+        FixupStructOffsetsAndSize(false, member);
+
       for(size_t rootMember = 0; rootMember + 1 < ssboVars.size(); rootMember++)
       {
         ShaderConstant &member = ssboVars[rootMember];
@@ -1808,14 +1927,14 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref
         const uint32_t memberSizeBound = ssboVars[rootMember + 1].byteOffset - member.byteOffset;
         const uint32_t stride = member.type.descriptor.arrayByteStride;
 
-        if(stride != 0 && member.type.descriptor.elements <= 1 && memberSizeBound > 2 * stride)
+        if(stride != 0 && member.type.descriptor.elements == ~0U)
         {
-          member.type.descriptor.elements = memberSizeBound / stride;
+          if(memberSizeBound >= 2 * stride)
+            member.type.descriptor.elements = memberSizeBound / stride;
+          else
+            member.type.descriptor.elements = 1;
         }
       }
-
-      for(ShaderConstant &member : ssboVars)
-        MakeChildByteOffsetsRelative(member);
     }
 
     delete[] members;
@@ -1870,8 +1989,9 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref
 
         sort(ubos[i]);
 
+        // can't make perfect guesses of struct alignment but assume std140 for ubos
         for(ShaderConstant &member : ubos[i])
-          MakeChildByteOffsetsRelative(member);
+          FixupStructOffsetsAndSize(true, member);
 
         std::swap(cblock.variables, ubos[i]);
 

renderdoc/driver/shaders/dxbc/dxbc_container.cpp

@@ -320,11 +320,6 @@ rdcstr TypeName(CBufferVariableType::Descriptor desc)
     if(desc.rows > 1)
     {
       ret = StringFormat::Fmt("%s%dx%d", type, desc.rows, desc.cols);
-
-      if(desc.varClass == CLASS_MATRIX_ROWS)
-      {
-        ret = "row_major " + ret;
-      }
     }
     else if(desc.cols > 1)
     {

renderdoc/driver/shaders/dxbc/dxbc_reflect.cpp

@@ -31,6 +31,22 @@
 static ShaderConstant MakeConstantBufferVariable(bool cbufferPacking,
                                                  const DXBC::CBufferVariable &var);
 
+static void FixupEmptyStructs(rdcarray<ShaderConstant> &members)
+{
+  for(size_t i = 0; i < members.size(); i++)
+  {
+    if(members[i].byteOffset == ~0U)
+    {
+      // don't try to calculate offset for trailing empty structs, just delete them
+      if(i == members.size() - 1)
+        members.pop_back();
+      // other empty struct members take the offset of the next member
+      else
+        members[i].byteOffset = members[i + 1].byteOffset;
+    }
+  }
+}
+
 static ShaderConstantType MakeShaderConstantType(bool cbufferPacking, DXBC::CBufferVariableType type)
 {
   ShaderConstantType ret;
@@ -56,7 +72,26 @@ static ShaderConstantType MakeShaderConstantType(bool cbufferPacking, DXBC::CBuf
 
   if(type.descriptor.varClass == DXBC::CLASS_STRUCT)
   {
+    // fxc's reported byte size for UAV structs is not reliable (booo). It seems to assume some
+    // padding that doesn't exist, especially in arrays. So e.g.:
+    // struct nested_with_padding
+    // {
+    //   float a; float4 b; float c; float3 d[4];
+    // };
+    // is tightly packed and only contains 1+4+1+4*3 floats = 18*4 = 72 bytes
+    // however an array of nested_with_padding foo[2] will have size listed as 176 bytes.
+    //
+    // fortunately, since packing is tight we can look at the 'columns' field which is the number of
+    // floats in the struct, and multiply that
     uint32_t stride = type.descriptor.bytesize / RDCMAX(1U, type.descriptor.elements);
+    if(!cbufferPacking)
+    {
+      stride = type.descriptor.cols * sizeof(float);
+      // the exception is empty structs have 1 cols, probably because of a max(1,...) somewhere
+      if(type.descriptor.bytesize == 0)
+        stride = 0;
+    }
+
     RDCASSERTMSG("Stride is too large for uint16_t", stride <= 0xffff);
 
     ret.descriptor.arrayByteStride = RDCMIN(stride, 0xffffu) & 0xffff;
@@ -65,6 +100,8 @@ static ShaderConstantType MakeShaderConstantType(bool cbufferPacking, DXBC::CBuf
       ret.descriptor.arrayByteStride = AlignUp16(ret.descriptor.arrayByteStride);
 
     ret.descriptor.rows = ret.descriptor.columns = 0;
+
+    ret.descriptor.type = VarType::Struct;
   }
   else
   {
@@ -78,6 +115,8 @@ static ShaderConstantType MakeShaderConstantType(bool cbufferPacking, DXBC::CBuf
   for(size_t i = 0; i < type.members.size(); i++)
     ret.members.push_back(MakeConstantBufferVariable(cbufferPacking, type.members[i]));
 
+  FixupEmptyStructs(ret.members);
+
   if(!ret.members.empty())
   {
     ret.descriptor.rows = 0;
@@ -96,6 +135,12 @@ static ShaderConstant MakeConstantBufferVariable(bool cbufferPacking, const DXBC
   ret.defaultValue = 0;
   ret.type = MakeShaderConstantType(cbufferPacking, var.type);
 
+  // fxc emits negative values for offsets of empty structs sometimes. Replace that with a single
+  // value so we can say 'use the previous value'
+  if(ret.type.descriptor.type == VarType::Struct && ret.type.members.empty() &&
+     ret.byteOffset > 0xF0000000)
+    ret.byteOffset = ~0U;
+
   return ret;
 }
 
@@ -337,6 +382,8 @@ void MakeShaderReflection(DXBC::DXBCContainer *dxbc, ShaderReflection *refl,
       cb.variables.push_back(
           MakeConstantBufferVariable(true, dxbc->GetReflection()->CBuffers[i].variables[v]));
     }
+
+    FixupEmptyStructs(cb.variables);
   }
 
   mapping->samplers.resize(dxbc->GetReflection()->Samplers.size());

renderdoc/driver/shaders/spirv/spirv_reflect.cpp

@@ -314,6 +314,117 @@ static bool HasCommandLineInModuleProcessed(rdcspv::Generator gen)
           gen == rdcspv::Generator::ShadercoverGlslang);
 }
 
+struct StructSizes
+{
+  uint32_t scalarAlign = 1;
+  uint32_t baseAlign = 1;
+  uint32_t extendedAlign = 1;
+
+  uint32_t scalarSize = 0;
+  uint32_t baseSize = 0;
+  uint32_t extendedSize = 0;
+};
+
+StructSizes CalculateStructProps(uint32_t emptyStructSize, const ShaderConstant &c)
+{
+  StructSizes ret;
+
+  if(c.type.descriptor.type != VarType::Struct)
+  {
+    // A scalar of size N has a scalar alignment of N.
+    // A vector or matrix type has a scalar alignment equal to that of its component type.
+    // An array type has a scalar alignment equal to that of its element type.
+    ret.scalarAlign = VarTypeByteSize(c.type.descriptor.type);
+
+    // A scalar has a base alignment equal to its scalar alignment.
+    ret.baseAlign = ret.scalarAlign;
+
+    // A row-major matrix of C columns has a base alignment equal to the base alignment of a vector
+    // of C matrix components.
+    uint8_t vecSize = c.type.descriptor.columns;
+    uint8_t matSize = c.type.descriptor.rows;
+
+    // A column-major matrix has a base alignment equal to the base alignment of the matrix column
+    // type.
+    if(c.type.descriptor.rows > 1 && c.type.descriptor.ColMajor())
+    {
+      vecSize = c.type.descriptor.rows;
+      matSize = c.type.descriptor.columns;
+    }
+
+    // A two-component vector has a base alignment equal to twice its scalar alignment.
+    if(vecSize == 2)
+      ret.baseAlign *= 2;
+    // A three- or four-component vector has a base alignment equal to four times its scalar
+    // alignment.
+    else if(vecSize == 3 || vecSize == 4)
+      ret.baseAlign *= 4;
+
+    // An array has a base alignment equal to the base alignment of its element type.
+    // N/A
+
+    // A scalar, vector or matrix type has an extended alignment equal to its base alignment.
+    ret.extendedAlign = ret.baseAlign;
+
+    // An array or structure type has an extended alignment equal to the largest extended alignment
+    // of any of its members, rounded up to a multiple of 16.
+    if(c.type.descriptor.elements > 1)
+      ret.extendedAlign = AlignUp16(ret.extendedAlign);
+
+    if(matSize > 1)
+      ret.extendedAlign = ret.baseAlign = c.type.descriptor.matrixByteStride;
+
+    ret.scalarSize = ret.scalarAlign * RDCMAX(c.type.descriptor.rows, (uint8_t)1) *
+                     RDCMAX(c.type.descriptor.columns, (uint8_t)1) *
+                     RDCMAX(c.type.descriptor.elements, 1U);
+    ret.baseSize = ret.baseAlign * matSize * RDCMAX(c.type.descriptor.elements, 1U);
+    ret.extendedSize = ret.extendedAlign * matSize * RDCMAX(c.type.descriptor.elements, 1U);
+  }
+  else
+  {
+    for(size_t i = 0; i < c.type.members.size(); i++)
+    {
+      const ShaderConstant &m = c.type.members[i];
+
+      StructSizes member = CalculateStructProps(emptyStructSize, m);
+      // A structure has a scalar alignment equal to the largest scalar alignment of any of its
+      // members.
+      ret.scalarAlign = RDCMAX(ret.scalarAlign, member.scalarAlign);
+      // A structure has a base alignment equal to the largest base alignment of any of its members.
+      ret.baseAlign = RDCMAX(ret.baseAlign, member.baseAlign);
+      // An array or structure type has an extended alignment equal to the largest extended
+      // alignment of any of its members, rounded up to a multiple of 16.
+      ret.extendedAlign = RDCMAX(ret.baseAlign, member.extendedAlign);
+
+      if(i + 1 == c.type.members.size())
+      {
+        ret.scalarSize = AlignUp(m.byteOffset + member.scalarSize, ret.scalarAlign);
+        ret.baseSize = AlignUp(m.byteOffset + member.baseSize, ret.baseAlign);
+        ret.extendedSize = AlignUp16(m.byteOffset + member.extendedSize);
+      }
+    }
+
+    ret.extendedAlign = AlignUp16(ret.extendedAlign);
+
+    // A structure has a base alignment equal to the largest base alignment of any of its members.
+    // An empty structure has a base alignment equal to the size of the smallest scalar type
+    // permitted by the capabilities declared in the SPIR-V module. (e.g., for a 1 byte aligned
+    // empty struct in the StorageBuffer storage class, StorageBuffer8BitAccess or
+    // UniformAndStorageBuffer8BitAccess must be declared in the SPIR-V module.)
+    if(c.type.members.empty())
+    {
+      ret.scalarSize = 0;
+      ret.scalarAlign = emptyStructSize;
+      ret.baseSize = emptyStructSize;
+      ret.baseAlign = emptyStructSize;
+      ret.extendedSize = AlignUp16(emptyStructSize);
+      ret.extendedAlign = AlignUp16(emptyStructSize);
+    }
+  }
+
+  return ret;
+}
+
 namespace rdcspv
 {
 Reflector::Reflector()
@@ -951,6 +1062,10 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st
       const bool pushConst = (global.storage == StorageClass::PushConstant);
       const bool atomicCounter = (global.storage == StorageClass::AtomicCounter);
 
+      rdcspv::StorageClass effectiveStorage = global.storage;
+      if(ssbo)
+        effectiveStorage = StorageClass::StorageBuffer;
+
       Bindpoint bindmap;
       // set something crazy so this doesn't overlap with a real buffer binding
       if(pushConst)
@@ -1076,8 +1191,8 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st
 
           ShaderConstant constant;
 
-          MakeConstantBlockVariable(constant, pointerTypes, *varType, strings[global.id],
-                                    decorations[global.id], specInfo);
+          MakeConstantBlockVariable(constant, pointerTypes, effectiveStorage, *varType,
+                                    strings[global.id], decorations[global.id], specInfo);
 
           if(isArray)
             constant.type.descriptor.elements = arraySize;
@@ -1112,8 +1227,8 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st
             res.variableType.descriptor.type = VarType::Float;
             res.variableType.descriptor.name = varType->name;
 
-            MakeConstantBlockVariables(*varType, 0, 0, res.variableType.members, pointerTypes,
-                                       specInfo);
+            MakeConstantBlockVariables(effectiveStorage, *varType, 0, 0, res.variableType.members,
+                                       pointerTypes, specInfo);
 
             rwresources.push_back(shaderrespair(bindmap, res));
           }
@@ -1126,7 +1241,8 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st
               cblock.name = StringFormat::Fmt("uniforms%u", global.id.value());
             cblock.bufferBacked = !pushConst;
 
-            MakeConstantBlockVariables(*varType, 0, 0, cblock.variables, pointerTypes, specInfo);
+            MakeConstantBlockVariables(effectiveStorage, *varType, 0, 0, cblock.variables,
+                                       pointerTypes, specInfo);
 
             if(!varType->children.empty())
               cblock.byteSize = CalculateMinimumByteSize(cblock.variables);
@@ -1160,8 +1276,8 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st
         name = StringFormat::Fmt("specID%u", decorations[c.id].specID);
 
       ShaderConstant spec;
-      MakeConstantBlockVariable(spec, pointerTypes, dataTypes[c.type], name, decorations[c.id],
-                                specInfo);
+      MakeConstantBlockVariable(spec, pointerTypes, rdcspv::StorageClass::PushConstant,
+                                dataTypes[c.type], name, decorations[c.id], specInfo);
       spec.byteOffset = uint32_t(specblock.variables.size() * sizeof(uint64_t));
       spec.defaultValue = c.value.value.u64v[0];
       specblock.variables.push_back(spec);
@@ -1388,8 +1504,8 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st
   {
     ShaderConstant dummy;
 
-    MakeConstantBlockVariable(dummy, pointerTypes, dataTypes[it->first], rdcstr(), Decorations(),
-                              specInfo);
+    MakeConstantBlockVariable(dummy, pointerTypes, dataTypes[it->first].pointerType.storage,
+                              dataTypes[it->first], rdcstr(), Decorations(), specInfo);
 
     if(it->second >= reflection.pointerTypes.size())
       reflection.pointerTypes.resize(it->second + 1);
@@ -1398,8 +1514,9 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st
   }
 }
 
-void Reflector::MakeConstantBlockVariables(const DataType &structType, uint32_t arraySize,
-                                           uint32_t arrayByteStride, rdcarray<ShaderConstant> &cblock,
+void Reflector::MakeConstantBlockVariables(rdcspv::StorageClass storage, const DataType &structType,
+                                           uint32_t arraySize, uint32_t arrayByteStride,
+                                           rdcarray<ShaderConstant> &cblock,
                                            SparseIdMap<uint16_t> &pointerTypes,
                                            const rdcarray<SpecConstant> &specInfo) const
 {
@@ -1414,8 +1531,8 @@ void Reflector::MakeConstantBlockVariables(const DataType &structType, uint32_t
     cblock.resize(arraySize);
     for(uint32_t i = 0; i < arraySize; i++)
     {
-      MakeConstantBlockVariable(cblock[i], pointerTypes, structType, StringFormat::Fmt("[%u]", i),
-                                decorations[structType.id], specInfo);
+      MakeConstantBlockVariable(cblock[i], pointerTypes, storage, structType,
+                                StringFormat::Fmt("[%u]", i), decorations[structType.id], specInfo);
 
       cblock[i].byteOffset = relativeOffset;
 
@@ -1430,13 +1547,90 @@ void Reflector::MakeConstantBlockVariables(const DataType &structType, uint32_t
 
   cblock.resize(structType.children.size());
   for(size_t i = 0; i < structType.children.size(); i++)
-    MakeConstantBlockVariable(cblock[i], pointerTypes, dataTypes[structType.children[i].type],
-                              structType.children[i].name, structType.children[i].decorations,
-                              specInfo);
+  {
+    MakeConstantBlockVariable(cblock[i], pointerTypes, storage,
+                              dataTypes[structType.children[i].type], structType.children[i].name,
+                              structType.children[i].decorations, specInfo);
+  }
+
+  uint32_t emptyStructSize = 4;
+
+  if(storage == rdcspv::StorageClass::StorageBuffer)
+  {
+    if(capabilities.find(rdcspv::Capability::StorageBuffer8BitAccess) != capabilities.end() ||
+       capabilities.find(rdcspv::Capability::UniformAndStorageBuffer8BitAccess) != capabilities.end())
+      emptyStructSize = 1;
+    else if(capabilities.find(rdcspv::Capability::StorageBuffer16BitAccess) != capabilities.end() ||
+            capabilities.find(rdcspv::Capability::UniformAndStorageBuffer16BitAccess) !=
+                capabilities.end())
+      emptyStructSize = 2;
+  }
+  else if(storage == rdcspv::StorageClass::Uniform)
+  {
+    if(capabilities.find(rdcspv::Capability::UniformAndStorageBuffer8BitAccess) != capabilities.end())
+      emptyStructSize = 1;
+    else if(capabilities.find(rdcspv::Capability::UniformAndStorageBuffer16BitAccess) !=
+            capabilities.end())
+      emptyStructSize = 2;
+  }
+  else if(storage == rdcspv::StorageClass::PushConstant)
+  {
+    if(capabilities.find(rdcspv::Capability::StoragePushConstant8) != capabilities.end())
+      emptyStructSize = 1;
+    else if(capabilities.find(rdcspv::Capability::StoragePushConstant16) != capabilities.end())
+      emptyStructSize = 2;
+  }
+
+  for(size_t i = 0; i < cblock.size(); i++)
+  {
+    // for structs that aren't in arrays, we need to define their byte size (stride). Without
+    // knowing the packing rules this shader is complying with, this is not fully possible.
+    //
+    // what we do is choose the most conservative size - so that this struct's size alone doesn't
+    // invalidate compliance with a particular ruleset (e.g. std140).
+    //
+    // we calculate the scalar, base, and extended sizes of the struct sizes of the struct. The
+    // largest one that fits between this struct and the next member is the one we use. If there is
+    // no next member, we always use the base size as it's impossible to tell how much trailing
+    // padding the shader expected.
+    //
+    // If we guess wrongly small, members after this struct will need an [[offset]] decoration,
+    // If we guess wrongly large the struct itself will need a [[size]] decoration
+    // Since we're choosing the largest valid size, it will always be just a [[size]] which might be
+    // unnecessary (if e.g. somewhere else the shader demonstrates scalar packing so the padded size
+    // is larger than the scalar calculated size) but that's only present in one place.
+
+    if(cblock[i].type.descriptor.type == VarType::Struct &&
+       cblock[i].type.descriptor.arrayByteStride == 0)
+    {
+      // this should not be an array - if it is SPIR-V requires an array byte stride, and this
+      // calculation below is also invalid.
+      RDCASSERTEQUAL(cblock[i].type.descriptor.elements, 1);
+
+      StructSizes sizes = CalculateStructProps(emptyStructSize, cblock[i]);
+
+      uint32_t availSize = ~0U;
+      if(i + 1 < cblock.size())
+        availSize = cblock[i + 1].byteOffset - cblock[i].byteOffset;
+      else if(arrayByteStride != 0)
+        availSize = arrayByteStride - cblock[i].byteOffset;
+
+      // expect at least the scalar size to be available otherwise this struct seems to overlap
+      RDCASSERT(sizes.scalarSize <= availSize, sizes.scalarSize, availSize);
+
+      if(sizes.extendedSize <= availSize)
+        cblock[i].type.descriptor.arrayByteStride = sizes.extendedSize;
+      else if(sizes.baseSize <= availSize)
+        cblock[i].type.descriptor.arrayByteStride = sizes.baseSize;
+      else
+        cblock[i].type.descriptor.arrayByteStride = sizes.scalarSize;
+    }
+  }
 }
 
 void Reflector::MakeConstantBlockVariable(ShaderConstant &outConst,
-                                          SparseIdMap<uint16_t> &pointerTypes, const DataType &type,
+                                          SparseIdMap<uint16_t> &pointerTypes,
+                                          rdcspv::StorageClass storage, const DataType &type,
                                           const rdcstr &name, const Decorations &varDecorations,
                                           const rdcarray<SpecConstant> &specInfo) const
 {
@@ -1524,20 +1718,23 @@ void Reflector::MakeConstantBlockVariable(ShaderConstant &outConst,
 
     RDCASSERT(curType->type == DataType::StructType || curType->type == DataType::ArrayType);
 
-    outConst.type.descriptor.type = VarType::Float;
+    outConst.type.descriptor.type = VarType::Struct;
     outConst.type.descriptor.rows = 0;
     outConst.type.descriptor.columns = 0;
 
     outConst.type.descriptor.name = curType->name;
 
-    MakeConstantBlockVariables(*curType, outConst.type.descriptor.elements,
+    MakeConstantBlockVariables(storage, *curType, outConst.type.descriptor.elements,
                                outConst.type.descriptor.arrayByteStride, outConst.type.members,
                                pointerTypes, specInfo);
 
     if(curType->type == DataType::ArrayType)
     {
+      outConst.type.descriptor.name = type.name;
+
       // if the inner type is an array, it will be expanded in our members list. So don't also
       // redundantly keep the element count
+      outConst.type.descriptor.arrayByteStride *= outConst.type.descriptor.elements;
       outConst.type.descriptor.elements = 1;
     }
   }

renderdoc/driver/shaders/spirv/spirv_reflect.h

@@ -107,13 +107,14 @@ private:
   rdcstr StringiseConstant(rdcspv::Id id) const;
   void CheckDebuggable(bool &debuggable, rdcstr &debugStatus) const;
 
-  void MakeConstantBlockVariables(const DataType &structType, uint32_t arraySize,
-                                  uint32_t arrayByteStride, rdcarray<ShaderConstant> &cblock,
+  void MakeConstantBlockVariables(rdcspv::StorageClass storage, const DataType &structType,
+                                  uint32_t arraySize, uint32_t arrayByteStride,
+                                  rdcarray<ShaderConstant> &cblock,
                                   SparseIdMap<uint16_t> &pointerTypes,
                                   const rdcarray<SpecConstant> &specInfo) const;
   void MakeConstantBlockVariable(ShaderConstant &outConst, SparseIdMap<uint16_t> &pointerTypes,
-                                 const DataType &type, const rdcstr &name,
-                                 const Decorations &varDecorations,
+                                 rdcspv::StorageClass storage, const DataType &type,
+                                 const rdcstr &name, const Decorations &varDecorations,
                                  const rdcarray<SpecConstant> &specInfo) const;
   void AddSignatureParameter(const bool isInput, const ShaderStage stage, const Id id,
                              const Id structID, uint32_t &regIndex,