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/ShaderResourceBindingVkImpl.hpp | 2 + .../src/ShaderResourceBindingVkImpl.cpp | 88 ++++++++++++++-------- 2 files changed, 60 insertions(+), 30 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp index 026bb9da..00f2c33a 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp @@ -76,6 +76,8 @@ public: bool StaticResourcesInitialized() const { return m_bStaticResourcesInitialized; } private: + void Destruct(); + ShaderResourceCacheVk m_ShaderResourceCache; ShaderVariableManagerVk* m_pShaderVarMgrs = nullptr; diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp index 21b216db..3e4a65cf 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp @@ -30,6 +30,7 @@ #include "PipelineStateVkImpl.hpp" #include "ShaderVkImpl.hpp" #include "RenderDeviceVkImpl.hpp" +#include "LinearAllocator.hpp" namespace Diligent { @@ -47,52 +48,79 @@ ShaderResourceBindingVkImpl::ShaderResourceBindingVkImpl(IReferenceCounters* pR m_ShaderResourceCache{ShaderResourceCacheVk::DbgCacheContentType::SRBResources} // clang-format on { - m_ResourceLayoutIndex.fill(-1); + try + { + m_ResourceLayoutIndex.fill(-1); - m_NumShaders = static_cast(pPSO->GetNumShaderStages()); + m_NumShaders = static_cast(pPSO->GetNumShaderStages()); - auto* pRenderDeviceVkImpl = pPSO->GetDevice(); - // This will only allocate memory and initialize descriptor sets in the resource cache - // Resources will be initialized by InitializeResourceMemoryInCache() - auto& ResourceCacheDataAllocator = pPSO->GetSRBMemoryAllocator().GetResourceCacheDataAllocator(0); - pPSO->GetPipelineLayout().InitResourceCache(pRenderDeviceVkImpl, m_ShaderResourceCache, ResourceCacheDataAllocator, pPSO->GetDesc().Name); + LinearAllocator MemPool{GetRawAllocator()}; + MemPool.AddSpace(m_NumShaders); + MemPool.Reserve(); + m_pShaderVarMgrs = MemPool.ConstructArray(m_NumShaders, std::ref(*this), std::ref(m_ShaderResourceCache)); - m_pShaderVarMgrs = ALLOCATE(GetRawAllocator(), "Raw memory for ShaderVariableManagerVk", ShaderVariableManagerVk, m_NumShaders); + // The memory is now owned by ShaderResourceBindingVkImpl and will be freed by Destruct(). + auto* Ptr = MemPool.ReleaseOwnership(); + VERIFY_EXPR(Ptr == m_pShaderVarMgrs); + (void)Ptr; - for (Uint32 s = 0; s < m_NumShaders; ++s) - { - auto ShaderInd = GetShaderTypePipelineIndex(pPSO->GetShaderStageType(s), pPSO->GetDesc().PipelineType); + // It is important to construct all objects before initializing them because if an exception is thrown, + // destructors will be called for all objects - m_ResourceLayoutIndex[ShaderInd] = static_cast(s); + auto* pRenderDeviceVkImpl = pPSO->GetDevice(); + // This will only allocate memory and initialize descriptor sets in the resource cache + // Resources will be initialized by InitializeResourceMemoryInCache() + auto& ResourceCacheDataAllocator = pPSO->GetSRBMemoryAllocator().GetResourceCacheDataAllocator(0); + pPSO->GetPipelineLayout().InitResourceCache(pRenderDeviceVkImpl, m_ShaderResourceCache, ResourceCacheDataAllocator, pPSO->GetDesc().Name); - auto& VarDataAllocator = pPSO->GetSRBMemoryAllocator().GetShaderVariableDataAllocator(s); + for (Uint32 s = 0; s < m_NumShaders; ++s) + { + auto ShaderInd = GetShaderTypePipelineIndex(pPSO->GetShaderStageType(s), pPSO->GetDesc().PipelineType); - const auto& SrcLayout = pPSO->GetShaderResLayout(s); - // Use source layout to initialize resource memory in the cache - SrcLayout.InitializeResourceMemoryInCache(m_ShaderResourceCache); + m_ResourceLayoutIndex[ShaderInd] = static_cast(s); - // Create shader variable manager in place - // Initialize vars manager to reference mutable and dynamic variables - // Note that the cache has space for all variable types - const SHADER_RESOURCE_VARIABLE_TYPE VarTypes[] = {SHADER_RESOURCE_VARIABLE_TYPE_MUTABLE, SHADER_RESOURCE_VARIABLE_TYPE_DYNAMIC}; - new (m_pShaderVarMgrs + s) ShaderVariableManagerVk{*this, m_ShaderResourceCache}; - m_pShaderVarMgrs[s].Initialize(SrcLayout, VarDataAllocator, VarTypes, _countof(VarTypes)); - } + auto& VarDataAllocator = pPSO->GetSRBMemoryAllocator().GetShaderVariableDataAllocator(s); + + const auto& SrcLayout = pPSO->GetShaderResLayout(s); + // Use source layout to initialize resource memory in the cache + SrcLayout.InitializeResourceMemoryInCache(m_ShaderResourceCache); + + // Create shader variable manager in place + // Initialize vars manager to reference mutable and dynamic variables + // Note that the cache has space for all variable types + const SHADER_RESOURCE_VARIABLE_TYPE VarTypes[] = {SHADER_RESOURCE_VARIABLE_TYPE_MUTABLE, SHADER_RESOURCE_VARIABLE_TYPE_DYNAMIC}; + m_pShaderVarMgrs[s].Initialize(SrcLayout, VarDataAllocator, VarTypes, _countof(VarTypes)); + } #ifdef DILIGENT_DEBUG - m_ShaderResourceCache.DbgVerifyResourceInitialization(); + m_ShaderResourceCache.DbgVerifyResourceInitialization(); #endif + } + catch (...) + { + Destruct(); + throw; + } } ShaderResourceBindingVkImpl::~ShaderResourceBindingVkImpl() { - for (Uint32 s = 0; s < m_NumShaders; ++s) + Destruct(); +} + +void ShaderResourceBindingVkImpl::Destruct() +{ + if (m_pShaderVarMgrs != nullptr) { - auto& VarDataAllocator = m_pPSO->GetSRBMemoryAllocator().GetShaderVariableDataAllocator(s); - m_pShaderVarMgrs[s].DestroyVariables(VarDataAllocator); - m_pShaderVarMgrs[s].~ShaderVariableManagerVk(); - } + auto& SRBMemAllocator = m_pPSO->GetSRBMemoryAllocator(); + for (Uint32 s = 0; s < m_NumShaders; ++s) + { + auto& VarDataAllocator = SRBMemAllocator.GetShaderVariableDataAllocator(s); + m_pShaderVarMgrs[s].DestroyVariables(VarDataAllocator); + m_pShaderVarMgrs[s].~ShaderVariableManagerVk(); + } - GetRawAllocator().Free(m_pShaderVarMgrs); + GetRawAllocator().Free(m_pShaderVarMgrs); + } } IMPLEMENT_QUERY_INTERFACE(ShaderResourceBindingVkImpl, IID_ShaderResourceBindingVk, TBase) -- cgit v1.2.3