From 83ac6d369718c2e04e765055a4d2ba2700768a96 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 13 Nov 2024 17:23:34 +0000 Subject: [PATCH] Ensure resources created mid-frame are properly forced referenced --- .../driver/d3d12/d3d12_command_list4_wrap.cpp | 3 +++ renderdoc/driver/d3d12/d3d12_device.cpp | 15 +++++++++++++++ renderdoc/driver/d3d12/d3d12_device.h | 6 +----- renderdoc/driver/vulkan/vk_core.cpp | 15 +++++++++++++++ renderdoc/driver/vulkan/vk_core.h | 6 +----- .../driver/vulkan/wrappers/vk_resource_funcs.cpp | 13 +++++++------ 6 files changed, 42 insertions(+), 16 deletions(-) diff --git a/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp b/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp index cdaa24b07..652cd7339 100644 --- a/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp +++ b/renderdoc/driver/d3d12/d3d12_command_list4_wrap.cpp @@ -783,6 +783,9 @@ bool WrappedID3D12GraphicsCommandList::ProcessASBuildAfterSubmission(ResourceId m_pDevice->CreateAS(dstASB, destASBOffset, byteSize, accStructAtDestOffset); m_pDevice->AddForcedReference(record); + // in case we're currently capturing, immediately consider the AS as referenced + GetResourceManager()->MarkResourceFrameReferenced(accStructAtDestOffset->GetResourceID(), + eFrameRef_Read); } else { diff --git a/renderdoc/driver/d3d12/d3d12_device.cpp b/renderdoc/driver/d3d12/d3d12_device.cpp index 6dd1dc985..ac8b0c49f 100644 --- a/renderdoc/driver/d3d12/d3d12_device.cpp +++ b/renderdoc/driver/d3d12/d3d12_device.cpp @@ -3331,6 +3331,21 @@ void WrappedID3D12Device::UploadBLASBufferAddresses() m_addressBufferUploaded = true; } +void WrappedID3D12Device::AddForcedReference(D3D12ResourceRecord *record) +{ + { + SCOPED_LOCK(m_ForcedReferencesLock); + m_ForcedReferences.push_back(record); + } + + // in case we're currently capturing, immediately consider the resource as referenced. If we're + // not capturing this will naturally be cleared before the frame capture starts and we don't have + // to consider races as this is internally locked. If we're racing with a frame capture starting + // we will either add this redundantly (after clear but before forced references are added) or as + // required (after references are cleared and after forced references are added) + GetResourceManager()->MarkResourceFrameReferenced(record->GetResourceID(), eFrameRef_Read); +} + void WrappedID3D12Device::ReleaseResource(ID3D12DeviceChild *res) { ResourceId id = GetResID(res); diff --git a/renderdoc/driver/d3d12/d3d12_device.h b/renderdoc/driver/d3d12/d3d12_device.h index a794e5181..4af5e36fc 100644 --- a/renderdoc/driver/d3d12/d3d12_device.h +++ b/renderdoc/driver/d3d12/d3d12_device.h @@ -888,11 +888,7 @@ public: const D3D12_FEATURE_DATA_D3D12_OPTIONS16 &GetOpts16() { return m_D3D12Opts16; } void RemoveQueue(WrappedID3D12CommandQueue *queue); - void AddForcedReference(D3D12ResourceRecord *record) - { - SCOPED_LOCK(m_ForcedReferencesLock); - m_ForcedReferences.push_back(record); - } + void AddForcedReference(D3D12ResourceRecord *record); // only valid on replay const std::map &GetResourceList() { return *m_ResourceList; } diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index f24f5eeec..dd42a220f 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -5349,6 +5349,21 @@ ResourceId WrappedVulkan::GetPartialCommandBuffer() return m_Partial.partialStack.back().cmdId; } +void WrappedVulkan::AddForcedReference(VkResourceRecord *record) +{ + { + SCOPED_LOCK(m_ForcedReferencesLock); + m_ForcedReferences.push_back(record); + } + + // in case we're currently capturing, immediately consider the resource as referenced. If we're + // not capturing this will naturally be cleared before the frame capture starts and we don't have + // to consider races as this is internally locked. If we're racing with a frame capture starting + // we will either add this redundantly (after clear but before forced references are added) or as + // required (after references are cleared and after forced references are added) + GetResourceManager()->MarkResourceFrameReferenced(record->GetResourceID(), eFrameRef_Read); +} + void WrappedVulkan::AddAction(const ActionDescription &a) { m_AddedAction = true; diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index a72ea88a6..df1eb27fe 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -952,11 +952,7 @@ private: return ret; } - void AddForcedReference(VkResourceRecord *record) - { - SCOPED_LOCK(m_ForcedReferencesLock); - m_ForcedReferences.push_back(record); - } + void AddForcedReference(VkResourceRecord *record); // used on replay side to track the queue family of command buffers and pools std::map m_commandQueueFamilies; diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 3a808940d..ded306724 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -1510,9 +1510,8 @@ VkResult WrappedVulkan::vkBindBufferMemory(VkDevice device, VkBuffer buffer, VkD // if the buffer was force-referenced, do the same with the memory if(IsForcedReference(record)) { - // in case we're currently capturing, immediately consider the buffer and backing memory as - // read-before-write referenced - GetResourceManager()->MarkResourceFrameReferenced(record->GetResourceID(), eFrameRef_Read); + // AddForcedReference will also call MarkResourceFrameReferenced() on the buffer in case + // we're currently capturing, do the same with the memory with the correct semantics. GetResourceManager()->MarkMemoryFrameReferenced(id, memoryOffset, record->memSize, eFrameRef_ReadBeforeWrite); @@ -3020,9 +3019,8 @@ VkResult WrappedVulkan::vkBindBufferMemory2(VkDevice device, uint32_t bindInfoCo // if the buffer was force-referenced, do the same with the memory if(IsForcedReference(bufrecord)) { - // in case we're currently capturing, immediately consider the buffer and backing memory as - // read-before-write referenced - GetResourceManager()->MarkResourceFrameReferenced(bufrecord->GetResourceID(), eFrameRef_Read); + // AddForcedReference will also call MarkResourceFrameReferenced() on the buffer in case + // we're currently capturing, do the same with the memory with the correct semantics. GetResourceManager()->MarkMemoryFrameReferenced( GetResID(pBindInfos[i].memory), pBindInfos[i].memoryOffset, bufrecord->memSize, eFrameRef_ReadBeforeWrite); @@ -3409,6 +3407,9 @@ VkResult WrappedVulkan::vkCreateAccelerationStructureKHR( // reference them. We force ref generics too as they could bottom or top level so we // conservatively assume they are bottom AddForcedReference(record); + + // in case we're currently capturing, immediately consider the AS as referenced + GetResourceManager()->MarkResourceFrameReferenced(record->GetResourceID(), eFrameRef_Read); } } else