From 06b8dd9051628258e20810bedd875d208cdacee1 Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 9 Sep 2025 13:51:15 +0100 Subject: [PATCH] Extend lifetime of images and image views similar to buffers * With descriptor buffers we need to ensure images and image views also do not get destroyed mid capture, similar to what we do for buffers and memory. The same reasoning applies - we don't want a single BDA to refer to two different buffers, and we also don't want a single descriptor to alias multiple resources either. --- renderdoc/driver/vulkan/vk_core.cpp | 35 +++++++++++++++++++ renderdoc/driver/vulkan/vk_core.h | 4 +++ .../driver/vulkan/wrappers/vk_misc_funcs.cpp | 33 +++++++++++++++-- 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index d1246d1cd..ba7b8fef4 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -2820,6 +2820,8 @@ bool WrappedVulkan::EndFrameCapture(DeviceOwnedWindow devWnd) rdcarray DeadMemories; rdcarray DeadBuffers; + rdcarray DeadImages; + rdcarray DeadImageViews; // transition back to IDLE atomically { @@ -2846,6 +2848,8 @@ bool WrappedVulkan::EndFrameCapture(DeviceOwnedWindow devWnd) SCOPED_LOCK(m_DeviceAddressResourcesLock); DeadMemories.swap(m_DeviceAddressResources.DeadMemories); DeadBuffers.swap(m_DeviceAddressResources.DeadBuffers); + DeadImages.swap(m_DeviceAddressResources.DeadImages); + DeadImageViews.swap(m_DeviceAddressResources.DeadImageViews); } } @@ -2855,6 +2859,12 @@ bool WrappedVulkan::EndFrameCapture(DeviceOwnedWindow devWnd) for(VkBuffer b : DeadBuffers) vkDestroyBuffer(m_Device, b, NULL); + for(VkImage i : DeadImages) + vkDestroyImage(m_Device, i, NULL); + + for(VkImageView v : DeadImageViews) + vkDestroyImageView(m_Device, v, NULL); + // gather backbuffer screenshot const uint32_t maxSize = 2048; RenderDoc::FramePixels fp; @@ -3224,6 +3234,11 @@ bool WrappedVulkan::DiscardFrameCapture(DeviceOwnedWindow devWnd) m_CapturedFrames.pop_back(); + rdcarray DeadMemories; + rdcarray DeadBuffers; + rdcarray DeadImages; + rdcarray DeadImageViews; + // transition back to IDLE atomically { SCOPED_WRITELOCK(m_CapTransitionLock); @@ -3243,8 +3258,28 @@ bool WrappedVulkan::DiscardFrameCapture(DeviceOwnedWindow devWnd) (*it)->memMapState->needRefData = false; } } + + { + SCOPED_LOCK(m_DeviceAddressResourcesLock); + DeadMemories.swap(m_DeviceAddressResources.DeadMemories); + DeadBuffers.swap(m_DeviceAddressResources.DeadBuffers); + DeadImages.swap(m_DeviceAddressResources.DeadImages); + DeadImageViews.swap(m_DeviceAddressResources.DeadImageViews); + } } + for(VkDeviceMemory m : DeadMemories) + vkFreeMemory(m_Device, m, NULL); + + for(VkBuffer b : DeadBuffers) + vkDestroyBuffer(m_Device, b, NULL); + + for(VkImage i : DeadImages) + vkDestroyImage(m_Device, i, NULL); + + for(VkImageView v : DeadImageViews) + vkDestroyImageView(m_Device, v, NULL); + Atomic::Inc32(&m_ReuseEnabled); // delete cmd buffers now - had to keep them alive until after serialiser flush. diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index 855085801..5938f12e7 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -1031,6 +1031,10 @@ private: rdcarray DeadMemories; rdcarray DeadBuffers; rdcarray IDs; + + // with descriptor buffers, we also need to hold onto images and image views + rdcarray DeadImages; + rdcarray DeadImageViews; } m_DeviceAddressResources; Threading::CriticalSection m_DeviceAddressResourcesLock; diff --git a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp index 959a195eb..36754c196 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp @@ -194,6 +194,20 @@ void WrappedVulkan::vkDestroyImageView(VkDevice device, VkImageView obj, const V { if(obj == VK_NULL_HANDLE) return; + + // with descriptor buffers, extend the lifespan of image views to ensure descriptors don't falsely + // alias + if(DescriptorBuffers()) + { + SCOPED_READLOCK(m_CapTransitionLock); + SCOPED_LOCK(m_DeviceAddressResourcesLock); + if(IsActiveCapturing(m_State)) + { + m_DeviceAddressResources.DeadImageViews.push_back(obj); + return; + } + } + VkImageView unwrappedObj = Unwrap(obj); { SCOPED_LOCK(m_ForcedReferencesLock); @@ -289,9 +303,6 @@ void WrappedVulkan::vkDestroyBuffer(VkDevice device, VkBuffer buffer, const VkAl if(buffer == VK_NULL_HANDLE) return; - if(IsCaptureMode(m_State)) - UntrackBufferAddress(device, buffer); - // artificially extend the lifespan of buffer device address memory or buffers, to ensure their // opaque capture address isn't re-used before the capture completes { @@ -305,6 +316,9 @@ void WrappedVulkan::vkDestroyBuffer(VkDevice device, VkBuffer buffer, const VkAl m_DeviceAddressResources.IDs.removeOne(GetResID(buffer)); } + if(IsCaptureMode(m_State)) + UntrackBufferAddress(device, buffer); + VkBuffer unwrappedObj = Unwrap(buffer); if(IsCaptureMode(m_State)) @@ -387,6 +401,19 @@ void WrappedVulkan::vkDestroyImage(VkDevice device, VkImage obj, const VkAllocat if(obj == VK_NULL_HANDLE) return; + // with descriptor buffers, extend the lifespan of images to ensure descriptors don't falsely + // alias + if(DescriptorBuffers()) + { + SCOPED_READLOCK(m_CapTransitionLock); + SCOPED_LOCK(m_DeviceAddressResourcesLock); + if(IsActiveCapturing(m_State)) + { + m_DeviceAddressResources.DeadImages.push_back(obj); + return; + } + } + { SCOPED_LOCK(m_ForcedReferencesLock); m_ForcedReferences.removeOne(GetRecord(obj));