Fix support for arrays-of-struct vertex outputs. Closes #1438

renderdoc/driver/shaders/spirv/spirv_disassemble.cpp

@@ -3922,8 +3922,9 @@ typedef bindpair<ConstantBlock> cblockpair;
 typedef bindpair<ShaderResource> shaderrespair;
 
 void AddSignatureParameter(bool isInput, ShaderStage stage, uint32_t id, uint32_t structID,
-                           uint32_t &regIndex, std::vector<uint32_t> accessChain, std::string varName,
-                           SPVTypeData *type, const std::vector<SPVDecoration> &decorations,
+                           uint32_t &regIndex, const SPIRVPatchData::InterfaceAccess &parentPatch,
+                           std::string varName, SPVTypeData *type,
+                           const std::vector<SPVDecoration> &decorations,
                            std::vector<SigParameter> &sigarray, SPIRVPatchData &patchData)
 {
   SigParameter sig;
@@ -3931,9 +3932,12 @@ void AddSignatureParameter(bool isInput, ShaderStage stage, uint32_t id, uint32_
   sig.needSemanticIndex = false;
 
   SPIRVPatchData::InterfaceAccess patch;
-  patch.accessChain = accessChain;
+  patch.accessChain = parentPatch.accessChain;
   patch.ID = id;
   patch.structID = structID;
+  patch.isArraySubsequentElement = parentPatch.isArraySubsequentElement;
+  if(structID)
+    patch.structMemberIndex = patch.accessChain.back();
 
   bool rowmajor = true;
 
@@ -3941,7 +3945,7 @@ void AddSignatureParameter(bool isInput, ShaderStage stage, uint32_t id, uint32_
   for(size_t d = 0; d < decorations.size(); d++)
   {
     if(decorations[d].decoration == spv::DecorationLocation)
-      sig.regIndex = decorations[d].val;
+      sig.regIndex = regIndex = decorations[d].val;
     else if(decorations[d].decoration == spv::DecorationBuiltIn)
       sig.systemValue = BuiltInToSystemAttribute(stage, (spv::BuiltIn)decorations[d].val);
     else if(decorations[d].decoration == spv::DecorationRowMajor)
@@ -3965,11 +3969,24 @@ void AddSignatureParameter(bool isInput, ShaderStage stage, uint32_t id, uint32_
     isArray = true;
     type = type->baseType;
 
+    // for geometry/tessellation evaluation shaders, ignore the root level of array-ness for inputs
+    if((stage == ShaderStage::Geometry || stage == ShaderStage::Tess_Eval) && isInput && structID == 0)
+      arraySize = 1;
+
+    // for tessellation control shaders, ignore the root level of array-ness for both inputs and
+    // outputs
+    if(stage == ShaderStage::Tess_Control && structID == 0)
+      arraySize = 1;
+
     // step through multi-dimensional arrays
     while(type->type == SPVTypeData::eArray)
       type = type->baseType;
   }
 
+  // arrays will need an extra access chain index
+  if(isArray)
+    patch.accessChain.push_back(0U);
+
   if(type->type == SPVTypeData::eStruct)
   {
     // it's invalid to include built-in and 'normal' outputs in the same struct. One
@@ -3993,6 +4010,7 @@ void AddSignatureParameter(bool isInput, ShaderStage stage, uint32_t id, uint32_
 
     for(uint32_t a = 0; a < arraySize; a++)
     {
+      // push the member-index access chain value
       patch.accessChain.push_back(0U);
 
       for(size_t c = 0; c < type->children.size(); c++)
@@ -4016,14 +4034,28 @@ void AddSignatureParameter(bool isInput, ShaderStage stage, uint32_t id, uint32_
             continue;
         }
 
-        std::string baseName = isArray ? StringFormat::Fmt("%s[%u]", varName.c_str(), a) : varName;
+        std::string baseName = varName;
 
-        AddSignatureParameter(isInput, stage, id, type->id, regIndex, patch.accessChain,
+        if(isArray)
+          baseName = StringFormat::Fmt("%s[%u]", varName.c_str(), a);
+
+        AddSignatureParameter(isInput, stage, id, type->id, regIndex, patch,
                               baseName + "." + type->children[c].second, type->children[c].first,
                               type->childDecorations[c], sigarray, patchData);
 
+        // increment the member-index access chain value
         patch.accessChain.back()++;
       }
+
+      // pop the member-index access chain value
+      patch.accessChain.pop_back();
+
+      // increment the array-index access chain value
+      if(isArray)
+      {
+        patch.accessChain.back()++;
+        patch.isArraySubsequentElement = true;
+      }
     }
 
     return;
@@ -4047,10 +4079,6 @@ void AddSignatureParameter(bool isInput, ShaderStage stage, uint32_t id, uint32_
 
   sig.regChannelMask = sig.channelUsedMask = (1 << type->vectorSize) - 1;
 
-  // arrays will need an extra access chain index
-  if(isArray)
-    patch.accessChain.push_back(0U);
-
   for(uint32_t a = 0; a < arraySize; a++)
   {
     std::string n = varName;
@@ -4097,15 +4125,18 @@ void AddSignatureParameter(bool isInput, ShaderStage stage, uint32_t id, uint32_
 
         regIndex++;
 
+        // increment the matrix column access chain
         patch.accessChain.back()++;
         patch.isArraySubsequentElement = true;
       }
 
+      // pop the matrix column access chain
       patch.isMatrix = false;
       patch.accessChain.pop_back();
     }
 
     sig.regIndex += RDCMAX(1U, type->matrixSize);
+    // increment the array index access chain (if it exists)
     if(isArray)
     {
       patch.accessChain.back()++;
@@ -4221,8 +4252,8 @@ void SPVModule::MakeReflection(GraphicsAPI sourceAPI, ShaderStage stage,
         nm = StringFormat::Fmt("sig%u", inst->id);
 
       uint32_t dummy = 0;
-      AddSignatureParameter(isInput, stage, inst->id, 0, dummy, std::vector<uint32_t>(), nm,
-                            inst->var->type, inst->decorations, *sigarray, patchData);
+      AddSignatureParameter(isInput, stage, inst->id, 0, dummy, {}, nm, inst->var->type,
+                            inst->decorations, *sigarray, patchData);
 
       // eliminate any members of gl_PerVertex that are actually unused and just came along
       // for the ride (usually with gl_Position, but maybe declared globally and still unused)

renderdoc/driver/shaders/spirv/spirv_reflect.cpp

@@ -103,10 +103,10 @@ void AddXFBAnnotations(const ShaderReflection &refl, const SPIRVPatchData &patch
       {
         for(size_t i = 0; i < outsig.size(); i++)
         {
-          if(outpatch[i].structID && !outpatch[i].accessChain.empty())
+          if(outpatch[i].structID)
           {
             if(it.opcode() == spv::OpMemberDecorate && it.word(1) == outpatch[i].structID &&
-               it.word(2) == outpatch[i].accessChain.back())
+               it.word(2) == outpatch[i].structMemberIndex)
             {
               editor.Remove(it);
             }
@@ -149,11 +149,11 @@ void AddXFBAnnotations(const ShaderReflection &refl, const SPIRVPatchData &patch
     {
       // do not patch anything as we only patch the base array, but reserve space in the stride
     }
-    else if(outpatch[i].structID && !outpatch[i].accessChain.empty())
+    else if(outpatch[i].structID)
     {
       editor.AddDecoration(rdcspv::Operation(
           spv::OpMemberDecorate,
-          {outpatch[i].structID, outpatch[i].accessChain.back(), spv::DecorationOffset, xfbStride}));
+          {outpatch[i].structID, outpatch[i].structMemberIndex, spv::DecorationOffset, xfbStride}));
     }
     else if(outpatch[i].ID)
     {

renderdoc/driver/shaders/spirv/spirv_reflect.h

@@ -43,10 +43,13 @@ struct SPIRVPatchData
   struct InterfaceAccess
   {
     // ID of the base variable
-    uint32_t ID;
+    uint32_t ID = 0;
 
     // ID of the struct parent of this variable
-    uint32_t structID;
+    uint32_t structID = 0;
+
+    // member in the parent struct of this variable (for MemberDecorate)
+    uint32_t structMemberIndex = 0;
 
     // the access chain of indices
     std::vector<uint32_t> accessChain;

util/test/demos/vk/vk_test.cpp

@@ -317,6 +317,8 @@ void VulkanGraphicsTest::Prepare(int argc, char **argv)
   std::vector<VkExtensionProperties> supportedExts;
   CHECK_VKR(vkh::enumerateDeviceExtensionProperties(supportedExts, phys, NULL));
 
+  vkGetPhysicalDeviceProperties(phys, &physProperties);
+
   for(const char *search : devExts)
   {
     bool found = false;
@@ -468,10 +470,7 @@ bool VulkanGraphicsTest::Init()
 
   vmaCreateAllocator(&allocInfo, &allocator);
 
-  VkPhysicalDeviceProperties physProps;
-  vkGetPhysicalDeviceProperties(phys, &physProps);
-
-  TEST_LOG("Running Vulkan test on %s", physProps.deviceName);
+  TEST_LOG("Running Vulkan test on %s", physProperties.deviceName);
 
   return true;
 }

util/test/demos/vk/vk_test.h

@@ -271,6 +271,7 @@ struct VulkanGraphicsTest : public GraphicsTest
   uint32_t queueFamilyIndex = ~0U;
   uint32_t queueCount;
   VkQueue queue;
+  VkPhysicalDeviceProperties physProperties;
 
   // utilities
   VkDebugUtilsMessengerEXT debugUtilsMessenger;

util/test/demos/vk/vk_vertex_attr_zoo.cpp

@@ -38,7 +38,6 @@ TEST(VK_Vertex_Attr_Zoo, VulkanGraphicsTest)
     double df[2];
     float arr0[2];
     float arr1[2];
-    float arr2[2];
     float mat0[2];
     float mat1[2];
   };
@@ -51,16 +50,16 @@ layout(location = 1) in vec4 InUNorm;
 layout(location = 2) in vec4 InUScaled;
 layout(location = 3) in uvec4 InUInt;
 layout(location = 4) in dvec2 InDouble;
-layout(location = 5) in vec2 InArray[3];
-layout(location = 8) in mat2x2 InMatrix;
+layout(location = 5) in vec2 InArray[2];
+layout(location = 7) in mat2x2 InMatrix;
 
 layout(location = 0) out vec4 OutSNorm;
 layout(location = 1) out vec4 OutUNorm;
 layout(location = 2) out vec4 OutUScaled;
 layout(location = 3) out uvec4 OutUInt;
 layout(location = 4) out dvec2 OutDouble;
-layout(location = 5) out vec2 OutArray[3];
-layout(location = 8) out mat2x2 OutMatrix;
+layout(location = 5) out vec2 OutArray[2];
+layout(location = 7) out mat2x2 OutMatrix;
 
 void main()
 {
@@ -88,8 +87,8 @@ layout(location = 1) in vec4 InUNorm;
 layout(location = 2) in vec4 InUScaled;
 layout(location = 3) flat in uvec4 InUInt;
 layout(location = 4) flat in dvec2 InDouble;
-layout(location = 5) in vec2 InArray[3];
-layout(location = 8) in mat2x2 InMatrix;
+layout(location = 5) in vec2 InArray[2];
+layout(location = 7) in mat2x2 InMatrix;
 
 layout(location = 0, index = 0) out vec4 Color;
 
@@ -133,16 +132,16 @@ layout(location = 1) in vec4 InUNorm[3];
 layout(location = 2) in vec4 InUScaled[3];
 layout(location = 3) in uvec4 InUInt[3];
 layout(location = 4) in dvec2 InDouble[3];
-layout(location = 5) in vec2 InArray[3][3];
-layout(location = 8) in mat2x2 InMatrix[3];
+layout(location = 5) in vec2 InArray[3][2];
+layout(location = 7) in mat2x2 InMatrix[3];
 
 layout(location = 0) out vec4 OutSNorm;
 layout(location = 1) out vec4 OutUNorm;
 layout(location = 2) out vec4 OutUScaled;
 layout(location = 3) out uvec4 OutUInt;
 layout(location = 4) out dvec2 OutDouble;
-layout(location = 5) out vec2 OutArray[3];
-layout(location = 8) out mat2x2 OutMatrix;
+layout(location = 5) out vec2 OutArray[2];
+layout(location = 7) out mat2x2 OutMatrix;
 
 void main()
 {
@@ -163,6 +162,108 @@ void main()
   EndPrimitive();
 }
 
+)EOSHADER";
+
+  std::string vertex2 = R"EOSHADER(
+#version 450 core
+
+layout(location = 0) out vec4 OutDummy;
+
+struct ArrayWrapper
+{
+  float foo[2];
+};
+
+struct SimpleWrapper
+{
+  float foo;
+};
+
+struct MyStruct
+{
+  float a;
+  float b[2];
+  ArrayWrapper c;
+  SimpleWrapper d[2];
+};
+
+layout(location = 1) out OutData
+{
+  MyStruct outStruct;
+} outData;
+
+void main()
+{
+  const vec4 verts[3] = vec4[3](vec4(-0.5, 0.5, 0.0, 1.0), vec4(0.0, -0.5, 0.0, 1.0),
+                                vec4(0.5, 0.5, 0.0, 1.0));
+
+  gl_Position = verts[gl_VertexIndex];
+
+  OutDummy = vec4(0,0,0,0);
+
+  outData.outStruct.a = 1.1f;
+  outData.outStruct.b[0] = 2.2f;
+  outData.outStruct.b[1] = 3.3f;
+  outData.outStruct.c.foo[0] = 4.4f;
+  outData.outStruct.c.foo[1] = 5.5f;
+  outData.outStruct.d[0].foo = 6.6f;
+  outData.outStruct.d[1].foo = 7.7f;
+}
+
+)EOSHADER";
+
+  std::string geom2 = R"EOSHADER(
+#version 450 core
+
+layout(triangles) in;
+layout(triangle_strip, max_vertices = 3) out;
+
+layout(location = 0) in vec4 InDummy[3];
+
+struct ArrayWrapper
+{
+  float foo[2];
+};
+
+struct SimpleWrapper
+{
+  float foo;
+};
+
+struct MyStruct
+{
+  float a;
+  float b[2];
+  ArrayWrapper c;
+  SimpleWrapper d[2];
+};
+
+layout(location = 1) in OutData
+{
+  MyStruct inStruct;
+} inData[3];
+
+layout(location = 0) out vec4 OutDummy;
+
+layout(location = 1) out OutData
+{
+  MyStruct outStruct;
+} outData;
+
+void main()
+{
+  for(int i = 0; i < 3; i++)
+  {
+    gl_Position = vec4(gl_in[i].gl_Position.yx, 0.4f, 1.2f);
+
+    OutDummy = InDummy[i];
+    outData.outStruct = inData[i].inStruct;
+
+    EmitVertex();
+  }
+  EndPrimitive();
+}
+
 )EOSHADER";
 
   void Prepare(int argc, char **argv)
@@ -172,6 +273,9 @@ void main()
     features.geometryShader = VK_TRUE;
 
     VulkanGraphicsTest::Prepare(argc, argv);
+
+    if(physProperties.limits.maxVertexOutputComponents < 128)
+      Avail = "Not enough vertex output components to run test";
   }
 
   int main()
@@ -197,9 +301,8 @@ void main()
         vkh::vertexAttrFormatted(4, 0, vertin, df, VK_FORMAT_R64G64_SFLOAT),
         vkh::vertexAttrFormatted(5, 0, vertin, arr0, VK_FORMAT_R32G32_SFLOAT),
         vkh::vertexAttrFormatted(6, 0, vertin, arr1, VK_FORMAT_R32G32_SFLOAT),
-        vkh::vertexAttrFormatted(7, 0, vertin, arr2, VK_FORMAT_R32G32_SFLOAT),
-        vkh::vertexAttrFormatted(8, 0, vertin, mat0, VK_FORMAT_R32G32_SFLOAT),
-        vkh::vertexAttrFormatted(9, 0, vertin, mat1, VK_FORMAT_R32G32_SFLOAT),
+        vkh::vertexAttrFormatted(7, 0, vertin, mat0, VK_FORMAT_R32G32_SFLOAT),
+        vkh::vertexAttrFormatted(8, 0, vertin, mat1, VK_FORMAT_R32G32_SFLOAT),
     };
 
     pipeCreateInfo.stages = {
@@ -210,6 +313,15 @@ void main()
 
     VkPipeline pipe = createGraphicsPipeline(pipeCreateInfo);
 
+    pipeCreateInfo.stages = {
+        CompileShaderModule(vertex2, ShaderLang::glsl, ShaderStage::vert, "main"),
+        CompileShaderModule(geom2, ShaderLang::glsl, ShaderStage::geom, "main"),
+    };
+
+    pipeCreateInfo.rasterizationState.rasterizerDiscardEnable = VK_TRUE;
+
+    VkPipeline pipe2 = createGraphicsPipeline(pipeCreateInfo);
+
     vertin triangle[] = {
         {
             {32767, -32768, 32767, -32767},
@@ -217,8 +329,7 @@ void main()
             {9.8765432109, -5.6789012345},
             {1.0f, 2.0f},
             {3.0f, 4.0f},
-            {5.0f, 6.0f},
-            {7.0, 8.0f},
+            {7.0f, 8.0f},
             {9.0f, 10.0f},
         },
         {
@@ -227,8 +338,7 @@ void main()
             {-7.89012345678, 6.54321098765},
             {11.0f, 12.0f},
             {13.0f, 14.0f},
-            {15.0f, 16.0f},
-            {17.0, 18.0f},
+            {17.0f, 18.0f},
             {19.0f, 20.0f},
         },
         {
@@ -237,8 +347,7 @@ void main()
             {0.1234567890123, 4.5678901234},
             {21.0f, 22.0f},
             {23.0f, 24.0f},
-            {25.0f, 26.0f},
-            {27.0, 28.0f},
+            {27.0f, 28.0f},
             {29.0f, 30.0f},
         },
     };
@@ -273,6 +382,9 @@ void main()
       vkh::cmdBindVertexBuffers(cmd, 0, {vb.buffer}, {0});
       vkCmdDraw(cmd, 3, 1, 0, 0);
 
+      vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipe2);
+      vkCmdDraw(cmd, 3, 1, 0, 0);
+
       vkCmdEndRenderPass(cmd);
 
       FinishUsingBackbuffer(cmd, VK_ACCESS_TRANSFER_WRITE_BIT, VK_IMAGE_LAYOUT_GENERAL);

util/test/tests/Vulkan/VK_Vertex_Attr_Zoo.py

@@ -27,7 +27,6 @@ class VK_Vertex_Attr_Zoo(rdtest.TestCase):
                 'Double': [9.8765432109, -5.6789012345],
                 'Array[0]': [1.0, 2.0],
                 'Array[1]': [3.0, 4.0],
-                'Array[2]': [5.0, 6.0],
                 'Matrix:row0': [7.0, 8.0],
                 'Matrix:row1': [9.0, 10.0],
             },
@@ -39,7 +38,6 @@ class VK_Vertex_Attr_Zoo(rdtest.TestCase):
                 'Double': [-7.89012345678, 6.54321098765],
                 'Array[0]': [11.0, 12.0],
                 'Array[1]': [13.0, 14.0],
-                'Array[2]': [15.0, 16.0],
                 'Matrix:row0': [17.0, 18.0],
                 'Matrix:row1': [19.0, 20.0],
             },
@@ -51,7 +49,6 @@ class VK_Vertex_Attr_Zoo(rdtest.TestCase):
                 'Double': [0.1234567890123, 4.5678901234],
                 'Array[0]': [21.0, 22.0],
                 'Array[1]': [23.0, 24.0],
-                'Array[2]': [25.0, 26.0],
                 'Matrix:row0': [27.0, 28.0],
                 'Matrix:row1': [29.0, 30.0],
             },
@@ -115,4 +112,31 @@ class VK_Vertex_Attr_Zoo(rdtest.TestCase):
 
         rdtest.log.success("Triangle picked value is as expected")
 
+        # Step to the next draw with awkward struct/array outputs
+        self.controller.SetFrameEvent(draw.next.eventId, False)
+
+        ref = {
+            0: {
+                'outData.outStruct.a': [1.1],
+                'outData.outStruct.b[0]': [2.2],
+                'outData.outStruct.b[1]': [3.3],
+                'outData.outStruct.c.foo[0]': [4.4],
+                'outData.outStruct.c.foo[1]': [5.5],
+                'outData.outStruct.d[0].foo': [6.6],
+                'outData.outStruct.d[1].foo': [7.7],
+            },
+        }
+
+        self.check_mesh_data(ref, self.get_postvs(rd.MeshDataStage.VSOut))
+
+        rdtest.log.success("Nested vertex output data is as expected")
+
+        # The array-of-structs data is a broken in transform feedback
+        del ref[0]['outData.outStruct.d[0].foo']
+        del ref[0]['outData.outStruct.d[1].foo']
+
+        self.check_mesh_data(ref, self.get_postvs(rd.MeshDataStage.GSOut))
+
+        rdtest.log.success("Nested geometry output data is as expected")
+
         out.Shutdown()