Adjust type layout when parameter block constains member using the same resource

If we have something like to following in HLSL:

    cbuffer C { Texture2D t; ... }

and we are compiling to GLSL, then both `C` and `C.t` consume the same kind of resource (a descriptor-table slot).
The way reflection was working right now, querying the index of `C` would return its binding (let's say it is `4` just to be concrete) and then a query on `C::t` would give its offset, which was being computed as `0` because it is the first field in the logical `struct` type.

That obviously leads to bad math and requires some subtle `+1`s in cases to get things right (e.g., when scalaring during lowering, I had to carefully add one in some cases).
It is unreasonable to expect users to deal with this.

This commit changes it so that the offset of field `C::t` is `1` so that hopefully more things Just Work.
The special-case logic in lowering is now gone.

One important catch here is that this pretty much only works in the case where the element type of a parameter block is a `struct` type (which is really all that makes sense right now).
If we ever want to generalize this in the future, then it will probably be necessary to change the `TypeLayout` case for parameter blocks to store a `VarLayout` for the element, rather than just a `TypeLayout`.

4 files changed, 101 insertions, 75 deletions

diff --git a/source/slang/lower.cpp b/source/slang/lower.cpp
index c7278201a..caa1ab075 100644
--- a/source/slang/lower.cpp
+++ b/source/slang/lower.cpp
@@ -1828,25 +1828,6 @@ struct LoweringVisitor
                             needsOffset = true;
                             break;
                         }
-
-                        // Even if the base offset of the parent is zero, we may still
-                        // need to offset the child, because the parent consumes a
-                        // resources of the same kind...
-                        if (primaryVarLayout->typeLayout->type->As<UniformParameterBlockType>())
-                        {
-                            switch (rr.kind)
-                            {
-                            default:
-                                break;
-
-                            case LayoutResourceKind::ConstantBuffer:
-                            case LayoutResourceKind::DescriptorTableSlot:
-                                needsOffset = true;
-                                break;
-                            }
-                            if (needsOffset)
-                                break;
-                        }
                     }
                 }
                 if (needsOffset)
@@ -1867,22 +1848,6 @@ struct LoweringVisitor
                         {
                             newResInfo->index += parentInfo->index;
                             newResInfo->space += parentInfo->space;
-
-                            // Very special-case hack to deal with the case where the parent
-                            // itself consumes a resources of the same type as the field.
-                            if (primaryVarLayout->typeLayout->type->As<UniformParameterBlockType>())
-                            {
-                                switch (resInfo.kind)
-                                {
-                                default:
-                                    break;
-
-                                case LayoutResourceKind::ConstantBuffer:
-                                case LayoutResourceKind::DescriptorTableSlot:
-                                    newResInfo->index += 1;
-                                    break;
-                                }
-                            }
                         }
                     }
 
diff --git a/source/slang/parameter-binding.cpp b/source/slang/parameter-binding.cpp
index 01727fb36..007c023e2 100644
--- a/source/slang/parameter-binding.cpp
+++ b/source/slang/parameter-binding.cpp
@@ -1342,8 +1342,9 @@ void generateParameterBindings(
     {
         auto globalConstantBufferLayout = createParameterBlockTypeLayout(
             nullptr,
-            globalScopeStructLayout,
-            globalScopeRules);
+            globalScopeRules,
+            globalScopeRules->GetObjectLayout(ShaderParameterKind::ConstantBuffer),
+            globalScopeStructLayout);
 
         globalScopeLayout = globalConstantBufferLayout;
     }
