From 2ffa86d824d5aac7b8e7d0e78a7335bf35d859e5 Mon Sep 17 00:00:00 2001 From: assiduous Date: Fri, 18 Dec 2020 21:15:15 -0800 Subject: ShaderResourceLayoutD3D12: updated error message for inconsistent variables in shaders from the same shader stage --- .../include/PipelineStateD3D12Impl.hpp | 1 - .../GraphicsEngineD3D12/include/RootSignature.hpp | 8 ++++++-- .../src/PipelineStateD3D12Impl.cpp | 14 ++++++------- Graphics/GraphicsEngineD3D12/src/RootSignature.cpp | 4 ++-- .../src/ShaderResourceLayoutD3D12.cpp | 24 +++++++++++++++++----- 5 files changed, 34 insertions(+), 17 deletions(-) (limited to 'Graphics/GraphicsEngineD3D12') 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 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 m_NumResources = {}; std::vector> 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 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(); auto& ExportDesc = *TempPool.Construct(); @@ -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); } -- cgit v1.2.3