From e1c95e71f186df33feb661e1bb0bbf29726a054b Mon Sep 17 00:00:00 2001 From: mchughalex <25671999+mchughalex@users.noreply.github.com> Date: Thu, 16 Jul 2026 22:33:51 +0100 Subject: [PATCH] vulkan: per-swapchain-image presentation-wait semaphores --- include/rex/ui/vulkan/presenter.h | 31 ++++++++++++- src/ui/vulkan/vulkan_presenter.cpp | 74 +++++++++++++++++++++++++----- 2 files changed, 92 insertions(+), 13 deletions(-) diff --git a/include/rex/ui/vulkan/presenter.h b/include/rex/ui/vulkan/presenter.h index 56ee43b..4194c8c 100644 --- a/include/rex/ui/vulkan/presenter.h +++ b/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 swapchain_images; std::vector 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 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 retired_present_semaphores; }; explicit VulkanPresenter(HostGpuLossCallback host_gpu_loss_callback, diff --git a/src/ui/vulkan/vulkan_presenter.cpp b/src/ui/vulkan/vulkan_presenter.cpp index 6efd223..b404ec8 100644 --- a/src/ui/vulkan/vulkan_presenter.cpp +++ b/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_) != - VK_SUCCESS) { - REXLOG_ERROR( - "VulkanPresenter: Failed to create a swapchain image presentation " - "semaphore"); - return false; - } + // Note: the presentation-wait semaphores are per-swapchain-image, not + // per-submission - see PaintContext::swapchain_present_semaphores. 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;