Add PIPELINE_RESOURCE_FLAG_VULKAN_PUSH_CONSTANT to PRS so we don't to have to deal with SPIRVShaderResourceAttribs stuffs in PRS creation.

hzqst · hzqst · commit f63da22f4bfa · 2025-12-07T16:59:58.000+08:00
diff --git a/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h b/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h
@@ -148,13 +148,12 @@ DILIGENT_TYPED_ENUM(PIPELINE_RESOURCE_FLAGS, Uint8)
     /// \note This flag is only valid in Vulkan.
     PIPELINE_RESOURCE_FLAG_GENERAL_INPUT_ATTACHMENT = 1u << 5,
 
-    /// Internal flag indicating that this inline constant is a Vulkan push constant
+    /// Indicates that this inline constant is a Vulkan push constant
     /// (declared with push_constant storage class in SPIR-V).
-    /// This flag is automatically set by the engine during shader reflection and should not
-    /// be set manually by the application.
-    ///
-    /// When this flag is set along with PIPELINE_RESOURCE_FLAG_INLINE_CONSTANTS, the engine
-    /// will use vkCmdPushConstants instead of a dynamic uniform buffer to update the constants.
+    /// This flag is must be explicitly set when creating PipelineResourceSignature
+    /// And must be used together with PIPELINE_RESOURCE_FLAG_INLINE_CONSTANTS
+    /// When this flag is set, the engine will use vkCmdPushConstants
+    /// instead of a dynamic uniform buffer to update the constants.
     ///
     /// \note This flag is only valid in Vulkan and is always combined with
     ///       PIPELINE_RESOURCE_FLAG_INLINE_CONSTANTS.
diff --git a/Graphics/GraphicsEngine/src/PipelineStateBase.cpp b/Graphics/GraphicsEngine/src/PipelineStateBase.cpp
@@ -849,6 +849,15 @@ void ValidatePipelineResourceCompatibility(const PipelineResourceDesc& ResDesc,
                             GetShaderResourceTypeLiteralName(ResDesc.ResourceType), "' specified in pipeline resource signature '", SignatureName, "'.");
     }
 
