From 4d26628a3c1e51543267bdfd3db46037a0e252d3 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 12 Jan 2022 11:21:15 +0000 Subject: [PATCH] Don't store cap-referenced desc sets as objects directly * When we see a descriptor copy during a frame capture we force the source descriptor set to be included since it may never be bound to a command buffer, but it may also be deleted so instead we AddRef the record to keep it alive and store the id & record directly. --- renderdoc/driver/vulkan/vk_core.cpp | 2 ++ renderdoc/driver/vulkan/vk_core.h | 2 +- renderdoc/driver/vulkan/vk_resources.h | 2 +- renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp | 4 +++- .../vulkan/wrappers/vk_descriptor_funcs.cpp | 6 +++++- .../driver/vulkan/wrappers/vk_queue_funcs.cpp | 15 +++++++++++---- 6 files changed, 23 insertions(+), 8 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index 00db7f2a7..fd606b41b 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -1827,6 +1827,8 @@ void WrappedVulkan::StartFrameCapture(void *dev, void *wnd) { SCOPED_LOCK(m_CapDescriptorsLock); + for(const rdcpair &it : m_CapDescriptors) + it.second->Delete(GetResourceManager()); m_CapDescriptors.clear(); } diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index d8f953cc5..2295bf50c 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -328,7 +328,7 @@ private: std::set m_StringDB; Threading::CriticalSection m_CapDescriptorsLock; - std::set m_CapDescriptors; + std::set> m_CapDescriptors; VkResourceRecord *m_FrameCaptureRecord; Chunk *m_HeaderChunk; diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index 799b5f1e3..ec3e309ff 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -1071,7 +1071,7 @@ struct CmdBufferRecordingInfo // a list of descriptor sets that are bound at any point in this command buffer // used to look up all the frame refs per-desc set and apply them on queue // submit with latest binding refs. - std::set boundDescSets; + std::set> boundDescSets; // barriers to apply when the current render pass ends. Calculated at begin time in case the // framebuffer is imageless and we need to use the image views passed in at begin time to diff --git a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp index d2d5a9388..199dafacd 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp @@ -3230,7 +3230,9 @@ void WrappedVulkan::vkCmdBindDescriptorSets(VkCommandBuffer commandBuffer, record->AddChunk(scope.Get(&record->cmdInfo->alloc)); record->MarkResourceFrameReferenced(GetResID(layout), eFrameRef_Read); - record->cmdInfo->boundDescSets.insert(pDescriptorSets, pDescriptorSets + setCount); + for(uint32_t i = 0; i < setCount; i++) + record->cmdInfo->boundDescSets.insert( + {GetResID(pDescriptorSets[i]), GetRecord(pDescriptorSets[i])}); } } diff --git a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp index eb3d67988..36a98aab0 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp @@ -1222,9 +1222,13 @@ void WrappedVulkan::vkUpdateDescriptorSets(VkDevice device, uint32_t writeCount, GetResourceManager()->MarkResourceFrameReferenced(GetResID(pDescriptorCopies[i].srcSet), eFrameRef_Read); + ResourceId id = GetResID(pDescriptorCopies[i].srcSet); + VkResourceRecord *record = GetRecord(pDescriptorCopies[i].srcSet); + { SCOPED_LOCK(m_CapDescriptorsLock); - m_CapDescriptors.insert(pDescriptorCopies[i].srcSet); + record->AddRef(); + m_CapDescriptors.insert({id, record}); } } } diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index 6d9730e09..cc9eb5507 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -827,15 +827,18 @@ void WrappedVulkan::CaptureQueueSubmit(VkQueue queue, std::unordered_set refdIDs; - std::set descriptorSets; + std::set> capDescriptors; + std::set> descriptorSets; // pull in any copy sources, conservatively if(capframe) { SCOPED_LOCK(m_CapDescriptorsLock); - descriptorSets.swap(m_CapDescriptors); + capDescriptors.swap(m_CapDescriptors); } + descriptorSets = capDescriptors; + for(size_t i = 0; i < commandBuffers.size(); i++) { ResourceId cmd = GetResID(commandBuffers[i]); @@ -941,9 +944,9 @@ void WrappedVulkan::CaptureQueueSubmit(VkQueue queue, // for each descriptor set, mark it referenced as well as all resources currently bound to it for(auto it = descriptorSets.begin(); it != descriptorSets.end(); ++it) { - rm->MarkResourceFrameReferenced(GetResID(*it), eFrameRef_Read); + rm->MarkResourceFrameReferenced(it->first, eFrameRef_Read); - VkResourceRecord *setrecord = GetRecord(*it); + VkResourceRecord *setrecord = it->second; DescriptorBindRefs refs; @@ -1159,6 +1162,10 @@ void WrappedVulkan::CaptureQueueSubmit(VkQueue queue, } } } + + for(const rdcpair &it : capDescriptors) + it.second->Delete(GetResourceManager()); + capDescriptors.clear(); } template