From cf0ac7570d0ffcca02f0b14a5d4546ef78bc954c Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Fri, 2 Mar 2018 11:07:07 -0500 Subject: [PATCH] D3D12: Keep a reference to pipelines until unused Previously we would remove the reference to pipelines in the destructor of the d3d12::*Pipeline objects which could cause the D3D12 pipeline state to be destroyed while still used by in-flight commands. Add a global queue of ComPtrs to keep alive in the d3d12::Device to fix this. --- src/backend/d3d12/ComputePipelineD3D12.cpp | 6 +++++- src/backend/d3d12/ComputePipelineD3D12.h | 4 ++++ src/backend/d3d12/D3D12Backend.cpp | 7 +++++++ src/backend/d3d12/D3D12Backend.h | 6 +++++- src/backend/d3d12/RenderPipelineD3D12.cpp | 7 ++++++- src/backend/d3d12/RenderPipelineD3D12.h | 5 +++++ 6 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/backend/d3d12/ComputePipelineD3D12.cpp b/src/backend/d3d12/ComputePipelineD3D12.cpp index f8822d674b..971da4cf47 100644 --- a/src/backend/d3d12/ComputePipelineD3D12.cpp +++ b/src/backend/d3d12/ComputePipelineD3D12.cpp @@ -24,7 +24,7 @@ namespace backend { namespace d3d12 { ComputePipeline::ComputePipeline(ComputePipelineBuilder* builder) - : ComputePipelineBase(builder) { + : ComputePipelineBase(builder), mDevice(ToBackend(builder->GetDevice())) { uint32_t compileFlags = 0; #if defined(_DEBUG) // Enable better shader debugging with the graphics debugging tools. @@ -57,6 +57,10 @@ namespace backend { namespace d3d12 { IID_PPV_ARGS(&mPipelineState)); } + ComputePipeline::~ComputePipeline() { + mDevice->ReferenceUntilUnused(mPipelineState); + } + ComPtr ComputePipeline::GetPipelineState() { return mPipelineState; } diff --git a/src/backend/d3d12/ComputePipelineD3D12.h b/src/backend/d3d12/ComputePipelineD3D12.h index fd9f2a67eb..eb56c0a97e 100644 --- a/src/backend/d3d12/ComputePipelineD3D12.h +++ b/src/backend/d3d12/ComputePipelineD3D12.h @@ -21,14 +21,18 @@ namespace backend { namespace d3d12 { + class Device; + class ComputePipeline : public ComputePipelineBase { public: ComputePipeline(ComputePipelineBuilder* builder); + ~ComputePipeline(); ComPtr GetPipelineState(); private: ComPtr mPipelineState; + Device* mDevice = nullptr; }; }} // namespace backend::d3d12 diff --git a/src/backend/d3d12/D3D12Backend.cpp b/src/backend/d3d12/D3D12Backend.cpp index 230d0a8280..f27d601693 100644 --- a/src/backend/d3d12/D3D12Backend.cpp +++ b/src/backend/d3d12/D3D12Backend.cpp @@ -149,6 +149,8 @@ namespace backend { namespace d3d12 { NextSerial(); WaitForSerial(currentSerial); // Wait for all in-flight commands to finish executing TickImpl(); // Call tick one last time so resources are cleaned up + ASSERT(mUsedComObjectRefs.Empty()); + delete mCommandAllocatorManager; delete mDescriptorHeapAllocator; delete mMapReadRequestTracker; @@ -214,6 +216,7 @@ namespace backend { namespace d3d12 { mCommandAllocatorManager->Tick(lastCompletedSerial); mDescriptorHeapAllocator->Tick(lastCompletedSerial); mMapReadRequestTracker->Tick(lastCompletedSerial); + mUsedComObjectRefs.ClearUpTo(lastCompletedSerial); ExecuteCommandLists({}); NextSerial(); } @@ -234,6 +237,10 @@ namespace backend { namespace d3d12 { } } + void Device::ReferenceUntilUnused(ComPtr object) { + mUsedComObjectRefs.Enqueue(object, mSerial); + } + void Device::ExecuteCommandLists(std::initializer_list commandLists) { // If there are pending commands, prepend them to ExecuteCommandLists if (mPendingCommands.open) { diff --git a/src/backend/d3d12/D3D12Backend.h b/src/backend/d3d12/D3D12Backend.h index 22f0938a9d..48fba23882 100644 --- a/src/backend/d3d12/D3D12Backend.h +++ b/src/backend/d3d12/D3D12Backend.h @@ -21,8 +21,8 @@ #include "backend/Device.h" #include "backend/RenderPass.h" #include "backend/ToBackend.h" - #include "backend/d3d12/d3d12_platform.h" +#include "common/SerialQueue.h" namespace backend { namespace d3d12 { @@ -127,6 +127,8 @@ namespace backend { namespace d3d12 { void NextSerial(); void WaitForSerial(uint64_t serial); + void ReferenceUntilUnused(ComPtr object); + void ExecuteCommandLists(std::initializer_list commandLists); private: @@ -149,6 +151,8 @@ namespace backend { namespace d3d12 { ComPtr commandList; bool open = false; } mPendingCommands; + + SerialQueue> mUsedComObjectRefs; }; class RenderPass : public RenderPassBase { diff --git a/src/backend/d3d12/RenderPipelineD3D12.cpp b/src/backend/d3d12/RenderPipelineD3D12.cpp index 3e23dfd28a..f0937226d5 100644 --- a/src/backend/d3d12/RenderPipelineD3D12.cpp +++ b/src/backend/d3d12/RenderPipelineD3D12.cpp @@ -64,7 +64,8 @@ namespace backend { namespace d3d12 { RenderPipeline::RenderPipeline(RenderPipelineBuilder* builder) : RenderPipelineBase(builder), - mD3d12PrimitiveTopology(D3D12PrimitiveTopology(GetPrimitiveTopology())) { + mD3d12PrimitiveTopology(D3D12PrimitiveTopology(GetPrimitiveTopology())), + mDevice(ToBackend(builder->GetDevice())) { uint32_t compileFlags = 0; #if defined(_DEBUG) // Enable better shader debugging with the graphics debugging tools. @@ -171,6 +172,10 @@ namespace backend { namespace d3d12 { &descriptor, IID_PPV_ARGS(&mPipelineState))); } + RenderPipeline::~RenderPipeline() { + mDevice->ReferenceUntilUnused(mPipelineState); + } + D3D12_PRIMITIVE_TOPOLOGY RenderPipeline::GetD3D12PrimitiveTopology() const { return mD3d12PrimitiveTopology; } diff --git a/src/backend/d3d12/RenderPipelineD3D12.h b/src/backend/d3d12/RenderPipelineD3D12.h index ad29870993..783cb5f670 100644 --- a/src/backend/d3d12/RenderPipelineD3D12.h +++ b/src/backend/d3d12/RenderPipelineD3D12.h @@ -21,9 +21,12 @@ namespace backend { namespace d3d12 { + class Device; + class RenderPipeline : public RenderPipelineBase { public: RenderPipeline(RenderPipelineBuilder* builder); + ~RenderPipeline(); D3D12_PRIMITIVE_TOPOLOGY GetD3D12PrimitiveTopology() const; ComPtr GetPipelineState(); @@ -31,6 +34,8 @@ namespace backend { namespace d3d12 { private: D3D12_PRIMITIVE_TOPOLOGY mD3d12PrimitiveTopology; ComPtr mPipelineState; + + Device* mDevice = nullptr; }; }} // namespace backend::d3d12