Skip to content
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

Multiple Viewports Mode Allows Multiple Windows to Have Focus #6299

Open
bedhededdy opened this issue Apr 4, 2023 · 9 comments
Open

Multiple Viewports Mode Allows Multiple Windows to Have Focus #6299

bedhededdy opened this issue Apr 4, 2023 · 9 comments

Comments

@bedhededdy
Copy link

Version/Branch of Dear ImGui:

Version: 1.89.4
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_sdl2.cpp + imgui_impl_opengl3.cpp
Compiler: Clang 15.0.1
Operating System: Windows 11

My Issue/Question:

I have an application where I would like certain keyboard shortcuts to do different things depending on which viewport is active. However, there is a scenario that arises where if I have a viewport spawned from the main OS window that has focus, and I either click the title bar or the main menu bar of the OS window, both windows have focus at once. If I click anywhere in the OS window besides these regions, the ImGui viewport correctly loses focus. Obviously this is an issue if I have a scenario where two windows share the same keyboard shortcut, as the code for both of them will execute.

Screenshots/Video

imgui_bug.mp4

Standalone, minimal, complete and verifiable example:

#include <SDL.h>
#include <SDL_opengl.h>

#include <imgui.h>
#include <imgui_impl_sdl2.h>
#include <imgui_impl_opengl3.h>

int main(int argc, char** argv) {
    SDL_Init(SDL_INIT_VIDEO);

    SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);

    SDL_Window* window = SDL_CreateWindow("Test", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 800, 800, SDL_WINDOW_OPENGL);
    SDL_GLContext gl_context = SDL_GL_CreateContext(window);

    ImGui::CreateContext();
    ImGuiIO& io = ImGui::GetIO();
    io.ConfigFlags |= ImGuiConfigFlags_ViewportsEnable;

    ImGui_ImplSDL2_InitForOpenGL(window, gl_context);
    ImGui_ImplOpenGL3_Init("#version 110");

    bool quit = false;
    bool show_win = false;
    while (!quit) {
        SDL_Event event;
        bool fpressed = false;
        while (SDL_PollEvent(&event)) {
            ImGui_ImplSDL2_ProcessEvent(&event);
            if (event.type == SDL_QUIT || (event.type == SDL_WINDOWEVENT && event.window.event == SDL_WINDOWEVENT_CLOSE)) {
                quit = true;
            } else if (event.type == SDL_KEYUP) {
                if (event.key.keysym.sym == SDLK_f)
                    fpressed = true;
            }
        }

        ImGui_ImplOpenGL3_NewFrame();
        ImGui_ImplSDL2_NewFrame(window);
        ImGui::NewFrame();

        if (ImGui::BeginMainMenuBar()) {
            if (ImGui::BeginMenu("Foo")) {
                if (ImGui::MenuItem("Open win"))
                    show_win = true;
                ImGui::EndMenu();
            }
            ImGui::EndMainMenuBar();
        }

        bool focused1 = false;
        if (show_win) {
            ImGui::Begin("Conf", &show_win);
            focused1 = ImGui::IsWindowFocused();
            ImGui::Text("This is dummy text");
            ImGui::End();
        }

        bool focused2 = SDL_GetWindowFlags(window) & SDL_WINDOW_INPUT_FOCUS;

        if (fpressed) {
            if (focused1)
                SDL_Log("Fpress 1");
            if (focused2)
                SDL_Log("Fpress 2");
        }

        glViewport(0, 0, (int)io.DisplaySize.x, (int)io.DisplaySize.y);
        glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
        glClear(GL_COLOR_BUFFER_BIT);
        ImGui::Render();
        ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData());

        if (io.ConfigFlags & ImGuiConfigFlags_ViewportsEnable) {
            ImGui::UpdatePlatformWindows();
            ImGui::RenderPlatformWindowsDefault();
            SDL_GL_MakeCurrent(window, gl_context);
        }

        SDL_GL_SwapWindow(window);
    }

    ImGui_ImplOpenGL3_Shutdown();
    ImGui_ImplSDL2_Shutdown();
    ImGui::DestroyContext();
    SDL_GL_DeleteContext(gl_context);
    SDL_DestroyWindow(window);
    SDL_Quit();

    return 0;
}
@ocornut
Copy link
Owner

ocornut commented Apr 4, 2023

both windows have focus at once.

What's happening is that one has Platform Focus and the other has Dear ImGui-side Focus, which are different things.

I guess part of your issue is that you'd want Platform Focus of your main host window to focus the underlying Dear ImGui window, which is something we are not doing yet. As that host window can contain multiple Dear ImGui windows it would involve finding the most recently focused one and focusing that one.

I have an application where I would like certain keyboard shortcuts to do different things depending on which viewport is active.

The whole premise seems incorrect, I believe you want to route shortcuts based on which Dear ImGui is focused.
Here your code is mashing two concepts together, but I do understand the lack of auto-focus-imgui-window-on-platform-focus may lead you to that.

