Vulkan: Correct synchronization for buffer readback

src/libANGLE/renderer/vulkan/BufferVk.cpp

@@ -158,16 +158,7 @@ angle::Result BufferVk::map(const gl::Context *context, GLenum access, void **ma
 {
     ASSERT(mBuffer.valid());
 
-    ContextVk *contextVk = vk::GetImpl(context);
-    return mapImpl(contextVk, mapPtr);
-}
-
-angle::Result BufferVk::mapImpl(ContextVk *contextVk, void **mapPtr)
-{
-    ANGLE_VK_TRY(contextVk,
-                 mBuffer.getDeviceMemory().map(contextVk->getDevice(), 0, mState.getSize(), 0,
-                                               reinterpret_cast<uint8_t **>(mapPtr)));
-    return angle::Result::Continue;
+    return mapImpl(vk::GetImpl(context), mapPtr);
 }
 
 angle::Result BufferVk::mapRange(const gl::Context *context,
@@ -176,9 +167,32 @@ angle::Result BufferVk::mapRange(const gl::Context *context,
                                  GLbitfield access,
                                  void **mapPtr)
 {
+    return mapRangeImpl(vk::GetImpl(context), offset, length, access, mapPtr);
+}
+
+angle::Result BufferVk::mapImpl(ContextVk *contextVk, void **mapPtr)
+{
+    return mapRangeImpl(contextVk, 0, mState.getSize(), 0, mapPtr);
+}
+
+angle::Result BufferVk::mapRangeImpl(ContextVk *contextVk,
+                                     VkDeviceSize offset,
+                                     VkDeviceSize length,
+                                     GLbitfield access,
+                                     void **mapPtr)
+{
     ASSERT(mBuffer.valid());
 
-    ContextVk *contextVk = vk::GetImpl(context);
+    if ((access & GL_MAP_UNSYNCHRONIZED_BIT) == 0)
+    {
+        // If there are pending commands for the buffer, flush them.
+        if (mBuffer.isResourceInUse(contextVk))
+        {
+            ANGLE_TRY(contextVk->flushImpl(nullptr));
+        }
+        // Make sure the GPU is done with the buffer.
+        ANGLE_TRY(contextVk->finishToSerial(mBuffer.getStoredQueueSerial()));
+    }
 
     ANGLE_VK_TRY(contextVk, mBuffer.getDeviceMemory().map(contextVk->getDevice(), offset, length, 0,
                                                           reinterpret_cast<uint8_t **>(mapPtr)));
@@ -213,7 +227,7 @@ angle::Result BufferVk::onRead(ContextVk *contextVk,
         vk::CommandBuffer *commandBuffer;
         ANGLE_TRY(mBuffer.recordCommands(contextVk, &commandBuffer));
 
-        mBuffer.onWrite(mDataWriteAccessFlags);
+        mBuffer.onWrite(contextVk, mDataWriteAccessFlags);
         mDataWriteAccessFlags = 0;
     }
 
@@ -319,7 +333,7 @@ angle::Result BufferVk::copyToBuffer(ContextVk *contextVk,
     commandBuffer->copyBuffer(mBuffer.getBuffer(), destBuffer->getBuffer(), copyCount, copies);
 
     mBuffer.onRead(destBuffer, VK_ACCESS_TRANSFER_READ_BIT);
-    destBuffer->onWrite(VK_ACCESS_TRANSFER_WRITE_BIT);
+    destBuffer->onWrite(contextVk, VK_ACCESS_TRANSFER_WRITE_BIT);
 
     return angle::Result::Continue;
 }

src/libANGLE/renderer/vulkan/BufferVk.h

@@ -92,6 +92,11 @@ class BufferVk : public BufferImpl
     }
 
     angle::Result mapImpl(ContextVk *contextVk, void **mapPtr);
+    angle::Result mapRangeImpl(ContextVk *contextVk,
+                               VkDeviceSize offset,
+                               VkDeviceSize length,
+                               GLbitfield access,
+                               void **mapPtr);
     angle::Result unmapImpl(ContextVk *contextVk);
 
     angle::Result onRead(ContextVk *contextVk,

src/libANGLE/renderer/vulkan/CommandGraph.cpp

@@ -112,6 +112,15 @@ const char *GetResourceTypeName(CommandGraphResourceType resourceType,
                     UNREACHABLE();
                     return "DebugMarker";
             }
+        case CommandGraphResourceType::HostAvailabilityOperation:
+            switch (function)
+            {
+                case CommandGraphNodeFunction::HostAvailabilityOperation:
+                    return "HostAvailabilityOperation";
+                default:
+                    UNREACHABLE();
+                    return "HostAvailabilityOperation";
+            }
         default:
             UNREACHABLE();
             return "";
@@ -509,9 +518,9 @@ angle::Result CommandGraphNode::visitAndExecute(vk::Context *context,
                 memoryBarrier.dstAccessMask   = mGlobalMemoryBarrierDstAccess;
 
                 // Use the all pipe stage to keep the state management simple.
-                primaryCommandBuffer->pipelineBarrier(VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
-                                                      VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, 1,
-                                                      &memoryBarrier, 0, nullptr, 0, nullptr);
+                primaryCommandBuffer->memoryBarrier(VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
+                                                    VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
+                                                    &memoryBarrier);
             }
 
             if (mOutsideRenderPassCommands.valid())
@@ -628,6 +637,21 @@ angle::Result CommandGraphNode::visitAndExecute(vk::Context *context,
             }
             break;
 
+        case CommandGraphNodeFunction::HostAvailabilityOperation:
+            // Make sure all writes to host-visible buffers are flushed.  We have no way of knowing
+            // whether any buffer will be mapped for readback in the future, and we can't afford to
+            // flush and wait on a one-pipeline-barrier command buffer on every map().
+            {
+                VkMemoryBarrier memoryBarrier = {};
+                memoryBarrier.sType           = VK_STRUCTURE_TYPE_MEMORY_BARRIER;
+                memoryBarrier.srcAccessMask   = VK_ACCESS_MEMORY_WRITE_BIT;
+                memoryBarrier.dstAccessMask   = VK_ACCESS_HOST_READ_BIT;
+
+                primaryCommandBuffer->memoryBarrier(VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
+                                                    VK_PIPELINE_STAGE_HOST_BIT, &memoryBarrier);
+            }
+            break;
+
         default:
             UNREACHABLE();
     }
@@ -914,6 +938,12 @@ void CommandGraph::popDebugMarker()
                         CommandGraphResourceType::DebugMarker, 0);
 }
 
