From 58687d731d9736b7358cb4ed183c510372948c46 Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Fri, 23 Nov 2018 23:17:40 -0800 Subject: Improved reporting of resource binding problems in InitializeStaticResources() --- .../GraphicsEngineVulkan/include/DescriptorPoolManager.h | 2 +- .../GraphicsEngineVulkan/include/DeviceContextVkImpl.h | 4 ++-- Graphics/GraphicsEngineVulkan/include/PipelineLayout.h | 9 ++++++--- .../GraphicsEngineVulkan/include/RenderDeviceVkImpl.h | 4 ++-- .../GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h | 2 +- Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h | 2 +- .../GraphicsEngineVulkan/src/DescriptorPoolManager.cpp | 6 +++--- .../GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp | 2 +- Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp | 15 ++++++++++++--- Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp | 10 ++++++++-- .../src/ShaderResourceBindingVkImpl.cpp | 10 ++++++++-- .../GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp | 5 ++++- Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp | 4 ++-- 13 files changed, 51 insertions(+), 24 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h b/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h index 6749b697..16717d27 100644 --- a/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h +++ b/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h @@ -198,7 +198,7 @@ public: ~DescriptorSetAllocator(); - DescriptorSetAllocation Allocate(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout); + DescriptorSetAllocation Allocate(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout, const char* DebugName = ""); #ifdef DEVELOPMENT int32_t GetAllocatedDescriptorSetCounter()const{return m_AllocatedSetCounter;} diff --git a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h index 0dc9371a..23d10485 100644 --- a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h +++ b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h @@ -184,11 +184,11 @@ public: virtual void TransitionResourceStates(Uint32 BarrierCount, StateTransitionDesc* pResourceBarriers)override final; - VkDescriptorSet AllocateDynamicDescriptorSet(VkDescriptorSetLayout SetLayout) + VkDescriptorSet AllocateDynamicDescriptorSet(VkDescriptorSetLayout SetLayout, const char* DebugName = "") { // Descriptor pools are externally synchronized, meaning that the application must not allocate // and/or free descriptor sets from the same pool in multiple threads simultaneously (13.2.3) - return m_DynamicDescrSetAllocator.Allocate(SetLayout, ""); + return m_DynamicDescrSetAllocator.Allocate(SetLayout, DebugName); } VulkanDynamicAllocation AllocateDynamicSpace(Uint32 SizeInBytes, Uint32 Alignment); diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h index fbe0ea20..a45146cf 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h +++ b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h @@ -38,13 +38,13 @@ namespace Diligent class RenderDeviceVkImpl; class DeviceContextVkImpl; - +class ShaderResourceCacheVk; /// Implementation of the Diligent::PipelineLayout class class PipelineLayout { public: - static VkDescriptorType GetVkDescriptorType(const SPIRVShaderResourceAttribs &Res); + static VkDescriptorType GetVkDescriptorType(const SPIRVShaderResourceAttribs& Res); PipelineLayout(); void Release(RenderDeviceVkImpl* pDeviceVkImpl, Uint64 CommandQueueMask); @@ -52,7 +52,10 @@ public: VkPipelineLayout GetVkPipelineLayout()const{return m_LayoutMgr.GetVkPipelineLayout();} std::array GetDescriptorSetSizes(Uint32& NumSets)const; - void InitResourceCache(RenderDeviceVkImpl* pDeviceVkImpl, class ShaderResourceCacheVk& ResourceCache, IMemoryAllocator& CacheMemAllocator)const; + void InitResourceCache(RenderDeviceVkImpl* pDeviceVkImpl, + ShaderResourceCacheVk& ResourceCache, + IMemoryAllocator& CacheMemAllocator, + const char* DbgPipelineName)const; void AllocateResourceSlot(const SPIRVShaderResourceAttribs& ResAttribs, VkSampler vkStaticSampler, diff --git a/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h b/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h index da87afd5..22ec6e15 100644 --- a/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h +++ b/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h @@ -99,9 +99,9 @@ public: virtual void ReleaseStaleResources(bool ForceRelease = false)override final; - DescriptorSetAllocation AllocateDescriptorSet(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout) + DescriptorSetAllocation AllocateDescriptorSet(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout, const char* DebugName = "") { - return m_DescriptorSetAllocator.Allocate(CommandQueueMask, SetLayout); + return m_DescriptorSetAllocator.Allocate(CommandQueueMask, SetLayout, DebugName); } DescriptorPoolManager& GetDynamicDescriptorPool(){return m_DynamicDescriptorPool;} diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h index c53d4d1f..e72c867e 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h @@ -230,7 +230,7 @@ public: ShaderResourceCacheVk& DstResourceCache)const; #ifdef DEVELOPMENT - void dvpVerifyBindings(const ShaderResourceCacheVk& ResourceCache)const; + bool dvpVerifyBindings(const ShaderResourceCacheVk& ResourceCache)const; #endif Uint32 GetResourceCount(SHADER_VARIABLE_TYPE VarType)const diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h b/Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h index ca0373bf..0c45c33d 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderVkImpl.h @@ -83,7 +83,7 @@ public: const char* GetEntryPoint() const { return m_EntryPoint.c_str(); } #ifdef DEVELOPMENT - void DvpVerifyStaticResourceBindings()const; + bool DvpVerifyStaticResourceBindings()const; #endif private: diff --git a/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp b/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp index cd2cb7a1..82c4d0cf 100644 --- a/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp +++ b/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp @@ -149,7 +149,7 @@ DescriptorSetAllocator::~DescriptorSetAllocator() DEV_CHECK_ERR(m_AllocatedSetCounter == 0, m_AllocatedSetCounter, " descriptor set(s) have not been returned to the allocator. If there are outstanding references to the sets in release queues, the app will crash when DescriptorSetAllocator::FreeDescriptorSet() is called"); } -DescriptorSetAllocation DescriptorSetAllocator::Allocate(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout) +DescriptorSetAllocation DescriptorSetAllocator::Allocate(Uint64 CommandQueueMask, VkDescriptorSetLayout SetLayout, const char* DebugName) { // Descriptor pools are externally synchronized, meaning that the application must not allocate // and/or free descriptor sets from the same pool in multiple threads simultaneously (13.2.3) @@ -160,7 +160,7 @@ DescriptorSetAllocation DescriptorSetAllocator::Allocate(Uint64 CommandQueueMask for(auto it = m_Pools.begin(); it != m_Pools.end(); ++it) { auto& Pool = *it; - auto Set = AllocateDescriptorSet(LogicalDevice, Pool, SetLayout, "Descriptor set"); + auto Set = AllocateDescriptorSet(LogicalDevice, Pool, SetLayout, DebugName); if (Set != VK_NULL_HANDLE) { // Move the pool to the front @@ -181,7 +181,7 @@ DescriptorSetAllocation DescriptorSetAllocator::Allocate(Uint64 CommandQueueMask m_Pools.emplace_front(CreateDescriptorPool("Descriptor pool")); auto& NewPool = m_Pools.front(); - auto Set = AllocateDescriptorSet(LogicalDevice, NewPool, SetLayout, ""); + auto Set = AllocateDescriptorSet(LogicalDevice, NewPool, SetLayout, DebugName); DEV_CHECK_ERR(Set != VK_NULL_HANDLE, "Failed to allocate descriptor set"); #ifdef DEVELOPMENT diff --git a/Graphics/GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp b/Graphics/GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp index 8137e7cf..7fcc6b27 100644 --- a/Graphics/GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp +++ b/Graphics/GraphicsEngineVulkan/src/GenerateMipsVkHelper.cpp @@ -183,7 +183,7 @@ namespace Diligent { // All PSOs are compatible auto& PSO = FindPSOs(TEX_FORMAT_RGBA8_UNORM); - PSO[0]->CreateShaderResourceBinding(ppSRB); + PSO[0]->CreateShaderResourceBinding(ppSRB, true); } std::array, 4>& GenerateMipsVkHelper::FindPSOs(TEXTURE_FORMAT Fmt) diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp index 9260450b..9897a1d2 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp @@ -404,7 +404,10 @@ std::array PipelineLayout::GetDescriptorSetSizes(Uint32& NumSets)cons return SetSizes; } -void PipelineLayout::InitResourceCache(RenderDeviceVkImpl* pDeviceVkImpl, ShaderResourceCacheVk& ResourceCache, IMemoryAllocator& CacheMemAllocator)const +void PipelineLayout::InitResourceCache(RenderDeviceVkImpl* pDeviceVkImpl, + ShaderResourceCacheVk& ResourceCache, + IMemoryAllocator& CacheMemAllocator, + const char* DbgPipelineName)const { Uint32 NumSets = 0; auto SetSizes = GetDescriptorSetSizes(NumSets); @@ -413,10 +416,16 @@ void PipelineLayout::InitResourceCache(RenderDeviceVkImpl* pDeviceVkImpl, Shader // Resources are initialized by source layout when shader resource binding objects are created ResourceCache.InitializeSets(CacheMemAllocator, NumSets, SetSizes.data()); - const auto &StaticAndMutSet = m_LayoutMgr.GetDescriptorSet(SHADER_VARIABLE_TYPE_STATIC); + const auto& StaticAndMutSet = m_LayoutMgr.GetDescriptorSet(SHADER_VARIABLE_TYPE_STATIC); if (StaticAndMutSet.SetIndex >= 0) { - DescriptorSetAllocation SetAllocation = pDeviceVkImpl->AllocateDescriptorSet(~Uint64{0}, StaticAndMutSet.VkLayout); + const char* DescrSetName = "Static/Mutable Descriptor Set"; +#ifdef DEVELOPMENT + std::string _DescrSetName(DbgPipelineName); + _DescrSetName.append(" - static/mutable set"); + DescrSetName = _DescrSetName.c_str(); +#endif + DescriptorSetAllocation SetAllocation = pDeviceVkImpl->AllocateDescriptorSet(~Uint64{0}, StaticAndMutSet.VkLayout, DescrSetName); ResourceCache.GetDescriptorSet(StaticAndMutSet.SetIndex).AssignDescriptorSetAllocation(std::move(SetAllocation)); } } diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp index 24d9b068..2761b011 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp @@ -536,7 +536,7 @@ void PipelineStateVkImpl::CommitAndTransitionShaderResources(IShaderResourceBind if (m_HasStaticResources && !pResBindingVkImpl->StaticResourcesInitialized()) { - LOG_ERROR_MESSAGE("Static resources have not been initialized in the shader resource binding object. Please call IShaderResourceBinding::InitializeStaticResources()."); + LOG_ERROR_MESSAGE("Static resources have not been initialized in the shader resource binding object being committed for PSO '", m_Desc.Name,"'. Please call IShaderResourceBinding::InitializeStaticResources()."); } #endif @@ -564,8 +564,14 @@ void PipelineStateVkImpl::CommitAndTransitionShaderResources(IShaderResourceBind auto DynamicDescriptorSetVkLayout = m_PipelineLayout.GetDynamicDescriptorSetVkLayout(); if (DynamicDescriptorSetVkLayout != VK_NULL_HANDLE) { + const char* DynamicDescrSetName = "Dynamic Descriptor Set"; +#ifdef DEVELOPMENT + std::string _DynamicDescrSetName(m_Desc.Name); + _DynamicDescrSetName.append(" - dynamic set"); + DynamicDescrSetName = _DynamicDescrSetName.c_str(); +#endif // Allocate vulkan descriptor set for dynamic resources - DynamicDescrSet = pCtxVkImpl->AllocateDynamicDescriptorSet(DynamicDescriptorSetVkLayout); + DynamicDescrSet = pCtxVkImpl->AllocateDynamicDescriptorSet(DynamicDescriptorSetVkLayout, DynamicDescrSetName); // Commit all dynamic resource descriptors for (Uint32 s=0; s < m_NumShaders; ++s) { diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp index d3f885a3..c4c899b5 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp @@ -42,7 +42,7 @@ ShaderResourceBindingVkImpl::ShaderResourceBindingVkImpl( IReferenceCounters* pR // This will only allocate memory and initialize descriptor sets in the resource cache // Resources will be initialized by InitializeResourceMemoryInCache() auto& ResourceCacheDataAllocator = pPSO->GetSRBMemoryAllocator().GetResourceCacheDataAllocator(0); - pPSO->GetPipelineLayout().InitResourceCache(pRenderDeviceVkImpl, m_ShaderResourceCache, ResourceCacheDataAllocator); + pPSO->GetPipelineLayout().InitResourceCache(pRenderDeviceVkImpl, m_ShaderResourceCache, ResourceCacheDataAllocator, pPSO->GetDesc().Name); auto *pVarMgrsRawMem = ALLOCATE(GetRawAllocator(), "Raw memory for ShaderVariableManagerVk", m_NumShaders * sizeof(ShaderVariableManagerVk)); m_pShaderVarMgrs = reinterpret_cast(pVarMgrsRawMem); @@ -159,7 +159,13 @@ void ShaderResourceBindingVkImpl::InitializeStaticResources(const IPipelineState { const auto* pShaderVk = pPSOVK->GetShader(s); #ifdef DEVELOPMENT - pShaderVk->DvpVerifyStaticResourceBindings(); + if (!pShaderVk->DvpVerifyStaticResourceBindings()) + { + LOG_ERROR_MESSAGE("Static resources in a SRB of PSO '", pPSOVK->GetDesc().Name, "' will not be successfully initialized " + "because not all static resource bindings in shader '", pShaderVk->GetDesc().Name, "' are valid. " + "Please make sure you bind all static resources to the shader before calling InitializeStaticResources() or " + "before creating a SRB via CreateShaderResourceBinding() method with InitStaticResources=true."); + } #endif const auto& StaticResLayout = pShaderVk->GetStaticResLayout(); const auto& StaticResCache = pShaderVk->GetStaticResCache(); diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp index d0ba2d3d..f544e6cb 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp @@ -725,8 +725,9 @@ void ShaderResourceLayoutVk::InitializeStaticResources(const ShaderResourceLayou #ifdef DEVELOPMENT -void ShaderResourceLayoutVk::dvpVerifyBindings(const ShaderResourceCacheVk& ResourceCache)const +bool ShaderResourceLayoutVk::dvpVerifyBindings(const ShaderResourceCacheVk& ResourceCache)const { + bool BindingsOK = true; for(SHADER_VARIABLE_TYPE VarType = SHADER_VARIABLE_TYPE_STATIC; VarType < SHADER_VARIABLE_TYPE_NUM_TYPES; VarType = static_cast(VarType+1)) { for(Uint32 r=0; r < m_NumResources[VarType]; ++r) @@ -742,6 +743,7 @@ void ShaderResourceLayoutVk::dvpVerifyBindings(const ShaderResourceCacheVk& Reso !(Res.SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler && Res.SpirvAttribs.IsImmutableSamplerAssigned())) { LOG_ERROR_MESSAGE("No resource is bound to ", GetShaderVariableTypeLiteralName(Res.SpirvAttribs.VarType), " variable '", Res.SpirvAttribs.GetPrintName(ArrInd), "' in shader '", GetShaderName(), "'"); + BindingsOK = false; } #ifdef _DEBUG auto vkDescSet = CachedDescrSet.GetVkDescriptorSet(); @@ -765,6 +767,7 @@ void ShaderResourceLayoutVk::dvpVerifyBindings(const ShaderResourceCacheVk& Reso } } } + return BindingsOK; } #endif diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp index a0a76499..a735c691 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderVkImpl.cpp @@ -129,9 +129,9 @@ ShaderVkImpl::~ShaderVkImpl() } #ifdef DEVELOPMENT -void ShaderVkImpl::DvpVerifyStaticResourceBindings()const +bool ShaderVkImpl::DvpVerifyStaticResourceBindings()const { - m_StaticResLayout.dvpVerifyBindings(m_StaticResCache); + return m_StaticResLayout.dvpVerifyBindings(m_StaticResCache); } #endif -- cgit v1.2.3