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

Focus and z-order are not restored on startup or when when multiple windows reappear at the same time #2304

Open
axd2201 opened this issue Jan 21, 2019 · 15 comments
Labels
focus settings .ini persistance

Comments

@axd2201
Copy link

axd2201 commented Jan 21, 2019

Version/Branch of Dear ImGui:

Version: Latest
Branch: Docking

Back-end/Renderer/Compiler/OS

Back-ends: Directx9 Renderer
Operating System: Windows 10

My Issue/Question:

Short version: Does not remenber the selected tab when stopped being rendered and starts again.

I have multiple tabs inside a single window, let's call them A B and C, if i open menu A is selected, i select B and close, when i open it again it opens with A as being selected/open instead of B.

Video Demonstrating the behaviour tried to be explained above:
https://streamable.com/qat2n

Source Code:
https://hastebin.com/kezosenoja.cpp

Extra Question
Well this is kinda of an extra question, that i searched so much and couldnt find anywhere, is the any way to create many Windows, and dock/tab them to a single window programatically?

@axd2201 axd2201 changed the title Docking specific example Tab selected index is not being saved Jan 21, 2019
@ocornut
Copy link
Owner

ocornut commented Jan 21, 2019

I have multiple tabs inside a single window, let's call them A B and C, if i open menu A is selected, i select B and close, when i open it again it opens with A as being selected/open instead of B

This is ambiguous, pleas provide a repro.

@axd2201
Copy link
Author

axd2201 commented Jan 22, 2019

@ocornut Hello, sorry for the bad explanation and lack of Demo and Source Code, i was on short time when reporting this, i hope with the extra info that i added to the issue it self will help you figure out the issue, will try to do a better job in the future, if you have any issues or want my help just tell me will be glad to help, thanks for your time have a nice day.

@ocornut
Copy link
Owner

ocornut commented Jan 22, 2019

sorry for the bad explanation and lack of Demo and Source Code, i was on short time when reporting this

I'm providing a service to the community and there are 2304 issues on this tracker. The least you can do when asking for help is to provide me with the information I am requesting. Thank you.

There are various possible ambiguity - I can't tell from your description if you are you using explicit tab bars, or windows that you docked/merged together. How do you hide and restore them etc? All those questions have an effect on the code flow. The code and gif allows to lift this ambiguity.

So - there is indeeed a bug with the current code which I can repro using:

static bool toggle = false;
ImGui::Checkbox("toggle", &toggle);
if (toggle)
{
    ImGui::Begin("AAAA");
    ImGui::Text("Hello from A window!");
    if (ImGui::Button("Close Me")) {}
    ImGui::End();

    ImGui::Begin("BBBB");
    ImGui::Text("Hello from B window!");
    if (ImGui::Button("Close Me")) {}
    ImGui::End();

    ImGui::Begin("CCCC");
    ImGui::Text("Hello from C window!");
    if (ImGui::Button("Close Me")) {}
    ImGui::End();
}

Then selecting e.g BBBB and clearing the toggle back and forth. Selection goes to CCCC.

Variation of this bug have been coming up regularly. I'm looking at this and I'll try to add dedicated tests in my testing system when I find a solution for it.

Well this is kinda of an extra question, that i searched so much and couldnt find anywhere, is the any way to create many Windows, and dock/tab them to a single window programatically?

This is discussed in the docking thread. It's not trivially faisible at the moment, but there's a WIP internal DockBuilder API which is not easy to use and going to evolve.

@ocornut ocornut changed the title Tab selected index is not being saved Relative focus ordering is not preserved when multiple windows reappear Jan 22, 2019
@ocornut
Copy link
Owner

ocornut commented Jan 22, 2019

FYI this issue isn't actually what I thought it was - it's not related to Docking in particular, but simply to the fact that when multiple windows are re-opened on the same frame they all reclaim the focus in order of submission.

So if you do e.g.

Frame N+0

  • submit A (takes focus)
  • submit B (takes focus)
  • submit C (takes focus)

Frame N+100

  • focus B

Frame N+200

  • stop submitting them for a brief moment

Frame N+300

  • submit A (takes focus)
  • submit B (takes focus)
  • submit C (takes focus)

Then at the end C is focused again.
This is regardless of any docking consideration. In this situation the Docking tab bar just follows the "currently focused" window.

I'd like to eventually enable for remembering the respeective z-order and focus-order of windows so multiple windows re-enabled in the same frame can restore their relative order. This is not a trivial change so I don't know when I'll be able to do it.

