Vulkan: Async Queue: Fix signal of external semaphores

Per the Vulkan spec:

> ... when a semaphore wait operation is submitted to a queue:
>
> - A binary semaphore must be signaled, or have an associated semaphore
>   signal operation that is pending execution.

It is incorrect for ANGLE to defer the submission that signals an
external semaphore, because then it's impossible for the application to
know when it can wait on the semaphore.

Bug: angleproject:6574
Bug: b/172704839
Change-Id: I5469b500b2f7d402acec31d9848585a9947843c6
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3226308
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>

src/libANGLE/renderer/vulkan/CommandProcessor.cpp

@@ -646,13 +646,6 @@ void CommandProcessor::handleDeviceLost(RendererVk *renderer)
     mCommandQueue.handleDeviceLost(renderer);
 }
 
-angle::Result CommandProcessor::finishAllWork(Context *context)
-{
-    ANGLE_TRACE_EVENT0("gpu.angle", "CommandProcessor::finishAllWork");
-    // Wait for GPU work to finish
-    return finishToSerial(context, Serial::Infinite(), mRenderer->getMaxFenceWaitTimeNs());
-}
-
 VkResult CommandProcessor::getLastAndClearPresentResult(VkSwapchainKHR swapchain)
 {
     std::unique_lock<std::mutex> lock(mSwapchainStatusMutex);
@@ -788,6 +781,11 @@ angle::Result CommandProcessor::flushRenderPassCommands(Context *context,
                                              renderPassCommands);
 }
 
+angle::Result CommandProcessor::ensureNoPendingWork(Context *context)
+{
+    return waitForWorkComplete(context);
+}
+
 // CommandQueue implementation.
 CommandQueue::CommandQueue() : mCurrentQueueSerial(mQueueSerialFactory.generate()) {}
 

src/libANGLE/renderer/vulkan/CommandProcessor.h

@@ -315,6 +315,10 @@ class CommandQueueInterface : angle::NonCopyable
                                                   const RenderPass &renderPass,
                                                   CommandBufferHelper **renderPassCommands) = 0;
 
+    // For correct synchronization with external, in particular when asked to signal an external
+    // semaphore, we need to ensure that there are no pending submissions.
+    virtual angle::Result ensureNoPendingWork(Context *context) = 0;
+
     virtual Serial getLastCompletedQueueSerial() const = 0;
     virtual Serial getCurrentQueueSerial() const       = 0;
     virtual bool isBusy() const                        = 0;
@@ -374,6 +378,8 @@ class CommandQueue final : public CommandQueueInterface
                                           const RenderPass &renderPass,
                                           CommandBufferHelper **renderPassCommands) override;
 
+    angle::Result ensureNoPendingWork(Context *context) override { return angle::Result::Continue; }
+
     Serial getLastCompletedQueueSerial() const override;
     Serial getCurrentQueueSerial() const override;
     bool isBusy() const override;
@@ -459,11 +465,6 @@ class CommandProcessor : public Context, public CommandQueueInterface
     CommandProcessor(RendererVk *renderer);
     ~CommandProcessor() override;
 
-    // Used by main thread to wait for worker thread to complete all outstanding work.
-    // TODO(jmadill): Make private. b/172704839
-    angle::Result waitForWorkComplete(Context *context);
-    angle::Result finishAllWork(Context *context);
-
     VkResult getLastPresentResult(VkSwapchainKHR swapchain)
     {
         return getLastAndClearPresentResult(swapchain);
@@ -524,6 +525,8 @@ class CommandProcessor : public Context, public CommandQueueInterface
                                           const RenderPass &renderPass,
                                           CommandBufferHelper **renderPassCommands) override;
 
+    angle::Result ensureNoPendingWork(Context *context) override;
+
     Serial getLastCompletedQueueSerial() const override;
     Serial getCurrentQueueSerial() const override;
     bool isBusy() const override;
@@ -560,6 +563,9 @@ class CommandProcessor : public Context, public CommandQueueInterface
     VkResult getLastAndClearPresentResult(VkSwapchainKHR swapchain);
     VkResult present(egl::ContextPriority priority, const VkPresentInfoKHR &presentInfo);
 
