From 738a5c1ea566e50071dbd09381b66868871e6539 Mon Sep 17 00:00:00 2001 From: baldurk Date: Sat, 24 Jan 2026 12:48:48 +0000 Subject: [PATCH] Track last descriptor buffer and normal descriptor set binds separately * We need to track these separately because descriptor buffer set invalidation can lead to empty sets with no valid pipeline layout as the last bound set (since we can't _clear_ the descriptor sets when descriptor buffers are invalidated, as there may be a descriptor set binding there). Track these separately so we can pick the right set to bind from if we don't have a known pipeline layout, and also handle the case where there are descriptor set states but the last bound is not valid. --- renderdoc/driver/vulkan/vk_overlay.cpp | 7 +- renderdoc/driver/vulkan/vk_state.cpp | 24 ++++-- renderdoc/driver/vulkan/vk_state.h | 16 +++- .../driver/vulkan/wrappers/vk_cmd_funcs.cpp | 77 +++++++++++++++---- 4 files changed, 102 insertions(+), 22 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_overlay.cpp b/renderdoc/driver/vulkan/vk_overlay.cpp index a54b3f102..499170fb7 100644 --- a/renderdoc/driver/vulkan/vk_overlay.cpp +++ b/renderdoc/driver/vulkan/vk_overlay.cpp @@ -103,7 +103,7 @@ struct VulkanQuadOverdrawCallback : public VulkanActionCallback const ResourceId layoutID = (pipestate.graphics.shaderObject) - ? pipestate.graphics.descSets[pipestate.graphics.lastBoundSet].pipeLayout + ? pipestate.graphics.descSets[pipestate.graphics.LastBoundSet()].pipeLayout : p.vertLayout; const VulkanCreationInfo::PipelineLayout &layout = @@ -292,7 +292,10 @@ struct VulkanQuadOverdrawCallback : public VulkanActionCallback if(pipestate.graphics.shaderObject) { pipestate.shaderObjects[4] = GetResID(shad.shad); - pipestate.graphics.lastBoundSet = shad.descSet; + if(pipestate.graphics.UsingDescBufs()) + pipestate.graphics.lastBoundDescBufSet = shad.descSet; + else + pipestate.graphics.lastBoundDescSet = shad.descSet; pipestate.graphics.pipeline = ResourceId(); RDCASSERT(pipestate.graphics.descSets.size() >= shad.descSet); diff --git a/renderdoc/driver/vulkan/vk_state.cpp b/renderdoc/driver/vulkan/vk_state.cpp index 97fcd1ae2..231bb9159 100644 --- a/renderdoc/driver/vulkan/vk_state.cpp +++ b/renderdoc/driver/vulkan/vk_state.cpp @@ -1002,8 +1002,15 @@ void VulkanRenderState::BindDescriptorSetsWithoutPipeline(WrappedVulkan *vk, VkC // compatible with it. Anything not compatible by definition has been invalidated so we don't need // to rebind it to be valid. + uint32_t lastSet = pipe.LastBoundSet(); + ResourceId pipeLayoutId = + lastSet < pipe.descSets.size() ? pipe.descSets[lastSet].pipeLayout : ResourceId(); + + if(pipeLayoutId == ResourceId()) + return; + const VulkanCreationInfo::PipelineLayout &refPipeLayout = - vk->GetDebugManager()->GetPipelineLayoutInfo(pipe.descSets[pipe.lastBoundSet].pipeLayout); + vk->GetDebugManager()->GetPipelineLayoutInfo(pipeLayoutId); for(size_t i = 0; i < pipe.descSets.size(); i++) { @@ -1013,7 +1020,7 @@ void VulkanRenderState::BindDescriptorSetsWithoutPipeline(WrappedVulkan *vk, VkC const VulkanCreationInfo::PipelineLayout &iPipeLayout = vk->GetDebugManager()->GetPipelineLayoutInfo(pipe.descSets[i].pipeLayout); - if(i != pipe.lastBoundSet) + if(i != pipe.LastBoundSet()) { // if we come to a descriptor set that isn't compatible with the pipeline layout used in the // last bound set, don't bind this descriptor set @@ -1025,10 +1032,10 @@ void VulkanRenderState::BindDescriptorSetsWithoutPipeline(WrappedVulkan *vk, VkC // quick check, if the pipeline layout is the same as the one used to bind the reference set // then its certainly compatible - if(pipe.descSets[i].pipeLayout != pipe.descSets[pipe.lastBoundSet].pipeLayout) + if(pipe.descSets[i].pipeLayout != pipe.descSets[pipe.LastBoundSet()].pipeLayout) { // are we below or above the last bound set - if(i < pipe.lastBoundSet) + if(i < pipe.LastBoundSet()) { // we only check if this set is compatible with the pipeline layout on this set. // Technically the set might have been perturbed still, or we might invalidate this @@ -1129,8 +1136,15 @@ void VulkanRenderState::BindDescriptorSetsForShaders(WrappedVulkan *vk, VkComman if(pipe.descSets.empty()) return; + uint32_t lastSet = pipe.LastBoundSet(); + ResourceId pipeLayoutId = + lastSet < pipe.descSets.size() ? pipe.descSets[lastSet].pipeLayout : ResourceId(); + + if(pipeLayoutId == ResourceId()) + return; + const rdcarray &descSetLayouts = - vk->GetDebugManager()->GetPipelineLayoutInfo(pipe.descSets[pipe.lastBoundSet].pipeLayout).descSetLayouts; + vk->GetDebugManager()->GetPipelineLayoutInfo(pipeLayoutId).descSetLayouts; for(size_t i = 0; i < descSetLayouts.size(); i++) { diff --git a/renderdoc/driver/vulkan/vk_state.h b/renderdoc/driver/vulkan/vk_state.h index b664b1119..c770ab4a2 100644 --- a/renderdoc/driver/vulkan/vk_state.h +++ b/renderdoc/driver/vulkan/vk_state.h @@ -71,7 +71,21 @@ struct VulkanStatePipeline // the index of the last set bound. In the case where we are re-binding sets and don't have a // valid pipeline to reference, this can help us resolve which descriptor sets to rebind in the // event that they're not all compatible - uint32_t lastBoundSet = 0; + // we need to track the last descriptor set separately from the last descriptor buffer set, + // because if a descriptor buffer set is invalidated the previous last descriptor set may still be + // valid. There is no way to do the other direction (any descriptor set bind invalidates all + // descriptor buffer set bindings) + int32_t lastBoundDescSet = -1; + int32_t lastBoundDescBufSet = -1; + + uint32_t LastBoundSet() const + { + if(UsingDescBufs() && lastBoundDescBufSet >= 0) + return lastBoundDescBufSet; + if(lastBoundDescSet >= 0) + return lastBoundDescSet; + return 0; + } bool UsingDescBufs() const { diff --git a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp index d000205d4..2db7fb5cf 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp @@ -3849,7 +3849,8 @@ bool WrappedVulkan::Serialise_vkCmdBindDescriptorSets( if(descsets.size() < firstSet + setCount) descsets.resize(firstSet + setCount); - pipeline.lastBoundSet = firstSet; + pipeline.lastBoundDescSet = firstSet; + pipeline.lastBoundDescBufSet = -1; const rdcarray &descSetLayouts = m_CreationInfo.m_PipelineLayout[GetResID(layout)].descSetLayouts; @@ -3998,7 +3999,8 @@ bool WrappedVulkan::Serialise_vkCmdBindDescriptorSets2( // expand as necessary descsets.resize_for_index(firstSet + BindDescriptorSetsInfo.descriptorSetCount - 1); - pipeline.lastBoundSet = firstSet; + pipeline.lastBoundDescSet = firstSet; + pipeline.lastBoundDescBufSet = -1; const rdcarray &descSetLayouts = m_CreationInfo.m_PipelineLayout[GetResID(BindDescriptorSetsInfo.layout)].descSetLayouts; @@ -6160,8 +6162,6 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSet(SerialiserType &ser, if(descsets.size() < set + 1) descsets.resize(set + 1); - pipeline.lastBoundSet = set; - descsets[set] = VulkanStatePipeline::DescriptorAndOffsets(); descsets[set].pipeLayout = GetResID(layout); descsets[set].descSet = setId; @@ -6171,6 +6171,17 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSet(SerialiserType &ser, descsets[set].descBufferPush = (m_CreationInfo.m_DescSetLayout[setLayout].flags & VK_DESCRIPTOR_SET_LAYOUT_CREATE_DESCRIPTOR_BUFFER_BIT_EXT) != 0; + + if(descsets[set].descBufferPush) + { + pipeline.lastBoundDescSet = -1; + pipeline.lastBoundDescBufSet = set; + } + else + { + pipeline.lastBoundDescSet = set; + pipeline.lastBoundDescBufSet = -1; + } } // actual replay of the command will happen below @@ -6445,8 +6456,6 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSetWithTemplate( if(descsets.size() < set + 1) descsets.resize(set + 1); - pipeline.lastBoundSet = set; - descsets[set] = VulkanStatePipeline::DescriptorAndOffsets(); descsets[set].pipeLayout = GetResID(layout); descsets[set].descSet = setId; @@ -6456,6 +6465,17 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSetWithTemplate( descsets[set].descBufferPush = (m_CreationInfo.m_DescSetLayout[setLayout].flags & VK_DESCRIPTOR_SET_LAYOUT_CREATE_DESCRIPTOR_BUFFER_BIT_EXT) != 0; + + if(descsets[set].descBufferPush) + { + pipeline.lastBoundDescSet = -1; + pipeline.lastBoundDescBufSet = set; + } + else + { + pipeline.lastBoundDescSet = set; + pipeline.lastBoundDescBufSet = -1; + } } // actual replay of the command will happen below @@ -9387,6 +9407,7 @@ bool WrappedVulkan::Serialise_vkCmdBindDescriptorBuffersEXT( VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR}) { VulkanStatePipeline &pipe = renderstate.GetPipeline(bindPoint); + pipe.lastBoundDescBufSet = -1; for(uint32_t i = 0; i < pipe.descSets.size(); i++) if(pipe.descSets[i].descSet == ResourceId()) @@ -9499,7 +9520,7 @@ bool WrappedVulkan::Serialise_vkCmdSetDescriptorBufferOffsetsEXT( VulkanRenderState &renderstate = GetCmdRenderState(); VulkanStatePipeline &pipeline = renderstate.GetPipeline(pipelineBindPoint); - pipeline.lastBoundSet = firstSet; + pipeline.lastBoundDescBufSet = firstSet; // descriptor set bindings are overwritten/cleared by descriptor buffer bindings for(uint32_t set = 0; set < setCount; set++) @@ -9515,6 +9536,7 @@ bool WrappedVulkan::Serialise_vkCmdSetDescriptorBufferOffsetsEXT( } // any normal descriptor set bindings are invalidated + pipeline.lastBoundDescSet = -1; for(VkPipelineBindPoint bindPoint : {VK_PIPELINE_BIND_POINT_GRAPHICS, VK_PIPELINE_BIND_POINT_COMPUTE, VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR}) @@ -9535,7 +9557,7 @@ bool WrappedVulkan::Serialise_vkCmdSetDescriptorBufferOffsetsEXT( VulkanRenderState &renderstate = m_BakedCmdBufferInfo[m_LastCmdBufferID].state; VulkanStatePipeline &pipeline = renderstate.GetPipeline(pipelineBindPoint); - pipeline.lastBoundSet = firstSet; + pipeline.lastBoundDescBufSet = firstSet; // descriptor set bindings are overwritten/cleared by descriptor buffer bindings for(uint32_t set = 0; set < setCount; set++) @@ -9551,6 +9573,7 @@ bool WrappedVulkan::Serialise_vkCmdSetDescriptorBufferOffsetsEXT( } // any normal descriptor set bindings are invalidated + pipeline.lastBoundDescSet = -1; for(VkPipelineBindPoint bindPoint : {VK_PIPELINE_BIND_POINT_GRAPHICS, VK_PIPELINE_BIND_POINT_COMPUTE, VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR}) @@ -9729,7 +9752,7 @@ bool WrappedVulkan::Serialise_vkCmdSetDescriptorBufferOffsets2EXT( { VulkanStatePipeline &pipeline = renderstate.GetPipeline(pipelineBindPoint); - pipeline.lastBoundSet = firstSet; + pipeline.lastBoundDescBufSet = firstSet; // descriptor set bindings are overwritten/cleared by descriptor buffer bindings for(uint32_t set = 0; set < SetDescriptorBufferOffsetsInfo.setCount; set++) @@ -9754,6 +9777,7 @@ bool WrappedVulkan::Serialise_vkCmdSetDescriptorBufferOffsets2EXT( VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR}) { VulkanStatePipeline &pipe = renderstate.GetPipeline(bindPoint); + pipe.lastBoundDescSet = -1; for(uint32_t i = 0; i < pipe.descSets.size(); i++) if(!pipe.descSets[i].IsDescBufferBound()) @@ -9770,7 +9794,7 @@ bool WrappedVulkan::Serialise_vkCmdSetDescriptorBufferOffsets2EXT( { VulkanStatePipeline &pipeline = renderstate.GetPipeline(pipelineBindPoint); - pipeline.lastBoundSet = firstSet; + pipeline.lastBoundDescBufSet = firstSet; // descriptor set bindings are overwritten/cleared by descriptor buffer bindings for(uint32_t set = 0; set < SetDescriptorBufferOffsetsInfo.setCount; set++) @@ -9795,6 +9819,7 @@ bool WrappedVulkan::Serialise_vkCmdSetDescriptorBufferOffsets2EXT( VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR}) { VulkanStatePipeline &pipe = renderstate.GetPipeline(bindPoint); + pipe.lastBoundDescSet = -1; for(uint32_t i = 0; i < pipe.descSets.size(); i++) if(!pipe.descSets[i].IsDescBufferBound()) @@ -9982,8 +10007,6 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSet2( if(descsets.size() < set + 1) descsets.resize(set + 1); - pipeline.lastBoundSet = set; - descsets[set] = VulkanStatePipeline::DescriptorAndOffsets(); descsets[set].pipeLayout = GetResID(PushDescriptorSetInfo.layout); descsets[set].descSet = setId; @@ -9993,6 +10016,17 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSet2( descsets[set].descBufferPush = (m_CreationInfo.m_DescSetLayout[setLayout].flags & VK_DESCRIPTOR_SET_LAYOUT_CREATE_DESCRIPTOR_BUFFER_BIT_EXT) != 0; + + if(descsets[set].descBufferPush) + { + pipeline.lastBoundDescSet = -1; + pipeline.lastBoundDescBufSet = set; + } + else + { + pipeline.lastBoundDescSet = set; + pipeline.lastBoundDescBufSet = -1; + } } // actual replay of the command will happen below @@ -10151,12 +10185,27 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSetWithTemplate2( if(descsets.size() < set + 1) descsets.resize(set + 1); - pipeline.lastBoundSet = set; - descsets[set] = VulkanStatePipeline::DescriptorAndOffsets(); descsets[set].pipeLayout = GetResID(PushDescriptorSetWithTemplateInfo.layout); descsets[set].descSet = setId; descsets[set].push = true; + ResourceId setLayout = + m_CreationInfo.m_PipelineLayout[GetResID(PushDescriptorSetWithTemplateInfo.layout)] + .descSetLayouts[set]; + descsets[set].descBufferPush = + (m_CreationInfo.m_DescSetLayout[setLayout].flags & + VK_DESCRIPTOR_SET_LAYOUT_CREATE_DESCRIPTOR_BUFFER_BIT_EXT) != 0; + + if(descsets[set].descBufferPush) + { + pipeline.lastBoundDescSet = -1; + pipeline.lastBoundDescBufSet = set; + } + else + { + pipeline.lastBoundDescSet = set; + pipeline.lastBoundDescBufSet = -1; + } } // actual replay of the command will happen below