From 59cb4761f10c0639f9e041088ba46cb63a402c4c Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Wed, 23 Oct 2019 23:03:45 -0700 Subject: Vk backend: optimized PrepareForDraw() by not calling BindDescriptorSetsWithDynamicOffsets if there are no dynamic buffers bound (fixed https://github.com/DiligentGraphics/DiligentCore/issues/106) --- .../GraphicsEngineVulkan/include/PipelineLayout.h | 1 + .../include/ShaderResourceCacheVk.h | 20 +++++-- .../include/ShaderResourceLayoutVk.h | 14 +++-- .../src/DeviceContextVkImpl.cpp | 6 ++- .../GraphicsEngineVulkan/src/PipelineLayout.cpp | 1 + .../src/PipelineStateVkImpl.cpp | 6 +++ .../src/ShaderResourceCacheVk.cpp | 26 ++++++++- .../src/ShaderResourceLayoutVk.cpp | 63 +++++++++++++++++----- 8 files changed, 112 insertions(+), 25 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h index e34ee108..5198e293 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h +++ b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h @@ -94,6 +94,7 @@ public: VkPipelineBindPoint BindPoint = VK_PIPELINE_BIND_POINT_MAX_ENUM; Uint32 SetCout = 0; Uint32 DynamicOffsetCount = 0; + bool DynamicBuffersBound = false; bool DynamicDescriptorsBound = false; #ifdef _DEBUG const PipelineLayout* pDbgPipelineLayout = nullptr; diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h index 201b8148..2ec63afe 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h @@ -168,12 +168,16 @@ public: return reinterpret_cast(m_pMemory)[Index]; } - inline Uint32 GetNumDescriptorSets()const{return m_NumSets; } + inline Uint32 GetNumDescriptorSets()const{return m_NumSets; } + inline Uint32 GetNumDynamicBuffers()const{return m_NumDynamicBuffers;} + + Uint16& GetDynamicBuffersCounter(){return m_NumDynamicBuffers;} #ifdef _DEBUG // Only for debug purposes: indicates what types of resources are stored in the cache DbgCacheContentType DbgGetContentType()const{return m_DbgContentType;} void DbgVerifyResourceInitialization()const; + void DbgVerifyDynamicBuffersCounter()const; #endif template @@ -187,11 +191,17 @@ private: { return reinterpret_cast(reinterpret_cast(m_pMemory) + m_NumSets); } + const Resource* GetFirstResourcePtr()const + { + return reinterpret_cast(reinterpret_cast(m_pMemory) + m_NumSets); + } - IMemoryAllocator* m_pAllocator = nullptr; - void* m_pMemory = nullptr; - Uint32 m_NumSets = 0; - Uint32 m_TotalResources = 0; + IMemoryAllocator* m_pAllocator = nullptr; + void* m_pMemory = nullptr; + Uint16 m_NumSets = 0; + // Total number of dynamic buffers bound in the resource cache regardless of the variable type + Uint16 m_NumDynamicBuffers = 0; + Uint32 m_TotalResources = 0; #ifdef _DEBUG // Only for debug purposes: indicates what types of resources are stored in the cache diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h index 8032d396..951e6bfa 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h @@ -224,17 +224,20 @@ public: void CacheUniformBuffer(IDeviceObject* pBuffer, ShaderResourceCacheVk::Resource& DstRes, VkDescriptorSet vkDescrSet, - Uint32 ArrayInd)const; + Uint32 ArrayInd, + Uint16& DynamicBuffersCounter)const; void CacheStorageBuffer(IDeviceObject* pBufferView, ShaderResourceCacheVk::Resource& DstRes, VkDescriptorSet vkDescrSet, - Uint32 ArrayInd)const; + Uint32 ArrayInd, + Uint16& DynamicBuffersCounter)const; void CacheTexelBuffer(IDeviceObject* pBufferView, ShaderResourceCacheVk::Resource& DstRes, VkDescriptorSet vkDescrSet, - Uint32 ArrayInd)const; + Uint32 ArrayInd, + Uint16& DynamicBuffersCounter)const; template void CacheImage(IDeviceObject* pTexView, @@ -248,9 +251,10 @@ public: VkDescriptorSet vkDescrSet, Uint32 ArrayInd)const; - template + template bool UpdateCachedResource(ShaderResourceCacheVk::Resource& DstRes, - RefCntAutoPtr&& pObject)const; + RefCntAutoPtr&& pObject, + TPreUpdateObject PreUpdateObject)const; }; // Copies static resources from SrcResourceCache defined by SrcLayout diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp index 7b6d050c..ebbb3dd4 100644 --- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp @@ -422,7 +422,11 @@ namespace Diligent if (m_DescrSetBindInfo.DynamicOffsetCount != 0) { - if (!m_DescrSetBindInfo.DynamicDescriptorsBound || (Flags & DRAW_FLAG_DYNAMIC_RESOURCE_BUFFERS_INTACT) == 0) + // First time we must always bind descriptor sets with dynamic offsets. + // If there are no dynamic buffers bound in the resource cache, for all subsequent + // cals we do not need to bind the sets again. + if (!m_DescrSetBindInfo.DynamicDescriptorsBound || + (m_DescrSetBindInfo.DynamicBuffersBound && (Flags & DRAW_FLAG_DYNAMIC_RESOURCE_BUFFERS_INTACT) == 0)) { m_pPipelineState->BindDescriptorSetsWithDynamicOffsets(GetCommandBuffer(), m_ContextId, this, m_DescrSetBindInfo); } diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp index b3b97ec6..d91a860b 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp @@ -486,6 +486,7 @@ void PipelineLayout::PrepareDescriptorSets(DeviceContextVkImpl* pCtxVkI #ifdef _DEBUG BindInfo.pDbgPipelineLayout = this; #endif + BindInfo.DynamicBuffersBound = ResourceCache.GetNumDynamicBuffers() > 0; if (TotalDynamicDescriptors == 0) { diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp index 3e9a64ee..41ff3a9e 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp @@ -573,6 +573,9 @@ void PipelineStateVkImpl::CommitAndTransitionShaderResources(IShaderResourceBind m_ShaderResourceLayouts[s].dvpVerifyBindings(ResourceCache); } #endif +#ifdef _DEBUG + ResourceCache.DbgVerifyDynamicBuffersCounter(); +#endif if (StateTransitionMode == RESOURCE_STATE_TRANSITION_MODE_TRANSITION) { @@ -678,6 +681,9 @@ void PipelineStateVkImpl::InitializeStaticSRBResources(ShaderResourceCacheVk& Re const auto& ShaderResourceLayouts = GetShaderResLayout(s); ShaderResourceLayouts.InitializeStaticResources(StaticResLayout, StaticResCache, ResourceCache); } +#ifdef _DEBUG + ResourceCache.DbgVerifyDynamicBuffersCounter(); +#endif } } diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp index e51603c8..c899d706 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp @@ -57,7 +57,8 @@ void ShaderResourceCacheVk::InitializeSets(IMemoryAllocator& MemAllocator, Uint3 VERIFY(m_pAllocator == nullptr && m_pMemory == nullptr, "Cache already initialized"); m_pAllocator = &MemAllocator; - m_NumSets = NumSets; + VERIFY(NumSets < std::numeric_limits::max(), "NumSets (", NumSets, ") exceed maximum representable value"); + m_NumSets = static_cast(NumSets); m_TotalResources = 0; for (Uint32 t=0; t < NumSets; ++t) m_TotalResources += SetSizes[t]; @@ -104,6 +105,29 @@ void ShaderResourceCacheVk::DbgVerifyResourceInitialization()const VERIFY(ResInitialized, "Not all resources in the cache have been initialized. This is a bug."); } } +void ShaderResourceCacheVk::DbgVerifyDynamicBuffersCounter()const +{ + const auto* pResources = GetFirstResourcePtr(); + Uint32 NumDynamicBuffers = 0; + for (Uint32 res=0; res < m_TotalResources; ++res) + { + const auto& Res = pResources[res]; + if (Res.Type == SPIRVShaderResourceAttribs::ResourceType::UniformBuffer) + { + if (Res.pObject && Res.pObject.RawPtr()->GetDesc().Usage == USAGE_DYNAMIC) + ++NumDynamicBuffers; + } + else if (Res.Type == SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer || + Res.Type == SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer || + Res.Type == SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer || + Res.Type == SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer) + { + if (Res.pObject && Res.pObject.RawPtr()->GetBuffer()->GetDesc().Usage == USAGE_DYNAMIC) + ++NumDynamicBuffers; + } + } + VERIFY(NumDynamicBuffers == m_NumDynamicBuffers, "The number of dynamic buffers (", m_NumDynamicBuffers, ") does not match the actual number (", NumDynamicBuffers, ")"); +} #endif ShaderResourceCacheVk::~ShaderResourceCacheVk() diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp index 0393bcb7..15d3647a 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp @@ -570,9 +570,10 @@ void ShaderResourceLayoutVk::VkResource::UpdateDescriptorHandle(VkDescriptorSet ParentResLayout.m_LogicalDevice.UpdateDescriptorSets(1, &WriteDescrSet, 0, nullptr); } -template +template bool ShaderResourceLayoutVk::VkResource::UpdateCachedResource(ShaderResourceCacheVk::Resource& DstRes, - RefCntAutoPtr&& pObject)const + RefCntAutoPtr&& pObject, + TPreUpdateObject PreUpdateObject)const { // We cannot use ValidatedCast<> here as the resource retrieved from the // resource mapping can be of wrong type @@ -585,6 +586,7 @@ bool ShaderResourceLayoutVk::VkResource::UpdateCachedResource(ShaderResourceCach return false; } + PreUpdateObject(DstRes.pObject.RawPtr(), pObject.RawPtr()); DstRes.pObject.Attach(pObject.Detach()); return true; } @@ -597,7 +599,8 @@ bool ShaderResourceLayoutVk::VkResource::UpdateCachedResource(ShaderResourceCach void ShaderResourceLayoutVk::VkResource::CacheUniformBuffer(IDeviceObject* pBuffer, ShaderResourceCacheVk::Resource& DstRes, VkDescriptorSet vkDescrSet, - Uint32 ArrayInd)const + Uint32 ArrayInd, + Uint16& DynamicBuffersCounter)const { VERIFY(SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::UniformBuffer, "Uniform buffer resource is expected"); RefCntAutoPtr pBufferVk(pBuffer, IID_BufferVk); @@ -605,7 +608,17 @@ void ShaderResourceLayoutVk::VkResource::CacheUniformBuffer(IDeviceObject* VerifyConstantBufferBinding(SpirvAttribs, GetVariableType(), ArrayInd, pBuffer, pBufferVk.RawPtr(), DstRes.pObject.RawPtr(), ParentResLayout.GetShaderName()); #endif - if (UpdateCachedResource(DstRes, std::move(pBufferVk))) + auto UpdateDynamicBuffersCounter = [&DynamicBuffersCounter](const BufferVkImpl* pOldBuffer, const BufferVkImpl* pNewBuffer) + { + if (pOldBuffer != nullptr && pOldBuffer->GetDesc().Usage == USAGE_DYNAMIC) + { + VERIFY(DynamicBuffersCounter > 0, "Dynamic buffers counter must be greater than zero when there is at least one dynamic buffer bound in the resource cache"); + --DynamicBuffersCounter; + } + if (pNewBuffer != nullptr && pNewBuffer->GetDesc().Usage == USAGE_DYNAMIC) + ++DynamicBuffersCounter; + }; + if (UpdateCachedResource(DstRes, std::move(pBufferVk), UpdateDynamicBuffersCounter)) { // VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER or VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC descriptor type require // buffer to be created with VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT @@ -623,7 +636,8 @@ void ShaderResourceLayoutVk::VkResource::CacheUniformBuffer(IDeviceObject* void ShaderResourceLayoutVk::VkResource::CacheStorageBuffer(IDeviceObject* pBufferView, ShaderResourceCacheVk::Resource& DstRes, VkDescriptorSet vkDescrSet, - Uint32 ArrayInd)const + Uint32 ArrayInd, + Uint16& DynamicBuffersCounter)const { VERIFY(SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer || SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer, @@ -638,7 +652,18 @@ void ShaderResourceLayoutVk::VkResource::CacheStorageBuffer(IDeviceObject* } #endif - if (UpdateCachedResource(DstRes, std::move(pBufferViewVk))) + auto UpdateDynamicBuffersCounter = [&DynamicBuffersCounter](const BufferViewVkImpl* pOldBufferView, const BufferViewVkImpl* pNewBufferView) + { + if (pOldBufferView != nullptr && pOldBufferView->GetBuffer()->GetDesc().Usage == USAGE_DYNAMIC) + { + VERIFY(DynamicBuffersCounter > 0, "Dynamic buffers counter must be greater than zero when there is at least one dynamic buffer bound in the resource cache"); + --DynamicBuffersCounter; + } + if (pNewBufferView != nullptr && pNewBufferView->GetBuffer()->GetDesc().Usage == USAGE_DYNAMIC) + ++DynamicBuffersCounter; + }; + + if (UpdateCachedResource(DstRes, std::move(pBufferViewVk), UpdateDynamicBuffersCounter)) { // VK_DESCRIPTOR_TYPE_STORAGE_BUFFER or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC descriptor type // require buffer to be created with VK_BUFFER_USAGE_STORAGE_BUFFER_BIT (13.2.4) @@ -656,7 +681,8 @@ void ShaderResourceLayoutVk::VkResource::CacheStorageBuffer(IDeviceObject* void ShaderResourceLayoutVk::VkResource::CacheTexelBuffer(IDeviceObject* pBufferView, ShaderResourceCacheVk::Resource& DstRes, VkDescriptorSet vkDescrSet, - Uint32 ArrayInd)const + Uint32 ArrayInd, + Uint16& DynamicBuffersCounter)const { VERIFY(SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer || SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer, @@ -671,7 +697,18 @@ void ShaderResourceLayoutVk::VkResource::CacheTexelBuffer(IDeviceObject* } #endif - if (UpdateCachedResource(DstRes, std::move(pBufferViewVk))) + auto UpdateDynamicBuffersCounter = [&DynamicBuffersCounter](const BufferViewVkImpl* pOldBufferView, const BufferViewVkImpl* pNewBufferView) + { + if (pOldBufferView != nullptr && pOldBufferView->GetBuffer()->GetDesc().Usage == USAGE_DYNAMIC) + { + VERIFY(DynamicBuffersCounter > 0, "Dynamic buffers counter must be greater than zero when there is at least one dynamic buffer bound in the resource cache"); + --DynamicBuffersCounter; + } + if (pNewBufferView != nullptr && pNewBufferView->GetBuffer()->GetDesc().Usage == USAGE_DYNAMIC) + ++DynamicBuffersCounter; + }; + + if (UpdateCachedResource(DstRes, std::move(pBufferViewVk), UpdateDynamicBuffersCounter)) { // The following bits must have been set at buffer creation time: // * VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER -> VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT @@ -707,7 +744,7 @@ void ShaderResourceLayoutVk::VkResource::CacheImage(IDeviceObject* VerifyResourceViewBinding(SpirvAttribs, GetVariableType(), ArrayInd, pTexView, pTexViewVk0.RawPtr(), {RequiredViewType}, DstRes.pObject.RawPtr(), ParentResLayout.GetShaderName()); } #endif - if (UpdateCachedResource(DstRes, std::move(pTexViewVk0))) + if (UpdateCachedResource(DstRes, std::move(pTexViewVk0), [](const TextureViewVkImpl*, const TextureViewVkImpl*){})) { // We can do RawPtr here safely since UpdateCachedResource() returned true auto* pTexViewVk = DstRes.pObject.RawPtr(); @@ -778,7 +815,7 @@ void ShaderResourceLayoutVk::VkResource::CacheSeparateSampler(IDeviceObject* "cause unpredicted behavior. Use another shader resource binding instance or label the variable as dynamic."); } #endif - if (UpdateCachedResource(DstRes, std::move(pSamplerVk))) + if (UpdateCachedResource(DstRes, std::move(pSamplerVk), [](const SamplerVkImpl*, const SamplerVkImpl*){})) { // Do not update descriptor for a dynamic sampler. All dynamic resource descriptors // are updated at once by CommitDynamicResources() when SRB is committed. @@ -823,17 +860,17 @@ void ShaderResourceLayoutVk::VkResource::BindResource(IDeviceObject* pObj, Uint3 switch (SpirvAttribs.Type) { case SPIRVShaderResourceAttribs::ResourceType::UniformBuffer: - CacheUniformBuffer(pObj, DstRes, vkDescrSet, ArrayIndex); + CacheUniformBuffer(pObj, DstRes, vkDescrSet, ArrayIndex, ResourceCache.GetDynamicBuffersCounter()); break; case SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer: case SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer: - CacheStorageBuffer(pObj, DstRes, vkDescrSet, ArrayIndex); + CacheStorageBuffer(pObj, DstRes, vkDescrSet, ArrayIndex, ResourceCache.GetDynamicBuffersCounter()); break; case SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer: case SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer: - CacheTexelBuffer(pObj, DstRes, vkDescrSet, ArrayIndex); + CacheTexelBuffer(pObj, DstRes, vkDescrSet, ArrayIndex, ResourceCache.GetDynamicBuffersCounter()); break; case SPIRVShaderResourceAttribs::ResourceType::StorageImage: -- cgit v1.2.3