From 090e4e5e9a27d5f5e187309acd91e849aa2a25f3 Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 31 Oct 2016 00:37:11 +0100 Subject: [PATCH] Handle stale descriptors hanging around referencing re-used resources --- renderdoc/driver/d3d12/d3d12_manager.cpp | 128 +++++++++++++++++++++ renderdoc/driver/d3d12/d3d12_resources.cpp | 2 + renderdoc/driver/d3d12/d3d12_resources.h | 6 +- renderdoc/driver/dxgi/dxgi_common.cpp | 8 +- 4 files changed, 136 insertions(+), 8 deletions(-) diff --git a/renderdoc/driver/d3d12/d3d12_manager.cpp b/renderdoc/driver/d3d12/d3d12_manager.cpp index 55c64d778..d82d9103d 100644 --- a/renderdoc/driver/d3d12/d3d12_manager.cpp +++ b/renderdoc/driver/d3d12/d3d12_manager.cpp @@ -23,6 +23,7 @@ ******************************************************************************/ #include "d3d12_manager.h" +#include "driver/dxgi/dxgi_common.h" #include "d3d12_command_list.h" #include "d3d12_command_queue.h" #include "d3d12_device.h" @@ -160,6 +161,52 @@ void D3D12Descriptor::Create(D3D12_DESCRIPTOR_HEAP_TYPE heapType, WrappedID3D12D } } + // it's possible to end up with invalid resource and descriptor combinations: + // 1. descriptor is created for ResID_1234 BC1_TYPELESS and a view BC1_UNORM + // 2. resource is freed. + // 3. some time later, new resource is created ResID_5678 BC3_UNORM + // 4. Key point is - descriptor has a pointer to the resource, and the slot is + // re-allocated in 3. + // 5. We now have a descriptor that is BC3_UNORM resource and BC1_UNORM view. + // + // This is unavoidable without recording back-references from resources to the + // descriptors that use them. Instead, we just detect the invalid case here + // and since we know the descriptor is unused (since it's invalid to use it + // after the resource is freed, and it would have to be recreated with a valid + // format combination) we can just force a null resource. + // + // so need to check if + // a) we have a non-NULL resource (otherwise any descriptor is fine) + // b) descriptor and resource have a non-UNKNOWN format (buffers have UNKNOWN + // type which can be cast arbitrarily by the view). + // c) when the resource is typed, the view must be identical, when it's typeless + // the view format must be castable + if(nonsamp.resource && desc) + { + DXGI_FORMAT resFormat = nonsamp.resource->GetDesc().Format; + DXGI_FORMAT viewFormat = desc->Format; + + if(resFormat != DXGI_FORMAT_UNKNOWN && viewFormat != DXGI_FORMAT_UNKNOWN) + { + if(!IsTypelessFormat(resFormat)) + { + if(resFormat != viewFormat) + { + nonsamp.resource = NULL; + desc = defaultSRV(); + } + } + else + { + if(resFormat != GetTypelessFormat(viewFormat)) + { + nonsamp.resource = NULL; + desc = defaultSRV(); + } + } + } + } + dev->CreateShaderResourceView(nonsamp.resource, desc, handle); break; } @@ -181,6 +228,33 @@ void D3D12Descriptor::Create(D3D12_DESCRIPTOR_HEAP_TYPE heapType, WrappedID3D12D } } + // see comment above in SRV case for what this code is doing + if(nonsamp.resource && desc) + { + DXGI_FORMAT resFormat = nonsamp.resource->GetDesc().Format; + DXGI_FORMAT viewFormat = desc->Format; + + if(resFormat != DXGI_FORMAT_UNKNOWN && viewFormat != DXGI_FORMAT_UNKNOWN) + { + if(!IsTypelessFormat(resFormat)) + { + if(resFormat != viewFormat) + { + nonsamp.resource = NULL; + desc = defaultRTV(); + } + } + else + { + if(resFormat != GetTypelessFormat(viewFormat)) + { + nonsamp.resource = NULL; + desc = defaultRTV(); + } + } + } + } + dev->CreateRenderTargetView(nonsamp.resource, desc, handle); break; } @@ -190,6 +264,33 @@ void D3D12Descriptor::Create(D3D12_DESCRIPTOR_HEAP_TYPE heapType, WrappedID3D12D if(desc->ViewDimension == D3D12_DSV_DIMENSION_UNKNOWN) desc = nonsamp.resource ? NULL : defaultDSV(); + // see comment above in SRV case for what this code is doing + if(nonsamp.resource && desc) + { + DXGI_FORMAT resFormat = nonsamp.resource->GetDesc().Format; + DXGI_FORMAT viewFormat = desc->Format; + + if(resFormat != DXGI_FORMAT_UNKNOWN && viewFormat != DXGI_FORMAT_UNKNOWN) + { + if(!IsTypelessFormat(resFormat)) + { + if(resFormat != viewFormat) + { + nonsamp.resource = NULL; + desc = defaultDSV(); + } + } + else + { + if(resFormat != GetTypelessFormat(viewFormat)) + { + nonsamp.resource = NULL; + desc = defaultDSV(); + } + } + } + } + dev->CreateDepthStencilView(nonsamp.resource, desc, handle); break; } @@ -223,6 +324,33 @@ void D3D12Descriptor::Create(D3D12_DESCRIPTOR_HEAP_TYPE heapType, WrappedID3D12D if(counter == NULL && desc && desc->ViewDimension == D3D12_UAV_DIMENSION_BUFFER) desc->Buffer.CounterOffsetInBytes = 0; + // see comment above in SRV case for what this code is doing + if(nonsamp.resource && desc) + { + DXGI_FORMAT resFormat = nonsamp.resource->GetDesc().Format; + DXGI_FORMAT viewFormat = desc->Format; + + if(resFormat != DXGI_FORMAT_UNKNOWN && viewFormat != DXGI_FORMAT_UNKNOWN) + { + if(!IsTypelessFormat(resFormat)) + { + if(resFormat != viewFormat) + { + nonsamp.resource = NULL; + desc = defaultUAV(); + } + } + else + { + if(resFormat != GetTypelessFormat(viewFormat)) + { + nonsamp.resource = NULL; + desc = defaultUAV(); + } + } + } + } + dev->CreateUnorderedAccessView(nonsamp.resource, counter, desc, handle); break; } diff --git a/renderdoc/driver/d3d12/d3d12_resources.cpp b/renderdoc/driver/d3d12/d3d12_resources.cpp index 528d10f5d..012475263 100644 --- a/renderdoc/driver/d3d12/d3d12_resources.cpp +++ b/renderdoc/driver/d3d12/d3d12_resources.cpp @@ -285,6 +285,8 @@ WrappedID3D12Resource::~WrappedID3D12Resource() m_Addresses.RemoveFrom(m_pReal->GetGPUVirtualAddress()); Shutdown(); + + m_ID = ResourceId(); } byte *WrappedID3D12Resource::GetMap(UINT Subresource) diff --git a/renderdoc/driver/d3d12/d3d12_resources.h b/renderdoc/driver/d3d12/d3d12_resources.h index 9d7bdfb1e..15e23ce15 100644 --- a/renderdoc/driver/d3d12/d3d12_resources.h +++ b/renderdoc/driver/d3d12/d3d12_resources.h @@ -39,7 +39,7 @@ public: ResourceId GetResourceID() { return m_ID; } D3D12ResourceRecord *GetResourceRecord() { return m_pRecord; } void SetResourceRecord(D3D12ResourceRecord *record) { m_pRecord = record; } -private: +protected: TrackedResource12(const TrackedResource12 &); TrackedResource12 &operator=(const TrackedResource12 &); @@ -677,7 +677,9 @@ class WrappedID3D12Resource : public WrappedDeviceChild12 bool resident; public: - ALLOCATE_WITH_WRAPPED_POOL(WrappedID3D12Resource); + static const int AllocPoolCount = 16384; + static const int AllocMaxByteSize = 1024 * 1024; + ALLOCATE_WITH_WRAPPED_POOL(WrappedID3D12Resource, AllocPoolCount, AllocMaxByteSize, false); static std::map *m_List; diff --git a/renderdoc/driver/dxgi/dxgi_common.cpp b/renderdoc/driver/dxgi/dxgi_common.cpp index 8fa57f9c5..9a6853a26 100644 --- a/renderdoc/driver/dxgi/dxgi_common.cpp +++ b/renderdoc/driver/dxgi/dxgi_common.cpp @@ -1225,13 +1225,9 @@ DXGI_FORMAT GetTypelessFormat(DXGI_FORMAT f) case DXGI_FORMAT_P208: case DXGI_FORMAT_V208: case DXGI_FORMAT_V408: - case DXGI_FORMAT_B4G4R4A4_UNORM: - RDCERR("No Typeless DXGI Format for %d", f); - return DXGI_FORMAT_UNKNOWN; + case DXGI_FORMAT_B4G4R4A4_UNORM: return f; - case DXGI_FORMAT_UNKNOWN: - RDCWARN("Getting Typeless format of DXGI_FORMAT_UNKNOWN"); - return DXGI_FORMAT_UNKNOWN; + case DXGI_FORMAT_UNKNOWN: return DXGI_FORMAT_UNKNOWN; default: RDCERR("Unrecognised DXGI Format: %d", f); return DXGI_FORMAT_UNKNOWN; }