+    // Used by main thread to wait for worker thread to complete all outstanding work.
+    angle::Result waitForWorkComplete(Context *context);
+
     std::queue<CommandProcessorTask> mTasks;
     mutable std::mutex mWorkerMutex;
     // Signal worker thread when work is available

src/libANGLE/renderer/vulkan/RendererVk.h

@@ -378,6 +378,18 @@ class RendererVk : angle::NonCopyable
         }
     }
 
+    angle::Result ensureNoPendingWork(vk::Context *context)
+    {
+        if (mFeatures.asyncCommandQueue.enabled)
+        {
+            return mCommandProcessor.ensureNoPendingWork(context);
+        }
+        else
+        {
+            return mCommandQueue.ensureNoPendingWork(context);
+        }
+    }
+
     egl::Display *getDisplay() const { return mDisplay; }
 
     VkResult getLastPresentResult(VkSwapchainKHR swapchain)
@@ -594,16 +606,17 @@ class RendererVk : angle::NonCopyable
     };
     std::deque<PendingOneOffCommands> mPendingOneOffCommands;
 
+    // Synchronous Command Queue
     std::mutex mCommandQueueMutex;
     vk::CommandQueue mCommandQueue;
 
+    // Async Command Queue
+    vk::CommandProcessor mCommandProcessor;
+
     // Command buffer pool management.
     std::mutex mCommandBufferRecyclerMutex;
     vk::CommandBufferRecycler mCommandBufferRecycler;
 
-    // Async Command Queue
-    vk::CommandProcessor mCommandProcessor;
-
     vk::BufferMemoryAllocator mBufferMemoryAllocator;
     vk::Allocator mAllocator;
     SamplerCache mSamplerCache;

src/libANGLE/renderer/vulkan/SemaphoreVk.cpp

@@ -160,8 +160,9 @@ angle::Result SemaphoreVk::signal(gl::Context *context,
                                   const gl::TextureBarrierVector &textureBarriers)
 {
     ContextVk *contextVk = vk::GetImpl(context);
+    RendererVk *renderer = contextVk->getRenderer();
 
-    uint32_t rendererQueueFamilyIndex = contextVk->getRenderer()->getQueueFamilyIndex();
+    uint32_t rendererQueueFamilyIndex = renderer->getQueueFamilyIndex();
 
     if (!bufferBarriers.empty())
     {
@@ -218,7 +219,19 @@ angle::Result SemaphoreVk::signal(gl::Context *context,
         ANGLE_TRY(contextVk->syncExternalMemory());
     }
 
-    return contextVk->flushImpl(&mSemaphore);
+    ANGLE_TRY(contextVk->flushImpl(&mSemaphore));
+
+    // The external has asked for the semaphore to be signaled.  It will wait on this semaphore and
+    // so we must ensure that the above flush (resulting in vkQueueSubmit) has actually been
+    // submitted (as opposed to simply being scheduled as a task for another thread).  Per the
+    // Vulkan spec:
+    //
+    // > ... when a semaphore wait operation is submitted to a queue:
+    // >
+    // > - A binary semaphore must be signaled, or have an associated semaphore signal operation
+    // >   that is pending execution.
+    //
+    return renderer->ensureNoPendingWork(contextVk);
 }
 
 angle::Result SemaphoreVk::importOpaqueFd(ContextVk *contextVk, GLint fd)

src/tests/angle_end2end_tests_expectations.txt

@@ -25,9 +25,6 @@
 4889 VULKAN : GeometryShaderTest.LayeredFramebufferPreRenderClear3DColor/* = SKIP
 4889 VULKAN : GLSLTest_ES31.VaryingTessellationSampleInAndOut/* = SKIP
 
-// Async queue defers submission that signals external semaphore, which is incorrect
-6574 VULKAN : VulkanExternalImageTest.*AsyncQueue = SKIP
-
 // Windows
 3786 WIN NVIDIA D3D11 : BufferDataOverflowTest.VertexBufferIntegerOverflow/* = SKIP
 4092 WIN VULKAN : BufferDataOverflowTest.VertexBufferIntegerOverflow/* = SKIP