Fix Vulkan swapchain synchronization

It is necessary to use a semaphore to synchronize rendering and
presenting even when the graphics and present queues are the same.

This fixes rendering artifacts encountered on a Mali-G78 device.

Bug: dawn:269
Change-Id: Ieb0240d3668b7f9f68857ed3cf8d3bfea18cf33a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/130221
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Albin Bernhardsson <albin.bernhardsson@arm.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn/native/vulkan/CommandRecordingContext.h

@@ -39,6 +39,7 @@ struct CommandPoolAndBuffer {
 struct CommandRecordingContext {
     VkCommandBuffer commandBuffer = VK_NULL_HANDLE;
     std::vector<VkSemaphore> waitSemaphores = {};
+    std::vector<VkSemaphore> signalSemaphores = {};
 
     // The internal buffers used in the workaround of texture-to-texture copies with compressed
     // formats.

src/dawn/native/vulkan/DeviceVk.cpp

@@ -314,11 +314,11 @@ MaybeError Device::SubmitPendingCommands() {
             fn, &mRecordingContext, mRecordingContext.mappableBuffersForEagerTransition);
     }
 
-    ScopedSignalSemaphore scopedSignalSemaphore(this, VK_NULL_HANDLE);
+    ScopedSignalSemaphore externalTextureSemaphore(this, VK_NULL_HANDLE);
     if (mRecordingContext.externalTexturesForEagerTransition.size() > 0) {
         // Create an external semaphore for all external textures that have been used in the pending
         // submit.
-        DAWN_TRY_ASSIGN(*scopedSignalSemaphore.InitializeInto(),
+        DAWN_TRY_ASSIGN(*externalTextureSemaphore.InitializeInto(),
                         mExternalSemaphoreService->CreateExportableSemaphore());
     }
 
@@ -336,6 +336,10 @@ MaybeError Device::SubmitPendingCommands() {
     std::vector<VkPipelineStageFlags> dstStageMasks(mRecordingContext.waitSemaphores.size(),
                                                     VK_PIPELINE_STAGE_ALL_COMMANDS_BIT);
 
+    if (externalTextureSemaphore.Get() != VK_NULL_HANDLE) {
+        mRecordingContext.signalSemaphores.push_back(externalTextureSemaphore.Get());
+    }
+
     VkSubmitInfo submitInfo;
     submitInfo.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
     submitInfo.pNext = nullptr;
@@ -344,8 +348,8 @@ MaybeError Device::SubmitPendingCommands() {
     submitInfo.pWaitDstStageMask = dstStageMasks.data();
     submitInfo.commandBufferCount = mRecordingContext.commandBufferList.size();
     submitInfo.pCommandBuffers = mRecordingContext.commandBufferList.data();
-    submitInfo.signalSemaphoreCount = (scopedSignalSemaphore.Get() == VK_NULL_HANDLE ? 0 : 1);
-    submitInfo.pSignalSemaphores = AsVkArray(scopedSignalSemaphore.InitializeInto());
+    submitInfo.signalSemaphoreCount = mRecordingContext.signalSemaphores.size();
+    submitInfo.pSignalSemaphores = AsVkArray(mRecordingContext.signalSemaphores.data());
 
     VkFence fence = VK_NULL_HANDLE;
     DAWN_TRY_ASSIGN(fence, GetUnusedFence());
@@ -376,7 +380,7 @@ MaybeError Device::SubmitPendingCommands() {
         // Export the signal semaphore.
         ExternalSemaphoreHandle semaphoreHandle;
         DAWN_TRY_ASSIGN(semaphoreHandle,
-                        mExternalSemaphoreService->ExportSemaphore(scopedSignalSemaphore.Get()));
+                        mExternalSemaphoreService->ExportSemaphore(externalTextureSemaphore.Get()));
 
         // Update all external textures, eagerly transitioned in the submit, with the exported
         // handle, and the duplicated handles.
@@ -1086,6 +1090,7 @@ void Device::DestroyImpl() {
         fn.DestroySemaphore(mVkDevice, semaphore, nullptr);
     }
     mRecordingContext.waitSemaphores.clear();
+    mRecordingContext.signalSemaphores.clear();
 
     // Some commands might still be marked as in-flight if we shut down because of a device
     // loss. Recycle them as unused so that we free them below.

src/dawn/native/vulkan/SwapChainVk.cpp

@@ -291,6 +291,15 @@ MaybeError SwapChain::Initialize(SwapChainBase* previousSwapChain) {
         ToBackend(previousSwapChain->GetDevice())
             ->GetFencedDeleter()
             ->DeleteWhenUnused(previousVkSwapChain);
+
+        // Delete the previous swapchain's semaphores once they are not in use.
+        // TODO(crbug.com/dawn/269): Wait for presentation to finish rather than submission.
+        for (VkSemaphore semaphore : previousVulkanSwapChain->mSwapChainSemaphores) {
+            ToBackend(previousSwapChain->GetDevice())
+                ->GetFencedDeleter()
+                ->DeleteWhenUnused(semaphore);
+        }
+        previousVulkanSwapChain->mSwapChainSemaphores.clear();
     }
 
     if (mVkSurface == VK_NULL_HANDLE) {
@@ -340,6 +349,21 @@ MaybeError SwapChain::Initialize(SwapChainBase* previousSwapChain) {
                                                         AsVkArray(mSwapChainImages.data())),
                        "GetSwapChainImages2"));
 
+    // Create one semaphore per swapchain image.
+    mSwapChainSemaphores.resize(count);
+
+    VkSemaphoreCreateInfo semaphoreCreateInfo;
+    semaphoreCreateInfo.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
+    semaphoreCreateInfo.pNext = nullptr;
+    semaphoreCreateInfo.flags = 0;
+
+    for (std::size_t i = 0; i < mSwapChainSemaphores.size(); i++) {
+        DAWN_TRY(
+            CheckVkSuccess(device->fn.CreateSemaphore(device->GetVkDevice(), &semaphoreCreateInfo,
+                                                      nullptr, &*mSwapChainSemaphores[i]),
+                           "CreateSemaphore"));
+    }
+
     return {};
 }
 
@@ -546,17 +570,17 @@ MaybeError SwapChain::PresentImpl() {
     mTexture->TransitionUsageNow(recordingContext, kPresentTextureUsage,
                                  mTexture->GetAllSubresources());
 
+    // Use a semaphore to make sure all rendering has finished before presenting.
+    VkSemaphore currentSemaphore = mSwapChainSemaphores[mLastImageIndex];
+    recordingContext->signalSemaphores.push_back(currentSemaphore);
+
     DAWN_TRY(device->SubmitPendingCommands());
 
-    // Assuming that the present queue is the same as the graphics queue, the proper
-    // synchronization has already been done on the queue so we don't need to wait on any
-    // semaphores.
-    // TODO(crbug.com/dawn/269): Support the present queue not being the main queue.
     VkPresentInfoKHR presentInfo;
     presentInfo.sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR;
     presentInfo.pNext = nullptr;
-    presentInfo.waitSemaphoreCount = 0;
-    presentInfo.pWaitSemaphores = nullptr;
+    presentInfo.waitSemaphoreCount = 1;
+    presentInfo.pWaitSemaphores = AsVkArray(&currentSemaphore);
     presentInfo.swapchainCount = 1;
     presentInfo.pSwapchains = &*mSwapChain;
     presentInfo.pImageIndices = &mLastImageIndex;
@@ -681,6 +705,12 @@ void SwapChain::DetachFromSurfaceImpl() {
         mBlitTexture = nullptr;
     }
 
+    for (VkSemaphore semaphore : mSwapChainSemaphores) {
+        // TODO(crbug.com/dawn/269): Wait for presentation to finish rather than submission.
+        ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(semaphore);
+    }
+    mSwapChainSemaphores.clear();
+
     // The swapchain images are destroyed with the swapchain.
     if (mSwapChain != VK_NULL_HANDLE) {
         ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mSwapChain);

src/dawn/native/vulkan/SwapChainVk.h

@@ -76,6 +76,7 @@ class SwapChain : public SwapChainBase {
     VkSurfaceKHR mVkSurface = VK_NULL_HANDLE;
     VkSwapchainKHR mSwapChain = VK_NULL_HANDLE;
     std::vector<VkImage> mSwapChainImages;
+    std::vector<VkSemaphore> mSwapChainSemaphores;
     uint32_t mLastImageIndex = 0;
 
     Ref<Texture> mBlitTexture;