From 7490abedcf5ecb4f36b2be025d9e4c72d7a8b2c1 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 19 Nov 2025 16:37:31 +0000 Subject: [PATCH] Remap locations/binds back to match real shader as needed --- renderdoc/api/replay/renderdoc_tostr.inl | 1 + renderdoc/api/replay/replay_enums.h | 7 + renderdoc/driver/gl/gl_shaderdebug.cpp | 86 ++++++- .../driver/gl/wrappers/gl_shader_funcs.cpp | 234 +++++++++++++++++- .../driver/shaders/spirv/spirv_reflect.cpp | 7 +- 5 files changed, 327 insertions(+), 8 deletions(-) diff --git a/renderdoc/api/replay/renderdoc_tostr.inl b/renderdoc/api/replay/renderdoc_tostr.inl index 3174747af..b165da697 100644 --- a/renderdoc/api/replay/renderdoc_tostr.inl +++ b/renderdoc/api/replay/renderdoc_tostr.inl @@ -1209,6 +1209,7 @@ rdcstr DoStringise(const ShaderVariableFlags &el) STRINGISE_BITFIELD_CLASS_BIT(SNorm); STRINGISE_BITFIELD_CLASS_BIT(Truncated); STRINGISE_BITFIELD_CLASS_BIT(SignedEnum); + STRINGISE_BITFIELD_CLASS_BIT(SingleElementArray); } END_BITFIELD_STRINGISE(); } diff --git a/renderdoc/api/replay/replay_enums.h b/renderdoc/api/replay/replay_enums.h index 69b4fd81c..6b809848c 100644 --- a/renderdoc/api/replay/replay_enums.h +++ b/renderdoc/api/replay/replay_enums.h @@ -4943,6 +4943,12 @@ displayed .. data:: SignedEnum For enums, the base type is a signed integer allowing signed values. + +.. data:: SingleElementArray + + In some cases it can be possible to represent and need to distinguish a variable declared as + an array of size 1 from a scalar object. This flag indicates that even though elements == 1 + normally means 'not an array', in this case it means an array of size 1. )"); enum class ShaderVariableFlags : uint32_t { @@ -4957,6 +4963,7 @@ enum class ShaderVariableFlags : uint32_t SNorm = 0x0080, Truncated = 0x0100, SignedEnum = 0x0200, + SingleElementArray = 0x0400, }; BITMASK_OPERATORS(ShaderVariableFlags); diff --git a/renderdoc/driver/gl/gl_shaderdebug.cpp b/renderdoc/driver/gl/gl_shaderdebug.cpp index 4fcbb7aeb..f047c12a9 100644 --- a/renderdoc/driver/gl/gl_shaderdebug.cpp +++ b/renderdoc/driver/gl/gl_shaderdebug.cpp @@ -51,11 +51,38 @@ RDOC_CONFIG(bool, OpenGL_Debug_ShaderDebugLogging, false, class GLAPIWrapper : public rdcspv::DebugAPIWrapper { public: - GLAPIWrapper(WrappedOpenGL *gl, ShaderStage stage, uint32_t eid, ResourceId shadId) + GLAPIWrapper(WrappedOpenGL *gl, ShaderStage stage, uint32_t eid, ResourceId shadId, + const ShaderReflection *autoMappedRefl) : m_EventID(eid), m_ShaderID(shadId), deviceThreadID(Threading::GetCurrentID()) { m_pDriver = gl; + // if we were passed a reflection, we need to look up in the existing program by name all the + // uniform locations since they may not match what glslang assigned out + // this is only necessary for bare uniforms: resources like textures are referenced by their interface + // index (via the DescriptorAccess) not whatever binding they might have been given, and input + // variables are filled out without ever going to the real program via the input fetcher + if(autoMappedRefl) + { + GLint prog = 0; + GL.glGetIntegerv(eGL_CURRENT_PROGRAM, &prog); + for(const ConstantBlock &cblock : autoMappedRefl->constantBlocks) + { + // uniforms need in-depth handling of their own to query out all locations + if(!cblock.bufferBacked && cblock.name == "$Globals") + { + const rdcstr prefix; + for(const ShaderConstant &c : cblock.variables) + { + // only the base value has a location so we just set it here, then GetUniformMapping + // increments as it goes + int location = c.byteOffset; + GetUniformMapping(prog, location, prefix, c); + } + } + } + } + // when we're first setting up, the state is pristine and no replay is needed m_ResourcesDirty = false; @@ -179,6 +206,15 @@ public: GLint prog = 0; GL.glGetIntegerv(eGL_CURRENT_PROGRAM, &prog); + if(!m_UniformLocationRemap.empty()) + location = m_UniformLocationRemap[location]; + + if(location < 0) + { + var.value.u8v.clear(); + return; + } + if(var.type == VarType::Float) { GL.glGetUniformfv(prog, location, var.value.f32v.data()); @@ -1400,6 +1436,8 @@ private: uint32_t m_EventID; ResourceId m_ShaderID; + std::map m_UniformLocationRemap; + rdcarray m_Access; rdcarray m_Descriptors; rdcarray m_SamplerDescriptors; @@ -1431,6 +1469,49 @@ private: Threading::RWLock imageCacheLock; std::map imageCache; + void GetUniformMapping(GLint prog, int &location, const rdcstr &prefix, const ShaderConstant &c) + { + rdcstr name = prefix + c.name; + + if(c.type.members.empty()) + { + if(c.type.elements > 1 || (c.type.flags & ShaderVariableFlags::SingleElementArray)) + { + for(uint32_t e = 0; e < c.type.elements; e++) + { + rdcstr elemName = StringFormat::Fmt("%s[%u]", name.c_str(), e); + GLint loc = GL.glGetUniformLocation(prog, elemName.c_str()); + + m_UniformLocationRemap[location] = loc; + + location++; + } + } + else + { + GLint loc = GL.glGetUniformLocation(prog, name.c_str()); + + m_UniformLocationRemap[location] = loc; + + location++; + } + } + else + { + rdcstr prefixName = name; + for(uint32_t e = 0; e < c.type.elements; e++) + { + prefixName = name; + if(c.type.elements > 1 || (c.type.flags & ShaderVariableFlags::SingleElementArray)) + prefixName += StringFormat::Fmt("[%u]", e); + if(!c.type.members[0].name.empty() && c.type.members[0].name[0] != '[') + prefixName += "."; + for(const ShaderConstant &mem : c.type.members) + GetUniformMapping(prog, location, prefixName, mem); + } + } + } + const Descriptor &GetDescriptor(const rdcstr &access, const ShaderBindIndex &index, bool &valid) { CHECK_DEVICE_THREAD(); @@ -2867,7 +2948,8 @@ ShaderDebugTrace *GLReplay::DebugPixel(uint32_t eventId, uint32_t x, uint32_t y, shadDetails.Disassemble(entryPoint); - GLAPIWrapper *apiWrapper = new GLAPIWrapper(m_pDriver, ShaderStage::Pixel, eventId, pixel); + GLAPIWrapper *apiWrapper = new GLAPIWrapper(m_pDriver, ShaderStage::Pixel, eventId, pixel, + shadDetails.convertedAutomapped ? refl : NULL); SubgroupSupport subgroupSupport = SubgroupSupport::None; uint32_t numThreads = 4; diff --git a/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp index ce48bc481..bb49d4021 100644 --- a/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp +++ b/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp @@ -288,6 +288,9 @@ void WrappedOpenGL::ShaderData::ProcessCompilation(WrappedOpenGL &drv, ResourceI settings.gles = IsGLES; settings.debugInfo = true; + ShaderReflection spvReflection; + SPIRVPatchData spvPatchData; + rdcstr err = rdcspv::Compile(settings, sources, convertedSpirvWords); if(!convertedSpirvWords.empty()) { @@ -295,7 +298,7 @@ void WrappedOpenGL::ShaderData::ProcessCompilation(WrappedOpenGL &drv, ResourceI spirv.Parse(convertedSpirvWords); spirv.MakeReflection(GraphicsAPI::OpenGL, ShaderStage(ShaderIdx(type)), "main", {}, - convertedRefl, convertedPatchData); + spvReflection, spvPatchData); } else { @@ -311,7 +314,7 @@ void WrappedOpenGL::ShaderData::ProcessCompilation(WrappedOpenGL &drv, ResourceI spirv.Parse(convertedSpirvWords); spirv.MakeReflection(GraphicsAPI::OpenGL, ShaderStage(ShaderIdx(type)), "main", {}, - convertedRefl, convertedPatchData); + spvReflection, spvPatchData); } else { @@ -322,12 +325,233 @@ void WrappedOpenGL::ShaderData::ProcessCompilation(WrappedOpenGL &drv, ResourceI if(convertedSPIRV) { // we could assert here that convertedRefl looks like the real reflection - reflection->debugInfo.debuggable = convertedRefl.debugInfo.debuggable; - reflection->debugInfo.debugStatus = convertedRefl.debugInfo.debugStatus; + reflection->debugInfo.debuggable = spvReflection.debugInfo.debuggable; + reflection->debugInfo.debugStatus = spvReflection.debugInfo.debugStatus; reflection->debugInfo.sourceDebugInformation = - convertedRefl.debugInfo.sourceDebugInformation; + spvReflection.debugInfo.sourceDebugInformation; if(reflection->debugInfo.sourceDebugInformation) reflection->debugInfo.compileFlags.flags.push_back({"preferSourceDebug", "1"}); + + // we must ensure the converted reflection & patch data matches the real reflection so + // that ShaderBindIndex references are consistent. We could do a manual remapping during + // debug to the right index, but ShaderBindIndex's are also baked into variable results. + convertedRefl = spvReflection; + convertedPatchData = spvPatchData; + + // for each interface, clear it, then look through the real reflection and find the + // matching SPIR-V reflection entry by name and assign it to the same index + // any holes generated by SPIR-V + + convertedRefl.constantBlocks.clear(); + convertedRefl.readOnlyResources.clear(); + convertedRefl.readWriteResources.clear(); + convertedRefl.samplers.clear(); + + convertedPatchData.cblockInterface.clear(); + convertedPatchData.roInterface.clear(); + convertedPatchData.rwInterface.clear(); + convertedPatchData.samplerInterface.clear(); + + for(size_t i = 0; i < reflection->constantBlocks.size(); i++) + { + rdcstr name = reflection->constantBlocks[i].name; + bool found = false; + for(size_t j = 0; j < spvReflection.constantBlocks.size(); j++) + { + if(name == spvReflection.constantBlocks[j].name) + { + convertedPatchData.cblockInterface.resize_for_index(i); + convertedPatchData.cblockInterface[i] = spvPatchData.cblockInterface[j]; + convertedRefl.constantBlocks.resize_for_index(i); + convertedRefl.constantBlocks[i] = spvReflection.constantBlocks[j]; + found = true; + break; + } + } + + if(!found) + { + // try again to match but via type name. a block like `uniform foo_type { .. } foo` + // will show up via SPIR-V reflection as 'foo' but could show up as 'foo_type' via GL reflection. + for(size_t j = 0; j < spvReflection.constantBlocks.size(); j++) + { + const rdcspv::DataType &outerType = + spirv.GetDataType(spirv.GetIDType(spvPatchData.cblockInterface[j])); + + rdcstr typeName = outerType.name; + if(outerType.type == rdcspv::DataType::PointerType) + typeName = spirv.GetDataType(outerType.InnerType()).name; + + if(name == typeName) + { + convertedPatchData.cblockInterface.resize_for_index(i); + convertedPatchData.cblockInterface[i] = spvPatchData.cblockInterface[j]; + convertedRefl.constantBlocks.resize_for_index(i); + convertedRefl.constantBlocks[i] = spvReflection.constantBlocks[j]; + found = true; + break; + } + } + } + + if(!found) + { + // try again with a _var suffix that is added automatically commonly for pointer-to-type + name += "_var"; + for(size_t j = 0; j < spvReflection.constantBlocks.size(); j++) + { + if(name == spvReflection.constantBlocks[j].name) + { + convertedPatchData.cblockInterface.resize_for_index(i); + convertedPatchData.cblockInterface[i] = spvPatchData.cblockInterface[j]; + convertedRefl.constantBlocks.resize_for_index(i); + convertedRefl.constantBlocks[i] = spvReflection.constantBlocks[j]; + found = true; + break; + } + } + } + + // not finding one is fine, it means the reflection generated some entries which didn't exist in SPIR-V + + if(!found) + { + RDCWARN( + "Found reflection entry %s that does not exist in SPIR-V. Using GLSL reflection", + reflection->constantBlocks[i].name.c_str()); + } + } + + // this is slightly more of a problem since the SPIR-V reflection will potentially refer + // to resources that don't exist at all. We hope that this only happens due to dead + // resources that won't be referenced. + if(spvReflection.constantBlocks.size() > reflection->constantBlocks.size()) + RDCWARN("Found some SPIR-V reflection entries that don't exist in the GLSL reflection"); + + for(size_t i = 0; i < reflection->readOnlyResources.size(); i++) + { + bool found = false; + for(size_t j = 0; j < spvReflection.readOnlyResources.size(); j++) + { + if(reflection->readOnlyResources[i].name == spvReflection.readOnlyResources[j].name) + { + convertedPatchData.roInterface.resize_for_index(i); + convertedPatchData.roInterface[i] = spvPatchData.roInterface[j]; + convertedRefl.readOnlyResources.resize_for_index(i); + convertedRefl.readOnlyResources[i] = spvReflection.readOnlyResources[j]; + found = true; + break; + } + } + + if(!found) + { + RDCWARN( + "Found reflection entry %s that does not exist in SPIR-V. Using GLSL reflection", + reflection->readOnlyResources[i].name.c_str()); + } + } + + if(spvReflection.readOnlyResources.size() > reflection->readOnlyResources.size()) + RDCWARN("Found some SPIR-V reflection entries that don't exist in the GLSL reflection"); + + for(size_t i = 0; i < reflection->readWriteResources.size(); i++) + { + rdcstr name = reflection->readWriteResources[i].name; + bool found = false; + for(size_t j = 0; j < spvReflection.readWriteResources.size(); j++) + { + if(reflection->readWriteResources[i].name == spvReflection.readWriteResources[j].name) + { + convertedPatchData.rwInterface.resize_for_index(i); + convertedPatchData.rwInterface[i] = spvPatchData.rwInterface[j]; + convertedRefl.readWriteResources.resize_for_index(i); + convertedRefl.readWriteResources[i] = spvReflection.readWriteResources[j]; + found = true; + break; + } + } + + if(!found) + { + // try again to match but via type name. a block like `uniform foo_type { .. } foo` + // will show up via SPIR-V reflection as 'foo' but could show up as 'foo_type' via GL reflection. + for(size_t j = 0; j < spvReflection.readWriteResources.size(); j++) + { + const rdcspv::DataType &outerType = + spirv.GetDataType(spirv.GetIDType(spvPatchData.rwInterface[j])); + + rdcstr typeName = outerType.name; + if(outerType.type == rdcspv::DataType::PointerType) + typeName = spirv.GetDataType(outerType.InnerType()).name; + + if(name == typeName) + { + convertedPatchData.rwInterface.resize_for_index(i); + convertedPatchData.rwInterface[i] = spvPatchData.rwInterface[j]; + convertedRefl.readWriteResources.resize_for_index(i); + convertedRefl.readWriteResources[i] = spvReflection.readWriteResources[j]; + found = true; + break; + } + } + } + + if(!found) + { + // try again with a _var suffix that is added automatically commonly for pointer-to-type + name += "_var"; + for(size_t j = 0; j < spvReflection.readWriteResources.size(); j++) + { + if(name == spvReflection.readWriteResources[j].name) + { + convertedPatchData.rwInterface.resize_for_index(i); + convertedPatchData.rwInterface[i] = spvPatchData.rwInterface[j]; + convertedRefl.readWriteResources.resize_for_index(i); + convertedRefl.readWriteResources[i] = spvReflection.readWriteResources[j]; + found = true; + break; + } + } + } + + if(!found) + { + RDCWARN( + "Found reflection entry %s that does not exist in SPIR-V. Using GLSL reflection", + reflection->readWriteResources[i].name.c_str()); + } + } + + if(spvReflection.readWriteResources.size() > reflection->readWriteResources.size()) + RDCWARN("Found some SPIR-V reflection entries that don't exist in the GLSL reflection"); + + for(size_t i = 0; i < reflection->samplers.size(); i++) + { + bool found = false; + for(size_t j = 0; j < spvReflection.samplers.size(); j++) + { + if(reflection->samplers[i].name == spvReflection.samplers[j].name) + { + convertedPatchData.samplerInterface.resize_for_index(i); + convertedPatchData.samplerInterface[i] = spvPatchData.samplerInterface[j]; + convertedRefl.samplers.resize_for_index(i); + convertedRefl.samplers[i] = spvReflection.samplers[j]; + found = true; + break; + } + } + + if(!found) + { + RDCWARN( + "Found reflection entry %s that does not exist in SPIR-V. Using GLSL reflection", + reflection->samplers[i].name.c_str()); + } + } + + if(spvReflection.samplers.size() > reflection->samplers.size()) + RDCWARN("Found some SPIR-V reflection entries that don't exist in the GLSL reflection"); } reflection->resourceId = id; diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index a4f463440..d9c713c83 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -1367,6 +1367,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st // if the outer type is an array, get the length and peel it off. uint32_t arraySize = 1; + bool singleArray = false; if(varType->type == DataType::ArrayType) { // runtime arrays have no length @@ -1374,6 +1375,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st arraySize = EvaluateConstant(varType->length, specInfo).value.u32v[0]; else arraySize = ~0U; + singleArray = (arraySize == 1); varType = &dataTypes[varType->InnerType()]; } @@ -1578,7 +1580,10 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st if(arraySize > 1) constant.type.elements = arraySize; else - constant.type.elements = 0; + constant.type.elements = 1; + + if(singleArray) + constant.type.flags |= ShaderVariableFlags::SingleElementArray; constant.byteOffset = decorations[global.id].location;