+void CommandGraph::makeHostVisibleBufferWriteAvailable()
+{
+    allocateBarrierNode(CommandGraphNodeFunction::HostAvailabilityOperation,
+                        CommandGraphResourceType::HostAvailabilityOperation, 0);
+}
+
 // Dumps the command graph into a dot file that works with graphviz.
 void CommandGraph::dumpGraphDotFile(std::ostream &out) const
 {
@@ -975,6 +1005,11 @@ void CommandGraph::dumpGraphDotFile(std::ostream &out) const
                 strstr << id;
             }
         }
+        else if (node->getResourceTypeForDiagnostics() ==
+                 CommandGraphResourceType::HostAvailabilityOperation)
+        {
+            // Nothing to append for this special node.  The name is sufficient.
+        }
         else
         {
             strstr << " ";

src/libANGLE/renderer/vulkan/CommandGraph.h

@@ -34,6 +34,7 @@ enum class CommandGraphResourceType
     Query,
     FenceSync,
     DebugMarker,
+    HostAvailabilityOperation,
 };
 
 // Certain functionality cannot be put in secondary command buffers, so they are special-cased in
@@ -49,6 +50,7 @@ enum class CommandGraphNodeFunction
     InsertDebugMarker,
     PushDebugMarker,
     PopDebugMarker,
+    HostAvailabilityOperation,
 };
 
 // Receives notifications when a command buffer is no longer able to record. Can be used with
@@ -451,6 +453,8 @@ class CommandGraph final : angle::NonCopyable
     void insertDebugMarker(GLenum source, std::string &&marker);
     void pushDebugMarker(GLenum source, std::string &&marker);
     void popDebugMarker();
