From 5ffca2eb901bde64ac8fab6309e053083505443a Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Wed, 6 May 2020 07:12:48 +0000 Subject: [PATCH] NativeSwapChainImplVk: Remove unnecessary transition on Configure Configure was transitioning the swapchain images from undefined to present layout but this was already happening because TextureVk starts with a mLastUsage of None that will force a transition from undefined when used. Also introduce an internal texture usage bit kPresentTextureUsage to prepare for the eventual remove of wgpu::TextureUsage::Present when old swapchains are removed. Bug: dawn:269 Change-Id: I57d26f18e34cacd5d91419a45787b2ece9558846 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/20881 Reviewed-by: Stephen White Reviewed-by: Austin Eng Commit-Queue: Corentin Wallez --- src/dawn_native/Texture.h | 3 +- src/dawn_native/d3d12/TextureD3D12.cpp | 6 ++- src/dawn_native/dawn_platform.h | 6 +++ .../vulkan/NativeSwapChainImplVk.cpp | 25 ---------- src/dawn_native/vulkan/TextureVk.cpp | 47 +++++++++++++------ 5 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h index 62eafc4965..d14696cd7a 100644 --- a/src/dawn_native/Texture.h +++ b/src/dawn_native/Texture.h @@ -35,8 +35,7 @@ namespace dawn_native { bool IsValidSampleCount(uint32_t sampleCount); static constexpr wgpu::TextureUsage kReadOnlyTextureUsages = - wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::Sampled | wgpu::TextureUsage::Present | - kReadonlyStorageTexture; + wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::Sampled | kReadonlyStorageTexture; static constexpr wgpu::TextureUsage kWritableTextureUsages = wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::Storage | diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp index 56aed13923..865bc2b19b 100644 --- a/src/dawn_native/d3d12/TextureD3D12.cpp +++ b/src/dawn_native/d3d12/TextureD3D12.cpp @@ -35,8 +35,10 @@ namespace dawn_native { namespace d3d12 { D3D12_RESOURCE_STATES D3D12TextureUsage(wgpu::TextureUsage usage, const Format& format) { D3D12_RESOURCE_STATES resourceState = D3D12_RESOURCE_STATE_COMMON; - // Present is an exclusive flag. - if (usage & wgpu::TextureUsage::Present) { + if (usage & kPresentTextureUsage) { + // The present usage is only used internally by the swapchain and is never used in + // combination with other usages. + ASSERT(usage == kPresentTextureUsage); return D3D12_RESOURCE_STATE_PRESENT; } diff --git a/src/dawn_native/dawn_platform.h b/src/dawn_native/dawn_platform.h index 728794581e..19d47e887b 100644 --- a/src/dawn_native/dawn_platform.h +++ b/src/dawn_native/dawn_platform.h @@ -29,6 +29,12 @@ namespace dawn_native { static_cast(0x80000000); static constexpr wgpu::TextureUsage kReadonlyStorageTexture = static_cast(0x80000000); + + // Add an extra texture usage for textures that will be presented, for use in backends + // that needs to transition to present usage. + // TODO(cwallez@chromium.org): It currently aliases wgpu::TextureUsage::Present, assign it + // some bit when wgpu::TextureUsage::Present is removed. + static constexpr wgpu::TextureUsage kPresentTextureUsage = wgpu::TextureUsage::Present; } // namespace dawn_native #endif // DAWNNATIVE_DAWNPLATFORM_H_ diff --git a/src/dawn_native/vulkan/NativeSwapChainImplVk.cpp b/src/dawn_native/vulkan/NativeSwapChainImplVk.cpp index 11bf0899eb..54dd06614a 100644 --- a/src/dawn_native/vulkan/NativeSwapChainImplVk.cpp +++ b/src/dawn_native/vulkan/NativeSwapChainImplVk.cpp @@ -155,31 +155,6 @@ namespace dawn_native { namespace vulkan { ASSERT(false); } - // Do the initial layout transition for all these images from an undefined layout to - // present so that it matches the "present" usage after the first GetNextTexture. - CommandRecordingContext* recordingContext = mDevice->GetPendingRecordingContext(); - for (VkImage image : mSwapChainImages) { - VkImageMemoryBarrier barrier; - barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; - barrier.pNext = nullptr; - barrier.srcAccessMask = 0; - barrier.dstAccessMask = 0; - barrier.oldLayout = VK_IMAGE_LAYOUT_UNDEFINED; - barrier.newLayout = VK_IMAGE_LAYOUT_PRESENT_SRC_KHR; - barrier.srcQueueFamilyIndex = 0; - barrier.dstQueueFamilyIndex = 0; - barrier.image = *image; - barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - barrier.subresourceRange.baseMipLevel = 0; - barrier.subresourceRange.levelCount = 1; - barrier.subresourceRange.baseArrayLayer = 0; - barrier.subresourceRange.layerCount = 1; - - mDevice->fn.CmdPipelineBarrier( - recordingContext->commandBuffer, VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, - VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier); - } - if (oldSwapchain != VK_NULL_HANDLE) { mDevice->GetFencedDeleter()->DeleteWhenUnused(oldSwapchain); } diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp index 7bebbf7f8a..bd036dd875 100644 --- a/src/dawn_native/vulkan/TextureVk.cpp +++ b/src/dawn_native/vulkan/TextureVk.cpp @@ -86,12 +86,22 @@ namespace dawn_native { namespace vulkan { VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; } } - if (usage & wgpu::TextureUsage::Present) { - // There is no access flag for present because the VK_KHR_SWAPCHAIN extension says - // that vkQueuePresentKHR makes the memory of the image visible to the presentation - // engine. There's also a note explicitly saying dstAccessMask should be 0. On the - // other side srcAccessMask can also be 0 because synchronization is required to - // happen with a semaphore instead. + if (usage & kPresentTextureUsage) { + // The present usage is only used internally by the swapchain and is never used in + // combination with other usages. + ASSERT(usage == kPresentTextureUsage); + // The Vulkan spec has the following note: + // + // When transitioning the image to VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR or + // VK_IMAGE_LAYOUT_PRESENT_SRC_KHR, there is no need to delay subsequent + // processing, or perform any visibility operations (as vkQueuePresentKHR performs + // automatic visibility operations). To achieve this, the dstAccessMask member of + // the VkImageMemoryBarrier should be set to 0, and the dstStageMask parameter + // should be set to VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT. + // + // So on the transition to Present we don't need an access flag. The other + // direction doesn't matter because swapchain textures always start a new frame + // as uninitialized. flags |= 0; } @@ -132,7 +142,7 @@ namespace dawn_native { namespace vulkan { } else { return VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; } - case wgpu::TextureUsage::Present: + case kPresentTextureUsage: return VK_IMAGE_LAYOUT_PRESENT_SRC_KHR; default: UNREACHABLE(); @@ -170,13 +180,22 @@ namespace dawn_native { namespace vulkan { flags |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; } } - if (usage & wgpu::TextureUsage::Present) { - // There is no pipeline stage for present but a pipeline stage is required so we use - // "bottom of pipe" to block as little as possible and vkQueuePresentKHR will make - // the memory visible to the presentation engine. The spec explicitly mentions that - // "bottom of pipe" is ok. On the other direction, synchronization happens with a - // semaphore so bottom of pipe is ok too (but maybe it could be "top of pipe" to - // block less?) + if (usage & kPresentTextureUsage) { + // The present usage is only used internally by the swapchain and is never used in + // combination with other usages. + ASSERT(usage == kPresentTextureUsage); + // The Vulkan spec has the following note: + // + // When transitioning the image to VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR or + // VK_IMAGE_LAYOUT_PRESENT_SRC_KHR, there is no need to delay subsequent + // processing, or perform any visibility operations (as vkQueuePresentKHR performs + // automatic visibility operations). To achieve this, the dstAccessMask member of + // the VkImageMemoryBarrier should be set to 0, and the dstStageMask parameter + // should be set to VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT. + // + // So on the transition to Present we use the "bottom of pipe" stage. The other + // direction doesn't matter because swapchain textures always start a new frame + // as uninitialized. flags |= VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT; }