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

Docking: Odd behavior with docked/tabbed windows and ImGuiCond_Appearing #2283

Open
zzzyap opened this issue Jan 15, 2019 · 5 comments
Open

Comments

@zzzyap
Copy link

zzzyap commented Jan 15, 2019

Regarding the docking branch.

Reporting an odd behavior that occurs as follow:

  • Tabbed window position is set via SetNextWindowPos with condition ImGuiCond_Appearing
  • Tab is clicked on
  • It goes from hidden/inactive to visible/active so “appearing” is triggered
  • Window automatically un-tabs and moves the window to the respective position.

20190115_56

This behavior kind of makes sense logically.
However automatically un-tabbing is undesirable (and looks like a bug) since the user did not untab themselves.

Visually a tabbed window is partially hidden and partially inactive, such that the user can still see and interact with the tab.
I would expect the tabbed window to do nothing in this case

Minor note: also slightly breaks symmetry with this similar behavior.
20190115_58

Thoughts?

@ocornut
Copy link
Owner

ocornut commented Jan 15, 2019

You are correct this is an issue.

This might be a little tricky because _Appearing has other uses, but all the uses from the currently exposed functions in imgui.h taking an ImGuiCond would probably need to rely on the "fixed" definition.

Other cases to consider are IsWindowAppearing(), SetItemDefaultFocus(),
(Tooltip and popups rely on another definition but won't be affected since they cannot be docked.)

I will look further into this.

@ocornut ocornut changed the title Odd behavior with tabbed windows and ImGuiCond_Appearing (docking) Docking: Odd behavior with docked/tabbed windows and ImGuiCond_Appearing Jan 16, 2019
@ocornut
Copy link
Owner

ocornut commented Jan 16, 2019

Could you somehow describe the context in which you are using SetNextWindowPos(pos, ImGuiCond_Appearing) ? As opposed to using e.g. ImGuiCond_Once or ImGuiCond_FistTimeEver ?

However automatically un-tabbing is undesirable (and looks like a bug) since the user did not untab themselves.

I'm not sure there - SetNextWindowPos is an explicit request from the code and it seems fair to undock the window to ensure it can reaches its target.

I can see four possibilities of how to handle that:
A) Set position and undock if docked
B) Ignore if docked
C) Move the dock node associated to the window (along with other windows docked in same location) and potentially undock this dock node if it is itself docked somewhere.
D) Move the dock node associated to the window (along with other windows docked in same location) while preserving the docking hierarchy, so all other nodes/windows including parents and sibling are moved relatively to the reference window and its new position.

Right now we are doing A and you are suggesting B. I think B would get in the way of many genuine operations. But we need to find out the circumstance in which SetNextWindowPos() is used other than to set an easy default. Perhaps SetNextWindowPos might need some flags to select behavior?

If there was a way to do IsWindowDocked() given a window name/handle (*) that would also reduce the need for B because the user could do if (!IsWindowDocked()) SetWindowPos().
(*) I would like to reorganize all the SetXXXWindowXXX calls to be able to take arbitrary window handles specified as "current" "next" "by name" "by id" etc.

@zzzyap
Copy link
Author

zzzyap commented Jan 16, 2019

Internally we have switched all ImGuiCond_Appearing to ImGuiCond_FistTimeEver to avoid this. Prior to noticing this, the use case was opening up a window in a preset position every time it opens. The user can chose to move the window and dock it somewhere if they want their own layout which persist. If a user chooses not to use docking the window should open at the preset.

It a bit confusing since docked and/or tabbed windows are visually a merged together window.
So this gets mixed up in that, since it's actually setting window position for what is now a visually a sub window (kind of). Exposing IsWindowDocked() or IsWindowFloating() would make sense, the user could now figure out the state of their window and do what is most appropriate.

It may be possible that A, B, and D are all genuine operations. This could be separated out as other ImGuiCond conditions, alternate Set*Pos() call, or as a separate flag to avoid confusion.
As a flag I would imagine something to determine sibling influence of the Set call.
So these can be phrased as:

  • None (A) Sets window position regardless of dock state.
  • Single (B) Set window position if undocked (a single window) thus respecting the dock state.
  • Multiple (D) Set top most floating window position, moving all docked and tabbed windows. If undocked logically the window is treated as the top most floating window.

The more I think about this, these behaviors are very open ended and exposing the windows dock state might be the cleanest option.

EDIT:
Thinking about it even more, C and D actually will cause problems.
Say both windows that are docked together both want to set position...
Now we will have a race condition for position and dancing windows.

@ocornut
Copy link
Owner

ocornut commented Jan 16, 2019

IsWindowDocked() is already exposed but it applies on the current window, so we are past the Begin() call and unable to use SetNextWindowPos().

Thinking about it even more, C and D actually will cause problems.
Say both windows that are docked together both want to set position...
Now we will have a race condition for position and dancing windows.

Yes that's true.

Interestingly, ImGuiCond is already using power-of-two values. This is merely an old artefact of how the values were used internally, and I was expecting to re-linearize the constants and enforce that it is an enum. However, instead we could add extra condition flags, e.g.

SetNextWindowPos(...., ImGuiCond_Appearing | ImGuiCond_Undocked)

EDIT:
One issue is that the type and prefix is ImGuiCond_ whereas to be fully consistent it should be ImGuiCondFlags_ but we could make an exception here.

( On the possibility of adding a variant of IsWindowDocked() to query a named window and following my (*) paragraph in the previous post: I drafted a matrix of which of Get/Set calls could ideally be reworked to support CurrentWindow vs NextWindow vs AnyNamedOrPointedWindow and realized we are not far off from what we need, because many combination don't make sense (e.g. GetWindowXXX calls can not work on Next, only work on Current and Any and the later would expose the user to more ordering/shearing issues, so not desirable). As a result I am inclined to have less resistance to the idea of adding an extra-entry point for IsWindowDocked() to be called on a named window. )

@zzzyap
Copy link
Author

zzzyap commented Jan 17, 2019

Regarding IsWindowDocked(), something as follows:

if(ImGui::IsWindowDocked("My Window"))
    ImGui::SetNextWindowPos(ImVec2(100.f, 100.f), ImGuiCond_Appearing);

ImGui::Begin("My Window");

The only problem would be the first query, where the Begin(...) has not been reached yet. Queries to any window by name would also need to account for if that window exists.

SetNextWindowPos(...., ImGuiCond_Appearing | ImGuiCond_Undocked)

Flags as such could be nice but may scale badly.
Example, other combos that sound like they should work ImGuiCond_Once | ImGuiCond_Appearing.

The condition flags could have two sets of meanings event state (once, always, appearing) and window state (docked, undocked). Personally, to avoid unintentional combos I would split the two.

// reads as set pos if appearing and undocked
SetNextWindowPos(..., ImGuiCond_Appearing, ImGuiWindowCond_Undocked) 

// "ImGuiWindowCond" defaults to none/ always (so ignores window state)
// reads as set pos if appearing
SetNextWindowPos(..., ImGuiCond_Appearing) 

However, both options sound good.

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