Vk backend: some minor updates + comments

7 files changed, 44 insertions, 31 deletions

diff --git a/Graphics/GraphicsEngine/interface/GraphicsTypes.h b/Graphics/GraphicsEngine/interface/GraphicsTypes.h
index fff9ca2a..5e236134 100644
--- a/Graphics/GraphicsEngine/interface/GraphicsTypes.h
+++ b/Graphics/GraphicsEngine/interface/GraphicsTypes.h
@@ -1644,10 +1644,11 @@ struct DeviceFeatures
     /// Indicates if device supports reading 8-bit types from uniform buffers.
     DEVICE_FEATURE_STATE UniformBuffer8BitAccess          DEFAULT_INITIALIZER(DEVICE_FEATURE_STATE_DISABLED);
 
-    /// Indicates if device supports runtime-sized arrays in shader.
-    /// DirectX 12 backend already supports this feature,
-    /// Vulkan backend has optional support,
-    /// other backends are does not support this feature.
+    /// Indicates if device supports runtime-sized shader arrays (e.g. arrays without a specific size).
+    ///
+    /// \remarks    This feature is always enabled in DirectX12 backend and
+    ///             can optionally be enabled in Vulkan backend.
+    ///             Run-time sized shader arrays are not available in other backends.
     DEVICE_FEATURE_STATE ShaderResourceRuntimeArray       DEFAULT_INITIALIZER(DEVICE_FEATURE_STATE_DISABLED);
 
 
diff --git a/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h b/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h
index a501a27c..05feb54f 100644
--- a/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h
+++ b/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h
@@ -108,7 +108,7 @@ DILIGENT_TYPED_ENUM(PIPELINE_RESOURCE_FLAGS, Uint8)
     ///             PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER flag.
     PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER   = 0x04,
 
-    /// DirectX 12 only - runtime sized array must be in separate space.
+    /// Indicates that resource is a run-time sized shader array (e.g. an array without a specific size).
     PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY      = 0x08,
     
     PIPELINE_RESOURCE_FLAG_LAST               = PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY
diff --git a/Graphics/GraphicsEngine/interface/PipelineState.h b/Graphics/GraphicsEngine/interface/PipelineState.h
index dedeee9a..d8d765c1 100644
--- a/Graphics/GraphicsEngine/interface/PipelineState.h
+++ b/Graphics/GraphicsEngine/interface/PipelineState.h
@@ -593,9 +593,15 @@ DILIGENT_BEGIN_INTERFACE(IPipelineState, IDeviceObject)
     ///             The function only checks compatibility of shader resource layouts. It does not take
     ///             into account vertex shader input layout, number of outputs, etc.
     /// 
-    /// \remarks    On Vulkan backend PSO may be partially compatible, on other backends this behavior is emulated.
-    ///             For Vulkan changing PSO between totally or partially compatible may increase performance,
-    ///             for DirectX 12 only changing PSO between compatible may increase performance.
+    ///             *Technical details*
+    ///
+    ///             PSOs may be partially compatible when some, but not all pipeline resource signatures are compatible.
+    ///             In Vulkan backend, switching PSOs that are partially compatible may increase performance
+    ///             as shader resource bindings (that map to descriptor sets) from compatible signatures may be preserved. 
+    ///             In Direct3D12 backend, only switching between fully compatible PSOs preserves shader resource bindings, 
+    ///             while switching partially compatible PSOs still requires re-binding all resource bindigns from all signatures.
+    ///             In other backends the behavior is emualted. Usually, the bindigs from the first N compatible resource signatures
+    ///             may be preserved.
     VIRTUAL bool METHOD(IsCompatibleWith)(THIS_
                                           const struct IPipelineState* pPSO) CONST PURE;
 
diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp
index 11c440b3..ea20151b 100644
--- a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp
@@ -259,9 +259,9 @@ public:
                       Uint32                 ResIndex,
                       ShaderResourceCacheVk& ResourceCache) const;
 
