From a39c4cf1db9ed398129ee03257e12b07807ed47c Mon Sep 17 00:00:00 2001 From: Jake Turner Date: Tue, 26 Nov 2024 11:10:35 +0000 Subject: [PATCH] D3D12 Pixel History do not copy depth for non-raster events This can be invalid i.e. a ClearRenderTarget() no guarantee what/if a depth target is bound --- renderdoc/driver/d3d12/d3d12_pixelhistory.cpp | 91 ++++++++++++++----- util/test/tests/D3D12/D3D12_Pixel_History.py | 5 +- 2 files changed, 70 insertions(+), 26 deletions(-) diff --git a/renderdoc/driver/d3d12/d3d12_pixelhistory.cpp b/renderdoc/driver/d3d12/d3d12_pixelhistory.cpp index e1e1ffbc99..8c4fb39b3e 100644 --- a/renderdoc/driver/d3d12/d3d12_pixelhistory.cpp +++ b/renderdoc/driver/d3d12/d3d12_pixelhistory.cpp @@ -653,6 +653,15 @@ struct D3D12PixelHistoryCallback : public D3D12ActionCallback void CopyImagePixel(ID3D12GraphicsCommandListX *cmd, D3D12CopyPixelParams &p, size_t offset) { + D3D12_RESOURCE_DESC srcDesc = p.srcImage->GetDesc(); + if((p.x < 0 || p.y < 0 || p.x >= srcDesc.Width || p.y >= srcDesc.Height)) + { + // If the pixel is out of bounds, we can't read from the target image + RDCERR("Pixel is out of bounds %d,%d Dimensions %d x %d", p.x, p.y, srcDesc.Width, + srcDesc.Height); + return; + } + uint32_t baseMip = m_CallbackInfo.targetSubresource.mip; bool copy3d = m_CallbackInfo.targetDesc.Dimension == D3D12_RESOURCE_DIMENSION_TEXTURE3D; uint32_t baseSlice = m_CallbackInfo.targetSubresource.slice; @@ -990,15 +999,21 @@ struct D3D12OcclusionCallback : public D3D12PixelHistoryCallback rdcarray m_OcclusionResults; }; +struct EventInfo +{ + D3D12_RESOURCE_STATES resourceState; + bool hasDepth; +}; + struct D3D12ColorAndStencilCallback : public D3D12PixelHistoryCallback { D3D12ColorAndStencilCallback(WrappedID3D12Device *device, D3D12PixelHistoryShaderCache *shaderCache, const D3D12PixelHistoryCallbackInfo &callbackInfo, const rdcarray &events, - std::map resourceStates) + const std::map &eventInfos) : D3D12PixelHistoryCallback(device, shaderCache, callbackInfo, NULL), m_Events(events), - m_ResourceStates(resourceStates) + m_EventInfos(eventInfos) { } @@ -1178,7 +1193,9 @@ struct D3D12ColorAndStencilCallback : public D3D12PixelHistoryCallback targetCopyParams.mip = m_CallbackInfo.targetSubresource.mip; targetCopyParams.arraySlice = m_CallbackInfo.targetSubresource.slice; targetCopyParams.multisampled = (m_CallbackInfo.targetDesc.SampleDesc.Count != 1); - D3D12_RESOURCE_STATES nonRtFallback = m_ResourceStates[eid]; + const EventInfo &eventInfo = m_EventInfos.at(eid); + bool hasDepth = eventInfo.hasDepth; + D3D12_RESOURCE_STATES nonRtFallback = eventInfo.resourceState; bool rtOutput = (nonRtFallback == D3D12_RESOURCE_STATE_RENDER_TARGET); D3D12_RESOURCE_STATES fallback = D3D12_RESOURCE_STATE_RENDER_TARGET; @@ -1218,6 +1235,10 @@ struct D3D12ColorAndStencilCallback : public D3D12PixelHistoryCallback if(depthTarget) return; + // return if the event does not have valid depth i.e. Clear, Copy, Dispatch + if(!hasDepth) + return; + // Get the bound depth format for this event WrappedID3D12PipelineState *pipe = m_pDevice->GetResourceManager()->GetCurrentAs(m_SavedState.pipe); @@ -1356,7 +1377,7 @@ struct D3D12ColorAndStencilCallback : public D3D12PixelHistoryCallback D3D12RenderState m_SavedState; std::map m_PipeCache; rdcarray m_Events; - std::map m_ResourceStates; + const std::map &m_EventInfos; // Key is event ID, and value is an index of where the event data is stored. std::map m_EventIndices; std::map m_DepthFormats; @@ -2887,12 +2908,17 @@ rdcarray D3D12Replay::PixelHistory(rdcarray event // to determine if these draws failed for some reason (for ex., depth test). rdcarray modEvents; rdcarray drawEvents; - std::map resourceStates; + std::map eventInfos; for(size_t ev = 0; ev < events.size(); ev++) { ResourceUsage usage = events[ev].usage; + const uint32_t eventId = events[ev].eventId; bool clear = (usage == ResourceUsage::Clear); - bool directWrite = IsDirectWrite(events[ev].usage); + bool directWrite = IsDirectWrite(usage); + EventInfo eventInfo = {}; + const ActionDescription *action = m_pDevice->GetAction(eventId); + eventInfo.hasDepth = + (action->flags & (ActionFlags::MeshDispatch | ActionFlags::Drawcall)) ? true : false; D3D12_RESOURCE_STATES resourceState = D3D12_RESOURCE_STATE_RENDER_TARGET; if(IsUavWrite(usage)) @@ -2901,26 +2927,28 @@ rdcarray D3D12Replay::PixelHistory(rdcarray event resourceState = D3D12_RESOURCE_STATE_RESOLVE_DEST; else if(IsCopyWrite(usage)) resourceState = D3D12_RESOURCE_STATE_COPY_DEST; - resourceStates[events[ev].eventId] = resourceState; + eventInfo.resourceState = resourceState; if(directWrite || clear) { - modEvents.push_back(events[ev].eventId); + modEvents.push_back(eventId); + RDCASSERT(eventInfo.hasDepth == false); } else { - uint64_t occlData = occlCb.GetOcclusionResult((uint32_t)events[ev].eventId); + uint64_t occlData = occlCb.GetOcclusionResult(eventId); if(occlData > 0) { D3D12MarkerRegion::Set(m_pDevice->GetQueue()->GetReal(), - StringFormat::Fmt("%u has occl %llu", events[ev].eventId, occlData)); - drawEvents.push_back(events[ev].eventId); - modEvents.push_back(events[ev].eventId); + StringFormat::Fmt("%u has occl %llu", eventId, occlData)); + drawEvents.push_back(eventId); + modEvents.push_back(eventId); } } + eventInfos[eventId] = eventInfo; } - D3D12ColorAndStencilCallback cb(m_pDevice, shaderCache, callbackInfo, modEvents, resourceStates); + D3D12ColorAndStencilCallback cb(m_pDevice, shaderCache, callbackInfo, modEvents, eventInfos); { D3D12MarkerRegion colorStencilRegion(m_pDevice->GetQueue()->GetReal(), "D3D12ColorAndStencilCallback"); @@ -3034,22 +3062,35 @@ rdcarray D3D12Replay::PixelHistory(rdcarray event FillInColor(fmt, ei.postmod, mod.postMod); } - DXGI_FORMAT depthFormat = cb.GetDepthFormat(mod.eventId); - if(depthFormat != DXGI_FORMAT_UNKNOWN) + EventInfo eventInfo = eventInfos[eid]; + bool hasDepth = eventInfo.hasDepth; + + if(hasDepth) { - mod.preMod.stencil = ei.premod.stencil; - mod.postMod.stencil = ei.postmod.stencil; - if(multisampled) + DXGI_FORMAT depthFormat = cb.GetDepthFormat(mod.eventId); + if(depthFormat != DXGI_FORMAT_UNKNOWN) { - mod.preMod.depth = ei.premod.depth.fdepth; - mod.postMod.depth = ei.postmod.depth.fdepth; - } - else - { - mod.preMod.depth = GetDepthValue(depthFormat, ei.premod); - mod.postMod.depth = GetDepthValue(depthFormat, ei.postmod); + mod.preMod.stencil = ei.premod.stencil; + mod.postMod.stencil = ei.postmod.stencil; + if(multisampled) + { + mod.preMod.depth = ei.premod.depth.fdepth; + mod.postMod.depth = ei.postmod.depth.fdepth; + } + else + { + mod.preMod.depth = GetDepthValue(depthFormat, ei.premod); + mod.postMod.depth = GetDepthValue(depthFormat, ei.postmod); + } } } + else + { + mod.preMod.stencil = -1; + mod.preMod.depth = -1; + mod.postMod.stencil = -1; + mod.postMod.depth = -1; + } int32_t frags = int32_t(ei.dsWithoutShaderDiscard[0]); int32_t fragsClipped = int32_t(ei.dsWithShaderDiscard[0]); diff --git a/util/test/tests/D3D12/D3D12_Pixel_History.py b/util/test/tests/D3D12/D3D12_Pixel_History.py index acf179c60e..55b54684db 100644 --- a/util/test/tests/D3D12/D3D12_Pixel_History.py +++ b/util/test/tests/D3D12/D3D12_Pixel_History.py @@ -471,7 +471,7 @@ def depth_target_test(self, begin_name: str): rdtest.log.print("Testing pixel {}, {}".format(x, y)) modifs: List[rd.PixelModification] = self.controller.PixelHistory(tex, x, y, sub, rt.format.compType) events = [ - [[event_id, begin_renderpass_eid], [passed, True], [get_post_mod_depth, 1.0]], + [[event_id, begin_renderpass_eid], [passed, True], [get_post_mod_depth, -1.0]], [[event_id, background_eid], [passed, True], [primitive_id, 0], [get_pre_mod_depth, 1.0], [get_post_mod_depth, 0.95]], [[event_id, test_eid], [passed, True], [depth_test_failed, False], [primitive_id, 0], [get_shader_out_depth, 0.5], [get_post_mod_depth, 0.5]], [[event_id, test_eid], [passed, False], [depth_test_failed, True], [primitive_id, 1], [get_shader_out_depth, 0.6], [get_post_mod_depth, 0.5]], @@ -549,6 +549,9 @@ def check_modifs_consistent(self, modifs): if self.is_depth: a = (modifs[i].postMod.depth, modifs[i].postMod.stencil) b = (modifs[i + 1].preMod.depth, modifs[i + 1].preMod.stencil) + # ignore if postmod depth data is unknwon + if a[0] == -1 and a[1] == -1: + continue if a != b: raise rdtest.TestFailureException(