ocornut / imgui

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

ImGui::SetWindowFocus() doesn't set OS-level focus on viewport #2605

Open cribalik opened 5 years ago

cribalik commented 5 years ago

Version/Branch of Dear ImGui:

Version: 171WIP (17003) (commit 596d81a973237c17008a4b64c72fa08bd380b79e) Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: custom Compiler: Visual Studio 2017 Operating System: Windows

My Issue/Question:

If the user calls ImGui::SetWindowFocus(), the intended behavior is (i assume?):

Currently though, Platform_SetWindowFocus() is only called from inside NavUpdateWindowing().

I am not very familiar with the internals, but moving the call to inside ImGui::FocusWindow() instead seems to fix the issue.

Standalone, minimal, complete and verifiable example:

ImGui::Begin("Example Bug");
ImGui::SetWindowFocus();
// The viewport containing the window doesn't get OS level focus; 
// PlatformIO.Platform_SetWindowFocus() is never called
cribalik commented 5 years ago

Currently we're running with the temporary fix of moving the Platform_SetWindowFocus() call inside ImGui::FocusWindow().

I'll get a PR up today or tomorrow after we've battle-tested it a bit :)

bsviglo commented 4 years ago

We've seen the same issue right now. Is there any plans to apply this PR on docking branch?

cribalik commented 4 years ago

If memory serves there were a few edge cases that this fix didn't solve, so wasn't confident in putting up a PR. If ocornut is happy with the fix as it is though, I'm okay with that.

ldsPatrick commented 3 years ago

Has there been any further movement on this issue. We just hit the same problem, where we want to bring to the front an already existing window (We're using the Docking branch), and the lack of OS callback leaves that window potentially hidden behind another. I added

if (g.PlatformIO.Platform_SetWindowFocus)
    g.PlatformIO.Platform_SetWindowFocus(display_front_window->Viewport);

At the bottom of ImGui::FocusWindow() and it resolves the issue, but it would be better to have an official solution.

dougbinks commented 3 years ago

My solution for this was to alter BringWindowToDisplayFront, as this respects the ImGuiWindowFlags_NoBringToFrontOnFocus flag:

https://github.com/dougbinks/imgui/commit/b97e4e3c82ac56e13944acce9a8c57b391620ff7

dougbinks commented 3 years ago

I've pushed a fix to the above as the viewport needs to be checked for NULL (edit:) and that PlatformWindowCreated is true.

    if (g.PlatformIO.Platform_SetWindowFocus && window->Viewport && window->Viewport->PlatformWindowCreated )
        g.PlatformIO.Platform_SetWindowFocus(window->Viewport);
dougbinks commented 3 years ago

I recently found that my fix above doesn't always work as the early out in BringWindowToDisplayFront was bypassing the Platform_SetWindowFocus if the window was the current ImGui front window. Fixed in: https://github.com/dougbinks/imgui/commit/e24c3978ef334b1708a821aef49fb657f2126e37

dougbinks commented 2 years ago

I've recently found that my implementation is the cause of the the mouse focus being lost when a window is pulled outside the main window, so a solution is to only set platform focus when explicitly called by the user through SetWindowFocus or SetNextWindowFocus.

I've implemented that in https://github.com/dougbinks/imgui/commit/7e03bd4fd9ca6f63848b9f5f84accd1a9a390c83 and this seems pretty smooth now, so I'll consider submitting this as a PR once I've done some more testing.

matrefeytontias commented 2 years ago

Bump, once again I ran into the same issue. Any news on the PR ?

dougbinks commented 1 year ago

@matrefeytontias does my branch docking_display_front or the commit https://github.com/dougbinks/imgui/commit/7e03bd4fd9ca6f63848b9f5f84accd1a9a390c83 resolve your problems? If so I will look into submitting this as a PR.