-    bool IsBound(Uint32                 ArrayIndex,
-                 Uint32                 ResIndex,
-                 ShaderResourceCacheVk& ResourceCache) const;
+    bool IsBound(Uint32                       ArrayIndex,
+                 Uint32                       ResIndex,
+                 const ShaderResourceCacheVk& ResourceCache) const;
 
     // Commits dynamic resources from ResourceCache to vkDynamicDescriptorSet
     void CommitDynamicResources(const ShaderResourceCacheVk& ResourceCache,
@@ -289,10 +289,10 @@ private:
     // Resource cache group identifier
     enum CACHE_GROUP : size_t
     {
-        CACHE_GROUP_DYN_UB = 0, // Uniform buffer with dynamic offset
-        CACHE_GROUP_DYN_SB,     // Storage buffer with dynamic offset
-        CACHE_GROUP_OTHER,      // Other resource type
-        CACHE_GROUP_COUNT_PER_Type,
+        CACHE_GROUP_DYN_UB = 0,         // Uniform buffer with dynamic offset
+        CACHE_GROUP_DYN_SB,             // Storage buffer with dynamic offset
+        CACHE_GROUP_OTHER,              // Other resource type
+        CACHE_GROUP_COUNT_PER_VAR_TYPE, // Cache group count per shader variable type
 
         CACHE_GROUP_DYN_UB_STAT_VAR = CACHE_GROUP_DYN_UB, // Uniform buffer with dynamic offset, static variable
         CACHE_GROUP_DYN_SB_STAT_VAR = CACHE_GROUP_DYN_SB, // Storage buffer with dynamic offset, static variable
@@ -304,7 +304,7 @@ private:
 
         CACHE_GROUP_COUNT
     };
-    static_assert(CACHE_GROUP_COUNT == CACHE_GROUP_COUNT_PER_Type * MAX_DESCRIPTOR_SETS, "Inconsistent cache group count");
+    static_assert(CACHE_GROUP_COUNT == CACHE_GROUP_COUNT_PER_VAR_TYPE * MAX_DESCRIPTOR_SETS, "Inconsistent cache group count");
 
     using CacheOffsetsType = std::array<Uint32, CACHE_GROUP_COUNT>; // [dynamic uniform buffers, dynamic storage buffers, other] x [descriptor sets] including ArraySize
     using BindingCountType = std::array<Uint32, CACHE_GROUP_COUNT>; // [dynamic uniform buffers, dynamic storage buffers, other] x [descriptor sets] not counting ArraySize
diff --git a/Graphics/GraphicsEngineVulkan/src/EngineFactoryVk.cpp b/Graphics/GraphicsEngineVulkan/src/EngineFactoryVk.cpp
index 26845497..7e08a361 100644
--- a/Graphics/GraphicsEngineVulkan/src/EngineFactoryVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/EngineFactoryVk.cpp
@@ -279,7 +279,7 @@ void EngineFactoryVkImpl::CreateDeviceAndContextsVk(const EngineVkCreateInfo& _E
         // clang-format on
 
         const auto& DescrIndexingFeats = DeviceExtFeatures.DescriptorIndexing;
-        ENABLE_FEATURE(DescrIndexingFeats.runtimeDescriptorArray != VK_FALSE, ShaderResourceRuntimeArray, "shader resource runtime array is");
+        ENABLE_FEATURE(DescrIndexingFeats.runtimeDescriptorArray != VK_FALSE, ShaderResourceRuntimeArray, "Shader resource runtime array is");
 
         ENABLE_FEATURE(DeviceExtFeatures.AccelStruct.accelerationStructure != VK_FALSE && DeviceExtFeatures.RayTracingPipeline.rayTracingPipeline != VK_FALSE, RayTracing, "Ray tracing is");
 #undef FeatureSupport
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp
index 03c610d1..3dcb2df6 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp
@@ -249,12 +249,12 @@ inline PipelineResourceSignatureVkImpl::CACHE_GROUP PipelineResourceSignatureVkI
     if (WithDynamicOffset && !UseTexelBuffer)
     {
         if (Res.ResourceType == SHADER_RESOURCE_TYPE_CONSTANT_BUFFER)
-            return static_cast<CACHE_GROUP>(SetId * CACHE_GROUP_COUNT_PER_Type + CACHE_GROUP_DYN_UB);
+            return static_cast<CACHE_GROUP>(SetId * CACHE_GROUP_COUNT_PER_VAR_TYPE + CACHE_GROUP_DYN_UB);
 
         if (Res.ResourceType == SHADER_RESOURCE_TYPE_BUFFER_SRV || Res.ResourceType == SHADER_RESOURCE_TYPE_BUFFER_UAV)
-            return static_cast<CACHE_GROUP>(SetId * CACHE_GROUP_COUNT_PER_Type + CACHE_GROUP_DYN_SB);
+            return static_cast<CACHE_GROUP>(SetId * CACHE_GROUP_COUNT_PER_VAR_TYPE + CACHE_GROUP_DYN_SB);
     }
-    return static_cast<CACHE_GROUP>(SetId * CACHE_GROUP_COUNT_PER_Type + CACHE_GROUP_OTHER);
+    return static_cast<CACHE_GROUP>(SetId * CACHE_GROUP_COUNT_PER_VAR_TYPE + CACHE_GROUP_OTHER);
 }
 
 inline PipelineResourceSignatureVkImpl::DESCRIPTOR_SET_ID PipelineResourceSignatureVkImpl::VarTypeToDescriptorSetId(SHADER_RESOURCE_VARIABLE_TYPE VarType)
@@ -1697,9 +1697,9 @@ void PipelineResourceSignatureVkImpl::BindResource(IDeviceObject*         pObj,
     Helper.BindResource(pObj);
 }
 
-bool PipelineResourceSignatureVkImpl::IsBound(Uint32                 ArrayIndex,
-                                              Uint32                 ResIndex,
-                                              ShaderResourceCacheVk& ResourceCache) const
+bool PipelineResourceSignatureVkImpl::IsBound(Uint32                       ArrayIndex,
+                                              Uint32                       ResIndex,
+                                              const ShaderResourceCacheVk& ResourceCache) const
 {
     const auto&  ResDesc     = GetResourceDesc(ResIndex);
     const auto&  Attribs     = GetResourceAttribs(ResIndex);
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
index 841e6cdc..f40bbe19 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
@@ -774,8 +774,8 @@ void PipelineStateVkImpl::CreateDefaultSignature(const PipelineStateCreateInfo&
                     {
                         if (Res.ArraySize == 0)
                         {
-                            LOG_ERROR_AND_THROW("Is shader '", pShader->GetDesc().Name, "' resource '", Res.Name, "' uses runtime sized array, ",
-                                                "you must explicitlly set resource signature to specify array size");
+                            LOG_ERROR_AND_THROW("Resource '", Res.Name, "' in shader '", pShader->GetDesc().Name, "' is a runtime-sized array. ",
+                                                "You must use explicit resource signature to specify the array size.");
                         }
 
                         SHADER_RESOURCE_TYPE    Type;
@@ -935,15 +935,21 @@ void PipelineStateVkImpl::InitPipelineLayout(const PipelineStateCreateInfo& Crea
 
                         if ((Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) != (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER))
                         {
-                            LOG_ERROR_AND_THROW("AZ TODO");
+                            LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource '", SPIRVAttribs.Name,
+                                                "' that is", ((Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) ? "" : " not"),
+                                                " labeled as formatted buffer, while the same resource specified by the pipeline resource signature '",
+                                                Info.Signature->GetDesc().Name, "' is", ((ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) ? "" : " not"),
+                                                " labeled as such.");
                         }
 
-                        // ArraySize == 0 means that used runtime sized array and ArraySize in resource signature may be any non-zero value.
-                        if (SPIRVAttribs.ArraySize > 0 && ResDesc.ArraySize != SPIRVAttribs.ArraySize)
+                        // SPIRVAttribs.ArraySize == 0 means that the resource is a runtime-sized array and ResDesc.ArraySize from the
+                        // resource signature may have any non-zero value.
+                        VERIFY(ResDesc.ArraySize > 0, "ResDesc.ArraySize can't be zero. This error should've be caught by ValidatePipelineResourceSignatureDesc().");
+                        if (ResDesc.ArraySize < SPIRVAttribs.ArraySize)
                         {
-                            LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource with name '", SPIRVAttribs.Name,
-                                                "' and array size (", SPIRVAttribs.ArraySize, ") that is not compatible with array size '",
-                                                ResDesc.ArraySize, "' in pipeline resource signature '", Info.Signature->GetDesc().Name, "'.");
+                            LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource '", SPIRVAttribs.Name,
+                                                "' whose array size (", SPIRVAttribs.ArraySize, ") is greater than the array size (",
+                                                ResDesc.ArraySize, ") specified by the pipeline resource signature '", Info.Signature->GetDesc().Name, "'.");
                         }
                     }