Few fixes to RootParamsManager

3 files changed, 58 insertions, 25 deletions

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<D3D12_DESCRIPTOR_RANGE_TYPE>(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<D3D12_DESCRIPTOR_RANGE_TYPE>(~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<decltype(m_RootIndex)>(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<D3D12_DESCRIPTOR_RANGE&>(d3d12Tbl.pDescriptorRanges[RangeIndex]);
-    VERIFY(DstRange.RangeType == static_cast<D3D12_DESCRIPTOR_RANGE_TYPE>(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<RootParameter*>(pNewMemory.get());
-    auto* pNewRootViews       = pNewRootTables + (m_NumRootTables + NumExtraRootTables);
-    auto* pDescriptorRangePtr = reinterpret_cast<D3D12_DESCRIPTOR_RANGE*>(pNewRootViews + m_NumRootViews + NumExtraRootViews);
+    auto* const pNewRootTables    = reinterpret_cast<RootParameter*>(pNewMemory.get());
+    auto* const pNewRootViews     = pNewRootTables + m_NumRootTables + NumExtraRootTables;
+    auto* const pDescriptorRanges = reinterpret_cast<D3D12_DESCRIPTOR_RANGE*>(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<const D3D12_ROOT_PARAMETER&>(SrcTbl).DescriptorTable;
-        auto  NumRanges   = d3d12SrcTbl.NumDescriptorRanges;
+        const auto& d3d12SrcTbl = static_cast<const D3D12_ROOT_PARAMETER&>(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