Vk backend: optimized PrepareForDraw() by not calling BindDescriptorSetsWithDynamicOffsets if there are no dynamic buffers bound (fixed https://github.com/DiligentGraphics/DiligentCore/issues/106)

8 files changed, 112 insertions, 25 deletions

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<const DescriptorSet*>(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<bool VerifyOnly>
@@ -187,11 +191,17 @@ private:
     {
         return reinterpret_cast<Resource*>(reinterpret_cast<DescriptorSet*>(m_pMemory) + m_NumSets);
     }
+    const Resource* GetFirstResourcePtr()const
+    {
+        return reinterpret_cast<const Resource*>(reinterpret_cast<const DescriptorSet*>(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<typename TCacheSampler>
         void CacheImage(IDeviceObject*                   pTexView,
@@ -248,9 +251,10 @@ public:
                                   VkDescriptorSet                  vkDescrSet,
                                   Uint32                           ArrayInd)const;
 
-        template<typename ObjectType>
+        template<typename ObjectType, typename TPreUpdateObject>
         bool UpdateCachedResource(ShaderResourceCacheVk::Resource&   DstRes,
-                                  RefCntAutoPtr<ObjectType>&&        pObject)const;
+                                  RefCntAutoPtr<ObjectType>&&        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<decltype(m_NumSets)>::max(), "NumSets (", NumSets, ") exceed maximum representable value");
+    m_NumSets        = static_cast<Uint16>(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<const BufferVkImpl>()->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<const BufferViewVkImpl>()->GetBuffer<const BufferVkImpl>()->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<typename ObjectType>
+template<typename ObjectType, typename TPreUpdateObject>
 bool ShaderResourceLayoutVk::VkResource::UpdateCachedResource(ShaderResourceCacheVk::Resource& DstRes,
-                                                              RefCntAutoPtr<ObjectType>&&      pObject)const
+                                                              RefCntAutoPtr<ObjectType>&&      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<const ObjectType>(), pObject.RawPtr<const ObjectType>());
         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<BufferVkImpl> 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<const BufferVkImpl>()->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<const BufferVkImpl>()->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<const BufferVkImpl>()->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<const BufferVkImpl>()->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<TextureViewVkImpl>();
@@ -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: