ShaderResourceLayoutD3D12: updated error message for inconsistent variables in shaders from the same shader stage

5 files changed, 34 insertions, 17 deletions

diff --git a/Graphics/GraphicsEngineD3D12/include/PipelineStateD3D12Impl.hpp b/Graphics/GraphicsEngineD3D12/include/PipelineStateD3D12Impl.hpp
index 52ab2db3..198acffd 100644
--- a/Graphics/GraphicsEngineD3D12/include/PipelineStateD3D12Impl.hpp
+++ b/Graphics/GraphicsEngineD3D12/include/PipelineStateD3D12Impl.hpp
@@ -154,7 +154,6 @@ private:
     CComPtr<ID3D12DeviceChild> m_pd3d12PSO;
     RootSignature              m_RootSig;
 
-    // Must be defined before default SRB
     SRBMemoryAllocator m_SRBMemAllocator;
 
     ShaderResourceLayoutD3D12*  m_pShaderResourceLayouts = nullptr; // [m_NumShaderStages * 2]
diff --git a/Graphics/GraphicsEngineD3D12/include/RootSignature.hpp b/Graphics/GraphicsEngineD3D12/include/RootSignature.hpp
index 2d5a968a..c8ba2fa2 100644
--- a/Graphics/GraphicsEngineD3D12/include/RootSignature.hpp
+++ b/Graphics/GraphicsEngineD3D12/include/RootSignature.hpp
@@ -37,8 +37,6 @@
 namespace Diligent
 {
 
-D3D12_DESCRIPTOR_HEAP_TYPE dbgHeapTypeFromRangeType(D3D12_DESCRIPTOR_RANGE_TYPE RangeType);
-
 class RootParameter
 {
 public:
@@ -490,6 +488,10 @@ public:
                               const char*                     SamplerSuffix,
                               const D3DShaderResourceAttribs& ShaderResAttribs);
 
+    // Allocates root signature slot for the given resource.
+    // For graphics and compute pipelines, BindPoint is the same as the original bind point.
+    // For ray-tracing pipeline, BindPoint will be overriden. Bind points are then
+    // remapped by PSO constructor.
     void AllocateResourceSlot(SHADER_TYPE                     ShaderType,
                               PIPELINE_TYPE                   PipelineType,
                               const D3DShaderResourceAttribs& ShaderResAttribs,
@@ -535,6 +537,8 @@ private:
 
     RootSignature& m_RootSig;
 
+    // Resource counters for every descriptor range type used to assign bind points
+    // for ray-tracing shaders.
     std::array<Uint32, D3D12_DESCRIPTOR_RANGE_TYPE_SAMPLER + 1> m_NumResources = {};
 
     std::vector<ImmutableSamplerAttribs, STDAllocatorRawMem<ImmutableSamplerAttribs>> m_ImmutableSamplers;
diff --git a/Graphics/GraphicsEngineD3D12/src/PipelineStateD3D12Impl.cpp b/Graphics/GraphicsEngineD3D12/src/PipelineStateD3D12Impl.cpp
index ea6b3a5c..49d9440d 100644
--- a/Graphics/GraphicsEngineD3D12/src/PipelineStateD3D12Impl.cpp
+++ b/Graphics/GraphicsEngineD3D12/src/PipelineStateD3D12Impl.cpp
@@ -111,7 +111,7 @@ void BuildRTPipelineDescription(const RayTracingPipelineStateCreateInfo& CreateI
 {
 #define LOG_PSO_ERROR_AND_THROW(...) LOG_ERROR_AND_THROW("Description of ray tracing PSO '", CreateInfo.PSODesc.Name, "' is invalid: ", ##__VA_ARGS__)
 
-    Uint32 ShaderIndex = 0;
+    Uint32 UnnamedShaderIndex = 0;
 
     std::unordered_map<IShader*, LPCWSTR> UniqueShaders;
 
@@ -131,8 +131,8 @@ void BuildRTPipelineDescription(const RayTracingPipelineStateCreateInfo& CreateI
         if (pShader == nullptr)
             return nullptr;
 
-        auto Result = UniqueShaders.emplace(pShader, nullptr);
-        if (Result.second)
+        auto it_inserted = UniqueShaders.emplace(pShader, nullptr);
+        if (it_inserted.second)
         {
             auto&  LibDesc      = *TempPool.Construct<D3D12_DXIL_LIBRARY_DESC>();
             auto&  ExportDesc   = *TempPool.Construct<D3D12_EXPORT_DESC>();
@@ -155,16 +155,16 @@ void BuildRTPipelineDescription(const RayTracingPipelineStateCreateInfo& CreateI
             if (Name != nullptr)
                 ExportDesc.Name = TempPool.CopyWString(Name);
             else
-                ExportDesc.Name = ShaderIndexToStr(++ShaderIndex);
+                ExportDesc.Name = ShaderIndexToStr(++UnnamedShaderIndex);
 
             Subobjects.push_back({D3D12_STATE_SUBOBJECT_TYPE_DXIL_LIBRARY, &LibDesc});
             ShaderBlobs.push_back(pBlob);
 
-            Result.first->second = ExportDesc.Name;
+            it_inserted.first->second = ExportDesc.Name;
             return ExportDesc.Name;
         }
         else
-            return Result.first->second;
+            return it_inserted.first->second;
     };
 
     ShaderBlobs.reserve(CreateInfo.GeneralShaderCount + CreateInfo.TriangleHitShaderCount + CreateInfo.ProceduralHitShaderCount);
@@ -776,7 +776,7 @@ void PipelineStateD3D12Impl::InitResourceLayouts(const PipelineStateCreateInfo&
                                                  TShaderStages&                 ShaderStages,
                                                  LocalRootSignature*            pLocalRoot)
 {
-    auto        pd3d12Device   = GetDevice()->GetD3D12Device();
+    auto* const pd3d12Device   = GetDevice()->GetD3D12Device();
     const auto& ResourceLayout = m_Desc.ResourceLayout;
 
 #ifdef DILIGENT_DEVELOPMENT
diff --git a/Graphics/GraphicsEngineD3D12/src/RootSignature.cpp b/Graphics/GraphicsEngineD3D12/src/RootSignature.cpp
index 5b45511b..336c8a94 100644
--- a/Graphics/GraphicsEngineD3D12/src/RootSignature.cpp
+++ b/Graphics/GraphicsEngineD3D12/src/RootSignature.cpp
@@ -231,8 +231,8 @@ void RootSignatureBuilder::AllocateResourceSlot(SHADER_TYPE
 
     if (ShaderType & RAY_TRACING_SHADER_TYPES)
     {
-        // For ray tracing shaders, original bind points are ignored as
-        // they will be remapped anyway.
+        // For ray tracing shaders, original bind points are ignored and
+        // will be remapped later by the PSO constructor.
         BindPoint = m_NumResources[RangeType];
         m_NumResources[RangeType] += ShaderResAttribs.BindCount;
     }
diff --git a/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp b/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp
index 75cb408b..41abfd9e 100644
--- a/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp
+++ b/Graphics/GraphicsEngineD3D12/src/ShaderResourceLayoutD3D12.cpp
@@ -311,12 +311,26 @@ void ShaderResourceLayoutD3D12::Initialize(ID3D12Device*
         }
         else
         {
-            // merge with existing
+            // Merge with existing
             auto& ExistingRes = GetResource(ResIter->second);
-            VERIFY_EXPR(ExistingRes.VariableType == VarType);
-            VERIFY_EXPR(ExistingRes.Attribs.GetInputType() == Attribs.GetInputType());
-            VERIFY_EXPR(ExistingRes.Attribs.GetSRVDimension() == Attribs.GetSRVDimension());
-            VERIFY_EXPR(ExistingRes.Attribs.BindCount == Attribs.BindCount);
+            VERIFY(ExistingRes.VariableType == VarType,
+                   "The type of variable '", Attribs.Name, "' does not match the type determined for previous shaders. This appears to be a bug.");
+
+            DEV_CHECK_ERR(ExistingRes.Attribs.GetInputType() == Attribs.GetInputType(),
+                          "Shader variable '", Attribs.Name,
+                          "' exists in multiple shaders from the same shader stage, but its input type is not consistent between "
+                          "shaders. All variables with the same name from the same shader stage must have the same input type.");
+
+            DEV_CHECK_ERR(ExistingRes.Attribs.GetSRVDimension() == Attribs.GetSRVDimension(),
+                          "Shader variable '", Attribs.Name,
+                          "' exists in multiple shaders from the same shader stage, but its SRV dimension is not consistent between "
+                          "shaders. All variables with the same name from the same shader stage must have the same SRV dimension.");
+
+            DEV_CHECK_ERR(ExistingRes.Attribs.BindCount == Attribs.BindCount,
+                          "Shader variable '", Attribs.Name,
+                          "' exists in multiple shaders from the same shader stage, but its array size is not consistent between "
+                          "shaders. All variables with the same name from the same shader stage must have the same array size.");
+
             VERIFY_EXPR(ExistingRes.Attribs.BindPoint == Attribs.BindPoint ||
                         Attribs.BindPoint == D3DShaderResourceAttribs::InvalidBindPoint);
         }