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

Shortcut(): RouteFocused behavior with docked parent windows #6798

Closed
Web-eWorks opened this issue Sep 8, 2023 · 5 comments
Closed

Shortcut(): RouteFocused behavior with docked parent windows #6798

Web-eWorks opened this issue Sep 8, 2023 · 5 comments

Comments

@Web-eWorks
Copy link

Web-eWorks commented Sep 8, 2023

Version/Branch of Dear ImGui:

Version: 1.89.8
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_sdl2.cpp + cursors + Pioneer renderer backend
Compiler: clang 15.0.7
Operating System: Solus Linux

My Issue/Question:

This is more of a "discussion" issue than a specific bug report, but it arises out of an unexpected behavior I found while implementing shortcut handling.

In my current editor architecture, I have "context" or "tool" top-level windows (which I expect to map 1:1 to Viewports when I get there) which host a DockSpace() and are responsible for submitting sub windows which should only dock inside that top-level window.

I currently only support one top-level window so I've implemented shortcuts via RouteGlobal to be able to use them from any part of the editor context window, but I'd like to support multiple context windows with different active shortcuts depending on which viewport or context window is currently focused. I attempted to do so via Shortcut(...ImGuiInputFlags_RouteFocused), and found that shortcuts are not available when interacting with a window docked inside of the window which defined the shortcuts.

I've verified the ParentWindow chain points all the way back to the window owning the dock space, and RootWindowDockTree is set as well, but the RootWindow for the docked window is null/unset.

I'm not sure if it semantically makes sense for the window that owns the dockspace to be part of the focused chain, but off the top of my head I'd expect it to be fairly intuitive for the user and the developer for shortcuts associated with a top-level window to be considered when a docked subwindow is currently focused.

The reason this is a discussion issue is that the proposed behavior still doesn't handle the case where the focused window is logically "owned" by a top-level "context" window, but has been undocked and is floating as a separate Viewport (e.g. on a second monitor). I think in that case I'll probably still have to register the "context" window shortcuts as RouteGlobal and determine whether to submit them based on which top-level context window was last focused.

I've been slowly poking around at the imgui_unreal_style_mdi.h example file linked in another issue, as my intended design is quite close to how UE4 handles their editor windows, but I don't believe that example has any shortcut handling at this current time.

I'd be happy to elucidate further on my current design/architecture to clarify, but I didn't want to write a full wall of text here.

Screenshots/Video

Ignore the Ctrl+Z / Ctrl+Shift+Z shortcuts, they're registered with RouteGlobal from the parent editor the demo is running in.

With parent window explicitly focused by clicking on the title bar:
image

With docked window focused via any of the "Focus Getter" buttons:
image

The docked window has a RootWindowDockTree pointing at the dockspace window, and the ParentWindow chain goes from the child window all the way to the owning window of the dockspace.

Standalone, minimal, complete and verifiable example:

if (ImGui::Begin("Test Window", nullptr, ImGuiWindowFlags_NoDocking)) {
	// _NoSplit optional, here to reduce ambiguity about the repro case
	ImGui::DockSpace(ImGui::GetID("Test Dockspace"), ImVec2(0, 0), ImGuiDockNodeFlags_NoSplit);

	if (ImGui::Shortcut(ImGuiKey_T | ImGuiKey_ModCtrl, 0, ImGuiInputFlags_RouteFocused))
		printf("Shortcut activated\n");
}
ImGui::End();

if (ImGui::Begin("Test Window _")) {
	ImGui::Button("Focus Getter");
	if (ImGui::BeginChild("Child Window")) {
		ImGui::Button("Focus Getter");
	}
	ImGui::EndChild();
}
ImGui::EndChild();
@ocornut ocornut added the inputs label Sep 8, 2023
@ocornut
Copy link
Owner

ocornut commented Sep 11, 2023

Hello,

Although this is still considered to be an undocumented and private API. I appreciate the feedback and advanced users trying to make use of it.

I'll need to investigate this later, and potentially attempt to integrate something in imgui_unreal_style_mdi (which now resides at #6487), but intuitively this seems like the way to go:

[...] has been undocked and is floating as a separate Viewport (e.g. on a second monitor). I think in that case I'll probably still have to register the "context" window shortcuts as RouteGlobal and determine whether to submit them based on which top-level context window was last focused.

Because whichever option we add to tie focus chain system with the docking tree, the case of document-owned floating windows will still be an issue, and that user-side solution would handle everything.

One idea I could think of to support it better on the library side. Perhaps we add a field in ImGuiWindowClass which can be used to create the missing connection for input routing, with perhaps a new dedicated ImGuiInputFlags_RouteXXX option to take advantage of that field. That would need further design.

@ocornut ocornut added this to the v1.91 milestone Jan 9, 2024
@ocornut
Copy link
Owner

ocornut commented Jan 11, 2024

I've verified the ParentWindow chain points all the way back to the window owning the dock space, and RootWindowDockTree is set as well, but the RootWindow for the docked window is null/unset.
I'm not sure if it semantically makes sense for the window that owns the dockspace to be part of the focused chain, but off the top of my head I'd expect it to be fairly intuitive for the user and the developer for shortcuts associated with a top-level window to be considered when a docked subwindow is currently focused.

I will be working on a more general-purpose solution for this but for now I have pushed 32a3c61 where RouteFocused behavior can cross through docked parent window, which is the commonly expected and low-hanging fruit baseline.

