From 07a4792c57e3617d1aa43c9fc3d98ff85107c350 Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 9 Feb 2021 19:45:10 -0800 Subject: Vk backend: some minor updates + comments --- Graphics/GraphicsEngine/interface/GraphicsTypes.h | 9 +++++---- .../interface/PipelineResourceSignature.h | 2 +- Graphics/GraphicsEngine/interface/PipelineState.h | 12 +++++++++--- .../include/PipelineResourceSignatureVkImpl.hpp | 16 ++++++++-------- .../GraphicsEngineVulkan/src/EngineFactoryVk.cpp | 2 +- .../src/PipelineResourceSignatureVkImpl.cpp | 12 ++++++------ .../src/PipelineStateVkImpl.cpp | 22 ++++++++++++++-------- 7 files changed, 44 insertions(+), 31 deletions(-) (limited to 'Graphics') diff --git a/Graphics/GraphicsEngine/interface/GraphicsTypes.h b/Graphics/GraphicsEngine/interface/GraphicsTypes.h index fff9ca2a..5e236134 100644 --- a/Graphics/GraphicsEngine/interface/GraphicsTypes.h +++ b/Graphics/GraphicsEngine/interface/GraphicsTypes.h @@ -1644,10 +1644,11 @@ struct DeviceFeatures /// Indicates if device supports reading 8-bit types from uniform buffers. DEVICE_FEATURE_STATE UniformBuffer8BitAccess DEFAULT_INITIALIZER(DEVICE_FEATURE_STATE_DISABLED); - /// Indicates if device supports runtime-sized arrays in shader. - /// DirectX 12 backend already supports this feature, - /// Vulkan backend has optional support, - /// other backends are does not support this feature. + /// Indicates if device supports runtime-sized shader arrays (e.g. arrays without a specific size). + /// + /// \remarks This feature is always enabled in DirectX12 backend and + /// can optionally be enabled in Vulkan backend. + /// Run-time sized shader arrays are not available in other backends. DEVICE_FEATURE_STATE ShaderResourceRuntimeArray DEFAULT_INITIALIZER(DEVICE_FEATURE_STATE_DISABLED); diff --git a/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h b/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h index a501a27c..05feb54f 100644 --- a/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h +++ b/Graphics/GraphicsEngine/interface/PipelineResourceSignature.h @@ -108,7 +108,7 @@ DILIGENT_TYPED_ENUM(PIPELINE_RESOURCE_FLAGS, Uint8) /// PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER flag. PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER = 0x04, - /// DirectX 12 only - runtime sized array must be in separate space. + /// Indicates that resource is a run-time sized shader array (e.g. an array without a specific size). PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY = 0x08, PIPELINE_RESOURCE_FLAG_LAST = PIPELINE_RESOURCE_FLAG_RUNTIME_ARRAY diff --git a/Graphics/GraphicsEngine/interface/PipelineState.h b/Graphics/GraphicsEngine/interface/PipelineState.h index dedeee9a..d8d765c1 100644 --- a/Graphics/GraphicsEngine/interface/PipelineState.h +++ b/Graphics/GraphicsEngine/interface/PipelineState.h @@ -593,9 +593,15 @@ DILIGENT_BEGIN_INTERFACE(IPipelineState, IDeviceObject) /// The function only checks compatibility of shader resource layouts. It does not take /// into account vertex shader input layout, number of outputs, etc. /// - /// \remarks On Vulkan backend PSO may be partially compatible, on other backends this behavior is emulated. - /// For Vulkan changing PSO between totally or partially compatible may increase performance, - /// for DirectX 12 only changing PSO between compatible may increase performance. + /// *Technical details* + /// + /// PSOs may be partially compatible when some, but not all pipeline resource signatures are compatible. + /// In Vulkan backend, switching PSOs that are partially compatible may increase performance + /// as shader resource bindings (that map to descriptor sets) from compatible signatures may be preserved. + /// In Direct3D12 backend, only switching between fully compatible PSOs preserves shader resource bindings, + /// while switching partially compatible PSOs still requires re-binding all resource bindigns from all signatures. + /// In other backends the behavior is emualted. Usually, the bindigs from the first N compatible resource signatures + /// may be preserved. VIRTUAL bool METHOD(IsCompatibleWith)(THIS_ const struct IPipelineState* pPSO) CONST PURE; diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp index 11c440b3..ea20151b 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp +++ b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp @@ -259,9 +259,9 @@ public: Uint32 ResIndex, ShaderResourceCacheVk& ResourceCache) const; - bool IsBound(Uint32 ArrayIndex, - Uint32 ResIndex, - ShaderResourceCacheVk& ResourceCache) const; + bool IsBound(Uint32 ArrayIndex, + Uint32 ResIndex, + const ShaderResourceCacheVk& ResourceCache) const; // Commits dynamic resources from ResourceCache to vkDynamicDescriptorSet void CommitDynamicResources(const ShaderResourceCacheVk& ResourceCache, @@ -289,10 +289,10 @@ private: // Resource cache group identifier enum CACHE_GROUP : size_t { - CACHE_GROUP_DYN_UB = 0, // Uniform buffer with dynamic offset - CACHE_GROUP_DYN_SB, // Storage buffer with dynamic offset - CACHE_GROUP_OTHER, // Other resource type - CACHE_GROUP_COUNT_PER_Type, + CACHE_GROUP_DYN_UB = 0, // Uniform buffer with dynamic offset + CACHE_GROUP_DYN_SB, // Storage buffer with dynamic offset + CACHE_GROUP_OTHER, // Other resource type + CACHE_GROUP_COUNT_PER_VAR_TYPE, // Cache group count per shader variable type CACHE_GROUP_DYN_UB_STAT_VAR = CACHE_GROUP_DYN_UB, // Uniform buffer with dynamic offset, static variable CACHE_GROUP_DYN_SB_STAT_VAR = CACHE_GROUP_DYN_SB, // Storage buffer with dynamic offset, static variable @@ -304,7 +304,7 @@ private: CACHE_GROUP_COUNT }; - static_assert(CACHE_GROUP_COUNT == CACHE_GROUP_COUNT_PER_Type * MAX_DESCRIPTOR_SETS, "Inconsistent cache group count"); + static_assert(CACHE_GROUP_COUNT == CACHE_GROUP_COUNT_PER_VAR_TYPE * MAX_DESCRIPTOR_SETS, "Inconsistent cache group count"); using CacheOffsetsType = std::array; // [dynamic uniform buffers, dynamic storage buffers, other] x [descriptor sets] including ArraySize using BindingCountType = std::array; // [dynamic uniform buffers, dynamic storage buffers, other] x [descriptor sets] not counting ArraySize diff --git a/Graphics/GraphicsEngineVulkan/src/EngineFactoryVk.cpp b/Graphics/GraphicsEngineVulkan/src/EngineFactoryVk.cpp index 26845497..7e08a361 100644 --- a/Graphics/GraphicsEngineVulkan/src/EngineFactoryVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/EngineFactoryVk.cpp @@ -279,7 +279,7 @@ void EngineFactoryVkImpl::CreateDeviceAndContextsVk(const EngineVkCreateInfo& _E // clang-format on const auto& DescrIndexingFeats = DeviceExtFeatures.DescriptorIndexing; - ENABLE_FEATURE(DescrIndexingFeats.runtimeDescriptorArray != VK_FALSE, ShaderResourceRuntimeArray, "shader resource runtime array is"); + ENABLE_FEATURE(DescrIndexingFeats.runtimeDescriptorArray != VK_FALSE, ShaderResourceRuntimeArray, "Shader resource runtime array is"); ENABLE_FEATURE(DeviceExtFeatures.AccelStruct.accelerationStructure != VK_FALSE && DeviceExtFeatures.RayTracingPipeline.rayTracingPipeline != VK_FALSE, RayTracing, "Ray tracing is"); #undef FeatureSupport diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp index 03c610d1..3dcb2df6 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineResourceSignatureVkImpl.cpp @@ -249,12 +249,12 @@ inline PipelineResourceSignatureVkImpl::CACHE_GROUP PipelineResourceSignatureVkI if (WithDynamicOffset && !UseTexelBuffer) { if (Res.ResourceType == SHADER_RESOURCE_TYPE_CONSTANT_BUFFER) - return static_cast(SetId * CACHE_GROUP_COUNT_PER_Type + CACHE_GROUP_DYN_UB); + return static_cast(SetId * CACHE_GROUP_COUNT_PER_VAR_TYPE + CACHE_GROUP_DYN_UB); if (Res.ResourceType == SHADER_RESOURCE_TYPE_BUFFER_SRV || Res.ResourceType == SHADER_RESOURCE_TYPE_BUFFER_UAV) - return static_cast(SetId * CACHE_GROUP_COUNT_PER_Type + CACHE_GROUP_DYN_SB); + return static_cast(SetId * CACHE_GROUP_COUNT_PER_VAR_TYPE + CACHE_GROUP_DYN_SB); } - return static_cast(SetId * CACHE_GROUP_COUNT_PER_Type + CACHE_GROUP_OTHER); + return static_cast(SetId * CACHE_GROUP_COUNT_PER_VAR_TYPE + CACHE_GROUP_OTHER); } inline PipelineResourceSignatureVkImpl::DESCRIPTOR_SET_ID PipelineResourceSignatureVkImpl::VarTypeToDescriptorSetId(SHADER_RESOURCE_VARIABLE_TYPE VarType) @@ -1697,9 +1697,9 @@ void PipelineResourceSignatureVkImpl::BindResource(IDeviceObject* pObj, Helper.BindResource(pObj); } -bool PipelineResourceSignatureVkImpl::IsBound(Uint32 ArrayIndex, - Uint32 ResIndex, - ShaderResourceCacheVk& ResourceCache) const +bool PipelineResourceSignatureVkImpl::IsBound(Uint32 ArrayIndex, + Uint32 ResIndex, + const ShaderResourceCacheVk& ResourceCache) const { const auto& ResDesc = GetResourceDesc(ResIndex); const auto& Attribs = GetResourceAttribs(ResIndex); diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp index 841e6cdc..f40bbe19 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp @@ -774,8 +774,8 @@ void PipelineStateVkImpl::CreateDefaultSignature(const PipelineStateCreateInfo& { if (Res.ArraySize == 0) { - LOG_ERROR_AND_THROW("Is shader '", pShader->GetDesc().Name, "' resource '", Res.Name, "' uses runtime sized array, ", - "you must explicitlly set resource signature to specify array size"); + LOG_ERROR_AND_THROW("Resource '", Res.Name, "' in shader '", pShader->GetDesc().Name, "' is a runtime-sized array. ", + "You must use explicit resource signature to specify the array size."); } SHADER_RESOURCE_TYPE Type; @@ -935,15 +935,21 @@ void PipelineStateVkImpl::InitPipelineLayout(const PipelineStateCreateInfo& Crea if ((Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) != (ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER)) { - LOG_ERROR_AND_THROW("AZ TODO"); + LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource '", SPIRVAttribs.Name, + "' that is", ((Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) ? "" : " not"), + " labeled as formatted buffer, while the same resource specified by the pipeline resource signature '", + Info.Signature->GetDesc().Name, "' is", ((ResDesc.Flags & PIPELINE_RESOURCE_FLAG_FORMATTED_BUFFER) ? "" : " not"), + " labeled as such."); } - // ArraySize == 0 means that used runtime sized array and ArraySize in resource signature may be any non-zero value. - if (SPIRVAttribs.ArraySize > 0 && ResDesc.ArraySize != SPIRVAttribs.ArraySize) + // SPIRVAttribs.ArraySize == 0 means that the resource is a runtime-sized array and ResDesc.ArraySize from the + // resource signature may have any non-zero value. + VERIFY(ResDesc.ArraySize > 0, "ResDesc.ArraySize can't be zero. This error should've be caught by ValidatePipelineResourceSignatureDesc()."); + if (ResDesc.ArraySize < SPIRVAttribs.ArraySize) { - LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource with name '", SPIRVAttribs.Name, - "' and array size (", SPIRVAttribs.ArraySize, ") that is not compatible with array size '", - ResDesc.ArraySize, "' in pipeline resource signature '", Info.Signature->GetDesc().Name, "'."); + LOG_ERROR_AND_THROW("Shader '", pShader->GetDesc().Name, "' contains resource '", SPIRVAttribs.Name, + "' whose array size (", SPIRVAttribs.ArraySize, ") is greater than the array size (", + ResDesc.ArraySize, ") specified by the pipeline resource signature '", Info.Signature->GetDesc().Name, "'."); } } -- cgit v1.2.3