Some refactoring of PipelineLayoutVk

3 files changed, 82 insertions, 52 deletions

diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineLayoutVk.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineLayoutVk.hpp
index d6b4c922..0e35f1ae 100644
--- a/Graphics/GraphicsEngineVulkan/include/PipelineLayoutVk.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/PipelineLayoutVk.hpp
@@ -33,8 +33,6 @@
 
 #include "PipelineResourceSignatureVkImpl.hpp"
 #include "VulkanUtilities/VulkanObjectWrappers.hpp"
-#include "VulkanUtilities/VulkanLogicalDevice.hpp"
-#include "VulkanUtilities/VulkanCommandBuffer.hpp"
 
 namespace Diligent
 {
@@ -47,6 +45,7 @@ class PipelineLayoutVk
 {
 public:
     PipelineLayoutVk();
+    ~PipelineLayoutVk();
 
     void Create(RenderDeviceVkImpl* pDeviceVk, IPipelineResourceSignature** ppSignatures, Uint32 SignatureCount);
     void Release(RenderDeviceVkImpl* pDeviceVkImpl, Uint64 CommandQueueMask);
@@ -64,16 +63,18 @@ public:
         return m_Signatures[index].RawPtr<PipelineResourceSignatureVkImpl>();
     }
 
-    Uint32 GetDescrSetIndex(const IPipelineResourceSignature* pPRS) const
+    // Returns the index of the first descriptor set used by the given resource signature
+    Uint32 GetFirstDescrSetIndex(const IPipelineResourceSignature* pPRS) const
     {
         Uint32 Index = pPRS->GetDesc().BindingIndex;
-        return Index < m_SignatureCount ? m_DescSetOffset[Index] : ~0u;
+        return Index < m_SignatureCount ? m_FirstDescrSetIndex[Index] : ~0u;
     }
 
-    Uint32 GetDynamicBufferOffset(const IPipelineResourceSignature* pPRS) const
+    // Returns the index of the first dynamic buffer used by the given resource signature
+    Uint32 GetFirstDynamicBufferIndex(const IPipelineResourceSignature* pPRS) const
     {
         Uint32 Index = pPRS->GetDesc().BindingIndex;
-        return Index < m_SignatureCount ? m_DynBufOffset[Index] : 0;
+        return Index < m_SignatureCount ? m_FirstDynBuffIndex[Index] : 0;
     }
 
     struct ResourceInfo
@@ -87,21 +88,28 @@ public:
 private:
     VulkanUtilities::PipelineLayoutWrapper m_VkPipelineLayout;
 
-    Uint32 m_DynamicOffsetCount : 25;
+    // The total number of dynamic offsets in this pipeline layout
+    Uint32 m_DynamicOffsetCount = 0;
 
-    Uint32 m_SignatureCount : 3;
-    static_assert(MAX_RESOURCE_SIGNATURES == (1 << 3), "Update m_SignatureCount bits count");
+    // The number of resource signatures used by this pipeline layout
+    // (Maximum is MAX_RESOURCE_SIGNATURES)
+    Uint16 m_SignatureCount = 0;
 
-    Uint32 m_DescrSetCount : 4;
-    static_assert(MAX_RESOURCE_SIGNATURES * 2 == (1 << 4), "Update m_DescrSetCount bits count");
+    // The total number of descriptor sets used by this pipeline layout.
+    // (Maximum is MAX_RESOURCE_SIGNATURES * 2)
+    Uint16 m_DescrSetCount = 0;
 
-    using SignatureArray     = std::array<RefCntAutoPtr<PipelineResourceSignatureVkImpl>, MAX_RESOURCE_SIGNATURES>;
-    using DescSetOffsetArray = std::array<Uint8, MAX_RESOURCE_SIGNATURES>;
-    using DynBufOffsetArray  = std::array<Uint16, MAX_RESOURCE_SIGNATURES>;
+    using SignatureArrayType          = std::array<RefCntAutoPtr<PipelineResourceSignatureVkImpl>, MAX_RESOURCE_SIGNATURES>;
+    using FirstDescrSetIndexArrayType = std::array<Uint8, MAX_RESOURCE_SIGNATURES>;
+    using FirstDynBuffIndexArrayType  = std::array<Uint16, MAX_RESOURCE_SIGNATURES>;
 
-    SignatureArray     m_Signatures;
-    DescSetOffsetArray m_DescSetOffset = {};
-    DynBufOffsetArray  m_DynBufOffset  = {};
+    SignatureArrayType m_Signatures;
+
+    // Index of the first descriptor set, for every resource signature
+    FirstDescrSetIndexArrayType m_FirstDescrSetIndex = {};
+
+    // Index of the first dynamic buffer, for every resource signature
+    FirstDynBuffIndexArrayType m_FirstDynBuffIndex = {};
 };
 
 } // namespace Diligent
diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
index 19bbd91a..603ce2b6 100644
--- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
@@ -416,7 +416,7 @@ void DeviceContextVkImpl::BindDescriptorSetsWithDynamicOffsets(DescriptorSetBind
         if ((BindInfo.PendingVkSet[i] || BindInfo.PendingDynamicDescriptors[i]) && Resources[i] != nullptr)
         {
             const auto*  pSignature    = Resources[i]->GetSignature();
-            const Uint32 DescrSetIdx   = Layout.GetDescrSetIndex(pSignature);
+            const Uint32 DescrSetIdx   = Layout.GetFirstDescrSetIndex(pSignature);
             const auto&  ResourceCache = Resources[i]->GetResourceCache();
             const Uint32 DSCount       = ResourceCache.GetNumDescriptorSets();
             const auto   DSOffset      = i * MAX_DESCR_SET_PER_SIGNATURE;
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp
index 376417ce..5f2a525c 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp
@@ -27,50 +27,57 @@
 
 #include "pch.h"
 
+#include <algorithm>
+#include <limits>
+
 #include "PipelineLayoutVk.hpp"
-#include "ShaderVkImpl.hpp"
 #include "RenderDeviceVkImpl.hpp"
-#include "DeviceContextVkImpl.hpp"
-#include "TextureVkImpl.hpp"
-#include "BufferVkImpl.hpp"
 #include "VulkanTypeConversions.hpp"
-#include "HashUtils.hpp"
 
 namespace Diligent
 {
 
-PipelineLayoutVk::PipelineLayoutVk() :
-    m_DynamicOffsetCount{0},
-    m_SignatureCount{0},
-    m_DescrSetCount{0}
+PipelineLayoutVk::PipelineLayoutVk()
 {
+    m_FirstDescrSetIndex.fill(std::numeric_limits<FirstDescrSetIndexArrayType::value_type>::max());
+    m_FirstDynBuffIndex.fill(std::numeric_limits<FirstDynBuffIndexArrayType::value_type>::max());
+}
+
+PipelineLayoutVk::~PipelineLayoutVk()
+{
+    VERIFY(!m_VkPipelineLayout, "Pipeline layout have not been released!");
 }
 
 void PipelineLayoutVk::Release(RenderDeviceVkImpl* pDeviceVk, Uint64 CommandQueueMask)
 {
-    pDeviceVk->SafeReleaseDeviceObject(std::move(m_VkPipelineLayout), CommandQueueMask);
+    if (m_VkPipelineLayout)
+    {
+        pDeviceVk->SafeReleaseDeviceObject(std::move(m_VkPipelineLayout), CommandQueueMask);
+    }
 }
 
 void PipelineLayoutVk::Create(RenderDeviceVkImpl* pDeviceVk, IPipelineResourceSignature** ppSignatures, Uint32 SignatureCount)
 {
     VERIFY(m_DynamicOffsetCount == 0 && m_SignatureCount == 0 && m_DescrSetCount == 0,
-           "pipeline layout is already initialized");
+           "This pipeline layout is already initialized");
 
     for (Uint32 i = 0; i < SignatureCount; ++i)
     {
         auto* pSignature = ValidatedCast<PipelineResourceSignatureVkImpl>(ppSignatures[i]);
-        if (!pSignature)
-            LOG_ERROR_AND_THROW("pipeline resource signature (", i, ") must not be null");
+        VERIFY(pSignature != nullptr, "Pipeline resource signature at index ", i, " is null.  This error should've been caught by ValidatePipelineResourceSignatures.");
 
         const Uint32 Index = pSignature->GetDesc().BindingIndex;
 
-        if (Index >= m_Signatures.size())
-            LOG_ERROR_AND_THROW("Pipeline resource signature '", pSignature->GetDesc().Name, "' index (", Uint32{Index}, ") must be less than (", m_Signatures.size(), ").");
+        VERIFY(Index < m_Signatures.size(),
+               "Pipeline resource signature specifies binding index ", Uint32{Index}, " that exceeds the limit (", m_Signatures.size() - 1,
+               "). This error should've been caught by ValidatePipelineResourceSignatureDesc.");
 
-        if (m_Signatures[Index] != nullptr)
-            LOG_ERROR_AND_THROW("Pipeline resource signature '", pSignature->GetDesc().Name, "' with index (", Uint32{Index}, ") overrides another resource signature '", m_Signatures[Index]->GetDesc().Name, "'.");
+        VERIFY(m_Signatures[Index] == nullptr,
+               "Pipeline resource signature '", pSignature->GetDesc().Name, "' at index ", Uint32{Index},
+               " conflicts with another resource signature '", m_Signatures[Index]->GetDesc().Name,
+               "' that uses the same index. This error should've been caught by ValidatePipelineResourceSignatureDesc.");
 
-        m_SignatureCount    = std::max(m_SignatureCount, Index + 1);
+        m_SignatureCount    = std::max(m_SignatureCount, static_cast<Uint16>(Index + 1));
         m_Signatures[Index] = pSignature;
     }
 
@@ -86,28 +93,31 @@ void PipelineLayoutVk::Create(RenderDeviceVkImpl* pDeviceVk, IPipelineResourceSi
 
     Uint32 DescSetLayoutCount = 0;
     Uint32 DynamicOffsetCount = 0;
-#ifdef DILIGENT_DEBUG
+
     Uint32 DynamicUniformBufferCount = 0;
     Uint32 DynamicStorageBufferCount = 0;
-#endif
 
     for (Uint32 i = 0; i < m_SignatureCount; ++i)
     {
         const auto& pSignature = m_Signatures[i];
         VERIFY_EXPR(pSignature != nullptr);
 
-        m_DescSetOffset[i] = static_cast<Uint8>(DescSetLayoutCount);
-        m_DynBufOffset[i]  = static_cast<Uint16>(DynamicOffsetCount);
+        VERIFY(DescSetLayoutCount <= std::numeric_limits<FirstDescrSetIndexArrayType::value_type>::max(),
+               "Descriptor set layout count (", DescSetLayoutCount, ") exceeds the maximum representable value");
+        m_FirstDescrSetIndex[i] = static_cast<FirstDescrSetIndexArrayType::value_type>(DescSetLayoutCount);
+
+        VERIFY(DescSetLayoutCount <= std::numeric_limits<FirstDynBuffIndexArrayType::value_type>::max(),
+               "Dynamic buffer count (", DynamicOffsetCount, ") exceeds the maximum representable value");
+        m_FirstDynBuffIndex[i] = static_cast<FirstDynBuffIndexArrayType::value_type>(DynamicOffsetCount);
 
         auto StaticDSLayout  = pSignature->GetStaticVkDescriptorSetLayout();
         auto DynamicDSLayout = pSignature->GetDynamicVkDescriptorSetLayout();
 
-        if (StaticDSLayout) DescSetLayouts[DescSetLayoutCount++] = StaticDSLayout;
-        if (DynamicDSLayout) DescSetLayouts[DescSetLayoutCount++] = DynamicDSLayout;
+        if (StaticDSLayout != VK_NULL_HANDLE) DescSetLayouts[DescSetLayoutCount++] = StaticDSLayout;
+        if (DynamicDSLayout != VK_NULL_HANDLE) DescSetLayouts[DescSetLayoutCount++] = DynamicDSLayout;
 
         DynamicOffsetCount += pSignature->GetDynamicBufferCount();
 
-#ifdef DILIGENT_DEBUG
         for (Uint32 r = 0, ResCount = pSignature->GetTotalResourceCount(); r < ResCount; ++r)
         {
             const auto& Attr = pSignature->GetAttribs(r);
@@ -122,8 +132,8 @@ void PipelineLayoutVk::Create(RenderDeviceVkImpl* pDeviceVk, IPipelineResourceSi
                 ++DynamicStorageBufferCount;
             }
         }
-#endif
     }
+    VERIFY_EXPR(DescSetLayoutCount <= MAX_RESOURCE_SIGNATURES * 2);
 
     VkPipelineLayoutCreateInfo PipelineLayoutCI = {};
 
@@ -137,15 +147,27 @@ void PipelineLayoutVk::Create(RenderDeviceVkImpl* pDeviceVk, IPipelineResourceSi
     m_VkPipelineLayout                      = pDeviceVk->GetLogicalDevice().CreatePipelineLayout(PipelineLayoutCI);
 
     const auto& Limits = pDeviceVk->GetPhysicalDevice().GetProperties().limits;
-    VERIFY_EXPR(DescSetLayoutCount <= Limits.maxBoundDescriptorSets);
-    VERIFY_EXPR(DescSetLayoutCount <= MAX_RESOURCE_SIGNATURES * 2);
+    if (DescSetLayoutCount > Limits.maxBoundDescriptorSets)
+    {
+        LOG_ERROR_AND_THROW("The total number of descriptor sets used by the pipeline layout (", DescSetLayoutCount,
+                            ") exceeds device limit (", Limits.maxBoundDescriptorSets, ")");
+    }
+
+    if (DynamicUniformBufferCount > Limits.maxDescriptorSetUniformBuffersDynamic)
+    {
+        LOG_ERROR_AND_THROW("The number of dynamic uniform buffers (", DynamicUniformBufferCount,
+                            ") exceeds device limit (", Limits.maxDescriptorSetUniformBuffersDynamic, ")");
+    }
 
-#ifdef DILIGENT_DEBUG
-    VERIFY_EXPR(DynamicUniformBufferCount <= Limits.maxDescriptorSetUniformBuffersDynamic);
-    VERIFY_EXPR(DynamicStorageBufferCount <= Limits.maxDescriptorSetStorageBuffersDynamic);
-#endif
+    if (DynamicStorageBufferCount > Limits.maxDescriptorSetStorageBuffersDynamic)
+    {
+        LOG_ERROR_AND_THROW("The number of dynamic storage buffers (", DynamicStorageBufferCount,
+                            ") exceeds device limit (", Limits.maxDescriptorSetStorageBuffersDynamic, ")");
+    }
 
-    m_DescrSetCount      = static_cast<Uint8>(DescSetLayoutCount);
+    VERIFY(m_DescrSetCount <= std::numeric_limits<decltype(m_DescrSetCount)>::max(),
+           "Descriptor set count (", DescSetLayoutCount, ") exceeds the maximum representable value");
+    m_DescrSetCount      = static_cast<decltype(m_DescrSetCount)>(DescSetLayoutCount);
     m_DynamicOffsetCount = DynamicOffsetCount;
 }
 
@@ -178,7 +200,7 @@ bool PipelineLayoutVk::GetResourceInfo(const char* Name, SHADER_TYPE Stage, Reso
             {
                 Info.Type          = Res.ResourceType;
                 Info.BindingIndex  = static_cast<Uint16>(Attr.BindingIndex);
-                Info.DescrSetIndex = m_DescSetOffset[i] + Attr.DescrSet;
+                Info.DescrSetIndex = m_FirstDescrSetIndex[i] + Attr.DescrSet;
                 return true;
             }
         }