From d053d3c7ced59e5671e908ffaf2628570f1da568 Mon Sep 17 00:00:00 2001 From: assiduous Date: Thu, 11 Feb 2021 18:58:34 -0800 Subject: Few fixes to RootParamsManager --- .../include/RootParamsManager.hpp | 11 ++-- .../src/PipelineResourceSignatureD3D12Impl.cpp | 12 ++--- .../GraphicsEngineD3D12/src/RootParamsManager.cpp | 60 ++++++++++++++++------ 3 files changed, 58 insertions(+), 25 deletions(-) (limited to 'Graphics/GraphicsEngineD3D12') diff --git a/Graphics/GraphicsEngineD3D12/include/RootParamsManager.hpp b/Graphics/GraphicsEngineD3D12/include/RootParamsManager.hpp index 7127ffcf..3941036e 100644 --- a/Graphics/GraphicsEngineD3D12/include/RootParamsManager.hpp +++ b/Graphics/GraphicsEngineD3D12/include/RootParamsManager.hpp @@ -49,7 +49,8 @@ class RootParameter public: RootParameter(ROOT_PARAMETER_GROUP Group, Uint32 RootIndex, - const D3D12_ROOT_PARAMETER& d3d12RootParam) noexcept; + const D3D12_ROOT_PARAMETER& d3d12RootParam, + Uint32 DescriptorTableSize = 0) noexcept; // clang-format off RootParameter (const RootParameter&) = delete; @@ -58,13 +59,15 @@ public: RootParameter& operator=(RootParameter&&) = delete; // clang-format on - // Initializes a descriptor range at the specified index. + // Initializes descriptor range at the specified index. // The parameter type must be D3D12_ROOT_PARAMETER_TYPE_DESCRIPTOR_TABLE. void InitDescriptorRange(UINT RangeIndex, const D3D12_DESCRIPTOR_RANGE& Range); ROOT_PARAMETER_GROUP GetGroup() const { return m_Group; } + static constexpr D3D12_DESCRIPTOR_RANGE_TYPE InvalidDescriptorRangeType = static_cast(0xFFFFFFFF); + Uint32 GetDescriptorTableSize() const { VERIFY(m_d3d12RootParam.ParameterType == D3D12_ROOT_PARAMETER_TYPE_DESCRIPTOR_TABLE, "Incorrect parameter type: descriptor table is expected"); @@ -123,7 +126,7 @@ public: return m_pRootViews[ViewInd]; } - // Adds a new root view and returns the pointer to it. + // Adds a new root view parameter and returns the pointer to it. RootParameter* AddRootView(D3D12_ROOT_PARAMETER_TYPE ParameterType, Uint32 RootIndex, UINT Register, @@ -131,7 +134,7 @@ public: D3D12_SHADER_VISIBILITY Visibility, ROOT_PARAMETER_GROUP RootType); - // Adds a new root table and returns the pointer to it. + // Adds a new root table parameter and returns the pointer to it. RootParameter* AddRootTable(Uint32 RootIndex, D3D12_SHADER_VISIBILITY Visibility, ROOT_PARAMETER_GROUP RootType, diff --git a/Graphics/GraphicsEngineD3D12/src/PipelineResourceSignatureD3D12Impl.cpp b/Graphics/GraphicsEngineD3D12/src/PipelineResourceSignatureD3D12Impl.cpp index b66a0ca1..618b0d7c 100644 --- a/Graphics/GraphicsEngineD3D12/src/PipelineResourceSignatureD3D12Impl.cpp +++ b/Graphics/GraphicsEngineD3D12/src/PipelineResourceSignatureD3D12Impl.cpp @@ -45,7 +45,7 @@ namespace Diligent namespace { -constexpr D3D12_DESCRIPTOR_RANGE_TYPE GetDescriptorRangeType(SHADER_RESOURCE_TYPE ResType) +D3D12_DESCRIPTOR_RANGE_TYPE GetDescriptorRangeType(SHADER_RESOURCE_TYPE ResType) { static_assert(SHADER_RESOURCE_TYPE_LAST == SHADER_RESOURCE_TYPE_ACCEL_STRUCT, "Please update the switch below to handle the new resource type"); @@ -63,7 +63,7 @@ constexpr D3D12_DESCRIPTOR_RANGE_TYPE GetDescriptorRangeType(SHADER_RESOURCE_TYP case SHADER_RESOURCE_TYPE_INPUT_ATTACHMENT: default: UNEXPECTED("Unknown resource type"); - return static_cast(~0u); + return RootParameter::InvalidDescriptorRangeType; } } @@ -572,7 +572,7 @@ void PipelineResourceSignatureD3D12Impl::CreateLayout() const bool IsRuntimeSizedArray = (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY) != 0; const auto DescriptorRangeType = GetDescriptorRangeType(ResDesc.ResourceType); - Uint32 BindPoint = IsRuntimeSizedArray ? 0 : NumResources[DescriptorRangeType]; + Uint32 Register = IsRuntimeSizedArray ? 0 : NumResources[DescriptorRangeType]; Uint32 Space = (IsRuntimeSizedArray ? m_NumSpaces++ : 0); Uint32 SRBRootIndex = ResourceAttribs::InvalidSRBRootIndex; Uint32 SRBOffsetFromTableStart = ResourceAttribs::InvalidOffset; @@ -623,8 +623,8 @@ void PipelineResourceSignatureD3D12Impl::CreateLayout() } else { - BindPoint = ImmutableSampler.ShaderRegister; - Space = ImmutableSampler.RegisterSpace; + Register = ImmutableSampler.ShaderRegister; + Space = ImmutableSampler.RegisterSpace; // Use previous bind point and decrease resource counter if (!IsRuntimeSizedArray) @@ -657,7 +657,7 @@ void PipelineResourceSignatureD3D12Impl::CreateLayout() if (ImmutableSampler.IsAssigned()) continue; - constexpr auto DescriptorRangeType = GetDescriptorRangeType(SHADER_RESOURCE_TYPE_SAMPLER); + const auto DescriptorRangeType = GetDescriptorRangeType(SHADER_RESOURCE_TYPE_SAMPLER); ImmutableSampler.RegisterSpace = FirstSpace; ImmutableSampler.ShaderRegister = NumResources[DescriptorRangeType]; diff --git a/Graphics/GraphicsEngineD3D12/src/RootParamsManager.cpp b/Graphics/GraphicsEngineD3D12/src/RootParamsManager.cpp index 4021a7c9..756568f8 100644 --- a/Graphics/GraphicsEngineD3D12/src/RootParamsManager.cpp +++ b/Graphics/GraphicsEngineD3D12/src/RootParamsManager.cpp @@ -35,12 +35,20 @@ namespace Diligent RootParameter::RootParameter(ROOT_PARAMETER_GROUP Group, Uint32 RootIndex, - const D3D12_ROOT_PARAMETER& d3d12RootParam) noexcept : + const D3D12_ROOT_PARAMETER& d3d12RootParam, + Uint32 DescriptorTableSize) noexcept : m_d3d12RootParam{d3d12RootParam}, m_RootIndex{static_cast(RootIndex)}, - m_Group{Group} + m_Group{Group}, + m_DescriptorTableSize{DescriptorTableSize} { VERIFY(m_RootIndex == RootIndex, "Root index (", RootIndex, ") exceeds representable range"); + VERIFY(DescriptorTableSize == 0 || m_d3d12RootParam.ParameterType == D3D12_ROOT_PARAMETER_TYPE_DESCRIPTOR_TABLE, + "Non-zer descriptor table size is only allowed for DESCRIPTOR_TABLE parameters"); +#ifdef DILIGENT_DEBUG + if (m_d3d12RootParam.ParameterType == D3D12_ROOT_PARAMETER_TYPE_DESCRIPTOR_TABLE) + DbgValidateAsTable(); +#endif } void RootParameter::InitDescriptorRange(UINT RangeIndex, @@ -53,10 +61,14 @@ void RootParameter::InitDescriptorRange(UINT RangeIndex VERIFY(RangeIndex < d3d12Tbl.NumDescriptorRanges, "Invalid descriptor range index"); auto& DstRange = const_cast(d3d12Tbl.pDescriptorRanges[RangeIndex]); - VERIFY(DstRange.RangeType == static_cast(0xFFFFFFFF), "Descriptor range has already been initialized."); + VERIFY(DstRange.RangeType == InvalidDescriptorRangeType, "Descriptor range has already been initialized."); DstRange = Range; m_DescriptorTableSize = std::max(m_DescriptorTableSize, Range.OffsetInDescriptorsFromTableStart + Range.NumDescriptors); + +#ifdef DILIGENT_DEBUG + DbgValidateAsTable(); +#endif } bool RootParameter::operator==(const RootParameter& rhs) const @@ -123,7 +135,21 @@ void RootParameter::DbgValidateAsTable() const for (Uint32 r = 0; r < d3d12SrcTbl.NumDescriptorRanges; ++r) { const auto& Range = d3d12SrcTbl.pDescriptorRanges[r]; - dbgTableSize = std::max(dbgTableSize, Range.OffsetInDescriptorsFromTableStart + Range.NumDescriptors); + if (r > 0) + { + const auto& PrevRange = d3d12SrcTbl.pDescriptorRanges[r - 1]; + if (PrevRange.RangeType != InvalidDescriptorRangeType) + { + VERIFY(Range.RangeType == InvalidDescriptorRangeType || PrevRange.OffsetInDescriptorsFromTableStart + PrevRange.NumDescriptors == Range.OffsetInDescriptorsFromTableStart, + "Descriptors should be tightly packed in the table"); + } + else + { + VERIFY(Range.RangeType == InvalidDescriptorRangeType, "All uninitialized ranges must be contiguous"); + } + } + if (Range.RangeType != InvalidDescriptorRangeType) + dbgTableSize = std::max(dbgTableSize, Range.OffsetInDescriptorsFromTableStart + Range.NumDescriptors); } } VERIFY(dbgTableSize == GetDescriptorTableSize(), "Incorrect descriptor table size"); @@ -177,29 +203,31 @@ void RootParamsManager::Extend(Uint32 NumExtraRootTables, #endif // Note: this order is more efficient than views->tables->ranges - auto* pNewRootTables = reinterpret_cast(pNewMemory.get()); - auto* pNewRootViews = pNewRootTables + (m_NumRootTables + NumExtraRootTables); - auto* pDescriptorRangePtr = reinterpret_cast(pNewRootViews + m_NumRootViews + NumExtraRootViews); + auto* const pNewRootTables = reinterpret_cast(pNewMemory.get()); + auto* const pNewRootViews = pNewRootTables + m_NumRootTables + NumExtraRootTables; + auto* const pDescriptorRanges = reinterpret_cast(pNewRootViews + m_NumRootViews + NumExtraRootViews); // Copy root tables to new memory + auto* pCurrDescrRangePtr = pDescriptorRanges; for (Uint32 rt = 0; rt < m_NumRootTables + NumExtraRootTables; ++rt) { const auto& SrcTbl = rt < m_NumRootTables ? GetRootTable(rt) : ExtraRootTables[rt - m_NumRootTables]; #ifdef DILIGENT_DEBUG SrcTbl.DbgValidateAsTable(); #endif - auto& d3d12SrcTbl = static_cast(SrcTbl).DescriptorTable; - auto NumRanges = d3d12SrcTbl.NumDescriptorRanges; + const auto& d3d12SrcTbl = static_cast(SrcTbl).DescriptorTable; + + auto NumRanges = d3d12SrcTbl.NumDescriptorRanges; if (rt == RootTableToAddRanges) { - VERIFY(d3d12SrcTbl.pDescriptorRanges == nullptr, "Adding extra descriptors to a new table. This is likely not intended."); + VERIFY(d3d12SrcTbl.pDescriptorRanges != nullptr, "Adding extra descriptors to a new table. This is likely not intended."); NumRanges += NumExtraDescriptorRanges; } // Copy existing ranges, if any (pDescriptorRanges == null for extra descriptor tables) if (d3d12SrcTbl.pDescriptorRanges != nullptr) { - memcpy(pDescriptorRangePtr, d3d12SrcTbl.pDescriptorRanges, d3d12SrcTbl.NumDescriptorRanges * sizeof(D3D12_DESCRIPTOR_RANGE)); + memcpy(pCurrDescrRangePtr, d3d12SrcTbl.pDescriptorRanges, d3d12SrcTbl.NumDescriptorRanges * sizeof(D3D12_DESCRIPTOR_RANGE)); } new (pNewRootTables + rt) RootParameter{ @@ -207,13 +235,15 @@ void RootParamsManager::Extend(Uint32 NumExtraRootTables, D3D12_ROOT_PARAMETER // { D3D12_ROOT_PARAMETER_TYPE_DESCRIPTOR_TABLE, - D3D12_ROOT_DESCRIPTOR_TABLE{NumRanges, pDescriptorRangePtr}, + D3D12_ROOT_DESCRIPTOR_TABLE{NumRanges, pCurrDescrRangePtr}, SrcTbl.GetShaderVisibility() // - } // + }, + SrcTbl.GetDescriptorTableSize() // }; - pDescriptorRangePtr += NumRanges; + pCurrDescrRangePtr += NumRanges; } + VERIFY_EXPR(pCurrDescrRangePtr == pDescriptorRanges + NewRangesCount); // Copy root views to new memory for (Uint32 rv = 0; rv < m_NumRootViews + NumExtraRootViews; ++rv) @@ -289,7 +319,7 @@ RootParameter* RootParamsManager::ExtendRootTable(Uint32 RootTableInd, Uint32 Nu { VERIFY_EXPR(RootTableInd < m_NumRootTables); Extend(0, nullptr, 0, nullptr, NumExtraRanges, RootTableInd); - return &m_pRootTables[RootTableInd - 1]; + return &m_pRootTables[RootTableInd]; } bool RootParamsManager::operator==(const RootParamsManager& RootParams) const -- cgit v1.2.3