From 4bb5004e4067a245b8bb199427310aaf656a422f Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 9 Mar 2021 23:03:41 -0800 Subject: Fixed issue with destruction of ShaderResourceBindingBase --- .../include/ShaderResourceBindingBase.hpp | 106 ++++++++++++--------- 1 file changed, 60 insertions(+), 46 deletions(-) (limited to 'Graphics/GraphicsEngine') diff --git a/Graphics/GraphicsEngine/include/ShaderResourceBindingBase.hpp b/Graphics/GraphicsEngine/include/ShaderResourceBindingBase.hpp index e6dddb0c..f3106afc 100644 --- a/Graphics/GraphicsEngine/include/ShaderResourceBindingBase.hpp +++ b/Graphics/GraphicsEngine/include/ShaderResourceBindingBase.hpp @@ -75,69 +75,67 @@ public: m_pPRS{pPRS}, m_ShaderResourceCache{ResourceCacheContentType::SRB} { - m_ActiveShaderStageIndex.fill(-1); - - const auto NumShaders = GetNumShaders(); - const auto PipelineType = GetPipelineType(); - for (Uint32 s = 0; s < NumShaders; ++s) + try { - const auto ShaderType = pPRS->GetActiveShaderStageType(s); - const auto ShaderInd = GetShaderTypePipelineIndex(ShaderType, PipelineType); + m_ActiveShaderStageIndex.fill(-1); - m_ActiveShaderStageIndex[ShaderInd] = static_cast(s); - } + const auto NumShaders = GetNumShaders(); + const auto PipelineType = GetPipelineType(); + for (Uint32 s = 0; s < NumShaders; ++s) + { + const auto ShaderType = pPRS->GetActiveShaderStageType(s); + const auto ShaderInd = GetShaderTypePipelineIndex(ShaderType, PipelineType); + + m_ActiveShaderStageIndex[ShaderInd] = static_cast(s); + } - FixedLinearAllocator MemPool{GetRawAllocator()}; - MemPool.AddSpace(NumShaders); - MemPool.Reserve(); - static_assert(std::is_nothrow_constructible::value, - "Constructor of ShaderVariableManagerImplType must be noexcept, so we can safely construct all managers"); - m_pShaderVarMgrs = MemPool.ConstructArray(NumShaders, std::ref(*this), std::ref(m_ShaderResourceCache)); + FixedLinearAllocator MemPool{GetRawAllocator()}; + MemPool.AddSpace(NumShaders); + MemPool.Reserve(); + static_assert(std::is_nothrow_constructible::value, + "Constructor of ShaderVariableManagerImplType must be noexcept, so we can safely construct all managers"); + m_pShaderVarMgrs = MemPool.ConstructArray(NumShaders, std::ref(*this), std::ref(m_ShaderResourceCache)); - // The memory is now owned by ShaderResourceBindingBase and will be freed by destructor. - auto* Ptr = MemPool.ReleaseOwnership(); - VERIFY_EXPR(Ptr == m_pShaderVarMgrs); - (void)Ptr; + // The memory is now owned by ShaderResourceBindingBase and will be freed by Destruct(). + auto* Ptr = MemPool.ReleaseOwnership(); + VERIFY_EXPR(Ptr == m_pShaderVarMgrs); + (void)Ptr; - // It is important to construct all objects before initializing them because if an exception is thrown, - // destructors will be called for all objects + // It is important to construct all objects before initializing them because if an exception is thrown, + // Destruct() will call destructors for all non-null objects. - pPRS->InitSRBResourceCache(m_ShaderResourceCache); + pPRS->InitSRBResourceCache(m_ShaderResourceCache); - auto& SRBMemAllocator = pPRS->GetSRBMemoryAllocator(); - for (Uint32 s = 0; s < NumShaders; ++s) - { - const auto ShaderType = pPRS->GetActiveShaderStageType(s); - const auto ShaderInd = GetShaderTypePipelineIndex(ShaderType, pPRS->GetPipelineType()); - const auto MgrInd = m_ActiveShaderStageIndex[ShaderInd]; - VERIFY_EXPR(MgrInd >= 0 && MgrInd < static_cast(NumShaders)); + auto& SRBMemAllocator = pPRS->GetSRBMemoryAllocator(); + for (Uint32 s = 0; s < NumShaders; ++s) + { + const auto ShaderType = pPRS->GetActiveShaderStageType(s); + const auto ShaderInd = GetShaderTypePipelineIndex(ShaderType, pPRS->GetPipelineType()); + const auto MgrInd = m_ActiveShaderStageIndex[ShaderInd]; + VERIFY_EXPR(MgrInd >= 0 && MgrInd < static_cast(NumShaders)); - auto& VarDataAllocator = SRBMemAllocator.GetShaderVariableDataAllocator(s); + auto& VarDataAllocator = SRBMemAllocator.GetShaderVariableDataAllocator(s); - // 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[MgrInd].Initialize(*pPRS, VarDataAllocator, VarTypes, _countof(VarTypes), ShaderType); + // 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[MgrInd].Initialize(*pPRS, VarDataAllocator, VarTypes, _countof(VarTypes), ShaderType); + } + } + catch (...) + { + // We must release objects manually as destructor will not be called. + Destruct(); + throw; } } ~ShaderResourceBindingBase() { - if (m_pShaderVarMgrs != nullptr) - { - auto& SRBMemAllocator = GetSignature()->GetSRBMemoryAllocator(); - for (Uint32 s = 0; s < GetNumShaders(); ++s) - { - auto& VarDataAllocator = SRBMemAllocator.GetShaderVariableDataAllocator(s); - m_pShaderVarMgrs[s].Destroy(VarDataAllocator); - m_pShaderVarMgrs[s].~ShaderVariableManagerImplType(); - } - GetRawAllocator().Free(m_pShaderVarMgrs); - } + Destruct(); } - IMPLEMENT_QUERY_INTERFACE_IN_PLACE(IID_ShaderResourceBinding, TObjectBase) Uint32 GetBindingIndex() const @@ -261,6 +259,22 @@ public: ShaderResourceCacheImplType& GetResourceCache() { return m_ShaderResourceCache; } const ShaderResourceCacheImplType& GetResourceCache() const { return m_ShaderResourceCache; } +private: + void Destruct() + { + if (m_pShaderVarMgrs != nullptr) + { + auto& SRBMemAllocator = GetSignature()->GetSRBMemoryAllocator(); + for (Uint32 s = 0; s < GetNumShaders(); ++s) + { + auto& VarDataAllocator = SRBMemAllocator.GetShaderVariableDataAllocator(s); + m_pShaderVarMgrs[s].Destroy(VarDataAllocator); + m_pShaderVarMgrs[s].~ShaderVariableManagerImplType(); + } + GetRawAllocator().Free(m_pShaderVarMgrs); + } + } + protected: /// Strong reference to pipeline resource signature. We must use strong reference, because /// shader resource binding uses pipeline resource signature's memory allocator to allocate -- cgit v1.2.3