ocornut / imgui

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

When ImGuiConfigFlags_ViewportsEnable is set, ImGuiWindowFlags_NoBringToFrontOnFocus is ignored upon window creation #7008

Open jshofmann opened 1 year ago

jshofmann commented 1 year ago

Version/Branch of Dear ImGui:

Version: 1.89.9 Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_win32.cpp + engine-specific custom render backend Compiler: Visual Studio Professional 2022 17.7.6 Operating System: Windows 11 Pro 23H2

My Issue/Question:

In our gbuffer debugger, we draw a crosshair centered at the mouse location with a tooltip next to the crosshair displaying the pixel's properties. When drawing the crosshair, we set the ImGuiWindowFlags_NoBringToFrontOnFocus flag, among others, so that anything drawn under the crosshairs will receive input events. If the ImGuiConfigFlags_ViewportsEnable flag is set, then the NoBringToFrontOnFocus flag is effectively ignored, causing problems with controls that might be located under those crosshairs being ignored.

What happens is this: within ImGui::Begin(), ImGui::CreateNewWindow() is called, which reads the NoBringToFrontOnFocus flag and properly locates the new window in the ImGuiContext::Windows array. So far, so good. But, if ViewportsEnable is set, ImGui::Begin() then eventually calls ImGui::WindowSelectViewport(), which calls ImGui::UpdateTryMergeWindowIntoHostViewport(), which unconditionally calls ImGui::BringWindowToDisplayFront() as the last thing it does. This is causing the crosshair window to be moved to the back of the ImGuiContext::Windows array, which results in ImGui::FindHoveredWindow() returning the crosshair window instead of any other imgui window the crosshairs might be hovering over (like the GBuffer Debugger controls window). All input events are then tested against the crosshairs instead of the GBuffer Debugger controls or any other imgui window we have open.

Screenshots/Video

GBuffer Debugger

Standalone, minimal, complete and verifiable example:

A one-line change to ImGui::UpdateTryMergeWindowIntoHostViewport() fixes our issues with imgui controls when ImGuiWindowFlags_NoBringToFrontOnFocus is used:

imgui.cpp docking branch lines 14328 - 14329, in the ImGui::UpdateTryMergeWindowIntoHostViewport() function: if (!(window->Flags & ImGuiWindowFlags_NoBringToFrontOnFocus)) // FIX: Don't call BringWindowToDisplayFront() if NoBringToFrontOnFocus is set BringWindowToDisplayFront(window);

Is this a complete fix for this problem? It fixes our issues, and the unconditional nature of UpdateTryMergeWindowIntoHostViewport()'s call to BringWindowToDisplayFront() feels wrong to me. Thank you for producing one of the most useful pieces of middleware we have!

GamingMinds-DanielC commented 1 year ago

Now that you mentioned it, I noticed a very similar issue recently, but didn't analyze it further, I just took a closer look. Sorry for hijacking this issue, but it should be similar enough.

My issue is also in the docking branch. I have sticky menu items (menu stays open if Ctrl is pressed while clicking) for many things, very useful for toggling multiple options within a menu quickly. If I want to create a few editor windows, I navigate to the correct sub menu, hold Ctrl and click on every window that I want. I create my windows with the ImGuiWindowFlags_NoFocusOnAppearing flag if and only if Ctrl is pressed so that they don't move in front of the menu that stays open. Works great except for one case. If I open a window that is docked into another already opened window, turning that window into a dock node with then two tabs, that dock node is moved to the front. If I open another window docked into the same dock node (as a third tab), the problem doesn't occur, only on the second tab when the dock node is created.

In the docking branch (state of the current commit 0941adc) in static void ImGui::DockNodeUpdate(ImGuiDockNode* node) is another BringWindowToDisplayFront (in imgui.cpp line 16695). Skipping that call solves my issue, but I don't know at a glance how I can get the flags of the just appearing window that is responsible for creating the dock node.

The ideal way would be to not bring an appearing dock node to the front if the window that caused its appearance has ImGuiWindowFlags_NoFocusOnAppearing set. That flag currently has the effect of not selecting the tab for that window within the dock node, something I actually would like it to do. My proposal for this would be a new flag called f.e. ImGuiWindowFlags_NoSelectDockNodeTabOnAppearing for when that behavior is intended by the user. That flag would naturally prevent the focus as well in case the window will actually end up in a then unselected tab.

ocornut commented 1 year ago

In our gbuffer debugger, we draw a crosshair centered at the mouse location with a tooltip next to the crosshair displaying the pixel's properties. When drawing the crosshair, we set the ImGuiWindowFlags_NoBringToFrontOnFocus flag, among others, so that anything drawn under the crosshairs will receive input events

