From 407c74de01c8b1fc8aa9f4e056b21aed796cd46c Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 9 Sep 2024 14:42:44 +0100 Subject: [PATCH] Fixed an issue with indirect count and secondary command buffers * Added logic to shift the beginEvent for any command nodes that occur after a draw indirect count action within a command tree. --- renderdoc/driver/vulkan/vk_core.cpp | 46 +++++++++++++++++++ renderdoc/driver/vulkan/vk_core.h | 6 +++ .../driver/vulkan/wrappers/vk_draw_funcs.cpp | 5 +- .../driver/vulkan/wrappers/vk_queue_funcs.cpp | 30 ++++++------ 4 files changed, 67 insertions(+), 20 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index ce56a8c59..11b7ef0ef 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -5220,6 +5220,52 @@ bool WrappedVulkan::ShouldUpdateRenderpassActive(ResourceId cmdId, bool dynamicR return IsCommandBufferPartialPrimary(cmdId); } +void WrappedVulkan::ShiftSuccessiveCommandNodes(uint32_t targetEvent, uint32_t eidShift, + CommandBufferNode *current) +{ + // first determine the primary command buffer node the target event occurs in. This will happen + // once, then current will be set for the following recursive cases. + if(current == NULL) + { + for(CommandBufferNode *primaryNode : m_Partial.commandTree) + { + if(IsEventInCommandBuffer(primaryNode, targetEvent, + m_BakedCmdBufferInfo[primaryNode->cmdId].eventCount)) + { + current = primaryNode; + break; + } + } + } + + if(IsEventInCommandBuffer(current, targetEvent, m_BakedCmdBufferInfo[current->cmdId].eventCount)) + { + // if the target event occurs within the scope of this command buffer, update the + // BakedCommandBufferInfo to account for the extra actions and events added by the indirect + // action + m_BakedCmdBufferInfo[current->cmdId].actionCount += eidShift; + m_BakedCmdBufferInfo[current->cmdId].eventCount += eidShift; + } + else if(current->beginEvent > targetEvent) + { + // if the target event occurs before the scope of this command buffer, shift the command buffer + // node's begin event to account for the events added by the indirect action + current->beginEvent += eidShift; + } + else + { + // otherwise the target event occurs after this command buffer, so do nothing and do not process + // any of this command buffer's children. + return; + } + + // if the target event is in or before this command buffer, we also need to update any child command buffers. + for(CommandBufferNode *childNode : current->childCmdNodes) + { + ShiftSuccessiveCommandNodes(targetEvent, eidShift, childNode); + } +} + bool WrappedVulkan::InRerecordRange(ResourceId cmdid) { // if we have an outside command buffer, assume the range is valid and we're replaying all events diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index 05e2843a0..76ca8d741 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -845,6 +845,12 @@ private: // determines whether we should track the open/close state of a renderpass. bool ShouldUpdateRenderpassActive(ResourceId cmdId, bool dynamicRendering = false); + // shifts the beginEvent of any command buffer nodes executed after targetEvent by eidShift. + // additionally updates the action and event counts for the corresponding BakedCmdBufferInfo. + // this function is used to account for events added by DrawIndirectCount calls + void ShiftSuccessiveCommandNodes(uint32_t targetEvent, uint32_t eidShift, + CommandBufferNode *current = NULL); + // if we're replaying just a single action or a particular command // buffer subsection of command events, we don't go through the // whole original command buffers to set up the partial replay, diff --git a/renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp index bfcb79352..7ba336dab 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp @@ -129,9 +129,8 @@ VkIndirectPatchData WrappedVulkan::FetchIndirectData(VkIndirectPatchType type, indirectPatch.stride = stride; indirectPatch.buf = paramsbuf; - // secondary command buffers need to know that their event count should be shifted - if(m_BakedCmdBufferInfo[m_LastCmdBufferID].level == VK_COMMAND_BUFFER_LEVEL_SECONDARY) - indirectPatch.commandBuffer = m_LastCmdBufferID; + // record the current command buffer ID so we can shift its event count and that of any successive command buffers. + indirectPatch.commandBuffer = m_LastCmdBufferID; return indirectPatch; } diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index 4e0c06451..021ebd33e 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -331,15 +331,17 @@ void WrappedVulkan::ReplayQueueSubmit(VkQueue queue, VkSubmitInfo2 submitInfo, r m_RootEventID++; } + // here we build a tree of command nodes representing the current command buffer and any + // descendant secondaries. this tree is later used during active replay to handle cases where + // the selected event occurs within a command buffer. the node returned here represents the + // current primary command buffer. + CommandBufferNode *rebaseNode = BuildSubmitTree(cmd, m_RootEventID); + m_Partial.commandTree.push_back(rebaseNode); + // insert the baked command buffer in-line into this list of notes, assigning new event // and drawIDs InsertActionsAndRefreshIDs(cmdBufInfo); - // only primary command buffers can be submitted - CommandBufferNode *rebaseNode = BuildSubmitTree(cmd, m_RootEventID); - - m_Partial.commandTree.push_back(rebaseNode); - for(size_t i = 0; i < cmdBufInfo.debugMessages.size(); i++) { m_DebugMessages.push_back(cmdBufInfo.debugMessages[i]); @@ -716,6 +718,12 @@ void WrappedVulkan::InsertActionsAndRefreshIDs(BakedCmdBufferInfo &cmdBufInfo) // happened) or clone the subdraw to create more that we can then patch. if(eidShift != 0) { + // the command buffer submission trees must be updated such that any command buffer nodes + // that occur after the draw indirect count action account for the new events. this + // function also updates the BakedCommandBufferInfo for the primary command buffer and any + // descendants that the indirect action occured in + ShiftSuccessiveCommandNodes(n.action.eventId + 2, eidShift); + // i is the pushmarker, so i + 1 is the sub draws, and i + 2 is the pop marker. // adjust all EIDs and action IDs after that point for(size_t j = i + 2; j < cmdBufNodes.size(); j++) @@ -736,18 +744,6 @@ void WrappedVulkan::InsertActionsAndRefreshIDs(BakedCmdBufferInfo &cmdBufInfo) cmdBufInfo.debugMessages[j].eventId += eidShift; } - cmdBufInfo.eventCount += eidShift; - cmdBufInfo.actionCount += eidShift; - - // we also need to patch the original secondary command buffer here, if the indirect call - // was on a secondary, so that vkCmdExecuteCommands knows accurately how many events are - // in the command buffer. - if(n.indirectPatch.commandBuffer != ResourceId()) - { - m_BakedCmdBufferInfo[n.indirectPatch.commandBuffer].eventCount += eidShift; - m_BakedCmdBufferInfo[n.indirectPatch.commandBuffer].actionCount += eidShift; - } - RDCASSERT(cmdBufNodes[i + 1].action.events.size() == 1); uint32_t chunkIndex = cmdBufNodes[i + 1].action.events[0].chunkIndex;