Fixed issue with commtting dynamic resources (can't use resource cache as this will not work when multiple threads commit the same cache)

6 files changed, 62 insertions, 37 deletions

diff --git a/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h b/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
index 822768e0..c29dcad9 100644
--- a/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
+++ b/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
@@ -76,6 +76,11 @@ public:
         return *this;
     }
 
+    operator bool()const
+    {
+        return Set != VK_NULL_HANDLE;
+    }
+
     void Release();
 
     ~DescriptorPoolAllocation()
diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h
index 947f27dc..eafdf192 100644
--- a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h
+++ b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h
@@ -76,10 +76,10 @@ public:
         return m_LayoutMgr.GetHash();
     }
 
-    // Allocates Vulkan descriptor set for dynamic resources and assigns the
-    // set to the resource cache
-    void AllocateDynamicDescriptorSet(DeviceContextVkImpl*    pCtxVkImpl,
-                                      ShaderResourceCacheVk&  ResourceCache)const;
+    VkDescriptorSetLayout GetDynamicDescriptorSetVkLayout()const
+    {
+        return m_LayoutMgr.GetDescriptorSet(SHADER_VARIABLE_TYPE_DYNAMIC).VkLayout;
+    }
 
     struct DescriptorSetBindInfo
     {
@@ -122,7 +122,8 @@ public:
     void PrepareDescriptorSets(DeviceContextVkImpl*    pCtxVkImpl,
                                bool                    IsCompute,
                                ShaderResourceCacheVk&  ResourceCache,
-                               DescriptorSetBindInfo&  BindInfo)const;
+                               DescriptorSetBindInfo&  BindInfo,
+                               VkDescriptorSet         VkDynamicDescrSet)const;
 
     // Computes dynamic offsets and binds descriptor sets
     void BindDescriptorSetsWithDynamicOffsets(DeviceContextVkImpl*    pCtxVkImpl,
@@ -196,7 +197,7 @@ private:
         uint8_t m_ActiveSets = 0;
     };
 
-    IMemoryAllocator &m_MemAllocator;
+    IMemoryAllocator&          m_MemAllocator;
     DescriptorSetLayoutManager m_LayoutMgr;
 };
 
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h
index 3f6e335e..17b21d16 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h
@@ -234,9 +234,9 @@ public:
     // Initializes resource slots in the ResourceCache
     void InitializeResourceMemoryInCache(ShaderResourceCacheVk& ResourceCache)const;
     
-    // Updates dynamic resource descriptors in the ResourceCache. The descriptor set is assigned 
-    // to the resource cache by PipelineLayout::AllocateDynamicDescriptorSet().
-    void CommitDynamicResources(const ShaderResourceCacheVk& ResourceCache)const;
+    // Writes dynamic resource descriptors from ResourceCache to vkDynamicDescriptorSet
+    void CommitDynamicResources(const ShaderResourceCacheVk& ResourceCache,
+                                VkDescriptorSet              vkDynamicDescriptorSet)const;
 
     const Char* GetShaderName()const;
 
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
index e3d88771..e9098c6e 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
@@ -413,22 +413,11 @@ void PipelineLayout::InitResourceCache(RenderDeviceVkImpl *pDeviceVkImpl, Shader
     }
 }
 
