Opened 10 years ago

Last modified 6 years ago

#9419 confirmed enhancement

wxAUI combine panes into notebooks.

Reported by: brianvanderburg Owned by:
Priority: normal Milestone:
Component: wxAui Version:
Keywords: has-code Cc: brianvanderburg, emilien.kia@…, bwilliams@…, hanmac@…
Blocked By: #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756, #14756 Blocking:
Patch: no

Description

wxAUI is very nice, making it quick simple to give applications the customizable features that modern applications are expected to have.

Some applications have a feature that AUI does not have (yet), but could hopefully be integrated into the main distribution. Panes could be docked 'together', and became part of a a notebook pane. I don't mean the current notebook support. Something like the GIMP, where two different dialogs can be docked together and become part of a tabbed pane.

The link below shows a picture of what I mean, the way panes 'Tools 1' and 'Tools 2' are tabbed as such. The link is a modification of AUI with several features, yet I think support for automatically tabbed panes would be a useful features.

http://wxforum.shadonet.com/viewtopic.php?t=14766

Attachments (1)

wxAuiPane_in_notebook.patch download (35.0 KB) - added by EmilienKia 7 years ago.
Patch to add ability to dock aui panes in notebook automatically.

Download all attachments as: .zip

Change History (70)

comment:1 Changed 9 years ago by wojdyr

  • Component set to wxAui

Changed 7 years ago by EmilienKia

Patch to add ability to dock aui panes in notebook automatically.

comment:2 Changed 7 years ago by EmilienKia

  • Cc emilien.kia@… added
  • Patch set

This is a patch adapted from ticket's message link and simplified to only add the ability to automatically dock aui panes into AuiNotebooks.

Work for wxGTK/trunk on LinuxMint 10, tested with aui sample.

The patch add a new pane dock position wxAUI_DOCK_NOTEBOOK_PAGE available when the wxAuiManager have not the style wxAUI_MGR_NO_AUTO_NOTEBOOK. such position make AuiManager create automatically wxAuiNotebook as pane to embed superposed panes. The Update() function (most of changes) is modified to take into account this case (AuiNotebook creation, deletion).

AuiManager now has a notion of "master manager" to differenciate AuiNotebook's manager from Frame's manager and link first one to second; the Frame's one taking preceding in Notebook tab dragging to attach/detach panes from notebooks. I have intend to replace this method by register/unregister event handler dynamically but without success so I let the initial proposal as is.

Best regards
Emilien

comment:3 Changed 6 years ago by vadz

  • Cc bwilliams@… added
  • Milestone set to 2.9.3
  • Status changed from new to confirmed

As usual, I can't really meaningfully review this patch so I'm probably going to apply it and hope for the best. How can I test the new functionality exactly in the aui sample?

Also, one question that I do have: is it a good idea to modify the perspective string format, it seems like we won't be able to restore it from the applications using the current wx version now, is it a problem? FWIW I think the perspective string should at least have a version number in it to help with future changes...

Ben, as usual any comments from you would be very welcome. TIA!

comment:4 Changed 6 years ago by EmilienKia

Hi,

You can test the patch with aui sample just by running the sample and try to drag a panel onto another. In certain place (perhaps too little), the shadow will cover all the target aui panel than when you drop the first panel, the second panel is replaced with a notebook and the two panels are docked in.

AFAIK the perspective string is compatible with old one, just one property has been added: notebookid. I think you are right to say a version number (or other distinction mark) can be added.

If I have two things to note : the first is the drop area is not usable enought for me (too small, misplaced ? the patch is an adaptation of another patch); the second is pane's notebook tab can not be moved in the same notbook as the move event is used to dock them out.
But I think there are not blocker and behavior can be easily enhance in future.

Best regards,

Emilien

comment:5 Changed 6 years ago by R.U.10

Hello,

Thank you for this very usefull work.

I'm trying your patch to integrate it in my own application and I detected some errors.
I'm using VS2005 on WinXP32 SP3 with wx 2.9.3 rev. 69407.

  • At the compilation:
>..\..\src\aui\framemanager.cpp(2849) : error C2668: 'wxAuiNotebook::InsertPage' : ambiguous call to overloaded function
>        (...)WX_2_9_TRUNK\include\wx/aui/auibook.h(605): can be 'bool wxAuiNotebook::InsertPage(size_t,wxWindow *,const wxString &,bool,int)'
>        (...)WX_2_9_TRUNK\include\wx/aui/auibook.h(530): or 'bool wxAuiNotebook::InsertPage(size_t,wxWindow *,const wxString &,bool,const wxBitmap &)'
>        when attempting to matching the argument list '(int, wxWindow *, wxString)'

(corrected by adding ',false,wxNullBitmap' at the function arguments).

  • At the use:

A corruption of the heap happens during this sequence of use:

  • run the sample in debug mode
  • drag the 'Tree pane' and drop it on the 'Text pane with hide prompt' to create the notebook
  • drag one of the two tabs and try to drop it in the newly created notebook one or two times
  • an error happens

Anyway, I am excited to see this patch functional and applied.

Regards,
KH

comment:6 follow-up: Changed 6 years ago by EmilienKia

Hello,

I just have tested again the patch with svn trunk r69411 with up-to-date LinuxMint 11.

The "wxAuiNotebook::InsertPage" compilation problem is probably due to an api modification between the patch creation and now. Your add seems good.

As you said, I have tested auidemo with wx compiled with debug informations.
Two problems occurs:

  • X makes auidemo exit sometimes with the following message:
    The program 'auidemo' received an X Window System error.
    This probably reflects a bug in the program.
    The error was 'BadWindow (invalid Window parameter)'.
      (Details: serial 17623 error_code 3 request_code 15 minor_code 0)
      (Note to programmers: normally, X errors are reported asynchronously;
       that is, you will receive the error a while after causing it.
       To debug your program, run it with the --sync command line
       option to change this behavior. You can then get a meaningful
       backtrace from your debugger if you break on the gdk_x_error() function.)
    

