Few minor improvements to shader resource management in Vk backend

4 files changed, 22 insertions, 62 deletions

diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.h
index 03f5018f..abdd733f 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.h
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.h
@@ -29,7 +29,7 @@
 // 
 //  * ShaderVariableManagerVk keeps list of variables of specific types
 //  * Every ShaderVariableVkImpl references VkResource from ShaderResourceLayoutVk
-//  * ShaderVariableManagerVk keeps pointer to ShaderResourceCacheVk
+//  * ShaderVariableManagerVk keeps reference to ShaderResourceCacheVk
 //  * ShaderVariableManagerVk is used by PipelineStateVkImpl to manage static resources and by
 //    ShaderResourceBindingVkImpl to manage mutable and dynamic resources
 //
@@ -37,10 +37,10 @@
 //         |                          |                 |                           |                            |                 |
 //    .----|  ShaderVariableManagerVk |---------------->|  ShaderVariableVkImpl[0]  |   ShaderVariableVkImpl[1]  |     ...         |
 //    |    |__________________________|                 |___________________________|____________________________|_________________|
-//    |                |                                                    \                          |
-//    |                |                                                    Ref                       Ref
-//    |                |                                                      \                        |
-//    |     ___________V_______________                  ______________________V_______________________V____________________________
+//    |                                                                     \                          |
+//    |                                                                     Ref                       Ref
+//    |                                                                       \                        |
+//    |     ___________________________                  ______________________V_______________________V____________________________
 //    |    |                           |   unique_ptr   |                   |                 |               |                     |
 //    |    | ShaderResourceLayoutVk    |--------------->|   VkResource[0]   |  VkResource[1]  |       ...     | VkResource[s+m+d-1] |
 //    |    |___________________________|                |___________________|_________________|_______________|_____________________|
