Vulkan: Fix glMemoryBarrier* implementation

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -2633,51 +2633,48 @@ angle::Result ContextVk::dispatchComputeIndirect(const gl::Context *context, GLi
 
 angle::Result ContextVk::memoryBarrier(const gl::Context *context, GLbitfield barriers)
 {
+    memoryBarrierImpl(barriers, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT);
+    return angle::Result::Continue;
+}
+
+angle::Result ContextVk::memoryBarrierByRegion(const gl::Context *context, GLbitfield barriers)
+{
+    // Note: memoryBarrierByRegion is expected to affect only the fragment pipeline, but is
+    // otherwise similar to memoryBarrier.
+
+    memoryBarrierImpl(barriers, VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT);
+    return angle::Result::Continue;
+}
+
+void ContextVk::memoryBarrierImpl(GLbitfield barriers, VkPipelineStageFlags stageMask)
+{
     // Note: most of the barriers specified here don't require us to issue a memory barrier, as
     // the relevant resources already insert the appropriate barriers.  They do however require
     // the resource writing nodes to finish so future buffer barriers are placed correctly, as
     // well as resource dependencies not creating a graph loop.  This is done by inserting a
     // command graph barrier that does nothing!
+    //
+    // The barriers that are necessary all have SHADER_WRITE as src access and the dst access is
+    // determined by the given bitfield.  Currently, all image-related barriers that require the
+    // image to change usage are handled through image layout transitions.  Most buffer-related
+    // barriers where the buffer usage changes are also handled automatically through dirty bits.
+    // The only barriers that are necessary are thus barriers in situations where the resource can
+    // be written to and read from without changing the bindings.
 
     VkAccessFlags srcAccess = 0;
     VkAccessFlags dstAccess = 0;
 
-    if ((barriers & GL_COMMAND_BARRIER_BIT) != 0)
-    {
-        srcAccess |= VK_ACCESS_SHADER_WRITE_BIT;
-        dstAccess |= VK_ACCESS_INDIRECT_COMMAND_READ_BIT;
-    }
-
-    if ((barriers & GL_SHADER_IMAGE_ACCESS_BARRIER_BIT) != 0)
-    {
-        srcAccess |= (VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_SHADER_READ_BIT);
-        dstAccess |= (VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_SHADER_READ_BIT);
-    }
+    // Both IMAGE_ACCESS and STORAGE barrier flags translate to the same Vulkan dst access mask.
+    constexpr GLbitfield kShaderWriteBarriers =
+        GL_SHADER_IMAGE_ACCESS_BARRIER_BIT | GL_SHADER_STORAGE_BARRIER_BIT;
 
-    if ((barriers & GL_FRAMEBUFFER_BARRIER_BIT) != 0)
+    if ((barriers & kShaderWriteBarriers) != 0)
     {
-        srcAccess |=
-            (VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT);
-        dstAccess |=
-            (VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT |
-             VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT);
+        srcAccess |= VK_ACCESS_SHADER_WRITE_BIT;
+        dstAccess |= VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_SHADER_READ_BIT;
     }
 
-    mCommandGraph.memoryBarrier(srcAccess, dstAccess, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT);
-    return angle::Result::Continue;
-}
-
-angle::Result ContextVk::memoryBarrierByRegion(const gl::Context *context, GLbitfield barriers)
-{
-    // There aren't any barrier bits here that aren't otherwise automatically handled.  We only
-    // need to make sure writer resources (framebuffers and the dispatcher) start a new node.
-    //
-    // Note: memoryBarrierByRegion is expected to affect only the fragment pipeline.  Specifying
-    // that here is currently unnecessary, but is a reminder of this fact in case we do need to
-    // especially handle some future barrier bit.
-
-    mCommandGraph.memoryBarrier(0, 0, VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT);
-    return angle::Result::Continue;
+    mCommandGraph.memoryBarrier(srcAccess, dstAccess, stageMask);
 }
 
 vk::DynamicQueryPool *ContextVk::getQueryPool(gl::QueryType queryType)

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -629,6 +629,7 @@ class ContextVk : public ContextImpl, public vk::Context, public vk::RenderPassO
     angle::Result submitFrame(const VkSubmitInfo &submitInfo,
                               vk::PrimaryCommandBuffer &&commandBuffer);
     angle::Result flushCommandGraph(vk::PrimaryCommandBuffer *commandBatch);
+    void memoryBarrierImpl(GLbitfield barriers, VkPipelineStageFlags stageMask);
 
     angle::Result synchronizeCpuGpuTime();
     angle::Result traceGpuEventImpl(vk::PrimaryCommandBuffer *commandBuffer,

src/tests/gl_tests/StateChangeTest.cpp

@@ -3170,9 +3170,6 @@ TEST_P(SimpleStateChangeTestES31, UpdateImageTextureInUse)
 // Test that we can alternate between image textures between different dispatchs.
 TEST_P(SimpleStateChangeTestES31, DispatchImageTextureAThenTextureBThenTextureA)
 {
-    // Fails in the last EXPECT call.  http://anglebug.com/3879
-    ANGLE_SKIP_TEST_IF(IsVulkan() && IsWindows() && IsAMD());
-
     std::array<GLColor, 4> colorsTexA = {
         {GLColor::cyan, GLColor::cyan, GLColor::cyan, GLColor::cyan}};