@ocornut ocornut changed the title Relative focus ordering is not preserved when multiple windows reappear Relative focus ordering is not preserved when multiple windows reappear at the same time Jan 22, 2019
ocornut added a commit that referenced this issue Mar 28, 2019
…g submitted. (#2109) Building on a little build of technical debt there, should transition toward a more general docking-agnostic system (#2304)
@ocornut ocornut added the settings .ini persistance label Mar 9, 2020
@ocornut ocornut changed the title Relative focus ordering is not preserved when multiple windows reappear at the same time Focus and z-order are not restored on startup or when when multiple windows reappear at the same time Mar 9, 2020
@nem0
Copy link
Contributor

nem0 commented Feb 5, 2021

I have a simple hack that kind of fixes this for docking - specifically the part where active tab is restored from imgui.ini. This is not in any way a correct fix for the general focus issue. Maybe somebody will find this useful.

restore_dock.zip it's a patch file made on 22d9a61

@ocornut ocornut added this to the v1.82 milestone Feb 11, 2021
@ocornut
Copy link
Owner

ocornut commented Feb 24, 2021

I looked at this and found two bizarre things:

  • Can't get your patch to work in my test case (which is submitting 4 windows, then I dock them, focus one, shutdown/reopen app). I'm sure it does work in your situation but it shows how there's quite some complexity to that system.
  • Very bizarrely I realize we DO have code saving this info in .ini file and attempting to restoring it. I honestly forgot this code even existed, it's seeing your patch that reminded me that data was saved. I don't think the restoring worked (or in only some cases) for a while.

I'm going to have to do some digging, design a battery of tests and see if we can fix this properly for docking or even for regular windows.

(EDIT To clarify about the "can't get your patch to work in my test case" I'm not suggesting to fix it. The fact that it doesn't work here is enough of an indication I need to go thoroughly through it and clean up the technical debt in focus authority/sync code)

@nem0
Copy link
Contributor

nem0 commented Feb 24, 2021

After I posted the patch file, I found some issues with it. I'm running slightly changed version https://github.com/nem0/LumixEngine/blob/master/external/imgui/imgui.cpp#L13452 for 20 days, no issues with it so far. New version does not use LastFrameActive to detect if DockNodeUpdateTabBar is being called for the first time.

@ocornut
Copy link
Owner

ocornut commented Mar 23, 2021

Just to confirm that we haven't forgot about that.

nem0: I was hesitant to pull your patch as-is because the "first time" thing leads to further inconsistencies when hiding and reenabling dockspaces multiple times during the session, and we have a few users who do this regularly. Right now I'm waiting on a new series of regression tests to be written related to focus and tab sync as there is a bunch of code smell in there, and we then will tackle and clean all of this. Thanks for your patience.

@nem0
Copy link
Contributor

nem0 commented Mar 23, 2021

No worries. I did not submit PR because I expected it not to be final/correct solution. I still have focus issues for "not the first time" cases, e.g. after minimizing and restoring window. They are not as annoying though and I'm pretty happy with current state.

ocornut added a commit that referenced this issue Apr 16, 2021
… windows in WindowsFocusOrder[] array. (toward #2304)
@ocornut
Copy link
Owner

ocornut commented May 24, 2021

Duke Nukem Forever release update: I was about to push a fix for this last month then stumbled on a severe flaw/side-effect and had to put it on hold. Been on and off continuously following the thread of making the right fix for that. In fact, the variety of seemingly random Navigation fixes in 1.83 are direct side-effects of following that thread (basically the non-problematic fix involve processing focus of reappearing windows at end of frame and there are a few stuff left which have remaining dependencies in Begin()).

@nem0
Copy link
Contributor

nem0 commented Jul 23, 2023

Since my old hack does not work anymore, here's a new one in case someone finds this useful
dock_restore_active.patch vs f8f805f

@ocornut
Copy link
Owner

ocornut commented Sep 18, 2023

Since my old hack does not work anymore, here's a new one in case someone finds this useful

For the records, this appears to fix "most" of the problem for a typical use case of "application with dockspace", but only effectively work on nodes that don't have focus by altering the tab bar selection.
Focus is not altered and so in a simple case of just adding Begin("A") and Begin("B") docked together, this won't work as either will claim focus and workaround will fail.

But very interestingly, with this, it becomes much easier to workaround the issue for the user, as submitting your "main" (e.g. game viewport) last or force-focusing it, the problem remains only on that single node which is much less likely to have multiple windows docked. And effectively you get to see all the others panels with the right tab selected.

In order words, this technically doesn't fix the problem stated in subject line ("Focus and z-order are not restored") but it can fix what matters the most to docking users (visible panels on restore). Lacking what I think is the correct-solution right now (which requires a bigger refactor) I'm going to work on something simpler based on this idea.

(PS: missing TabIdFromSettings = 0 initialization in ImGuiDockNode().)

ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Sep 20, 2023
ocornut added a commit that referenced this issue Sep 20, 2023
…tab in dock nodes that don't carry the currently focused window. (#2304)

In TestSuite: see "docking_tab_focus_restore".
Remove old code ~ ed3c015 8bac6d4 8cac70d
+ Fix potential crash in IMGUI_DEBUG_LOG_DOCKING() path when using amended buttons.
@ocornut
Copy link
Owner

ocornut commented Sep 20, 2023

I have pushed 61acb34 which afaik should lead to the same result as your patch nem0 afaik non-focused node should correctly restore selected tabs. (focused nodes still wont).

The bulk of this topic is still open but this is a good quality of life improvement in the meanwhile.

And reworked a test bed for that ocornut/imgui_test_engine@e4ea74f

@nem0
Copy link
Contributor

nem0 commented Sep 20, 2023

I tested 61acb34, it seems to help but does not completely fix my issue (neither does my patch). Some nodes are restored correctly and some are not.

@ocornut
Copy link
Owner

ocornut commented Sep 20, 2023

The ones that are restored correctly are the one which are the time of display do NOT host the focused window.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus settings .ini persistance
Projects
None yet
Development

No branches or pull requests

3 participants