Obviously this is an issue if I have a scenario where two windows share the same keyboard shortcut, as the code for both of them will execute.

I suggest you look at Shortcut() in imgui_internal.h which is a thing I committed a few months ago which aims to solve this.

You can find some demo code in this branch https://github.com/ocornut/imgui/commits/features/demo_input_owner_and_routing and particularly in fbc9395 but mostly read the comment around the Shortcut() declaration.

@bedhededdy
Copy link
Author

bedhededdy commented Apr 4, 2023

Thanks for the suggestion and the speedy reply! I sincerely appreciate it :)

I believe you want to route shortcuts based on which Dear ImGui is focused.

This is incorrect, although I should have been more clear in explaining my actual use case as opposed to the minimal example. In my actual use case, the host window is also allowed to have shortcuts. And since it's a game, it will also process some inputs that are not shortcuts (i.e. WASD for movement).

I suggest you look at Shortcut() in imgui_internal.h which is a thing I committed a few months ago which aims to solve this."

As such, I don't think this solution will work for me, as the host window is not an imgui window.

I guess part of your issue is that you'd want Platform Focus of your main host window to focus the underlying Dear ImGui window, which is something we are not doing yet.

That's not actually my issue with the behavior. My issue is that clicking the center of the host window will relinquish the imgui viewport's focus (desired behavior), but clicking the title bar or main menu bar of the host window will not relinquish the imgui viewport's focus (undesired behavior). My issue is the inconsistency in the behavior based on where I click on the window. Clicking on the host window should either clear the viewport's focus or not; it seems quite bizarre that clicking the title bar would not do this, but clicking elsewhere on the window does.

@ocornut
Copy link
Owner

ocornut commented Apr 4, 2023

What's in your platform host window exactly? A menu-bar + a decoration-less window showing an ImGui::Image() with a render-to-texture? Or is the game diplayed in the underlying screen-buffer before dear imgui renders?

My issue is that clicking the center of the host window will relinquish the imgui viewport's focus (desired behavior), but clicking the title bar or main menu bar of the host window will not relinquish the imgui viewport's focus (undesired behavior).

That seems to be the issue I'm referring to in my second paragraph. Clicking the platform window decoration has no effect on imgui logic presently. It should be fixable. It may not be trivial because we need to distinguish intentional platform window focus from the one happening when e.g. creating a new one.

Another manifestation of that same issue is, e.g.

  • Extract two secondary viewports.
  • Press ALT+TAB to switch between them using Windows facility.
  • Notice the dear imgui title-bar don't get focus.

@bedhededdy
Copy link
Author

What's in your platform host window exactly? A menu-bar + a decoration-less window showing an ImGui::Image() with a render-to-texture? Or is the game diplayed in the underlying screen-buffer before dear imgui renders?

The game is displayed in the underlying screen-buffer before dear imgui renders. The menu-bar is then drawn on top of that when dear imgui renders.

I think for now I can just work around the issue by doing a check for if any viewport has focus. If there is a viewport with focus, I will just assume the input is for the viewport instead of the host window even if both of them have focus. It's not ideal, but it should be fine until you're able to fix the issue. Thanks again for the help!

ocornut added a commit that referenced this issue Apr 5, 2023
ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Apr 5, 2023
@ocornut
Copy link
Owner

ocornut commented Apr 5, 2023

I have pushed support for this dcb6335
It seemed fairly straightforward but out of experience we will probably hear about this again.

And a test as well as some support in test-engine:
ocornut/imgui_test_engine@2b3fb61

(
I also noticed that while clicking on Platform Title Bar always sets focus (and thus makes Dear ImGui sets its focus too), ALT+Tab doesn't always and it depends on whether both windows have a Parent<>Child relation at Platform Level.

  • Win32: secondary viewports are child of main viewport unless ConfigViewportsNoDefaultParent=true;
  • GLFW: secondary viewports are not child of main viewport.
  • SDL: secondary viewports are not child of main viewport.

For avoidance of doubt, it is better and more consistent if secondary viewports are not child of main viewport, as that relationship enforce z-ordering at platform level, at least on Windows.

I think that adding a ImGuiBackendFlags_HasViewportParenting flag which win32 backend would set (since it is the only one that supports both configuration) we could set a better dynamic default.
)

I'd appreciate if you can update and confirm that this fixes things for you.

@bedhededdy
Copy link
Author

I see no change in behavior with regards to clicking on the window decorations in the minimum example I posted between the original version of Dear ImGui I was using and commit dcb6335.

For me, clicking on the menu bar in both cases refocuses the last ImGui viewport, clicking the center of the window clears ImGui focus, and clicking the title bar focuses the main window and keeps any previous ImGui viewport's focus.
What I observe seems to contradict what you observe here.

I also noticed that while clicking on Platform Title Bar always sets focus (and thus makes Dear ImGui sets its focus too)

What is the expected behavior in the above scenarios I listed? And where should differences in behavior be expected between the old and new version? I think maybe I am not fully understanding what you think the proper behavior should be.

