From 65dd6c5aa598653cf6b828efd51fc359fc758b36 Mon Sep 17 00:00:00 2001 From: azhirnov Date: Thu, 25 Feb 2021 21:00:12 +0300 Subject: some fixes and improvements for D3D12 & Vulkan --- .../include/PipelineResourceSignatureBase.hpp | 49 +++++++++++++++++++++- Graphics/GraphicsEngine/interface/GraphicsTypes.h | 2 +- .../interface/PipelineResourceSignature.h | 3 -- Graphics/GraphicsEngine/interface/PipelineState.h | 7 +++- .../src/PipelineResourceSignatureBase.cpp | 7 +++- .../include/PipelineResourceSignatureD3D12Impl.hpp | 7 +++- .../include/PipelineResourceSignatureVkImpl.hpp | 7 +++- .../include/PipelineStateVkImpl.hpp | 1 - .../include/ShaderResourceBindingVkImpl.hpp | 2 +- .../src/DeviceContextVkImpl.cpp | 21 +++++++--- .../GraphicsEngineVulkan/src/PipelineLayoutVk.cpp | 32 +------------- .../src/PipelineStateVkImpl.cpp | 2 +- .../src/ShaderResourceBindingVkImpl.cpp | 1 + 13 files changed, 89 insertions(+), 52 deletions(-) (limited to 'Graphics') diff --git a/Graphics/GraphicsEngine/include/PipelineResourceSignatureBase.hpp b/Graphics/GraphicsEngine/include/PipelineResourceSignatureBase.hpp index 4cc9f3b6..49be28cf 100644 --- a/Graphics/GraphicsEngine/include/PipelineResourceSignatureBase.hpp +++ b/Graphics/GraphicsEngine/include/PipelineResourceSignatureBase.hpp @@ -47,7 +47,7 @@ namespace Diligent { /// Validates pipeline resource signature description and throws an exception in case of an error. -void ValidatePipelineResourceSignatureDesc(const PipelineResourceSignatureDesc& Desc) noexcept(false); +void ValidatePipelineResourceSignatureDesc(const PipelineResourceSignatureDesc& Desc, bool ShaderResourceRuntimeArraySupported) noexcept(false); static constexpr Uint32 InvalidImmutableSamplerIndex = ~0u; /// Finds an immutable sampler for the resource name 'ResourceName' that is defined in shader stages 'ShaderStages'. @@ -94,7 +94,7 @@ public: this->m_Desc.ImmutableSamplers = nullptr; this->m_Desc.CombinedSamplerSuffix = nullptr; - ValidatePipelineResourceSignatureDesc(Desc); + ValidatePipelineResourceSignatureDesc(Desc, pDevice->GetDeviceCaps().Features.ShaderResourceRuntimeArray); // Determine shader stages that have any resources as well as // shader stages that have static resources. @@ -155,6 +155,11 @@ public: return PlatformMisc::CountOneBits(Uint32{m_ShaderStages}); } + SHADER_TYPE GetActiveShaderStages() const + { + return m_ShaderStages; + } + // Returns the number of shader stages that have static resources. Uint32 GetNumStaticResStages() const { @@ -214,6 +219,46 @@ public: return this->m_Desc.ImmutableSamplers[SampIndex]; } + template + static void CopyResourceSignatures(PIPELINE_TYPE PipelineType, + const Uint32 SignatureCount, + IPipelineResourceSignature* ppResourceSignatures[], + RefCntAutoPtr DstSignatures[], + const size_t MaxDstSignatureCount, + Uint8& DstSignatureCount) + { + for (Uint32 i = 0; i < SignatureCount; ++i) + { + auto* pSignature = ValidatedCast(ppResourceSignatures[i]); + VERIFY(pSignature != nullptr, "Pipeline resource signature at index ", i, " is null. This error should've been caught by ValidatePipelineResourceSignatures."); + + const Uint8 Index = pSignature->GetDesc().BindingIndex; + +#ifdef DILIGENT_DEBUG + VERIFY(Index < MaxDstSignatureCount, + "Pipeline resource signature specifies binding index ", Uint32{Index}, " that exceeds the limit (", MaxDstSignatureCount - 1, + "). This error should've been caught by ValidatePipelineResourceSignatureDesc."); + + VERIFY(DstSignatures[Index] == nullptr, + "Pipeline resource signature '", pSignature->GetDesc().Name, "' at index ", Uint32{Index}, + " conflicts with another resource signature '", DstSignatures[Index]->GetDesc().Name, + "' that uses the same index. This error should've been caught by ValidatePipelineResourceSignatures."); + + for (Uint32 s = 0, StageCount = pSignature->GetNumActiveShaderStages(); s < StageCount; ++s) + { + const auto ShaderType = pSignature->GetActiveShaderStageType(s); + VERIFY(IsConsistentShaderType(ShaderType, PipelineType), + "Pipeline resource signature '", pSignature->GetDesc().Name, "' at index ", Uint32{Index}, + " has shader stage '", GetShaderTypeLiteralName(ShaderType), "' that is not compatible with pipeline type '", + GetPipelineTypeString(PipelineType), "'."); + } +#endif + + DstSignatureCount = std::max(DstSignatureCount, Index + 1); + DstSignatures[Index] = pSignature; + } + } + protected: void ReserveSpaceForDescription(FixedLinearAllocator& Allocator, const PipelineResourceSignatureDesc& Desc) const noexcept(false) { diff --git a/Graphics/GraphicsEngine/interface/GraphicsTypes.h b/Graphics/GraphicsEngine/interface/GraphicsTypes.h index 5e236134..ae11a655 100644 --- a/Graphics/GraphicsEngine/interface/GraphicsTypes.h +++ b/Graphics/GraphicsEngine/interface/GraphicsTypes.h @@ -860,7 +860,7 @@ DILIGENT_TYPED_ENUM(TEXTURE_ADDRESS_MODE, Uint8) /// Direct3D Counterpart: D3D11_TEXTURE_ADDRESS_CLAMP/D3D12_TEXTURE_ADDRESS_MODE_CLAMP. OpenGL counterpart: GL_CLAMP_TO_EDGE TEXTURE_ADDRESS_CLAMP = 3, - /// Texture coordinates outside the range [0.0, 1.0] are set to the border color specified + /// Texture coordinates outside the range [0.0, 1.0] are set to the border color /// specified in SamplerDesc structure. \n /// Direct3D Counterpart: D3D11_TEXTURE_ADDRESS_BORDER/D3D12_TEXTURE_ADDRESS_MODE_BORDER. OpenGL counterpart: GL_CLAMP_TO_BORDER TEXTURE_ADDRESS_BORDER = 4, diff --git a/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h b/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h index 5b6aac96..e4461e75 100644 --- a/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h +++ b/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h @@ -185,9 +185,6 @@ struct PipelineResourceSignatureDesc DILIGENT_DERIVE(DeviceObjectAttribs) /// to different slots. Uint8 BindingIndex DEFAULT_INITIALIZER(0); - /// AZ TODO: comment - Uint16 BindingOffsets [SHADER_RESOURCE_TYPE_LAST + 1] DEFAULT_INITIALIZER({}); - /// If set to true, textures will be combined with texture samplers. /// The CombinedSamplerSuffix member defines the suffix added to the texture variable /// name to get corresponding sampler name. When using combined samplers, diff --git a/Graphics/GraphicsEngine/interface/PipelineState.h b/Graphics/GraphicsEngine/interface/PipelineState.h index 801548e1..749344a0 100644 --- a/Graphics/GraphicsEngine/interface/PipelineState.h +++ b/Graphics/GraphicsEngine/interface/PipelineState.h @@ -630,7 +630,7 @@ DILIGENT_BEGIN_INTERFACE(IPipelineState, IDeviceObject) /// If two pipeline state objects are compatible, they can use shader resource binding /// objects interchangebly, i.e. SRBs created by one PSO can be committed /// when another PSO is bound. - /// \param [in] pPSO - Pointer to the pipeline state object to check compatibility with + /// \param [in] pPSO - Pointer to the pipeline state object to check compatibility with. /// \return true if this PSO is compatbile with pPSO. false otherwise. /// \remarks The function only checks that shader resource layouts are compatible, but /// does not check if resource types match. For instance, if a pixel shader in one PSO @@ -652,11 +652,14 @@ DILIGENT_BEGIN_INTERFACE(IPipelineState, IDeviceObject) VIRTUAL bool METHOD(IsCompatibleWith)(THIS_ const struct IPipelineState* pPSO) CONST PURE; - + /// Returns the number of pipeline resource signature used to created this pipeline. VIRTUAL Uint32 METHOD(GetResourceSignatureCount)(THIS) CONST PURE; /// Returns pipeline resource signature at the give index. + + /// \param [in] Index - Index of the resource signature, same as BindingIndex in PipelineResourceSignatureDesc. + /// \return Pointer to pipeline resource signature interface. VIRTUAL IPipelineResourceSignature* METHOD(GetResourceSignature)(THIS_ Uint32 Index) CONST PURE; }; diff --git a/Graphics/GraphicsEngine/src/PipelineResourceSignatureBase.cpp b/Graphics/GraphicsEngine/src/PipelineResourceSignatureBase.cpp index a85e20dd..0f359e52 100644 --- a/Graphics/GraphicsEngine/src/PipelineResourceSignatureBase.cpp +++ b/Graphics/GraphicsEngine/src/PipelineResourceSignatureBase.cpp @@ -37,7 +37,7 @@ namespace Diligent #define LOG_PRS_ERROR_AND_THROW(...) LOG_ERROR_AND_THROW("Description of a pipeline resource signature '", (Desc.Name ? Desc.Name : ""), "' is invalid: ", ##__VA_ARGS__) -void ValidatePipelineResourceSignatureDesc(const PipelineResourceSignatureDesc& Desc) noexcept(false) +void ValidatePipelineResourceSignatureDesc(const PipelineResourceSignatureDesc& Desc, bool ShaderResourceRuntimeArraySupported) noexcept(false) { if (Desc.BindingIndex >= MAX_RESOURCE_SIGNATURES) LOG_PRS_ERROR_AND_THROW("Desc.BindingIndex (", Uint32{Desc.BindingIndex}, ") exceeds the maximum allowed value (", MAX_RESOURCE_SIGNATURES - 1, ")."); @@ -81,6 +81,11 @@ void ValidatePipelineResourceSignatureDesc(const PipelineResourceSignatureDesc& } UsedStages |= Res.ShaderStages; + if ((Res.Flags & PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY) != 0 && !ShaderResourceRuntimeArraySupported) + { + LOG_PRS_ERROR_AND_THROW("Incorrect Desc.Resources[", i, "].Flags: RUNTIME_ARRAY can be used only if ShaderResourceRuntimeArray device feature is enabled."); + } + static_assert(SHADER_RESOURCE_TYPE_LAST == 8, "Please add the new resource type to the switch below"); switch (Res.ResourceType) { diff --git a/Graphics/GraphicsEngineD3D12/include/PipelineResourceSignatureD3D12Impl.hpp b/Graphics/GraphicsEngineD3D12/include/PipelineResourceSignatureD3D12Impl.hpp index eb62b2ef..99148ae7 100644 --- a/Graphics/GraphicsEngineD3D12/include/PipelineResourceSignatureD3D12Impl.hpp +++ b/Graphics/GraphicsEngineD3D12/include/PipelineResourceSignatureD3D12Impl.hpp @@ -222,7 +222,10 @@ public: virtual bool DILIGENT_CALL_TYPE IsCompatibleWith(const IPipelineResourceSignature* pPRS) const override final { - VERIFY_EXPR(pPRS != nullptr); + if (pPRS == nullptr) + { + return GetHash() == 0; + } return IsCompatibleWith(*ValidatedCast(pPRS)); } @@ -300,7 +303,7 @@ private: private: ResourceAttribs* m_pResourceAttribs = nullptr; // [m_Desc.NumResources] ShaderResourceCacheD3D12* m_pStaticResCache = nullptr; - ShaderVariableManagerD3D12* m_StaticVarsMgrs = nullptr; // [m_NumShaderStages] + ShaderVariableManagerD3D12* m_StaticVarsMgrs = nullptr; // [GetNumStaticResStages()] ImmutableSamplerAttribs* m_ImmutableSamplers = nullptr; // [m_Desc.NumImmutableSamplers] RootParamsManager m_RootParams; diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp index f8215d49..e1dd094d 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp +++ b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp @@ -224,7 +224,10 @@ public: /// Implementation of IPipelineResourceSignature::IsCompatibleWith. virtual bool DILIGENT_CALL_TYPE IsCompatibleWith(const IPipelineResourceSignature* pPRS) const override final { - VERIFY_EXPR(pPRS != nullptr); + if (pPRS == nullptr) + { + return GetHash() == 0; + } return IsCompatibleWith(*ValidatedCast(pPRS)); } @@ -329,7 +332,7 @@ private: // Static resource cache for all static resources ShaderResourceCacheVk* m_pStaticResCache = nullptr; // Static variables manager for every shader stage - ShaderVariableManagerVk* m_StaticVarsMgrs = nullptr; // [m_NumShaderStages] + ShaderVariableManagerVk* m_StaticVarsMgrs = nullptr; // [GetNumStaticResStages()] ImmutableSamplerAttribs* m_ImmutableSamplers = nullptr; // [m_Desc.NumImmutableSamplers] diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.hpp index a37638dd..16206349 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.hpp +++ b/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.hpp @@ -92,7 +92,6 @@ public: SubpassDesc& SubpassDesc); - void InitializeStaticSRBResources(ShaderResourceCacheVk& ResourceCache) const; struct ShaderStageInfo { diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp index 54cd3b3a..c0e012a2 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceBindingVkImpl.hpp @@ -74,7 +74,7 @@ private: void Destruct(); ShaderResourceCacheVk m_ShaderResourceCache; - ShaderVariableManagerVk* m_pShaderVarMgrs = nullptr; + ShaderVariableManagerVk* m_pShaderVarMgrs = nullptr; // [m_NumShaders] }; } // namespace Diligent diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp index 2ae180e7..27cdfbee 100644 --- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp @@ -326,7 +326,7 @@ void DeviceContextVkImpl::SetPipelineState(IPipelineState* pPipelineState) for (Uint32 i = 0; i < SignCount; ++i) { auto* pSignature = Layout.GetSignature(i); - if (pSignature == nullptr) + if (pSignature == nullptr || pSignature->GetNumDescriptorSets() == 0) continue; BindInfo.ActiveSRBMask |= 1u << i; @@ -349,13 +349,20 @@ void DeviceContextVkImpl::SetPipelineState(IPipelineState* pPipelineState) Uint32 sign = 0; for (; sign < SignCount; ++sign) { - const auto* LayoutSign = Layout.GetSignature(sign); - if (LayoutSign == nullptr) - continue; + const auto* pLayoutSign = Layout.GetSignature(sign); + const auto* pSRBSign = BindInfo.SRBs[sign] != nullptr ? BindInfo.SRBs[sign]->GetSignature() : nullptr; - auto* pSRB = BindInfo.SRBs[sign]; - if (pSRB == nullptr || LayoutSign->IsIncompatibleWith(*pSRB->GetSignature())) + if ((pLayoutSign == nullptr || pLayoutSign->GetNumDescriptorSets() == 0) != (pSRBSign == nullptr || pSRBSign->GetNumDescriptorSets() == 0)) + { + // One signature is null or empty while the other is not - SRB is not compatible with the layout. break; + } + + if (pLayoutSign != nullptr && pSRBSign != nullptr && pLayoutSign->IsIncompatibleWith(*pSRBSign)) + { + // Signatures are incompatible + break; + } } // Unbind incompatible shader resources @@ -375,6 +382,8 @@ void DeviceContextVkImpl::SetPipelineState(IPipelineState* pPipelineState) BindInfo.ClearDynamicBufferBit(sign); } #endif + + m_State.CommittedResourcesValidated = false; } DeviceContextVkImpl::DescriptorSetBindInfo& DeviceContextVkImpl::GetDescriptorSetBindInfo(PIPELINE_TYPE Type) diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp index b7aaaa26..febc0002 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayoutVk.cpp @@ -61,36 +61,8 @@ void PipelineLayoutVk::Create(RenderDeviceVkImpl* pDeviceVk, PIPELINE_TYPE Pipel VERIFY(m_SignatureCount == 0 && m_DescrSetCount == 0 && !m_VkPipelineLayout, "This pipeline layout is already initialized"); - for (Uint32 i = 0; i < SignatureCount; ++i) - { - auto* pSignature = ValidatedCast(ppSignatures[i]); - VERIFY(pSignature != nullptr, "Pipeline resource signature at index ", i, " is null. This error should've been caught by ValidatePipelineResourceSignatures."); - - const Uint8 Index = pSignature->GetDesc().BindingIndex; - -#ifdef DILIGENT_DEBUG - VERIFY(Index < m_Signatures.size(), - "Pipeline resource signature specifies binding index ", Uint32{Index}, " that exceeds the limit (", m_Signatures.size() - 1, - "). This error should've been caught by ValidatePipelineResourceSignatureDesc."); - - VERIFY(m_Signatures[Index] == nullptr, - "Pipeline resource signature '", pSignature->GetDesc().Name, "' at index ", Uint32{Index}, - " conflicts with another resource signature '", m_Signatures[Index]->GetDesc().Name, - "' that uses the same index. This error should've been caught by ValidatePipelineResourceSignatures."); - - for (Uint32 s = 0, StageCount = pSignature->GetNumActiveShaderStages(); s < StageCount; ++s) - { - const auto ShaderType = pSignature->GetActiveShaderStageType(s); - VERIFY(IsConsistentShaderType(ShaderType, PipelineType), - "Pipeline resource signature '", pSignature->GetDesc().Name, "' at index ", Uint32{Index}, - " has shader stage '", GetShaderTypeLiteralName(ShaderType), "' that is not compatible with pipeline type '", - GetPipelineTypeString(PipelineType), "'."); - } -#endif - - m_SignatureCount = std::max(m_SignatureCount, Index + 1); - m_Signatures[Index] = pSignature; - } + PipelineResourceSignatureVkImpl::CopyResourceSignatures(PipelineType, SignatureCount, ppSignatures, + m_Signatures.data(), m_Signatures.size(), m_SignatureCount); std::array DescSetLayouts; diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp index 93262266..14417af0 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp @@ -1114,7 +1114,7 @@ void PipelineStateVkImpl::DvpVerifySRBResources(SRBArray& SRBs) const } else { - LOG_ERROR_MESSAGE("No SRB is bound to binding index ", SignBindIndex, " for signature '", SignDesc.Name, '\''); + LOG_ERROR_MESSAGE("No SRB is bound to binding index ", Uint32{SignBindIndex}, " for signature '", SignDesc.Name, '\''); } } ++res_info; diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp index 7456e66c..e0d0d39f 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp @@ -110,6 +110,7 @@ void ShaderResourceBindingVkImpl::Destruct() } GetRawAllocator().Free(m_pShaderVarMgrs); + m_pShaderVarMgrs = nullptr; } } -- cgit v1.2.3