+    if ((ResourceFlags & PIPELINE_RESOURCE_FLAG_VULKAN_PUSH_CONSTANT) != (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_VULKAN_PUSH_CONSTANT))
+    {
+        LOG_ERROR_AND_THROW("Shader '", ShaderName, "' contains resource '", ResDesc.Name,
+                            "' that is", ((ResourceFlags & PIPELINE_RESOURCE_FLAG_VULKAN_PUSH_CONSTANT) ? "" : " not"),
+                            " labeled as vulkan's push constant, while the same resource specified by the pipeline resource signature '",
+                            SignatureName, "' is", ((ResDesc.Flags & PIPELINE_RESOURCE_FLAG_VULKAN_PUSH_CONSTANT) ? "" : " not"),
+                            " labeled as such.");
+    }
+
     if ((ResourceFlags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) != (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER))
     {
         LOG_ERROR_AND_THROW("Shader '", ShaderName, "' contains resource '", ResDesc.Name,
diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineResourceAttribsVk.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineResourceAttribsVk.hpp
@@ -68,12 +68,13 @@ struct PipelineResourceAttribsVk
 private:
     static constexpr Uint32 _BindingIndexBits    = 16;
     static constexpr Uint32 _SamplerIndBits      = 16;
-    static constexpr Uint32 _ArraySizeBits       = 25;
+    static constexpr Uint32 _ArraySizeBits       = 24;
     static constexpr Uint32 _DescrTypeBits       = 5;
     static constexpr Uint32 _DescrSetBits        = 1;
     static constexpr Uint32 _SamplerAssignedBits = 1;
+    static constexpr Uint32 _IsPushConstantBits  = 1;
 
-    static_assert((_BindingIndexBits + _ArraySizeBits + _SamplerIndBits + _DescrTypeBits + _DescrSetBits + _SamplerAssignedBits) % 32 == 0, "Bits are not optimally packed");
+    static_assert((_BindingIndexBits + _ArraySizeBits + _SamplerIndBits + _DescrTypeBits + _DescrSetBits + _SamplerAssignedBits + _IsPushConstantBits) % 32 == 0, "Bits are not optimally packed");
 
     // clang-format off
     static_assert((1u << _DescrTypeBits)    >  static_cast<Uint32>(DescriptorType::Count), "Not enough bits to store DescriptorType values");
@@ -88,10 +89,11 @@ struct PipelineResourceAttribsVk
     // clang-format off
     const Uint32  BindingIndex         : _BindingIndexBits;    // Binding in the descriptor set
     const Uint32  SamplerInd           : _SamplerIndBits;      // Index of the assigned sampler in m_Desc.Resources and m_pPipelineResourceAttribsVk
-    const Uint32  ArraySize            : _ArraySizeBits;       // Array size
+    const Uint32  ArraySize            : _ArraySizeBits;       // Array size (for push constants, this is the number of 32-bit constants)
     const Uint32  DescrType            : _DescrTypeBits;       // Descriptor type (DescriptorType)
     const Uint32  DescrSet             : _DescrSetBits;        // Descriptor set (0 or 1)
     const Uint32  ImtblSamplerAssigned : _SamplerAssignedBits; // Immutable sampler flag
+    const Uint32  IsPushConstant       : _IsPushConstantBits;  // True if this is a Vulkan push constant (not using descriptor sets)
 
     const Uint32  SRBCacheOffset;                              // Offset in the SRB resource cache
     const Uint32  StaticCacheOffset;                           // Offset in the static resource cache
@@ -103,6 +105,7 @@ struct PipelineResourceAttribsVk
                               DescriptorType _DescrType,
                               Uint32         _DescrSet,
                               bool           _ImtblSamplerAssigned,
+                              bool           _IsPushConstant,
                               Uint32         _SRBCacheOffset,
                               Uint32         _StaticCacheOffset) noexcept :
         // clang-format off
@@ -112,6 +115,7 @@ struct PipelineResourceAttribsVk
         DescrType            {static_cast<Uint32>(_DescrType)},
         DescrSet             {_DescrSet                      },
         ImtblSamplerAssigned {_ImtblSamplerAssigned ? 1u : 0u},
+        IsPushConstant       {_IsPushConstant ? 1u : 0u      },
         SRBCacheOffset       {_SRBCacheOffset                },
         StaticCacheOffset    {_StaticCacheOffset             }
     // clang-format on
@@ -127,7 +131,7 @@ struct PipelineResourceAttribsVk
 
     // Only for serialization
     PipelineResourceAttribsVk() noexcept :
-        PipelineResourceAttribsVk{0, 0, 0, DescriptorType::Unknown, 0, false, 0, 0}
+        PipelineResourceAttribsVk{0, 0, 0, DescriptorType::Unknown, 0, false, false, 0, 0}
     {}
 
 
@@ -151,6 +155,11 @@ struct PipelineResourceAttribsVk
         return SamplerInd != InvalidSamplerInd;
     }
 
+    bool IsPushConstantBuffer() const
+    {
+        return IsPushConstant != 0;
+    }
+
     bool IsCompatibleWith(const PipelineResourceAttribsVk& rhs) const
     {
         // Ignore sampler index and cache offsets.
@@ -159,13 +168,14 @@ struct PipelineResourceAttribsVk
                ArraySize            == rhs.ArraySize    &&
                DescrType            == rhs.DescrType    &&
                DescrSet             == rhs.DescrSet     &&
-               ImtblSamplerAssigned == rhs.ImtblSamplerAssigned;
+               ImtblSamplerAssigned == rhs.ImtblSamplerAssigned &&
+               IsPushConstant       == rhs.IsPushConstant;
         // clang-format on
     }
 
     size_t GetHash() const
     {
-        return ComputeHash(BindingIndex, ArraySize, DescrType, DescrSet, ImtblSamplerAssigned);
+        return ComputeHash(BindingIndex, ArraySize, DescrType, DescrSet, ImtblSamplerAssigned, IsPushConstant);
     }
 };
 ASSERT_SIZEOF(PipelineResourceAttribsVk, 16, "The struct is used in serialization and must be tightly packed");
diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp
@@ -65,11 +65,13 @@ ASSERT_SIZEOF(ImmutableSamplerAttribsVk, 8, "The struct is used in serialization
 /// 2. Emulated inline constants - use dynamic uniform buffers (similar to D3D11 backend)
 struct InlineConstantBufferAttribsVk
 {
-    Uint32                      DescrSet         = 0;     // Descriptor set index
+    Uint32                      ResIndex         = 0;     // Resource index in the signature (used for matching)
+    Uint32                      DescrSet         = 0;     // Descriptor set index (0 for push constants as placeholder)
     Uint32                      BindingIndex     = 0;     // Binding index within the descriptor set
     Uint32                      NumConstants     = 0;     // Number of 32-bit constants
-    bool                        IsPushConstant = false; // True if this is a Vulkan push constant (not emulated)
+    bool                        IsPushConstant   = false; // True if this is a Vulkan push constant (not emulated)
     RefCntAutoPtr<BufferVkImpl> pBuffer;                  // Internal dynamic uniform buffer (null for true push constants)
+    void*                       pPushConstantData = nullptr; // CPU-side staging buffer for push constants (only used when IsPushConstant is true)
 };
 
 struct PipelineResourceSignatureInternalDataVk : PipelineResourceSignatureInternalData<PipelineResourceAttribsVk, ImmutableSamplerAttribsVk>
@@ -149,6 +151,7 @@ class PipelineResourceSignatureVkImpl final : public PipelineResourceSignatureBa
                                 VkDescriptorSet              vkDynamicDescriptorSet) const;
 
     // Updates inline constant buffers by mapping the internal buffers and copying data from the resource cache
+    // ResourceCache must be valid - each SRB has its own copy of push constant data stored in the cache
     void UpdateInlineConstantBuffers(const ShaderResourceCacheVk& ResourceCache,
                                      DeviceContextVkImpl&         Ctx) const;
 
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp
@@ -274,6 +274,37 @@ class ShaderResourceCacheVk : public ShaderResourceCacheBase
             STDDeleter<void, IMemoryAllocator>(Allocator)};
     }
 
+    // Sets the push constant data pointer for the given index
+    // This is used to store per-SRB push constant data for STATIC push constants
+    void SetPushConstantDataPtr(Uint32 Index, void* pData)
+    {
+        VERIFY_EXPR(Index < m_NumPushConstantBuffers);
+        m_pPushConstantDataPtrs[Index] = pData;
+    }
+
+    // Gets the push constant data pointer for the given index
+    void* GetPushConstantDataPtr(Uint32 Index) const
+    {
+        VERIFY_EXPR(Index < m_NumPushConstantBuffers);
+        return m_pPushConstantDataPtrs[Index];
+    }
+
+    // Initializes push constant data pointers array
+    void InitializePushConstantDataPtrs(Uint32 NumPushConstants)
+    {
+        m_NumPushConstantBuffers = static_cast<Uint16>(NumPushConstants);
+        if (NumPushConstants > 0)
+        {
+            m_pPushConstantDataPtrs = std::make_unique<void*[]>(NumPushConstants);
+            for (Uint32 i = 0; i < NumPushConstants; ++i)
+                m_pPushConstantDataPtrs[i] = nullptr;
+            // Mark that this cache has inline constants (push constants are a type of inline constants)
+            m_HasInlineConstants = 1;
+        }
+    }
+
+    Uint32 GetNumPushConstantBuffers() const { return m_NumPushConstantBuffers; }
+
     Uint32 GetNumDescriptorSets() const { return m_NumSets; }
     bool   HasDynamicResources() const { return m_NumDynamicBuffers > 0; }
 
@@ -318,7 +349,12 @@ class ShaderResourceCacheVk : public ShaderResourceCacheBase
     // Memory for inline constant data (allocated separately, freed in destructor)
     std::unique_ptr<void, STDDeleter<void, IMemoryAllocator>> m_pInlineConstantMemory;
 
