ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
59.46k stars 10.14k forks source link

Multi viewport windows: Docking node artifacting when secondary window is quickly moved over node #6733

Open PatrickDahlin opened 1 year ago

PatrickDahlin commented 1 year ago

Version/Branch of Dear ImGui:

Version: 1.87 Branch: docking and multi-viewports enabled

Back-end/Renderer/Compiler/OS Vulkan with GLFW built on msvc

Back-ends: imgui_impl_vulkan.cpp + imgui_impl_glfw.cpp Operating System: Windows 10

My Issue/Question:

Docking nodes seem to glitch/bug out visually when moving a secondary window ontop. (See video for reference) This has been reproduced in both Walnut and a private project using a newer version of imgui (v1.89.7). It's worth noting that while Walnut is using a fork with some slight modifications, the private project I used with 1.89.7 has no modifications and still reproduces this. (vulkan, glfw, so similar setup as walnut)

I'm 100% sure this isnt an issue with the modifications that Walnut has done to it's fork of imgui because of the earlier mentioned private project being implemented using non-modified version and still showing this behaviour.

This delay of the docking node movement seems to be more noticable on a higher refreshrate monitor, which in turn would lend itself to this being a latency issue with the window movement. (wild guess)

I am not sure how to debug an issue as this since it seems to be with frame to frame timing and window movements, and I'm not that familiar with that part of imgui. I will gather more info if need be.

Screenshots/Video

Note how the docking node is moving around when the secondary window is moved ontop of the main window. https://github.com/ocornut/imgui/assets/33965707/a85bb6da-117e-4cab-aede-2cf6bdf503b0

Cloning Walnut from master and running that project is one way to reproduce this.

PathogenDavid commented 1 year ago

This is a known issue, see https://github.com/ocornut/imgui/issues/2361

(Also thanks for filling out the entire issue template!)

ocornut commented 1 year ago

I do believe that Vulkan now has ways to request swap simultaneously on multiple swap chains and I wonder if that would at least fix it for affected drivers. Worth trying to implement that.

uchytilc commented 7 months ago

I just wanted to provide some information on what I've found that helps reduce/eliminate the node artifacting. If using Vulkan, all window swap chains can be provided in a single present call. This eliminates the framerate issues associated with having multiple undocked windows but doesn't completely eliminate the node artifacting. If the undocked windows are also place earlier in the VkPresentInfoKHR struct so that they are drawn first, this seems to completely eliminates the artifacting. I think as long as the viewport being moved is placed earlier in the VkPresentInfoKHR then the viewport being docked to no artifacting will occur. If the viewport being moved can be easily identified, it can be drawn first each frame and I think that will remove artifacting. It's possible this doesn't completely fix the issue but from testing I haven't seen any artifacting as of yet.

EDIT: I recently updated my NVIDIA graphics drivers to 551.61 and this no longer works, the artifacting is quite bad.

NostraMagister commented 1 month ago

@uchytilc if multiple swapchains (and hence multiple related image indices) would be passed to the vkQueuePresentKHR() two problems will occur in the current design of the Dear ImGui courtesy Vulkan backend.

`VkPresentInfoKHR info = {}; info.sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR; info.waitSemaphoreCount = <<< how many semaphores must signal; info.pWaitSemaphores = <<< which or how many render semaphores to use here? info.swapchainCount = <<< swapchain & image indices size. info.pSwapchains = <<< swapchain handle array info.pImageIndices = <<< image indices array'

The rendering for EACH involved swapchain image that will be presented must be done/finished completely before present may occur, if not artefacts are guaranteed because one or more surfaces will only be partially rendered. Yet, the vkQueuePresentKHR() function will return immediately and wait for the rendering semaphore to become signaled for that effective presentation to start, because it is an asynchronous function.

Now you have, say, 4 viewports and hence 4 swapchains with associated indices. Yet, the current code above in the backend waits for only one semaphore.

That explains why your re-ordering suggestion has effect. If you wait for the semaphore of the LAST QueueSubmit() and if rendering occurs sequentially (it does, there is only one queue used for all viewports: err = vkQueuePresentKHR(v->Queue, &info);), the signaling of the last semaphore is the one that implies that all the acquired images of each swapchain involved are rendered. No more artefacts due to incomplete render output.

It has in fact, IMO, nothing to do with what draws first, it just has to do with all rendering being finished if you provide the semaphore of the last queue submit. There is an exception if you would have used the SAME rendering semaphore for all queue submits, then the inner-race condition would define if you are lucky or not. I didn't check if the main viewport uses the same queue, if not it introduces an extra artefact cause if the other windows are on top of it, and this because exactly the same problem as explained above.

Alternatively, one could wait for ALL semaphores and present will only progress when all are signaled. Using the semaphores of ALL renderings (QueueSubmits()) will possibly solve things but your code will be seriously slowed down. That is because in that scenario no work can be done to present the already rendered viewports as long as the last one is not ready. Also keep in mind that Vulkan DOES NOT correlate the swapchain array with the semaphore array!!! There is no relation except the contract that when all semaphores signal, all swapchains are presented. Here also the main viewport needs to be considered.

Also keep in mind that in the docking branch the render and present (SwapBuffer) are different function callbacks. You'd have to build something to gain awareness about how many swapchains you are going to present at ones. Dear ImGui has a courtesy function allowing you to walk an array with all viewports and, IMO, if you would go that way then it is all or nothing. Clustering viewports in groups would introduce tremendous difficulty and don't forget there is also a main viewport!

As a note: Artefacts are cause by TWO, and ONLY TWO, things:

The second thing is the swapchain rebuild flag. If the flag is set for a single swapchain, you cannot include that swapchain in the presentation. But it gets worse.

The return value of the present vkQueuePresentKHR() can request a swapchain rebuild. if (err == VK_ERROR_OUT_OF_DATE_KHR || err == VK_SUBOPTIMAL_KHR) then the swapchain rebuild flag needs to be set. Unfortunately you will not know for which of the group of presented swapchains the error value has been returned. Hence, to be good you would have to rebuild them all. Again a loss of performance.

There is actually much more to say about all this, but the above give you an idea about what aspects need to be considered when trying the solve this within the chalk lines of the current backend design.