Skip to content

Conversation

ChrisTom-94
Copy link
Contributor

TL;DR

Fixes a vulkan swapchain synchronization bug reported in #8795 by properly distinguishing FrameIndex (used for command buffers, fences, and acquire semaphores) from ImageIndex (used for the current frame’s render-complete semaphore).
Removes the unnecessary extra semaphore per image and waits fences before acquiring the next image.
Supersedes PR #8842 and ensures consistent behavior across all example vulkan backends (GLFW, SDL2, SDL3, Win32).

Description

This pull request provides a fix for issue #8795.
It supersedes PR #8842, which attempted to solve the problem but did not fully resolve it.
The reason why the problem doesn't occur every time is mentioned in the Vulkan guide on swapchain semaphore reuse

Since Vulkan SDK 1.4.313, the validation layer reports cases where the present wait semaphore is not used safely.

So, if the device (and the installed SDK with the validation layer) is running below the 1.4.313 the issue isn't reported.

Changes included

  • Clarifies the usage of two indices in ImGui_ImplVulkanH_Window:
    • FrameIndex: used for VkCommandBuffer, VkFence, ImageAcquiredSemaphore, etc.
    • ImageIndex (ex: SemaphoreIndex): returned by vkAcquireNextImageKHR, used to retrieve the current frame’s RenderCompleteSemaphore and as pImageIndices for the VkQueuePresentKHR.
  • Removed SemaphoreCount (previously ImageCount + 1); we only need one acquire/render semaphore pair per image.
  • Ensures the fence is waited before acquiring the next image.
  • Fix the issues in all vulkan related examples using the ImGui_ImplVulkanH_Window.

Why this works

As discussed in #8795, the Vulkan guide on swapchain semaphore reuse explains the problem well.
The RenderCompleteSemaphore and the 'VkQueuePresentKHR' depends on the acquired image index, while other objects should follow the frame ring index.

Testing

  • Tested in Vulkan SDK (and device supports) up to 1.4.321
  • Verified on all example backends using Vulkan (GLFW, SDL2, SDL3, Win32).
  • Same implementation is used in a personal Vulkan backend for ImGui with viewports enabled (uses 'timeline semaphores', so results are less directly relevant, but the acquire/present process is consistent, and I present to all secondary swapchains at once without any problem).

Possible improvements

  • Consider refactoring the common Vulkan example code: FrameRender and FramePresent are nearly identical across all backends.
  • As mentioned in #8795, using timeline semaphores would be a worthwhile long-term improvement.

Possible breaking changes

  • End-users who directly use ImGui_ImplVulkanH_Window and reference SemaphoreCount or SemaphoreIndex may need to update their code.
  • SemaphoreIndex was renamed for clarity, but it could also be kept unchanged if backward compatibility is preferred.
  • Normally, most end-users do not interact with this struct directly, so impact should be minimal.

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.

2 participants