Few minor code changes + comments

5 files changed, 50 insertions, 45 deletions

diff --git a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.hpp
index e2354274..88ebd252 100644
--- a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.hpp
@@ -524,7 +524,7 @@ private:
 
         __forceinline bool RequireUpdate(bool DynamicBuffersIntact = false) const
         {
-            return (StaleSRBMask & ActiveSRBMask) != 0 || ((DynamicBuffersMask & ActiveSRBMask) && !DynamicBuffersIntact);
+            return (StaleSRBMask & ActiveSRBMask) != 0 || ((DynamicBuffersMask & ActiveSRBMask) != 0 && !DynamicBuffersIntact);
         }
 
         void SetStaleSRBBit(Uint32 Index) { StaleSRBMask |= static_cast<Bitfield>(1u << Index); }
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp
index c3fbee22..e2026a67 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp
@@ -85,12 +85,12 @@ private:
     std::array<Int8, MAX_SHADERS_IN_PIPELINE> m_ShaderVarIndex = {-1, -1, -1, -1, -1, -1};
     static_assert(MAX_SHADERS_IN_PIPELINE == 6, "Please update the initializer list above");
 
-    ShaderResourceCacheVk    m_ShaderResourceCache;
-    ShaderVariableManagerVk* m_pShaderVarMgrs = nullptr;
-
     bool m_bStaticResourcesInitialized = false;
 
     const Uint8 m_NumShaders = 0;
+
+    ShaderResourceCacheVk    m_ShaderResourceCache;
+    ShaderVariableManagerVk* m_pShaderVarMgrs = nullptr;
 };
 
 } // namespace Diligent
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.hpp
index 76f9a9e4..d0a5bb41 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.hpp
@@ -31,31 +31,31 @@
 /// Declaration of Diligent::ShaderVariableManagerVk and Diligent::ShaderVariableVkImpl classes
 
 //
-//  * ShaderVariableManagerVk keeps list of variables of specific types
-//  * Every ShaderVariableVkImpl references VkResource from ShaderResourceLayoutVk
+//  * ShaderVariableManagerVk keeps the list of variables of specific types
+//  * Every ShaderVariableVkImpl references ResourceAttribs by index from PipelineResourceSignatureVkImpl
 //  * ShaderVariableManagerVk keeps reference to ShaderResourceCacheVk
-//  * ShaderVariableManagerVk is used by PipelineStateVkImpl to manage static resources and by
+//  * ShaderVariableManagerVk is used by PipelineResourceSignatureVkImpl to manage static resources and by
 //    ShaderResourceBindingVkImpl to manage mutable and dynamic resources
 //
-//          __________________________                   __________________________________________________________________________
-//         |                          |                 |                           |                            |                 |
-//    .----|  ShaderVariableManagerVk |---------------->|  ShaderVariableVkImpl[0]  |   ShaderVariableVkImpl[1]  |     ...         |
-//    |    |__________________________|                 |___________________________|____________________________|_________________|
-//    |                                                                     \                          |
-//    |                                                                     Ref                       Ref
-//    |                                                                       \                        |
-//    |     ___________________________                  ______________________V_______________________V____________________________
-//    |    |                           |   unique_ptr   |                   |                 |               |                     |
-//    |    | ShaderResourceLayoutVk    |--------------->|   VkResource[0]   |  VkResource[1]  |       ...     | VkResource[s+m+d-1] |
-//    |    |___________________________|                |___________________|_________________|_______________|_____________________|
-//    |                                                        |                                                            |
-//    |                                                        |                                                            |
-//    |                                                        | (DescriptorSet, CacheOffset)                              / (DescriptorSet, CacheOffset)
-//    |                                                         \                                                         /
-//    |     __________________________                   ________V_______________________________________________________V_______
-//    |    |                          |                 |                                                                        |
-//    '--->|   ShaderResourceCacheVk  |---------------->|                                   Resources                            |
-//         |__________________________|                 |________________________________________________________________________|
+//          __________________________                             __________________________________________________________________________
+//         |                          |                           |                           |                            |                 |
+//    .----|  ShaderVariableManagerVk |-------------------------->|  ShaderVariableVkImpl[0]  |   ShaderVariableVkImpl[1]  |     ...         |
+//    |    |__________________________|                           |___________________________|____________________________|_________________|
+//    |                |                                                              \                          |
+//    |           m_pSignature                                                     m_ResIndex               m_ResIndex
+//    |                |                                                                \                        |
+//    |     ___________V_____________________                      ______________________V_______________________V____________________________
+//    |    |                                 | m_pResourceAttribs |                  |                |               |                     |
+//    |    | PipelineResourceSignatureVkImpl |------------------->|    Resource[0]   |   Resource[1]  |       ...     |  Resource[s+m+d-1]  |
+//    |    |_________________________________|                    |__________________|________________|_______________|_____________________|
+//    |                                                                  |                                                        |
+//    |                                                                  |                                                        |
+//    |                                                                  | (DescriptorSet, CacheOffset)                          / (DescriptorSet, CacheOffset)
+//    |                                                                   \                                                     /
+//    |     __________________________                             ________V___________________________________________________V_______
+//    |    |                          |                           |                                                                    |
+//    '--->|   ShaderResourceCacheVk  |-------------------------->|                                   Resources                        |
+//         |__________________________|                           |____________________________________________________________________|
 //
 //
 
