From 13632c11883fb960bb4166d47adfc361486a1c02 Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Sun, 14 Apr 2019 13:17:04 -0700 Subject: Improved handling of read-only storage buffers (aka buffer SRVs) in Vulkan back-end --- .../GraphicsEngineVulkan/src/PipelineLayout.cpp | 5 +-- .../src/ShaderResourceCacheVk.cpp | 42 +++++++++++++++------- .../src/ShaderResourceLayoutVk.cpp | 18 ++++++---- 3 files changed, 45 insertions(+), 20 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp index 3ee16537..228a6f64 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp @@ -59,9 +59,10 @@ class ResourceTypeToVkDescriptorType public: ResourceTypeToVkDescriptorType() { - static_assert(SPIRVShaderResourceAttribs::ResourceType::NumResourceTypes == 9, "Please add corresponding decriptor type"); + static_assert(SPIRVShaderResourceAttribs::ResourceType::NumResourceTypes == 10, "Please add corresponding decriptor type"); m_Map[SPIRVShaderResourceAttribs::ResourceType::UniformBuffer] = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC; - m_Map[SPIRVShaderResourceAttribs::ResourceType::StorageBuffer] = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC; + m_Map[SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer] = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC; + m_Map[SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer] = VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC; m_Map[SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer] = VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER; m_Map[SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer] = VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER; m_Map[SPIRVShaderResourceAttribs::ResourceType::StorageImage] = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE; diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp index 4aad41d9..06bd2d9a 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp @@ -162,7 +162,8 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) } break; - case SPIRVShaderResourceAttribs::ResourceType::StorageBuffer: + case SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer: + case SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer: case SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer: case SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer: { @@ -170,11 +171,13 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) auto* pBufferVk = pBuffViewVk != nullptr ? ValidatedCast(pBuffViewVk->GetBuffer()) : nullptr; if (pBufferVk != nullptr && pBufferVk->IsInKnownState()) { - const RESOURCE_STATE RequiredState = (Res.Type == SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer) ? - RESOURCE_STATE_SHADER_RESOURCE : RESOURCE_STATE_UNORDERED_ACCESS; + const RESOURCE_STATE RequiredState = + (Res.Type == SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer || + Res.Type == SPIRVShaderResourceAttribs::ResourceType::StorageTexelBuffer) ? + RESOURCE_STATE_UNORDERED_ACCESS : RESOURCE_STATE_SHADER_RESOURCE; #ifdef _DEBUG - const VkAccessFlags RequiredAccessFlags = (Res.Type == SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer) ? - VK_ACCESS_SHADER_READ_BIT : (VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT); + const VkAccessFlags RequiredAccessFlags = (RequiredState == RESOURCE_STATE_SHADER_RESOURCE) ? + VK_ACCESS_SHADER_READ_BIT : (VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT);; VERIFY_EXPR( (ResourceStateFlagsToVkAccessFlags(RequiredState) & RequiredAccessFlags) == RequiredAccessFlags); #endif const bool IsInRequiredState = pBufferVk->CheckState(RequiredState); @@ -310,17 +313,30 @@ VkDescriptorBufferInfo ShaderResourceCacheVk::Resource::GetUniformBufferDescript VkDescriptorBufferInfo ShaderResourceCacheVk::Resource::GetStorageBufferDescriptorWriteInfo()const { - VERIFY(Type == SPIRVShaderResourceAttribs::ResourceType::StorageBuffer, "Storage buffer resource is expected"); + VERIFY(Type == SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer || + Type == SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer, + "Storage buffer resource is expected"); DEV_CHECK_ERR(pObject != nullptr, "Unable to get storage buffer write info: cached object is null"); auto* pBuffViewVk = pObject.RawPtr(); const auto& ViewDesc = pBuffViewVk->GetDesc(); - VERIFY_EXPR(ViewDesc.ViewType == BUFFER_VIEW_UNORDERED_ACCESS); - auto* pBuffVk = pBuffViewVk->GetBufferVk(); + // 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) - VERIFY_EXPR((pBuffVk->GetDesc().BindFlags & BIND_UNORDERED_ACCESS) != 0); + if (Type == SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer) + { + // HLSL buffer SRVs are mapped to read-only storge buffers in SPIR-V + 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"); + } + else + { + 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"); + } VkDescriptorBufferInfo DescrBuffInfo; DescrBuffInfo.buffer = pBuffVk->GetVkBuffer(); @@ -448,7 +464,8 @@ Uint32 ShaderResourceCacheVk::GetDynamicBufferOffsets(DeviceContextVkImpl *pCtxV while (res < DescrSet.GetSize()) { const auto& Res = DescrSet.GetResource(res); - if (Res.Type != SPIRVShaderResourceAttribs::ResourceType::StorageBuffer) + if (Res.Type != SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer && + Res.Type != SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer) break; const auto* pBufferVkView = Res.pObject.RawPtr(); @@ -463,8 +480,9 @@ Uint32 ShaderResourceCacheVk::GetDynamicBufferOffsets(DeviceContextVkImpl *pCtxV for (; res < DescrSet.GetSize(); ++res) { const auto& Res = DescrSet.GetResource(res); - VERIFY(Res.Type != SPIRVShaderResourceAttribs::ResourceType::UniformBuffer && - Res.Type != SPIRVShaderResourceAttribs::ResourceType::StorageBuffer, + VERIFY(Res.Type != SPIRVShaderResourceAttribs::ResourceType::UniformBuffer && + Res.Type != SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer && + Res.Type != SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer, "All uniform and storage buffers are expected to go first in the beginning of each descriptor set"); } #endif diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp index 8486d586..7d88244e 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp @@ -464,7 +464,7 @@ void ShaderResourceLayoutVk::Initialize(IRenderDevice* }, [&](const SPIRVShaderResourceAttribs& SB, Uint32) { - VERIFY_EXPR(SB.Type == SPIRVShaderResourceAttribs::ResourceType::StorageBuffer); + VERIFY_EXPR(SB.Type == SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer || SB.Type == SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer); // Skip }, [&](const SPIRVShaderResourceAttribs& Img, Uint32) @@ -650,7 +650,9 @@ void ShaderResourceLayoutVk::VkResource::CacheStorageBuffer(IDeviceObject* VkDescriptorSet vkDescrSet, Uint32 ArrayInd)const { - VERIFY(SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::StorageBuffer, "Storage buffer resource is expected"); + VERIFY(SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer || + SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer, + "Storage buffer resource is expected"); if( UpdateCachedResource(DstRes, ArrayInd, pBuffer, IID_BufferViewVk, "buffer view") ) { @@ -659,9 +661,11 @@ void ShaderResourceLayoutVk::VkResource::CacheStorageBuffer(IDeviceObject* // require buffer to be created with VK_BUFFER_USAGE_STORAGE_BUFFER_BIT (13.2.4) auto* pBuffViewVk = DstRes.pObject.RawPtr(); auto* pBuffVk = pBuffViewVk->GetBufferVk(); // Use final type - if ((pBuffVk->GetDesc().BindFlags & BIND_UNORDERED_ACCESS) == 0) + // HLSL buffer SRVs are mapped to storge buffers in GLSL + auto RequiredBindFlag = SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer ? BIND_SHADER_RESOURCE : BIND_UNORDERED_ACCESS; + if ((pBuffVk->GetDesc().BindFlags & RequiredBindFlag ) == 0) { - LOG_RESOURCE_BINDING_ERROR("buffer", pBuffer, SpirvAttribs.GetPrintName(ArrayInd), ParentResLayout.GetShaderName(), "Buffer was not created with BIND_UNORDERED_ACCESS flag.") + LOG_RESOURCE_BINDING_ERROR("buffer", pBuffer, SpirvAttribs.GetPrintName(ArrayInd), ParentResLayout.GetShaderName(), "The buffer was not created with ", GetBindFlagString(RequiredBindFlag), " flags.") DstRes.pObject.Release(); return; } @@ -846,7 +850,8 @@ void ShaderResourceLayoutVk::VkResource::BindResource(IDeviceObject* pObj, Uint3 CacheUniformBuffer(pObj, DstRes, vkDescrSet, ArrayIndex); break; - case SPIRVShaderResourceAttribs::ResourceType::StorageBuffer: + case SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer: + case SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer: CacheStorageBuffer(pObj, DstRes, vkDescrSet, ArrayIndex); break; @@ -1088,7 +1093,8 @@ void ShaderResourceLayoutVk::CommitDynamicResources(const ShaderResourceCacheVk& } break; - case SPIRVShaderResourceAttribs::ResourceType::StorageBuffer: + case SPIRVShaderResourceAttribs::ResourceType::ROStorageBuffer: + case SPIRVShaderResourceAttribs::ResourceType::RWStorageBuffer: WriteDescrSetIt->pBufferInfo = &(*DescrBuffIt); while (ArrElem < Res.SpirvAttribs.ArraySize && DescrBuffIt != DescrBuffInfoArr.end()) { -- cgit v1.2.3