Skip to content

Backends: Vulkan: fix present semaphore reuse for secondary viewports. - #9420

Open
Tom-Olsen wants to merge 1 commit into
ocornut:dockingfrom
Tom-Olsen:fix-vulkan-present-semaphore-reuse
Open

Backends: Vulkan: fix present semaphore reuse for secondary viewports.#9420
Tom-Olsen wants to merge 1 commit into
ocornut:dockingfrom
Tom-Olsen:fix-vulkan-present-semaphore-reuse

Conversation

@Tom-Olsen

@Tom-Olsen Tom-Olsen commented May 30, 2026

Copy link
Copy Markdown

What changed:
Keep image acquisition semaphores indexed by the rotating SemaphoreIndex, but select RenderCompleteSemaphore using the acquired swapchain image FrameIndex.

Reasoning:
A semaphore passed to vkQueuePresentKHR may remain in use until the corresponding swapchain image is acquired again. Rotating present wait semaphores independently can therefore trigger:

VUID-vkQueueSubmit-pSignalSemaphores-00067

My setup:
Im using Linux (Fedora) with vulkan 1.4.341, sdl3, gcc 16.1.1, and ran with x11 as wayland doesnt support separate windwos due to lack of global mouse positioning if i understood correctly.

Error before fix:
[12:06:05] Ember: Validation layer ERROR: vkQueueSubmit(): pSubmits[0].pSignalSemaphores[0] (VkSemaphore 0x19930000001993) is being signaled by VkQueue 0x2b272940[Queue_Graphics], but it may still be in use by VkSwapchainKHR 0x19780000001978.
Most recently acquired image indices: 2, 1, [0], 2, 1, 2, 1.
(Brackets mark the last use of VkSemaphore 0x19930000001993 in a presentation operation.)
Swapchain image 0 was presented but was not re-acquired, so VkSemaphore 0x19930000001993 may still be in use and cannot be safely reused with image index 1.
Vulkan insight: See https://docs.vulkan.org/guide/latest/swapchain_semaphore_reuse.html for details on swapchain semaphore reuse. Examples of possible approaches:
a) Use a separate semaphore per swapchain image. Index these semaphores using the index of the acquired image.
b) Consider the VK_KHR_swapchain_maintenance1 extension. It allows using a VkFence with the presentation operation.
The Vulkan spec states: Each binary semaphore element of the pSignalSemaphores member of any element of pSubmits must be unsignaled when the semaphore signal operation it defines is executed on the device (https://docs.vulkan.org/spec/latest/chapters/cmdbuffers.html#VUID-vkQueueSubmit-pSignalSemaphores-00067)

@Gooh456

Gooh456 commented Jul 22, 2026

Copy link
Copy Markdown

Read-through review (didn't build/run this — no Vulkan/GPU setup here, so this is code-reading only, not tested).

Your diagnosis matches what I see reading ImGui_ImplVulkanH_Window: SemaphoreCount = ImageCount + 1 plus the rotating SemaphoreIndex (which is unrelated to which swapchain image actually got acquired) is exactly the shape of thing the Vulkan guide's option (a) warns about for the signal semaphore that present waits on. Switching RenderCompleteSemaphore to be indexed by the actually-acquired FrameIndex, while leaving ImageAcquiredSemaphore on the rotating SemaphoreIndex, matches that documented fix.

One thing worth flagging before merge: this only patches the multi-viewport/secondary-window path (ImGui_ImplVulkan_RenderWindow/SwapBuffers in the backend). The primary-viewport example apps have the exact same acquire/submit/present pattern and don't look touched — e.g. examples/example_glfw_vulkan/main.cpp's FrameRender/FramePresent still look up both image_acquired_semaphore and render_complete_semaphore via the same rotating wd->SemaphoreIndex (around lines 267-268, 334, 349 on docking). Since it's the identical ImGui_ImplVulkanH_Window acquire-submit-present sequence, that path looks just as susceptible to VUID-vkQueueSubmit-pSignalSemaphores-00067 as the one being fixed here — "for secondary viewports" in the title may be underselling how general the underlying issue is. Might be worth applying the same fix to the example main.cpp files too, or at least flagging that they still need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants