Vulkan: Cleanup buffer dependencies

src/libANGLE/renderer/vulkan/BufferVk.cpp

@@ -64,7 +64,7 @@ BufferVk::VertexConversionBuffer::VertexConversionBuffer(VertexConversionBuffer
 BufferVk::VertexConversionBuffer::~VertexConversionBuffer() = default;
 
 // BufferVk implementation.
-BufferVk::BufferVk(const gl::BufferState &state) : BufferImpl(state), mDataWriteAccessFlags(0) {}
+BufferVk::BufferVk(const gl::BufferState &state) : BufferImpl(state) {}
 
 BufferVk::~BufferVk() {}
 
@@ -169,8 +169,8 @@ angle::Result BufferVk::copySubData(const gl::Context *context,
     {
         ANGLE_TRY(mBuffer.recordCommands(contextVk, &commandBuffer));
 
-        sourceBuffer->mBuffer.onRead(&mBuffer, VK_ACCESS_TRANSFER_READ_BIT);
-        mBuffer.onWrite(contextVk, VK_ACCESS_TRANSFER_WRITE_BIT);
+        sourceBuffer->mBuffer.onReadByBuffer(contextVk, &mBuffer, VK_ACCESS_TRANSFER_READ_BIT,
+                                             VK_ACCESS_TRANSFER_WRITE_BIT);
     }
 
     // Enqueue a copy command on the GPU.
@@ -239,33 +239,13 @@ angle::Result BufferVk::unmapImpl(ContextVk *contextVk)
     ASSERT(mBuffer.valid());
 
     mBuffer.getDeviceMemory().unmap(contextVk->getDevice());
-    mDataWriteAccessFlags = VK_ACCESS_HOST_WRITE_BIT;
+    mBuffer.onExternalWrite(VK_ACCESS_HOST_WRITE_BIT);
 
     markConversionBuffersDirty();
 
     return angle::Result::Continue;
 }
 
-angle::Result BufferVk::onRead(ContextVk *contextVk,
-                               vk::CommandGraphResource *reader,
-                               VkAccessFlagBits readAccessType)
-{
-    // Now that the buffer helper is being used (and will be part of the command graph), make sure
-    // its data write barrier is executed.
-    if (mDataWriteAccessFlags != 0)
-    {
-        vk::CommandBuffer *commandBuffer;
-        ANGLE_TRY(mBuffer.recordCommands(contextVk, &commandBuffer));
-
-        mBuffer.onWrite(contextVk, mDataWriteAccessFlags);
-        mDataWriteAccessFlags = 0;
-    }
-
-    mBuffer.onRead(reader, readAccessType);
-
-    return angle::Result::Continue;
-}
-
 angle::Result BufferVk::getIndexRange(const gl::Context *context,
                                       gl::DrawElementsType type,
                                       size_t offset,
@@ -344,7 +324,7 @@ angle::Result BufferVk::setDataImpl(ContextVk *contextVk,
         memcpy(mapPointer, data, size);
 
         mBuffer.getDeviceMemory().unmap(device);
-        mDataWriteAccessFlags = VK_ACCESS_HOST_WRITE_BIT;
+        mBuffer.onExternalWrite(VK_ACCESS_HOST_WRITE_BIT);
     }
 
     // Update conversions
@@ -362,8 +342,8 @@ angle::Result BufferVk::copyToBuffer(ContextVk *contextVk,
     ANGLE_TRY(destBuffer->recordCommands(contextVk, &commandBuffer));
     commandBuffer->copyBuffer(mBuffer.getBuffer(), destBuffer->getBuffer(), copyCount, copies);
 
-    mBuffer.onRead(destBuffer, VK_ACCESS_TRANSFER_READ_BIT);
-    destBuffer->onWrite(contextVk, VK_ACCESS_TRANSFER_WRITE_BIT);
+    mBuffer.onReadByBuffer(contextVk, destBuffer, VK_ACCESS_TRANSFER_READ_BIT,
+                           VK_ACCESS_TRANSFER_WRITE_BIT);
 
     return angle::Result::Continue;
 }

src/libANGLE/renderer/vulkan/BufferVk.h

@@ -99,10 +99,6 @@ class BufferVk : public BufferImpl
                                void **mapPtr);
     angle::Result unmapImpl(ContextVk *contextVk);
 
-    angle::Result onRead(ContextVk *contextVk,
-                         vk::CommandGraphResource *reader,
-                         VkAccessFlagBits readAccessType);
-
     // Calls copyBuffer internally.
     angle::Result copyToBuffer(ContextVk *contextVk,
                                vk::BufferHelper *destBuffer,
@@ -139,7 +135,6 @@ class BufferVk : public BufferImpl
     };
 
     vk::BufferHelper mBuffer;
-    VkAccessFlags mDataWriteAccessFlags;
 
     // A cache of converted vertex data.
     std::vector<VertexConversionBuffer> mVertexConversionBuffers;

src/libANGLE/renderer/vulkan/ProgramVk.cpp

@@ -981,7 +981,7 @@ angle::Result ProgramVk::updateUniformBuffersDescriptorSet(ContextVk *contextVk,
         VkDeviceSize blockSize         = uniformBlock.dataSize;
         vk::BufferHelper &bufferHelper = bufferVk->getBuffer();
 
-        ANGLE_TRY(bufferVk->onRead(contextVk, framebuffer, VK_ACCESS_UNIFORM_READ_BIT));
+        bufferHelper.onRead(framebuffer, VK_ACCESS_UNIFORM_READ_BIT);
 
         // If size is 0, we can't always use VK_WHOLE_SIZE (or bufferHelper.getSize()), as the
         // backing buffer may be larger than maxUniformBufferRange.  In that case, we use the

src/libANGLE/renderer/vulkan/TransformFeedbackVk.cpp

@@ -129,8 +129,7 @@ void TransformFeedbackVk::addFramebufferDependency(ContextVk *contextVk,
         ASSERT(buffer != nullptr);
 
         vk::BufferHelper &bufferHelper = vk::GetImpl(buffer)->getBuffer();
-        bufferHelper.addWriteDependency(framebuffer);
-        bufferHelper.onWrite(contextVk, VK_ACCESS_SHADER_WRITE_BIT);
+        bufferHelper.onWrite(contextVk, framebuffer, 0, VK_ACCESS_SHADER_WRITE_BIT);
     }
 }
 

src/libANGLE/renderer/vulkan/UtilsVk.cpp

@@ -586,7 +586,7 @@ angle::Result UtilsVk::clearBuffer(ContextVk *contextVk,
     ANGLE_TRY(dest->recordCommands(contextVk, &commandBuffer));
 
     // Tell dest it's being written to.
-    dest->onWrite(contextVk, VK_ACCESS_SHADER_WRITE_BIT);
+    dest->onSelfReadWrite(contextVk, 0, VK_ACCESS_SHADER_WRITE_BIT);
 
     const vk::Format &destFormat = dest->getViewFormat();
 
@@ -637,10 +637,8 @@ angle::Result UtilsVk::convertIndexBuffer(ContextVk *contextVk,
     vk::CommandBuffer *commandBuffer;
     ANGLE_TRY(dest->recordCommands(contextVk, &commandBuffer));
 
-    // Tell src we are going to read from it.
-    src->onRead(dest, VK_ACCESS_SHADER_READ_BIT);
-    // Tell dest it's being written to.
-    dest->onWrite(contextVk, VK_ACCESS_SHADER_WRITE_BIT);
+    // Tell src we are going to read from it and dest it's being written to.
+    src->onReadByBuffer(contextVk, dest, VK_ACCESS_SHADER_READ_BIT, VK_ACCESS_SHADER_WRITE_BIT);
 
     VkDescriptorSet descriptorSet;
     vk::RefCountedDescriptorPoolBinding descriptorPoolBinding;
@@ -700,10 +698,8 @@ angle::Result UtilsVk::convertVertexBuffer(ContextVk *contextVk,
     vk::CommandBuffer *commandBuffer;
     ANGLE_TRY(dest->recordCommands(contextVk, &commandBuffer));
 
-    // Tell src we are going to read from it.
-    src->onRead(dest, VK_ACCESS_SHADER_READ_BIT);
-    // Tell dest it's being written to.
-    dest->onWrite(contextVk, VK_ACCESS_SHADER_WRITE_BIT);
+    // Tell src we are going to read from it and dest it's being written to.
+    src->onReadByBuffer(contextVk, dest, VK_ACCESS_SHADER_READ_BIT, VK_ACCESS_SHADER_WRITE_BIT);
 
     ConvertVertexShaderParams shaderParams;
     shaderParams.Ns = params.srcFormat->channelCount;

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -1296,36 +1296,39 @@ void BufferHelper::release(DisplayVk *display, std::vector<GarbageObjectBase> *g
     mDeviceMemory.dumpResources(garbageQueue);
 }
 
-void BufferHelper::onWrite(ContextVk *contextVk, VkAccessFlags writeAccessType)
+bool BufferHelper::needsOnWriteBarrier(VkAccessFlags readAccessType,
+                                       VkAccessFlags writeAccessType,
+                                       VkAccessFlags *barrierSrcOut,
+                                       VkAccessFlags *barrierDstOut)
 {
-    if (mCurrentReadAccess != 0 || mCurrentWriteAccess != 0)
-    {
-        addGlobalMemoryBarrier(mCurrentReadAccess | mCurrentWriteAccess, writeAccessType);
-    }
+    bool needsBarrier = mCurrentReadAccess != 0 || mCurrentWriteAccess != 0;
+
+    // Note: mCurrentReadAccess is not part of barrier src flags as "anything-after-read" is
+    // satisified by execution barriers alone.
+    *barrierSrcOut = mCurrentWriteAccess;
+    *barrierDstOut = readAccessType | writeAccessType;
 
     mCurrentWriteAccess = writeAccessType;
-    mCurrentReadAccess  = 0;
+    mCurrentReadAccess  = readAccessType;
 
-    bool hostVisible = mMemoryPropertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT;
-    if (hostVisible && writeAccessType != VK_ACCESS_HOST_WRITE_BIT)
-    {
-        contextVk->onHostVisibleBufferWrite();
-    }
+    return needsBarrier;
 }
 
-void BufferHelper::onSelfReadWrite(ContextVk *contextVk,
-                                   VkAccessFlags readAccessType,
-                                   VkAccessFlags writeAccessType)
+void BufferHelper::onWriteAccess(ContextVk *contextVk,
+                                 VkAccessFlags readAccessType,
+                                 VkAccessFlags writeAccessType)
 {
-    if (mCurrentReadAccess || mCurrentWriteAccess)
+    VkAccessFlags barrierSrc, barrierDst;
+    if (needsOnWriteBarrier(readAccessType, writeAccessType, &barrierSrc, &barrierDst))
     {
-        finishCurrentCommands(contextVk);
-        addGlobalMemoryBarrier(mCurrentReadAccess | mCurrentWriteAccess,
-                               readAccessType | writeAccessType);
+        addGlobalMemoryBarrier(barrierSrc, barrierDst);
     }
 
-    mCurrentReadAccess  = readAccessType;
-    mCurrentWriteAccess = writeAccessType;
+    bool hostVisible = mMemoryPropertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT;
+    if (hostVisible && writeAccessType != VK_ACCESS_HOST_WRITE_BIT)
+    {
+        contextVk->onHostVisibleBufferWrite();
+    }
 }
 
 angle::Result BufferHelper::copyFromBuffer(ContextVk *contextVk,

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -451,22 +451,50 @@ class BufferHelper final : public CommandGraphResource
     const DeviceMemory &getDeviceMemory() const { return mDeviceMemory; }
     VkDeviceSize getSize() const { return mSize; }
 
-    // Helpers for setting the graph dependencies *and* setting the appropriate barrier.
-    ANGLE_INLINE void onRead(CommandGraphResource *reader, VkAccessFlags readAccessType)
+    // Helpers for setting the graph dependencies *and* setting the appropriate barrier.  These are
+    // made for dependencies to non-buffer resources, as only one of two resources participating in
+    // the dependency would require a memory barrier.  Note that onWrite takes read access flags
+    // too, as output buffers could be read as well.
+    void onRead(CommandGraphResource *reader, VkAccessFlags readAccessType)
     {
         addReadDependency(reader);
-
-        if (mCurrentWriteAccess != 0 && (mCurrentReadAccess & readAccessType) != readAccessType)
-        {
-            reader->addGlobalMemoryBarrier(mCurrentWriteAccess, readAccessType);
-            mCurrentReadAccess |= readAccessType;
-        }
+        onReadAccess(reader, readAccessType);
     }
-
-    void onWrite(ContextVk *contextVk, VkAccessFlags writeAccessType);
+    void onWrite(ContextVk *contextVk,
+                 CommandGraphResource *writer,
+                 VkAccessFlags readAccessType,
+                 VkAccessFlags writeAccessType)
+    {
+        addWriteDependency(writer);
+        onWriteAccess(contextVk, readAccessType, writeAccessType);
+    }
+    // Helper for setting a graph dependency between two buffers.  This is a specialized function as
+    // both buffers may incur a memory barrier.  Using |onRead| followed by |onWrite| between the
+    // buffers is impossible as it would result in a command graph loop.
+    void onReadByBuffer(ContextVk *contextVk,
+                        BufferHelper *reader,
+                        VkAccessFlags readAccessType,
+                        VkAccessFlags writeAccessType)
+    {
+        addReadDependency(reader);
+        onReadAccess(reader, readAccessType);
+        reader->onWriteAccess(contextVk, 0, writeAccessType);
+    }
+    // Helper for setting a barrier when different parts of the same buffer is being read from and
+    // written to.
     void onSelfReadWrite(ContextVk *contextVk,
                          VkAccessFlags readAccessType,
-                         VkAccessFlags writeAccessType);
+                         VkAccessFlags writeAccessType)
+    {
+        if (mCurrentReadAccess || mCurrentWriteAccess)
+        {
+            finishCurrentCommands(contextVk);
+        }
+        onWriteAccess(contextVk, readAccessType, writeAccessType);
+    }
+    // Set write access mask when the buffer is modified externally, e.g. by host.  There is no
+    // graph resource to create a dependency to.
+    void onExternalWrite(VkAccessFlags writeAccessType) { mCurrentWriteAccess |= writeAccessType; }
 
     // Also implicitly sets up the correct barriers.
     angle::Result copyFromBuffer(ContextVk *contextVk,
@@ -509,6 +537,34 @@ class BufferHelper final : public CommandGraphResource
 
   private:
     angle::Result mapImpl(ContextVk *contextVk);
+    bool needsOnReadBarrier(VkAccessFlags readAccessType,
+                            VkAccessFlags *barrierSrcOut,
+                            VkAccessFlags *barrierDstOut)
+    {
+        bool needsBarrier =
+            mCurrentWriteAccess != 0 && (mCurrentReadAccess & readAccessType) != readAccessType;
+
+        *barrierSrcOut = mCurrentWriteAccess;
+        *barrierDstOut = readAccessType;
+
+        mCurrentReadAccess |= readAccessType;
+        return needsBarrier;
+    }
+    void onReadAccess(CommandGraphResource *reader, VkAccessFlags readAccessType)
+    {
+        VkAccessFlags barrierSrc, barrierDst;
+        if (needsOnReadBarrier(readAccessType, &barrierSrc, &barrierDst))
+        {
+            reader->addGlobalMemoryBarrier(barrierSrc, barrierDst);
+        }
+    }
+    bool needsOnWriteBarrier(VkAccessFlags readAccessType,
+                             VkAccessFlags writeAccessType,
+                             VkAccessFlags *barrierSrcOut,
+                             VkAccessFlags *barrierDstOut);
+    void onWriteAccess(ContextVk *contextVk,
+                       VkAccessFlags readAccessType,
+                       VkAccessFlags writeAccessType);
 
     // Vulkan objects.
     Buffer mBuffer;