Now if you consider the possibility that said docked window may be undocked and moved e.g. outside into a standalone viewport, that link is gone and shortcuts won't work. I'm still figuring out the right design for expressing that link. Setting a custom ImGuiWindow* pointers e.g. ImGuiWindow* FocusRouteParent would have some advantages including the one of being simple because we have the storage and desired lifetime. And I would like whatever chain is used in routing computation to be naturally available/readable with e.g. IsWindowFocused() queries, so they would line up quite naturally.

But, it wouldn't align nicely with how I wanted PushFocusScope() to work, aka dissociating focus scopes from windows. It is currently possible (but undocumented and probably not used just yet) to use PushFocusScope() in a window so that Tabbing be limited and cycle within certain items (think: a node editor where each node has a focus scope where you can cycle through with Tab), and multiple multi-select in same window would rely on focus scope to...

So ideally the link would be to a focus scope, not to a window, but storing/maintaining/referring to that additional hierarchy is going to be hairy. So maybe we can mix things up by having transient focus scope data, but manual linking can only connect to the root focus scope of a window (so if you link a panel/viewport to a document, it is to the root focus scope of that document, via a ImGuiWindow* pointer, which is an acceptable constraint).

(Sorry if this is confusing to anyone but me - leaving myself notes :)

@Web-eWorks
Copy link
Author

So maybe we can mix things up by having transient focus scope data, but manual linking can only connect to the root focus scope of a window (so if you link a panel/viewport to a document, it is to the root focus scope of that document, via a ImGuiWindow* pointer, which is an acceptable constraint).

This seems an acceptable compromise on the face of it - I'd generally expect an application to be modeled such that document-global shortcuts which should be triggered when owned subwindows have active focus are defined at the root of the top-level "owning window" concerned with hosting the document itself.

If I understand the WIP FocusScope feature you mention here, it seems like something that would be more concerned with constraining "local focus"; each individual focus scope having a "parent window" pointer (for the case of windows submitted inside another Begin()/End() pair, maybe implicitly set to the surrounding window?) seems as though it's concerned with "global focus" or perhaps better thought of in terms of expressing the window hierarchy of the application.

ocornut added a commit that referenced this issue Jan 15, 2024
… own buffer. Fixed debug break in SetShortcutRouting(). (#6798, #2637, #456)
ocornut added a commit that referenced this issue Jan 16, 2024
…ndowForFocusRoute. Automatically set on child-window, manually configurable otherwise. (#6798, #2637, #456)
ocornut added a commit that referenced this issue Jan 16, 2024
… + ParentWindowForFocusRoute. (#6798, #2637, #456)

Amend d474836
Begin: tweak clearing of CurrentWindow as FocusWindow() relies on it now.
Addded SetWindowParentWindowForFocusRoute() helper.
ocornut added a commit that referenced this issue Jan 16, 2024
…ng SetWindowParentWindowForFocusRoute(). (#6798)

The revert doesn't look the same as 32a3c61 as since then we are baking focus roue into NavFocusScopePath().
ocornut added a commit that referenced this issue Jan 16, 2024
… facing version of SetWindowParentWindowForFocusRoute() (#6798, #2637, #456)
ocornut added a commit that referenced this issue Jan 16, 2024
…ure a dock node to automatically set ParentWindowForFocusRoute on its docked windows. (#6798, #2637, #456)
@ocornut
Copy link
Owner

ocornut commented Jan 16, 2024

See commits in the thread for what led to this, but basically:

(1)

  • Added ImGuiWindowClass::FocusRouteParentWindowId as a way to connect the focus route between a tool window to a parent document window, so that Shortcuts in the documents are routed when the tool is focused (regardless of whether the tool is docked or in a floating viewport, etc.) (Shortcut(): RouteFocused behavior with docked parent windows #6798)

The first one is what you would mostly use. In the MDI sample I simply used:

doc->ToolWindowsClass.FocusRouteParentWindowId = ImGui::GetCurrentWindow()->ID;        // Setup shortcut routing

Note that the underlying value is a window->ParentWindowForFocusRoute field which might also be set directly using SetWindowParentWindowForFocusRoute(). But because ImGuiWindowClass is conveniently used for many things it made sense to add a field in there, in spite of the extraneous indirection (window->window id->window).


(2)

The second one is a way to implement a convenient default for this:

I'm not sure if it semantically makes sense for the window that owns the dockspace to be part of the focused chain, but off the top of my head I'd expect it to be fairly intuitive for the user and the developer for shortcuts associated with a top-level window to be considered when a docked subwindow is currently focused.

I'm not sure either if it would be a correct default so I made it a DockSpace() flag.
This is less "complete" than the first option and won't link Tools to their parent Documents when undocked and into their own viewport, but it feels useful to have the option for completeness.

Normally this should be solved now.

@ocornut ocornut closed this as completed Jan 16, 2024
@Web-eWorks
Copy link
Author

I'm not sure either if it would be a correct default so I made it a DockSpace() flag.
This is less "complete" than the first option and won't link Tools to their parent Documents when undocked and into their own viewport, but it feels useful to have the option for completeness.

Falling back to the Unreal Engine analogy, this will definitely be useful in the case of a "root level" dockspace containing individual document window tabs (which themselves have constrained dockspaces only hosting tool windows from that specific document).

Thanks for taking the time to work on this - I'm looking forward to integrating this functionality into our codebase!

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