Vulkan: Clean up ImageHelper barrier functions.

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -4364,12 +4364,10 @@ angle::Result ContextVk::onImageRead(VkImageAspectFlags aspectFlags,
     // than a transfer read. So we cannot support simultaneous read usage as easily as for Buffers.
     ANGLE_TRY(endRenderPassIfImageUsed(*image));
 
-    if (image->isLayoutChangeNecessary(imageLayout))
-    {
-        image->changeLayout(aspectFlags, imageLayout,
+    image->recordReadBarrier(aspectFlags, imageLayout,
                              &mOutsideRenderPassCommands->getCommandBuffer());
-    }
     image->retain(&mResourceUseList);
+
     return angle::Result::Continue;
 }
 
@@ -4380,12 +4378,10 @@ angle::Result ContextVk::onImageWrite(VkImageAspectFlags aspectFlags,
     ASSERT(!image->isReleasedToExternal());
     ASSERT(image->getImageSerial().valid());
 
-    // Barriers are always required for image writes.
-    ASSERT(image->isLayoutChangeNecessary(imageLayout));
-
     ANGLE_TRY(endRenderPassIfImageUsed(*image));
 
-    image->changeLayout(aspectFlags, imageLayout, &mOutsideRenderPassCommands->getCommandBuffer());
+    image->recordWriteBarrier(aspectFlags, imageLayout,
+                              &mOutsideRenderPassCommands->getCommandBuffer());
     image->retain(&mResourceUseList);
     image->onWrite();
 

src/libANGLE/renderer/vulkan/SurfaceVk.cpp

@@ -1195,7 +1195,8 @@ angle::Result WindowSurfaceVk::present(ContextVk *contextVk,
     }
 
     // This does nothing if it already in the requested layout
-    image.image.changeLayout(VK_IMAGE_ASPECT_COLOR_BIT, vk::ImageLayout::Present, commandBuffer);
+    image.image.recordReadBarrier(VK_IMAGE_ASPECT_COLOR_BIT, vk::ImageLayout::Present,
+                                  commandBuffer);
 
     // Knowing that the kSwapHistorySize'th submission ago has finished, we can know that the
     // (kSwapHistorySize+1)'th present ago of this image is definitely finished and so its wait

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -65,6 +65,12 @@ constexpr angle::PackedEnumMap<PipelineStage, VkPipelineStageFlagBits> kPipeline
 
 constexpr size_t kDefaultPoolAllocatorPageSize = 16 * 1024;
 
+enum BarrierType
+{
+    ReadOnly,
+    Write
+};
+
 struct ImageMemoryBarrierData
 {
     // The Vk layout corresponding to the ImageLayout key.
@@ -80,13 +86,8 @@ struct ImageMemoryBarrierData
     // Access mask when transitioning out from this layout.  Note that source access mask never
     // needs a READ bit, as WAR hazards don't need memory barriers (just execution barriers).
     VkAccessFlags srcAccessMask;
-
-    // If access is read-only, the memory barrier can be skipped altogether if retransitioning to
-    // the same layout.  This is because read-after-read does not need an execution or memory
-    // barrier.
-    //
-    // Otherwise, some same-layout transitions require a memory barrier.
-    bool sameLayoutTransitionRequiresBarrier;
+    // Read or write.
+    BarrierType type;
     // CommandBufferHelper tracks an array of PipelineBarriers. This indicates which array element
     // this should be merged into. Right now we track individual barrier for every PipelineStage. If
     // layout has a single stage mask bit, we use that stage as index. If layout has multiple stage
@@ -110,7 +111,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             0,
             // Transition from: there's no data in the image to care about.
             0,
-            false,
+            BarrierType::ReadOnly,
             PipelineStage::InvalidEnum,
         },
     },
@@ -124,7 +125,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             0,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_MEMORY_WRITE_BIT,
-            false,
+            BarrierType::ReadOnly,
             PipelineStage::InvalidEnum,
         },
     },
@@ -138,7 +139,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT,
             // Transition from: RAR and WAR don't need memory barrier.
             0,
-            false,
+            BarrierType::ReadOnly,
             // In case of multiple destination stages, We barrier the earliest stage
             PipelineStage::TopOfPipe,
         },
@@ -153,7 +154,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_SHADER_WRITE_BIT,
-            true,
+            BarrierType::Write,
             // In case of multiple destination stages, We barrier the earliest stage
             PipelineStage::TopOfPipe,
         },
