From 876034c5e736200d471057b8cf555ded566345fa Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Sat, 2 Mar 2019 13:33:32 -0800 Subject: Updated ShaderVariableVk: not creating variables for immutable samplers + code cleanup --- .../include/ShaderVariableVk.h | 14 ++-- .../GraphicsEngineVulkan/src/ShaderVariableVk.cpp | 83 ++++++++++++---------- 2 files changed, 51 insertions(+), 46 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.h index 5f7a0628..bb555881 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.h @@ -30,7 +30,7 @@ // * ShaderVariableManagerVk keeps list of variables of specific types // * Every ShaderVariableVkImpl references VkResource from ShaderResourceLayoutVk // * ShaderVariableManagerVk keeps pointer to ShaderResourceCacheVk -// * ShaderVariableManagerVk is used by ShaderVkImpl to manage static resources and by +// * ShaderVariableManagerVk is used by PipelineStateVkImpl to manage static resources and by // ShaderResourceBindingVkImpl to manage mutable and dynamic resources // // __________________________ __________________________________________________________________________ @@ -83,7 +83,7 @@ public: ShaderVariableVkImpl* GetVariable(const Char* Name); ShaderVariableVkImpl* GetVariable(Uint32 Index); - void BindResources( IResourceMapping* pResourceMapping, Uint32 Flags); + void BindResources(IResourceMapping* pResourceMapping, Uint32 Flags); static size_t GetRequiredMemorySize(const ShaderResourceLayoutVk& Layout, const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes, @@ -98,10 +98,10 @@ private: Uint32 GetVariableIndex(const ShaderVariableVkImpl& Variable); IObject& m_Owner; - // Variable mgr is owned by either Shader object (in which case m_pResourceLayout points to - // static resource layout owned by the same shader object), or by SRB object (in which case + // Variable mgr is owned by either Pipeline state object (in which case m_pResourceLayout points to + // static resource layout owned by the same PSO object), or by SRB object (in which case // m_pResourceLayout points to corresponding layout in pipeline state). Since SRB keeps strong - // reference to PSO, the layout is guaranteed be alive while SRB is alive + // reference to PSO, the layout is guaranteed to be alive while SRB is alive const ShaderResourceLayoutVk* m_pResourceLayout= nullptr; ShaderResourceCacheVk* m_pResourceCache = nullptr; @@ -147,7 +147,7 @@ public: return m_ParentManager.m_Owner.Release(); } - void QueryInterface(const INTERFACE_ID &IID, IObject **ppInterface)override final + void QueryInterface(const INTERFACE_ID& IID, IObject** ppInterface)override final { if (ppInterface == nullptr) return; @@ -165,7 +165,7 @@ public: return m_Resource.GetVariableType(); } - virtual void Set(IDeviceObject *pObject)override final + virtual void Set(IDeviceObject* pObject)override final { VERIFY_EXPR(m_ParentManager.m_pResourceCache != nullptr); m_Resource.BindResource(pObject, 0, *m_ParentManager.m_pResourceCache); diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVariableVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVariableVk.cpp index 22acaf5c..6698dd53 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderVariableVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderVariableVk.cpp @@ -29,42 +29,43 @@ namespace Diligent { -size_t ShaderVariableManagerVk::GetRequiredMemorySize(const ShaderResourceLayoutVk& Layout, +size_t ShaderVariableManagerVk::GetRequiredMemorySize(const ShaderResourceLayoutVk& Layout, const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes, - Uint32 NumAllowedTypes, - Uint32& NumVariables) + Uint32 NumAllowedTypes, + Uint32& NumVariables) { NumVariables = 0; - Uint32 AllowedTypeBits = GetAllowedTypeBits(AllowedVarTypes, NumAllowedTypes); - for(SHADER_RESOURCE_VARIABLE_TYPE VarType = SHADER_RESOURCE_VARIABLE_TYPE_STATIC; VarType < SHADER_RESOURCE_VARIABLE_TYPE_NUM_TYPES; VarType = static_cast(VarType+1)) + const Uint32 AllowedTypeBits = GetAllowedTypeBits(AllowedVarTypes, NumAllowedTypes); + const bool UsingSeparateSamplers = Layout.IsUsingSeparateSamplers(); + for (SHADER_RESOURCE_VARIABLE_TYPE VarType = SHADER_RESOURCE_VARIABLE_TYPE_STATIC; VarType < SHADER_RESOURCE_VARIABLE_TYPE_NUM_TYPES; VarType = static_cast(VarType+1)) { if (IsAllowedType(VarType, AllowedTypeBits)) { auto NumResources = Layout.GetResourceCount(VarType); - if (Layout.IsUsingSeparateSamplers()) - NumVariables += NumResources; - else + for (Uint32 r=0; r < NumResources; ++r) { - // When using HLSL-style combined image samplers, we need to skip separate samplers - for( Uint32 r=0; r < NumResources; ++r ) - { - const auto& SrcRes = Layout.GetResource(VarType, r); - if (SrcRes.SpirvAttribs.Type != SPIRVShaderResourceAttribs::ResourceType::SeparateSampler) - ++NumVariables; - } + const auto& SrcRes = Layout.GetResource(VarType, r); + + // When using HLSL-style combined image samplers, we need to skip separate samplers. + // Also always skip immutable separate samplers. + if (SrcRes.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler && + (!UsingSeparateSamplers || SrcRes.IsImmutableSamplerAssigned()) ) + continue; + + ++NumVariables; } } } - return NumVariables*sizeof(ShaderVariableVkImpl); + return NumVariables * sizeof(ShaderVariableVkImpl); } // Creates shader variable for every resource from SrcLayout whose type is one AllowedVarTypes -void ShaderVariableManagerVk::Initialize(const ShaderResourceLayoutVk& SrcLayout, - IMemoryAllocator& Allocator, +void ShaderVariableManagerVk::Initialize(const ShaderResourceLayoutVk& SrcLayout, + IMemoryAllocator& Allocator, const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes, - Uint32 NumAllowedTypes, - ShaderResourceCacheVk& ResourceCache) + Uint32 NumAllowedTypes, + ShaderResourceCacheVk& ResourceCache) { m_pResourceLayout = &SrcLayout; m_pResourceCache = &ResourceCache; @@ -79,23 +80,26 @@ void ShaderVariableManagerVk::Initialize(const ShaderResourceLayoutVk& SrcLayout if(m_NumVariables == 0) return; - auto *pRawMem = ALLOCATE(Allocator, "Raw memory buffer for shader variables", MemSize); + auto* pRawMem = ALLOCATE(Allocator, "Raw memory buffer for shader variables", MemSize); m_pVariables = reinterpret_cast(pRawMem); Uint32 VarInd = 0; - for(SHADER_RESOURCE_VARIABLE_TYPE VarType = SHADER_RESOURCE_VARIABLE_TYPE_STATIC; VarType < SHADER_RESOURCE_VARIABLE_TYPE_NUM_TYPES; VarType = static_cast(VarType+1)) + const bool UsingSeparateSamplers = SrcLayout.IsUsingSeparateSamplers(); + for (SHADER_RESOURCE_VARIABLE_TYPE VarType = SHADER_RESOURCE_VARIABLE_TYPE_STATIC; VarType < SHADER_RESOURCE_VARIABLE_TYPE_NUM_TYPES; VarType = static_cast(VarType+1)) { if (!IsAllowedType(VarType, AllowedTypeBits)) continue; Uint32 NumResources = SrcLayout.GetResourceCount(VarType); - for( Uint32 r=0; r < NumResources; ++r ) + for (Uint32 r=0; r < NumResources; ++r) { const auto& SrcRes = SrcLayout.GetResource(VarType, r); - if (!SrcLayout.IsUsingSeparateSamplers() && SrcRes.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler) + // Skip separate samplers when using combined HLSL-style image samplers. Also always skip immutable separate samplers. + if (SrcRes.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler && + (!UsingSeparateSamplers || SrcRes.IsImmutableSamplerAssigned()) ) continue; - ::new (m_pVariables + VarInd) ShaderVariableVkImpl(*this, SrcRes ); + ::new (m_pVariables + VarInd) ShaderVariableVkImpl(*this, SrcRes); ++VarInd; } } @@ -111,9 +115,9 @@ void ShaderVariableManagerVk::Destroy(IMemoryAllocator &Allocator) { VERIFY(m_pDbgAllocator == &Allocator, "Incosistent alloctor"); - if(m_pVariables != nullptr) + if (m_pVariables != nullptr) { - for(Uint32 v=0; v < m_NumVariables; ++v) + for (Uint32 v=0; v < m_NumVariables; ++v) m_pVariables[v].~ShaderVariableVkImpl(); Allocator.Free(m_pVariables); m_pVariables = nullptr; @@ -125,7 +129,7 @@ ShaderVariableVkImpl* ShaderVariableManagerVk::GetVariable(const Char* Name) ShaderVariableVkImpl* pVar = nullptr; for (Uint32 v = 0; v < m_NumVariables; ++v) { - auto &Var = m_pVariables[v]; + auto& Var = m_pVariables[v]; const auto& Res = Var.m_Resource; if (strcmp(Res.SpirvAttribs.Name, Name) == 0) { @@ -168,11 +172,11 @@ Uint32 ShaderVariableManagerVk::GetVariableIndex(const ShaderVariableVkImpl& Var } } -void ShaderVariableManagerVk::BindResources( IResourceMapping* pResourceMapping, Uint32 Flags) +void ShaderVariableManagerVk::BindResources(IResourceMapping* pResourceMapping, Uint32 Flags) { VERIFY_EXPR(m_pResourceCache != nullptr); - if( !pResourceMapping ) + if (!pResourceMapping) { LOG_ERROR_MESSAGE( "Failed to bind resources: resource mapping is null" ); return; @@ -181,34 +185,35 @@ void ShaderVariableManagerVk::BindResources( IResourceMapping* pResourceMapping, if ( (Flags & BIND_SHADER_RESOURCES_UPDATE_ALL) == 0 ) Flags |= BIND_SHADER_RESOURCES_UPDATE_ALL; - for(Uint32 v=0; v < m_NumVariables; ++v) + for (Uint32 v=0; v < m_NumVariables; ++v) { - auto &Var = m_pVariables[v]; + auto& Var = m_pVariables[v]; const auto& Res = Var.m_Resource; - // Skip immutable separate samplers - if (Res.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler && Res.IsImmutableSamplerAssigned()) - continue; + // There should be no immutable separate samplers + VERIFY(Res.SpirvAttribs.Type != SPIRVShaderResourceAttribs::ResourceType::SeparateSampler || !Res.IsImmutableSamplerAssigned(), + "There must be no shader resource variables for immutable separate samplers"); if ( (Flags & (1 << Res.GetVariableType())) == 0 ) continue; for (Uint32 ArrInd = 0; ArrInd < Res.SpirvAttribs.ArraySize; ++ArrInd) { - if( (Flags & BIND_SHADER_RESOURCES_KEEP_EXISTING) && Res.IsBound(ArrInd, *m_pResourceCache) ) + if ( (Flags & BIND_SHADER_RESOURCES_KEEP_EXISTING) && Res.IsBound(ArrInd, *m_pResourceCache) ) continue; const auto* VarName = Res.SpirvAttribs.Name; RefCntAutoPtr pObj; pResourceMapping->GetResource( VarName, &pObj, ArrInd ); - if( pObj ) + if (pObj) { Res.BindResource(pObj, ArrInd, *m_pResourceCache); } else { - if( (Flags & BIND_SHADER_RESOURCES_VERIFY_ALL_RESOLVED) && !Res.IsBound(ArrInd, *m_pResourceCache) ) - LOG_ERROR_MESSAGE( "Unable to bind resource to shader variable '", Res.SpirvAttribs.GetPrintName(ArrInd), "': resource is not found in the resource mapping" ); + if ( (Flags & BIND_SHADER_RESOURCES_VERIFY_ALL_RESOLVED) && !Res.IsBound(ArrInd, *m_pResourceCache) ) + LOG_ERROR_MESSAGE( "Unable to bind resource to shader variable '", Res.SpirvAttribs.GetPrintName(ArrInd), "': resource is not found in the resource mapping. " + "Do not use BIND_SHADER_RESOURCES_VERIFY_ALL_RESOLVED flag to suppress the message if this is not an issue." ); } } } -- cgit v1.2.3