diff --git a/source/slang/type-layout.cpp b/source/slang/type-layout.cpp
index 2e0f3035a..6ca2d68fb 100644
--- a/source/slang/type-layout.cpp
+++ b/source/slang/type-layout.cpp
@@ -681,28 +681,14 @@ static SimpleLayoutInfo getParameterBlockLayoutInfo(
 RefPtr<ParameterBlockTypeLayout>
 createParameterBlockTypeLayout(
     RefPtr<ParameterBlockType>  parameterBlockType,
-    RefPtr<TypeLayout>          elementTypeLayout,
-    LayoutRulesImpl*            rules)
+    LayoutRulesImpl*            parameterBlockRules,
+    SimpleLayoutInfo            parameterBlockInfo,
+    RefPtr<TypeLayout>          elementTypeLayout)
 {
-    SimpleLayoutInfo info;
-    if (parameterBlockType)
-    {
-        info = getParameterBlockLayoutInfo(
-            parameterBlockType,
-            rules);
-    }
-    else
-    {
-        // If there is no concrete type, then it seems like we are
-        // being asked to compute layout for the global scope
-        info = rules->GetObjectLayout(ShaderParameterKind::ConstantBuffer);
-    }
- 
-
     auto typeLayout = new ParameterBlockTypeLayout();
 
     typeLayout->type = parameterBlockType;
-    typeLayout->rules = rules;
+    typeLayout->rules = parameterBlockRules;
 
     typeLayout->elementTypeLayout = elementTypeLayout;
 
@@ -711,7 +697,7 @@ createParameterBlockTypeLayout(
     // originally (which should be a single binding "slot"
     // and hence no uniform data).
     // 
-    typeLayout->uniformAlignment = info.alignment;
+    typeLayout->uniformAlignment = parameterBlockInfo.alignment;
     assert(!typeLayout->FindResourceInfo(LayoutResourceKind::Uniform));
     assert(typeLayout->uniformAlignment == 1);
 
@@ -725,11 +711,11 @@ createParameterBlockTypeLayout(
 
     // Make sure that we allocate resource usage for the
     // parameter block itself.
-    if( info.size )
+    if( parameterBlockInfo.size )
     {
         typeLayout->addResourceUsage(
-            info.kind,
-            info.size);
+            parameterBlockInfo.kind,
+            parameterBlockInfo.size);
     }
 
     // Now, if the element type itself had any resources, then
@@ -749,6 +735,48 @@ createParameterBlockTypeLayout(
     return typeLayout;
 }
 
+RefPtr<ParameterBlockTypeLayout>
+createParameterBlockTypeLayout(
+    RefPtr<ParameterBlockType>  parameterBlockType,
+    LayoutRulesImpl*            parameterBlockRules,
+    RefPtr<ExpressionType>      elementType,
+    LayoutRulesImpl*            elementTypeRules)
+{
+    // First compute resource usage of the block itself.
+    // For now we assume that the layout of the block can
+    // always be described in a `SimpleLayoutInfo` (only
+    // a single resource kind consumed).
+    SimpleLayoutInfo info;
+    if (parameterBlockType)
+    {
+        info = getParameterBlockLayoutInfo(
+            parameterBlockType,
+            parameterBlockRules);
+    }
+    else
+    {
+        // If there is no concrete type, then it seems like we are
+        // being asked to compute layout for the global scope
+        info = parameterBlockRules->GetObjectLayout(ShaderParameterKind::ConstantBuffer);
+    }
+
+    // Now compute a layout for the elements of the parameter block.
+    // Note that we need to be careful and deal with the case where
+    // the elements of the block use the same resource kind consumed
+    // by the block itself.
+
+    auto elementTypeLayout = CreateTypeLayout(
+        elementType,
+        elementTypeRules,
+        info);
+
+    return createParameterBlockTypeLayout(
+        parameterBlockType,
+        parameterBlockRules,
+        info,
+        elementTypeLayout);
+}
+
 LayoutRulesImpl* getParameterBufferElementTypeLayoutRules(
     RefPtr<ParameterBlockType>  parameterBlockType,
     LayoutRulesImpl*            rules)
@@ -783,22 +811,20 @@ LayoutRulesImpl* getParameterBufferElementTypeLayoutRules(
 RefPtr<ParameterBlockTypeLayout>
 createParameterBlockTypeLayout(
     RefPtr<ParameterBlockType>  parameterBlockType,
-    LayoutRulesImpl*            rules)
+    LayoutRulesImpl*            parameterBlockRules)
 {
     // Determine the layout rules to use for the contents of the block
-    auto parameterBlockLayoutRules = getParameterBufferElementTypeLayoutRules(
+    auto elementTypeRules = getParameterBufferElementTypeLayoutRules(
         parameterBlockType,
-        rules);
+        parameterBlockRules);
 
-    // Create and save type layout for the buffer contents.
-    auto elementTypeLayout = CreateTypeLayout(
-        parameterBlockType->elementType.Ptr(),
-        parameterBlockLayoutRules);
+    auto elementType = parameterBlockType->elementType;
 
     return createParameterBlockTypeLayout(
         parameterBlockType,
-        elementTypeLayout,
-        rules);
+        parameterBlockRules,
+        elementType,
+        elementTypeRules);
 }
 
 // Create a type layout for a structured buffer type.
@@ -863,8 +889,23 @@ createStructuredBufferTypeLayout(
 SimpleLayoutInfo GetLayoutImpl(
     ExpressionType*     type,
     LayoutRulesImpl*    rules,
+    RefPtr<TypeLayout>* outTypeLayout,
+    SimpleLayoutInfo    offset);
+
+SimpleLayoutInfo GetLayoutImpl(
+    ExpressionType*     type,
+    LayoutRulesImpl*    rules,
     RefPtr<TypeLayout>* outTypeLayout)
 {
+    return GetLayoutImpl(type, rules, outTypeLayout, SimpleLayoutInfo());
+}
+
+SimpleLayoutInfo GetLayoutImpl(
+    ExpressionType*     type,
+    LayoutRulesImpl*    rules,
+    RefPtr<TypeLayout>* outTypeLayout,
+    SimpleLayoutInfo    offset)
+{
     if (auto parameterBlockType = type->As<ParameterBlockType>())
     {
         // If the user is just interested in uniform layout info,
@@ -1184,6 +1225,15 @@ SimpleLayoutInfo GetLayoutImpl(
                         fieldResourceInfo->index = structTypeResourceInfo->count;
                         structTypeResourceInfo->count += fieldTypeResourceInfo.count;
                     }
+
+                    // If the user passed in offset info, then apply it here
+                    if (offset.size)
+                    {
+                        if (auto fieldResInfo = fieldLayout->FindResourceInfo(offset.kind))
+                        {
+                            fieldResInfo->index += offset.size;
+                        }
+                    }
                 }
             }
 
@@ -1226,13 +1276,18 @@ SimpleLayoutInfo GetLayout(ExpressionType* inType, LayoutRulesImpl* rules)
     return GetLayoutImpl(inType, rules, nullptr);
 }
 
-RefPtr<TypeLayout> CreateTypeLayout(ExpressionType* type, LayoutRulesImpl* rules)
+RefPtr<TypeLayout> CreateTypeLayout(ExpressionType* type, LayoutRulesImpl* rules, SimpleLayoutInfo offset)
 {
     RefPtr<TypeLayout> typeLayout;
-    GetLayoutImpl(type, rules, &typeLayout);
+    GetLayoutImpl(type, rules, &typeLayout, offset);
     return typeLayout;
 }
 
+RefPtr<TypeLayout> CreateTypeLayout(ExpressionType* type, LayoutRulesImpl* rules)
+{
+    return CreateTypeLayout(type, rules, SimpleLayoutInfo());
+}
+
 SimpleLayoutInfo GetLayout(ExpressionType* type, LayoutRule rule)
 {
     LayoutRulesImpl* rulesImpl = GetLayoutRulesImpl(rule);
diff --git a/source/slang/type-layout.h b/source/slang/type-layout.h
index d2254c9e5..d5c96abf2 100644
--- a/source/slang/type-layout.h
+++ b/source/slang/type-layout.h
@@ -535,6 +535,7 @@ SimpleLayoutInfo GetLayout(ExpressionType* type, LayoutRulesImpl* rules);
 SimpleLayoutInfo GetLayout(ExpressionType* type, LayoutRule rule = LayoutRule::Std430);
 
 RefPtr<TypeLayout> CreateTypeLayout(ExpressionType* type, LayoutRulesImpl* rules);
+RefPtr<TypeLayout> CreateTypeLayout(ExpressionType* type, LayoutRulesImpl* rules, SimpleLayoutInfo offset);
 
 //
 
@@ -544,15 +545,19 @@ createParameterBlockTypeLayout(
     RefPtr<ParameterBlockType>  parameterBlockType,
     LayoutRulesImpl*            rules);
 
-// Create a type layout for a constant buffer type,
-// in the case where we already know the layout
-// for the element type.
 RefPtr<ParameterBlockTypeLayout>
 createParameterBlockTypeLayout(
     RefPtr<ParameterBlockType>  parameterBlockType,
-    RefPtr<TypeLayout>          elementTypeLayout,
-    LayoutRulesImpl*            rules);
+    LayoutRulesImpl*            parameterBlockRules,
+    RefPtr<ExpressionType>      elementType,
+    LayoutRulesImpl*            elementTypeRules);
 
+RefPtr<ParameterBlockTypeLayout>
+createParameterBlockTypeLayout(
+    RefPtr<ParameterBlockType>  parameterBlockType,
+    LayoutRulesImpl*            parameterBlockRules,
+    SimpleLayoutInfo            parameterBlockInfo,
+    RefPtr<TypeLayout>          elementTypeLayout);
 
 // Create a type layout for a structured buffer type.
 RefPtr<StructuredBufferTypeLayout>