@@ -168,7 +169,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_TRANSFER_READ_BIT,
             // Transition from: RAR and WAR don't need memory barrier.
             0,
-            false,
+            BarrierType::ReadOnly,
             PipelineStage::Transfer,
         },
     },
@@ -182,7 +183,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_TRANSFER_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_TRANSFER_WRITE_BIT,
-            true,
+            BarrierType::Write,
             PipelineStage::Transfer,
         },
     },
@@ -196,7 +197,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT,
             // Transition from: RAR and WAR don't need memory barrier.
             0,
-            false,
+            BarrierType::ReadOnly,
             PipelineStage::VertexShader,
         },
     },
@@ -210,7 +211,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_SHADER_WRITE_BIT,
-            true,
+            BarrierType::Write,
             PipelineStage::VertexShader,
         },
     },
@@ -224,7 +225,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT,
             // Transition from: RAR and WAR don't need memory barrier.
             0,
-            false,
+            BarrierType::ReadOnly,
             PipelineStage::GeometryShader,
         },
     },
@@ -238,7 +239,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_SHADER_WRITE_BIT,
-            true,
+            BarrierType::Write,
             PipelineStage::GeometryShader,
         },
     },
@@ -252,7 +253,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT,
             // Transition from: RAR and WAR don't need memory barrier.
             0,
-            false,
+            BarrierType::ReadOnly,
             PipelineStage::FragmentShader,
         },
     },
@@ -266,7 +267,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_SHADER_WRITE_BIT,
-            true,
+            BarrierType::Write,
             PipelineStage::FragmentShader,
         },
     },
@@ -280,7 +281,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT,
             // Transition from: RAR and WAR don't need memory barrier.
             0,
-            false,
+            BarrierType::ReadOnly,
             PipelineStage::ComputeShader,
         },
     },
@@ -294,7 +295,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_SHADER_WRITE_BIT,
-            true,
+            BarrierType::Write,
             PipelineStage::ComputeShader,
         },
     },
@@ -308,7 +309,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT,
             // Transition from: RAR and WAR don't need memory barrier.
             0,
-            false,
+            BarrierType::ReadOnly,
             // In case of multiple destination stages, We barrier the earliest stage
             PipelineStage::VertexShader,
         },
@@ -323,7 +324,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_SHADER_WRITE_BIT,
-            true,
+            BarrierType::Write,
             // In case of multiple destination stages, We barrier the earliest stage
             PipelineStage::VertexShader,
         },
@@ -338,7 +339,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
-            true,
+            BarrierType::Write,
             PipelineStage::ColorAttachmentOutput,
         },
     },
@@ -352,7 +353,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT,
             // Transition from: all writes must finish before barrier.
             VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT,
-            true,
+            BarrierType::Write,
             PipelineStage::EarlyFragmentTest,
         },
     },
@@ -371,7 +372,7 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
             0,
             // Transition from: RAR and WAR don't need memory barrier.
             0,
-            false,
+            BarrierType::ReadOnly,
             PipelineStage::BottomOfPipe,
         },
     },
