From c4f71f6223154bb636e1fb44daddd517258c77c3 Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 18 Nov 2025 13:52:38 +0000 Subject: [PATCH] Support plain bare uniforms in GL SPIR-V --- renderdoc/driver/gl/gl_shaderdebug.cpp | 32 ++++ renderdoc/driver/shaders/spirv/spirv_debug.h | 5 +- .../shaders/spirv/spirv_debug_setup.cpp | 138 +++++++++++++----- .../driver/shaders/spirv/spirv_reflect.cpp | 41 ++++-- .../driver/shaders/spirv/spirv_reflect.h | 4 +- renderdoc/driver/vulkan/vk_shaderdebug.cpp | 5 + 6 files changed, 175 insertions(+), 50 deletions(-) diff --git a/renderdoc/driver/gl/gl_shaderdebug.cpp b/renderdoc/driver/gl/gl_shaderdebug.cpp index c3f1ef62e..4df798e04 100644 --- a/renderdoc/driver/gl/gl_shaderdebug.cpp +++ b/renderdoc/driver/gl/gl_shaderdebug.cpp @@ -174,6 +174,38 @@ public: return length; } + virtual void ReadLocationValue(int32_t location, ShaderVariable &var) override + { + GLint prog = 0; + GL.glGetIntegerv(eGL_CURRENT_PROGRAM, &prog); + + if(var.type == VarType::Float) + { + GL.glGetUniformfv(prog, location, var.value.f32v.data()); + } + else if(var.type == VarType::UInt) + { + GL.glGetUniformuiv(prog, location, var.value.u32v.data()); + } + else if(var.type == VarType::SInt) + { + GL.glGetUniformiv(prog, location, var.value.s32v.data()); + } + else + { + RDCERR("Unexpected type of variable"); + } + + // If the uniform queried is a matrix, the values of the matrix are returned in column major order. + if(var.columns > 1) + { + ShaderVariable tmp = var; + for(uint8_t r = 0; r < var.rows; r++) + for(uint8_t c = 0; c < var.columns; c++) + copyComp(var, r * var.columns + c, tmp, c * var.rows + r); + } + } + virtual void ReadBufferValue(const ShaderBindIndex &bind, uint64_t offset, uint64_t byteSize, void *dst) override { diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.h b/renderdoc/driver/shaders/spirv/spirv_debug.h index d9aec6481..42baf1aaf 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.h +++ b/renderdoc/driver/shaders/spirv/spirv_debug.h @@ -114,6 +114,8 @@ public: virtual uint64_t GetBufferLength(const ShaderBindIndex &bind) = 0; + virtual void ReadLocationValue(int32_t location, ShaderVariable &var) = 0; + virtual void ReadBufferValue(const ShaderBindIndex &bind, uint64_t offset, uint64_t byteSize, void *dst) = 0; virtual void WriteBufferValue(const ShaderBindIndex &bind, uint64_t offset, uint64_t byteSize, @@ -746,7 +748,8 @@ private: template uint32_t WalkVariable(const Decorations &curDecorations, const DataType &type, - uint64_t offsetOrLocation, ShaderVarType &var, const rdcstr &accessSuffix, + uint64_t offsetOrLocation, bool locationUniform, ShaderVarType &var, + const rdcstr &accessSuffix, std::function callback) const; diff --git a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp index 011b7993f..d9d25c87b 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp @@ -1144,6 +1144,10 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s rdcarray pointerIDs; + // tracking for any GL bare uniforms + uint32_t uniformsCBuffer = ~0U; + rdcarray> bareUniformPointers; + // allocate storage for globals with opaque storage classes, and prepare to set up pointers to // them for the global variables themselves for(const Variable &v : globals) @@ -1263,7 +1267,7 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s workgroup[laneIndex].inputs.push_back(var); WalkVariable(decorations[v.id], dataTypes[type.InnerType()], ~0U, - workgroup[laneIndex].inputs.back(), rdcstr(), + false, workgroup[laneIndex].inputs.back(), rdcstr(), fillInputCallback); } @@ -1272,8 +1276,8 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s } else { - WalkVariable(decorations[v.id], dataTypes[type.InnerType()], ~0U, var, - rdcstr(), fillInputCallback); + WalkVariable(decorations[v.id], dataTypes[type.InnerType()], ~0U, + false, var, rdcstr(), fillInputCallback); active.outputs.push_back(var); liveGlobals.push_back(v.id); @@ -1451,14 +1455,14 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s binding.arrayElement = a; var.members.push_back(ShaderVariable()); var.members.back().name = StringFormat::Fmt("[%u]", a); - WalkVariable(decorations[v.id], *innertype, 0U, + WalkVariable(decorations[v.id], *innertype, 0U, false, var.members.back(), rdcstr(), cbufferCallback); } } else { - WalkVariable(decorations[v.id], *innertype, 0U, var, rdcstr(), - cbufferCallback); + WalkVariable(decorations[v.id], *innertype, 0U, false, var, + rdcstr(), cbufferCallback); } sourceVar.type = VarType::ConstantBlock; @@ -1482,7 +1486,8 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s if(!patchData.usedIds.contains(v.id)) continue; - // only images/samplers are allowed to be in UniformConstant + // only images/samplers are allowed to be in UniformConstant in Vulkan SPIR-V. In GL SPIR-V + // these can also be values, but we default to this and override below as needed ShaderVariable var; var.rows = 1; var.columns = 1; @@ -1516,18 +1521,23 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s DebugVariableType debugType = DebugVariableType::ReadOnlyResource; - uint32_t set = 0, bind = 0; + uint32_t set = 0, bind = 0, location = ~0U; if(decorations[v.id].flags & Decorations::HasDescriptorSet) set = decorations[v.id].set; if(decorations[v.id].flags & Decorations::HasBinding) bind = decorations[v.id].binding; + if(decorations[v.id].flags & Decorations::HasLocation) + location = decorations[v.id].location; - if(innertype->type == DataType::ArrayType) + // don't step into arrays when they're bare uniforms with locations + if(innertype->type == DataType::ArrayType && location == ~0U) { enablePointerFlags(var, PointerFlags::GlobalArrayBinding); innertype = &dataTypes[innertype->InnerType()]; } + bool bareUniform = false; + if(innertype->type == DataType::SamplerType) { var.type = VarType::Sampler; @@ -1602,20 +1612,75 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s global.readOnlyResources.push_back(var); pointerIDs.push_back(GLOBAL_POINTER(v.id, readOnlyResources)); } + else if(innertype->type == DataType::StructType || innertype->type == DataType::ArrayType || + innertype->type == DataType::MatrixType || innertype->type == DataType::VectorType || + innertype->type == DataType::ScalarType) + { + // plain variable + bareUniform = true; + + // if we haven't already added a virtual uniforms cbuffer, do so now + if(uniformsCBuffer == ~0U) + { + ShaderVariable uniformsVar; + uniformsVar.rows = 1; + uniformsVar.columns = 1; + uniformsVar.type = VarType::ConstantBlock; + + SourceVariableMapping sourceVar; + sourceVar.name = uniformsVar.name = "uniforms"; + sourceVar.type = VarType::ConstantBlock; + sourceVar.rows = 1; + sourceVar.columns = 1; + sourceVar.offset = 0; + sourceVar.variables.push_back( + DebugVariableReference(DebugVariableType::Constant, uniformsVar.name)); + + uniformsCBuffer = global.constantBlocks.size(); + + global.constantBlocks.push_back(uniformsVar); + pointerIDs.push_back(GLOBAL_POINTER(v.id, constantBlocks)); + + ret->sourceVars.push_back(sourceVar); + } + + rdcarray &uniforms = global.constantBlocks[uniformsCBuffer].members; + + // record that this variable id needs to be pointed to the n'th member of the virtual + // cbuffer, which we're about to add + bareUniformPointers.push_back({v.id, uniforms.size()}); + + var = ShaderVariable(); + var.name = GetHumanName(v.id); + + auto uniformCallback = [this](ShaderVariable &var, const Decorations &curDecorations, + const DataType &type, uint64_t location, const rdcstr &) { + if(var.members.empty()) + this->apiWrapper->ReadLocationValue((uint32_t)location, var); + }; + + WalkVariable(decorations[v.id], *innertype, ~0U, false, var, rdcstr(), + uniformCallback); + + uniforms.push_back(var); + } else { RDCERR("Unhandled type of uniform: %u", innertype->type); } - SourceVariableMapping sourceVar; - sourceVar.name = sourceName; - sourceVar.type = var.type; - sourceVar.rows = 1; - sourceVar.columns = 1; - sourceVar.offset = 0; - sourceVar.variables.push_back(DebugVariableReference(debugType, var.name)); + if(!bareUniform) + { + SourceVariableMapping sourceVar; + sourceVar.name = sourceName; + sourceVar.type = var.type; + sourceVar.rows = 1; + sourceVar.columns = 1; + sourceVar.offset = 0; + sourceVar.variables.push_back(DebugVariableReference(debugType, var.name)); - ret->sourceVars.push_back(sourceVar); + ret->sourceVars.push_back(sourceVar); + } } else if(v.storage == StorageClass::Private || v.storage == StorageClass::Workgroup) { @@ -1638,8 +1703,8 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s memset(&var.value, 0xcc, sizeof(var.value)); }; - WalkVariable(decorations[v.id], dataTypes[type.InnerType()], ~0U, var, - rdcstr(), uninitialisedCallback); + WalkVariable(decorations[v.id], dataTypes[type.InnerType()], ~0U, false, + var, rdcstr(), uninitialisedCallback); if(v.initializer != Id()) AssignValue(var, active.ids[v.initializer]); @@ -1719,6 +1784,12 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s for(const PointerId &p : pointerIDs) p.Set(*this, global, lane, isActiveLane); + for(const rdcpair &u : bareUniformPointers) + { + lane.ids[u.first] = + MakePointerVariable(u.first, &global.constantBlocks[uniformsCBuffer].members[u.second]); + } + if(isActiveLane) { for(const PointerId &p : pointerIDs) @@ -3410,7 +3481,7 @@ DeviceOpResult Debugger::ReadFromPointer(const ShaderVariable &ptr, ShaderVariab } }; - WalkVariable(parentDecorations, dataTypes[typeId], byteOffset, ret, + WalkVariable(parentDecorations, dataTypes[typeId], byteOffset, false, ret, rdcstr(), readCallback); ret.name = ptr.name; @@ -3666,8 +3737,8 @@ DeviceOpResult Debugger::WriteThroughPointer(ShaderVariable &ptr, const ShaderVa } }; - WalkVariable(parentDecorations, dataTypes[typeId], byteOffset, val, - rdcstr(), writeCallback); + WalkVariable(parentDecorations, dataTypes[typeId], byteOffset, + false, val, rdcstr(), writeCallback); return DeviceOpResult::Succeeded; } @@ -3773,13 +3844,13 @@ void Debugger::AllocateVariable(Id id, Id typeId, ShaderVariable &outVar) const }; WalkVariable(Decorations(), dataTypes[dataTypes[typeId].InnerType()], ~0U, - outVar, rdcstr(), initCallback); + false, outVar, rdcstr(), initCallback); } template uint32_t Debugger::WalkVariable( const Decorations &curDecorations, const DataType &type, uint64_t offsetOrLocation, - ShaderVarType &var, const rdcstr &accessSuffix, + bool locationUniform, ShaderVarType &var, const rdcstr &accessSuffix, std::function callback) const { @@ -3792,7 +3863,8 @@ uint32_t Debugger::WalkVariable( // we're auto-assigning from there we shouldn't encounter another location decoration somewhere // further down the struct chain. This also prevents us from using the same location for every // element in an array, since we have the same set of decorations on the array as on the members - if((curDecorations.flags & Decorations::HasLocation) && offsetOrLocation == ~0U) + const bool hasLocation = (curDecorations.flags & Decorations::HasLocation) != 0 || locationUniform; + if(hasLocation && offsetOrLocation == ~0U) offsetOrLocation = curDecorations.location; uint32_t numLocations = 0; @@ -3853,13 +3925,13 @@ uint32_t Debugger::WalkVariable( // if the struct is concrete, it must have an offset. Otherwise it's opaque and we're using // locations - if(childDecorations.flags & Decorations::HasOffset) - childOffsetOrLocation += childDecorations.offset; - else if(offsetOrLocation != ~0U) + if(hasLocation) childOffsetOrLocation += numLocations; + else if(childDecorations.flags & Decorations::HasOffset) + childOffsetOrLocation += childDecorations.offset; uint32_t childLocations = WalkVariable( - childDecorations, dataTypes[type.children[i].type], childOffsetOrLocation, + childDecorations, dataTypes[type.children[i].type], childOffsetOrLocation, hasLocation, var.members[i], childAccess, callback); numLocations += childLocations; @@ -3886,16 +3958,16 @@ uint32_t Debugger::WalkVariable( uint32_t childLocations = WalkVariable( curDecorations, dataTypes[type.InnerType()], offsetOrLocation + childOffset, - var.members[i], childAccess, callback); + hasLocation, var.members[i], childAccess, callback); numLocations += childLocations; // as above - either the type is concrete and has an array stride, or else we're using // locations - if(typeDecorations.flags & Decorations::HasArrayStride) - childOffset += decorations[type.id].arrayStride; - else if(offsetOrLocation != ~0U) + if(hasLocation) childOffset = numLocations; + else if(typeDecorations.flags & Decorations::HasArrayStride) + childOffset += decorations[type.id].arrayStride; } break; } diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index 95425e031..a4f463440 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -1379,6 +1379,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st // new SSBOs are in the storage buffer class, previously they were in uniform with BufferBlock // decoration + const bool block = (decorations[varType->id].flags & Decorations::Block); const bool ssbo = (global.storage == StorageClass::StorageBuffer) || (decorations[varType->id].flags & Decorations::BufferBlock); const bool pushConst = (global.storage == StorageClass::PushConstant); @@ -1546,7 +1547,8 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st } else { - if(varType->type != DataType::StructType) + if(varType->type != DataType::StructType || + (sourceAPI == GraphicsAPI::OpenGL && !block && !ssbo)) { if(taskPayload) { @@ -1563,14 +1565,15 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st else { // global loose variable - add to $Globals block - RDCASSERT(varType->type == DataType::ScalarType || varType->type == DataType::VectorType || - varType->type == DataType::MatrixType || varType->type == DataType::ArrayType); + RDCASSERT(varType->type == DataType::ScalarType || + varType->type == DataType::VectorType || varType->type == DataType::MatrixType || + varType->type == DataType::ArrayType || varType->type == DataType::StructType); RDCASSERT(sourceAPI == GraphicsAPI::OpenGL); ShaderConstant constant; MakeConstantBlockVariable(constant, pointerTypes, effectiveStorage, *varType, - strings[global.id], decorations[global.id], specInfo); + strings[global.id], decorations[global.id], true, specInfo); if(arraySize > 1) constant.type.elements = arraySize; @@ -1590,7 +1593,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st taskPayloadBlock.bufferBacked = false; MakeConstantBlockVariables(effectiveStorage, *varType, 0, 0, taskPayloadBlock.variables, - pointerTypes, specInfo); + pointerTypes, false, specInfo); CalculateScalarLayout(0, taskPayloadBlock.variables); } @@ -1624,12 +1627,16 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st res.variableType.name = varType->name; MakeConstantBlockVariables(effectiveStorage, *varType, 0, 0, res.variableType.members, - pointerTypes, specInfo); + pointerTypes, false, specInfo); rwresources.push_back(sortedres(global.id, res)); } else { + // except on OpenGL, a struct type should be either Storage/BufferBlock or Block + // decorated. The GL case is handled above + RDCASSERTMSG("Should be block-decorated", block); + ConstantBlock cblock; cblock.name = strings[global.id]; @@ -1643,7 +1650,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st cblock.bindArraySize = arraySize; MakeConstantBlockVariables(effectiveStorage, *varType, 0, 0, cblock.variables, - pointerTypes, specInfo); + pointerTypes, false, specInfo); if(!varType->children.empty()) cblock.byteSize = CalculateMinimumByteSize(cblock.variables); @@ -1678,7 +1685,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st ShaderConstant spec; MakeConstantBlockVariable(spec, pointerTypes, rdcspv::StorageClass::PushConstant, - dataTypes[c.type], name, decorations[c.id], specInfo); + dataTypes[c.type], name, decorations[c.id], false, specInfo); spec.byteOffset = uint32_t(specblock.variables.size() * sizeof(uint64_t)); spec.defaultValue = c.value.value.u64v[0]; specblock.variables.push_back(spec); @@ -1899,7 +1906,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st { ShaderConstant dummy; MakeConstantBlockVariable(dummy, pointerTypes, dataTypes[id].pointerType.storage, - dataTypes[id], rdcstr(), Decorations(), specInfo); + dataTypes[id], rdcstr(), Decorations(), false, specInfo); } // continue if we generated some more @@ -1912,7 +1919,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st ShaderConstant dummy; MakeConstantBlockVariable(dummy, pointerTypes, dataTypes[it->first].pointerType.storage, - dataTypes[it->first], rdcstr(), Decorations(), specInfo); + dataTypes[it->first], rdcstr(), Decorations(), false, specInfo); if(it->second >= reflection.pointerTypes.size()) reflection.pointerTypes.resize(it->second + 1); @@ -1927,7 +1934,7 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st void Reflector::MakeConstantBlockVariables(rdcspv::StorageClass storage, const DataType &structType, uint32_t arraySize, uint32_t arrayByteStride, rdcarray &cblock, - SparseIdMap &pointerTypes, + SparseIdMap &pointerTypes, bool bareUniforms, const rdcarray &specInfo) const { // we get here for multi-dimensional arrays @@ -1942,7 +1949,8 @@ void Reflector::MakeConstantBlockVariables(rdcspv::StorageClass storage, const D for(uint32_t i = 0; i < arraySize; i++) { MakeConstantBlockVariable(cblock[i], pointerTypes, storage, structType, - StringFormat::Fmt("[%u]", i), decorations[structType.id], specInfo); + StringFormat::Fmt("[%u]", i), decorations[structType.id], + bareUniforms, specInfo); cblock[i].byteOffset = relativeOffset; @@ -1963,7 +1971,7 @@ void Reflector::MakeConstantBlockVariables(rdcspv::StorageClass storage, const D name = StringFormat::Fmt("_child%zu", i); MakeConstantBlockVariable(cblock[i], pointerTypes, storage, dataTypes[structType.children[i].type], name, - structType.children[i].decorations, specInfo); + structType.children[i].decorations, bareUniforms, specInfo); } uint32_t emptyStructSize = 4; @@ -1998,6 +2006,10 @@ void Reflector::MakeConstantBlockVariables(rdcspv::StorageClass storage, const D return; } + // don't enforce sizes on GL opaque uniforms + if(bareUniforms) + return; + for(size_t i = 0; i < cblock.size(); i++) { // for structs that aren't in arrays, we need to define their byte size (stride). Without @@ -2074,6 +2086,7 @@ void Reflector::MakeConstantBlockVariable(ShaderConstant &outConst, SparseIdMap &pointerTypes, rdcspv::StorageClass storage, const DataType &type, const rdcstr &name, const Decorations &varDecorations, + bool bareUniforms, const rdcarray &specInfo) const { outConst.name = name; @@ -2164,7 +2177,7 @@ void Reflector::MakeConstantBlockVariable(ShaderConstant &outConst, MakeConstantBlockVariables(storage, *curType, outConst.type.elements, outConst.type.arrayByteStride, outConst.type.members, pointerTypes, - specInfo); + bareUniforms, specInfo); if(curType->type == DataType::ArrayType) { diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.h b/renderdoc/driver/shaders/spirv/spirv_reflect.h index 71523c319..83bf39258 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.h +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.h @@ -131,12 +131,12 @@ private: void MakeConstantBlockVariables(rdcspv::StorageClass storage, const DataType &structType, uint32_t arraySize, uint32_t arrayByteStride, rdcarray &cblock, - SparseIdMap &pointerTypes, + SparseIdMap &pointerTypes, bool bareUniforms, const rdcarray &specInfo) const; void MakeConstantBlockVariable(ShaderConstant &outConst, SparseIdMap &pointerTypes, rdcspv::StorageClass storage, const DataType &type, const rdcstr &name, const Decorations &varDecorations, - const rdcarray &specInfo) const; + bool bareUniforms, const rdcarray &specInfo) const; void AddSignatureParameter(const bool isInput, const ShaderStage stage, const Id id, const Id structID, uint32_t ®Index, const SPIRVInterfaceAccess &parentPatch, const rdcstr &varName, diff --git a/renderdoc/driver/vulkan/vk_shaderdebug.cpp b/renderdoc/driver/vulkan/vk_shaderdebug.cpp index b16813930..361952751 100644 --- a/renderdoc/driver/vulkan/vk_shaderdebug.cpp +++ b/renderdoc/driver/vulkan/vk_shaderdebug.cpp @@ -306,6 +306,11 @@ public: return length; } + virtual void ReadLocationValue(int32_t location, ShaderVariable &var) override + { + RDCERR("Invalid by-location read"); + } + virtual void ReadBufferValue(const ShaderBindIndex &bind, uint64_t offset, uint64_t byteSize, void *dst) override {