From 308807cc287cedb9a4f5077dff7e00ea1c8cb082 Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Sat, 19 May 2018 12:19:30 -0700 Subject: Fixed some issues with Vk Shader Resource Layout --- .../include/ShaderResourceCacheVk.h | 12 ++-- .../include/ShaderResourceLayoutVk.h | 41 +++++++------ .../src/ShaderResourceLayoutVk.cpp | 69 ++++++++++++---------- 3 files changed, 64 insertions(+), 58 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h index ff6aee2d..5ad52522 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h @@ -135,14 +135,14 @@ public: #endif private: - ShaderResourceCacheVk(const ShaderResourceCacheVk&) = delete; - ShaderResourceCacheVk(ShaderResourceCacheVk&&) = delete; + ShaderResourceCacheVk (const ShaderResourceCacheVk&) = delete; + ShaderResourceCacheVk (ShaderResourceCacheVk&&) = delete; ShaderResourceCacheVk& operator = (const ShaderResourceCacheVk&) = delete; - ShaderResourceCacheVk& operator = (ShaderResourceCacheVk&&) = delete; + ShaderResourceCacheVk& operator = (ShaderResourceCacheVk&&) = delete; - IMemoryAllocator *m_pAllocator=nullptr; - void *m_pMemory = nullptr; - Uint32 m_NumSets = 0; + IMemoryAllocator* m_pAllocator = nullptr; + void* m_pMemory = nullptr; + Uint32 m_NumSets = 0; #ifdef _DEBUG // Only for debug purposes: indicates what types of resources are stored in the cache diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h index 1a217684..68feaded 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.h @@ -46,16 +46,14 @@ // // // Every VkResource structure holds a reference to SPIRVShaderResourceAttribs structure from ShaderResources. -// ShaderResourceLayoutVk holds shared pointer to ShaderResourcesVk instance. Note that ShaderResources::SamplerId -// references a sampler in ShaderResources, while VkResource::SamplerId references a sampler in ShaderResourceLayoutVk, -// and the two are not the same +// ShaderResourceLayoutVk holds shared pointer to ShaderResourcesVk instance. // // ______________________ ________________________________________________________________________ // | | unique_ptr | | | | | | | | // | SPIRVShaderResources |--------------->| UBs | SBs | StrgImgs | SmplImgs | ACs | SepImgs | SepSamplers | // |______________________| |________|_________|__________|__________|_______|_________|_____________| -// A A A -// | \ | +// A A A +// | | | // |shared_ptr Ref Ref // ________|__________________ ________\____________________|_____________________________________________ // | | unique_ptr | | | | | @@ -63,7 +61,7 @@ // |___________________________| |___________________|_________________|_______________|_____________________| // | | | // | Raw ptr | | -// | | / +// | | (DescriptorSet, CacheOffset) / (DescriptorSet, CacheOffset) // | \ / // ________V_________________ ________V_______________________________________________________V_______ // | | | | @@ -113,7 +111,7 @@ namespace Diligent { /// Diligent::ShaderResourceLayoutVk class -// sizeof(ShaderResourceLayoutVk)==?? (MS compiler, x64) +// sizeof(ShaderResourceLayoutVk)==72 (MS compiler, x64) class ShaderResourceLayoutVk { public: @@ -128,10 +126,10 @@ public: Uint32 NumAllowedTypes, ShaderResourceCacheVk &ResourceCache); - ShaderResourceLayoutVk(const ShaderResourceLayoutVk&) = delete; - ShaderResourceLayoutVk(ShaderResourceLayoutVk&&) = delete; - ShaderResourceLayoutVk& operator =(const ShaderResourceLayoutVk&) = delete; - ShaderResourceLayoutVk& operator =(ShaderResourceLayoutVk&&) = delete; + ShaderResourceLayoutVk (const ShaderResourceLayoutVk&) = delete; + ShaderResourceLayoutVk (ShaderResourceLayoutVk&&) = delete; + ShaderResourceLayoutVk& operator = (const ShaderResourceLayoutVk&) = delete; + ShaderResourceLayoutVk& operator = (ShaderResourceLayoutVk&&) = delete; ~ShaderResourceLayoutVk(); @@ -150,7 +148,7 @@ public: std::vector* pSPIRV, class PipelineLayout* pPipelineLayout); - // sizeof(VkResource) == ?? (x64) + // sizeof(VkResource) == 32 (x64) struct VkResource : IShaderVariable { VkResource(const VkResource&) = delete; @@ -168,7 +166,7 @@ public: const SPIRVShaderResourceAttribs& _SpirvAttribs, uint32_t _Binding, uint32_t _DescriptorSet, - Uint32 _CacheOffset) : + Uint32 _CacheOffset)noexcept : Binding (static_cast(_Binding)), DescriptorSet (static_cast(_DescriptorSet)), CacheOffset (_CacheOffset), @@ -180,7 +178,7 @@ public: } VkResource(ShaderResourceLayoutVk& _ParentLayout, - const VkResource& _SrcRes) : + const VkResource& _SrcRes)noexcept : Binding (_SrcRes.Binding), DescriptorSet (_SrcRes.DescriptorSet), CacheOffset (_SrcRes.CacheOffset), @@ -280,13 +278,6 @@ private: const Char* GetShaderName()const; - // There is no need to use shared ptr as referenced resource cache is either part of the - // parent ShaderVkImpl object or ShaderResourceBindingVkImpl object - ShaderResourceCacheVk *m_pResourceCache; - - std::unique_ptr > m_ResourceBuffer; - std::array m_NumResources = {}; - Uint32 GetResourceOffset(SHADER_VARIABLE_TYPE VarType, Uint32 r)const { VERIFY_EXPR( r < m_NumResources[VarType] ); @@ -323,6 +314,14 @@ private: void AllocateMemory(IMemoryAllocator &Allocator); + + // There is no need to use shared ptr as referenced resource cache is either part of the + // parent ShaderVkImpl object or ShaderResourceBindingVkImpl object + ShaderResourceCacheVk *m_pResourceCache; + + std::unique_ptr > m_ResourceBuffer; + std::array m_NumResources = {}; + #if USE_VARIABLE_HASH_MAP // Hash map to look up shader variables by name. // Note that sizeof(m_VariableHash)==128 (release mode, MS compiler, x64). diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp index 1bfacee3..6a95af24 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp @@ -197,13 +197,15 @@ void ShaderResourceLayoutVk::Initialize(const VulkanUtilities::VulkanLogicalDevi { // If pipeline layout is not provided - use artifial layout to store // static shader resources: - // Uniform buffers at index SPIRVShaderResourceAttribs::ResourceType::UniformBuffer (0) - // Storage buffers at index SPIRVShaderResourceAttribs::ResourceType::StorageBuffer (1) - // Storage images at index SPIRVShaderResourceAttribs::ResourceType::StorageImage (2) - // Sampled images at index SPIRVShaderResourceAttribs::ResourceType::SampledImage (3) - // Atomic counters at index SPIRVShaderResourceAttribs::ResourceType::AtomicCounter (4) - // Separate images at index SPIRVShaderResourceAttribs::ResourceType::SeparateImage (5) - // Separate samplers at index SPIRVShaderResourceAttribs::ResourceType::SeparateSampler(6) + // Uniform buffers at index SPIRVShaderResourceAttribs::ResourceType::UniformBuffer (0) + // Storage buffers at index SPIRVShaderResourceAttribs::ResourceType::StorageBuffer (1) + // Unifrom txl buffs at index SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer (2) + // Storage txl buffs at index SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer (3) + // Storage images at index SPIRVShaderResourceAttribs::ResourceType::StorageImage (4) + // Sampled images at index SPIRVShaderResourceAttribs::ResourceType::SampledImage (5) + // Atomic counters at index SPIRVShaderResourceAttribs::ResourceType::AtomicCounter (6) + // Separate images at index SPIRVShaderResourceAttribs::ResourceType::SeparateImage (7) + // Separate samplers at index SPIRVShaderResourceAttribs::ResourceType::SeparateSampler (8) VERIFY_EXPR(m_pResourceCache != nullptr); DescriptorSet = Attribs.Type; @@ -410,8 +412,8 @@ void ShaderResourceLayoutVk::VkResource::CacheTexelBuffer(IDeviceObject* #ifdef VERIFY_SHADER_BINDINGS const auto& ViewDesc = pBuffViewVk->GetDesc(); const auto ViewType = ViewDesc.ViewType; - const bool IsStorageBuffer = SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::StorageBuffer; - const auto dbgExpectedViewType = IsStorageBuffer ? TEXTURE_VIEW_UNORDERED_ACCESS : TEXTURE_VIEW_SHADER_RESOURCE; + const bool IsStorageBuffer = SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer; + const auto dbgExpectedViewType = IsStorageBuffer ? BUFFER_VIEW_UNORDERED_ACCESS : BUFFER_VIEW_SHADER_RESOURCE; if (ViewType != dbgExpectedViewType) { const auto *ExpectedViewTypeName = GetViewTypeLiteralName(dbgExpectedViewType); @@ -430,10 +432,11 @@ void ShaderResourceLayoutVk::VkResource::CacheTexelBuffer(IDeviceObject* } } } -void ShaderResourceLayoutVk::VkResource::CacheImage(IDeviceObject *pTexView, + +void ShaderResourceLayoutVk::VkResource::CacheImage(IDeviceObject* pTexView, ShaderResourceCacheVk::Resource& DstRes, - VkDescriptorSet vkDescrSet, - Uint32 ArrayInd) + VkDescriptorSet vkDescrSet, + Uint32 ArrayInd) { VERIFY(SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::StorageImage || SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateImage || @@ -443,11 +446,11 @@ void ShaderResourceLayoutVk::VkResource::CacheImage(IDeviceObject *pTexView, if (UpdateCachedResource(DstRes, ArrayInd, pTexView, IID_TextureViewVk, "texture view") ) { auto* pTexViewVk = DstRes.pObject.RawPtr(); + const bool IsStorageImage = SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::StorageImage; #ifdef VERIFY_SHADER_BINDINGS const auto& ViewDesc = pTexViewVk->GetDesc(); const auto ViewType = ViewDesc.ViewType; - const bool IsStorageImage = SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::StorageImage; const auto dbgExpectedViewType = IsStorageImage ? TEXTURE_VIEW_UNORDERED_ACCESS : TEXTURE_VIEW_SHADER_RESOURCE; if (ViewType != dbgExpectedViewType) { @@ -483,14 +486,14 @@ void ShaderResourceLayoutVk::VkResource::CacheImage(IDeviceObject *pTexView, } } -void ShaderResourceLayoutVk::VkResource::CacheSeparateSampler(IDeviceObject *pSampler, - ShaderResourceCacheVk::Resource& DstRes, - VkDescriptorSet vkDescrSet, - Uint32 ArrayInd) +void ShaderResourceLayoutVk::VkResource::CacheSeparateSampler(IDeviceObject* pSampler, + ShaderResourceCacheVk::Resource& DstRes, + VkDescriptorSet vkDescrSet, + Uint32 ArrayInd) { VERIFY(SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateSampler, "Separate sampler resource is expected"); - if (UpdateCachedResource(DstRes, ArrayInd, pSampler, IID_TextureViewVk, "sampler")) + if (UpdateCachedResource(DstRes, ArrayInd, pSampler, IID_Sampler, "sampler")) { auto* pSamplerVk = DstRes.pObject.RawPtr(); if (vkDescrSet != VK_NULL_HANDLE) @@ -502,10 +505,6 @@ void ShaderResourceLayoutVk::VkResource::CacheSeparateSampler(IDeviceObject *pSa UpdateDescriptorHandle(vkDescrSet, ArrayInd, &DescrImgInfo, nullptr, nullptr); } } - else - { - LOG_RESOURCE_BINDING_ERROR("Sampler", pSampler, SpirvAttribs.GetPrintName(ArrayInd), ParentResLayout.GetShaderName(), "Incorrect resource type: sampler is expected.") - } } @@ -518,12 +517,18 @@ void ShaderResourceLayoutVk::VkResource::BindResource(IDeviceObject *pObj, Uint3 auto &DstDescrSet = pResourceCache->GetDescriptorSet(DescriptorSet); auto vkDescrSet = DstDescrSet.GetVkDescriptorSet(); +#ifdef _DEBUG if (pResourceCache->DbgGetContentType() == ShaderResourceCacheVk::DbgCacheContentType::SRBResources) { VERIFY(SpirvAttribs.VarType == SHADER_VARIABLE_TYPE_DYNAMIC && vkDescrSet == VK_NULL_HANDLE || SpirvAttribs.VarType != SHADER_VARIABLE_TYPE_DYNAMIC && vkDescrSet != VK_NULL_HANDLE, "Static and mutable variables and only them are expected to have valid descriptor set assigned"); } + else if (pResourceCache->DbgGetContentType() == ShaderResourceCacheVk::DbgCacheContentType::StaticShaderResources) + { + VERIFY(vkDescrSet == VK_NULL_HANDLE, "Static shader resource cache should not have vulkan descriptor set allocation"); + } +#endif auto &DstRes = DstDescrSet.GetResource(CacheOffset + ArrayIndex); if( pObj ) @@ -553,7 +558,7 @@ void ShaderResourceLayoutVk::VkResource::BindResource(IDeviceObject *pObj, Uint3 } else { - LOG_ERROR_MESSAGE("Attempting to assign sampler to static sampler '", SpirvAttribs.Name, '\''); + LOG_ERROR_MESSAGE("Attempting to assign a sampler to a static sampler '", SpirvAttribs.Name, '\''); } break; @@ -679,13 +684,15 @@ void ShaderResourceLayoutVk::InitializeStaticResources(const ShaderResourceLayou VERIFY(SrcLayout.m_pResources->GetShaderType() == m_pResources->GetShaderType(), "Incosistent shader types"); // Static shader resources are stored as follows: - // Uniform buffers at index SPIRVShaderResourceAttribs::ResourceType::UniformBuffer (0) - // Storage buffers at index SPIRVShaderResourceAttribs::ResourceType::StorageBuffer (1) - // Storage images at index SPIRVShaderResourceAttribs::ResourceType::StorageImage (2) - // Sampled images at index SPIRVShaderResourceAttribs::ResourceType::SampledImage (3) - // Atomic counters at index SPIRVShaderResourceAttribs::ResourceType::AtomicCounter (4) - // Separate images at index SPIRVShaderResourceAttribs::ResourceType::SeparateImage (5) - // Separate samplers at index SPIRVShaderResourceAttribs::ResourceType::SeparateSampler(6) + // Uniform buffers at index SPIRVShaderResourceAttribs::ResourceType::UniformBuffer (0) + // Storage buffers at index SPIRVShaderResourceAttribs::ResourceType::StorageBuffer (1) + // Unifrom txl buffs at index SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer (2) + // Storage txl buffs at index SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer (3) + // Storage images at index SPIRVShaderResourceAttribs::ResourceType::StorageImage (4) + // Sampled images at index SPIRVShaderResourceAttribs::ResourceType::SampledImage (5) + // Atomic counters at index SPIRVShaderResourceAttribs::ResourceType::AtomicCounter (6) + // Separate images at index SPIRVShaderResourceAttribs::ResourceType::SeparateImage (7) + // Separate samplers at index SPIRVShaderResourceAttribs::ResourceType::SeparateSampler (8) for(Uint32 r=0; r < m_NumResources[SHADER_VARIABLE_TYPE_STATIC]; ++r) { @@ -746,8 +753,8 @@ void ShaderResourceLayoutVk::dbgVerifyBindings()const } } } -#endif } +#endif const Char* ShaderResourceLayoutVk::GetShaderName()const -- cgit v1.2.3