From 0c7e8e2efc95362cbc0998558bb41789a3943037 Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 20 Oct 2020 14:06:33 -0700 Subject: Improved exception safety of SRB object creation --- .../include/ShaderResourceBindingD3D12Impl.hpp | 2 + .../src/ShaderResourceBindingD3D12Impl.cpp | 88 ++++++++++++++-------- 2 files changed, 59 insertions(+), 31 deletions(-) (limited to 'Graphics/GraphicsEngineD3D12') diff --git a/Graphics/GraphicsEngineD3D12/include/ShaderResourceBindingD3D12Impl.hpp b/Graphics/GraphicsEngineD3D12/include/ShaderResourceBindingD3D12Impl.hpp index 846cb329..7be3372d 100644 --- a/Graphics/GraphicsEngineD3D12/include/ShaderResourceBindingD3D12Impl.hpp +++ b/Graphics/GraphicsEngineD3D12/include/ShaderResourceBindingD3D12Impl.hpp @@ -78,6 +78,8 @@ public: } private: + void Destruct(); + ShaderResourceCacheD3D12 m_ShaderResourceCache; ShaderVariableManagerD3D12* m_pShaderVarMgrs = nullptr; diff --git a/Graphics/GraphicsEngineD3D12/src/ShaderResourceBindingD3D12Impl.cpp b/Graphics/GraphicsEngineD3D12/src/ShaderResourceBindingD3D12Impl.cpp index 1c5ae5f5..1511bd01 100644 --- a/Graphics/GraphicsEngineD3D12/src/ShaderResourceBindingD3D12Impl.cpp +++ b/Graphics/GraphicsEngineD3D12/src/ShaderResourceBindingD3D12Impl.cpp @@ -30,6 +30,7 @@ #include "PipelineStateD3D12Impl.hpp" #include "ShaderD3D12Impl.hpp" #include "RenderDeviceD3D12Impl.hpp" +#include "LinearAllocator.hpp" namespace Diligent { @@ -48,48 +49,73 @@ ShaderResourceBindingD3D12Impl::ShaderResourceBindingD3D12Impl(IReferenceCounter m_NumShaders {static_cast(pPSO->GetNumShaderStages())} // clang-format on { - m_ResourceLayoutIndex.fill(-1); + try + { + m_ResourceLayoutIndex.fill(-1); + + LinearAllocator MemPool{GetRawAllocator()}; + MemPool.AddSpace(m_NumShaders); + MemPool.Reserve(); + m_pShaderVarMgrs = MemPool.ConstructArray(m_NumShaders, std::ref(*this), std::ref(m_ShaderResourceCache)); - auto* pRenderDeviceD3D12Impl = ValidatedCast(pPSO->GetDevice()); - auto& ResCacheDataAllocator = pPSO->GetSRBMemoryAllocator().GetResourceCacheDataAllocator(0); - pPSO->GetRootSignature().InitResourceCache(pRenderDeviceD3D12Impl, m_ShaderResourceCache, ResCacheDataAllocator); + // The memory is now owned by ShaderResourceBindingD3D12Impl and will be freed by Destruct(). + auto* Ptr = MemPool.ReleaseOwnership(); + VERIFY_EXPR(Ptr == m_pShaderVarMgrs); + (void)Ptr; - m_pShaderVarMgrs = ALLOCATE(GetRawAllocator(), "Raw memory for ShaderVariableManagerD3D12", ShaderVariableManagerD3D12, m_NumShaders); + // It is important to construct all objects before initializing them because if an exception is thrown, + // destructors will be called for all objects - for (Uint32 s = 0; s < m_NumShaders; ++s) + auto* pRenderDeviceD3D12Impl = ValidatedCast(pPSO->GetDevice()); + auto& ResCacheDataAllocator = pPSO->GetSRBMemoryAllocator().GetResourceCacheDataAllocator(0); + pPSO->GetRootSignature().InitResourceCache(pRenderDeviceD3D12Impl, m_ShaderResourceCache, ResCacheDataAllocator); + + for (Uint32 s = 0; s < m_NumShaders; ++s) + { + const auto ShaderType = pPSO->GetShaderStageType(s); + const auto& SrcLayout = pPSO->GetShaderResLayout(s); + const auto ShaderInd = GetShaderTypePipelineIndex(ShaderType, pPSO->GetDesc().PipelineType); + + auto& VarDataAllocator = pPSO->GetSRBMemoryAllocator().GetShaderVariableDataAllocator(s); + + // http://diligentgraphics.com/diligent-engine/architecture/d3d12/shader-resource-layout#Initializing-Resource-Layouts-in-a-Shader-Resource-Binding-Object + const SHADER_RESOURCE_VARIABLE_TYPE AllowedVarTypes[] = {SHADER_RESOURCE_VARIABLE_TYPE_MUTABLE, SHADER_RESOURCE_VARIABLE_TYPE_DYNAMIC}; + m_pShaderVarMgrs[s].Initialize( + SrcLayout, + VarDataAllocator, + AllowedVarTypes, + _countof(AllowedVarTypes) // + ); + + m_ResourceLayoutIndex[ShaderInd] = static_cast(s); + } + } + catch (...) { - auto ShaderType = pPSO->GetShaderStageType(s); - auto ShaderInd = GetShaderTypePipelineIndex(ShaderType, pPSO->GetDesc().PipelineType); - - auto& VarDataAllocator = pPSO->GetSRBMemoryAllocator().GetShaderVariableDataAllocator(s); - - // http://diligentgraphics.com/diligent-engine/architecture/d3d12/shader-resource-layout#Initializing-Resource-Layouts-in-a-Shader-Resource-Binding-Object - const SHADER_RESOURCE_VARIABLE_TYPE AllowedVarTypes[] = {SHADER_RESOURCE_VARIABLE_TYPE_MUTABLE, SHADER_RESOURCE_VARIABLE_TYPE_DYNAMIC}; - const auto& SrcLayout = pPSO->GetShaderResLayout(s); - // Create shader variable manager in place - new (m_pShaderVarMgrs + s) ShaderVariableManagerD3D12{*this, m_ShaderResourceCache}; - m_pShaderVarMgrs[s].Initialize( - SrcLayout, - VarDataAllocator, - AllowedVarTypes, - _countof(AllowedVarTypes) // - ); - - m_ResourceLayoutIndex[ShaderInd] = static_cast(s); + Destruct(); + throw; } } + ShaderResourceBindingD3D12Impl::~ShaderResourceBindingD3D12Impl() { - auto* pPSO = ValidatedCast(m_pPSO); - for (Uint32 s = 0; s < m_NumShaders; ++s) + Destruct(); +} + +void ShaderResourceBindingD3D12Impl::Destruct() +{ + if (m_pShaderVarMgrs != nullptr) { - auto& VarDataAllocator = pPSO->GetSRBMemoryAllocator().GetShaderVariableDataAllocator(s); - m_pShaderVarMgrs[s].Destroy(VarDataAllocator); - m_pShaderVarMgrs[s].~ShaderVariableManagerD3D12(); + auto& SRBMemAllocator = m_pPSO->GetSRBMemoryAllocator(); + for (Uint32 s = 0; s < m_NumShaders; ++s) + { + auto& VarDataAllocator = SRBMemAllocator.GetShaderVariableDataAllocator(s); + m_pShaderVarMgrs[s].Destroy(VarDataAllocator); + m_pShaderVarMgrs[s].~ShaderVariableManagerD3D12(); + } + GetRawAllocator().Free(m_pShaderVarMgrs); } - - GetRawAllocator().Free(m_pShaderVarMgrs); } IMPLEMENT_QUERY_INTERFACE(ShaderResourceBindingD3D12Impl, IID_ShaderResourceBindingD3D12, TBase) -- cgit v1.2.3