vulkan: per-swapchain-image presentation-wait semaphores

include/rex/ui/vulkan/presenter.h

@@ -281,7 +281,6 @@ class VulkanPresenter final : public Presenter {
       ~Submission();
 
       VkSemaphore acquire_semaphore() const { return acquire_semaphore_; }
-      VkSemaphore present_semaphore() const { return present_semaphore_; }
       VkCommandPool draw_command_pool() const { return draw_command_pool_; }
       VkCommandBuffer draw_command_buffer() const { return draw_command_buffer_; }
 
@@ -291,8 +290,14 @@ class VulkanPresenter final : public Presenter {
       bool Initialize();
 
       const VulkanDevice* vulkan_device_;
+      // Swapchain image acquisition semaphore. Safe to pool per-submission:
+      // reuse is gated by the paint submission fence (kSubmissionCount frames
+      // back), and that fence covers the vkQueueSubmit that waits on this
+      // semaphore, which is what returns it to the unsignaled state.
+      // The present-wait semaphores, however, are NOT per-submission - they
+      // are per-swapchain-image (PaintContext::swapchain_present_semaphores),
+      // because no fence covers the presentation engine's wait on them.
       VkSemaphore acquire_semaphore_ = VK_NULL_HANDLE;
-      VkSemaphore present_semaphore_ = VK_NULL_HANDLE;
       VkCommandPool draw_command_pool_ = VK_NULL_HANDLE;
       VkCommandBuffer draw_command_buffer_ = VK_NULL_HANDLE;
     };
@@ -416,6 +421,28 @@ class VulkanPresenter final : public Presenter {
     bool swapchain_is_fifo = false;
     std::vector<VkImage> swapchain_images;
     std::vector<SwapchainFramebuffer> swapchain_framebuffers;
+    // One presentation-wait ("draw complete") semaphore per swapchain image,
+    // indexed by the acquired image index. Signaled by the paint vkQueueSubmit
+    // that writes the image, waited on by vkQueuePresentKHR. Binary present
+    // semaphores must not be re-signaled while the presentation engine may
+    // still be waiting on them, and vkQueuePresentKHR provides no host-visible
+    // completion signal - the only host-observable proof that the previous
+    // present of an image (and therefore its semaphore wait) has completed is
+    // vkAcquireNextImageKHR returning that same image again. Keying the
+    // semaphore by the acquired image index makes every re-signal follow such
+    // a re-acquisition, which is the standard validation-clean discipline
+    // (VUID-vkQueueSubmit-pSignalSemaphores-00067).
+    std::vector<VkSemaphore> swapchain_present_semaphores;
+    // Present-wait semaphores of retired swapchains. A retired swapchain may
+    // still have presents in flight whose semaphore waits can't be proven
+    // complete from the host, so the semaphores are neither destroyed nor
+    // reused with the new swapchain (a carried-over pending/stale signal would
+    // let a later vkQueuePresentKHR wait pair with an old signal, leaving the
+    // new frame's writes unordered against presentation - the
+    // SYNC-HAZARD-PRESENT-AFTER-WRITE case). Destroyed in ~VulkanPresenter
+    // after all submissions have completed and the swapchain releasing them
+    // has been destroyed.
+    std::vector<VkSemaphore> retired_present_semaphores;
   };
 
   explicit VulkanPresenter(HostGpuLossCallback host_gpu_loss_callback,

src/ui/vulkan/vulkan_presenter.cpp

@@ -197,9 +197,6 @@ VulkanPresenter::PaintContext::Submission::~Submission() {
     dfn.vkDestroyCommandPool(device, draw_command_pool_, nullptr);
   }
 
-  if (present_semaphore_ != VK_NULL_HANDLE) {
-    dfn.vkDestroySemaphore(device, present_semaphore_, nullptr);
-  }
   if (acquire_semaphore_ != VK_NULL_HANDLE) {
     dfn.vkDestroySemaphore(device, acquire_semaphore_, nullptr);
   }
@@ -220,13 +217,8 @@ bool VulkanPresenter::PaintContext::Submission::Initialize() {
         "semaphore");
     return false;
   }
-  if (dfn.vkCreateSemaphore(device, &semaphore_create_info, nullptr, &present_semaphore_) !=
+  // Note: the presentation-wait semaphores are per-swapchain-image, not
-      VK_SUCCESS) {
+  // per-submission - see PaintContext::swapchain_present_semaphores.
-    REXLOG_ERROR(
-        "VulkanPresenter: Failed to create a swapchain image presentation "
-        "semaphore");
-    return false;
-  }
 
   VkCommandPoolCreateInfo command_pool_create_info;
   command_pool_create_info.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO;
@@ -282,6 +274,17 @@ VulkanPresenter::~VulkanPresenter() {
   const VulkanDevice::Functions& dfn = vulkan_device_->functions();
   const VkDevice device = vulkan_device_->device();
 
+  // All paint submissions have completed and the swapchains (whose pending
+  // present operations' semaphore waits are assumed to be tracked internally
+  // by the WSI, as described above) have been destroyed - the present-wait
+  // semaphores of the current and all retired swapchains
+  // (PrepareForSwapchainRetirement moves the current ones into the retired
+  // list) can now be destroyed.
+  for (VkSemaphore retired_present_semaphore : paint_context_.retired_present_semaphores) {
+    dfn.vkDestroySemaphore(device, retired_present_semaphore, nullptr);
+  }
+  paint_context_.retired_present_semaphores.clear();
+
   if (paint_context_.swapchain_render_pass != VK_NULL_HANDLE) {
     dfn.vkDestroyRenderPass(device, paint_context_.swapchain_render_pass, nullptr);
   }
@@ -1099,6 +1102,32 @@ VulkanPresenter::ConnectOrReconnectPaintingToSurfaceFromUIThread(Surface& new_su
     paint_context_.swapchain_framebuffers.emplace_back(image_view, framebuffer);
   }
 
+  // Create one presentation-wait semaphore per swapchain image, always fresh
+  // for a new swapchain (the previous swapchain's semaphores were moved to
+  // retired_present_semaphores by PrepareForSwapchainRetirement). The paint
+  // submission signals the semaphore of the acquired image, and
+  // vkQueuePresentKHR waits on it - re-signaling is safe because the image
+  // index has just been re-acquired, proving its previous present (and thus
+  // the presentation engine's wait on this semaphore) has completed.
+  assert_true(paint_context_.swapchain_present_semaphores.empty());
+  paint_context_.swapchain_present_semaphores.reserve(paint_context_.swapchain_images.size());
+  VkSemaphoreCreateInfo present_semaphore_create_info;
+  present_semaphore_create_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
+  present_semaphore_create_info.pNext = nullptr;
+  present_semaphore_create_info.flags = 0;
+  for (size_t i = 0; i < paint_context_.swapchain_images.size(); ++i) {
+    VkSemaphore present_semaphore;
+    if (dfn.vkCreateSemaphore(device, &present_semaphore_create_info, nullptr,
+                              &present_semaphore) != VK_SUCCESS) {
+      REXLOG_ERROR(
+          "VulkanPresenter: Failed to create a swapchain image presentation "
+          "semaphore");
+      paint_context_.DestroySwapchainAndVulkanSurface();
+      return SurfacePaintConnectResult::kFailure;
+    }
+    paint_context_.swapchain_present_semaphores.push_back(present_semaphore);
+  }
+
   is_vsync_implicit_out = paint_context_.swapchain_is_fifo;
   return SurfacePaintConnectResult::kSuccess;
 }
@@ -1500,6 +1529,22 @@ VkSwapchainKHR VulkanPresenter::PaintContext::PrepareForSwapchainRetirement() {
     dfn.vkDestroyImageView(device, framebuffer.image_view, nullptr);
   }
   swapchain_framebuffers.clear();
+  // The retired swapchain may still have presents in flight, and
+  // vkQueuePresentKHR provides no host-visible signal for the completion of
+  // its semaphore wait operations (the submission fences awaited above only
+  // cover the vkQueueSubmit that signaled them). Retire the per-image
+  // present-wait semaphores instead of destroying or reusing them - a new set
+  // is created for the new swapchain, so no stale pending signal can ever be
+  // carried over into it (that carry-over caused
+  // SYNC-HAZARD-PRESENT-AFTER-WRITE: a present pairing with an old signal
+  // leaves the new frame's swapchain image writes unordered against
+  // presentation). Semaphores are tiny and swapchain recreations are rare, so
+  // the retired list stays small; it's destroyed in ~VulkanPresenter once all
+  // submissions have completed and the swapchains have been destroyed.
+  retired_present_semaphores.insert(retired_present_semaphores.end(),
+                                    swapchain_present_semaphores.begin(),
+                                    swapchain_present_semaphores.end());
+  swapchain_present_semaphores.clear();
   swapchain_images.clear();
   swapchain_extent.width = 0;
   swapchain_extent.height = 0;
@@ -2249,7 +2294,14 @@ Presenter::PaintResult VulkanPresenter::PaintAndPresentImpl(bool execute_ui_draw
     paint_context_.ui_setup_command_buffer_current_index = SIZE_MAX;
   }
   command_buffers[command_buffer_count++] = draw_command_buffer;
-  VkSemaphore present_semaphore = paint_submission.present_semaphore();
+  // Signal the presentation-wait semaphore belonging to the acquired swapchain
+  // image (not a per-submission one - the submission ring's fence can't prove
+  // the presentation engine has finished waiting on a present semaphore, only
+  // re-acquisition of the same image can, so per-image semaphores are the only
+  // safe pooling; see PaintContext::swapchain_present_semaphores).
+  assert_true(size_t(swapchain_image_index) < paint_context_.swapchain_present_semaphores.size());
+  VkSemaphore present_semaphore =
+      paint_context_.swapchain_present_semaphores[swapchain_image_index];
   VkSubmitInfo submit_info;
   submit_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
   submit_info.pNext = nullptr;