@@ -132,13 +132,13 @@ private:
 
     IObject& m_Owner;
 
-    // Variable mgr is owned by either Pipeline state object (in which case m_ResourceCache references
-    // static resource cache owned by the same PSO object), or by SRB object (in which case
-    // m_ResourceCache references the cache in the SRB). Thus the cache and the resource layout
+    // Variable mgr is owned by either Pipeline Resource Signature (in which case m_ResourceCache references
+    // static resource cache owned by the same PRS object), or by SRB object (in which case
+    // m_ResourceCache references the cache in the SRB). Thus the cache and the signature
     // (which the variables reference) are guaranteed to be alive while the manager is alive.
     ShaderResourceCacheVk& m_ResourceCache;
 
-    // Memory is allocated through the allocator provided by the pipeline state. If allocation granularity > 1, fixed block
+    // Memory is allocated through the allocator provided by the resource signature. If allocation granularity > 1, fixed block
     // memory allocator is used. This ensures that all resources from different shader resource bindings reside in
     // continuous memory. If allocation granularity == 1, raw allocator is used.
     ShaderVariableVkImpl* m_pVariables   = nullptr;
@@ -149,7 +149,7 @@ private:
 #endif
 };
 
-// sizeof(ShaderVariableVkImpl) == 16 (x64)
+// sizeof(ShaderVariableVkImpl) == 24 (x64)
 class ShaderVariableVkImpl final : public IShaderResourceVariable
 {
 public:
diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
index 60fc1a60..fe8786ff 100644
--- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
@@ -356,6 +356,9 @@ void DeviceContextVkImpl::SetPipelineState(IPipelineState* pPipelineState)
     }
 
     // Unbind incompatible shader resources
