Improved reporting of resource binding problems in InitializeStaticResources()

13 files changed, 51 insertions, 24 deletions

diff --git a/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h b/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
index 6749b697..16717d27 100644
--- a/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
+++ b/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
@@ -198,7 +198,7 @@ public:
 
     ~DescriptorSetAllocator();
 
-    DescriptorSetAllocation Allocate(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout);
+    DescriptorSetAllocation Allocate(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout, const char* DebugName = "");
 
 #ifdef DEVELOPMENT
     int32_t GetAllocatedDescriptorSetCounter()const{return m_AllocatedSetCounter;}
diff --git a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h
index 0dc9371a..23d10485 100644
--- a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h
+++ b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h
@@ -184,11 +184,11 @@ public:
 
     virtual void TransitionResourceStates(Uint32 BarrierCount, StateTransitionDesc* pResourceBarriers)override final;
 
-    VkDescriptorSet AllocateDynamicDescriptorSet(VkDescriptorSetLayout SetLayout)
+    VkDescriptorSet AllocateDynamicDescriptorSet(VkDescriptorSetLayout SetLayout, const char* DebugName = "")
     {
         // Descriptor pools are externally synchronized, meaning that the application must not allocate 
         // and/or free descriptor sets from the same pool in multiple threads simultaneously (13.2.3)
-        return m_DynamicDescrSetAllocator.Allocate(SetLayout, "");
+        return m_DynamicDescrSetAllocator.Allocate(SetLayout, DebugName);
     }
 
     VulkanDynamicAllocation AllocateDynamicSpace(Uint32 SizeInBytes, Uint32 Alignment);
diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h
index fbe0ea20..a45146cf 100644
--- a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h
+++ b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h
@@ -38,13 +38,13 @@ namespace Diligent
 
 class RenderDeviceVkImpl;
 class DeviceContextVkImpl;
-
+class ShaderResourceCacheVk;
 
 /// Implementation of the Diligent::PipelineLayout class
 class PipelineLayout
 {
 public:
-    static VkDescriptorType GetVkDescriptorType(const SPIRVShaderResourceAttribs &Res);
+    static VkDescriptorType GetVkDescriptorType(const SPIRVShaderResourceAttribs& Res);
 
     PipelineLayout();
     void Release(RenderDeviceVkImpl* pDeviceVkImpl, Uint64 CommandQueueMask);
@@ -52,7 +52,10 @@ public:
 
     VkPipelineLayout GetVkPipelineLayout()const{return m_LayoutMgr.GetVkPipelineLayout();}
     std::array<Uint32, 2> GetDescriptorSetSizes(Uint32& NumSets)const;
-    void InitResourceCache(RenderDeviceVkImpl* pDeviceVkImpl, class ShaderResourceCacheVk& ResourceCache, IMemoryAllocator& CacheMemAllocator)const;
+    void InitResourceCache(RenderDeviceVkImpl*    pDeviceVkImpl,
+                           ShaderResourceCacheVk& ResourceCache,
+                           IMemoryAllocator&      CacheMemAllocator,
+                           const char*            DbgPipelineName)const;
 
     void AllocateResourceSlot(const SPIRVShaderResourceAttribs& ResAttribs, 
                               VkSampler                         vkStaticSampler,
diff --git a/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h b/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
index da87afd5..22ec6e15 100644
--- a/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
+++ b/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
@@ -99,9 +99,9 @@ public:
 
     virtual void ReleaseStaleResources(bool ForceRelease = false)override final;
 
-    DescriptorSetAllocation AllocateDescriptorSet(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout)
+    DescriptorSetAllocation AllocateDescriptorSet(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout, const char* DebugName = "")
     {
-        return m_DescriptorSetAllocator.Allocate(CommandQueueMask, SetLayout);
+        return m_DescriptorSetAllocator.Allocate(CommandQueueMask, SetLayout, DebugName);
     }
     DescriptorPoolManager& GetDynamicDescriptorPool(){return m_DynamicDescriptorPool;}
 
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h
index c53d4d1f..e72c867e 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h
@@ -230,7 +230,7 @@ public:
                                    ShaderResourceCacheVk&        DstResourceCache)const;
 
 #ifdef DEVELOPMENT
-    void dvpVerifyBindings(const ShaderResourceCacheVk& ResourceCache)const;
+    bool dvpVerifyBindings(const ShaderResourceCacheVk& ResourceCache)const;
 #endif
 
     Uint32 GetResourceCount(SHADER_VARIABLE_TYPE VarType)const
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h b/Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h
index ca0373bf..0c45c33d 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h
@@ -83,7 +83,7 @@ public:
     const char* GetEntryPoint() const { return m_EntryPoint.c_str(); }
 
 #ifdef DEVELOPMENT
-    void DvpVerifyStaticResourceBindings()const;
+    bool DvpVerifyStaticResourceBindings()const;
 #endif
     
 private:
diff --git a/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp b/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp
index cd2cb7a1..82c4d0cf 100644
--- a/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp
@@ -149,7 +149,7 @@ DescriptorSetAllocator::~DescriptorSetAllocator()
     DEV_CHECK_ERR(m_AllocatedSetCounter == 0, m_AllocatedSetCounter, " descriptor set(s) have not been returned to the allocator. If there are outstanding references to the sets in release queues, the app will crash when DescriptorSetAllocator::FreeDescriptorSet() is called");
 }
 
