Viewports does not take parent viewport into consideration when attempting to merge to main viewport

[lnfera]

Version/Branch of Dear ImGui:

Version 1.92.5+, Branch: docking

Back-ends:

imgui_impl_dx12.cpp + imgui_impl_win32.cpp & custom engine

Compiler, OS:

MSVC 2022, Windows 11

Full config/build information:

Dear ImGui 1.92.6 WIP (19251)
--------------------------------

sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: _WIN32
define: _WIN64
define: _MSC_VER=1943
define: _MSVC_LANG=202002
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
IM_ASSERT: runs expression: OK. expand size: OK
--------------------------------

io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: imgui_impl_dx12
io.ConfigFlags: 0x00000483
 NavEnableKeyboard
 NavEnableGamepad
 DockingEnable
 ViewportsEnable
io.ConfigDpiScaleFonts
io.ConfigDpiScaleViewports
io.ConfigViewportsNoDecoration
io.ConfigViewportsNoDefaultParent
io.ConfigNavCaptureKeyboard
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00003C1E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
 HasParentViewport
 RendererHasVtxOffset
 RendererHasTextures
 RendererHasViewports
--------------------------------

io.Fonts: 3 fonts, Flags: 0x00000000, TexSize: 512,128
io.Fonts->FontLoaderName: stb_truetype
io.DisplaySize: 1898.00,1144.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------

style.WindowPadding: 12.00,12.00
style.WindowBorderSize: 1.00
style.FramePadding: 6.00,4.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 12.00,6.00
style.ItemInnerSpacing: 6.00,6.00

Details:

My Issue:

Hi
Currently a popup window can be merged into a viewport that is under its parent causing it to render under its parent. This happens when the popup is contained within the viewport that is suitable for merging but not its parent viewport.

Would it be possible add a check to see if the window trying to merge has a parent, and if that parent is linked to that same viewport that we are trying to merge into?

I tried the following to solve this:

// From 1.92.5 in imgui.cpp -> ImGui::UpdateTryMergeWindowIntoHostViewport()
@@ -16510,6 +16510,10 @@
         return false;
     if (GetWindowAlwaysWantOwnViewport(window))
         return false;
+    if( window->ParentWindow && window->ParentWindow->Viewport != viewport )
+        return false;

     for (ImGuiViewportP* viewport_2 : g.Viewports)
     {

Repro with DemoWindow:

1. Open DemoWindow
2. Move it to the side of the main viewport. ( See image )
3. Open the popup tab and adjust the window so that it is aligned like the image
4. Press the "With a menu" button.
5. See how the popup window has merged and is now rendering on the wrong viewport.

Screenshots/Video:

Minimal, Complete and Verifiable Example code:

// Minimal repro for implementation
if(ImGui::Begin("Example Bug"))
{
       if (ImGui::BeginCombo("##Combo", "PreviewVal"))
        {
            ImGui::Selectable("Foo");
            ImGui::Selectable("Bar");
            ImGui::Selectable("Baz");
            ImGui::EndCombo();
        }
}
ImGui::End();

[ocornut]

#9172, #9131, #9128 are probably the same and caused by #8948, the bug first appeared in e7aa0de, then still present in 6fd4cf8.

[ocornut]

I pushed commits to fix this:

(1) 7038e71: Viewports: renamed local to make algorithm easier to follow + comments. Should be no-op.

(2) 9eebd37: Viewports: update LastFocusedStampCount earlier. Fix (#8948, #9172, #9131, #9128) but probably have other side effects too. (Amend f1ae07e. Initial value of LastFocusedStampCount was set at the time of platform ShowWindow which is too late for first-frame UpdateTryMergeWindowIntoHostViewport() evaluation.)

(3) c389a95: Viewports: reworked IsViewportAbove() test as an alternate fix for #8948, #9172, #9131, #9128.

I tested both with and without backend supporting ImGuiBackendFlags_HasParentViewport.

Second commit likely has other subtle side effects - likely positive - which I can't identify precisely right now but I'm going to follow that trail a little more and investigate other viewport/focus/depth related issues.

[lnfera]

I tested with the latest changes and it works.
Thank you so much!