From 81c0cd872bcb94f1f05abc3b234d8918d237d77c Mon Sep 17 00:00:00 2001
From: Tim Foley <tfoleyNV@users.noreply.github.com>
Date: Fri, 15 Dec 2017 11:57:53 -0800
Subject: More fixups for parameter block binding generation (#311)

* More fixups for parameter block binding generation

The bug in this case arises when there is both a parameter block and global-scope resources, all of which are relying on automatic binding assignment. If the parameter block is the first global-scope parameter that gets encountered, then it is possible for it to allocate regsiter space/set zero for itself, which confuses the logic for handling other global-scope parameters (which assumes that *they* get space/set zero).

I've also made some fixup to the reflection test harness and reflection API code:

- Have the hardness handle register-space allocations when printing, and be sure to only show their `index` and not their `space` (since that would be redundant)

- Have the reflection API only auto-redirect queries on a parameter group type layout to its container type layout *if* the container type layout has a non-zero number of resource allocations. The problem that arises here is a `ParameterBlock<X>` where `X` doesn't contain any uniforms, so that no container is needed. In that case the container ends up with no resource allocation(s).

* Fixups for test failures.

- The thread-group size tests failed because they had shader parameters with no resources to back them (built-in `SV_` inputs), and the printing of those changed. I fixed up the baseline, but also had to fix a few bugs in the reflection test fixture's printing logic.

- The GLSL parameter block test revealed a corner case of the existing logic: because we always need to generate a binding for the "hack" sampler (even if code doesn't end up needing it), and that sampler should always go in the "default" set (should be set zero), the user's `ParameterBlock` will always end up as `set=1` or later, even if there are no other global-scope parameters.

  - This will be fixed once we don't have to rely on glslang's annoying behavior in this one case, either because glslang gets fixed, or because we implement our own SPIR-V codegen.
---
 tools/slang-reflection-test/main.cpp | 68 +++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 32 deletions(-)

(limited to 'tools/slang-reflection-test/main.cpp')

diff --git a/tools/slang-reflection-test/main.cpp b/tools/slang-reflection-test/main.cpp
index 057bcd127..2b5477b4a 100644
--- a/tools/slang-reflection-test/main.cpp
+++ b/tools/slang-reflection-test/main.cpp
@@ -116,6 +116,7 @@ static void emitReflectionVarBindingInfoJSON(
     CASE(DESCRIPTOR_TABLE_SLOT, descriptorTableSlot);
     CASE(SPECIALIZATION_CONSTANT, specializationConstant);
     CASE(MIXED, mixed);
+    CASE(REGISTER_SPACE, registerSpace);
     #undef CASE
 
         default:
@@ -124,7 +125,7 @@ static void emitReflectionVarBindingInfoJSON(
             break;
         }
         write(writer, "\"");
-        if( space )
+        if( space && category != SLANG_PARAMETER_CATEGORY_REGISTER_SPACE)
         {
             write(writer, ", ");
             write(writer, "\"space\": ");
@@ -149,6 +150,7 @@ static void emitReflectionVarBindingInfoJSON(
     auto stage = var->getStage();
     if (stage != SLANG_STAGE_NONE)
     {
+        write(writer, ",\n");
         char const* stageName = "UNKNOWN";
         switch (stage)
         {
@@ -165,45 +167,49 @@ static void emitReflectionVarBindingInfoJSON(
 
         write(writer, "\"stage\": \"");
         write(writer, stageName);
-        write(writer, "\",\n");
+        write(writer, "\"");
     }
 
     auto typeLayout = var->getTypeLayout();
     auto categoryCount = var->getCategoryCount();
 
-    if( categoryCount != 1 )
+    if (categoryCount)
     {
-        write(writer,"\"bindings\": [\n");
-    }
-    else
-    {
-        write(writer,"\"binding\": ");
-    }
-    indent(writer);
+        write(writer, ",\n");
+        if( categoryCount != 1 )
+        {
+            write(writer,"\"bindings\": [\n");
+        }
+        else
+        {
+            write(writer,"\"binding\": ");
+        }
+        indent(writer);
 
-    for(uint32_t cc = 0; cc < categoryCount; ++cc )
-    {
-        auto category = var->getCategoryByIndex(cc);
-        auto index = var->getOffset(category);
-        auto space = var->getBindingSpace(category);
-        auto count = typeLayout->getSize(category);
+        for(uint32_t cc = 0; cc < categoryCount; ++cc )
+        {
+            auto category = var->getCategoryByIndex(cc);
+            auto index = var->getOffset(category);
+            auto space = var->getBindingSpace(category);
+            auto count = typeLayout->getSize(category);
 
-        if (cc != 0) write(writer, ",\n");
+            if (cc != 0) write(writer, ",\n");
 
-        write(writer,"{");
-        emitReflectionVarBindingInfoJSON(
-            writer,
-            category,
-            index,
-            count,
-            space);
-        write(writer,"}");
-    }
+            write(writer,"{");
+            emitReflectionVarBindingInfoJSON(
+                writer,
+                category,
+                index,
+                count,
+                space);
+            write(writer,"}");
+        }
 
-    dedent(writer);
-    if( categoryCount != 1 )
-    {
-        write(writer,"\n]");
+        dedent(writer);
+        if( categoryCount != 1 )
+        {
+            write(writer,"\n]");
+        }
     }
 
     if (auto semanticName = var->getSemanticName())
@@ -244,7 +250,6 @@ static void emitReflectionVarLayoutJSON(
 
     write(writer, "\"type\": ");
     emitReflectionTypeLayoutJSON(writer, var->getTypeLayout());
-    write(writer, ",\n");
 
     emitReflectionVarBindingInfoJSON(writer, var);
 
@@ -596,7 +601,6 @@ static void emitReflectionParamJSON(
     indent(writer);
 
     emitReflectionNameInfoJSON(writer, param->getName());
-    write(writer, ",\n");
 
     emitReflectionVarBindingInfoJSON(writer, param);
     write(writer, ",\n");
-- 
cgit v1.2.3