I do now understand why the behavior exists in the way that it does, though. If the user is not using multiple viewports mode, it makes sense that merely clicking the title bar should not clear their ImGui focus. My original argument was that if a user is using multiple viewports mode, where each viewport is a platform window, it should exhibit the same behavior as normal desktop windows, where clicking anywhere on Window A will clear the focus of Window B.

I understand if this is not a change you want to make, as it would lead to a discrepancy in behavior between multiple viewports mode and normal mode. I also understand that it would be tricky (or maybe impossible) to handle a scenario where some of the viewports are within the main window (where they are treated as normal viewports to my understanding) and some are outside the main window (where they are treated as platform windows to my understanding).

Therefore, without major changes to the way that things work, at least the way I understand things, the anticipated behavior should be that clicking the center of the window should clear ImGui focus, and clicking the menu bar or the title bar should refocus the last ImGui viewport. However, on the new commit, I do not get an ImGui refocus when clicking the title bar.

@ocornut
Copy link
Owner

ocornut commented Apr 6, 2023

For me, clicking on the menu bar in both cases refocuses the last ImGui viewport, clicking the center of the window clears ImGui focus, and clicking the title bar focuses the main window and keeps any previous ImGui viewport's focus.
What I observe seems to contradict what you observe here.

As the subject matter can be extremely confusing/ambiguous, best to be extra clear and explicit with your wording.
Maybe the Glossary can help.

For me, clicking on the menu bar in both cases refocuses the last ImGui viewport,

This is ambiguous so I can't say if that's intended of not.

clicking the center of the window clears ImGui focus,

Assuming clicking the void/black in the center of the main viewport: yes it does on only on second click which is a new bug (see 2 below).

and clicking the title bar focuses the main window and keeps any previous ImGui viewport's focus.

That seems to be the bug I mentioned in (1) below.

(1) From what I understand, the issue has to do with BeginMainMenuBar()/EndMainMenuBar() which is itself programmed to relinquish focus to the previously focused window when not active. In this configuration it seems to be undoing what we're doing to do.

If you change things to e.g.

ImGui::Begin("test", NULL, ImGuiWindowFlags_MenuBar);
if (ImGui::BeginMenuBar()) {
    if (ImGui::BeginMenu("Foo")) {
        if (ImGui::MenuItem("Open win"))
            show_win = true;
        ImGui::EndMenu();
    }
    ImGui::EndMenuBar();
}
ImGui::End();

You'll observe different behavior.

(2) There's also an issue with my new code where if "Conf" is focused (both Platform and ImGui focus) and you click in the void/black section of the main viewport, my newly added code will focus whatever was last-focused in that viewport. Specificially when clicking on "void" it should unfocus imgui window. It works on the second click.

@ocornut ocornut added the bug label Apr 6, 2023
ocornut added a commit that referenced this issue Apr 6, 2023
…n was actionned from a click within imgui boundaries and (2) restore a null focus as well. (#6299)
@ocornut
Copy link
Owner

ocornut commented Apr 6, 2023

I have pushed fixes for (2) main commit 63370be (two other earlier commits with small refactor) + new tests ocornut/imgui_test_engine@0320aee

AFAIK things work better except in the case of clicking inside the main menu bar which is still a specific thing as it auto-relinquish focus. That logic exists intentionally so that e.g. with keyboard input you can ctrl+tab to main menu bar, activate an item and return when you are. As a result clicking the menu-menu bar tends to never visibly steal focus from another place at the moment.

Could you evaluate with this update and in light of understanding that main menu bar does that?

Thank you!

@bedhededdy
Copy link
Author

From what I understand, the issue has to do with BeginMainMenuBar()/EndMainMenuBar() which is itself programmed to relinquish focus to the previously focused window when not active

I observe this behavior

I have pushed fixes for (2)

Yup, clicking in the "void" space properly unfocuses the imgui window on the first click.

I am now also observing that clicking on the platform decoration correctly clears the imgui window focus when the imgui window is being treated as a platform window. It does not clear the imgui window focus when the imgui window is not a platform window, which again is the anticipated behavior.

Everything is working as you say it should now. I do observe the bug behavior for (1), but as you say, that is what I should be experiencing as of now. Let me know when you solve (1) so I can test that out as well.

Thanks for working so diligently on this!

ocornut added a commit that referenced this issue Apr 21, 2023
…ally closing modals (#6299, #6357)

+ Fixed double-assignment static analyzer warning.

# Conflicts:
#	imgui.cpp
ocornut added a commit that referenced this issue Apr 21, 2023
…ally closing modals (#6299, #6357)

+ Fixed double-assignment static analyzer warning.

# Conflicts:
#	imgui.cpp
ocornut added a commit that referenced this issue May 20, 2023
ocornut added a commit to ocornut/imgui_test_engine that referenced this issue May 20, 2023
ocornut added a commit that referenced this issue May 30, 2023
…#6462, #6299)

Avoid applying imgui-side focus when focus change is due to a viewport destruction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants