From 615c02dbd72e21948534aa83159de3af518f09d5 Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 23 Jun 2023 18:44:30 +0100 Subject: [PATCH] Fix handling when rendering to mips of an image in pixel history --- renderdoc/driver/d3d11/d3d11_debug.h | 4 +- renderdoc/driver/d3d11/d3d11_pixelhistory.cpp | 13 +++-- renderdoc/driver/vulkan/vk_debug.h | 4 +- renderdoc/driver/vulkan/vk_pixelhistory.cpp | 56 ++++++++++++------- .../driver/vulkan/wrappers/vk_cmd_funcs.cpp | 9 ++- 5 files changed, 53 insertions(+), 33 deletions(-) diff --git a/renderdoc/driver/d3d11/d3d11_debug.h b/renderdoc/driver/d3d11/d3d11_debug.h index 566efc974..1f10d3435 100644 --- a/renderdoc/driver/d3d11/d3d11_debug.h +++ b/renderdoc/driver/d3d11/d3d11_debug.h @@ -38,7 +38,7 @@ class WrappedID3D11DeviceContext; class D3D11ResourceManager; -struct CopyPixelParams; +struct D3D11CopyPixelParams; namespace DXBC { @@ -147,7 +147,7 @@ public: TextureShaderDetails GetShaderDetails(ResourceId id, CompType typeCast, bool rawOutput); - void PixelHistoryCopyPixel(CopyPixelParams ¶ms, size_t eventSlot, uint32_t storeSlot); + void PixelHistoryCopyPixel(D3D11CopyPixelParams ¶ms, size_t eventSlot, uint32_t storeSlot); struct CacheElem { diff --git a/renderdoc/driver/d3d11/d3d11_pixelhistory.cpp b/renderdoc/driver/d3d11/d3d11_pixelhistory.cpp index 33a4159f3..77f436b73 100644 --- a/renderdoc/driver/d3d11/d3d11_pixelhistory.cpp +++ b/renderdoc/driver/d3d11/d3d11_pixelhistory.cpp @@ -131,7 +131,7 @@ * PixelModifications */ -struct CopyPixelParams +struct D3D11CopyPixelParams { bool multisampled; bool floatTex; @@ -160,7 +160,8 @@ static const uint32_t pixstoreStride = 4; // Helper function to copy a single pixel out of a source texture, which will handle any texture // type and binding type, doing any copying as needed. Writes the result to a given buffer UAV. -void D3D11DebugManager::PixelHistoryCopyPixel(CopyPixelParams &p, size_t eventSlot, uint32_t storeSlot) +void D3D11DebugManager::PixelHistoryCopyPixel(D3D11CopyPixelParams &p, size_t eventSlot, + uint32_t storeSlot) { // perform a subresource copy if the real source tex couldn't be directly bound as SRV if(p.sourceTex != p.srvTex && p.sourceTex && p.srvTex) @@ -611,7 +612,7 @@ rdcarray D3D11Replay::PixelHistory(rdcarray event WrappedID3D11Texture3D1::m_TextureList.end()) targetres = WrappedID3D11Texture3D1::m_TextureList[target].m_Texture; - CopyPixelParams colourCopyParams = {}; + D3D11CopyPixelParams colourCopyParams = {}; // common parameters colourCopyParams.multisampled = multisampled; @@ -625,7 +626,7 @@ rdcarray D3D11Replay::PixelHistory(rdcarray event else colourCopyParams.subres = details.texArraySize * slice + mip; - CopyPixelParams depthCopyParams = colourCopyParams; + D3D11CopyPixelParams depthCopyParams = colourCopyParams; colourCopyParams.depthcopy = false; colourCopyParams.sourceTex = (ID3D11Texture2D *)targetres; @@ -1274,7 +1275,7 @@ rdcarray D3D11Replay::PixelHistory(rdcarray event m_pImmediateContext->OMGetRenderTargetsAndUnorderedAccessViews( UAVStartSlot, prevRTVs, &prevDSV, UAVStartSlot, numUAVs - UAVStartSlot, prevUAVs); - CopyPixelParams params = depthCopyParams; + D3D11CopyPixelParams params = depthCopyParams; params.depthbound = true; params.srvTex = params.sourceTex = shaddepthOutput; params.srv[0] = shaddepthOutputDepthSRV; @@ -1987,7 +1988,7 @@ rdcarray D3D11Replay::PixelHistory(rdcarray event ID3D11DepthStencilState *ds = NULL; - CopyPixelParams shadoutCopyParams = colourCopyParams; + D3D11CopyPixelParams shadoutCopyParams = colourCopyParams; shadoutCopyParams.sourceTex = shadoutCopyParams.srvTex = shadOutput; shadoutCopyParams.srv[0] = shadOutputSRV; shadoutCopyParams.uav = shadoutStoreUAV; diff --git a/renderdoc/driver/vulkan/vk_debug.h b/renderdoc/driver/vulkan/vk_debug.h index 5a0952700..84c5b2f07 100644 --- a/renderdoc/driver/vulkan/vk_debug.h +++ b/renderdoc/driver/vulkan/vk_debug.h @@ -49,7 +49,7 @@ struct VKMeshDisplayPipelines uint32_t secondaryStridePadding = 0; }; -struct CopyPixelParams; +struct VkCopyPixelParams; struct PixelHistoryResources; @@ -99,7 +99,7 @@ public: const Subresource &sub, uint32_t numEvents); bool PixelHistoryDestroyResources(const PixelHistoryResources &resources); - void PixelHistoryCopyPixel(VkCommandBuffer cmd, CopyPixelParams &p, size_t offset); + void PixelHistoryCopyPixel(VkCommandBuffer cmd, VkCopyPixelParams &p, size_t offset); VkImageLayout GetImageLayout(ResourceId image, VkImageAspectFlagBits aspect, uint32_t mip, uint32_t slice); diff --git a/renderdoc/driver/vulkan/vk_pixelhistory.cpp b/renderdoc/driver/vulkan/vk_pixelhistory.cpp index ace78b2eb..500ef7b28 100644 --- a/renderdoc/driver/vulkan/vk_pixelhistory.cpp +++ b/renderdoc/driver/vulkan/vk_pixelhistory.cpp @@ -146,13 +146,14 @@ enum : uint32_t DepthTest_GreaterEqual = 7U << DepthTest_Shift, }; -struct CopyPixelParams +struct VkCopyPixelParams { VkImage srcImage; VkFormat srcImageFormat; VkImageLayout srcImageLayout; bool multisampled; bool multiview; + Subresource sub; }; struct PixelHistoryResources @@ -1247,7 +1248,7 @@ protected: return descSet; } - void CopyImagePixel(VkCommandBuffer cmd, CopyPixelParams &p, size_t offset) + void CopyImagePixel(VkCommandBuffer cmd, VkCopyPixelParams &p, size_t offset) { VkImageAspectFlags aspectFlags = 0; bool depthCopy = IsDepthOrStencilFormat(p.srcImageFormat); @@ -1263,8 +1264,8 @@ protected: aspectFlags = VK_IMAGE_ASPECT_COLOR_BIT; } - uint32_t baseMip = m_CallbackInfo.targetSubresource.mip; - uint32_t baseSlice = m_CallbackInfo.targetSubresource.slice; + uint32_t baseMip = p.sub.mip; + uint32_t baseSlice = p.sub.slice; // The images that are created specifically for evaluating pixel history are // already based on the target mip/slice. Unless we're using multiview, in which case the output // is still view-dependent. @@ -1657,12 +1658,13 @@ struct VulkanColorAndStencilCallback : public VulkanPixelHistoryCallback pipestate.back = pipestate.front; ReplayDraw(cmd, eid, true); - CopyPixelParams params = {}; + VkCopyPixelParams params = {}; params.srcImage = m_CallbackInfo.dsImage; params.srcImageLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; params.srcImageFormat = m_CallbackInfo.dsFormat; params.multisampled = (m_CallbackInfo.samples != VK_SAMPLE_COUNT_1_BIT); params.multiview = multiview; + params.sub = m_CallbackInfo.targetSubresource; // Copy stencil value that indicates the number of fragments ignoring // shader discard. CopyImagePixel(cmd, params, storeOffset + offsetof(struct EventInfo, dsWithoutShaderDiscard)); @@ -1828,14 +1830,14 @@ struct VulkanColorAndStencilCallback : public VulkanPixelHistoryCallback if(!m_Events.contains(eid)) return; size_t storeOffset = m_EventIndices.size() * sizeof(EventInfo); - CopyPixel(eid, cmd, storeOffset); + CopyPixel(eid, cmd, storeOffset, false); } bool PostDispatch(uint32_t eid, VkCommandBuffer cmd) { if(!m_Events.contains(eid)) return false; size_t storeOffset = m_EventIndices.size() * sizeof(EventInfo); - CopyPixel(eid, cmd, storeOffset + offsetof(struct EventInfo, postmod)); + CopyPixel(eid, cmd, storeOffset + offsetof(struct EventInfo, postmod), false); m_EventIndices.insert(std::make_pair(eid, m_EventIndices.size())); return false; } @@ -1920,12 +1922,13 @@ struct VulkanColorAndStencilCallback : public VulkanPixelHistoryCallback } private: - void CopyPixel(uint32_t eid, VkCommandBuffer cmd, size_t offset) + void CopyPixel(uint32_t eid, VkCommandBuffer cmd, size_t offset, bool autoDepthCopy = true) { - CopyPixelParams targetCopyParams = {}; + VkCopyPixelParams targetCopyParams = {}; targetCopyParams.srcImage = m_CallbackInfo.targetImage; targetCopyParams.srcImageFormat = m_CallbackInfo.targetImageFormat; targetCopyParams.multisampled = (m_CallbackInfo.samples != VK_SAMPLE_COUNT_1_BIT); + targetCopyParams.sub = m_CallbackInfo.targetSubresource; VkImageAspectFlagBits aspect = VK_IMAGE_ASPECT_COLOR_BIT; if(IsDepthOrStencilFormat(m_CallbackInfo.targetImageFormat)) { @@ -1938,14 +1941,17 @@ private: CopyImagePixel(cmd, targetCopyParams, offset); // If the target image is a depth/stencil attachment, we already - // copied the value above. - if(IsDepthOrStencilFormat(m_CallbackInfo.targetImageFormat)) + // copied the value above. Or if we are not wanting to automatically copy the depth (e.g. in the + // case that this is a clear, dispatch, or other write where copying depth is irrelevant and the + // pixel co-ordinate we're tracking may even be out of bounds) + if(IsDepthOrStencilFormat(m_CallbackInfo.targetImageFormat) || autoDepthCopy == false) return; const VulkanRenderState &state = m_pDriver->GetCmdRenderState(); ResourceId depthImageId; VkImageLayout depthLayout = VK_IMAGE_LAYOUT_UNDEFINED; + Subresource depthSub; if(state.dynamicRendering.active) { @@ -1959,7 +1965,10 @@ private: if(dsView != VK_NULL_HANDLE) { - depthImageId = m_pDriver->GetDebugManager()->GetImageViewInfo(GetResID(dsView)).image; + const VulkanCreationInfo::ImageView &viewInfo = + m_pDriver->GetDebugManager()->GetImageViewInfo(GetResID(dsView)); + depthImageId = viewInfo.image; + depthSub = {viewInfo.range.baseMipLevel, viewInfo.range.baseArrayLayer}; } } else if(state.GetRenderPass() != ResourceId()) @@ -1972,7 +1981,10 @@ private: if(att >= 0) { - depthImageId = m_pDriver->GetDebugManager()->GetImageViewInfo(atts[att]).image; + const VulkanCreationInfo::ImageView &viewInfo = + m_pDriver->GetDebugManager()->GetImageViewInfo(atts[att]); + depthImageId = viewInfo.image; + depthSub = {viewInfo.range.baseMipLevel, viewInfo.range.baseArrayLayer}; depthLayout = rpInfo.subpasses[state.subpass].depthLayout; } } @@ -1983,11 +1995,12 @@ private: const VulkanCreationInfo::Image &imginfo = m_pDriver->GetDebugManager()->GetImageInfo(depthImageId); - CopyPixelParams depthCopyParams = targetCopyParams; + VkCopyPixelParams depthCopyParams = targetCopyParams; depthCopyParams.srcImage = depthImage; depthCopyParams.srcImageLayout = depthLayout; depthCopyParams.srcImageFormat = imginfo.format; depthCopyParams.multisampled = (imginfo.samples != VK_SAMPLE_COUNT_1_BIT); + depthCopyParams.sub = depthSub; CopyImagePixel(cmd, depthCopyParams, offset + offsetof(struct PixelHistoryValue, depth)); m_DepthFormats.insert(std::make_pair(eid, imginfo.format)); } @@ -2710,11 +2723,12 @@ struct VulkanPixelHistoryPerFragmentCallback : VulkanPixelHistoryCallback pipesIter[0] = pipes.primitiveIdPipe; pipesIter[1] = pipes.shaderOutPipe; - CopyPixelParams colourCopyParams = {}; + VkCopyPixelParams colourCopyParams = {}; colourCopyParams.srcImage = m_CallbackInfo.subImage; colourCopyParams.srcImageFormat = VK_FORMAT_R32G32B32A32_SFLOAT; colourCopyParams.multisampled = (m_CallbackInfo.samples != VK_SAMPLE_COUNT_1_BIT); colourCopyParams.multiview = multiview; + colourCopyParams.sub = m_CallbackInfo.targetSubresource; if(IsDepthOrStencilFormat(m_CallbackInfo.targetImageFormat)) { colourCopyParams.srcImageLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; @@ -2829,7 +2843,7 @@ struct VulkanPixelHistoryPerFragmentCallback : VulkanPixelHistoryCallback storeOffset += offsetof(struct PerFragmentInfo, shaderOut); if(depthEnabled) { - CopyPixelParams depthCopyParams = colourCopyParams; + VkCopyPixelParams depthCopyParams = colourCopyParams; depthCopyParams.srcImage = m_CallbackInfo.dsImage; depthCopyParams.srcImageLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; depthCopyParams.srcImageFormat = m_CallbackInfo.dsFormat; @@ -2952,7 +2966,7 @@ struct VulkanPixelHistoryPerFragmentCallback : VulkanPixelHistoryCallback if(depthImage != VK_NULL_HANDLE) { - CopyPixelParams depthCopyParams = colourCopyParams; + VkCopyPixelParams depthCopyParams = colourCopyParams; depthCopyParams.srcImage = m_CallbackInfo.dsImage; depthCopyParams.srcImageLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; depthCopyParams.srcImageFormat = m_CallbackInfo.dsFormat; @@ -3836,11 +3850,12 @@ rdcarray VulkanReplay::PixelHistory(rdcarray even if(events[ev].view != ResourceId()) { - // TODO: Check that the slice and mip matches. VulkanCreationInfo::ImageView viewInfo = m_pDriver->GetDebugManager()->GetImageViewInfo(events[ev].view); uint32_t layerEnd = viewInfo.range.baseArrayLayer + viewInfo.range.layerCount; - if(sub.slice < viewInfo.range.baseArrayLayer || sub.slice >= layerEnd) + uint32_t levelEnd = viewInfo.range.baseMipLevel + viewInfo.range.levelCount; + if(sub.slice < viewInfo.range.baseArrayLayer || sub.slice >= layerEnd || + sub.mip < viewInfo.range.baseMipLevel || sub.mip >= levelEnd) { RDCDEBUG("Usage %d at %u didn't refer to the matching mip/slice (%u/%u)", events[ev].usage, events[ev].eventId, sub.mip, sub.slice); @@ -3895,7 +3910,8 @@ rdcarray VulkanReplay::PixelHistory(rdcarray even bool clear = (events[ev].usage == ResourceUsage::Clear); bool directWrite = isDirectWrite(events[ev].usage); - if(drawEvents.contains(events[ev].eventId) || clear || directWrite) + if(drawEvents.contains(events[ev].eventId) || + (modEvents.contains(events[ev].eventId) && (clear || directWrite))) { PixelModification mod; RDCEraseEl(mod); diff --git a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp index 70babc0e3..41390d2cc 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp @@ -1977,7 +1977,8 @@ bool WrappedVulkan::Serialise_vkCmdBeginRenderPass(SerialiserType &ser, VkComman image, EventUsage(m_BakedCmdBufferInfo[m_LastCmdBufferID].curEventID, rpinfo.attachments[i].loadOp == VK_ATTACHMENT_LOAD_OP_CLEAR ? ResourceUsage::Clear - : ResourceUsage::Discard))); + : ResourceUsage::Discard, + fbattachments[i]))); } } @@ -2631,7 +2632,8 @@ bool WrappedVulkan::Serialise_vkCmdBeginRenderPass2(SerialiserType &ser, image, EventUsage(m_BakedCmdBufferInfo[m_LastCmdBufferID].curEventID, rpinfo.attachments[i].loadOp == VK_ATTACHMENT_LOAD_OP_CLEAR ? ResourceUsage::Clear - : ResourceUsage::Discard))); + : ResourceUsage::Discard, + fbattachments[i]))); } } @@ -7147,7 +7149,8 @@ bool WrappedVulkan::Serialise_vkCmdBeginRendering(SerialiserType &ser, VkCommand m_BakedCmdBufferInfo[m_LastCmdBufferID].resourceUsage.push_back(make_rdcpair( image, EventUsage(m_BakedCmdBufferInfo[m_LastCmdBufferID].curEventID, att->loadOp == VK_ATTACHMENT_LOAD_OP_CLEAR ? ResourceUsage::Clear - : ResourceUsage::Discard))); + : ResourceUsage::Discard, + GetResID(att->imageView)))); } }