Add support for global uniforms in SPIR-V and non-bound textures.

* This isn't relevant for Vulkan, but in GL it's valid to not specify a binding
  and fetch it at runtime (and even if a binding is specified, it's not
  immutable and can be changed).
* Similarly GL allows bare uniforms that aren't in a buffer, which we handle in
  the same way by wrapping them into a $Globals UBO.

qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp

@@ -893,7 +893,7 @@ void GLPipelineStateViewer::setShaderState(const GLPipe::Shader &stage, RDLabel
     {
       for(const ConstantBlock &bind : shaderDetails->constantBlocks)
       {
-        if(mapping.constantBlocks[bind.bindPoint].bind == i)
+        if(bind.bufferBacked && mapping.constantBlocks[bind.bindPoint].bind == i)
         {
           shaderCBuf = &bind;
           map = &mapping.constantBlocks[bind.bindPoint];

renderdoc/driver/gl/gl_common.h

@@ -524,6 +524,9 @@ void ClearGLErrors();
 GLuint GetBoundVertexBuffer(GLuint idx);
 GLint GetNumVertexBuffers();
 
+void EvaluateSPIRVBindpointMapping(GLuint curProg, int shadIdx, ShaderReflection *refl,
+                                   ShaderBindpointMapping &mapping);
+
 void GetBindpointMapping(GLuint curProg, int shadIdx, ShaderReflection *refl,
                          ShaderBindpointMapping &mapping);
 

renderdoc/driver/gl/gl_replay.cpp

@@ -696,7 +696,7 @@ string GLReplay::DisassembleShader(ResourceId pipeline, const ShaderReflection *
   auto &shaderDetails =
       m_pDriver->m_Shaders[m_pDriver->GetResourceManager()->GetLiveID(refl->resourceId)];
 
-  if(shaderDetails.sources.empty())
+  if(shaderDetails.sources.empty() && shaderDetails.spirvWords.empty())
     return "; Invalid Shader Specified";
 
   if(target == SPIRVDisassemblyTarget || target.empty())
@@ -960,6 +960,8 @@ void GLReplay::SavePipelineState()
           {
             stages[i]->bindpointMapping = shaderDetails.mapping;
             spirv[i] = true;
+
+            EvaluateSPIRVBindpointMapping(curProg, (int)i, refls[i], stages[i]->bindpointMapping);
           }
           else
           {
@@ -1000,6 +1002,8 @@ void GLReplay::SavePipelineState()
         {
           stages[i]->bindpointMapping = shaderDetails.mapping;
           spirv[i] = true;
+
+          EvaluateSPIRVBindpointMapping(curProg, (int)i, refls[i], stages[i]->bindpointMapping);
         }
         else
         {
@@ -2121,6 +2125,10 @@ void GLReplay::FillCBufferVariables(ResourceId shader, string entryPoint, uint32
 
       FillSpecConstantVariables(cblock.variables, outvars, specconsts);
     }
+    else if(!cblock.bufferBacked)
+    {
+      FillCBufferVariables(curProg, false, "", cblock.variables, outvars, data);
+    }
     else
     {
       SPIRVFillCBufferVariables(cblock.variables, outvars, data, 0);

renderdoc/driver/gl/gl_shader_refl.cpp

@@ -2006,7 +2006,7 @@ void GetBindpointMapping(GLuint curProg, int shadIdx, ShaderReflection *refl,
     return;
   }
 
-  // in case of bugs, we readback into this array instead of
+  // in case of bugs, we readback into this array instead of a single int
   GLint dummyReadback[32];
 
 #if ENABLED(RDOC_DEVEL)
@@ -2286,6 +2286,111 @@ void GetBindpointMapping(GLuint curProg, int shadIdx, ShaderReflection *refl,
 #endif
 }
 
+void EvaluateSPIRVBindpointMapping(GLuint curProg, int shadIdx, ShaderReflection *refl,
+                                   ShaderBindpointMapping &mapping)
+{
+  // this is similar in principle to GetBindpointMapping - we want to look up the actual uniform
+  // values right now and replace the bindpoint mapping list. However for SPIR-V we can't call
+  // glGetUniformLocation. Instead we assume the *current* bind value is a location, and we
+  // overwrite it with the read uniform value.
+
+  // in case of bugs, we readback into this array instead of a single int
+  GLint dummyReadback[32];
+
+#if ENABLED(RDOC_DEVEL)
+  for(size_t i = 1; i < ARRAY_COUNT(dummyReadback); i++)
+    dummyReadback[i] = 0x6c7b8a9d;
+#endif
+
+  // GL_ARB_gl_spirv spec says that glBindAttribLocation does nothing on SPIR-V, so we don't have to
+  // remap inputAttributes.
+
+  // It's fuzzy on whether UBOs can be remapped with glUniformBlockBinding so for now we hope that
+  // anyone using UBOs and SPIR-V will at least specify immutable bindings in the SPIR-V.
+  for(size_t i = 0; i < mapping.constantBlocks.size(); i++)
+  {
+    Bindpoint &bind = mapping.constantBlocks[i];
+
+    if(!bind.used)
+      continue;
+
+    if(bind.bind < 0)
+    {
+      RDCERR("Invalid constant block binding found: '%s' = %d",
+             refl->constantBlocks[i].name.c_str(), bind.bind);
+      bind.bind = 0;
+    }
+  }
+
+  // shouldn't have any separate samplers - this is GL
+  RDCASSERT(mapping.samplers.size() == 0);
+
+  // for other resources we handle textures only, other resource types are assumed to have valid fix
+  // binds already. Any negative inputs are locations, so get the uniform value and assign it as the
+  // binding index.
+  for(size_t i = 0; i < refl->readOnlyResources.size(); i++)
+  {
+    if(!mapping.readOnlyResources[i].used)
+      continue;
+
+    if(refl->readOnlyResources[i].isTexture && mapping.readOnlyResources[i].bind < 0)
+    {
+      GL.glGetUniformiv(curProg, -mapping.readOnlyResources[i].bind, dummyReadback);
+      mapping.readOnlyResources[i].bind = dummyReadback[0];
+
+      if(mapping.readOnlyResources[i].bind < 0)
+      {
+        RDCERR("Invalid uniform value retrieved: '%s' = %d",
+               refl->readOnlyResources[i].name.c_str(), mapping.readOnlyResources[i].bind);
+        mapping.readOnlyResources[i].bind = 0;
+      }
+    }
+    else
+    {
+      if(mapping.readOnlyResources[i].bind < 0)
+      {
+        RDCERR("Invalid read-only resource binding found: '%s' = %d",
+               refl->readOnlyResources[i].name.c_str(), mapping.readOnlyResources[i].bind);
+        mapping.readOnlyResources[i].bind = 0;
+      }
+    }
+  }
+
+  for(size_t i = 0; i < refl->readWriteResources.size(); i++)
+  {
+    if(!mapping.readWriteResources[i].used)
+      continue;
+
+    if(refl->readWriteResources[i].isTexture && mapping.readWriteResources[i].bind < 0)
+    {
+      GL.glGetUniformiv(curProg, -mapping.readWriteResources[i].bind, dummyReadback);
+      mapping.readWriteResources[i].bind = dummyReadback[0];
+
+      if(mapping.readWriteResources[i].bind < 0)
+      {
+        RDCERR("Invalid uniform value retrieved: '%s' = %d",
+               refl->readWriteResources[i].name.c_str(), mapping.readWriteResources[i].bind);
+        mapping.readWriteResources[i].bind = 0;
+      }
+    }
+    else
+    {
+      if(mapping.readWriteResources[i].bind < 0)
+      {
+        RDCERR("Invalid read-only resource binding found: '%s' = %d",
+               refl->readWriteResources[i].name.c_str(), mapping.readWriteResources[i].bind);
+        mapping.readWriteResources[i].bind = 0;
+      }
+    }
+  }
+
+#if ENABLED(RDOC_DEVEL)
+  for(size_t i = 1; i < ARRAY_COUNT(dummyReadback); i++)
+    if(dummyReadback[i] != 0x6c7b8a9d)
+      RDCERR("Invalid uniform readback - data beyond first element modified!");
+#endif
+}
+
 // first int - the mapping index, second int - the binding
 typedef std::vector<std::pair<size_t, int> > Permutation;
 

renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp

@@ -69,7 +69,8 @@ void WrappedOpenGL::ShaderData::ProcessSPIRVCompilation(WrappedOpenGL &drv, Reso
   // we discard this too, because we don't need it - we don't do any SPIR-V patching in GL
   SPIRVPatchData patchData;
 
-  spirv.MakeReflection(ShaderStage(ShaderIdx(type)), pEntryPoint, reflection, mapping, patchData);
+  spirv.MakeReflection(GraphicsAPI::OpenGL, ShaderStage(ShaderIdx(type)), pEntryPoint, reflection,
+                       mapping, patchData);
 
   version = 460;
 

renderdoc/driver/shaders/spirv/spirv_common.h

@@ -69,6 +69,7 @@ void ShutdownSPIRVCompiler();
 
 struct SPVInstruction;
 
+enum class GraphicsAPI : uint32_t;
 enum class ShaderStage : uint32_t;
 enum class ShaderBuiltin : uint32_t;
 struct ShaderReflection;
@@ -145,8 +146,9 @@ struct SPVModule
   std::vector<std::string> EntryPoints() const;
   ShaderStage StageForEntry(const string &entryPoint) const;
 
-  void MakeReflection(ShaderStage stage, const string &entryPoint, ShaderReflection &reflection,
-                      ShaderBindpointMapping &mapping, SPIRVPatchData &patchData) const;
+  void MakeReflection(GraphicsAPI sourceAPI, ShaderStage stage, const string &entryPoint,
+                      ShaderReflection &reflection, ShaderBindpointMapping &mapping,
+                      SPIRVPatchData &patchData) const;
 };
 
 string CompileSPIRV(const SPIRVCompilationSettings &settings, const vector<string> &sources,

renderdoc/driver/shaders/spirv/spirv_disassemble.cpp

@@ -75,6 +75,8 @@ namespace spv
 // show the offset/arraystride/matrixstride decorations for structure packing
 #define SHOW_STRUCT_PACKING 0
 
+#define INVALID_BIND -INT_MAX
+
 namespace spv
 {
 Op OpUnknown = (Op)~0U;
@@ -3758,12 +3760,12 @@ struct bindpair
     if(map.bindset != o.map.bindset)
       return map.bindset < o.map.bindset;
 
-    // sort -1 to the end
-    if(map.bind == -1 && o.map.bind == -1)    // equal
+    // sort INVALID_BIND to the end
+    if(map.bind == INVALID_BIND && o.map.bind == INVALID_BIND)    // equal
       return false;
-    if(map.bind == -1)    // -1 not less than anything
+    if(map.bind == INVALID_BIND)    // INVALID_BIND not less than anything
       return false;
-    if(o.map.bind == -1)    // anything less than -1
+    if(o.map.bind == INVALID_BIND)    // anything less than INVALID_BIND
       return true;
 
     return map.bind < o.map.bind;
@@ -3996,7 +3998,7 @@ ShaderStage SPVModule::StageForEntry(const string &entryPoint) const
   return ShaderStage::Count;
 }
 
-void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
+void SPVModule::MakeReflection(GraphicsAPI sourceAPI, ShaderStage stage, const string &entryPoint,
                                ShaderReflection &reflection, ShaderBindpointMapping &mapping,
                                SPIRVPatchData &patchData) const
 {
@@ -4038,6 +4040,8 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
   reflection.dispatchThreadsDimension[1] = 0;
   reflection.dispatchThreadsDimension[2] = 0;
 
+  ConstantBlock globalsblock;
+
   for(size_t i = 0; i < globals.size(); i++)
   {
     SPVInstruction *inst = globals[i];
@@ -4211,15 +4215,42 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
       if(type->type == SPVTypeData::ePointer)
         type = type->baseType;
 
+      bool isArray = false;
       uint32_t arraySize = 1;
       if(type->type == SPVTypeData::eArray)
       {
+        isArray = true;
         if(type->arraySize != ~0U)
           arraySize = type->arraySize;
         type = type->baseType;
       }
 
-      if(type->type == SPVTypeData::eStruct)
+      if(type->type < SPVTypeData::eCompositeCount)
+      {
+        // global loose variable - add to $Globals block
+        RDCASSERT(type->type != SPVTypeData::ePointer);
+        RDCASSERT(sourceAPI == GraphicsAPI::OpenGL);
+
+        ShaderConstant constant;
+
+        MakeConstantBlockVariable(constant, type, inst->str, inst->decorations);
+
+        if(isArray)
+          constant.type.descriptor.elements = arraySize;
+        else
+          constant.type.descriptor.elements = 0;
+
+        for(size_t d = 0; d < inst->decorations.size(); d++)
+        {
+          if(inst->decorations[d].decoration == spv::DecorationLocation)
+            constant.reg.vec = (int32_t)inst->decorations[d].val;
+        }
+
+        constant.reg.comp = 0;
+
+        globalsblock.variables.push_back(constant);
+      }
+      else if(type->type == SPVTypeData::eStruct)
       {
         ConstantBlock cblock;
 
@@ -4233,9 +4264,9 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
 
         Bindpoint bindmap;
         // set can be implicitly 0, but the binding must be set explicitly.
-        // If no binding is found, we set -1 and sort to the end of the resources
+        // If no binding is found, we set INVALID_BIND and sort to the end of the resources
         // list as it's not bound anywhere (most likely, declared but not used)
-        bindmap.bind = -1;
+        bindmap.bind = INVALID_BIND;
 
         bool ssbo = false;
 
@@ -4304,9 +4335,22 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
           }
         }
 
-        // should never have elements that have no binding declared but
-        // are used, unless it's push constants (which is handled elsewhere)
-        RDCASSERT(!bindmap.used || !cblock.bufferBacked || bindmap.bind >= 0);
+        // we should always have a location. Put that in as the bind, it will be overwritten
+        // dynamically with the actual value.
+        if(sourceAPI == GraphicsAPI::OpenGL)
+        {
+          for(size_t d = 0; d < inst->decorations.size(); d++)
+          {
+            if(inst->decorations[d].decoration == spv::DecorationLocation)
+              bindmap.bind = -(int32_t)inst->decorations[d].val;
+          }
+        }
+
+        // on Vulkan should never have elements that have no binding declared but are used, unless
+        // it's push constants (which is handled elsewhere). On GL we should have gotten a location
+        // above, which will be rewritten later when looking up the pipeline state since it's
+        // mutable from draw to draw in theory.
+        RDCASSERT(!bindmap.used || !cblock.bufferBacked || bindmap.bind != INVALID_BIND);
 
         if(ssbo)
           rwresources.push_back(shaderrespair(bindmap, res));
@@ -4395,9 +4439,9 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
 
         Bindpoint bindmap;
         // set can be implicitly 0, but the binding must be set explicitly.
-        // If no binding is found, we set -1 and sort to the end of the resources
+        // If no binding is found, we set INVALID_BIND and sort to the end of the resources
         // list as it's not bound anywhere (most likely, declared but not used)
-        bindmap.bind = -1;
+        bindmap.bind = INVALID_BIND;
 
         for(size_t d = 0; d < inst->decorations.size(); d++)
         {
@@ -4426,9 +4470,22 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
           }
         }
 
-        // should never have elements that have no binding declared but
-        // are used
-        RDCASSERT(!bindmap.used || bindmap.bind >= 0);
+        // we should always have a location. Put that in as the bind, it will be overwritten
+        // dynamically with the actual value.
+        if(sourceAPI == GraphicsAPI::OpenGL)
+        {
+          for(size_t d = 0; d < inst->decorations.size(); d++)
+          {
+            if(inst->decorations[d].decoration == spv::DecorationLocation)
+              bindmap.bind = -(int32_t)inst->decorations[d].val;
+          }
+        }
+
+        // on Vulkan should never have elements that have no binding declared but are used, unless
+        // it's push constants (which is handled elsewhere). On GL we should have gotten a location
+        // above, which will be rewritten later when looking up the pipeline state since it's
+        // mutable from draw to draw in theory.
+        RDCASSERT(!bindmap.used || bindmap.bind != INVALID_BIND);
 
         if(sepSampler)
           samplers.push_back(shaderrespair(bindmap, res));
@@ -4463,7 +4520,7 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
     // set something crazy so this doesn't overlap with a real buffer binding
     // also identify this as specialization constant data
     bindmap.bindset = SpecializationConstantBindSet;
-    bindmap.bind = -1;
+    bindmap.bind = INVALID_BIND;
     bindmap.arraySize = 1;
     bindmap.used = true;
 
@@ -4505,6 +4562,21 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
     cblocks.push_back(cblockpair(bindmap, cblock));
   }
 
+  if(!globalsblock.variables.empty())
+  {
+    globalsblock.name = "$Globals";
+    globalsblock.bufferBacked = false;
+    globalsblock.bindPoint = (int)cblocks.size();
+
+    Bindpoint bindmap;
+    bindmap.bindset = 0;
+    bindmap.bind = INVALID_BIND;
+    bindmap.arraySize = 1;
+    bindmap.used = true;
+
+    cblocks.push_back(cblockpair(bindmap, globalsblock));
+  }
+
   // look for execution modes that affect the reflection and apply them
   for(SPVInstruction *inst : entries)
   {
@@ -4624,7 +4696,7 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
 
   mapping.inputAttributes.resize(numInputs);
   for(size_t i = 0; i < numInputs; i++)
-    mapping.inputAttributes[i] = -1;
+    mapping.inputAttributes[i] = INVALID_BIND;
 
   for(size_t i = 0; i < reflection.inputSignature.size(); i++)
     if(reflection.inputSignature[i].systemValue == ShaderBuiltin::Undefined)
@@ -4650,10 +4722,10 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
   for(size_t i = 0; i < cblocks.size(); i++)
   {
     mapping.constantBlocks[i] = cblocks[i].map;
-    // fix up any bind points marked with -1. They were sorted to the end
+    // fix up any bind points marked with INVALID_BIND. They were sorted to the end
     // but from here on we want to just be able to index with the bind point
     // without any special casing.
-    if(mapping.constantBlocks[i].bind == -1)
+    if(mapping.constantBlocks[i].bind == INVALID_BIND)
       mapping.constantBlocks[i].bind = 0;
     reflection.constantBlocks[i] = cblocks[i].bindres;
     reflection.constantBlocks[i].bindPoint = (int32_t)i;
@@ -4662,10 +4734,10 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
   for(size_t i = 0; i < samplers.size(); i++)
   {
     mapping.samplers[i] = samplers[i].map;
-    // fix up any bind points marked with -1. They were sorted to the end
+    // fix up any bind points marked with INVALID_BIND. They were sorted to the end
     // but from here on we want to just be able to index with the bind point
     // without any special casing.
-    if(mapping.samplers[i].bind == -1)
+    if(mapping.samplers[i].bind == INVALID_BIND)
       mapping.samplers[i].bind = 0;
     reflection.samplers[i].name = samplers[i].bindres.name;
     reflection.samplers[i].bindPoint = (int32_t)i;
@@ -4674,10 +4746,10 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
   for(size_t i = 0; i < roresources.size(); i++)
   {
     mapping.readOnlyResources[i] = roresources[i].map;
-    // fix up any bind points marked with -1. They were sorted to the end
+    // fix up any bind points marked with INVALID_BIND. They were sorted to the end
     // but from here on we want to just be able to index with the bind point
     // without any special casing.
-    if(mapping.readOnlyResources[i].bind == -1)
+    if(mapping.readOnlyResources[i].bind == INVALID_BIND)
       mapping.readOnlyResources[i].bind = 0;
     reflection.readOnlyResources[i] = roresources[i].bindres;
     reflection.readOnlyResources[i].bindPoint = (int32_t)i;
@@ -4686,10 +4758,10 @@ void SPVModule::MakeReflection(ShaderStage stage, const string &entryPoint,
   for(size_t i = 0; i < rwresources.size(); i++)
   {
     mapping.readWriteResources[i] = rwresources[i].map;
-    // fix up any bind points marked with -1. They were sorted to the end
+    // fix up any bind points marked with INVALID_BIND. They were sorted to the end
     // but from here on we want to just be able to index with the bind point
     // without any special casing.
-    if(mapping.readWriteResources[i].bind == -1)
+    if(mapping.readWriteResources[i].bind == INVALID_BIND)
       mapping.readWriteResources[i].bind = 0;
     reflection.readWriteResources[i] = rwresources[i].bindres;
     reflection.readWriteResources[i].bindPoint = (int32_t)i;

renderdoc/driver/vulkan/vk_info.cpp

@@ -859,7 +859,8 @@ void VulkanCreationInfo::ShaderModule::Reflection::Init(VulkanResourceManager *r
     entryPoint = entry;
     stageIndex = StageIndex(stage);
 
-    spv.MakeReflection(ShaderStage(stageIndex), entryPoint, refl, mapping, patchData);
+    spv.MakeReflection(GraphicsAPI::Vulkan, ShaderStage(stageIndex), entryPoint, refl, mapping,
+                       patchData);
 
     refl.resourceId = resourceMan->GetOriginalID(id);
     refl.entryPoint = entryPoint;