@@ -118,7 +118,7 @@ private:
 class ShaderVariableVkImpl final : public IShaderResourceVariable
 {
 public:
-    ShaderVariableVkImpl(ShaderVariableManagerVk& ParentManager,
+    ShaderVariableVkImpl(ShaderVariableManagerVk&                  ParentManager,
                          const ShaderResourceLayoutVk::VkResource& Resource) :
         m_ParentManager(ParentManager),
         m_Resource(Resource)
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp
index 2bfa922e..1d060871 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp
@@ -99,7 +99,7 @@ void ShaderResourceCacheVk::InitializeResources(Uint32 Set, Uint32 Offset, Uint3
 #ifdef _DEBUG
 void ShaderResourceCacheVk::DbgVerifyResourceInitialization()const
 {
-    for (const auto &SetFlags : m_DbgInitializedResources)
+    for (const auto& SetFlags : m_DbgInitializedResources)
     {
         for (auto ResInitialized : SetFlags)
             VERIFY(ResInitialized, "Not all resources in the cache have been initialized. This is a bug.");
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
index e65aaa91..595c49f1 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
@@ -481,6 +481,8 @@ void ShaderResourceLayoutVk::Initialize(IRenderDevice*
         {
             VERIFY(CurrResInd[s][VarType] == Layout.m_NumResources[VarType], "Not all resources have been initialized, which will cause a crash when dtor is called. This is a bug.");
         }
+        // Some immutable samplers may never be initialized if they are not present in shaders
+        VERIFY_EXPR(CurrImmutableSamplerInd[s] <= Layout.m_NumImmutableSamplers);
     }
 #endif
 }
@@ -495,17 +497,19 @@ Uint32 ShaderResourceLayoutVk::FindAssignedSampler(const SPIRVShaderResourceAttr
     Uint32 SamplerInd = VkResource::InvalidSamplerInd;
     if (m_pResources->IsUsingCombinedSamplers() && SepImg.IsValidSepSamplerAssigned())
     {
-        const auto& SepSampler = m_pResources->GetSepSmplr(SepImg.GetAssignedSepSamplerInd());
+        const auto& SepSampler = m_pResources->GetAssignedSepSampler(SepImg);
         for (SamplerInd = 0; SamplerInd < CurrResourceCount; ++SamplerInd)
         {
             const auto& Res = GetResource(ImgVarType, SamplerInd);
             if (Res.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler && 
                 strcmp(Res.SpirvAttribs.Name, SepSampler.Name) == 0)
             {
-                DEV_CHECK_ERR(ImgVarType == Res.GetVariableType(),
-                              "The type (", GetShaderVariableTypeLiteralName(ImgVarType),") of separate image variable '", SepImg.Name,
-                              "' is not consistent with the type (", GetShaderVariableTypeLiteralName(Res.GetVariableType()),
-                              ") of the separate sampler '", SepSampler.Name, "' that is assigned to it.");
+                VERIFY(ImgVarType == Res.GetVariableType(),
+                       "The type (", GetShaderVariableTypeLiteralName(ImgVarType),") of separate image variable '", SepImg.Name,
+                       "' is not consistent with the type (", GetShaderVariableTypeLiteralName(Res.GetVariableType()),
+                       ") of the separate sampler '", SepSampler.Name, "' that is assigned to it. "
+                       "This should never happen as when HLSL-style combined texture samplers are used, the type of the sampler "
+                       "is derived from the type of the corresponding separate image.");
                 break;
             }
         }
@@ -835,7 +839,7 @@ void ShaderResourceLayoutVk::VkResource::BindResource(IDeviceObject* pObj, Uint3
                 CacheImage(pObj, DstRes, vkDescrSet, ArrayIndex, 
                     [&](const VkResource& SeparateSampler, ISampler* pSampler)
                     {
-                        VERIFY(!SeparateSampler.IsImmutableSamplerAssigned(), "This separate sampler is assigned an immutable sampler '", SeparateSampler.SpirvAttribs.Name, "'");
+                        VERIFY(!SeparateSampler.IsImmutableSamplerAssigned(), "Separate sampler '", SeparateSampler.SpirvAttribs.Name, "' is assigned an immutable sampler");
                         VERIFY_EXPR(SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateImage);
                         DEV_CHECK_ERR(SeparateSampler.SpirvAttribs.ArraySize == 1 || SeparateSampler.SpirvAttribs.ArraySize == SpirvAttribs.ArraySize,
                                       "Array size (", SeparateSampler.SpirvAttribs.ArraySize,") of separate sampler variable '",
@@ -856,7 +860,7 @@ void ShaderResourceLayoutVk::VkResource::BindResource(IDeviceObject* pObj, Uint3
                 {
                     // Immutable samplers are permanently bound into the set layout; later binding a sampler 
                     // into an immutable sampler slot in a descriptor set is not allowed (13.2.1)
-                    LOG_ERROR_MESSAGE("Attempting to assign a sampler to a static sampler '", SpirvAttribs.Name, '\'');
+                    LOG_ERROR_MESSAGE("Attempting to assign a sampler to an immutable sampler '", SpirvAttribs.Name, '\'');
                 }
             break;
 
@@ -1011,9 +1015,9 @@ void ShaderResourceLayoutVk::CommitDynamicResources(const ShaderResourceCacheVk&
 #endif
 
     // Do not zero-initiaize arrays!
-    std::array<VkDescriptorImageInfo,  ImgUpdateBatchSize>       DescrImgInfoArr;
-    std::array<VkDescriptorBufferInfo, BuffUpdateBatchSize>      DescrBuffInfoArr;
-    std::array<VkBufferView,           TexelBuffUpdateBatchSize> DescrBuffViewArr;
+    std::array<VkDescriptorImageInfo,  ImgUpdateBatchSize>          DescrImgInfoArr;
+    std::array<VkDescriptorBufferInfo, BuffUpdateBatchSize>         DescrBuffInfoArr;
+    std::array<VkBufferView,           TexelBuffUpdateBatchSize>    DescrBuffViewArr;
     std::array<VkWriteDescriptorSet,   WriteDescriptorSetBatchSize> WriteDescrSetArr;
 
     Uint32 ResNum = 0, ArrElem = 0;
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp
index 9ec51af6..5409b278 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp
@@ -137,51 +137,7 @@ ShaderResourceDesc ShaderVkImpl::GetResource(Uint32 Index)const
     if (Index < ResCount)
     {
         const auto& SPIRVResource = m_pShaderResources->GetResource(Index);
-        ResourceDesc.Name      = SPIRVResource.Name;
-        ResourceDesc.ArraySize = SPIRVResource.ArraySize;
-        static_assert(SPIRVShaderResourceAttribs::ResourceType::NumResourceTypes == 9, "Please update switch statement below");
-        switch (SPIRVResource.Type)
-        {
-            case SPIRVShaderResourceAttribs::ResourceType::UniformBuffer:
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_CONSTANT_BUFFER;
-            break;
-
-            case SPIRVShaderResourceAttribs::ResourceType::StorageBuffer:
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_BUFFER_UAV;
-            break;
-
-            case SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer:
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_BUFFER_SRV;
-            break;
-
-            case SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer:
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_BUFFER_UAV;
-            break;
-
-            case SPIRVShaderResourceAttribs::ResourceType::StorageImage:
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_TEXTURE_UAV;
-            break;
-
-            case SPIRVShaderResourceAttribs::ResourceType::SampledImage:
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_TEXTURE_SRV;
-            break;
-
-            case SPIRVShaderResourceAttribs::ResourceType::AtomicCounter:
-                LOG_WARNING_MESSAGE("There is no appropriate shader resource type for atomic counter resource '", SPIRVResource.Name, "'");
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_BUFFER_UAV;
-            break;
-
-            case SPIRVShaderResourceAttribs::ResourceType::SeparateImage:
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_TEXTURE_SRV;
-            break;
-
-            case SPIRVShaderResourceAttribs::ResourceType::SeparateSampler:
-                ResourceDesc.Type = SHADER_RESOURCE_TYPE_SAMPLER;
-            break;
-
-            default:
-                UNEXPECTED("Unknown SPIRV resource type");
-        }
+        ResourceDesc = SPIRVResource.GetResourceDesc();
     }
     return ResourceDesc;
 }