From dfd8eef07fcea79e0f99b7f05c1f03db6c81316d Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Sun, 9 Dec 2018 15:54:00 -0800 Subject: Fixed missing UAV barriers in Vulkan backend --- .../src/DeviceContextVkImpl.cpp | 6 +++- .../src/ShaderResourceCacheVk.cpp | 39 ++++++++++++++++------ 2 files changed, 33 insertions(+), 12 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp index 8ec11f0b..856d20ca 100644 --- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp @@ -1796,6 +1796,8 @@ namespace Diligent pSubresRange->aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; } + // Note that when both old and new states are RESOURCE_STATE_UNORDERED_ACCESS, we need to execute UAV barrier + // to make sure that all UAV writes are complete and visible. auto OldLayout = ResourceStateToVkImageLayout(OldState); auto NewLayout = ResourceStateToVkImageLayout(NewState); m_CommandBuffer.TransitionImageLayout(vkImg, OldLayout, NewLayout, *pSubresRange); @@ -1880,7 +1882,9 @@ namespace Diligent } } - if ((OldState & NewState) != NewState) + // When both old and new states are RESOURCE_STATE_UNORDERED_ACCESS, we need to execute UAV barrier + // to make sure that all UAV writes are complete and visible. + if (((OldState & NewState) != NewState) || NewState == RESOURCE_STATE_UNORDERED_ACCESS) { DEV_CHECK_ERR(BufferVk.m_VulkanBuffer != VK_NULL_HANDLE, "Cannot transition suballocated buffer"); VERIFY_EXPR(BufferVk.GetDynamicOffset(m_ContextId, this) == 0); diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp index eeee5058..925b5274 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp @@ -115,9 +115,10 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) { constexpr RESOURCE_STATE RequiredState = RESOURCE_STATE_CONSTANT_BUFFER; VERIFY_EXPR((ResourceStateFlagsToVkAccessFlags(RequiredState) & VK_ACCESS_UNIFORM_READ_BIT) == VK_ACCESS_UNIFORM_READ_BIT); - if (!pBufferVk->CheckState(RequiredState)) + const bool IsInRequiredState = pBufferVk->CheckState(RequiredState); + if (VerifyOnly) { - if (VerifyOnly) + if (!IsInRequiredState) { LOG_ERROR_MESSAGE("State of buffer '", pBufferVk->GetDesc().Name, "' is incorrect. Required state: ", GetResourceStateString(RequiredState), ". Actual state: ", @@ -126,11 +127,14 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) "when calling IDeviceContext::CommitShaderResources() or explicitly transition the buffer state " "with IDeviceContext::TransitionResourceStates()."); } - else + } + else + { + if (!IsInRequiredState) { pCtxVkImpl->TransitionBufferState(*pBufferVk, RESOURCE_STATE_UNKNOWN, RequiredState, true); - VERIFY_EXPR(pBufferVk->CheckAccessFlags(VK_ACCESS_UNIFORM_READ_BIT)); } + VERIFY_EXPR(pBufferVk->CheckAccessFlags(VK_ACCESS_UNIFORM_READ_BIT)); } } } @@ -151,9 +155,11 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) VK_ACCESS_SHADER_READ_BIT : (VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT); VERIFY_EXPR( (ResourceStateFlagsToVkAccessFlags(RequiredState) & RequiredAccessFlags) == RequiredAccessFlags); #endif - if (!pBufferVk->CheckState(RequiredState)) + const bool IsInRequiredState = pBufferVk->CheckState(RequiredState); + + if (VerifyOnly) { - if (VerifyOnly) + if (!IsInRequiredState) { LOG_ERROR_MESSAGE("State of buffer '", pBufferVk->GetDesc().Name, "' is incorrect. Required state: ", GetResourceStateString(RequiredState), ". Actual state: ", @@ -162,11 +168,16 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) "when calling IDeviceContext::CommitShaderResources() or explicitly transition the buffer state " "with IDeviceContext::TransitionResourceStates()."); } - else + } + else + { + // When both old and new states are RESOURCE_STATE_UNORDERED_ACCESS, we need to execute UAV barrier + // to make sure that all UAV writes are complete and visible. + if (!IsInRequiredState || RequiredState == RESOURCE_STATE_UNORDERED_ACCESS) { pCtxVkImpl->TransitionBufferState(*pBufferVk, RESOURCE_STATE_UNKNOWN, RequiredState, true); - VERIFY_EXPR(pBufferVk->CheckAccessFlags(RequiredAccessFlags)); } + VERIFY_EXPR(pBufferVk->CheckAccessFlags(RequiredAccessFlags)); } } } @@ -208,10 +219,11 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) VERIFY_EXPR(ResourceStateToVkImageLayout(RequiredState) == VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); } } + const bool IsInRequiredState = pTextureVk->CheckState(RequiredState); - if (!pTextureVk->CheckState(RequiredState)) + if (VerifyOnly) { - if (VerifyOnly) + if (!IsInRequiredState) { LOG_ERROR_MESSAGE("State of texture '", pTextureVk->GetDesc().Name, "' is incorrect. Required state: ", GetResourceStateString(RequiredState), ". Actual state: ", @@ -220,7 +232,12 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl* pCtxVkImpl) "when calling IDeviceContext::CommitShaderResources() or explicitly transition the texture state " "with IDeviceContext::TransitionResourceStates()."); } - else + } + else + { + // When both old and new states are RESOURCE_STATE_UNORDERED_ACCESS, we need to execute UAV barrier + // to make sure that all UAV writes are complete and visible. + if (!IsInRequiredState || RequiredState == RESOURCE_STATE_UNORDERED_ACCESS) { pCtxVkImpl->TransitionTextureState(*pTextureVk, RESOURCE_STATE_UNKNOWN, RequiredState, true); } -- cgit v1.2.3