diff --git a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp index 59643a497..b638b00e2 100644 --- a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp +++ b/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]; diff --git a/renderdoc/driver/gl/gl_common.h b/renderdoc/driver/gl/gl_common.h index ad6f6632e..2ed0e19a2 100644 --- a/renderdoc/driver/gl/gl_common.h +++ b/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); diff --git a/renderdoc/driver/gl/gl_replay.cpp b/renderdoc/driver/gl/gl_replay.cpp index 89fb52e28..acfd52543 100644 --- a/renderdoc/driver/gl/gl_replay.cpp +++ b/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); diff --git a/renderdoc/driver/gl/gl_shader_refl.cpp b/renderdoc/driver/gl/gl_shader_refl.cpp index e41f06664..0b37dfd46 100644 --- a/renderdoc/driver/gl/gl_shader_refl.cpp +++ b/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 > Permutation; diff --git a/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp index 8b9576214..183379f38 100644 --- a/renderdoc/driver/gl/wrappers/gl_shader_funcs.cpp +++ b/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; diff --git a/renderdoc/driver/shaders/spirv/spirv_common.h b/renderdoc/driver/shaders/spirv/spirv_common.h index 0785ba169..04c12ae23 100644 --- a/renderdoc/driver/shaders/spirv/spirv_common.h +++ b/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 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 &sources, diff --git a/renderdoc/driver/shaders/spirv/spirv_disassemble.cpp b/renderdoc/driver/shaders/spirv/spirv_disassemble.cpp index 5488255e1..692e4bff9 100644 --- a/renderdoc/driver/shaders/spirv/spirv_disassemble.cpp +++ b/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; diff --git a/renderdoc/driver/vulkan/vk_info.cpp b/renderdoc/driver/vulkan/vk_info.cpp index 88038ee6a..821baa6ea 100644 --- a/renderdoc/driver/vulkan/vk_info.cpp +++ b/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;