While I generally agree with the reasoning following that, I'm not sure I am understanding this part correctly. Is the crosshair its own window? Wouldn't using the _NoInputs flag on that window prevents the whole chain of problem from happening?

I will investigate your proposal separately regardless.

About Daniel's issue: seems like there's two steps for this. Making the dock node using _NoFocusOnAppearing if all just appearing windows share this flag, I think is a reasonable first move and would fix the biggest problem you have (focus stolen away from current menu), at the small cost of having the tab not selected. We can later think about a solution for the second part of the problem but window flags are limited and it doesn't seem like it would be worth allocating one for that.

GamingMinds-DanielC commented 1 year ago

Making the dock node using _NoFocusOnAppearing if all just appearing windows share this flag, I think is a reasonable first move and would fix the biggest problem you have (focus stolen away from current menu), at the small cost of having the tab not selected.

That would be great and would actually solve my problem entirely. I just experimented a bit, turns out the unselected tab is no problem after all. At least it is easy to remedy in a little helper function:

namespace ImGuiEx
{
    void SelectWindowInDockNodeTab( ImGuiWindow* wnd )
    {
        // see implementation of ImGui::FocusWindow for reference
        if ( wnd->DockIsActive )
        {
            ImGuiTabBar* tabBar = wnd->DockNode->TabBar;
            if ( tabBar != nullptr )
                tabBar->SelectedTabId = tabBar->NextSelectedTabId = wnd->TabId;
        }
    }
}

With all access to internals wrapped in helpers like this, collected in a small extension, potential changes to the library are no issue. If something breaks, I always have a comment for where to look for how to do it and can update accordingly. And it keeps hacks like this out of production code.

With ImGui::Begin() and ImGui::End() properly wrapped in my editor window base class I can then easily force the behavior I want without having to change individual windows.

jshofmann commented 1 year ago

While I generally agree with the reasoning following that, I'm not sure I am understanding this part correctly. Is the crosshair its own window? Wouldn't using the _NoInputs flag on that window prevents the whole chain of problem from happening?

The crosshairs are part of a window containing the crosshairs and the pixel properties tooltip, here's how they are drawn. I am setting ImGuiWindowFlags_NoNav on the crosshairs, if I add ImGuiWindowFlags_NoMouseInputs then the hover test that's used to display the pixel properties tooltip will fail due to the (window->Flags & ImGuiWindowFlags_NoMouseInputs) test in ImGui::FindHoveredWindow().

ImGuiWindowFlags windowFlags =
    ImGuiWindowFlags_NoBringToFrontOnFocus | ImGuiWindowFlags_NoFocusOnAppearing |
    ImGuiWindowFlags_NoDecoration | ImGuiWindowFlags_NoMove | ImGuiWindowFlags_NoResize |
    ImGuiWindowFlags_NoSavedSettings | ImGuiWindowFlags_NoNav | ImGuiWindowFlags_NoDocking;
if (ImGui::Begin("##PixelPickerCrosshair", nullptr, windowFlags))
{
    DrawCrosshairs(22.0f, 8.0f, 1.0f); // a few ImGui::AddRectFilled() calls added to ImGui::GetWindowDrawList()
    ImGui::InvisibleButton("canvas", ImVec2(22.0f, 22.0f));
    HoverTooltip( pixelPropertiesString ); // char* pixelPropertiesString
}
ImGui::End();
void HoverTooltip( const char* tooltip )
{
    if (ImGui::IsItemHovered())
    {
        ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(8, 8));
        ImGui::BeginTooltip();
        ImGui::PushTextWrapPos(250);
        ImGui::TextUnformatted(tooltip);
        ImGui::PopTextWrapPos();
        ImGui::EndTooltip();
        ImGui::PopStyleVar();
    }
}

I don't want to remove the hover test, it does what we want in the case where ImGuiConfigFlags_ViewportsEnable is not set. When ViewportsEnable is set, then the window display order in ImGuiContext::Windows is changed by the unconditional call to ImGui::BringWindowToDisplayFront() in ImGui::UpdateTryMergeWindowIntoHostViewport(). That causes the ImGui::IsItemHovered() test in HoverTooltip() above to fail. Gating the call to ImGui::BringWindowToDisplayFront() with a test of the ImGuiWindowFlags_NoBringToFrontOnFocus flag fixes the window display order and restores ImGui::IsItemHovered()'s behavior in the case where ViewportsEnable is set.