From dd7383d6a963319bfac75cbb30ea4354d47b9ea7 Mon Sep 17 00:00:00 2001 From: assiduous Date: Sat, 30 Jan 2021 19:04:02 -0800 Subject: Shader resource cache Vk: added debug checks for dynamic buffers and offsets --- .../include/PipelineResourceSignatureVkImpl.hpp | 4 +++ .../include/ShaderResourceCacheVk.hpp | 19 +++++++------ .../src/ShaderResourceCacheVk.cpp | 33 ++++++++++++++-------- 3 files changed, 37 insertions(+), 19 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp index 94be0f23..55d5bfe7 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp +++ b/Graphics/GraphicsEngineVulkan/include/PipelineResourceSignatureVkImpl.hpp @@ -337,7 +337,11 @@ private: // Shader stages that have resources. SHADER_TYPE m_ShaderStages = SHADER_TYPE_UNKNOWN; + // The total number of uniform buffers with dynamic offsets in both descriptor sets, + // accounting for array size. Uint16 m_DynamicUniformBufferCount = 0; + // The total number storage buffers with dynamic offsets in both descriptor sets, + // accounting for array size. Uint16 m_DynamicStorageBufferCount = 0; // Mapping from shader type index given by GetShaderTypePipelineIndex() to diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp index db5510d6..7b546bdd 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp @@ -211,8 +211,8 @@ private: void* m_pMemory = nullptr; Uint16 m_NumSets = 0; - // Total number of dynamic buffers (that was created with USAGE_DYNAMIC) bound in the resource cache regardless of the variable type. - // This variable is not equal to dynamic offsets count. + // 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. Uint16 m_NumDynamicBuffers = 0; Uint32 m_TotalResources : 31; @@ -236,15 +236,18 @@ __forceinline Uint32 ShaderResourceCacheVk::GetDynamicBufferOffsets(Uint32 // numbers (unclear if this is SPIRV binding or VkDescriptorSetLayoutBinding number) in the // descriptor set layouts; and within a binding array, elements are in order. (13.2.5) - // In each descriptor set, all uniform buffers for every shader stage come first, - // followed by all storage buffers for every shader stage, followed by all other resources + // In each descriptor set, all uniform buffers with dynamic offsets (DescriptorType::UniformBufferDynamic) + // for every shader stage come first, followed by all storage buffers with dynamic offsets + // (DescriptorType::StorageBufferDynamic and DescriptorType::StorageBufferDynamic_ReadOnly) for every shader stage, + // followed by all other resources. Uint32 OffsetInd = 0; for (Uint32 set = 0; set < m_NumSets; ++set) { const auto& DescrSet = GetDescriptorSet(set); - Uint32 res = 0; + const auto SetSize = DescrSet.GetSize(); - while (res < DescrSet.GetSize()) + Uint32 res = 0; + while (res < SetSize) { const auto& Res = DescrSet.GetResource(res); if (Res.Type == DescriptorType::UniformBufferDynamic) @@ -258,7 +261,7 @@ __forceinline Uint32 ShaderResourceCacheVk::GetDynamicBufferOffsets(Uint32 break; } - while (res < DescrSet.GetSize()) + while (res < SetSize) { const auto& Res = DescrSet.GetResource(res); if (Res.Type == DescriptorType::StorageBufferDynamic || @@ -275,7 +278,7 @@ __forceinline Uint32 ShaderResourceCacheVk::GetDynamicBufferOffsets(Uint32 } #ifdef DILIGENT_DEBUG - for (; res < DescrSet.GetSize(); ++res) + for (; res < SetSize; ++res) { const auto& Res = DescrSet.GetResource(res); // clang-format off diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp index 177256db..9964313c 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp @@ -126,21 +126,26 @@ void ShaderResourceCacheVk::DbgVerifyDynamicBuffersCounter() const switch (Res.Type) { case DescriptorType::UniformBuffer: + VERIFY(!Res.pObject || Res.pObject.RawPtr()->GetDesc().Usage != USAGE_DYNAMIC, + "Dynamic uniform buffer is bound to a non-dynamic descriptor. The buffer's dynamic offset will not be properly handled."); + break; + case DescriptorType::UniformBufferDynamic: - { if (Res.pObject && Res.pObject.RawPtr()->GetDesc().Usage == USAGE_DYNAMIC) ++NumDynamicBuffers; break; - } + case DescriptorType::StorageBuffer: - case DescriptorType::StorageBufferDynamic: case DescriptorType::StorageBuffer_ReadOnly: + VERIFY(!Res.pObject || Res.pObject.RawPtr()->GetBuffer()->GetDesc().Usage != USAGE_DYNAMIC, + "Dynamic storage buffer is bound to a non-dynamic descriptor. The buffer's dynamic offset will not be properly handled."); + break; + + case DescriptorType::StorageBufferDynamic: case DescriptorType::StorageBufferDynamic_ReadOnly: - { if (Res.pObject && Res.pObject.RawPtr()->GetBuffer()->GetDesc().Usage == USAGE_DYNAMIC) ++NumDynamicBuffers; break; - } } } VERIFY(NumDynamicBuffers == m_NumDynamicBuffers, "The number of dynamic buffers (", m_NumDynamicBuffers, ") does not match the actual number (", NumDynamicBuffers, ")"); @@ -332,8 +337,8 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) auto* pTLASVk = Res.pObject.RawPtr(); if (pTLASVk != nullptr && pTLASVk->IsInKnownState()) { - constexpr RESOURCE_STATE RequiredState = RESOURCE_STATE_RAY_TRACING; - const bool IsInRequiredState = pTLASVk->CheckState(RequiredState); + constexpr auto RequiredState = RESOURCE_STATE_RAY_TRACING; + const bool IsInRequiredState = pTLASVk->CheckState(RequiredState); if (VerifyOnly) { if (!IsInRequiredState) @@ -383,6 +388,8 @@ VkDescriptorBufferInfo ShaderResourceCacheVk::Resource::GetUniformBufferDescript // VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER or VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC descriptor type require // buffer to be created with VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT VERIFY_EXPR((pBuffVk->GetDesc().BindFlags & BIND_UNIFORM_BUFFER) != 0); + VERIFY(Type == DescriptorType::UniformBufferDynamic || pBuffVk->GetDesc().Usage != USAGE_DYNAMIC, + "Dynamic buffer must be used with UniformBufferDynamic descriptor"); VkDescriptorBufferInfo DescrBuffInfo; DescrBuffInfo.buffer = pBuffVk->GetVkBuffer(); @@ -404,9 +411,11 @@ VkDescriptorBufferInfo ShaderResourceCacheVk::Resource::GetStorageBufferDescript // clang-format on DEV_CHECK_ERR(pObject != nullptr, "Unable to get storage buffer write info: cached object is null"); - auto* pBuffViewVk = pObject.RawPtr(); + auto* const pBuffViewVk = pObject.RawPtr(); const auto& ViewDesc = pBuffViewVk->GetDesc(); - auto* pBuffVk = pBuffViewVk->GetBufferVk(); + auto* const pBuffVk = pBuffViewVk->GetBufferVk(); + VERIFY(Type == DescriptorType::StorageBufferDynamic || Type == DescriptorType::StorageBufferDynamic_ReadOnly || pBuffVk->GetDesc().Usage != USAGE_DYNAMIC, + "Dynamic buffer must be used with StorageBufferDynamic or StorageBufferDynamic_ReadOnly descriptor"); // VK_DESCRIPTOR_TYPE_STORAGE_BUFFER or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC descriptor type // require buffer to be created with VK_BUFFER_USAGE_STORAGE_BUFFER_BIT (13.2.4) @@ -416,14 +425,16 @@ VkDescriptorBufferInfo ShaderResourceCacheVk::Resource::GetStorageBufferDescript VERIFY(ViewDesc.ViewType == BUFFER_VIEW_SHADER_RESOURCE, "Attempting to bind buffer view '", ViewDesc.Name, "' as read-only storage buffer. Expected view type is BUFFER_VIEW_SHADER_RESOURCE. Actual type: ", GetBufferViewTypeLiteralName(ViewDesc.ViewType)); - VERIFY((pBuffVk->GetDesc().BindFlags & BIND_SHADER_RESOURCE) != 0, "Buffer '", pBuffVk->GetDesc().Name, "' being set as read-only storage buffer was not created with BIND_SHADER_RESOURCE flag"); + VERIFY((pBuffVk->GetDesc().BindFlags & BIND_SHADER_RESOURCE) != 0, + "Buffer '", pBuffVk->GetDesc().Name, "' being set as read-only storage buffer was not created with BIND_SHADER_RESOURCE flag"); } else if (Type == DescriptorType::StorageBuffer || Type == DescriptorType::StorageBufferDynamic) { VERIFY(ViewDesc.ViewType == BUFFER_VIEW_UNORDERED_ACCESS, "Attempting to bind buffer view '", ViewDesc.Name, "' as writable storage buffer. Expected view type is BUFFER_VIEW_UNORDERED_ACCESS. Actual type: ", GetBufferViewTypeLiteralName(ViewDesc.ViewType)); - VERIFY((pBuffVk->GetDesc().BindFlags & BIND_UNORDERED_ACCESS) != 0, "Buffer '", pBuffVk->GetDesc().Name, "' being set as writable storage buffer was not created with BIND_UNORDERED_ACCESS flag"); + VERIFY((pBuffVk->GetDesc().BindFlags & BIND_UNORDERED_ACCESS) != 0, + "Buffer '", pBuffVk->GetDesc().Name, "' being set as writable storage buffer was not created with BIND_UNORDERED_ACCESS flag"); } else { -- cgit v1.2.3