From c144757d11f59cfcf4d1754225040d44e011d2f8 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 23 Nov 2022 15:19:51 +0000 Subject: [PATCH] Re-use initial replay to fetch shader feedback without polluting state * Previously with shader feedback being fetched separately we may have had to replay multiple times to ensure clean results. If we do it as part of the initial replay on selecting the event this issue goes away and we remove the extra redundant replay. --- renderdoc/driver/d3d12/d3d12_replay.cpp | 6 +++ renderdoc/driver/d3d12/d3d12_replay.h | 2 +- .../driver/d3d12/d3d12_shader_feedback.cpp | 39 +++++++++---------- renderdoc/driver/vulkan/vk_overlay.cpp | 8 ---- renderdoc/driver/vulkan/vk_replay.cpp | 6 +++ renderdoc/driver/vulkan/vk_replay.h | 2 +- .../driver/vulkan/vk_shader_feedback.cpp | 23 ++++++----- 7 files changed, 44 insertions(+), 42 deletions(-) diff --git a/renderdoc/driver/d3d12/d3d12_replay.cpp b/renderdoc/driver/d3d12/d3d12_replay.cpp index 3b3123e63..a9badb9e3 100644 --- a/renderdoc/driver/d3d12/d3d12_replay.cpp +++ b/renderdoc/driver/d3d12/d3d12_replay.cpp @@ -281,6 +281,12 @@ APIProperties D3D12Replay::GetAPIProperties() void D3D12Replay::ReplayLog(uint32_t endEventID, ReplayLogType replayType) { + if(replayType == eReplay_OnlyDraw) + { + bool replayed = FetchShaderFeedback(endEventID); + if(replayed) + return; + } m_pDevice->ReplayLog(0, endEventID, replayType); } diff --git a/renderdoc/driver/d3d12/d3d12_replay.h b/renderdoc/driver/d3d12/d3d12_replay.h index d45533068..2a3a5273d 100644 --- a/renderdoc/driver/d3d12/d3d12_replay.h +++ b/renderdoc/driver/d3d12/d3d12_replay.h @@ -266,7 +266,7 @@ private: bool CreateSOBuffers(); void ClearPostVSCache(); - void FetchShaderFeedback(uint32_t eventId); + bool FetchShaderFeedback(uint32_t eventId); void ClearFeedbackCache(); void RefreshDerivedReplacements(); diff --git a/renderdoc/driver/d3d12/d3d12_shader_feedback.cpp b/renderdoc/driver/d3d12/d3d12_shader_feedback.cpp index 3cff6c6e7..d48a3fab8 100644 --- a/renderdoc/driver/d3d12/d3d12_shader_feedback.cpp +++ b/renderdoc/driver/d3d12/d3d12_shader_feedback.cpp @@ -1360,16 +1360,16 @@ struct D3D12StatCallback : public D3D12ActionCallback ID3D12QueryHeap *m_PipeStatsQueryHeap; }; -void D3D12Replay::FetchShaderFeedback(uint32_t eventId) +bool D3D12Replay::FetchShaderFeedback(uint32_t eventId) { if(m_BindlessFeedback.Usage.find(eventId) != m_BindlessFeedback.Usage.end()) - return; + return false; if(!D3D12_BindlessFeedback()) - return; + return false; if(m_pDevice->HasFatalError()) - return; + return false; // create it here so we won't re-run any code if the event is re-selected. We'll mark it as valid // if it actually has any data in it later. @@ -1381,7 +1381,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) { // deliberately show no bindings as used for non-draws result.valid = true; - return; + return false; } result.compute = bool(action->flags & ActionFlags::Dispatch); @@ -1398,7 +1398,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) { RDCERR("Can't fetch shader feedback, no pipeline state bound"); result.valid = true; - return; + return false; } bytebuf editedBlob[(uint32_t)ShaderStage::Count]; @@ -1436,7 +1436,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) if(!sig) { result.valid = true; - return; + return false; } modsig = ((WrappedID3D12RootSignature *)sig)->sig; @@ -1456,7 +1456,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) if(!sig) { result.valid = true; - return; + return false; } modsig = ((WrappedID3D12RootSignature *)sig)->sig; @@ -1487,7 +1487,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) // Silently return if(numSlots == numReservedSlots) { - return; + return false; } // need to be able to add a descriptor of our UAV without hitting the 64 DWORD limit @@ -1519,7 +1519,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) if(FAILED(hr)) { RDCERR("Failed to create shader feedback pipeline query heap HRESULT: %s", ToStr(hr).c_str()); - return; + return false; } } @@ -1556,7 +1556,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) if(m_BindlessFeedback.FeedbackBuffer == NULL || FAILED(hr)) { RDCERR("Couldn't create feedback buffer with %u slots: %s", numSlots, ToStr(hr).c_str()); - return; + return false; } m_BindlessFeedback.FeedbackBuffer->SetName(L"m_BindlessFeedback.FeedbackBuffer"); @@ -1579,7 +1579,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) ID3D12GraphicsCommandList *list = m_pDevice->GetNewList(); if(!list) - return; + return false; GetDebugManager()->SetDescriptorHeaps(list, true, false); @@ -1602,7 +1602,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) if(annotatedSig == NULL || FAILED(hr)) { RDCERR("Couldn't create feedback modified root signature: %s", ToStr(hr).c_str()); - return; + return false; } } @@ -1616,11 +1616,11 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) { SAFE_RELEASE(annotatedSig); RDCERR("Couldn't create feedback modified pipeline: %s", ToStr(hr).c_str()); - return; + return false; } } - D3D12RenderState prev = rs; + D3D12RenderState prev = m_pDevice->GetQueue()->GetCommandData()->GetCurRenderState(); rs.pipe = GetResID(annotatedPipe); @@ -1648,7 +1648,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) ID3D12GraphicsCommandList *list = m_pDevice->GetNewList(); if(!list) - return; + return true; list->ResolveQueryData(m_BindlessFeedback.PipeStatsHeap, D3D12_QUERY_TYPE_PIPELINE_STATISTICS, 0, 1, m_BindlessFeedback.FeedbackBuffer, numSlots * sizeof(uint32_t)); @@ -1662,7 +1662,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) SAFE_RELEASE(annotatedPipe); SAFE_RELEASE(annotatedSig); - rs = prev; + rs = m_pDevice->GetQueue()->GetCommandData()->GetCurRenderState() = prev; bytebuf results; GetDebugManager()->GetBufferData(m_BindlessFeedback.FeedbackBuffer, 0, 0, results); @@ -1725,7 +1725,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) visMask = (uint32_t)ShaderStageMask::Geometry; break; case D3D12_SHADER_VISIBILITY_PIXEL: visMask = uint32_t(ShaderStageMask::Pixel); break; - default: RDCERR("Unexpected shader visibility %d", p.ShaderVisibility); return; + default: RDCERR("Unexpected shader visibility %d", p.ShaderVisibility); return true; } // set the key type @@ -1860,8 +1860,7 @@ void D3D12Replay::FetchShaderFeedback(uint32_t eventId) } } - // replay from the start as we may have corrupted state while fetching the above feedback. - m_pDevice->ReplayLog(0, eventId, eReplay_Full); + return true; } void D3D12Replay::ClearFeedbackCache() diff --git a/renderdoc/driver/vulkan/vk_overlay.cpp b/renderdoc/driver/vulkan/vk_overlay.cpp index 03530a792..2d3e493ce 100644 --- a/renderdoc/driver/vulkan/vk_overlay.cpp +++ b/renderdoc/driver/vulkan/vk_overlay.cpp @@ -1918,14 +1918,6 @@ ResourceId VulkanReplay::RenderOverlay(ResourceId texid, FloatVector clearCol, D size_t startEvent = 0; - // this is a bit of a hack, but it's the simplest fix without refactoring the ClearBefore.. - // overlay to copy the result off into the overlay texture (which has its own issues with - // alpha channels since the overlay expects to be blended). If we are selecting a new event - // with ClearBeforeDraw enabled then we do this here, and then later the pipeline is fetched - // and that triggers the bindless feedback - which will reset the state and undo our clear. If - // we force it to be cached here then it won't mess things up later. - FetchShaderFeedback(eventId); - // if we're ClearBeforePass the first event will be a vkBeginRenderPass. // if there are any other events, we need to play up to right before them // so that we have all the render state set up to do diff --git a/renderdoc/driver/vulkan/vk_replay.cpp b/renderdoc/driver/vulkan/vk_replay.cpp index 75fedbe97..9bf681974 100644 --- a/renderdoc/driver/vulkan/vk_replay.cpp +++ b/renderdoc/driver/vulkan/vk_replay.cpp @@ -227,6 +227,12 @@ RDResult VulkanReplay::ReadLogInitialisation(RDCFile *rdc, bool storeStructuredB void VulkanReplay::ReplayLog(uint32_t endEventID, ReplayLogType replayType) { + if(replayType == eReplay_OnlyDraw) + { + bool replayed = FetchShaderFeedback(endEventID); + if(replayed) + return; + } m_pDriver->ReplayLog(0, endEventID, replayType); } diff --git a/renderdoc/driver/vulkan/vk_replay.h b/renderdoc/driver/vulkan/vk_replay.h index dc022dd20..aebf330e8 100644 --- a/renderdoc/driver/vulkan/vk_replay.h +++ b/renderdoc/driver/vulkan/vk_replay.h @@ -446,7 +446,7 @@ public: uint32_t bufferOffset, VkFormat format, VkDescriptorSet descSet); private: - void FetchShaderFeedback(uint32_t eventId); + bool FetchShaderFeedback(uint32_t eventId); void ClearFeedbackCache(); void PatchReservedDescriptors(const VulkanStatePipeline &pipe, VkDescriptorPool &descpool, diff --git a/renderdoc/driver/vulkan/vk_shader_feedback.cpp b/renderdoc/driver/vulkan/vk_shader_feedback.cpp index adda42619..e711b41ee 100644 --- a/renderdoc/driver/vulkan/vk_shader_feedback.cpp +++ b/renderdoc/driver/vulkan/vk_shader_feedback.cpp @@ -1354,13 +1354,13 @@ void VulkanReplay::ClearFeedbackCache() m_BindlessFeedback.Usage.clear(); } -void VulkanReplay::FetchShaderFeedback(uint32_t eventId) +bool VulkanReplay::FetchShaderFeedback(uint32_t eventId) { if(m_BindlessFeedback.Usage.find(eventId) != m_BindlessFeedback.Usage.end()) - return; + return false; if(!Vulkan_BindlessFeedback()) - return; + return false; // create it here so we won't re-run any code if the event is re-selected. We'll mark it as valid // if it actually has any data in it later. @@ -1385,7 +1385,7 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) { // deliberately show no bindings as used for non-draws result.valid = true; - return; + return false; } result.compute = bool(action->flags & ActionFlags::Dispatch); @@ -1395,7 +1395,7 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) if(pipe.pipeline == ResourceId()) { result.valid = true; - return; + return false; } const VulkanCreationInfo::Pipeline &pipeInfo = creationInfo.m_Pipeline[pipe.pipeline]; @@ -1501,7 +1501,7 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) // if we don't have any array descriptors or printf's to feedback then just return now if(offsetMap.empty() && !usesPrintf) { - return; + return false; } if(!result.compute) @@ -1510,13 +1510,10 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) if(!m_pDriver->GetDeviceEnabledFeatures().vertexPipelineStoresAndAtomics && !m_pDriver->GetDeviceEnabledFeatures().fragmentStoresAndAtomics) { - return; + return false; } } - // replay from the start without this action, as we want a clean state to do printfs from. - m_pDriver->ReplayLog(0, eventId, eReplay_WithoutDraw); - // we go through the driver for all these creations since they need to be properly // registered in order to be put in the partial replay state. Our patched shader is valid so we // don't need to replay after doing the feedback execute @@ -1576,7 +1573,7 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) // if the pool failed due to limits, it will be NULL so bail now if(descpool == VK_NULL_HANDLE) - return; + return false; // create pipeline layout with new descriptor set layouts { @@ -1775,7 +1772,7 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) VkCommandBuffer cmd = m_pDriver->GetNextCmd(); if(cmd == VK_NULL_HANDLE) - return; + return false; VkCommandBufferBeginInfo beginInfo = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO, NULL, VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT}; @@ -1994,6 +1991,8 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) for(size_t i = 0; i < ARRAY_COUNT(modules); i++) if(modules[i] != VK_NULL_HANDLE) m_pDriver->vkDestroyShaderModule(dev, modules[i], NULL); + + return true; } #if ENABLED(ENABLE_UNIT_TESTS)