diff --git a/renderdoc/data/glsl_shaders.cpp b/renderdoc/data/glsl_shaders.cpp index 0829d67ef..691d9aa6e 100644 --- a/renderdoc/data/glsl_shaders.cpp +++ b/renderdoc/data/glsl_shaders.cpp @@ -306,6 +306,7 @@ void main() { INFO("UBO: " << cblock.name.c_str()); CHECK(!cblock.bufferBacked); + CHECK(!cblock.compileConstants); REQUIRE_ARRAY_SIZE(cblock.variables.size(), 2); { @@ -473,10 +474,10 @@ void main() { const ConstantBlock &cblock = refl.constantBlocks[0]; INFO("UBO: " << cblock.name.c_str()); - CHECK(cblock.fixedBindSetOrSpace == SpecializationConstantBindSet); CHECK(cblock.fixedBindNumber == 0); CHECK(cblock.bindArraySize == 1); CHECK(!cblock.bufferBacked); + CHECK(cblock.compileConstants); CHECK(cblock.byteSize == 0); REQUIRE_ARRAY_SIZE(cblock.variables.size(), 2); @@ -548,10 +549,9 @@ void main() { const ConstantBlock &cblock = refl.constantBlocks[0]; INFO("UBO: " << cblock.name.c_str()); - CHECK(cblock.fixedBindSetOrSpace == PushConstantBindSet); - CHECK(cblock.fixedBindNumber == 0); CHECK(cblock.bindArraySize == 1); CHECK(!cblock.bufferBacked); + CHECK(!cblock.compileConstants); CHECK(cblock.byteSize == 16); REQUIRE_ARRAY_SIZE(cblock.variables.size(), 3); diff --git a/renderdoc/driver/gl/gl_replay.cpp b/renderdoc/driver/gl/gl_replay.cpp index 0db776fdd..96192b161 100644 --- a/renderdoc/driver/gl/gl_replay.cpp +++ b/renderdoc/driver/gl/gl_replay.cpp @@ -2840,7 +2840,7 @@ void GLReplay::FillCBufferVariables(ResourceId pipeline, ResourceId shader, Shad } else { - if(cblock.fixedBindSetOrSpace == SpecializationConstantBindSet) + if(cblock.compileConstants) { rdcarray specconsts; diff --git a/renderdoc/driver/shaders/spirv/spirv_common.h b/renderdoc/driver/shaders/spirv/spirv_common.h index 62b5786fa..9012250b6 100644 --- a/renderdoc/driver/shaders/spirv/spirv_common.h +++ b/renderdoc/driver/shaders/spirv/spirv_common.h @@ -439,9 +439,6 @@ struct OpSwitch64 }; // namespace rdcspv -static const uint32_t SpecializationConstantBindSet = 1234567; -static const uint32_t PushConstantBindSet = 1234568; - struct SpecConstant { SpecConstant() = default; diff --git a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp index 00965821d..c6dfed24b 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp @@ -1083,19 +1083,6 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s decorations[v.id].binding); } - uint32_t bindset = 0, bind = 0; - if(v.storage == StorageClass::PushConstant) - { - bindset = PushConstantBindSet; - } - else - { - if(decorations[v.id].flags & Decorations::HasDescriptorSet) - bindset = decorations[v.id].set; - if(decorations[v.id].flags & Decorations::HasBinding) - bind = decorations[v.id].binding; - } - SourceVariableMapping sourceVar; sourceVar.name = sourceName; sourceVar.offset = 0; diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index 382832d43..bb04a2176 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -1353,11 +1353,8 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st if(ssbo) effectiveStorage = StorageClass::StorageBuffer; - uint32_t bindset; - // set something crazy so this doesn't overlap with a real buffer binding - if(pushConst) - bindset = PushConstantBindSet; - else + uint32_t bindset = 0; + if(!pushConst) bindset = GetDescSet(decorations[global.id].set); uint32_t bind = GetBinding(decorations[global.id].binding); @@ -1646,7 +1643,6 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st specblock.inlineDataBytes = true; specblock.compileConstants = true; specblock.byteSize = 0; - specblock.fixedBindSetOrSpace = SpecializationConstantBindSet; // set the binding number to some huge value to try to sort it to the end specblock.fixedBindNumber = 0x8000000; specblock.bindArraySize = 1; diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index 8eac8a338..f35febe44 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -168,6 +168,8 @@ WrappedVulkan::WrappedVulkan() m_FrameCaptureRecord = NULL; ResourceIDGen::SetReplayResourceIDs(); + + m_CreationInfo.pushConstantDescriptorStorage = ResourceIDGen::GetNewUniqueID(); } } diff --git a/renderdoc/driver/vulkan/vk_info.cpp b/renderdoc/driver/vulkan/vk_info.cpp index 2b818687c..78b1fc984 100644 --- a/renderdoc/driver/vulkan/vk_info.cpp +++ b/renderdoc/driver/vulkan/vk_info.cpp @@ -820,6 +820,7 @@ bool CreateDescriptorWritesForSlotData(WrappedVulkan *vk, rdcarray &staticDescriptorAccess, rdcarray setLayoutInfos) const { @@ -844,30 +845,39 @@ void VulkanCreationInfo::Pipeline::Shader::ProcessStaticDescriptorAccess( if(bind.bindArraySize > 1) continue; - // VkShaderStageFlagBits and ShaderStageMask are identical bit-for-bit. - // this might be deliberate if the binding is never actually used dynamically, only - // statically used bindings must be declared - if((setLayoutInfos[bind.fixedBindSetOrSpace]->bindings[bind.fixedBindNumber].stageFlags & - (VkShaderStageFlags)MaskForStage(refl->stage)) == 0) - continue; - access.type = DescriptorType::ConstantBuffer; access.index = i; - if(bind.fixedBindSetOrSpace == PushConstantBindSet) + if(!bind.bufferBacked) { - access.byteSize = bind.fixedBindSetOrSpace; - access.byteOffset = 0; - staticDescriptorAccess.push_back(access); - } - else if(bind.fixedBindSetOrSpace == SpecializationConstantBindSet) - { - access.byteSize = bind.fixedBindSetOrSpace; - access.byteOffset = 0; - staticDescriptorAccess.push_back(access); + if(bind.compileConstants) + { + // spec constants + access.descriptorStore = specStorage; + access.byteSize = 1; + access.byteOffset = 0; + staticDescriptorAccess.push_back(access); + } + else + { + // push constants + access.descriptorStore = pushStorage; + access.byteSize = 1; + access.byteOffset = 0; + staticDescriptorAccess.push_back(access); + } } else { + access.descriptorStore = ResourceId(); + + // VkShaderStageFlagBits and ShaderStageMask are identical bit-for-bit. + // this might be deliberate if the binding is never actually used dynamically, only + // statically used bindings must be declared + if((setLayoutInfos[bind.fixedBindSetOrSpace]->bindings[bind.fixedBindNumber].stageFlags & + (VkShaderStageFlags)MaskForStage(refl->stage)) == 0) + continue; + access.byteSize = bind.fixedBindSetOrSpace; access.byteOffset = setLayoutInfos[bind.fixedBindSetOrSpace]->bindings[bind.fixedBindNumber].elemOffset + @@ -876,6 +886,8 @@ void VulkanCreationInfo::Pipeline::Shader::ProcessStaticDescriptorAccess( } } + access.descriptorStore = ResourceId(); + RDCASSERT(refl->samplers.size() < 0xffff, refl->samplers.size()); for(uint16_t i = 0; i < refl->samplers.size(); i++) { @@ -1624,7 +1636,9 @@ void VulkanCreationInfo::Pipeline::Init(VulkanResourceManager *resourceMan, setLayoutInfos.push_back(&info.m_DescSetLayout[setLayout]); for(const Shader &shad : shaders) - shad.ProcessStaticDescriptorAccess(staticDescriptorAccess, setLayoutInfos); + shad.ProcessStaticDescriptorAccess(info.pushConstantDescriptorStorage, + resourceMan->GetOriginalID(id), staticDescriptorAccess, + setLayoutInfos); } void VulkanCreationInfo::Pipeline::Init(VulkanResourceManager *resourceMan, VulkanCreationInfo &info, @@ -1732,7 +1746,9 @@ void VulkanCreationInfo::Pipeline::Init(VulkanResourceManager *resourceMan, Vulk setLayoutInfos.push_back(&info.m_DescSetLayout[setLayout]); for(const Shader &shad : shaders) - shad.ProcessStaticDescriptorAccess(staticDescriptorAccess, setLayoutInfos); + shad.ProcessStaticDescriptorAccess(info.pushConstantDescriptorStorage, + resourceMan->GetOriginalID(id), staticDescriptorAccess, + setLayoutInfos); } void VulkanCreationInfo::PipelineLayout::Init(VulkanResourceManager *resourceMan, diff --git a/renderdoc/driver/vulkan/vk_info.h b/renderdoc/driver/vulkan/vk_info.h index 4e1a0c57c..4ee5fd556 100644 --- a/renderdoc/driver/vulkan/vk_info.h +++ b/renderdoc/driver/vulkan/vk_info.h @@ -281,7 +281,8 @@ struct VulkanCreationInfo // VkPipelineShaderStageRequiredSubgroupSizeCreateInfo uint32_t requiredSubgroupSize = 0; - void ProcessStaticDescriptorAccess(rdcarray &staticDescriptorAccess, + void ProcessStaticDescriptorAccess(ResourceId pushStorage, ResourceId specStorage, + rdcarray &staticDescriptorAccess, rdcarray setLayoutInfos) const; }; Shader shaders[NumShaderStages]; @@ -745,6 +746,9 @@ struct VulkanCreationInfo // just contains the queueFamilyIndex (after remapping) std::unordered_map m_Queue; + // the fake ID of the 'command buffer' descriptor store for push constants + ResourceId pushConstantDescriptorStorage; + void erase(ResourceId id) { m_QueryPool.erase(id); diff --git a/renderdoc/driver/vulkan/vk_replay.cpp b/renderdoc/driver/vulkan/vk_replay.cpp index e2a2293c0..1410ad117 100644 --- a/renderdoc/driver/vulkan/vk_replay.cpp +++ b/renderdoc/driver/vulkan/vk_replay.cpp @@ -982,8 +982,7 @@ void VulkanReplay::GetBufferData(ResourceId buff, uint64_t offset, uint64_t len, { for(size_t cb = 0; cb < p.shaders[i].refl->constantBlocks.size(); cb++) { - if(p.shaders[i].refl->constantBlocks[cb].fixedBindSetOrSpace == - SpecializationConstantBindSet) + if(p.shaders[i].refl->constantBlocks[cb].compileConstants) { for(const ShaderConstant &sc : p.shaders[i].refl->constantBlocks[cb].variables) { @@ -1220,8 +1219,7 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) { for(size_t cb = 0; cb < p.shaders[i].refl->constantBlocks.size(); cb++) { - if(p.shaders[i].refl->constantBlocks[cb].fixedBindSetOrSpace == - SpecializationConstantBindSet) + if(p.shaders[i].refl->constantBlocks[cb].compileConstants) { for(const ShaderConstant &sc : p.shaders[i].refl->constantBlocks[cb].variables) { @@ -1352,8 +1350,7 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) { for(size_t cb = 0; cb < p.shaders[i].refl->constantBlocks.size(); cb++) { - if(p.shaders[i].refl->constantBlocks[cb].fixedBindSetOrSpace == - SpecializationConstantBindSet) + if(p.shaders[i].refl->constantBlocks[cb].compileConstants) { for(const ShaderConstant &sc : p.shaders[i].refl->constantBlocks[cb].variables) { @@ -2540,19 +2537,11 @@ rdcarray VulkanReplay::GetDescriptorAccess(uint32_t eventId) { uint32_t bindset = (uint32_t)access.byteSize; access.byteSize = 1; - if(bindset == PushConstantBindSet) + if(access.descriptorStore == m_pDriver->m_CreationInfo.pushConstantDescriptorStorage) { - // push constants are stored as a virtual descriptor in the command buffer which references - // itself for storage access.descriptorStore = m_pDriver->GetPushConstantCommandBuffer(); } - else if(bindset == SpecializationConstantBindSet) - { - // push constants are stored as a virtual descriptor in the pipeline, the same way - access.descriptorStore = rm->GetOriginalID( - access.stage == ShaderStage::Compute ? state.compute.pipeline : state.graphics.pipeline); - } - else + else if(access.descriptorStore == ResourceId()) { const rdcarray &descSets = access.stage == ShaderStage::Compute ? state.compute.descSets : state.graphics.descSets; @@ -2760,7 +2749,7 @@ void VulkanReplay::FillCBufferVariables(ResourceId pipeline, ResourceId shader, else { // specialised path to display specialization constants - if(c.fixedBindSetOrSpace == SpecializationConstantBindSet) + if(c.compileConstants) { auto pipeIt = m_pDriver->m_CreationInfo.m_Pipeline.find(pipeline);