-DescriptorSetAllocation DescriptorSetAllocator::Allocate(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout)
+DescriptorSetAllocation DescriptorSetAllocator::Allocate(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout, const char* DebugName)
 {
     // Descriptor pools are externally synchronized, meaning that the application must not allocate 
     // and/or free descriptor sets from the same pool in multiple threads simultaneously (13.2.3)
@@ -160,7 +160,7 @@ DescriptorSetAllocation DescriptorSetAllocator::Allocate(Uint64 CommandQueueMask
     for(auto it = m_Pools.begin(); it != m_Pools.end(); ++it)
     {
         auto& Pool = *it;
-        auto Set = AllocateDescriptorSet(LogicalDevice, Pool, SetLayout, "Descriptor set");
+        auto Set = AllocateDescriptorSet(LogicalDevice, Pool, SetLayout, DebugName);
         if (Set != VK_NULL_HANDLE)
         {
             // Move the pool to the front
@@ -181,7 +181,7 @@ DescriptorSetAllocation DescriptorSetAllocator::Allocate(Uint64 CommandQueueMask
     m_Pools.emplace_front(CreateDescriptorPool("Descriptor pool"));
 
     auto& NewPool = m_Pools.front();
-    auto Set = AllocateDescriptorSet(LogicalDevice, NewPool, SetLayout, "");
+    auto Set = AllocateDescriptorSet(LogicalDevice, NewPool, SetLayout, DebugName);
     DEV_CHECK_ERR(Set != VK_NULL_HANDLE, "Failed to allocate descriptor set");
 
 #ifdef DEVELOPMENT
diff --git a/Graphics/GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp b/Graphics/GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp
index 8137e7cf..7fcc6b27 100644
--- a/Graphics/GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp
@@ -183,7 +183,7 @@ namespace Diligent
     {
         // All PSOs are compatible
         auto& PSO = FindPSOs(TEX_FORMAT_RGBA8_UNORM);
-        PSO[0]->CreateShaderResourceBinding(ppSRB);
+        PSO[0]->CreateShaderResourceBinding(ppSRB, true);
     }
 
     std::array<RefCntAutoPtr<IPipelineState>, 4>& GenerateMipsVkHelper::FindPSOs(TEXTURE_FORMAT Fmt)
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
index 9260450b..9897a1d2 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
@@ -404,7 +404,10 @@ std::array<Uint32, 2> PipelineLayout::GetDescriptorSetSizes(Uint32& NumSets)cons
     return SetSizes;
 }
 
-void PipelineLayout::InitResourceCache(RenderDeviceVkImpl* pDeviceVkImpl, ShaderResourceCacheVk& ResourceCache, IMemoryAllocator& CacheMemAllocator)const
+void PipelineLayout::InitResourceCache(RenderDeviceVkImpl*    pDeviceVkImpl,
+                                       ShaderResourceCacheVk& ResourceCache,
+                                       IMemoryAllocator&      CacheMemAllocator,
+                                       const char*            DbgPipelineName)const
 {
     Uint32 NumSets = 0;
     auto SetSizes = GetDescriptorSetSizes(NumSets);
@@ -413,10 +416,16 @@ void PipelineLayout::InitResourceCache(RenderDeviceVkImpl* pDeviceVkImpl, Shader
     // Resources are initialized by source layout when shader resource binding objects are created
     ResourceCache.InitializeSets(CacheMemAllocator, NumSets, SetSizes.data());
 
-    const auto &StaticAndMutSet = m_LayoutMgr.GetDescriptorSet(SHADER_VARIABLE_TYPE_STATIC);
+    const auto& StaticAndMutSet = m_LayoutMgr.GetDescriptorSet(SHADER_VARIABLE_TYPE_STATIC);
     if (StaticAndMutSet.SetIndex >= 0)
     {
-        DescriptorSetAllocation SetAllocation = pDeviceVkImpl->AllocateDescriptorSet(~Uint64{0}, StaticAndMutSet.VkLayout);
+        const char* DescrSetName = "Static/Mutable Descriptor Set";
+#ifdef DEVELOPMENT
+        std::string _DescrSetName(DbgPipelineName);
+        _DescrSetName.append(" - static/mutable set");
+        DescrSetName = _DescrSetName.c_str();
+#endif
+        DescriptorSetAllocation SetAllocation = pDeviceVkImpl->AllocateDescriptorSet(~Uint64{0}, StaticAndMutSet.VkLayout, DescrSetName);
         ResourceCache.GetDescriptorSet(StaticAndMutSet.SetIndex).AssignDescriptorSetAllocation(std::move(SetAllocation));
     }
 }
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
index 24d9b068..2761b011 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
@@ -536,7 +536,7 @@ void PipelineStateVkImpl::CommitAndTransitionShaderResources(IShaderResourceBind
 
     if (m_HasStaticResources && !pResBindingVkImpl->StaticResourcesInitialized())
     {
-        LOG_ERROR_MESSAGE("Static resources have not been initialized in the shader resource binding object. Please call IShaderResourceBinding::InitializeStaticResources().");
+        LOG_ERROR_MESSAGE("Static resources have not been initialized in the shader resource binding object being committed for PSO '", m_Desc.Name,"'. Please call IShaderResourceBinding::InitializeStaticResources().");
     }
 #endif
 
@@ -564,8 +564,14 @@ void PipelineStateVkImpl::CommitAndTransitionShaderResources(IShaderResourceBind
         auto DynamicDescriptorSetVkLayout = m_PipelineLayout.GetDynamicDescriptorSetVkLayout();
         if (DynamicDescriptorSetVkLayout != VK_NULL_HANDLE)
         {
+            const char* DynamicDescrSetName = "Dynamic Descriptor Set";
+#ifdef DEVELOPMENT
+            std::string _DynamicDescrSetName(m_Desc.Name);
+            _DynamicDescrSetName.append(" - dynamic set");
+            DynamicDescrSetName = _DynamicDescrSetName.c_str();
+#endif
             // Allocate vulkan descriptor set for dynamic resources
-            DynamicDescrSet = pCtxVkImpl->AllocateDynamicDescriptorSet(DynamicDescriptorSetVkLayout);
+            DynamicDescrSet = pCtxVkImpl->AllocateDynamicDescriptorSet(DynamicDescriptorSetVkLayout, DynamicDescrSetName);
             // Commit all dynamic resource descriptors
             for (Uint32 s=0; s < m_NumShaders; ++s)
             {
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp
index d3f885a3..c4c899b5 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp
@@ -42,7 +42,7 @@ ShaderResourceBindingVkImpl::ShaderResourceBindingVkImpl( IReferenceCounters* pR
     // This will only allocate memory and initialize descriptor sets in the resource cache
     // Resources will be initialized by InitializeResourceMemoryInCache()
     auto& ResourceCacheDataAllocator = pPSO->GetSRBMemoryAllocator().GetResourceCacheDataAllocator(0);
-    pPSO->GetPipelineLayout().InitResourceCache(pRenderDeviceVkImpl, m_ShaderResourceCache, ResourceCacheDataAllocator);
+    pPSO->GetPipelineLayout().InitResourceCache(pRenderDeviceVkImpl, m_ShaderResourceCache, ResourceCacheDataAllocator, pPSO->GetDesc().Name);
     
     auto *pVarMgrsRawMem = ALLOCATE(GetRawAllocator(), "Raw memory for ShaderVariableManagerVk", m_NumShaders * sizeof(ShaderVariableManagerVk));
     m_pShaderVarMgrs = reinterpret_cast<ShaderVariableManagerVk*>(pVarMgrsRawMem);
@@ -159,7 +159,13 @@ void ShaderResourceBindingVkImpl::InitializeStaticResources(const IPipelineState
     {
         const auto* pShaderVk = pPSOVK->GetShader<const ShaderVkImpl>(s);
 #ifdef DEVELOPMENT
-        pShaderVk->DvpVerifyStaticResourceBindings();
+        if (!pShaderVk->DvpVerifyStaticResourceBindings())
+        {
+            LOG_ERROR_MESSAGE("Static resources in a SRB of PSO '", pPSOVK->GetDesc().Name, "' will not be successfully initialized "
+                              "because not all static resource bindings in shader '", pShaderVk->GetDesc().Name, "' are valid. "
+                              "Please make sure you bind all static resources to the shader before calling InitializeStaticResources() or "
+                              "before creating a SRB via CreateShaderResourceBinding() method with InitStaticResources=true.");
+        }
 #endif
         const auto& StaticResLayout = pShaderVk->GetStaticResLayout();
         const auto& StaticResCache = pShaderVk->GetStaticResCache();
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
index d0ba2d3d..f544e6cb 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
@@ -725,8 +725,9 @@ void ShaderResourceLayoutVk::InitializeStaticResources(const ShaderResourceLayou
 
 
 #ifdef DEVELOPMENT
-void ShaderResourceLayoutVk::dvpVerifyBindings(const ShaderResourceCacheVk& ResourceCache)const
+bool ShaderResourceLayoutVk::dvpVerifyBindings(const ShaderResourceCacheVk& ResourceCache)const
 {
+    bool BindingsOK = true;
     for(SHADER_VARIABLE_TYPE VarType = SHADER_VARIABLE_TYPE_STATIC; VarType < SHADER_VARIABLE_TYPE_NUM_TYPES; VarType = static_cast<SHADER_VARIABLE_TYPE>(VarType+1))
     {
         for(Uint32 r=0; r < m_NumResources[VarType]; ++r)
@@ -742,6 +743,7 @@ void ShaderResourceLayoutVk::dvpVerifyBindings(const ShaderResourceCacheVk& Reso
                    !(Res.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler && Res.SpirvAttribs.IsImmutableSamplerAssigned()))
                 {
                     LOG_ERROR_MESSAGE("No resource is bound to ", GetShaderVariableTypeLiteralName(Res.SpirvAttribs.VarType), " variable '", Res.SpirvAttribs.GetPrintName(ArrInd), "' in shader '", GetShaderName(), "'");
+                    BindingsOK = false;
                 }
 #ifdef _DEBUG
                 auto vkDescSet = CachedDescrSet.GetVkDescriptorSet();
@@ -765,6 +767,7 @@ void ShaderResourceLayoutVk::dvpVerifyBindings(const ShaderResourceCacheVk& Reso
             }
         }
     }
+    return BindingsOK;
 }
 #endif
 
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp
index a0a76499..a735c691 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp
@@ -129,9 +129,9 @@ ShaderVkImpl::~ShaderVkImpl()
 }
 
 #ifdef DEVELOPMENT
-void ShaderVkImpl::DvpVerifyStaticResourceBindings()const
+bool ShaderVkImpl::DvpVerifyStaticResourceBindings()const
 {
-    m_StaticResLayout.dvpVerifyBindings(m_StaticResCache);
+    return m_StaticResLayout.dvpVerifyBindings(m_StaticResCache);
 }
 #endif