Unified pipeline resource compatibility validation in D3D12 and Vk; added more PRS creation failure tests

4 files changed, 75 insertions, 70 deletions

diff --git a/Graphics/GraphicsEngine/include/PipelineStateBase.hpp b/Graphics/GraphicsEngine/include/PipelineStateBase.hpp
index 5d1e65e9..573dca44 100644
--- a/Graphics/GraphicsEngine/include/PipelineStateBase.hpp
+++ b/Graphics/GraphicsEngine/include/PipelineStateBase.hpp
@@ -51,6 +51,16 @@ void ValidateGraphicsPipelineCreateInfo(const GraphicsPipelineStateCreateInfo& C
 void ValidateComputePipelineCreateInfo(const ComputePipelineStateCreateInfo& CreateInfo) noexcept(false);
 void ValidateRayTracingPipelineCreateInfo(IRenderDevice* pDevice, Uint32 MaxRecursion, const RayTracingPipelineStateCreateInfo& CreateInfo) noexcept(false);
 
+/// Validates that pipeline resource description 'ResDesc' is compatible with the actual resource
+/// attributes and throws an exception in case of an error.
+void ValidatePipelineResourceCompatibility(const PipelineResourceDesc& ResDesc,
+                                           SHADER_RESOURCE_TYPE        Type,
+                                           PIPELINE_RESOURCE_FLAGS     ResourceFlags,
+                                           Uint32                      ArraySize,
+                                           const char*                 ShaderName,
+                                           const char*                 SignatureName) noexcept(false);
+
+
 /// Copies ray tracing shader group names and also initializes the mapping from the group name to its index.
 void CopyRTShaderGroupNames(std::unordered_map<HashMapStringKey, Uint32, HashMapStringKey::Hasher>& NameToGroupIndex,
                             const RayTracingPipelineStateCreateInfo&                                CreateInfo,
diff --git a/Graphics/GraphicsEngine/src/PipelineStateBase.cpp b/Graphics/GraphicsEngine/src/PipelineStateBase.cpp
index c1a314a6..4c095669 100644
--- a/Graphics/GraphicsEngine/src/PipelineStateBase.cpp
+++ b/Graphics/GraphicsEngine/src/PipelineStateBase.cpp
@@ -514,6 +514,60 @@ void CopyRTShaderGroupNames(std::unordered_map<HashMapStringKey, Uint32, HashMap
 #undef VALIDATE_SHADER_TYPE
 #undef LOG_PSO_ERROR_AND_THROW
 
+void ValidatePipelineResourceCompatibility(const PipelineResourceDesc& ResDesc,
+                                           SHADER_RESOURCE_TYPE        Type,
+                                           PIPELINE_RESOURCE_FLAGS     ResourceFlags,
+                                           Uint32                      ArraySize,
+                                           const char*                 ShaderName,
+                                           const char*                 SignatureName) noexcept(false)
+{
+    if (Type != ResDesc.ResourceType)
+    {
+        LOG_ERROR_AND_THROW("Shader '", ShaderName, "' contains resource with name '", ResDesc.Name,
+                            "' and type '", GetShaderResourceTypeLiteralName(Type), "' that is not compatible with type '",
+                            GetShaderResourceTypeLiteralName(ResDesc.ResourceType), "' specified in pipeline resource signature '", SignatureName, "'.");
+    }
+
+    if ((ResourceFlags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) != (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER))
+    {
+        LOG_ERROR_AND_THROW("Shader '", ShaderName, "' contains resource '", ResDesc.Name,
+                            "' that is", ((ResourceFlags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) ? "" : " not"),
+                            " labeled as formatted buffer, while the same resource specified by the pipeline resource signature '",
+                            SignatureName, "' is", ((ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) ? "" : " not"),
+                            " labeled as such.");
+    }
+
+    VERIFY(ResDesc.ArraySize > 0, "ResDesc.ArraySize can't be zero. This error should've be caught by ValidatePipelineResourceSignatureDesc().");
+
+    if (ArraySize == 0)
+    {
+        // ArraySize == 0 means that the resource is a runtime-sized array and ResDesc.ArraySize from the
+        // resource signature may have any non-zero value.
+        if ((ResDesc.Flags & PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY) == 0)
+        {
+            LOG_ERROR_AND_THROW("Shader '", ShaderName, "' contains resource '", ResDesc.Name,
+                                "' that is a runtime-sized array, but in the resource signature '", SignatureName,
+                                "' the resource is defined without the PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY flag.");
+        }
+    }
+    else
+    {
+        if (ResDesc.ArraySize < ArraySize)
+        {
+            LOG_ERROR_AND_THROW("Shader '", ShaderName, "' contains resource '", ResDesc.Name,
+                                "' whose array size (", ArraySize, ") is greater than the array size (",
+                                ResDesc.ArraySize, ") specified by the pipeline resource signature '", SignatureName, "'.");
+        }
+
+        //if (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY)
+        //{
+        //    LOG_WARNING_MESSAGE("Shader '", ShaderName, "' contains resource with name '", ResDesc.Name,
+        //                        "' that is defined in resource signature '", SignatureName,
+        //                        "' with flag PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY, but the resource is not a runtime-sized array.");
+        //}
+    }
+}
+
 void CorrectGraphicsPipelineDesc(GraphicsPipelineDesc& GraphicsPipeline) noexcept
 {
     CorrectBlendStateDesc(GraphicsPipeline);
diff --git a/Graphics/GraphicsEngineD3D12/src/PipelineStateD3D12Impl.cpp b/Graphics/GraphicsEngineD3D12/src/PipelineStateD3D12Impl.cpp
index 1ac3e9dd..c98b1090 100644
--- a/Graphics/GraphicsEngineD3D12/src/PipelineStateD3D12Impl.cpp
+++ b/Graphics/GraphicsEngineD3D12/src/PipelineStateD3D12Impl.cpp
@@ -650,7 +650,7 @@ void PipelineStateD3D12Impl::InitRootSignature(const PipelineStateCreateInfo& Cr
             if (IsDXILBytecode(pBytecode->GetBufferPointer(), pBytecode->GetBufferSize()))
             {
                 if (!compiler)
-                    LOG_ERROR_AND_THROW("DXC compiler is not exists, can not remap resource bindings");
+                    LOG_ERROR_AND_THROW("DXC compiler does not exists, can not remap resource bindings");
 
                 if (!compiler->RemapResourceBindings(ResourceMap, reinterpret_cast<IDxcBlob*>(pBytecode.p), reinterpret_cast<IDxcBlob**>(&pBlob)))
                     LOG_ERROR_AND_THROW("Failed to remap resource bindings in shader '", pShader->GetDesc().Name, "'.");
@@ -731,52 +731,11 @@ void PipelineStateD3D12Impl::DvpValidateShaderResources(const ShaderD3D12Impl* p
 
             if (ResAttribution.ResourceIndex != ResourceAttribution::InvalidResourceIndex)
             {
-                const auto& ResDesc = pSignature->GetResourceDesc(ResAttribution.ResourceIndex);
-
-                auto ResourceType = ResDesc.ResourceType;
-                if (ResourceType == SHADER_RESOURCE_TYPE_INPUT_ATTACHMENT)
-                    ResourceType = SHADER_RESOURCE_TYPE_TEXTURE_SRV;
-                if (Type != ResourceType)
-                {
-                    LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource with name '", Attribs.Name,
-                                        "' and type '", GetShaderResourceTypeLiteralName(Type), "' that is not compatible with type '",
-                                        GetShaderResourceTypeLiteralName(ResDesc.ResourceType), "' in pipeline resource signature '", pSignature->GetDesc().Name, "'.");
-                }
-
-                if ((Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) != (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER))
-                {
-                    LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource '", Attribs.Name,
-                                        "' that is", ((Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) ? "" : " not"),
-                                        " labeled as formatted buffer, while the same resource specified by the pipeline resource signature '",
-                                        pSignature->GetDesc().Name, "' is", ((ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) ? "" : " not"),
-                                        " labeled as such.");
-                }
-
-                if (Attribs.BindCount == 0)
-                {
-                    if ((ResDesc.Flags & PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY) == 0)
-                    {
-                        LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource with name '", Attribs.Name,
-                                            "' that is runtime-sized array, but in resource signature '", pSignature->GetDesc().Name,
-                                            "' resource defined without PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY flag.");
-                    }
-                }
-                else
-                {
-                    if (ResDesc.ArraySize < Attribs.BindCount)
-                    {
-                        LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource '", Attribs.Name,
-                                            "' whose array size (", Attribs.BindCount, ") is greater than the array size (",
-                                            ResDesc.ArraySize, ") specified by the pipeline resource signature '", pSignature->GetDesc().Name, "'.");
-                    }
-
-                    if (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY)
-                    {
-                        LOG_WARNING_MESSAGE("Shader '", pShader->GetDesc().Name, "' contains resource with name '", Attribs.Name,
-                                            "' that defined in resource signature '", pSignature->GetDesc().Name,
-                                            "' with flag PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY, but resource is not a runtime-sized array.");
-                    }
-                }
+                auto ResDesc = pSignature->GetResourceDesc(ResAttribution.ResourceIndex);
+                if (ResDesc.ResourceType == SHADER_RESOURCE_TYPE_INPUT_ATTACHMENT)
+                    ResDesc.ResourceType = SHADER_RESOURCE_TYPE_TEXTURE_SRV;
+                ValidatePipelineResourceCompatibility(ResDesc, Type, Flags, Attribs.BindCount,
+                                                      pShader->GetDesc().Name, pSignature->GetDesc().Name);
             }
             else if (ResAttribution.ImmutableSamplerIndex != ResourceAttribution::InvalidResourceIndex)
             {
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
index ade2d0f1..3a3f8092 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
@@ -922,8 +922,8 @@ void PipelineStateVkImpl::InitPipelineLayout(const PipelineStateCreateInfo& Crea
                     }
                     if (!Info)
                     {
-                        LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource with name '", SPIRVAttribs.Name,
-                                            "' that is not present in any pipeline resource signature that is used to create pipeline state '",
+                        LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource '", SPIRVAttribs.Name,
+                                            "' that is not present in any pipeline resource signature used to create pipeline state '",
                                             m_Desc.Name, "'.");
                     }
 
@@ -934,33 +934,15 @@ void PipelineStateVkImpl::InitPipelineLayout(const PipelineStateCreateInfo& Crea
                     {
                         LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource with name '", SPIRVAttribs.Name,
                                             "' and type '", GetShaderResourceTypeLiteralName(Type), "' that is not compatible with type '",
-                                            GetShaderResourceTypeLiteralName(Info.Type), "' in pipeline resource signature '", Info.Signature->GetDesc().Name, "'.");
+                                            GetShaderResourceTypeLiteralName(Info.Type), "' specified in pipeline resource signature '", Info.Signature->GetDesc().Name, "'.");
                     }
 
                     if (Info.ResIndex != ~0u)
                     {
                         const auto& ResDesc = Info.Signature->GetResourceDesc(Info.ResIndex);
-
-                        if ((Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) != (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER))
-                        {
-                            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.");
-                        }
-
-                        // 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 '", 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, "'.");
-                        }
+                        ValidatePipelineResourceCompatibility(ResDesc, Type, Flags, SPIRVAttribs.ArraySize,
+                                                              pShader->GetDesc().Name, Info.Signature->GetDesc().Name);
                     }
-
                     SPIRV[SPIRVAttribs.BindingDecorationOffset]       = Info.BindingIndex;
                     SPIRV[SPIRVAttribs.DescriptorSetDecorationOffset] = Info.DescrSetIndex;