Fix for vk-shift-* GLSL emit issue (#3004)

* Handle different resource kinds that can appear via the vk-shift-* allowing some HLSL kinds in GLSL emit.

* Determine the used kind for emit.

* Added vk-shift-uniform-issue.slang

* Use a better function name.
Improve comments.

2 files changed, 136 insertions, 8 deletions

diff --git a/source/slang/slang-emit-glsl.cpp b/source/slang/slang-emit-glsl.cpp
index 853ef69ae..b378f69dc 100644
--- a/source/slang/slang-emit-glsl.cpp
+++ b/source/slang/slang-emit-glsl.cpp
@@ -145,6 +145,28 @@ void GLSLSourceEmitter::_requireGLSLVersion(int version)
     }
 }
 
+
+// This function was needed because the vk-shift-* change allows kinds which are "in effect" DescriptorSlots but appear as HLSL
+// kinds. 
+// 
+// This seems safe as a test, because in the description of `usesResourceKind`, it appears that VarLayout require offsets even if
+// it's zero. 
+static LayoutResourceKind _findUsedDescriptorSlotLikeKind(IRVarLayout* layout, LayoutResourceKind kind)
+{
+    // DescriptorTableSlot is the most likely, so look for that first
+    if (layout->usesResourceKind(LayoutResourceKind::DescriptorTableSlot))
+    {
+        return LayoutResourceKind::DescriptorTableSlot;
+    }
+    else if (layout->usesResourceKind(kind))
+    {
+        // If we find the optional kind use that
+        return kind;
+    }
+    // We'll just assume descriptor slot then
+    return LayoutResourceKind::DescriptorTableSlot;
+}
+
 void GLSLSourceEmitter::_emitGLSLStructuredBuffer(IRGlobalParam* varDecl, IRHLSLStructuredBufferTypeBase* structuredBufferType)
 {
     // Shader storage buffer is an OpenGL 430 feature
@@ -158,11 +180,11 @@ void GLSLSourceEmitter::_emitGLSLStructuredBuffer(IRGlobalParam* varDecl, IRHLSL
     auto layout = getVarLayout(varDecl);
     if (layout)
     {
-        LayoutResourceKind kind = LayoutResourceKind::DescriptorTableSlot;
+        const LayoutResourceKind usedKind = _findUsedDescriptorSlotLikeKind(layout, LayoutResourceKind::ShaderResource);
         EmitVarChain chain(layout);
 
-        const UInt index = getBindingOffset(&chain, kind);
-        const UInt space = getBindingSpace(&chain, kind);
+        const UInt index = getBindingOffset(&chain, usedKind);
+        const UInt space = getBindingSpace(&chain, usedKind);
 
         m_writer->emit(", binding = ");
         m_writer->emit(index);
@@ -216,6 +238,7 @@ void GLSLSourceEmitter::_emitGLSLStructuredBuffer(IRGlobalParam* varDecl, IRHLSL
     m_writer->emit(";\n");
 }
 
+
 void GLSLSourceEmitter::_emitGLSLByteAddressBuffer(IRGlobalParam* varDecl, IRByteAddressBufferTypeBase* byteAddressBufferType)
 {
     // TODO: A lot of this logic is copy-pasted from `emitIRStructuredBuffer_GLSL`.
@@ -233,11 +256,12 @@ void GLSLSourceEmitter::_emitGLSLByteAddressBuffer(IRGlobalParam* varDecl, IRByt
     auto layout = getVarLayout(varDecl);
     if (layout)
     {
-        LayoutResourceKind kind = LayoutResourceKind::DescriptorTableSlot;
+        const LayoutResourceKind usedKind = _findUsedDescriptorSlotLikeKind(layout, LayoutResourceKind::ShaderResource);
+
         EmitVarChain chain(layout);
 
-        const UInt index = getBindingOffset(&chain, kind);
-        const UInt space = getBindingSpace(&chain, kind);
+        const UInt index = getBindingOffset(&chain, usedKind);
+        const UInt space = getBindingSpace(&chain, usedKind);
 
         m_writer->emit(", binding = ");
         m_writer->emit(index);
@@ -310,7 +334,16 @@ void GLSLSourceEmitter::_emitGLSLParameterGroup(IRGlobalParam* varDecl, IRUnifor
     or IRGLSLShaderStorageBufferType which is read write.
     */
 
-    _emitGLSLLayoutQualifier(LayoutResourceKind::DescriptorTableSlot, &containerChain);
+    {
+        // TODO(JS):
+        // The *assumption* here is that we only need to detect the LayoutResourceKind as used on the initial containerChain.varLayout
+        // rather than search up the chain. 
+        //
+        // Perhaps there is still an issue here, because perhaps it's possible (?) to have a mixture of ConstantBuffer/DescriptorSlot
+        // and in that case the usedKind would just restrict to one and so produce the wrong answer.
+        const auto usedKind = _findUsedDescriptorSlotLikeKind(containerChain.varLayout, LayoutResourceKind::ConstantBuffer);
+        _emitGLSLLayoutQualifier(usedKind, &containerChain);
+    }
     _emitGLSLLayoutQualifier(LayoutResourceKind::PushConstantBuffer, &containerChain);
     bool isShaderRecord = _emitGLSLLayoutQualifier(LayoutResourceKind::ShaderRecord, &containerChain);
 
@@ -598,6 +631,7 @@ bool GLSLSourceEmitter::_emitGLSLLayoutQualifier(LayoutResourceKind kind, EmitVa
         case LayoutResourceKind::ShaderResource:
         case LayoutResourceKind::UnorderedAccess:
         case LayoutResourceKind::SamplerState:
+
         case LayoutResourceKind::DescriptorTableSlot:
             m_writer->emit("layout(binding = ");
             m_writer->emit(index);
@@ -1372,8 +1406,16 @@ void GLSLSourceEmitter::emitLayoutQualifiersImpl(IRVarLayout* layout)
     {
         switch (rr->getResourceKind())
         {
-            case LayoutResourceKind::Uniform:
+            // These can occur for vk-shift-* scenarios, and are in effect equivalent to DescriptorTableSlot
             case LayoutResourceKind::ShaderResource:
+            case LayoutResourceKind::ConstantBuffer:
+            case LayoutResourceKind::SamplerState:
+            case LayoutResourceKind::UnorderedAccess:
+
+            // 
+            case LayoutResourceKind::Uniform:
+
+            //
             case LayoutResourceKind::DescriptorTableSlot:
                 m_writer->emit("uniform ");
                 break;
diff --git a/tests/bugs/vk-shift-uniform-issue.slang b/tests/bugs/vk-shift-uniform-issue.slang
new file mode 100644
index 000000000..950a01c6f
--- /dev/null
+++ b/tests/bugs/vk-shift-uniform-issue.slang
@@ -0,0 +1,86 @@
+//TEST:SIMPLE(filecheck=CHECK):-target glsl -profile ps_4_0 -entry main -fvk-t-shift 10 all  -fvk-s-shift 100 all -fvk-u-shift 100 all -fvk-b-shift 1000 all
+
+// CHECK:layout(binding = 10)
+// CHECK:uniform texture2D texture0_0;
+
+// CHECK:layout(binding = 100)
+// CHECK:uniform sampler sampler0_0;
+
+// CHECK:layout(binding = 11, set = 2)
+// CHECK:uniform texture2D texture1_0;
+
+// CHECK:layout(binding = 101, set = 2)
+// CHECK:uniform sampler sampler1_0;
+
+// CHECK:layout(binding = 1004)
+// CHECK:layout(std140) uniform _S1
+
+// CHECK:layout(binding = 1003)
+// CHECK:layout(std140) uniform _S2
+
+// CHECK:layout(binding = 1002)
+// CHECK:layout(std140) uniform _S3
+
+// CHECK:layout(binding = 1001)
+// CHECK:layout(std140) uniform _S4
+
+// CHECK:layout(binding = 0)
+// CHECK:layout(std140) uniform _S5
+
+Texture2D texture0; 
+SamplerState sampler0;
+
+Texture2D texture1 : register(t1, space2); 
+SamplerState sampler1 : register(s1, space2); 
+
+float g_value;
+
+cbuffer ConstantBufferA 
+{ 
+    float constantA;
+}; 
+
+cbuffer ConstantBufferB 
+{ 
+    float constantB;
+}; 
+
+cbuffer ConstantBufferC
+{
+    float constantC; 
+}; 
+
+cbuffer ConstantBufferD
+{ 
+    float constantD;
+}; 
+
+struct StructA 
+{ 
+    float a;
+};
+ 
+[[ vk::push_constant ]] 
+StructA pushConstantA;
+  
+struct PixelInput
+{ 
+    float4 t : TEXCOORD0; 
+}; 
+
+struct PixelOutput
+{ 
+    float4 color : SV_Target0; 
+}; 
+
+float4 use(Texture2D t, SamplerState s) { return t.SampleLevel(s, 0.0, 0.0); }
+
+PixelOutput main(PixelInput i)
+{ 
+    PixelOutput o ; 
+    float4 b = use(texture0, sampler0) + use(texture1, sampler1);
+    float a = pushConstantA.a + constantD + constantC + constantB + constantA + g_value + length(b);
+    o.color = float4(1, 2, 3, 4) * a;
+    return o ; 
+} 
+