The problem does not occur with wx without debug info.

  • auidemo signals a segmentation fault with the following stack:
    Program received signal SIGSEGV, Segmentation fault.
    0x00007ffff6e91181 in wxBitmapRefData::~wxBitmapRefData (this=0xbf8710, __in_chrg=<value optimized out>) at ./src/gtk/bitmap.cpp:262
    262	    delete m_mask;
    (gdb) bt
    #0  0x00007ffff6e91181 in wxBitmapRefData::~wxBitmapRefData (this=0xbf8710, __in_chrg=<value optimized out>) at ./src/gtk/bitmap.cpp:262
    #1  0x00007ffff6e911f0 in wxBitmapRefData::~wxBitmapRefData (this=0xbf8710, __in_chrg=<value optimized out>) at ./src/gtk/bitmap.cpp:263
    #2  0x00007ffff64ae955 in wxRefCounter::DecRef (this=0xbf8710) at ./src/common/object.cpp:356
    #3  0x00007ffff64ae9e5 in wxObject::UnRef (this=0xb4b928) at ./src/common/object.cpp:389
    #4  0x00007ffff6e7143c in wxObject::~wxObject (this=0xb4b928, __in_chrg=<value optimized out>) at ./include/wx/object.h:369
    #5  0x00007ffff6e80056 in wxGDIObject::~wxGDIObject (this=0xb4b928, __in_chrg=<value optimized out>) at ./include/wx/gdiobj.h:43
    #6  0x00007ffff6e8015e in wxBitmapBase::~wxBitmapBase (this=0xb4b928, __in_chrg=<value optimized out>) at ./include/wx/bitmap.h:142
    #7  0x00007ffff6e91518 in wxBitmap::~wxBitmap (this=0xb4b928, __in_chrg=<value optimized out>) at ./src/gtk/bitmap.cpp:308
    #8  0x00007ffff7b8d40b in wxAuiDefaultDockArt::~wxAuiDefaultDockArt (this=0xb4b8d0, __in_chrg=<value optimized out>) at ./include/wx/aui/dockart.h:88
    #9  0x00007ffff7b8d6ac in wxAuiDefaultDockArt::~wxAuiDefaultDockArt (this=0xb4b8d0, __in_chrg=<value optimized out>) at ./include/wx/aui/dockart.h:88
    #10 0x00007ffff7b6c795 in wxAuiManager::~wxAuiManager (this=0xbf4c80, __in_chrg=<value optimized out>) at ./src/aui/framemanager.cpp:642
    #11 0x00007ffff7b8da8d in wxAuiFloatingFrame::~wxAuiFloatingFrame (this=0xbf4800, __in_chrg=<value optimized out>) at ./src/aui/floatpane.cpp:73
    #12 0x00007ffff7b8db2c in wxAuiFloatingFrame::~wxAuiFloatingFrame (this=0xbf4800, __in_chrg=<value optimized out>) at ./src/aui/floatpane.cpp:82
    #13 0x00007ffff642afe4 in wxAppConsoleBase::DeletePendingObjects (this=0x6bf3b0) at ./src/common/appbase.cpp:571
    #14 0x00007ffff642a835 in wxAppConsoleBase::ProcessIdle (this=0x6bf3b0) at ./src/common/appbase.cpp:377
    #15 0x00007ffff6f4a188 in wxAppBase::ProcessIdle (this=0x6bf3b0) at ./src/common/appcmn.cpp:346
    #16 0x00007ffff6e8cb0c in wxApp::DoIdle (this=0x6bf3b0) at ./src/gtk/app.cpp:139
    #17 0x00007ffff6e8ca4f in wxapp_idle_callback () at ./src/gtk/app.cpp:108
    

I do not understand yet why the segfault occurs but I will investigate.

Regards

Emilien

comment:7 Changed 6 years ago by vadz

  • Milestone 2.9.3 deleted

Seeing the problems above I don't think this can be included in 2.9.3 so resetting the milestone.

Thanks for testing and reporting these problems!

comment:8 in reply to: ↑ 6 ; follow-up: Changed 6 years ago by asmcoder

somehow my previous post wasnt saved...?

anyways, i just tried to merge the patch with wxw 293, and i keep getting a zero-dereference error in the constructor of wxAuiPaneInfo. :(

I desperately need this feature for a project i am planning. Emilien, could you please, please check this out? Tabbed panes is the last last feature that is missing with wxAUI...dont really want to switch to Qt, just because i am too stupid to apply patches... :/

my warmest thanks to anyone who can help to resolve this problem... ;)

comment:9 in reply to: ↑ 8 Changed 6 years ago by EmilienKia

Replying to asmcoder:

somehow my previous post wasnt saved...?

anyways, i just tried to merge the patch with wxw 293, and i keep getting a zero-dereference error in the constructor of wxAuiPaneInfo. :(

I desperately need this feature for a project i am planning. Emilien, could you please, please check this out? Tabbed panes is the last last feature that is missing with wxAUI...dont really want to switch to Qt, just because i am too stupid to apply patches... :/

my warmest thanks to anyone who can help to resolve this problem... ;)

wxAUI is a component which is moving continuously and I have not the time to keep the patch up-to-date regarding to trunk. I probably can do an update of the patch but in its current state, the patch is not applicable. An obsure problem of crach I have intend to solve but as I am not the initial author of the patch, somethings are trouble for me.

Thinking about the patch I am not sure it is really relevant to apply it as-is as it make loose some functionalities (like the tab repositioning by drag in the same auipane's notebook).
Probably a complete rewrite of the patch (inspirited by this one) is probably preferable than modifying this first.

I wish having this functionality too, but I have not enough time to do it.
If anyone want to do it, enjoy yourself, have fun ;).

comment:10 Changed 6 years ago by vadz

  • Keywords has-code added
  • Patch unset

Considering the above comment (and thanks for sharing it), let me reset the "Patch" flag and add a new "has-code" keyword to indicate that this can't/shouldn't be applied right now.

FWIW I agree that this would be very useful functionality but, just like the others, I don't have any time to spend on this myself.

comment:11 Changed 6 years ago by asmcoder

hi again, and thanks for replying! :)

I dont get any of those errors, but plain heap corruptions.

speculation:

When dragging the tabbed pane around, the notebook control somewhere fires a

wxAuiTabCtrl::OnMotion

event, but because the notebook-page has been turned into a floating pane already, this somehow causes trouble. I`m currently investigating this, but am still in the process of familiarizing myself with the library.

Your thoughts on this? :)

btw, two panes get merged into a notebook control when the being-dragged-pane hovers in the middle 1/3 of the width AND the middle 1/3 of the height of the already exiting pane/notebook. Is this done on purpose? Additionally, with left-docked panes sometimes the panes are only merged when i hover at the far, far left of the existing pane... But let`s sort out the heap corruption first... :)

best regards,

comment:12 Changed 6 years ago by asmcoder

This seems to be the problem indeed.

When i have two panes in pane-notebook and i want to drag out one, the notebook is destroyed, but wxw is still in the process of processing events emitted by that notebook. In my case usually left-click and tab-moving events.

If you now have three panes in a pane-notebook, there is *no* heap corruption and everything works just fine. This is presumably because the notebook hasnt been destroyed yet, as there are still two panes in it.

Unfortunately, i dont know enough about the event-system to suggest an adequate solution. Maybe someone can steer me in the right direction...? ;)

comment:13 Changed 6 years ago by asmcoder

Are you guys still there?:)

more speculations:

Could the bug be related to something like this:

            // the following block is a workaround for bug #1531361
            // (see wxWidgets sourceforge page).  On wxGTK (only), when
            // a frame is shown/hidden, a move event unfortunately
            // also gets fired.  Because we may be dragging around
            // a pane, we need to cancel that action here to prevent
            // a spurious crash.

The event causing the crash is processed here:

void wxAuiTabCtrl::OnMotion(wxMouseEvent& evt)
{
...

    if (abs(pos.x - m_clickPt.x) > drag_x_threshold ||
        abs(pos.y - m_clickPt.y) > drag_y_threshold)
    {
        wxAuiNotebookEvent evt(wxEVT_COMMAND_AUINOTEBOOK_BEGIN_DRAG, m_windowId);
        evt.SetSelection(GetIdxFromWindow(m_clickTab));
        evt.SetOldSelection(evt.GetSelection());
        evt.SetEventObject(this);

		GetEventHandler()->ProcessEvent(evt); //<--here

        m_isDragging = true;
    }
}

here are the patched files for wxw293 (NOT svn), if someone needs them:

http://speedy.sh/q2bzX/modaui.rar

direct:

http://speedy.sh/q2bzX/download/modaui.rar

load/save perspective patches are not included. The above heap corruption problem(s) still persist!

comment:14 Changed 6 years ago by Hanmac

  • Cc hanmac@… added

i try to implment this function from scatch, first i try how far can i go without changing the source (testing in a sample)

currently i implmented the drag-out a Pane from a NoteBook using the Patch as inpiration.
but i failed a bit because AuiNotebook does not realy fire currectly the LeaveWindow Event ...
but i think i can "fake" that ...

comment:15 Changed 6 years ago by mmacleod

Hello All,

I should point out that even if this patch were updated and some of the crashes removed, I don't think it should be applied.
This patch is one that has been revived a few times over the years by different authors, and has been rejected every time, the reason amongst other things is that the way in which it is implemented is a hack (no offence intended to any of the authors) - it works by creating new wxAuiNotebook's on the fly as needed and destroying them. This is flawed in several ways:
1) This method is prone to errors and unnecessary complexity
2) It doesn't work in a decent (and backwards compatible) way with perspective saving and loading
3) It makes the codebase larger and harder to maintain
4) It has several limitations in terms of both current and future capability

The correct way to do this is instead to re-architecture the way AUI layouts work on a lower level within the manager, by adding a new concept of (pages/z-ordering) into the manager - and then have the manager take care of drawing tabs where needed (Without wxAuiNotebook getting involved) - wxAuiNotebook itself then should be re-factored to take advantage of this new capability which will greatly reduce the complexity of the Aui notebook change.

The bulk of the work to do this correctly was already done by myself as a GSOC project in 2009, which due to unfortunate unforeseen circumstances in my life over the last few years never managed to get merged due to a lack of time; Further nobody has really shown an interest which also hasn't helped matters.
I have recently managed to put some time in to begin merging this work with trunk again, I have a git branch in which this is taking place (https://gitorious.org/~mmacleod/wxpython_trunk_mirror/wxwidgets-mmacleod/commits/AUI_GSOC) - At this point I have merged everything but am still in the processing of fixing a few bugs that have arisen as a result, I hope to put some work into this on the weekend and have a fully usable branch again by the end of the weekend.

To this end I think that any effort spent on the above patch will be completely wasted, which is why I have mentioned the above. I see that several people seem to be spending time on this and I do feel that this time will be completely wasted (so while I can't tell you what to do - I do strongly suggest to not spend any further time on this patch). If anybody really wants to see this functionality happen I think the best use of your time would be to get the git branch (I will let you know when its ready) and start testing that - it will need quite some real world testing before it can reach a point that its ready to be accepted into the actual wx tree, and it would certainly help me a lot to iron out final bugs and issues if people are actually using it.

comment:16 Changed 6 years ago by asmcoder

fair enough.

I see you have already done a lot of work in the branch you have there. Nice. Just let us know when you`re done merging, and if we can help in any way.

btw, compiling is broken (vs08) for your branch, not sure why.

I was planing on looking on two minor issues myself once this had been done, but as you are probably much more familiar with the code than me, maybe you can consider this in your work:

  • when turning a docked pane into a floating one by dragging it away, the position of the newly created floating pane is not where the mouse cursor is. The position gets updated on the first mouse event (usually moving), which is why it`s hard to see. To reproduce it, carefully click on a docked pane, then move your mouse "pixel by pixel" away, until the pane gets undocked, and note the random position of the pane. Continue moving your mouse and the pane will be moved along with the mouse.
  • a double click on the title bar of a pane should dock/undock it.

br and thanks for your work so far :)

comment:17 Changed 6 years ago by biwillia76

Agreed on the "hackiness" of this patch. This is why my response to this patch has been continuously tepid. The way we originally intended for this to be implemented was to add a "tab level." Just as one has docking levels to achieve an onion-like layered layout, we desire for there to be a tab level to determine the tab position of of a given pane.

comment:18 Changed 6 years ago by Hanmac

i wanted to make an entire new patch because i thought noone is working on this patch anymore ...

the features i want to see if possible:

  • it should be possible to change the position of AuiNoteBookPages without undocking it
  • says to the Pane that it cant be docked to a Notebook, or an Option to the Notebook that it dont want Panes inside.

comment:19 Changed 6 years ago by asmcoder

currently, you can drag a wxAuiNotebook tab out of its notebook, and a new wxAUiNotebook will be created with just this one page. Similarly, you can drag the page back into its original notebook, which it then becomes a part of, and the previously created notebook is destroyed.

What about this? Isn't this equally "hacky"?

comment:20 Changed 6 years ago by mmacleod

ac: btw, compiling is broken (vs08) for your branch, not sure why.

Not a big surprise, at that point I had only done merge work via my GTK box and had not even tested windows yet, since this mornings commit the branch should now work for MSW (osx still completely untested)
Use the "auinew" sample to try out the new code - the "aui" sample is only there for testing backwards compatibility.

ac: *snip* - when turning a docked pane into a floating one by dragging it away *snip*

I did a video recording of dragging with mouse slowly and then played it back frame by frame and I can't see the problem you describe, so this appears to be something that is already solved in my branch (Given that the code is completely different this isn't a very big surprise either)

ac: a double click on the title bar of a pane should dock/undock it.

I don't really know if this behavior should be standard or not, I don't really have strong feelings either way, but this is certainly something that can be pretty trivially implemented.

hm: it should be possible to change the position of AuiNoteBookPages without undocking it

This is already the behavior within the branched code

hm: says to the Pane that it cant be docked to a Notebook, or an Option to the Notebook that it dont want Panes inside.

This is not yet implemented - it is something that I wanted to do, and it should be pretty easy to add, but I probably would leave stuff like that until after gettign the core changes merged - or else we run the risk of never getting this code right.

ac: <snip> What about this? Isn't this equally "hacky"?

Not sure what you mean here.

Anyway, it is probably not a good idea to keep talking about this on trac and making this bug page unnecessarily busy. After this weekends changes the branch should be generally usable on WIN32 and GTK, although the GTK native art provider causes some crashes.
I will post a more detailed email outside of trac to wx-dev in the next day or two, giving more details of what has changed and the status etc. and any further discussion should probably then take place there.

comment:21 Changed 6 years ago by biwillia76

@asmcoder: Yes, that is just as 'hacky,' and I don't want the same thing to happen to wxAuiManager. One goal of wxAuiManager is to perform everything in a 'light-weight' manner (to use an old Java swing term). In other words, we don't want to create any window objects during a layout operation (of course with the one necessary exception of floating panes). Doing things this way makes creating and saving perspectives function much better. Otherwise, when creating/loading perspectives, you would be destroying/creating windows, which would be a big mess.

comment:22 Changed 6 years ago by asmcoder

just to let you know, there are still errors with wxw.

1) you`re missing a 56kb setup.h file in "mmacleod-wxwidgets-mmacleod-AUI_GSOC\wxpython_trunk_mirror-wxwidgets-mmacleod\lib\vc_lib\mswud\wx\"

2) the vc9.sln is for VS2010, there is none for VS2008

Copied the setup.h file over from wxw293, created my own map & compiled: no errors, but quiet some warnings.

Compiled the aui_new sample...but couldnt find any new features ;) Is there a changelog somewhere?

Additionally, i encountered the following problems:


1) demo freezes, when closing a tab of the main tab view ("floatable pane 5")
==> until you close all docked panes, then it is refreshed & works

2) when having 2 tabs in wxauintoebook and closing one, the notebook will be deleted, but the last tab isnt converted to a tab, but rather to a fixed part of the UI, which one cant close/drag into another notebook anymore.

3) captions are not drawn correctly for notebook tabs under the "simple" theme.


not sure how final the stage of this trunk is, but i though i`d better let you know :)

thanks for the work you put into this so far! :)

comment:23 Changed 6 years ago by mmacleod

1) For what its worth setup.h (which is looked for in include/wx/msw/setup.h" is not present in the main subversion either - and in fact there is a svn:ignore property to prevent it being accidentally committed.
I think you are meant to generate this yourself somehow after checking the repository out (or just copy setup0.h manually) - However for now for simplicity sakes I have added it to the git repo.

2) Thanks, I've updated the project files again they should be okay now.

Regarding the rest of your comments:
No there isn't a full changelog at this point however I have updated the "auinew" sample to be a bit more user friendly now (instead of just being a testbed for myself) and to also mention the main changes and explain whats going on.

1) Okay there was a minor glitch here to do with center panes - it is now fixed.
2) When there is only one pane left it reverts to being a normal pane - what you were probably seeing is that the notebook in the center was formed from two center panes (which are set to not have captions by default) - so when it reverts to being a normal pane it then has no caption. I've changed the sample a bit to prevent this.
3) Okay, I've fixed the simple theme artwork up a bit; this looks like it may have been a merge related regression from trunk (or just a trunk regression in general I'm not entirely sure)

I've posted a long message to the wx-users mailing list with more information - I suggest we take any further discussion to that thread. I'm trying to avoid any further noise now on this bug report...

comment:24 Changed 5 years ago by mmacleod

  • Blocked By 14756 added

comment:25 Changed 5 years ago by mmacleod

  • Blocked By

comment:26 Changed 5 years ago by vadz

  • Blocked By

(In #14756) Thanks, I've applied the latest patch now and visually it's indeed much better, it still seems somewhat sluggish but at least it looks correctly, thanks for fixing this!

However I can trivially make it crash, e.g. by just dropping any notebook page on the title bar of the lower messages window with the following back trace:

 	auidemo.exe!wxBaseArrayPtrVoid::GetCount()  Line 832 + 0x11 bytes	C++
 	auidemo.exe!wxAuiTabContainer::SetActivePage(wxWindow * wnd=0x00aa28a0)  Line 976 + 0xb bytes	C++
>	auidemo.exe!wxAuiManager::SetActivePane(wxWindow * activePane=0x00aa28a0)  Line 757	C++
 	auidemo.exe!wxAuiManager::OnLeftUp(wxMouseEvent & evt={...})  Line 5843	C++

It seems that ctrl in wxAuiManager::SetActivePane() is garbage/uninitialized. Do we need to check for FindTab() return value perhaps?

comment:27 Changed 5 years ago by mmacleod

  • Blocked By

(In #14756) Hmm, no.
Although it is probably a good idea to check that anyway, but it isn't the root cause. (It will just not work correctly instead of crashing - which is of course an improvement)

The problem is that its trying to set an active pane when the wxAuiTabContainer does not yet contain that pane.
The latest patch is meant to fix that and I could have sworn it did in my branch, but perhaps I missed something. (Or maybe the patch missed something)
I'll take a look in the morning and try provide a more permanent fix for this.

comment:28 Changed 5 years ago by mmacleod

  • Blocked By

(In #14756) It seems that the code was getting a bit confused about external drags between managers (from the notebook manager to the surrounding aui manager) - even though the drop was being disallowed it was still going ahead and causing a crash.

The above patch improves the way external drags are handled and should fix the crash, let me know if it doesn't.
(Note the drag won't actually succeed in this case because the parent manager does not handle the wxEVT_AUI_ALLOW_DND event, but if you create a new notebook via the menu you should be able to drag externally between the two notebooks (The old sample implements wxEVT_AUI_ALLOW_DND only for notebooks)

comment:29 Changed 5 years ago by mmacleod

  • Blocked By

comment:30 Changed 5 years ago by vadz

  • Blocked By

(In #14756) Thanks, one question after looking at the patch: could we avoid the need for dynamic cast to wxAuiNotebook by always generating wxEVT_AUI_ALLOW_DND and then generating wxEVT_COMMAND_AUINOTEBOOK_ALLOW_DND from its default handler in wxAuiNotebook? If we could do it like this, it'd be much cleaner IMHO. But maybe I'm missing something that prevents us from doing it like this?

comment:31 Changed 5 years ago by mmacleod

  • Blocked By

(In #14756) Yes, I don't see anything stopping it from being done that way and I agree it would be cleaner.

comment:32 Changed 5 years ago by jens

  • Blocked By

(In #14756) First:
sorry for the delay.
Here is a first patch that fixes issues with left/right tabs.
Moving the tabs with the up-/down-arrows works, furthermore the up-arrow is visible again.

I implemented a short loop, that ensures we can see as much tabs as fit on the screen, or in other words: while there is enough free space on the right or bottom of the tab-control for a tab (and not all tabs are shown) the taboffset is decreased.
This option can possibly be made configurable with a flag.

I also change dthe default tab-height from 30 to 42, but that's just another hack until we can use calculated values again.

comment:33 Changed 5 years ago by Hanmac

  • Blocked By

comment:34 Changed 5 years ago by vadz

  • Blocked By

(In #14756) Thanks for the patches but I'm concerned about the use of wxDC::GetSize() as AFAIR it's not implemented in wxOSX. Why do we have to use it, don't we have the size in the window itself somewhere?

Also, the situation with variable names is completely catastrophic :-( We can't keep renaming them like this, with one patch doing 's/foo_bar/fooBar/g' and the next one doing 's/fooBar/foo_bar/g'. Ideal would be to keep the original names until the dust settles and then do one single renaming at the end. But this is going to be difficult as the changes of the original patch contained a lot of name changes.

BTW, speaking of this, I am really not sure we want to deprecate the old non m_-prefixed names of wxAuiPaneInfo fields. There is no real harm in not using m_ here and this results in hundreds of deprecation warnings, so I'll probably undo this before merging into the trunk.

comment:35 Changed 5 years ago by jens

  • Blocked By

(In #14756) About wxDC::GetSize(), yes it's needed at the moment, the window-size is unusable at the moment, but if it is really needed, I will find another solution.
As far as I know I only use it in wxAuiGtkTabArt, is wxGTK usable on Mac ? and if it is, wouldn't it use the same wxDC than on linux ?

About the variable names, where did the change from fooBar to foo_bar ?
I tried to keep them as they are in the initial merge-patch.
But it might be, that I have overread some.

comment:36 Changed 5 years ago by vadz

  • Blocked By

(In #14756) Replying to jens:

About wxDC::GetSize(), yes it's needed at the moment, the window-size is unusable at the moment

I don't really understand how can wxDC have the correct size if the window doesn't. Where does the DC size come from?

, but if it is really needed, I will find another solution.

As far as I know I only use it in wxAuiGtkTabArt, is wxGTK usable on Mac ?

Maybe/probably, but it's true that I overlooked the fact that it was only used in wxGTK-specific code so wxOSX considerations are irrelevant here. Still, I'm bothered by the fact that window size can't be used, it's probably another bug somewhere.

About the variable names, where did the change from fooBar to foo_bar ?

totWidth is being changed to tot_width in several places.

There are also not really necessarily changes of m_flags & XXX to HasFlag(XXX). It's not that I disagree that the latter is better but I'd really like to avoid making all these changes at the same time, it would be much better to leave them off until after the merge.

comment:37 Changed 5 years ago by jens

  • Blocked By

(In #14756) Replying to vadz:

Replying to jens:

About wxDC::GetSize(), yes it's needed at the moment, the window-size is unusable at the moment

I don't really understand how can wxDC have the correct size if the window doesn't. Where does the DC size come from?

, but if it is really needed, I will find another solution.

As far as I know I only use it in wxAuiGtkTabArt, is wxGTK usable on Mac ?

Maybe/probably, but it's true that I overlooked the fact that it was only used in wxGTK-specific code so wxOSX considerations are irrelevant here. Still, I'm bothered by the fact that window size can't be used, it's probably another bug somewhere.

The DrawButton seems to get the frame containing the whole notebook (including the pane) as parameter.
The dc is a memory dc especially created for drawing and it has the correct size.
Before the merge the window was the wxAuiTabCtrl, which no longer exists.
The rect can not be used directly, because the width is decreased for each drawn button, so the close and window-list button can be drawn at the right side, but the up button needs to be drawn in the middle of the rect, but this can not be determined after the width has decreased.
But I'm sure there will be another solurtion.

About the variable names, where did the change from fooBar to foo_bar ?

totWidth is being changed to tot_width in several places.

Ooops, sorry. I will change it back, or you can do it.

There are also not really necessarily changes of m_flags & XXX to HasFlag(XXX). It's not that I disagree that the latter is better but I'd really like to avoid making all these changes at the same time, it would be much better to leave them off until after the merge.

I did this, to be more consistent with other parts of the merge.
But I agree, it makes it harder to review the patches.

comment:38 Changed 5 years ago by jens

  • Blocked By

(In #14756) I fixed the sizing problems, or more exactly calculation of tabs (width and height) now works correctly.
Also the drawing of the buttons/arrows.

The tabart-class now has to care for the placement of the buttons and no longer the Render()-function, what was wrong in my opinion.
The tabart-class has to "decide" which buttons should be placed where.

Drawing of the right-tabs with gtk is also fixed.

Some minor issues are left with bottom tabs (at least with generic-tabart.
But this should not be a greatl problem.

Should I create a new patch based on the old ones (assuming the old patches have been applied) or should I create (a) new patch(es).

And what about the naming (camelCase vs. however_it_is_called) and using of HasFlag instead of m_flags&xxx ?
Shall I fix it also ?

I can upload (a) patch(es) this evening

comment:39 Changed 5 years ago by mmacleod

  • Blocked By

(In #14756) > BTW, speaking of this, I am really not sure we want to deprecate the old non m_-prefixed names of wxAuiPaneInfo fields. There is no real harm in not using m_ here and this results in hundreds of deprecation warnings, so I'll probably undo this before merging into the trunk.

I agree here that there is no need to rush the deprecation, feel free to take the deprecation away for now.

comment:40 Changed 5 years ago by vadz

  • Blocked By

(In #14756) Replying to jens:

I fixed the sizing problems, or more exactly calculation of tabs (width and height) now works correctly.
Also the drawing of the buttons/arrows.

Great, thanks!

Should I create a new patch based on the old ones (assuming the old patches have been applied) or should I create (a) new patch(es).

The currently applied patches are in the aui branch of git@…:vadz/wxWidgets.git

And what about the naming (camelCase vs. however_it_is_called) and using of HasFlag instead of m_flags&xxx ?
Shall I fix it also ?

I'd be really glad if all the patches could only include the important/critical changes right now and we postponed the rest of them until later. I'll actually probably try to revert some of the naming changes of the original patch just to be able to see more clearly what it does.

I can upload (a) patch(es) this evening

If you do, please let me know if they should be applied after aui_fix_external_tab_drop_crash2.patch or before it. Unfortunately I won't have time to apply this one today.

comment:41 Changed 5 years ago by jens

  • Blocked By

(In #14756) I just attached two patches, that (hopefully) fix almost all drawing issues in auinotebook.
You can apply them before or after the drop-crash patch, both works for me.

The calculate-tab-size patch must be applied before the fix-drawing-issues patch.

What stiil needs to be done is reimplementing the fixes for #14710.

That's the next thing I will do.

There are other sizing issues left, when draging (or better after dropping) tabs in some cases.
And I still can produce crashes on drop (even with the crash-drop-patch).

I will try to give detailed steps to reproduce, or find the reason.
But the drawing stuff has a higher priority for me, because I'm more familiar with the drawing code.

comment:42 Changed 5 years ago by mmacleod

  • Blocked By

(In #14756) Replying to jens:

I just attached two patches, that (hopefully) fix almost all drawing issues in auinotebook.
You can apply them before or after the drop-crash patch, both works for me.
The calculate-tab-size patch must be applied before the fix-drawing-issues patch.

Thanks, will try have a look over it this afternoon.

There are other sizing issues left, when draging (or better after dropping) tabs in some cases.

If you can let me know what cases I can have a look from this side as well.

And I still can produce crashes on drop (even with the crash-drop-patch).
I will try to give detailed steps to reproduce, or find the reason.

If you can give a stack race that should be enough, although any detailed steps are also appreciated.
Will have another look from this side as well.

But the drawing stuff has a higher priority for me, because I'm more familiar with the drawing code.

Yes, its probably more efficient if we each focus on the code we are more familiar with.

Thanks,
Malcolm

comment:43 Changed 5 years ago by jens

  • Blocked By

(In #14756) I just saw that fixed-size of tabs is broken.

There is one typo, but also a real issue with the order of calculating dynamic and fixed width and height.

I try to find a solution and post a patch.
Other stuff should not be affected.

Jens

comment:44 Changed 5 years ago by jens

  • Blocked By

(In #14756) Replying to jens:

I just saw that fixed-size of tabs is broken.

I just attached a patch that should fix this.

Jens

comment:45 Changed 5 years ago by jens

  • Blocked By

(In #14756) Hi Malcolm,

I just uploaded two backtraces after a drop-crash.
Your patch and my new patches are installed.

The first one appeared after a drop near the bottom of the auidemo window.
It happened sveral times, but I did not find a way to reproduce it reliably.
The scond one happened with close on all tabs active. I pressed the mouse over the close-button and dragged the tab, no hint-rectangle was visible before the crash.

Jens

comment:46 Changed 5 years ago by vadz

  • Blocked By

(In #14756) I applied (admittedly, without any review as I'm simply too lost here) all the patches to facilitate testing but the aui sample still crashes all the time. E.g. just make the tree pane floating -- it dies in wxAuiFloatingFrame::OnMoveFinished()...

comment:47 Changed 5 years ago by mmacleod

  • Blocked By

(In #14756) Okay thanks.
I've managed to reproduce the crash here and I should have some spare time tomorrow so will try get to the bottom of it and do a bit more stress testing.

Regarding the left/right changes I have a concern, for left/right notebooks "fixed width" seems to have been "re-purposed" to mean "fixed height".
I don't think this is a good idea, as it could be confusing for users. Left/Right notebooks are currently allocating as much space as necessary to fit the "page title" for very long page titles this can look somewhat unwieldy and as such it is not hard to imagine that a program author in this situation might want a "fixed width" for right/left tabs, getting instead a fixed height would be annoying.
Should we not consider just separating the two and having a "fixed height" as well, one could have both "fixed width" and "fixed height" for left/right notebooks, while for top/bottom notebooks "fixed height" could be ignored. (Unless of course some meaningful behavior could be done here as well)

comment:48 Changed 5 years ago by jens

  • Blocked By

(In #14756) At the moment "fixed height" for left/right is the same as "fixed width" for top/bottom: distribute the available space equally on all tabs, using a hard-coded min value.
Maybe wee need (or better should introduce) a min (and probably) a max-size for all alignments and also a real fixed width (or better size), which is not dynamical as it is now for top/bottom tabs.
SetMinSize(), SetMaxSize() and probaly a SetSize() could be used.
What we have now is more or less the same as sizeritems with wxEXPAND set, with a hardcoded min- and max-size and same proportion for all of them.

comment:49 Changed 5 years ago by mmacleod

  • Blocked By

(In #14756) Above patch should fix those two crashes.
The one fix is not 100% right as it fixes th symptom but not the underlying cause, I will need to look into it in a bit more detail to fix the underlying cause as well, will try do that this afternoon.
In the meantime would be good to know if this at least fixes the crashes for you or if they continue.

Thanks,
Malcolm

comment:50 Changed 5 years ago by mmacleod

  • Blocked By

(In #14756) Okay, updated version of the patch attached.

This properly fixes the cause of the above crashes as well as one or two other "strange" behaviors that were related. There seems to be one more crash lurking but it takes quite a while to reproduce and I've not yet managed to catch it in a debugger, will carry on looking into it but that should probably be applied separately (after this patch)

comment:51 Changed 5 years ago by jens

  • Blocked By

(In #14756) I attached three patches:

the first one reimplements the changes made for #14710,

the second fixes minor drawing issues in generic tabart,

the third fixes build issues I mentioned in windows 7:
auibook.h needs containr.h for wxNavigationEnabled and framemanager.cpp needs dcmemory.h for wxMemoryDC.

comment:52 Changed 5 years ago by jens

  • Blocked By

(In #14756) The newest patch implements a better calculation of tab- and gapbox-size.

comment:53 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) I tested the auidemo under GTK2 and GTK3 applying also all the patches starting from aui_fix_tab_drops.patch,
let me know if this is correct (where is the new auidemo?).

I got some debug warnings under GTK2 and a crash on GTK3.

Both warnings and crash seems to happen in some circumstances when dragging some wxAuiToolBar control, where in GTK2 in particular, the vertical toolbar sometimes, trying to dock it when floated, disappears.
Other than that, when switching to the 'all panes' perspective, some toolbars shrinks to the left (same issue I have in my AUI XRC patch), so troubles (crash and warnings) starts when trying to move them (they seems to resize back to the original size once dragged).

The only weird thing I noticed (without considering the wxAuiToolBar issues) is the 'global' close button on the right, when no notebook tab have one, that disappears when changing perspective and go back again to the startup one, resulting with the original 'close button in all tabs' behaviour.

About the warnings I'm not sure if they are related to my GTK2 theme I'm using in my Ubuntu 12.10 + LXDE system, I have to test again applying changes in my local trunk.

comment:54 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) > The only weird thing I noticed (without considering the wxAuiToolBar issues) is the 'global' close button on the right, when no notebook tab have one, that disappears when changing perspective and go back again to the startup one, resulting with the original 'close button in all tabs' behaviour.

Oops, this happens when changing notebook theme and default behaviour is 'close button in active tab'.

comment:55 Changed 5 years ago by jens

  • Blocked By

(In #14756) Replying to RedTide:

The only weird thing I noticed (without considering the wxAuiToolBar issues) is the 'global' close button on the right, when no notebook tab have one, that disappears when changing perspective and go back again to the startup one, resulting with the original 'close button in all tabs' behaviour.

Oops, this happens when changing notebook theme and default behaviour is 'close button in active tab'.

I think this is related to the changes of the constants for the auibook-flags in the dynamic aui branch.
Should be easily fixable.

In fact I saw it also, but it has low priority at the moment.

Gtk3 is more interestant, but drawing (and calculating of sizes) has changed a lot, so I still have no patch to attach here.
It compile sfine with gtk3 and drawing works also, but tabsize calculating still needs some more work.

comment:56 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) Replying to jens:

Replying to RedTide:

Gtk3 is more interestant, but drawing (and calculating of sizes) has changed a lot, so I still have no patch to attach here.
It compile sfine with gtk3 and drawing works also, but tabsize calculating still needs some more work.

I think that this should be done later, if possible, in wxRenderer with a 'new' (wxUniv have it already) DrawTab().

comment:57 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) I tested also on MSW and I got a segmentation fault adding a new notebook and then dragging a notebook page to the new one.
I'm using XP SP3 under VirtualBox + MinGW (official, not TDM) + MSYS (GCC 4.7.2).
It seems that gdb won't redirect output to a file like it does in linux, and interpret them as auidemo command line parameters, so, if needed, I can provide a jpg of the backtrace...

comment:58 Changed 5 years ago by vadz

  • Blocked By

(In #14756) Thanks for the testing. I still didn't have time to do much debugging of it myself but your results show that we still can't merge this in, getting crashes is really not acceptable.

So the question is whether we should delay 2.9.5 until somebody has time to fully iron out all the fatal bugs in this code or just go ahead with 2.9.5 and postpone this for 3.2.

Neither alternative is very appealing unfortunately :-(

comment:59 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) Replying to vadz:

So the question is whether we should delay 2.9.5 until somebody has time to fully iron out all the fatal bugs in this code or just go ahead with 2.9.5 and postpone this for 3.2.

Neither alternative is very appealing unfortunately :-(

I don't see why a release of a development version shouldn't wait a bit, but this depends to how many hands/heads will work on this issue.
I'm for waiting this to be fixed and applied before 2.9.5, with some effort also from other devs also if not much interested on AUI.
If 2.9.5 will be last development version before 3.0 release, users will test it more deeply, but this is just my own opinion.

comment:60 Changed 5 years ago by jens

  • Blocked By

(In #14756) If this branch will not make it into 2.9.5 I would like to (back-)port the left-/right alignment stuff to trunk (in fact it was done already and should work without much effort).

It's a nice feature to have the tabs at the left or right side in times of wide-scale monitors.

There is already a ticket open (currently blocked by this one).
Last comment there was http://trac.wxwidgets.org/ticket/13787#comment:20 .

In any case we should decide what to do with the fixed-width/fixed-height(?) flag.

Currently it's more a give all tabs an equal width flag, which indeed not makes much sense at all and more or less no sense for left-/aright-alignment. Her we should have something like max width or make fixed-width a real fixed-width no matter what the content is.

comment:61 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) Replying to jens:

If this branch will not make it into 2.9.5 I would like to (back-)port the left-/right alignment stuff to trunk (in fact it was done already and should work without much effort).

Is your GTK3 work to be going in wxRenderer?

I would like to see drawing stuff applied to it to having a native drawing apart, this could make code modular
and some parts also of this patch could be extracted and applied already and other non AUI users could benefit from it, but maybe I wrong/miss something.
Anyway I did some initial code to patch against wxRenderer also for tabs other than toolbars if it could be useful.

Vadim, could be possible to apply all the patches above and sync your clone with current trunk to be able to work on this issue meanwhile?

comment:62 Changed 5 years ago by vadz

  • Blocked By

(In #14756) Replying to RedTide:

Vadim, could be possible to apply all the patches above and sync your clone with current trunk to be able to work on this issue meanwhile?

I'm actually lost as to which patches should be applied (and in which order). I had started hacking on my own version about 3 weeks ago, trying to get rid of insignificant changes (and thus understand the significant ones better) but never finished it so right now my tree is a mess...

Anyhow, does anybody have a branch with all the patches applied to it on Github by chance? I think it would be more productive to work on this there instead of attaching patches to this ticket.

TIA!

comment:63 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) Replying to vadz:

Replying to RedTide:

Vadim, could be possible to apply all the patches above and sync your clone with current trunk to be able to work on this issue meanwhile?

I'm actually lost as to which patches should be applied (and in which order). I had started hacking on my own version about 3 weeks ago, trying to get rid of insignificant changes (and thus understand the significant ones better) but never finished it so right now my tree is a mess...

Anyhow, does anybody have a branch with all the patches applied to it on Github by chance? I think it would be more productive to work on this there instead of attaching patches to this ticket.

TIA!

The order, AFAIU, is starting from aui_fix_tab_drops.patch, that seems replacing the previous one (I had to add a --ignore-whitespaces to apply) and below.

I have it at home with all the patch applied, so I could, if Malcom and Jens agree, upload it in my Github account and take care to sync it with the main upstream adding you all with write permission after uploading (merge the current trunk to it after upload as first commit, using it as origin where you will work on and adding the read only wx trunk as upstream to be able to keep this branch in sync with it).
I can do it only starting from the next week so meanwhile let me know if this could be a useful/correct solution.

comment:64 Changed 5 years ago by vadz

  • Blocked By

(In #14756) It would be definitely useful, thanks.

But I'm still tempted to release 2.9.5 now because I didn't manage to work on this this weekend (got ill and spent what little time I managed on fixing a couple of new bad regressions instead) and so we're basically not going to release 2.9.5 at all this year if we delay it further.

We could do a 2.9.6 early in the next year if this stabilizes enough for then though.

comment:65 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) I've pushed the changes to https://github.com/redtide/wxWidgets/tree/aui-dynamic-notebook
against the current trunk code:
after some talking with Brian, I realized that should be better that anyone have to keep his own wxWidgets forked repo and pull only from other ones instead sharing the same, so I hope that my simple file replacement with the files with all patches applied is fine too.
Let me know about any problem/issue, meanwhile I'll test it.

comment:66 Changed 5 years ago by vadz

  • Blocked By

(In #14756) We need to integrate r73263 (see #10848) into this branch,

comment:67 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) Replying to vadz:

We need to integrate r73263 (see #10848) into this branch,

I suppose also r73288 / #14927
I think I'll wait for Malcom before do this my self on my cloned repo.

comment:68 Changed 5 years ago by RedTide

  • Blocked By

(In #14756) Replying to RedTide:

Replying to vadz:

We need to integrate r73263 (see #10848) into this branch,

I suppose also r73288 / #14927
I think I'll wait for Malcom before do this my self on my cloned repo.

I'm not familiar with this code, but it is so different that I suppose that both issues was already managed, so I merged the code with a recent trunk version leaving auibook.cpp code unmodified.

comment:69 Changed 5 years ago by vadz

  • Blocked By

(In #14756) Sorry, this clearly won't happen for 3.0 :-(

Note: See TracTickets for help on using tickets.