+    // Host-visible buffer write availability operation:
+    void makeHostVisibleBufferWriteAvailable();
 
   private:
     CommandGraphNode *allocateBarrierNode(CommandGraphNodeFunction function,

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -173,6 +173,7 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk
       mCurrentDrawElementsType(gl::DrawElementsType::InvalidEnum),
       mClearColorMask(kAllColorChannelsMask),
       mFlipYForCurrentSurface(false),
+      mIsAnyHostVisibleBufferWritten(false),
       mDriverUniformsBuffer(VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, sizeof(DriverUniforms) * 16, true),
       mDriverUniformsDescriptorSet(VK_NULL_HANDLE),
       mDriverUniformsDynamicOffset(0),
@@ -755,6 +756,12 @@ void ContextVk::freeAllInFlightResources()
 
 angle::Result ContextVk::flushCommandGraph(vk::PrimaryCommandBuffer *commandBatch)
 {
+    if (mIsAnyHostVisibleBufferWritten)
+    {
+        mCommandGraph.makeHostVisibleBufferWriteAvailable();
+    }
+    mIsAnyHostVisibleBufferWritten = false;
+
     return mCommandGraph.submitCommands(this, mCurrentQueueSerial, &mRenderPassCache, &mCommandPool,
                                         commandBatch);
 }

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -213,6 +213,7 @@ class ContextVk : public ContextImpl, public vk::Context, public vk::CommandBuff
     void invalidateDefaultAttribute(size_t attribIndex);
     void invalidateDefaultAttributes(const gl::AttributesMask &dirtyMask);
     void onFramebufferChange(const vk::RenderPassDesc &renderPassDesc);
+    void onHostVisibleBufferWrite() { mIsAnyHostVisibleBufferWritten = true; }
 
     vk::DynamicQueryPool *getQueryPool(gl::QueryType queryType);
 
@@ -450,6 +451,10 @@ class ContextVk : public ContextImpl, public vk::Context, public vk::CommandBuff
     bool mFlipViewportForDrawFramebuffer;
     bool mFlipViewportForReadFramebuffer;
 
+    // If any host-visible buffer is written by the GPU since last submission, a barrier is inserted
+    // at the end of the command buffer to make that write available to the host.
+    bool mIsAnyHostVisibleBufferWritten;
+
     // For shader uniforms such as gl_DepthRange and the viewport size.
     struct DriverUniforms
     {

src/libANGLE/renderer/vulkan/UtilsVk.cpp

@@ -568,7 +568,7 @@ angle::Result UtilsVk::clearBuffer(ContextVk *contextVk,
     ANGLE_TRY(dest->recordCommands(contextVk, &commandBuffer));
 
     // Tell dest it's being written to.
-    dest->onWrite(VK_ACCESS_SHADER_WRITE_BIT);
+    dest->onWrite(contextVk, VK_ACCESS_SHADER_WRITE_BIT);
 
     const vk::Format &destFormat = dest->getViewFormat();
 
@@ -622,7 +622,7 @@ angle::Result UtilsVk::convertIndexBuffer(ContextVk *contextVk,
     // 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(VK_ACCESS_SHADER_WRITE_BIT);
+    dest->onWrite(contextVk, VK_ACCESS_SHADER_WRITE_BIT);
 
     VkDescriptorSet descriptorSet;
     vk::RefCountedDescriptorPoolBinding descriptorPoolBinding;
@@ -685,7 +685,7 @@ angle::Result UtilsVk::convertVertexBuffer(ContextVk *contextVk,
     // 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(VK_ACCESS_SHADER_WRITE_BIT);
+    dest->onWrite(contextVk, VK_ACCESS_SHADER_WRITE_BIT);
 
     ConvertVertexShaderParams shaderParams;
     shaderParams.Ns = params.srcFormat->channelCount();

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -1215,7 +1215,7 @@ void BufferHelper::release(DisplayVk *display, std::vector<GarbageObjectBase> *g
     mDeviceMemory.dumpResources(garbageQueue);
 }
 
-void BufferHelper::onWrite(VkAccessFlags writeAccessType)
+void BufferHelper::onWrite(ContextVk *contextVk, VkAccessFlags writeAccessType)
 {
     if (mCurrentReadAccess != 0 || mCurrentWriteAccess != 0)
     {
@@ -1224,6 +1224,12 @@ void BufferHelper::onWrite(VkAccessFlags writeAccessType)
 
     mCurrentWriteAccess = writeAccessType;
     mCurrentReadAccess  = 0;
+
+    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

@@ -445,7 +445,7 @@ class BufferHelper final : public CommandGraphResource
         }
     }
 
-    void onWrite(VkAccessFlags writeAccessType);
+    void onWrite(ContextVk *contextVk, VkAccessFlags writeAccessType);
 
     // Also implicitly sets up the correct barriers.
     angle::Result copyFromBuffer(ContextVk *contextVk,