Selected tab disappears when manager's update is done on idle event

[Kinaou]

• Apply the following modifications on the auidemo.cpp file then run the demo
• select 'View' > 'Add Panes' => 'Added pane 6' is selected and visible (tabs are right aligned)
• select 'View' > 'Refresh on idle' => 'Added pane 6' is always selected but no more visible (tabs are left aligned)
• move the mouse cursor on the left arrow of the tabs control => the tabs are right aligned anew

@mjmacleod, do you have an idea why the refresh on idle event looses the alignment of the tabs?

@@ -65,0 +66,2 @@
+        ID_AddPanes,
+        ID_RefreshIdle,
@@ -141,0 +144,3 @@
+    void OnAddPanes(wxCommandEvent& WXUNUSED(event));
+    void OnRefreshOnIdle(wxCommandEvent& WXUNUSED(event));
+    void DoRefreshOnIdle(wxIdleEvent& WXUNUSED(event));
@@ -581,0 +587,2 @@
+    EVT_MENU(MyFrame::ID_AddPanes, MyFrame::OnAddPanes)
+    EVT_MENU(MyFrame::ID_RefreshIdle, MyFrame::OnRefreshOnIdle)
@@ -659,0 +667,2 @@
+    view_menu->Append(ID_AddPanes, _("Add Panes"));
+    view_menu->Append(ID_RefreshIdle, _("Refresh on idle"));
@@ -1093,0 +1103,25 @@
+}
+
+void MyFrame::OnAddPanes(wxCommandEvent& WXUNUSED(event))
+{
+    static int id = 1;
+    wxHtmlWindow *ctrl = NULL;
+    for (; id < 7; ++id) {
+        ctrl = CreateHTMLCtrl();
+        m_mgr.AddPane(ctrl, wxAuiPaneInfo().Center().
+                    Name(wxString::Format("Pane%i", id)).Caption(wxString::Format("Added pane %i", id)).
+                    CloseButton(true).MaximizeButton(true).Page(id+2).Floatable(true).Dockable(true).CenterDockable(true));
+        m_mgr.Update();
+    }
+    if (ctrl)
+        m_mgr.SetActivePane(ctrl);
+    m_mgr.Update();
+}
+void MyFrame::OnRefreshOnIdle(wxCommandEvent& WXUNUSED(event))
+{
+    Bind(wxEVT_IDLE, &MyFrame::DoRefreshOnIdle, this);
+}
+void MyFrame::DoRefreshOnIdle(wxIdleEvent& WXUNUSED(event))
+{
+    m_mgr.Update();
+    Unbind(wxEVT_IDLE, &MyFrame::DoRefreshOnIdle, this);

[mjmacleod]

Not entirely sure, is it possibly related to my other comment in one of the other bug reports where a similar thing was happening https://github.com/nhold/wxWidgets/issues/76#issuecomment-128141407 ?

[mjmacleod]

Or is it maybe throwing the 'cached' tab containers away and recreating them for each update? I have no idea why the idle event would be special here, if you e.g. make a button and call update each time the button is pressed does the same thing happen?

[Kinaou]

Well done! Commenting the part of code you highlighted in https://github.com/nhold/wxWidgets/issues/76#issuecomment-128141407 resolves this issue :dizzy_face:

[mjmacleod]

Okay interesting, the only problem now is I don't 100% fully understand what that codes original intention was (only that it seems problematic to me) so I don't know what we break if we remove it :(

[Kinaou]

if you e.g. make a button and call update each time the button is pressed does the same thing happen?

Yes. Therefore is it not specific to the idle event.

Or is it maybe throwing the 'cached' tab containers away and recreating them for each update?

AFAICS, the tabs are recreated at each update of the manager: wxAuiManager::Update() -> Repaint() -> Render(dc) -> wxAuiTabContainer::Render()

[mjmacleod]

Can you maybe check if wxAuiTabContainer constructor gets called for each update (unexpected) or if it only gets called the first time? If it gets called for each update then thats probably the issue, otherwise its the code above from #76 that needs closed inspection.

[Kinaou]

check if wxAuiTabContainer constructor gets called for each update

Yes, wxAuiTabContainer ctor is called at each update: wxAuiManager::Update() -> LayoutAll() -> LayoutAddDock() -> notebookContainer = new wxAuiTabContainer(m_tab_art,this);

[mjmacleod]

Hrm, I had in my mind that the code keeps a 'cache' of wxAuiTabContainer's and only reconstructs them if required otherwise rather reuses them. I see that isn't the case. (I don't know if it was the case but changed at some point or if my memory is just fuzzy now)

I guess that means that things like scrolling will reset every time Layout is called which is not always ideal, though of course layout shouldn't be called for no reason either it should only be called when there actually are changes to the layout.

So I guess there are a few issues to consider here: 1) Is it really necessary to call Layout() for every single Update() call? If we could avoid this it would probably solve the issue and have other performance benefits, perhaps this is where some of the 'slowness' complaints of some users stem from. Does the old AUI call Layout() inside Update() as well? If not then why are we, and when did we start doing this?

2) If it is desirable for Update to also call Layout then I suppose there should be some kind of 'dirty' flag so that it only calls Layout if there are actually layout changes, and not for every single Update. The problem I guess is that the panes can be modified at any time without us knowing via member variable access, if we could restrict member variable access to member functions we could set a dirty flag to detect changes - however we can't do that because then we are 'breaking backwards compatibility...

3) Is it really necessary to reconstruct the tab containers for every layout, ideally we should be able to reuse the existing ones in most cases, however I suppose it will complicate the code quite a bit to do this correctly. Reusing tab containers would help prevent any issues with positions being lost when layout calls are made etc. Also sometimes (e.g. when restoring perspectives) it is desired that everything including position resets so in those cases the wxAuiTabContainers should not be reused - I'm not sure there is (currently) a way to distinguish between when it is desirable to lose this information and when it is not?

