-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Logic error in ImGui_ImplVulkan_RenderWindow() function. #7831
Comments
Hello, Thanks for reporting. |
@ocornut, this is a completely separated problem from what I reported with #7825. I mentioned my #7825 here because this one can create the conditions needed to make the indices problem of #7825 to occur. They are in the same .cpp but its different functions. There will probably be others and I will report them as in my attempt to go to the bottom of my #7602 ticket I decided to write a Vulkan/SDL3 backend from scratch (because as commented in #7602 it is a strange problem), and hence I walk the complete Dear ImGui Vulkan/SDL3 backends because i want to understand every single line of it. It is a quite simple and straight forward job but the EuroCup, Tour de France and the Olympics delay me :) If it is OK with you I would like to keep this #7831 separated from #7825, because IMO it is a separate issue. I have found several other things but I'll report on them when I am more certain of what I think I found. |
Understood. Could you clarify in which setup you managed to create this situation where Also note the On "Attention 2: The check_vk_result(err) is a black spot" I think we should avoid calling check_vk_result() on |
I cannot provide an exact situation because my backend for the docking branch is a 'chantier ouvert' since it isn't completely finished and can't run yet. But here is how you could proceed. Create one of the situations below using the Dear ImGui standard SDL3/Vulkan backends (I didn't check the others because I only do those two) that in all cases need to be prepared for the VK_SUBOPTIMAL_KHR return value even if currently only Harry Potter could provoke one. A VK_SUBOPTIMAL_KHR can come in one of the following situations (you could add a log to see it happen): Surface Size Change Surface Transformation Change Presentation Mode Change Multi-Monitor Setup Changes |
Additional info: In the ImGui_ImplVulkan_SwapBuffers() the following line checks the rebuild flag.
This means that you will need an EXTRA flag to qualify why the swapchain needs rebuild. If it is for ANY other reason then a VK_SUBOPTIMAL_KHR then the existing code is OK. But if you adopt the suggestion of setting the swapchain rebuild flag on the VK_SUBOPTIMAL_KHR return code, then the extra flag may be used to Present the frame no matter the fact that the SwapChainNeedRebuild is true in the line above. UNLESS of course you decide to loose that frame as happens when an VK_ERROR_OUT_OF_DATE_KHR return code is the reason why the rebuild flag is set. |
Version/Branch of Dear ImGui:
Version 1.90.9 Docking branch
Back-ends:
imgui_impl_vulkan.cpp
Compiler, OS:
All
Full config/build information:
All
Details:
Partial Code:
Comment:
In the code above the vkAcquireNextImageKHR() can ALSO return VK_SUBOPTIMAL_KHR.
It cannot return VK_TIMEOUT or VK_NOT_READY because the 'timeout' argument has been set to UINT64_MAX.
In the code above the VK_SUBOPTIMAL_KHR is correctly treated (unless see ATTENTION 2 below), as
if VK_SUCCESS was returned, however the Vulkan specification say this:
Source: [https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#swapchain-acquire-forward-progress]
Therefor, IMO, an extra line afther
check_vk_result(err)
is neededif(err == VK_SUBOPTIMAL_KHR) vd->SwapChainNeedRebuild = true;
Screenshots/Video:
No response
Minimal, Complete and Verifiable Example code:
No response
The text was updated successfully, but these errors were encountered: