Include array index in reflected uniform names more consistently

* This comes from the resolution of issues 4, 5 & 6 in
  ARB_program_interface_query, stating that uniform buffers should have their
  members expanded out as normal and arrays should have elements added.
* If a buffer block has a large array e.g. [10000] we don't want to iterate over
  every array element. Instead we should only expand out the first [0] element,
  then expand as normal from there.
* The array name should still be appended with [0] to indicate that it's an
  array.

diff --git a/StandAlone/StandAlone.cpp b/StandAlone/StandAlone.cpp
index d26040c..fd69810 100644 (file)
--- a/StandAlone/StandAlone.cpp
+++ b/StandAlone/StandAlone.cpp
@@ -524,6 +524,8 @@ void ProcessArguments(std::vector<std::unique_ptr<glslang::TWorkItem>>& workItem
                         Options |= EOptionNoStorageFormat;
                     } else if (lowerword == "relaxed-errors") {
                         Options |= EOptionRelaxedErrors;
+                    } else if (lowerword == "reflect-strict-array-suffix") {
+                        ReflectOptions |= EShReflectionStrictArraySuffix;
                     } else if (lowerword == "resource-set-bindings" ||  // synonyms
                                lowerword == "resource-set-binding"  ||
                                lowerword == "rsb") {
@@ -1519,6 +1521,7 @@ void usage()
            "  --invert-y | --iy                 invert position.Y output in vertex shader\n"
            "  --keep-uncalled | --ku            don't eliminate uncalled functions\n"
            "  --no-storage-format | --nsf       use Unknown image format\n"
+           "  --reflect-strict-array-suffix     use strict array suffix rules when reflecting\n"
            "  --resource-set-binding [stage] name set binding\n"
            "                                    set descriptor set and binding for\n"
            "                                    individual resources\n"

diff --git a/Test/baseResults/reflection.options.vert.out b/Test/baseResults/reflection.options.vert.out
new file mode 100644 (file)
index 0000000..a4bf815
--- /dev/null
+++ b/Test/baseResults/reflection.options.vert.out
@@ -0,0 +1,15 @@
+reflection.options.vert
+Uniform reflection:
+t[0].v[0].position: offset 0, type 1406, size 3, index 0, binding -1, stages 1
+t[0].v[1].position: offset 24, type 1406, size 3, index 0, binding -1, stages 1
+t[0].v[2].position: offset 48, type 1406, size 3, index 0, binding -1, stages 1
+t[0].v[0].normal: offset 12, type 1406, size 3, index 0, binding -1, stages 1
+t[0].v[1].normal: offset 36, type 1406, size 3, index 0, binding -1, stages 1
+t[0].v[2].normal: offset 60, type 1406, size 3, index 0, binding -1, stages 1
+
+Uniform block reflection:
+VertexCollection: offset -1, type ffffffff, size 360, index -1, binding -1, stages 0
+
+Vertex attribute reflection:
+gl_InstanceID: offset 0, type 1404, size 0, index 0, binding -1, stages 0
+

diff --git a/Test/baseResults/reflection.vert.out b/Test/baseResults/reflection.vert.out
index 5d13523..30aa7a3 100644 (file)
--- a/Test/baseResults/reflection.vert.out
+++ b/Test/baseResults/reflection.vert.out
@@ -84,6 +84,43 @@ nested2.b[2].a: offset 80, type 1406, size 1, index 16, binding -1, stages 1
 nested2.b[3].a: offset 96, type 1406, size 1, index 16, binding -1, stages 1
 nested2.c.a: offset 112, type 1406, size 1, index 16, binding -1, stages 1
 nested2.d.a: offset 144, type 1406, size 1, index 16, binding -1, stages 1
+t.v.position: offset 0, type 1406, size 1, index 17, binding -1, stages 1
+t.v[0].position: offset 0, type 1406, size 3, index 17, binding -1, stages 1
+t.v[1].position: offset 24, type 1406, size 3, index 17, binding -1, stages 1
+t.v[2].position: offset 48, type 1406, size 3, index 17, binding -1, stages 1
+t.v[0].normal: offset 12, type 1406, size 3, index 17, binding -1, stages 1
+t.v[1].normal: offset 36, type 1406, size 3, index 17, binding -1, stages 1
+t.v[2].normal: offset 60, type 1406, size 3, index 17, binding -1, stages 1
+t[0].v[0].position: offset 0, type 1406, size 3, index 17, binding -1, stages 1
+t[0].v[0].normal: offset 12, type 1406, size 3, index 17, binding -1, stages 1
+t[0].v[1].position: offset 24, type 1406, size 3, index 17, binding -1, stages 1
+t[0].v[1].normal: offset 36, type 1406, size 3, index 17, binding -1, stages 1
+t[0].v[2].position: offset 48, type 1406, size 3, index 17, binding -1, stages 1
+t[0].v[2].normal: offset 60, type 1406, size 3, index 17, binding -1, stages 1
+t[1].v[0].position: offset 72, type 1406, size 3, index 17, binding -1, stages 1
+t[1].v[0].normal: offset 84, type 1406, size 3, index 17, binding -1, stages 1
+t[1].v[1].position: offset 96, type 1406, size 3, index 17, binding -1, stages 1
+t[1].v[1].normal: offset 108, type 1406, size 3, index 17, binding -1, stages 1
+t[1].v[2].position: offset 120, type 1406, size 3, index 17, binding -1, stages 1
+t[1].v[2].normal: offset 132, type 1406, size 3, index 17, binding -1, stages 1
+t[2].v[0].position: offset 144, type 1406, size 3, index 17, binding -1, stages 1
+t[2].v[0].normal: offset 156, type 1406, size 3, index 17, binding -1, stages 1
+t[2].v[1].position: offset 168, type 1406, size 3, index 17, binding -1, stages 1
+t[2].v[1].normal: offset 180, type 1406, size 3, index 17, binding -1, stages 1
+t[2].v[2].position: offset 192, type 1406, size 3, index 17, binding -1, stages 1
+t[2].v[2].normal: offset 204, type 1406, size 3, index 17, binding -1, stages 1
+t[3].v[0].position: offset 216, type 1406, size 3, index 17, binding -1, stages 1
+t[3].v[0].normal: offset 228, type 1406, size 3, index 17, binding -1, stages 1
+t[3].v[1].position: offset 240, type 1406, size 3, index 17, binding -1, stages 1
+t[3].v[1].normal: offset 252, type 1406, size 3, index 17, binding -1, stages 1
+t[3].v[2].position: offset 264, type 1406, size 3, index 17, binding -1, stages 1
+t[3].v[2].normal: offset 276, type 1406, size 3, index 17, binding -1, stages 1
+t[4].v[0].position: offset 288, type 1406, size 3, index 17, binding -1, stages 1
+t[4].v[0].normal: offset 300, type 1406, size 3, index 17, binding -1, stages 1
+t[4].v[1].position: offset 312, type 1406, size 3, index 17, binding -1, stages 1
+t[4].v[1].normal: offset 324, type 1406, size 3, index 17, binding -1, stages 1
+t[4].v[2].position: offset 336, type 1406, size 3, index 17, binding -1, stages 1
+t[4].v[2].normal: offset 348, type 1406, size 3, index 17, binding -1, stages 1
 anonMember1: offset 0, type 8b51, size 1, index 0, binding -1, stages 1
 uf1: offset -1, type 1406, size 1, index -1, binding -1, stages 1
 uf2: offset -1, type 1406, size 1, index -1, binding -1, stages 1
@@ -107,6 +144,7 @@ buf2: offset -1, type ffffffff, size 4, index -1, binding -1, stages 0
 buf3: offset -1, type ffffffff, size 4, index -1, binding -1, stages 0
 buf4: offset -1, type ffffffff, size 4, index -1, binding -1, stages 0
 nested2: offset -1, type ffffffff, size 208, index -1, binding -1, stages 0
+VertexCollection: offset -1, type ffffffff, size 360, index -1, binding -1, stages 0
 
 Vertex attribute reflection:
 attributeFloat: offset 0, type 1406, size 0, index 0, binding -1, stages 0

diff --git a/Test/reflection.options.vert b/Test/reflection.options.vert
new file mode 100644 (file)
index 0000000..c671a62
--- /dev/null
+++ b/Test/reflection.options.vert
@@ -0,0 +1,23 @@
+#version 440 core\r
+\r
+struct VertexInfo {\r
+    float position[3];\r
+    float normal[3];\r
+};\r
+\r
+struct TriangleInfo {\r
+    VertexInfo v[3];\r
+};\r
+\r
+buffer VertexCollection {\r
+    TriangleInfo t[5];\r
+};\r
+\r
+void main()\r
+{\r
+    float f;\r
+    f += t[0].v[0].position[0];\r
+    f += t[gl_InstanceID].v[gl_InstanceID].position[gl_InstanceID];\r
+    f += t[gl_InstanceID].v[gl_InstanceID].normal[gl_InstanceID];\r
+    TriangleInfo tlocal[5] = t;\r
+}\r

diff --git a/Test/reflection.vert b/Test/reflection.vert
index 9b3b45c..76ec5a3 100644 (file)
--- a/Test/reflection.vert
+++ b/Test/reflection.vert
@@ -161,6 +161,19 @@ buffer buf4 {
     N2 runtimeArray[];\r
 } buf4i;\r
 \r
+struct VertexInfo {\r
+    float position[3];\r
+    float normal[3];\r
+};\r
+\r
+struct TriangleInfo {\r
+    VertexInfo v[3];\r
+};\r
+\r
+buffer VertexCollection {\r
+    TriangleInfo t[5];\r
+};\r
+\r
 void main()\r
 {\r
     liveFunction1(image_ui2D, sampler_2D, sampler_2DMSArray);\r
@@ -216,4 +229,9 @@ void main()
     N1 b[4] = nest2.b;\r
     f += nest2.c[1].a;\r
     f += nest2.d[gl_InstanceID].a;\r
+\r
+    f += t[0].v[0].position[0];\r
+    f += t[gl_InstanceID].v[gl_InstanceID].position[gl_InstanceID];\r
+    f += t[gl_InstanceID].v[gl_InstanceID].normal[gl_InstanceID];\r
+    TriangleInfo tlocal[5] = t;\r
 }\r

diff --git a/Test/runtests b/Test/runtests
index c88cb59..890ebac 100755 (executable)
--- a/Test/runtests
+++ b/Test/runtests
@@ -32,6 +32,8 @@ diff -b $BASEDIR/badMacroArgs.frag.out $TARGETDIR/badMacroArgs.frag.out || HASER
 echo Running reflection...
 $EXE -l -q -C reflection.vert > $TARGETDIR/reflection.vert.out
 diff -b $BASEDIR/reflection.vert.out $TARGETDIR/reflection.vert.out || HASERROR=1
+$EXE -l -q -C --reflect-strict-array-suffix reflection.options.vert > $TARGETDIR/reflection.options.vert.out
+diff -b $BASEDIR/reflection.options.vert.out $TARGETDIR/reflection.options.vert.out || HASERROR=1
 $EXE -D -Od -e flizv -l -q -C -V -Od hlsl.reflection.vert > $TARGETDIR/hlsl.reflection.vert.out
 diff -b $BASEDIR/hlsl.reflection.vert.out $TARGETDIR/hlsl.reflection.vert.out || HASERROR=1
 $EXE -D -Od -e main -l -q -C -V -Od hlsl.reflection.binding.frag > $TARGETDIR/hlsl.reflection.binding.frag.out

diff --git a/glslang/MachineIndependent/reflection.cpp b/glslang/MachineIndependent/reflection.cpp
index caa5cbf..0f46daa 100644 (file)
--- a/glslang/MachineIndependent/reflection.cpp
+++ b/glslang/MachineIndependent/reflection.cpp
@@ -223,6 +223,16 @@ public:
     void blowUpActiveAggregate(const TType& baseType, const TString& baseName, const TList<TIntermBinary*>& derefs,
                                TList<TIntermBinary*>::const_iterator deref, int offset, int blockIndex, int arraySize)
     {
+        // when strictArraySuffix is enabled, we closely follow the rules from ARB_program_interface_query.
+        // Broadly:
+        // * arrays-of-structs always have a [x] suffix.
+        // * with array-of-struct variables in the root of a buffer block, only ever return [0].
+        // * otherwise, array suffixes are added whenever we iterate, even if that means expanding out an array.
+        const bool strictArraySuffix = (reflection.options & EShReflectionStrictArraySuffix);
+
+        // is this variable inside a buffer block. This flag is set back to false after we iterate inside the first array element.
+        bool blockParent = (baseType.getBasicType() == EbtBlock && baseType.getQualifier().storage == EvqBuffer);
+
         // process the part of the dereference chain that was explicit in the shader
         TString name = baseName;
         const TType* terminalType = &baseType;
@@ -237,7 +247,9 @@ public:
                 // Visit all the indices of this array, and for each one add on the remaining dereferencing
                 for (int i = 0; i < std::max(visitNode->getLeft()->getType().getOuterArraySize(), 1); ++i) {
                     TString newBaseName = name;
-                    if (baseType.getBasicType() != EbtBlock)
+                    if (strictArraySuffix && blockParent)
+                        newBaseName.append(TString("[0]"));
+                    else if (strictArraySuffix || baseType.getBasicType() != EbtBlock)
                         newBaseName.append(TString("[") + String(i) + "]");
                     TList<TIntermBinary*>::const_iterator nextDeref = deref;
                     ++nextDeref;
@@ -253,12 +265,15 @@ public:
             }
             case EOpIndexDirect:
                 index = visitNode->getRight()->getAsConstantUnion()->getConstArray()[0].getIConst();
-                if (baseType.getBasicType() != EbtBlock) {
+                if (strictArraySuffix && blockParent) {
+                    name.append(TString("[0]"));
+                } else if (strictArraySuffix || baseType.getBasicType() != EbtBlock) {
                     name.append(TString("[") + String(index) + "]");
 
                     if (offset >= 0)
                       offset += getArrayStride(baseType, visitNode->getLeft()->getType()) * index;
                 }
+                blockParent = false;
                 break;
             case EOpIndexDirectStruct:
                 index = visitNode->getRight()->getAsConstantUnion()->getConstArray()[0].getIConst();
@@ -286,7 +301,13 @@ public:
                 if (offset >= 0)
                     stride = getArrayStride(baseType, *terminalType);
 
-                for (int i = 0; i < std::max(terminalType->getOuterArraySize(), 1); ++i) {
+                int arrayIterateSize = std::max(terminalType->getOuterArraySize(), 1);
+
+                // for top-level arrays in blocks, only expand [0] to avoid explosion of items
+                if (strictArraySuffix && blockParent)
+                    arrayIterateSize = 1;
+
+                for (int i = 0; i < arrayIterateSize; ++i) {
                     TString newBaseName = name;
                     newBaseName.append(TString("[") + String(i) + "]");
                     TType derefType(*terminalType, 0);

diff --git a/glslang/Public/ShaderLang.h b/glslang/Public/ShaderLang.h
index 3bb9f5e..3866f2d 100755 (executable)
--- a/glslang/Public/ShaderLang.h
+++ b/glslang/Public/ShaderLang.h
@@ -244,6 +244,7 @@ enum EShMessages {
 //
 typedef enum {
     EShReflectionDefault           = 0,        // default is original behaviour before options were added
+    EShReflectionStrictArraySuffix = (1 << 0), // reflection will follow stricter rules for array-of-structs suffixes
 } EShReflectionOptions;
 
 //