From aaa2ed4803c0d62c6d2f085aa5b72fee6f893c5c Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Sun, 20 May 2018 16:54:40 -0700 Subject: Updated static resource intialization in Vk --- .../include/DeviceContextVkImpl.h | 4 +-- .../include/ShaderResourceLayoutVk.h | 2 +- .../src/DeviceContextVkImpl.cpp | 7 ++-- .../src/PipelineStateVkImpl.cpp | 9 ++--- .../src/ShaderResourceBindingVkImpl.cpp | 12 +++++-- .../src/ShaderResourceLayoutVk.cpp | 38 +++++++++++++--------- Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp | 6 ++-- 7 files changed, 43 insertions(+), 35 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h index 7c87e5fb..774cc37a 100644 --- a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h +++ b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h @@ -162,9 +162,9 @@ private: GenerateMipsHelper m_MipsGenerator; class DynamicUploadHeap* m_pUploadHeap = nullptr; - +#endif class ShaderResourceCacheVk *m_pCommittedResourceCache = nullptr; - +#if 0 FixedBlockMemoryAllocator m_CmdListAllocator; const Uint32 m_ContextId; #endif diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h index e52f6b08..4bb3b1b7 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h @@ -265,7 +265,7 @@ public: // dbgResourceCache is only used for sanity check and as a remainder that the resource cache must be alive // while Layout is alive void BindResources( IResourceMapping* pResourceMapping, Uint32 Flags, const ShaderResourceCacheVk *dbgResourceCache ); - IShaderVariable* GetShaderVariable( const Char* Name ); + VkResource* GetShaderVariable( const Char* Name ); #ifdef VERIFY_SHADER_BINDINGS void dbgVerifyBindings()const; diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp index 32e8116e..e5281833 100644 --- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp @@ -202,12 +202,11 @@ namespace Diligent { if (!DeviceContextBase::CommitShaderResources(pShaderResourceBinding, Flags, 0 /*Dummy*/)) return; -#if 0 - auto *pCtx = RequestCmdContext(); + + EnsureVkCmdBuffer(); auto *pPipelineStateVk = m_pPipelineState.RawPtr(); - m_pCommittedResourceCache = pPipelineStateVk->CommitAndTransitionShaderResources(pShaderResourceBinding, *pCtx, true, (Flags & COMMIT_SHADER_RESOURCES_FLAG_TRANSITION_RESOURCES)!=0); -#endif + m_pCommittedResourceCache = pPipelineStateVk->CommitAndTransitionShaderResources(pShaderResourceBinding, m_CommandBuffer, true, (Flags & COMMIT_SHADER_RESOURCES_FLAG_TRANSITION_RESOURCES)!=0); } void DeviceContextVkImpl::SetStencilRef(Uint32 StencilRef) diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp index e7e0a578..2126f404 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp @@ -502,11 +502,8 @@ ShaderResourceCacheVk* PipelineStateVkImpl::CommitAndTransitionShaderResources(I bool CommitResources, bool TransitionResources)const { -#if 0 #ifdef VERIFY_SHADER_BINDINGS - if (pShaderResourceBinding == nullptr && - (m_RootSig.GetTotalSrvCbvUavSlots(SHADER_VARIABLE_TYPE_MUTABLE) != 0 || - m_RootSig.GetTotalSrvCbvUavSlots(SHADER_VARIABLE_TYPE_DYNAMIC) != 0)) + if (pShaderResourceBinding == nullptr && !m_pDefaultShaderResBinding) { LOG_ERROR_MESSAGE("Pipeline state \"", m_Desc.Name, "\" contains mutable/dynamic shader variables and requires shader resource binding to commit all resources, but none is provided."); } @@ -537,6 +534,7 @@ ShaderResourceCacheVk* PipelineStateVkImpl::CommitAndTransitionShaderResources(I auto *pDeviceVkImpl = ValidatedCast( GetDevice() ); auto &ResourceCache = pResBindingVkImpl->GetResourceCache(); +#if 0 if(CommitResources) { if(m_Desc.IsComputePipeline) @@ -554,9 +552,8 @@ ShaderResourceCacheVk* PipelineStateVkImpl::CommitAndTransitionShaderResources(I VERIFY(TransitionResources, "Resources should be transitioned or committed or both"); m_RootSig.TransitionResources(ResourceCache, Ctx); } - return &ResourceCache; #endif - return nullptr; + return &ResourceCache; } #if 0 diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp index 1db2b68f..973ab200 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp @@ -95,10 +95,16 @@ IShaderVariable *ShaderResourceBindingVkImpl::GetVariable(SHADER_TYPE ShaderType return ValidatedCast(GetPipelineState())->GetDummyShaderVar(); } auto *pVar = m_pResourceLayouts[ResLayoutInd].GetShaderVariable(Name); - if(pVar == nullptr) - pVar = ValidatedCast(GetPipelineState())->GetDummyShaderVar(); + if(pVar->SpirvAttribs.VarType == SHADER_VARIABLE_TYPE_STATIC) + { + LOG_ERROR_MESSAGE("Static shader variable \"", Name, "\" must not be accessed through shader resource binding object. Static variable should be set through shader objects."); + pVar = nullptr; + } - return pVar; + if(pVar == nullptr) + return ValidatedCast(GetPipelineState())->GetDummyShaderVar(); + else + return pVar; } #ifdef VERIFY_SHADER_BINDINGS diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp index b9ab2fd7..6010c4d9 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp @@ -636,9 +636,9 @@ void ShaderResourceLayoutVk::BindResources( IResourceMapping* pResourceMapping, } } -IShaderVariable* ShaderResourceLayoutVk::GetShaderVariable(const Char* Name) +ShaderResourceLayoutVk::VkResource* ShaderResourceLayoutVk::GetShaderVariable(const Char* Name) { - IShaderVariable* pVar = nullptr; + ShaderResourceLayoutVk::VkResource* pVar = nullptr; #if USE_VARIABLE_HASH_MAP // Name will be implicitly converted to HashMapStringKey without making a copy auto it = m_VariableHash.find( Name ); @@ -679,7 +679,8 @@ void ShaderResourceLayoutVk::InitializeStaticResources(const ShaderResourceLayou return; } - VERIFY(m_NumResources[SHADER_VARIABLE_TYPE_STATIC] == SrcLayout.m_NumResources[SHADER_VARIABLE_TYPE_STATIC], "Inconsistent number of static resources"); + auto NumStaticResources = m_NumResources[SHADER_VARIABLE_TYPE_STATIC]; + VERIFY(NumStaticResources == SrcLayout.m_NumResources[SHADER_VARIABLE_TYPE_STATIC], "Inconsistent number of static resources"); VERIFY(SrcLayout.m_pResources->GetShaderType() == m_pResources->GetShaderType(), "Incosistent shader types"); // Static shader resources are stored as follows: @@ -693,7 +694,7 @@ void ShaderResourceLayoutVk::InitializeStaticResources(const ShaderResourceLayou // Separate images at index SPIRVShaderResourceAttribs::ResourceType::SeparateImage (7) // Separate samplers at index SPIRVShaderResourceAttribs::ResourceType::SeparateSampler (8) - for(Uint32 r=0; r < m_NumResources[SHADER_VARIABLE_TYPE_STATIC]; ++r) + for(Uint32 r=0; r < NumStaticResources; ++r) { // Get resource attributes auto &DstRes = GetResource(SHADER_VARIABLE_TYPE_STATIC, r); @@ -701,6 +702,10 @@ void ShaderResourceLayoutVk::InitializeStaticResources(const ShaderResourceLayou VERIFY(SrcRes.Binding == r, "Unexpected binding"); VERIFY(SrcRes.SpirvAttribs.ArraySize == DstRes.SpirvAttribs.ArraySize, "Inconsistent array size"); + if(DstRes.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler && + DstRes.SpirvAttribs.StaticSamplerInd >= 0) + continue; // Skip static samplers + for(Uint32 ArrInd = 0; ArrInd < DstRes.SpirvAttribs.ArraySize; ++ArrInd) { auto SrcOffset = SrcRes.CacheOffset + ArrInd; @@ -712,7 +717,7 @@ void ShaderResourceLayoutVk::InitializeStaticResources(const ShaderResourceLayou IDeviceObject *pCachedResource = m_pResourceCache->GetDescriptorSet(DstRes.DescriptorSet).GetResource(DstOffset).pObject; if(pCachedResource != pObject) { - VERIFY(pObject == nullptr, "Static resource has already been initialized, and the resource to be assigned from the shader does not match previously assigned resource"); + VERIFY(pCachedResource == nullptr, "Static resource has already been initialized, and the resource to be assigned from the shader does not match previously assigned resource"); DstRes.SetArray(&pObject, ArrInd, 1); } } @@ -735,20 +740,23 @@ void ShaderResourceLayoutVk::dbgVerifyBindings()const { auto &CachedDescrSet = m_pResourceCache->GetDescriptorSet(Res.DescriptorSet); const auto &CachedRes = CachedDescrSet.GetResource(Res.CacheOffset + ArrInd); - if(CachedRes.pObject == nullptr) + if(CachedRes.pObject == nullptr && + !(Res.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler && Res.SpirvAttribs.StaticSamplerInd >= 0)) { LOG_ERROR_MESSAGE("No resource is bound to ", GetShaderVariableTypeLiteralName(Res.SpirvAttribs.VarType), " variable \"", Res.SpirvAttribs.GetPrintName(ArrInd), "\" in shader \"", GetShaderName(), "\""); } - if(VarType == SHADER_VARIABLE_TYPE_DYNAMIC ) - { - if (CachedDescrSet.GetVkDescriptorSet() == VK_NULL_HANDLE) - LOG_ERROR_MESSAGE("Dynamic resources should not have Vulkan descriptor set in the cache"); - } +#ifdef _DEBUG + auto vkDescSet = CachedDescrSet.GetVkDescriptorSet(); + auto dbgCacheContentType = m_pResourceCache->DbgGetContentType(); + if(dbgCacheContentType == ShaderResourceCacheVk::DbgCacheContentType::StaticShaderResources) + VERIFY(vkDescSet == VK_NULL_HANDLE, "Static resource cache should never have vulkan descriptor set"); + else if (dbgCacheContentType == ShaderResourceCacheVk::DbgCacheContentType::SRBResources) + VERIFY(VarType == SHADER_VARIABLE_TYPE_DYNAMIC && vkDescSet == VK_NULL_HANDLE || + VarType != SHADER_VARIABLE_TYPE_DYNAMIC && vkDescSet != VK_NULL_HANDLE, + "Static and mutable resources (and only them) must have non-null vulkan descriptor set assigned"); else - { - if(CachedDescrSet.GetVkDescriptorSet() != VK_NULL_HANDLE) - LOG_ERROR_MESSAGE("Static and mutable resources must have non-null vulkan descriptor set assigned"); - } + UNEXPECTED("Unexpected cache content type"); +#endif } } } diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp index aeba9e1b..ae6c745d 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp @@ -74,10 +74,8 @@ void ShaderVkImpl::BindResources(IResourceMapping* pResourceMapping, Uint32 Flag IShaderVariable* ShaderVkImpl::GetShaderVariable(const Char* Name) { - auto *pVar = m_StaticResLayout.GetShaderVariable(Name); - if(pVar == nullptr) - pVar = &m_DummyShaderVar; - return pVar; + IShaderVariable *pVar = m_StaticResLayout.GetShaderVariable(Name); + return (pVar != nullptr) ? pVar : &m_DummyShaderVar; } #ifdef VERIFY_SHADER_BINDINGS -- cgit v1.2.3