From 87a172167fdfa1adf9859e9db94e2b8e72093942 Mon Sep 17 00:00:00 2001 From: Benson Joeris Date: Tue, 1 Oct 2019 09:14:40 -0400 Subject: [PATCH] Reset images when backing memory is Reset There are some situations that cause us to reset the memory backing an image, but not the image itself--this puts the image in a corrupted state. This fix updates the image initialization logic to check the bound memory. If the memory is written by any captured command, or if the memory is reset, then the entire is image is assumed to be uninitialized. Change-Id: Ifb1ec6bfd83f93def0818dbabcb0ea57f0febe54 --- renderdoc/driver/vulkan/vk_core.cpp | 2 +- renderdoc/driver/vulkan/vk_initstate.cpp | 85 ++++++++++++++----- renderdoc/driver/vulkan/vk_manager.cpp | 4 +- renderdoc/driver/vulkan/vk_rendertexture.cpp | 2 +- renderdoc/driver/vulkan/vk_replay.cpp | 8 +- renderdoc/driver/vulkan/vk_resources.h | 5 +- .../vulkan/wrappers/vk_resource_funcs.cpp | 20 +++-- .../driver/vulkan/wrappers/vk_wsi_funcs.cpp | 4 +- 8 files changed, 90 insertions(+), 40 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index d232f3ee4..1a4d51dc3 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -1382,7 +1382,7 @@ bool WrappedVulkan::Serialise_BeginCaptureFrame(SerialiserType &ser) // PREINIT as if it was GENERAL. for(auto it = m_ImageLayouts.begin(); it != m_ImageLayouts.end(); ++it) { - if(!it->second.memoryBound) + if(!it->second.isMemoryBound) continue; for(auto stit = it->second.subresourceStates.begin(); diff --git a/renderdoc/driver/vulkan/vk_initstate.cpp b/renderdoc/driver/vulkan/vk_initstate.cpp index b50272c5f..eb0d69d4f 100644 --- a/renderdoc/driver/vulkan/vk_initstate.cpp +++ b/renderdoc/driver/vulkan/vk_initstate.cpp @@ -124,7 +124,7 @@ bool WrappedVulkan::Prepare_InitialState(WrappedVkRes *res) } // if the image has no memory bound, nothing is to be fetched - if(!layout->memoryBound) + if(!layout->isMemoryBound) return true; VkDevice d = GetDev(); @@ -1482,6 +1482,7 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten ResourceId orig = GetResourceManager()->GetOriginalID(id); ImgRefs *imgRefs = GetResourceManager()->FindImgRefs(orig); + bool initialized = false; InitPolicy policy = GetResourceManager()->GetInitPolicy(); @@ -1491,6 +1492,44 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten imgRefs->initializedLiveRes = live; } + ImageLayouts &layout = m_ImageLayouts[id]; + + if(initialized && layout.boundMemory != ResourceId()) + { + ResourceId origMem = GetResourceManager()->GetOriginalID(layout.boundMemory); + if(origMem != ResourceId()) + { + MemRefs *memRefs = GetResourceManager()->FindMemRefs(origMem); + // Check whether any portion of the device memory range bound to this image is written. + // The memory might be written by a captured command (e.g. mapped memory), or might have + // initial contents. + if(memRefs == NULL) + { + // The device memory allocation is missing reference info, and so the memory will be reset + // before each frame; this means the image needs to be treated as if it is uninitialized + // at the beginning of each replay. + initialized = false; + } + else + { + for(auto it = memRefs->rangeRefs.find(layout.boundMemoryOffset); + it != memRefs->rangeRefs.end() && + it->start() < layout.boundMemoryOffset + layout.boundMemorySize; + ++it) + { + if(IncludesWrite(it->value()) || + InitReq(it->value(), policy, initialized) != eInitReq_None) + { + // The bound memory is written, either by a captured command or by the device memory + // initialization policy. + initialized = false; + break; + } + } + } + } + } + if(initial.tag == VkInitialContents::Sparse) { Apply_SparseInitialState((WrappedVkImage *)live, initial); @@ -1502,7 +1541,7 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten { if(initial.tag == VkInitialContents::ClearColorImage) { - VkFormat format = m_ImageLayouts[id].imageInfo.format; + VkFormat format = layout.imageInfo.format; // can't clear these, so leave them alone. if(IsBlockFormat(format) || IsYUVFormat(format)) @@ -1520,7 +1559,7 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten 0, VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, - m_ImageLayouts[id].queueFamilyIndex, + layout.queueFamilyIndex, m_QueueFamilyIdx, ToHandle(live), {VK_IMAGE_ASPECT_COLOR_BIT, 0, VK_REMAINING_MIP_LEVELS, 0, VK_REMAINING_ARRAY_LAYERS}, @@ -1541,10 +1580,10 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten RDCASSERTEQUAL(vkr, VK_SUCCESS); } - for(size_t si = 0; si < m_ImageLayouts[id].subresourceStates.size(); si++) + for(size_t si = 0; si < layout.subresourceStates.size(); si++) { - barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; - barrier.oldLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + barrier.subresourceRange = layout.subresourceStates[si].subresourceRange; + barrier.oldLayout = layout.subresourceStates[si].newLayout; SanitiseOldImageLayout(barrier.oldLayout); @@ -1583,10 +1622,10 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten RDCASSERTEQUAL(vkr, VK_SUCCESS); } - for(size_t si = 0; si < m_ImageLayouts[id].subresourceStates.size(); si++) + for(size_t si = 0; si < layout.subresourceStates.size(); si++) { - barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; - barrier.newLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + barrier.subresourceRange = layout.subresourceStates[si].subresourceRange; + barrier.newLayout = layout.subresourceStates[si].newLayout; barrier.dstAccessMask |= MakeAccessMask(barrier.newLayout); SanitiseNewImageLayout(barrier.newLayout); @@ -1630,7 +1669,7 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten 0, VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, - m_ImageLayouts[id].queueFamilyIndex, + layout.queueFamilyIndex, m_QueueFamilyIdx, ToHandle(live), {VK_IMAGE_ASPECT_COLOR_BIT, 0, VK_REMAINING_MIP_LEVELS, 0, VK_REMAINING_ARRAY_LAYERS}, @@ -1651,10 +1690,10 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten RDCASSERTEQUAL(vkr, VK_SUCCESS); } - for(size_t si = 0; si < m_ImageLayouts[id].subresourceStates.size(); si++) + for(size_t si = 0; si < layout.subresourceStates.size(); si++) { - barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; - barrier.oldLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + barrier.subresourceRange = layout.subresourceStates[si].subresourceRange; + barrier.oldLayout = layout.subresourceStates[si].newLayout; SanitiseOldImageLayout(barrier.oldLayout); @@ -1694,10 +1733,10 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten RDCASSERTEQUAL(vkr, VK_SUCCESS); } - for(size_t si = 0; si < m_ImageLayouts[id].subresourceStates.size(); si++) + for(size_t si = 0; si < layout.subresourceStates.size(); si++) { - barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; - barrier.newLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + barrier.subresourceRange = layout.subresourceStates[si].subresourceRange; + barrier.newLayout = layout.subresourceStates[si].newLayout; SanitiseNewImageLayout(barrier.newLayout); @@ -1761,7 +1800,7 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten 0, VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_GENERAL, - m_ImageLayouts[id].queueFamilyIndex, + layout.queueFamilyIndex, m_QueueFamilyIdx, ToHandle(live), {aspectFlags, 0, VK_REMAINING_MIP_LEVELS, 0, VK_REMAINING_ARRAY_LAYERS}, @@ -1781,10 +1820,10 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten RDCASSERTEQUAL(vkr, VK_SUCCESS); } - for(size_t si = 0; si < m_ImageLayouts[id].subresourceStates.size(); si++) + for(size_t si = 0; si < layout.subresourceStates.size(); si++) { - barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; - barrier.oldLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + barrier.subresourceRange = layout.subresourceStates[si].subresourceRange; + barrier.oldLayout = layout.subresourceStates[si].newLayout; SanitiseOldImageLayout(barrier.oldLayout); @@ -1830,10 +1869,10 @@ void WrappedVulkan::Apply_InitialState(WrappedVkRes *live, const VkInitialConten RDCASSERTEQUAL(vkr, VK_SUCCESS); } - for(size_t si = 0; si < m_ImageLayouts[id].subresourceStates.size(); si++) + for(size_t si = 0; si < layout.subresourceStates.size(); si++) { - barrier.subresourceRange = m_ImageLayouts[id].subresourceStates[si].subresourceRange; - barrier.newLayout = m_ImageLayouts[id].subresourceStates[si].newLayout; + barrier.subresourceRange = layout.subresourceStates[si].subresourceRange; + barrier.newLayout = layout.subresourceStates[si].newLayout; SanitiseNewImageLayout(barrier.newLayout); diff --git a/renderdoc/driver/vulkan/vk_manager.cpp b/renderdoc/driver/vulkan/vk_manager.cpp index b60bd6f09..1499fbd61 100644 --- a/renderdoc/driver/vulkan/vk_manager.cpp +++ b/renderdoc/driver/vulkan/vk_manager.cpp @@ -296,7 +296,7 @@ void VulkanResourceManager::SerialiseImageStates(SerialiserType &ser, auto stit = states.find(liveid); - if(stit == states.end() || stit->second.memoryBound) + if(stit == states.end() || stit->second.isMemoryBound) { barriers.push_back(t); vec.push_back(make_rdcpair(liveid, state)); @@ -339,7 +339,7 @@ void VulkanResourceManager::SerialiseImageStates(SerialiserType &ser, auto stit = states.find(liveid); - if(stit == states.end() || stit->second.memoryBound) + if(stit == states.end() || stit->second.isMemoryBound) { barriers.push_back(t); vec.push_back(make_rdcpair(liveid, state)); diff --git a/renderdoc/driver/vulkan/vk_rendertexture.cpp b/renderdoc/driver/vulkan/vk_rendertexture.cpp index 4f187cfa2..2b3364a10 100644 --- a/renderdoc/driver/vulkan/vk_rendertexture.cpp +++ b/renderdoc/driver/vulkan/vk_rendertexture.cpp @@ -166,7 +166,7 @@ bool VulkanReplay::RenderTextureInternal(TextureDisplay cfg, VkRenderPassBeginIn VkImage liveIm = m_pDriver->GetResourceManager()->GetCurrentHandle(cfg.resourceId); const ImageInfo &imageInfo = layouts.imageInfo; - if(!layouts.memoryBound) + if(!layouts.isMemoryBound) return false; CreateTexImageView(liveIm, iminfo, cfg.typeHint, texviews); diff --git a/renderdoc/driver/vulkan/vk_replay.cpp b/renderdoc/driver/vulkan/vk_replay.cpp index 4243c8117..d41a0bb37 100644 --- a/renderdoc/driver/vulkan/vk_replay.cpp +++ b/renderdoc/driver/vulkan/vk_replay.cpp @@ -2172,7 +2172,7 @@ bool VulkanReplay::GetMinMax(ResourceId texid, uint32_t sliceFace, uint32_t mip, TextureDisplayViews &texviews = m_TexRender.TextureViews[texid]; VkImage liveIm = m_pDriver->GetResourceManager()->GetCurrentHandle(texid); - if(!layouts.memoryBound) + if(!layouts.isMemoryBound) return false; if(!IsStencilFormat(iminfo.format)) @@ -2494,7 +2494,7 @@ bool VulkanReplay::GetHistogram(ResourceId texid, uint32_t sliceFace, uint32_t m TextureDisplayViews &texviews = m_TexRender.TextureViews[texid]; VkImage liveIm = m_pDriver->GetResourceManager()->GetCurrentHandle(texid); - if(!layouts.memoryBound) + if(!layouts.isMemoryBound) return false; bool stencil = false; @@ -2814,7 +2814,7 @@ void VulkanReplay::GetTextureData(ResourceId tex, uint32_t arrayIdx, uint32_t mi ImageLayouts &layouts = m_pDriver->m_ImageLayouts[tex]; - if(!layouts.memoryBound) + if(!layouts.isMemoryBound) return; VkImageCreateInfo imCreateInfo = { @@ -4254,4 +4254,4 @@ void Vulkan_ProcessStructured(RDCFile *rdc, SDFile &output) } static StructuredProcessRegistration VulkanProcessRegistration(RDCDriver::Vulkan, - &Vulkan_ProcessStructured); \ No newline at end of file + &Vulkan_ProcessStructured); diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index 48dee1f85..9cef261a3 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -1701,7 +1701,10 @@ struct ImageLayouts { uint32_t queueFamilyIndex = 0; std::vector subresourceStates; - bool memoryBound = false; + bool isMemoryBound = false; + ResourceId boundMemory = ResourceId(); + VkDeviceSize boundMemoryOffset = 0ull; + VkDeviceSize boundMemorySize = 0ull; VkImageLayout initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; ImageInfo imageInfo; }; diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 4c03f574b..7a93553b3 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -958,7 +958,11 @@ bool WrappedVulkan::Serialise_vkBindImageMemory(SerialiserType &ser, VkDevice de ObjDisp(device)->BindImageMemory(Unwrap(device), Unwrap(image), Unwrap(memory), memoryOffset); - m_ImageLayouts[GetResID(image)].memoryBound = true; + ImageLayouts &layout = m_ImageLayouts[GetResID(image)]; + layout.isMemoryBound = true; + layout.boundMemory = GetResID(memory); + layout.boundMemoryOffset = memoryOffset; + layout.boundMemorySize = mrq.size; GetReplay()->GetResourceDesc(memOrigId).derivedResources.push_back(resOrigId); GetReplay()->GetResourceDesc(resOrigId).parentResources.push_back(memOrigId); @@ -998,7 +1002,7 @@ VkResult WrappedVulkan::vkBindImageMemory(VkDevice device, VkImage image, VkDevi layout = &m_ImageLayouts[GetResID(image)]; } - layout->memoryBound = true; + layout->isMemoryBound = true; // memory object bindings are immutable and must happen before creation or use, // so this can always go into the record, even if a resource is created and bound @@ -1014,7 +1018,7 @@ VkResult WrappedVulkan::vkBindImageMemory(VkDevice device, VkImage image, VkDevi } else { - m_ImageLayouts[GetResID(image)].memoryBound = true; + m_ImageLayouts[GetResID(image)].isMemoryBound = true; } return ret; @@ -2032,7 +2036,11 @@ bool WrappedVulkan::Serialise_vkBindImageMemory2(SerialiserType &ser, VkDevice d if(!ok) return false; - m_ImageLayouts[GetResID(bindInfo.image)].memoryBound = true; + ImageLayouts &imageLayouts = m_ImageLayouts[GetResID(bindInfo.image)]; + imageLayouts.isMemoryBound = true; + imageLayouts.boundMemory = GetResID(bindInfo.memory); + imageLayouts.boundMemoryOffset = bindInfo.memoryOffset; + imageLayouts.boundMemorySize = mrq.size; GetReplay()->GetResourceDesc(memOrigId).derivedResources.push_back(resOrigId); GetReplay()->GetResourceDesc(resOrigId).parentResources.push_back(memOrigId); @@ -2081,7 +2089,7 @@ VkResult WrappedVulkan::vkBindImageMemory2(VkDevice device, uint32_t bindInfoCou layout = &m_ImageLayouts[imgrecord->GetResourceID()]; } - layout->memoryBound = true; + layout->isMemoryBound = true; // memory object bindings are immutable and must happen before creation or use, // so this can always go into the record, even if a resource is created and bound @@ -2099,7 +2107,7 @@ VkResult WrappedVulkan::vkBindImageMemory2(VkDevice device, uint32_t bindInfoCou else { for(uint32_t i = 0; i < bindInfoCount; i++) - m_ImageLayouts[GetResID(pBindInfos[i].image)].memoryBound = true; + m_ImageLayouts[GetResID(pBindInfos[i].image)].isMemoryBound = true; } return ret; diff --git a/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp index 50568bed9..0b08dafd6 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp @@ -453,7 +453,7 @@ bool WrappedVulkan::Serialise_vkCreateSwapchainKHR(SerialiserType &ser, VkDevice layouts.imageInfo = ImageInfo(swapinfo); - layouts.memoryBound = true; + layouts.isMemoryBound = true; layouts.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; layouts.subresourceStates.clear(); @@ -594,7 +594,7 @@ void WrappedVulkan::WrapAndProcessCreatedSwapchain(VkDevice device, layout = &m_ImageLayouts[imid]; } layout->imageInfo = GetRecord(images[i])->resInfo->imageInfo; - layout->memoryBound = true; + layout->isMemoryBound = true; layout->initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; layout->subresourceStates.clear();