-void PipelineLayout::AllocateDynamicDescriptorSet(DeviceContextVkImpl*    pCtxVkImpl,
-                                                  ShaderResourceCacheVk&  ResourceCache)const
-{
-    const auto &DynSet = m_LayoutMgr.GetDescriptorSet(SHADER_VARIABLE_TYPE_DYNAMIC);
-    if (DynSet.SetIndex >= 0)
-    {
-        auto DynamicSetAllocation = pCtxVkImpl->AllocateDynamicDescriptorSet(DynSet.VkLayout);
-        auto &DynamicSetCache = ResourceCache.GetDescriptorSet(DynSet.SetIndex);
-        DynamicSetCache.AssignDescriptorSetAllocation(std::move(DynamicSetAllocation));
-    }
-}
-
 void PipelineLayout::PrepareDescriptorSets(DeviceContextVkImpl*    pCtxVkImpl,
                                            bool                    IsCompute,
                                            ShaderResourceCacheVk&  ResourceCache,
-                                           DescriptorSetBindInfo&  BindInfo)const
+                                           DescriptorSetBindInfo&  BindInfo,
+                                           VkDescriptorSet         VkDynamicDescrSet)const
 {
 #ifdef _DEBUG
     BindInfo.vkSets.clear();
@@ -450,7 +439,13 @@ void PipelineLayout::PrepareDescriptorSets(DeviceContextVkImpl*    pCtxVkImpl,
             if(BindInfo.SetCout > BindInfo.vkSets.size())
                 BindInfo.vkSets.resize(BindInfo.SetCout);
             VERIFY_EXPR(BindInfo.vkSets[Set.SetIndex] == VK_NULL_HANDLE);
-            BindInfo.vkSets[Set.SetIndex] = ResourceCache.GetDescriptorSet(Set.SetIndex).GetVkDescriptorSet();
+            if(VarType == SHADER_VARIABLE_TYPE_MUTABLE)
+                BindInfo.vkSets[Set.SetIndex] = ResourceCache.GetDescriptorSet(Set.SetIndex).GetVkDescriptorSet();
+            else
+            {
+                VERIFY_EXPR(ResourceCache.GetDescriptorSet(Set.SetIndex).GetVkDescriptorSet() == VK_NULL_HANDLE);
+                BindInfo.vkSets[Set.SetIndex] = VkDynamicDescrSet;
+            }
             VERIFY(BindInfo.vkSets[Set.SetIndex] != VK_NULL_HANDLE, "Descriptor set must not be null");
         }
         TotalDynamicDescriptors += Set.NumDynamicDescriptors;
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
index 9f36dcb1..ad2b497d 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
@@ -481,7 +481,7 @@ void PipelineStateVkImpl::CommitAndTransitionShaderResources(IShaderResourceBind
                                                              DeviceContextVkImpl*                   pCtxVkImpl,
                                                              bool                                   CommitResources,
                                                              bool                                   TransitionResources,
-                                                            PipelineLayout::DescriptorSetBindInfo*  pDescrSetBindInfo)const
+                                                             PipelineLayout::DescriptorSetBindInfo* pDescrSetBindInfo)const
 {
     if(!m_HasStaticResources && !m_HasNonStaticResources)
         return;
@@ -543,17 +543,27 @@ void PipelineStateVkImpl::CommitAndTransitionShaderResources(IShaderResourceBind
             ResourceCache.TransitionResources<true>(pCtxVkImpl);
 #endif
         }
-    
-        // Allocate vulkan descriptor set for dynamic resources
-        m_PipelineLayout.AllocateDynamicDescriptorSet(pCtxVkImpl, ResourceCache);
-        // Commit all dynamic resource descriptors
-        for(Uint32 s=0; s < m_NumShaders; ++s)
+
+        DescriptorPoolAllocation DynamicDescrSetAllocation;
+        auto DynamicDescriptorSetVkLayout = m_PipelineLayout.GetDynamicDescriptorSetVkLayout();
+        if(DynamicDescriptorSetVkLayout != VK_NULL_HANDLE)
         {
-            m_ShaderResourceLayouts[s].CommitDynamicResources(ResourceCache);
+            // Allocate vulkan descriptor set for dynamic resources
+            DynamicDescrSetAllocation = pCtxVkImpl->AllocateDynamicDescriptorSet(DynamicDescriptorSetVkLayout);
+            // Commit all dynamic resource descriptors
+            for(Uint32 s=0; s < m_NumShaders; ++s)
+            {
+                const auto &Layout = m_ShaderResourceLayouts[s];
+                if(Layout.GetResourceCount(SHADER_VARIABLE_TYPE_DYNAMIC) != 0)
+                    Layout.CommitDynamicResources(ResourceCache, DynamicDescrSetAllocation.GetVkDescriptorSet());
+            }
         }
         // Prepare descriptor sets, and also bind them if there are no dynamic descriptors
         VERIFY_EXPR(pDescrSetBindInfo != nullptr);
-        m_PipelineLayout.PrepareDescriptorSets(pCtxVkImpl, m_Desc.IsComputePipeline, ResourceCache, *pDescrSetBindInfo);
+        m_PipelineLayout.PrepareDescriptorSets(pCtxVkImpl, m_Desc.IsComputePipeline, ResourceCache, *pDescrSetBindInfo, DynamicDescrSetAllocation.GetVkDescriptorSet());
+        // Dynamic descriptor set allocation automatically goes back to the context's dynamic descriptor pool. 
+        // release queue. It will stay there until the next command list is executed, at which point it will be discarded
+        // and actually released later
     }
     else
     {
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
index f98e7b16..1696e4a7 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
@@ -674,7 +674,10 @@ void ShaderResourceLayoutVk::dbgVerifyBindings(const ShaderResourceCacheVk& Reso
                     if (VarType == SHADER_VARIABLE_TYPE_STATIC || VarType == SHADER_VARIABLE_TYPE_MUTABLE)
                     {
                         VERIFY(vkDescSet != VK_NULL_HANDLE, "Static and mutable variables must have valid vulkan descriptor set assigned");
-                        // Dynamic variables do not have vulkan descriptor set only until they are assigned one the first time
+                    }
+                    else if (VarType == SHADER_VARIABLE_TYPE_DYNAMIC )
+                    {
+                        VERIFY(vkDescSet == VK_NULL_HANDLE, "Dynamic variables must not be assigned a vulkan descriptor set");
                     }
                 }
                 else
@@ -728,11 +731,12 @@ void ShaderResourceLayoutVk::InitializeResourceMemoryInCache(ShaderResourceCache
     }
 }
 