+    // A consequence of layout compatibility is that when the implementation compiles a pipeline
+    // layout and maps pipeline resources to implementation resources, the mechanism for set N
+    // should only be a function of sets [0..N].
     for (; sign < SignCount; ++sign)
     {
         BindInfo.SRBs[sign] = nullptr;
@@ -373,6 +376,8 @@ void DeviceContextVkImpl::SetPipelineState(IPipelineState* pPipelineState)
 
 DeviceContextVkImpl::DescriptorSetBindInfo& DeviceContextVkImpl::GetDescriptorSetBindInfo(PIPELINE_TYPE Type)
 {
+    VERIFY_EXPR(Type != PIPELINE_TYPE_INVALID);
+
     // clang-format off
     static_assert(PIPELINE_TYPE_GRAPHICS    == 0, "PIPELINE_TYPE_GRAPHICS == 0 is expected");
     static_assert(PIPELINE_TYPE_COMPUTE     == 1, "PIPELINE_TYPE_COMPUTE == 1 is expected");
@@ -392,6 +397,7 @@ DeviceContextVkImpl::DescriptorSetBindInfo& DeviceContextVkImpl::GetDescriptorSe
 
 void DeviceContextVkImpl::CommitDescriptorSets(DescriptorSetBindInfo& BindInfo)
 {
+    // Commit all stale descriptor sets or these that have dynamic buffers, which are used by the PSO
     auto StaleSRBFlags = (Uint32{BindInfo.StaleSRBMask} | Uint32{BindInfo.DynamicBuffersMask}) & Uint32{BindInfo.ActiveSRBMask};
     while (StaleSRBFlags != 0)
     {
@@ -402,7 +408,8 @@ void DeviceContextVkImpl::CommitDescriptorSets(DescriptorSetBindInfo& BindInfo)
         auto& ResInfo = BindInfo.Resources[sign];
         VERIFY_EXPR(ResInfo.pResourceCache != nullptr);
 
-        VERIFY(ResInfo.vkSets[0] != VK_NULL_HANDLE, "At least one descriptor set in an active SRB must not be NULL");
+        VERIFY(ResInfo.vkSets[0] != VK_NULL_HANDLE,
+               "At least one descriptor set in the stale SRB must not be NULL. Empty SRBs should not be marked as stale by CommitShaderResources()");
         const Uint32 SetCount = 1 + (ResInfo.vkSets[1] != VK_NULL_HANDLE ? 1 : 0);
 
         VERIFY_EXPR(SetCount == ResInfo.pResourceCache->GetNumDescriptorSets());
@@ -410,7 +417,7 @@ void DeviceContextVkImpl::CommitDescriptorSets(DescriptorSetBindInfo& BindInfo)
         if (ResInfo.DynamicOffsetCount > 0)
         {
             VERIFY(m_DynamicBufferOffsets.size() >= ResInfo.DynamicOffsetCount,
-                   "m_DynamicBufferOffsets must've been resized by CommitShaderResources to have enough space");
+                   "m_DynamicBufferOffsets must've been resized by CommitShaderResources() to have enough space");
 
             auto NumOffsetsWritten = ResInfo.pResourceCache->GetDynamicBufferOffsets(m_ContextId, this, m_DynamicBufferOffsets);
             VERIFY_EXPR(NumOffsetsWritten == ResInfo.DynamicOffsetCount);
@@ -461,12 +468,12 @@ void DeviceContextVkImpl::DvpValidateCommittedShaderResources()
             continue;
         }
 
-        auto* ResSign = pSRB->GetSignature();
-        VERIFY_EXPR(ResSign != nullptr);
+        auto* pSRBSign = pSRB->GetSignature();
+        VERIFY_EXPR(pSRBSign != nullptr);
 
-        if (!pLayoutSign->IsCompatibleWith(*ResSign))
+        if (!pLayoutSign->IsCompatibleWith(*pSRBSign))
         {
-            LOG_ERROR_MESSAGE("Shader resource binding with signature '", ResSign->GetDesc().Name,
+            LOG_ERROR_MESSAGE("Shader resource binding at index ", i, " with signature '", pSRBSign->GetDesc().Name,
                               "' is not compatible with pipeline layout in current pipeline '", m_pPipelineState->GetDesc().Name, "'.");
         }
 
@@ -477,8 +484,8 @@ void DeviceContextVkImpl::DvpValidateCommittedShaderResources()
         for (Uint32 s = 0; s < DSCount; ++s)
         {
             DEV_CHECK_ERR(ResInfo.vkSets[s] != VK_NULL_HANDLE,
-                          "descriptor set with index (", s, ") is not bound for resource signature '",
-                          pLayoutSign->GetDesc().Name, "' binding index (", i, ").");
+                          "descriptor set with index ", s, " is not bound for resource signature '",
+                          pLayoutSign->GetDesc().Name, "', binding index ", i, ".");
         }
     }
 
@@ -519,7 +526,7 @@ void DeviceContextVkImpl::CommitShaderResources(IShaderResourceBinding* pShaderR
     auto& ResourceCache     = pResBindingVkImpl->GetResourceCache();
     if (ResourceCache.GetNumDescriptorSets() == 0)
     {
-        // Skip SRBs that contain no resources
+        // Ignore SRBs that contain no resources
         return;
     }
 
@@ -733,8 +740,7 @@ void DeviceContextVkImpl::PrepareForDraw(DRAW_FLAGS Flags)
 #endif
 
     auto& DescrSetBindInfo = GetDescriptorSetBindInfo(PIPELINE_TYPE_GRAPHICS);
-
-    // First time we must always bind descriptor sets with dynamic offsets.
+    // First time we must always bind descriptor sets with dynamic offsets as SRBs are stale.
     // 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 (DescrSetBindInfo.RequireUpdate(Flags & DRAW_FLAG_DYNAMIC_RESOURCE_BUFFERS_INTACT))
@@ -888,7 +894,6 @@ void DeviceContextVkImpl::PrepareForDispatchCompute()
         m_CommandBuffer.EndRenderPass();
 
     auto& DescrSetBindInfo = GetDescriptorSetBindInfo(PIPELINE_TYPE_COMPUTE);
-
     if (DescrSetBindInfo.RequireUpdate())
     {
         CommitDescriptorSets(DescrSetBindInfo);
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp
index 4e412e0e..151e3d66 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp
@@ -183,7 +183,7 @@ PipelineLayoutVk::ResourceInfo PipelineLayoutVk::GetResourceInfo(const char* Nam
             const auto& ResDesc = pSignature->GetResourceDesc(r);
             const auto& Attr    = pSignature->GetResourceAttribs(r);
 
-            if ((ResDesc.ShaderStages & Stage) && strcmp(ResDesc.Name, Name) == 0)
+            if ((ResDesc.ShaderStages & Stage) != 0 && strcmp(ResDesc.Name, Name) == 0)
             {
                 Info.Signature     = pSignature;
                 Info.Type          = ResDesc.ResourceType;
@@ -211,7 +211,7 @@ PipelineLayoutVk::ResourceInfo PipelineLayoutVk::GetImmutableSamplerInfo(const c
             const auto& Desc = pSignature->GetImmutableSamplerDesc(s);
             const auto& Attr = pSignature->GetImmutableSamplerAttribs(s);
 
-            if (Attr.Ptr && (Desc.ShaderStages & Stage) && StreqSuff(Name, Desc.SamplerOrTextureName, pSignature->GetCombinedSamplerSuffix()))
+            if (Attr.Ptr && (Desc.ShaderStages & Stage) != 0 && StreqSuff(Name, Desc.SamplerOrTextureName, pSignature->GetCombinedSamplerSuffix()))
             {
                 Info.Signature     = pSignature;
                 Info.Type          = SHADER_RESOURCE_TYPE_SAMPLER;