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.
---
 source/slang/reflection.cpp | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

(limited to 'source/slang/reflection.cpp')

diff --git a/source/slang/reflection.cpp b/source/slang/reflection.cpp
index cc2c6b289..bd95f48fa 100644
--- a/source/slang/reflection.cpp
+++ b/source/slang/reflection.cpp
@@ -574,15 +574,26 @@ static SlangParameterCategory getParameterCategory(
     return SLANG_PARAMETER_CATEGORY_MIXED;
 }
 
+static TypeLayout* maybeGetContainerLayout(TypeLayout* typeLayout)
+{
+    if (auto parameterGroupTypeLayout = dynamic_cast<ParameterGroupTypeLayout*>(typeLayout))
+    {
+        auto containerTypeLayout = parameterGroupTypeLayout->containerVarLayout->typeLayout;
+        if (containerTypeLayout->resourceInfos.Count() != 0)
+        {
+            return containerTypeLayout;
+        }
+    }
+
+    return typeLayout;
+}
+
 SLANG_API SlangParameterCategory spReflectionTypeLayout_GetParameterCategory(SlangReflectionTypeLayout* inTypeLayout)
 {
     auto typeLayout = convert(inTypeLayout);
     if(!typeLayout) return SLANG_PARAMETER_CATEGORY_NONE;
 
-    if (auto parameterGroupTypeLayout = dynamic_cast<ParameterGroupTypeLayout*>(typeLayout))
-    {
-        typeLayout = parameterGroupTypeLayout->containerVarLayout->typeLayout;
-    }
+    typeLayout = maybeGetContainerLayout(typeLayout);
 
     return getParameterCategory(typeLayout);
 }
@@ -592,10 +603,7 @@ SLANG_API unsigned spReflectionTypeLayout_GetCategoryCount(SlangReflectionTypeLa
     auto typeLayout = convert(inTypeLayout);
     if(!typeLayout) return 0;
 
-    if (auto parameterGroupTypeLayout = dynamic_cast<ParameterGroupTypeLayout*>(typeLayout))
-    {
-        typeLayout = parameterGroupTypeLayout->containerVarLayout->typeLayout;
-    }
+    typeLayout = maybeGetContainerLayout(typeLayout);
 
     return (unsigned) typeLayout->resourceInfos.Count();
 }
@@ -605,10 +613,7 @@ SLANG_API SlangParameterCategory spReflectionTypeLayout_GetCategoryByIndex(Slang
     auto typeLayout = convert(inTypeLayout);
     if(!typeLayout) return SLANG_PARAMETER_CATEGORY_NONE;
 
-    if (auto parameterGroupTypeLayout = dynamic_cast<ParameterGroupTypeLayout*>(typeLayout))
-    {
-        typeLayout = parameterGroupTypeLayout->containerVarLayout->typeLayout;
-    }
+    typeLayout = maybeGetContainerLayout(typeLayout);
 
     return typeLayout->resourceInfos[index].kind;
 }
-- 
cgit v1.2.3