diff --git a/renderdoc/driver/d3d12/d3d12_device.cpp b/renderdoc/driver/d3d12/d3d12_device.cpp index 9531b32fe..c791113c8 100644 --- a/renderdoc/driver/d3d12/d3d12_device.cpp +++ b/renderdoc/driver/d3d12/d3d12_device.cpp @@ -1829,8 +1829,8 @@ void WrappedID3D12Device::StartFrameCapture(void *dev, void *wnd) eFrameRef_Read); } - // also keep buffers alive - WrappedID3D12Resource1::AddRefBuffersBeforeCapture(GetResourceManager()); + m_RefQueues = m_Queues; + m_RefBuffers = WrappedID3D12Resource1::AddRefBuffersBeforeCapture(GetResourceManager()); } GetResourceManager()->MarkResourceFrameReferenced(m_ResourceID, eFrameRef_Read); @@ -2136,14 +2136,14 @@ bool WrappedID3D12Device::EndFrameCapture(void *dev, void *wnd) SAFE_DELETE(m_HeaderChunk); for(auto it = queues.begin(); it != queues.end(); ++it) - { (*it)->ClearAfterCapture(); - // remove the reference held during capture, potentially releasing the queue. - (*it)->Release(); - } + // remove the references held during capture, potentially releasing the queue/buffer. + for(WrappedID3D12CommandQueue *q : m_RefQueues) + q->Release(); - WrappedID3D12Resource1::ReleaseBuffersAfterCapture(GetResourceManager()); + for(ID3D12Resource *r : m_RefBuffers) + r->Release(); GetResourceManager()->MarkUnwrittenResources(); @@ -2189,6 +2189,13 @@ bool WrappedID3D12Device::DiscardFrameCapture(void *dev, void *wnd) for(auto it = queues.begin(); it != queues.end(); ++it) (*it)->ClearAfterCapture(); + // remove the reference held during capture, potentially releasing the queue. + for(WrappedID3D12CommandQueue *q : m_RefQueues) + q->Release(); + + for(ID3D12Resource *r : m_RefBuffers) + r->Release(); + GetResourceManager()->MarkUnwrittenResources(); GetResourceManager()->ClearReferencedResources(); diff --git a/renderdoc/driver/d3d12/d3d12_device.h b/renderdoc/driver/d3d12/d3d12_device.h index 8a23b6099..9c54bcfeb 100644 --- a/renderdoc/driver/d3d12/d3d12_device.h +++ b/renderdoc/driver/d3d12/d3d12_device.h @@ -361,6 +361,13 @@ private: rdcarray m_Queues; rdcarray m_QueueFences; + // list of queues and buffers kept alive during capture artificially even if the user destroys + // them, so we can use them in the capture. Storing this separately prevents races where a + // queue/buffer is added between us transitioning away from active capturing (so we don't addref + // it) and us releasing our reference on them. + rdcarray m_RefQueues; + rdcarray m_RefBuffers; + // the queue we use for all internal work, the first DIRECT queue WrappedID3D12CommandQueue *m_Queue; diff --git a/renderdoc/driver/d3d12/d3d12_device_wrap.cpp b/renderdoc/driver/d3d12/d3d12_device_wrap.cpp index 981bffe7b..31057606b 100644 --- a/renderdoc/driver/d3d12/d3d12_device_wrap.cpp +++ b/renderdoc/driver/d3d12/d3d12_device_wrap.cpp @@ -136,15 +136,20 @@ HRESULT WrappedID3D12Device::CreateCommandQueue(const D3D12_COMMAND_QUEUE_DESC * { SCOPED_READLOCK(m_CapTransitionLock); capframe = IsActiveCapturing(m_State); + + // while capturing don't allow any queues to be freed, by adding another refcount, since we + // gather any commands submitted to them at the end of the capture. + if(capframe) + { + wrapped->AddRef(); + m_RefQueues.push_back(wrapped); + } } - // while capturing don't allow any queues to be freed, by adding another refcount, since we - // gather any commands submitted to them at the end of the capture. if(capframe) { GetResourceManager()->MarkResourceFrameReferenced( wrapped->GetCreationRecord()->GetResourceID(), eFrameRef_Read); - wrapped->AddRef(); } *ppCommandQueue = (ID3D12CommandQueue *)wrapped; @@ -1475,6 +1480,18 @@ HRESULT WrappedID3D12Device::CreateCommittedResource(const D3D12_HEAP_PROPERTIES } *ppvResource = (ID3D12Resource *)wrapped; + + // while actively capturing we keep all buffers around to prevent the address lookup from + // losing addresses we might need (or the manageable but annoying problem of an address being + // re-used) + { + SCOPED_READLOCK(m_CapTransitionLock); + if(IsActiveCapturing(m_State)) + { + wrapped->AddRef(); + m_RefBuffers.push_back(wrapped); + } + } } return ret; @@ -1750,6 +1767,18 @@ HRESULT WrappedID3D12Device::CreatePlacedResource(ID3D12Heap *pHeap, UINT64 Heap } *ppvResource = (ID3D12Resource *)wrapped; + + // while actively capturing we keep all buffers around to prevent the address lookup from + // losing addresses we might need (or the manageable but annoying problem of an address being + // re-used) + { + SCOPED_READLOCK(m_CapTransitionLock); + if(IsActiveCapturing(m_State)) + { + wrapped->AddRef(); + m_RefBuffers.push_back(wrapped); + } + } } return ret; @@ -2574,6 +2603,18 @@ HRESULT WrappedID3D12Device::OpenSharedHandleInternal(D3D12Chunk chunkType, REFI states.fill(GetNumSubresources(m_pDevice, &desc), InitialResourceState); } + + // while actively capturing we keep all buffers around to prevent the address lookup from + // losing addresses we might need (or the manageable but annoying problem of an address being + // re-used) + { + SCOPED_READLOCK(m_CapTransitionLock); + if(IsActiveCapturing(m_State)) + { + wrapped->AddRef(); + m_RefBuffers.push_back(wrapped); + } + } } CACHE_THREAD_SERIALISER(); diff --git a/renderdoc/driver/d3d12/d3d12_device_wrap4.cpp b/renderdoc/driver/d3d12/d3d12_device_wrap4.cpp index b3f8c6839..95e54619c 100644 --- a/renderdoc/driver/d3d12/d3d12_device_wrap4.cpp +++ b/renderdoc/driver/d3d12/d3d12_device_wrap4.cpp @@ -379,6 +379,18 @@ HRESULT WrappedID3D12Device::CreateCommittedResource1( } *ppvResource = (ID3D12Resource *)wrapped; + + // while actively capturing we keep all buffers around to prevent the address lookup from + // losing addresses we might need (or the manageable but annoying problem of an address being + // re-used) + { + SCOPED_READLOCK(m_CapTransitionLock); + if(IsActiveCapturing(m_State)) + { + wrapped->AddRef(); + m_RefBuffers.push_back(wrapped); + } + } } return ret; diff --git a/renderdoc/driver/d3d12/d3d12_resources.cpp b/renderdoc/driver/d3d12/d3d12_resources.cpp index 0a231eb96..f05135766 100644 --- a/renderdoc/driver/d3d12/d3d12_resources.cpp +++ b/renderdoc/driver/d3d12/d3d12_resources.cpp @@ -463,26 +463,24 @@ void WrappedID3D12Resource1::RefBuffers(D3D12ResourceManager *rm) rm->MarkResourceFrameReferenced(m_Addresses.addresses[i].id, eFrameRef_Read); } -void WrappedID3D12Resource1::AddRefBuffersBeforeCapture(D3D12ResourceManager *rm) +rdcarray WrappedID3D12Resource1::AddRefBuffersBeforeCapture(D3D12ResourceManager *rm) { - SCOPED_READLOCK(m_Addresses.addressLock); - for(size_t i = 0; i < m_Addresses.addresses.size(); i++) - rm->GetCurrentResource(m_Addresses.addresses[i].id)->AddRef(); -} + rdcarray ret; -void WrappedID3D12Resource1::ReleaseBuffersAfterCapture(D3D12ResourceManager *rm) -{ - // make a copy because we might release the last reference on a buffer which will need to modify - // the actual m_Addresses rdcarray addresses; - { SCOPED_READLOCK(m_Addresses.addressLock); addresses = m_Addresses.addresses; } for(size_t i = 0; i < addresses.size(); i++) - rm->GetCurrentResource(addresses[i].id)->Release(); + { + ID3D12Resource *resource = (ID3D12Resource *)rm->GetCurrentResource(m_Addresses.addresses[i].id); + resource->AddRef(); + ret.push_back(resource); + } + + return ret; } WrappedID3D12DescriptorHeap::WrappedID3D12DescriptorHeap(ID3D12DescriptorHeap *real, diff --git a/renderdoc/driver/d3d12/d3d12_resources.h b/renderdoc/driver/d3d12/d3d12_resources.h index 43396b12e..4edc0addc 100644 --- a/renderdoc/driver/d3d12/d3d12_resources.h +++ b/renderdoc/driver/d3d12/d3d12_resources.h @@ -879,8 +879,7 @@ public: static void RefBuffers(D3D12ResourceManager *rm); - static void AddRefBuffersBeforeCapture(D3D12ResourceManager *rm); - static void ReleaseBuffersAfterCapture(D3D12ResourceManager *rm); + static rdcarray AddRefBuffersBeforeCapture(D3D12ResourceManager *rm); static void GetResIDFromAddr(D3D12_GPU_VIRTUAL_ADDRESS addr, ResourceId &id, UINT64 &offs) { @@ -924,12 +923,6 @@ public: range.id = GetResourceID(); m_Addresses.AddTo(range); - - // while actively capturing we keep all buffers around to prevent the address lookup from - // losing addresses we might need (or the manageable but annoying problem of an address being - // re-used) - if(IsActiveCapturing(device->GetState())) - AddRef(); } } virtual ~WrappedID3D12Resource1();