[mjmacleod]

I see we do cache the tab positions of the tab containers at least - that must have been what I was thinking about...

'''' // cache the offset positions for any notebooks we have, so that if we are just resizing a dock for example our notebook tabs don't jump around. NotebookOffsetHash cachednotebookoffsets; // cache the notebook that has the current focus so that it can keep the focus when doing a resize for example. wxString focusnotebook = wxT(""); int uiPartsCount = m_uiParts.GetCount(); ''''

So maybe the first thing to figure out is why this isn't working in this instance then?

[Kinaou]

Cached data seem to work as expected and the behavior of the old aui seems preserved as regards the updating operations. The performance concerns should probably be addressed in the future to avoid breaking too much things for now :innocent:.

I studied the interest of the part of code https://github.com/nhold/wxWidgets/issues/76#issuecomment-128141407 and it is useful for readjusting dynamically the position of the tabs when the notebook is resized, to allow seeing a maximum of tabs inside the visible area of the tabs container.

m_tabOffset is the index of the first visible tab. And working directly on the attribute m_tabOffset allows to reset the position of the first visible tab dynamically during the resizing (as it was done before the modifications https://github.com/mjmacleod/wxWidgets/commit/b9406eee0b92aecd1f39a0ca77f2f65e78adf0c1#diff-f74530d83e0cc6ec05ebd11bb2884c01).

To illustrate a bit, suppose a notebook that is reduced and can display 3 tabs out of 6, the sixth tab being activated, the 'tab offset' is set to 3 and the fourth to sixth tabs are visible (the first to third are hidden).

3 cases:

• Case 1: the code https://github.com/nhold/wxWidgets/issues/76#issuecomment-128141407 is commented
• Case 2 : the code https://github.com/nhold/wxWidgets/issues/76#issuecomment-128141407 is kept (m_tabOffset attribute modified directly)
• Case 3 : the code is modified such as https://github.com/mjmacleod/wxWidgets/commit/b9406eee0b92aecd1f39a0ca77f2f65e78adf0c1#diff-f74530d83e0cc6ec05ebd11bb2884c01

Now, when the notebook is enlarged:

• Case 1: the hidden tabs stay hidden and the tabs container's background is just enlarged. The side arrows must be used to display the hidden tabs.
• Case 2&3: the hidden tabs are redisplayed one by one until seeing all the tabs.

Then, when the notebook is reduced anew:

• Case 1: the tabs container's background is reduced and the tabs do not move
• Case 2: the first tabs stay visible (the 'tab offset' has been reset) and the last tabs are hidden one by one
• Case 3: the first tabs are hidden one by one until encountering the 'tab offset', then the last tabs are hidden one by one until encountering the min size of the pane

Not sure to know which behavior is the most natural between the case 2 and the case 3?

Getting back to my issue, for redefining the 'tab offset' in the code https://github.com/nhold/wxWidgets/issues/76#issuecomment-128141407, the method IsTabVisible() is used to know from which tab decrement the offset. But this method is efficient only if the tab's buttons are already rendered, which is not yet the case at this place in the code. I will send a patch soon to ensure a correct behavior of this part of code.

[mjmacleod]

If I understand correctly we can revert back to case 2 (using m_tabOffset) instead of the modified code from #76 and then apply the above pull request and both this and #76 will behave as expected? i.e. no tabs jumping around when calling SetSelection() for #76?

[Kinaou]

Please check the behavior in the 3 cases thanks to the following compiled aui demo app: http://dl.free.fr/vJv2xLTF5

The useful menu items are in the menu View:

• For panes: Add pane, Select pane
• For embedded notebook: Create wxAuiNotebook, Add page to notebook, Select notebook page
• For both: Refresh

[nhold]

I am a little confused, is case 2 the repository as is? Or is case 2 your commit 484cfa1?

[Kinaou]

Case 2 is my PR https://github.com/nhold/wxWidgets/pull/128 Case 3 is the repository as is (with the commit https://github.com/mjmacleod/wxWidgets/commit/b9406eee0b92aecd1f39a0ca77f2f65e78adf0c1#diff-f74530d83e0cc6ec05ebd11bb2884c01) Case 1 is for testing purpose

[Kinaou]

Is my PR https://github.com/nhold/wxWidgets/pull/128 (case 2) seems good for you?

[mjmacleod]

Only had time for a brief look, but it seems okay to me.