-    Uint16 m_NumSets = 0;
+    // Array of push constant data pointers (one per push constant buffer)
+    // Each pointer points to memory within m_pInlineConstantMemory
+    std::unique_ptr<void*[]> m_pPushConstantDataPtrs;
+
+    Uint16 m_NumSets               = 0;
+    Uint16 m_NumPushConstantBuffers = 0;
 
     // Total actual number of dynamic buffers (that were created with USAGE_DYNAMIC) bound in the resource cache
     // regardless of the variable type. Note this variable is not equal to dynamic offsets count, which is constant.
diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
@@ -435,7 +435,15 @@ void DeviceContextVkImpl::UpdateInlineConstantBuffers(ResourceBindInfo& BindInfo
             continue;
 
         const ShaderResourceCacheVk* pResourceCache = BindInfo.ResourceCaches[i];
-        if (pResourceCache == nullptr || !pResourceCache->HasInlineConstants())
+        if (pResourceCache == nullptr)
+        {
+            // Each signature with inline constants must have a bound SRB with a valid resource cache
+            DEV_CHECK_ERR(false, "Signature '", pSign->GetDesc().Name, "' has inline constants but no SRB is bound. "
+                                 "Did you call CommitShaderResources()?");
+            continue;
+        }
+
+        if (!pResourceCache->HasInlineConstants())
             continue;
 
         // Update inline constant buffers
@@ -461,13 +469,14 @@ void DeviceContextVkImpl::SetPushConstants(const void* pData, Uint32 Offset, Uin
                           "Push constant data range [", Offset, ", ", Offset + Size, ") exceeds the push constant block size (", Layout.GetPushConstantSize(), ")");
         }
     }
-
-    if (Size > 0)
+    else
     {
-        memcpy(m_PushConstantsData.data() + Offset, pData, Size);
-        m_PushConstantsDataSize = std::max(m_PushConstantsDataSize, Offset + Size);
-        m_State.PushConstantsDirty = true;
+        DEV_ERROR("A valid PipelineState must be bound before calling SetPushConstants!");
     }
+
+    memcpy(m_PushConstantsData.data() + Offset, pData, Size);
+    m_PushConstantsDataSize    = std::max(m_PushConstantsDataSize, Offset + Size);
+    m_State.PushConstantsDirty = true;
 }
 
 void DeviceContextVkImpl::CommitPushConstants()