@@ -594,7 +595,7 @@ void CommandBufferHelper::bufferRead(ResourceUseList *resourceUseList,
 {
     buffer->retain(resourceUseList);
     VkPipelineStageFlagBits stageBits = kPipelineStageFlagBitMap[readStage];
-    if (buffer->updateReadBarrier(readAccessType, stageBits, &mPipelineBarriers[readStage]))
+    if (buffer->recordReadBarrier(readAccessType, stageBits, &mPipelineBarriers[readStage]))
     {
         mPipelineBarrierMask.set(readStage);
     }
@@ -614,7 +615,7 @@ void CommandBufferHelper::bufferWrite(ResourceUseList *resourceUseList,
 {
     buffer->retain(resourceUseList);
     VkPipelineStageFlagBits stageBits = kPipelineStageFlagBitMap[writeStage];
-    if (buffer->updateWriteBarrier(writeAccessType, stageBits, &mPipelineBarriers[writeStage]))
+    if (buffer->recordWriteBarrier(writeAccessType, stageBits, &mPipelineBarriers[writeStage]))
     {
         mPipelineBarrierMask.set(writeStage);
     }
@@ -636,7 +637,8 @@ void CommandBufferHelper::imageRead(ResourceUseList *resourceUseList,
                                     ImageHelper *image)
 {
     image->retain(resourceUseList);
-    if (image->isLayoutChangeNecessary(imageLayout))
+
+    if (image->isReadBarrierNecessary(imageLayout))
     {
         PipelineStage barrierIndex = kImageMemoryBarrierData[imageLayout].barrierIndex;
         ASSERT(barrierIndex != PipelineStage::InvalidEnum);
@@ -2599,7 +2601,7 @@ bool BufferHelper::isReleasedToExternal() const
 #endif
 }
 
-bool BufferHelper::updateReadBarrier(VkAccessFlags readAccessType,
+bool BufferHelper::recordReadBarrier(VkAccessFlags readAccessType,
                                      VkPipelineStageFlags readStage,
                                      PipelineBarrier *barrier)
 {
@@ -2620,7 +2622,7 @@ bool BufferHelper::updateReadBarrier(VkAccessFlags readAccessType,
     return barrierModified;
 }
 
-bool BufferHelper::updateWriteBarrier(VkAccessFlags writeAccessType,
+bool BufferHelper::recordWriteBarrier(VkAccessFlags writeAccessType,
                                       VkPipelineStageFlags writeStage,
                                       PipelineBarrier *barrier)
 {
@@ -2827,7 +2829,7 @@ angle::Result ImageHelper::initializeNonZeroMemory(Context *context, VkDeviceSiz
     ANGLE_TRY(renderer->getCommandBufferOneOff(context, &commandBuffer));
 
     // Queue a DMA copy.
-    forceChangeLayoutAndQueue(getAspectFlags(), ImageLayout::TransferDst, mCurrentQueueFamilyIndex,
+    barrierImpl(getAspectFlags(), ImageLayout::TransferDst, mCurrentQueueFamilyIndex,
                 &commandBuffer);
 
     StagingBuffer stagingBuffer;
@@ -3187,17 +3189,17 @@ gl::Extents ImageHelper::getLevelExtents2D(uint32_t levelVK) const
     return extents;
 }
 
-bool ImageHelper::isLayoutChangeNecessary(ImageLayout newLayout) const
+bool ImageHelper::isReadBarrierNecessary(ImageLayout newLayout) const
 {
-    const ImageMemoryBarrierData &layoutData = kImageMemoryBarrierData[mCurrentLayout];
-
-    // If transitioning to the same layout, we don't need a barrier if the layout is read-only as
-    // RAR (read-after-read) doesn't need a barrier.  WAW (write-after-write) does require a memory
-    // barrier though.
-    bool sameLayoutAndNoNeedForBarrier =
-        mCurrentLayout == newLayout && !layoutData.sameLayoutTransitionRequiresBarrier;
+    // If transitioning to a different layout, we need always need a barrier.
+    if (mCurrentLayout != newLayout)
+    {
+        return true;
+    }
 
-    return !sameLayoutAndNoNeedForBarrier;
+    // RAW (read-after-write) hazards always requires a memory barrier.
+    const ImageMemoryBarrierData &layoutData = kImageMemoryBarrierData[mCurrentLayout];
+    return layoutData.type == BarrierType::Write;
 }
 
 void ImageHelper::changeLayoutAndQueue(VkImageAspectFlags aspectMask,
@@ -3206,7 +3208,7 @@ void ImageHelper::changeLayoutAndQueue(VkImageAspectFlags aspectMask,
                                        CommandBuffer *commandBuffer)
 {
     ASSERT(isQueueChangeNeccesary(newQueueFamilyIndex));
-    forceChangeLayoutAndQueue(aspectMask, newLayout, newQueueFamilyIndex, commandBuffer);
+    barrierImpl(aspectMask, newLayout, newQueueFamilyIndex, commandBuffer);
 }
 
 void ImageHelper::acquireFromExternal(ContextVk *contextVk,
@@ -3282,7 +3284,7 @@ ANGLE_INLINE void ImageHelper::initImageMemoryBarrierStruct(
 
 // Generalized to accept both "primary" and "secondary" command buffers.
 template <typename CommandBufferT>
-void ImageHelper::forceChangeLayoutAndQueue(VkImageAspectFlags aspectMask,
+void ImageHelper::barrierImpl(VkImageAspectFlags aspectMask,
                               ImageLayout newLayout,
                               uint32_t newQueueFamilyIndex,
                               CommandBufferT *commandBuffer)
@@ -3315,7 +3317,7 @@ bool ImageHelper::updateLayoutAndBarrier(VkImageAspectFlags aspectMask,
     if (newLayout == mCurrentLayout)
     {
         const ImageMemoryBarrierData &layoutData = kImageMemoryBarrierData[mCurrentLayout];
-        ASSERT(layoutData.sameLayoutTransitionRequiresBarrier);
+        ASSERT(layoutData.type == BarrierType::Write);
         // No layout change, only memory barrier is required
         barrier->mergeMemoryBarrier(layoutData.srcStageMask, layoutData.dstStageMask,
                                     layoutData.srcAccessMask, layoutData.dstAccessMask);
@@ -4378,7 +4380,7 @@ angle::Result ImageHelper::flushStagedUpdates(ContextVk *contextVk,
         if (updateLayerCount >= kMaxParallelSubresourceUpload)
         {
             // If there are more subresources than bits we can track, always insert a barrier.
-            changeLayout(aspectFlags, ImageLayout::TransferDst, commandBuffer);
+            recordWriteBarrier(aspectFlags, ImageLayout::TransferDst, commandBuffer);
             subresourceUploadsInProgress = std::numeric_limits<uint64_t>::max();
         }
         else
@@ -4392,7 +4394,7 @@ angle::Result ImageHelper::flushStagedUpdates(ContextVk *contextVk,
             if ((subresourceUploadsInProgress & subresourceHash) != 0)
             {
                 // If there's overlap in subresource upload, issue a barrier.
-                changeLayout(aspectFlags, ImageLayout::TransferDst, commandBuffer);
+                recordWriteBarrier(aspectFlags, ImageLayout::TransferDst, commandBuffer);
                 subresourceUploadsInProgress = 0;
             }
             subresourceUploadsInProgress |= subresourceHash;

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -798,11 +798,11 @@ class BufferHelper final : public Resource
     // Returns true if the image is owned by an external API or instance.
     bool isReleasedToExternal() const;
 
-    bool updateReadBarrier(VkAccessFlags readAccessType,
+    bool recordReadBarrier(VkAccessFlags readAccessType,
                            VkPipelineStageFlags readStage,
                            PipelineBarrier *barrier);
 
-    bool updateWriteBarrier(VkAccessFlags writeAccessType,
+    bool recordWriteBarrier(VkAccessFlags writeAccessType,
                             VkPipelineStageFlags writeStage,
                             PipelineBarrier *barrier);
 
@@ -1353,22 +1353,26 @@ class ImageHelper final : public Resource, public angle::Subject
     bool isUpdateStaged(uint32_t levelGL, uint32_t layer);
     bool hasStagedUpdates() const { return !mSubresourceUpdates.empty(); }
 
-    // changeLayout automatically skips the layout change if it's unnecessary.  This function can be
-    // used to prevent creating a command graph node and subsequently a command buffer for the sole
-    // purpose of performing a transition (which may then not be issued).
-    bool isLayoutChangeNecessary(ImageLayout newLayout) const;
+    void recordWriteBarrier(VkImageAspectFlags aspectMask,
+                            ImageLayout newLayout,
+                            CommandBuffer *commandBuffer)
+    {
+        barrierImpl(aspectMask, newLayout, mCurrentQueueFamilyIndex, commandBuffer);
+    }
 
-    template <typename CommandBufferT>
-    void changeLayout(VkImageAspectFlags aspectMask,
+    // This function can be used to prevent issuing redundant layout transition commands.
+    bool isReadBarrierNecessary(ImageLayout newLayout) const;
+
+    void recordReadBarrier(VkImageAspectFlags aspectMask,
                            ImageLayout newLayout,
-                      CommandBufferT *commandBuffer)
+                           CommandBuffer *commandBuffer)
     {
-        if (!isLayoutChangeNecessary(newLayout))
+        if (!isReadBarrierNecessary(newLayout))
         {
             return;
         }
 
-        forceChangeLayoutAndQueue(aspectMask, newLayout, mCurrentQueueFamilyIndex, commandBuffer);
+        barrierImpl(aspectMask, newLayout, mCurrentQueueFamilyIndex, commandBuffer);
     }
 
     bool isQueueChangeNeccesary(uint32_t newQueueFamilyIndex) const
@@ -1531,7 +1535,7 @@ class ImageHelper final : public Resource, public angle::Subject
 
     // Generalized to accept both "primary" and "secondary" command buffers.
     template <typename CommandBufferT>
-    void forceChangeLayoutAndQueue(VkImageAspectFlags aspectMask,
+    void barrierImpl(VkImageAspectFlags aspectMask,
                      ImageLayout newLayout,
                      uint32_t newQueueFamilyIndex,
                      CommandBufferT *commandBuffer);