-void ShaderResourceLayoutVk::CommitDynamicResources(const ShaderResourceCacheVk& ResourceCache)const
+void ShaderResourceLayoutVk::CommitDynamicResources(const ShaderResourceCacheVk& ResourceCache, 
+                                                    VkDescriptorSet              vkDynamicDescriptorSet)const
 {
     Uint32 NumDynamicResources = m_NumResources[SHADER_VARIABLE_TYPE_DYNAMIC];
-    if(NumDynamicResources == 0)
-        return;
+    VERIFY(NumDynamicResources != 0, "This shader resource layout does not contain dynamic resources");
+    VERIFY_EXPR(vkDynamicDescriptorSet != VK_NULL_HANDLE);
 
 #ifdef _DEBUG
     static constexpr size_t ImgUpdateBatchSize = 4;
@@ -758,15 +762,25 @@ void ShaderResourceLayoutVk::CommitDynamicResources(const ShaderResourceCacheVk&
     auto BuffViewIt  = DescrBuffViewArr.begin();
     auto WriteDescrSetIt = WriteDescrSetArr.begin();
 
+#ifdef _DEBUG
+    Int32 DynamicDescrSetIndex = -1;
+#endif
+
     while(ResNum < NumDynamicResources)
     {
         const auto& Res = GetResource(SHADER_VARIABLE_TYPE_DYNAMIC, ResNum);
         VERIFY_EXPR(Res.SpirvAttribs.VarType == SHADER_VARIABLE_TYPE_DYNAMIC);
+#ifdef _DEBUG
+        if(DynamicDescrSetIndex < 0)
+            DynamicDescrSetIndex = Res.DescriptorSet;
+        else
+            VERIFY(DynamicDescrSetIndex == Res.DescriptorSet, "Inconsistent dynamic resource desriptor set index");
+#endif
         auto& SetResources = ResourceCache.GetDescriptorSet(Res.DescriptorSet);
-
         WriteDescrSetIt->sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;
         WriteDescrSetIt->pNext = nullptr;
-        WriteDescrSetIt->dstSet = SetResources.GetVkDescriptorSet();
+        VERIFY(SetResources.GetVkDescriptorSet() == VK_NULL_HANDLE, "Dynamic descriptor set must not be assigned to the resource cache");
+        WriteDescrSetIt->dstSet = vkDynamicDescriptorSet;
         VERIFY(WriteDescrSetIt->dstSet != VK_NULL_HANDLE, "Vulkan descriptor set must not be null");
         WriteDescrSetIt->dstBinding = Res.Binding;
         WriteDescrSetIt->dstArrayElement = ArrElem;