@@ -495,6 +504,27 @@ void DeviceContextVkImpl::CommitDescriptorSets(ResourceBindInfo& BindInfo, Uint3
 {
     VERIFY(CommitSRBMask != 0, "This method should not be called when there is nothing to commit");
 
+    // Filter out SRBs that only have push constants (no descriptor sets)
+    // These SRBs are included in CommitSRBMask due to InlineConstantsSRBMask, but they don't need descriptor set binding
+    Uint32 FilteredCommitMask = CommitSRBMask;
+    Uint32 ResourceSignaturesCount = m_pPipelineState->GetResourceSignatureCount();
+    for (Uint32 sign = 0; sign < ResourceSignaturesCount; ++sign)
+    {
+        if ((CommitSRBMask & (1u << sign)) != 0)
+        {
+            const ShaderResourceCacheVk* pResourceCache = BindInfo.ResourceCaches[sign];
+            if (pResourceCache != nullptr && pResourceCache->GetNumDescriptorSets() == 0)
+            {
+                // This SRB only has push constants, no descriptor sets to commit
+                FilteredCommitMask &= ~(1u << sign);
+            }
+        }
+    }
+
+    // If all SRBs were filtered out (only push constants), nothing to commit
+    if (FilteredCommitMask == 0)
+        return;
+
     const Uint32 FirstSign = PlatformMisc::GetLSB(CommitSRBMask);
     const Uint32 LastSign  = PlatformMisc::GetMSB(CommitSRBMask);
     VERIFY_EXPR(LastSign < m_pPipelineState->GetResourceSignatureCount());
@@ -563,6 +593,8 @@ void DeviceContextVkImpl::DvpValidateCommittedShaderResources(ResourceBindInfo&
     if (BindInfo.ResourcesValidated)
         return;
 
+    // Use custom signature getter to skip signatures that have no resources at all
+    // Note: Signatures with only push constants still need SRB to store push constant data
     DvpVerifySRBCompatibility(BindInfo);
 
     const Uint32 SignCount = m_pPipelineState->GetResourceSignatureCount();
@@ -612,9 +644,24 @@ void DeviceContextVkImpl::CommitShaderResources(IShaderResourceBinding* pShaderR
 
     ShaderResourceBindingVkImpl* pResBindingVkImpl = ClassPtrCast<ShaderResourceBindingVkImpl>(pShaderResourceBinding);
     ShaderResourceCacheVk&       ResourceCache     = pResBindingVkImpl->GetResourceCache();
+
+    const Uint32                           SRBIndex   = pResBindingVkImpl->GetBindingIndex();
+    const PipelineResourceSignatureVkImpl* pSignature = pResBindingVkImpl->GetSignature();
+    ResourceBindInfo&                      BindInfo   = GetBindInfo(pResBindingVkImpl->GetPipelineType());
+    ResourceBindInfo::DescriptorSetInfo&   SetInfo    = BindInfo.SetInfo[SRBIndex];
+
+    // Always bind the SRB, even if it has no descriptor sets (e.g., only push constants)
+    // The resource cache is needed to store push constant data
+    BindInfo.Set(SRBIndex, pResBindingVkImpl);
+
+    // If there are no descriptor sets, we're done (SRB only contains push constants)
+    // Clear the stale flag since there are no descriptor sets to commit
     if (ResourceCache.GetNumDescriptorSets() == 0)
     {
-        // Ignore SRBs that contain no resources
+        // Clear the stale bit for this SRB since it has no descriptor sets to commit
+        // The SRB is still bound and ResourceCaches[SRBIndex] is set, so push constants can be accessed
+        const auto SRBBit = static_cast<ResourceBindInfo::SRBMaskType>(1u << SRBIndex);
+        BindInfo.StaleSRBMask &= ~SRBBit;
         return;
     }
 
@@ -633,12 +680,6 @@ void DeviceContextVkImpl::CommitShaderResources(IShaderResourceBinding* pShaderR
     }
 #endif
 
-    const Uint32                           SRBIndex   = pResBindingVkImpl->GetBindingIndex();
-    const PipelineResourceSignatureVkImpl* pSignature = pResBindingVkImpl->GetSignature();
-    ResourceBindInfo&                      BindInfo   = GetBindInfo(pResBindingVkImpl->GetPipelineType());
-    ResourceBindInfo::DescriptorSetInfo&   SetInfo    = BindInfo.SetInfo[SRBIndex];
-
-    BindInfo.Set(SRBIndex, pResBindingVkImpl);
     // We must not clear entire ResInfo as DescriptorSetBaseInd and DynamicOffsetCount
     // are set by SetPipelineState().
     SetInfo.vkSets = {};
@@ -836,6 +877,7 @@ void DeviceContextVkImpl::PrepareForDraw(DRAW_FLAGS Flags)
     ResourceBindInfo& BindInfo = GetBindInfo(PIPELINE_TYPE_GRAPHICS);
 
     // Update inline constant buffers before binding descriptor sets
+    const bool DynamicBuffersIntact  = (Flags & DRAW_FLAG_DYNAMIC_RESOURCE_BUFFERS_INTACT) != 0;
     const bool InlineConstantsIntact = (Flags & DRAW_FLAG_INLINE_CONSTANTS_INTACT) != 0;
     if (!InlineConstantsIntact)
     {
@@ -845,7 +887,7 @@ void DeviceContextVkImpl::PrepareForDraw(DRAW_FLAGS Flags)
     // First time we must always bind descriptor sets with dynamic offsets as SRBs are stale.
     // If there are no dynamic buffers bound in the resource cache, for all subsequent
     // calls we do not need to bind the sets again.
-    if (Uint32 CommitMask = BindInfo.GetCommitMask(Flags & DRAW_FLAG_DYNAMIC_RESOURCE_BUFFERS_INTACT, Flags & DRAW_FLAG_INLINE_CONSTANTS_INTACT))
+    if (Uint32 CommitMask = BindInfo.GetCommitMask(DynamicBuffersIntact, InlineConstantsIntact))
     {
         CommitDescriptorSets(BindInfo, CommitMask);
     }
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVariableManagerVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVariableManagerVk.cpp
