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 <senorblanco@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

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 |
+        wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::Sampled | kReadonlyStorageTexture;
-        kReadonlyStorageTexture;
 
     static constexpr wgpu::TextureUsage kWritableTextureUsages =
         wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::Storage |

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 & kPresentTextureUsage) {
-            if (usage & wgpu::TextureUsage::Present) {
+                // 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;
             }
 

src/dawn_native/dawn_platform.h

@@ -29,6 +29,12 @@ namespace dawn_native {
         static_cast<wgpu::BufferUsage>(0x80000000);
     static constexpr wgpu::TextureUsage kReadonlyStorageTexture =
         static_cast<wgpu::TextureUsage>(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_

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);
         }

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) {
+            if (usage & kPresentTextureUsage) {
-                // There is no access flag for present because the VK_KHR_SWAPCHAIN extension says
+                // The present usage is only used internally by the swapchain and is never used in
-                // that vkQueuePresentKHR makes the memory of the image visible to the presentation
+                // combination with other usages.
-                // engine. There's also a note explicitly saying dstAccessMask should be 0. On the
+                ASSERT(usage == kPresentTextureUsage);
-                // other side srcAccessMask can also be 0 because synchronization is required to
+                // The Vulkan spec has the following note:
-                // happen with a semaphore instead.
+                //
+                //   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) {
+            if (usage & kPresentTextureUsage) {
-                // There is no pipeline stage for present but a pipeline stage is required so we use
+                // The present usage is only used internally by the swapchain and is never used in
-                // "bottom of pipe" to block as little as possible and vkQueuePresentKHR will make
+                // combination with other usages.
-                // the memory visible to the presentation engine. The spec explicitly mentions that
+                ASSERT(usage == kPresentTextureUsage);
-                // "bottom of pipe" is ok. On the other direction, synchronization happens with a
+                // The Vulkan spec has the following note:
-                // semaphore so bottom of pipe is ok too (but maybe it could be "top of pipe" to
+                //
-                // block less?)
+                //   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;
             }