Opened 5 years ago

Last modified 17 months ago

#14756 accepted enhancement

Merge 'dynamic notebook' branch of AUI

Reported by: mmacleod Owned by: vadz
Priority: normal Milestone: 3.2.0
Component: wxAui Version:
Keywords: aui notebook dynamic Cc: bpetty, hanmac@…
Blocked By: Blocking: #8346, #9419, #10466, #10466, #13787, #13787, #13787
Patch: yes

Description

Please find attached a patch containing all changes from the 'dynamic notebook' branch of AUI that I have been maintaining.
While far from perfect at this point I think that it is time (and the pragmatic thing to do) to go ahead with this and then iron out the problems as they happen, with my work load and the huge amount of other patches coming in it is unlikely that a better opportunity will arise to do this merge and if we wait too much longer we risk it just becoming simply impossible.
It is also crucial I think that this must make it into wxWidgets 3, and therefore 2.9.5 is the last opportunity for proper testing and stability fixes, so it makes sense to push this before 2.9.5

This patch regresses the following revisions that will have to be looked at again after merging (one of them overly critical I don't think):
72641
72627
71560

The following (minor) documentation patches will have to be looked at again and/or redone, I unfortunately don't have the time currently for this, but can try get around to it in the coming weeks after the merge, of course if anybody else wants to assist thats even better...
72686
72418
71907
71204
70807
70708
70691
70541
70248
70183
69280
69267
67428
67280
67279
66698
66670
66007
65096
64940

In addition to the above there are some small visual regressions in wxAuiNotebook art to look into, and probably also various other wxAuiNotebook regressions that will appear with more testing. The new functionality of course is also bound to have numerous bugs.

Attachments (19)

aui_dynamic_notebooks.patch.2.gz download (124.9 KB) - added by mmacleod 5 years ago.
aui_dynamic_notebooks.patch.gz download (138.7 KB) - added by mmacleod 5 years ago.
Initial proposed patch for merge (compressed for 275kb limit)
aui_dynamic_notebooks_vadz_diff.patch download (220.1 KB) - added by mmacleod 5 years ago.
Patch against latest vadz git from my latest git
aui_dynamic_notebooks_fixes_since_initial.patch download (21.7 KB) - added by mmacleod 5 years ago.
Seperate patch containing differences since initial patch
aui_fix_external_tab_drop_crash.patch download (11.5 KB) - added by mmacleod 5 years ago.
aui_fix_external_tab_drop_crash2.patch download (12.4 KB) - added by mmacleod 5 years ago.
first_left_right_tabs_fix.patch download (5.4 KB) - added by jens 5 years ago.
make up-arrow visible again, make up-/down-arrows work for left and right aligned tabs, minimize free space after tabs
first_fixed_width_fixes.patch download (12.0 KB) - added by jens 5 years ago.
make fixed height basically work (still needs some tweaking); more exact calculation of required width and height
fix_calculating_tab_size.diff download (11.5 KB) - added by jens 5 years ago.
fix to calculate the tabs size instead of using fixed width and default height (apply first)
fix_drawing_in_tabart.diff download (22.5 KB) - added by jens 5 years ago.
fix drawing issues in tabart-classes introduced with the merge
fix_for_fixed_size_tabs.diff download (4.7 KB) - added by jens 5 years ago.
fix for fixed width/height notebook-tabs
fix_drop_crashes.patch download (3.3 KB) - added by mmacleod 5 years ago.
aui_fix_tab_drops.patch download (10.4 KB) - added by mmacleod 5 years ago.
reimplement_fixes_for_ticket_14710.patch download (19.1 KB) - added by jens 5 years ago.
reimplement the changes made for closing #14710
fix_drawing_issues_in_generic_tabart.patch download (1.6 KB) - added by jens 5 years ago.
tiny fixes for drawing issues in generic tabart
windows_build_fixes.patch download (812 bytes) - added by jens 5 years ago.
build fixes needed on windows 7 with MinGW 4.7
better_calculation_of_tab_and_gapbox_size.patch download (6.2 KB) - added by jens 5 years ago.
better calculation of tab- and gapbox-size
gtk2_debug_messages.txt download (4.3 KB) - added by RedTide 5 years ago.
GTK2 debug warnings
gtk3_auidemo_dragging_toolbars_bt.txt download (14.3 KB) - added by RedTide 5 years ago.
GTK 3 backtrace

Download all attachments as: .zip

Change History (84)

comment:1 Changed 5 years ago by bpetty

  • Cc bpetty added

I know I've mostly been absent from active development for a while here, but I'm going to try and at least help out with taking care of getting this branch merged in. This needs a lot of work on review and as many eyes as possible.

comment:2 Changed 5 years ago by mmacleod

comment:3 Changed 5 years ago by mmacleod

Oh yes one more thing, old 'aui' sample is left basically untouched in order to help test backwards compatibility, while 'auinew' makes use of all the new API functionality.

comment:4 Changed 5 years ago by vadz

  • Owner set to vadz
  • Status changed from new to accepted

OK, the changes here are too massive for any meaningful review (apparently a lot of stuff just got moved around but unfortunately it's not easy to see it in the diff), so I'll wait a couple of days for people to test this patch and complain about any breakage and just apply it if nobody does because it's this or just never merge it at all because I don't know of any reasonable way to review 20kLoC patch...

Please do test it if you use wxAUI!

comment:5 Changed 5 years ago by vadz

Unfortunately my first impressions are not good. At least under MSW there are pretty bad visual artefacts when dragging notebook pages, e.g. in the (old) aui sample I can see one page show through the other while dragging and after landing it sometimes I get another page displayed in the lower part of the page I dragged. There is also bad flicker when dragging which I think is new.

Notebook tabs on the left/right work but scrolling them doesn't, so some pages can become inaccessible. Also, the scrolling arrow doesn't seem to disappear once it has appeared, even if the notebook is resized to have enough space. But this is minor compared to the problem above.

Please let me know if you don't see this and I'll make some screenshots and describe the actions in more details but I think it should be easy to reproduce, there are pretty bad visual artefacts (again, at least under MSW with Windows 7, I didn't test elsewhere yet).

Changed 5 years ago by mmacleod

comment:6 Changed 5 years ago by jens

I will test it and try to find a solution (at least for the left-right related stuff, as this is based on a patch of mine).
Should I add patches here, or in the original thread ?

comment:7 Changed 5 years ago by vadz

Jens, are you using git too? If so, I think I could commit this branch into a github repository and you could work on it there (I know that Malcolm already has his repository on gitorious but it doesn't seem to be compatible with our official git mirror so I'd rather use that one). Because otherwise things risk getting very messy because I already did some local changes to the patch (mostly minor style things but some renamings too)...

comment:8 Changed 5 years ago by jens

Vadim, yes, I use git and git-svn.
If you can commit the branch to github it would be fine for me.

comment:9 Changed 5 years ago by vadz

OK, I've pushed my current version of the patch to https://github.com/vadz/wxWidgets/tree/aui-dynamic-notebook

comment:10 Changed 5 years ago by mmacleod

@vadz
Note that I've since uploaded a new version which fixes several issues that have been picked up, including (hopefully) most of the visual artifacts you mentioned. Not sure how we should proceed if you already have local patches as well? I'm busy setting up a new windows test VM and will have a look if I can find any remaining issues (on GTK at least everything now seems solid)

@jens
Thanks that will be a big help, let me know if you have any questions though, I'm also available on wxwidgets IRC channel if you want to discuss anything.

comment:11 Changed 5 years ago by mmacleod

Trying with the latest patch on an XP VM I do not see any of the visual glitches or artifacts, so I'm going to assume for now that the latest patch has fixed these. Please let me know if they are still occurring, I will try set up a win 7 box to test on as well.

Please ignore ( aui_dynamic_notebooks.patch.2.gz ) file - this was a mistake, I would delete it but there doesn't seem to be an option for that.

Changed 5 years ago by mmacleod

Initial proposed patch for merge (compressed for 275kb limit)

comment:12 Changed 5 years ago by vadz

Malcolm, sorry, I didn't see your other patch and now I'm stuck because I don't know what to do with this one as I don't have the original version of the patch any more. Could you please reupload the original one here so that I could apply the difference between two patches? Of course, if you can make a patch directly against the URL from comment:9 it would be perfect.

TIA!

Changed 5 years ago by mmacleod

Patch against latest vadz git from my latest git

comment:13 Changed 5 years ago by mmacleod

Okay, I've pulled your latest git and generated a patch between the two (above). Will do any further work against your git so that we don't have any more issues like this.

comment:14 Changed 5 years ago by vadz

It's still almost impossible to apply :-( The trouble is that it remodifies the things that I undid/removed from the original patch, e.g. all the extra white space additions and changes (" *" vs "* " -- it would be really better to avoid them) and renames back "wxAuiXXX" functions to "Aui_" ones.

I'm afraid the only solution is to make a diff against the original diff, otherwise I simply don't see what changed and can't apply it.

Changed 5 years ago by mmacleod

Seperate patch containing differences since initial patch

comment:15 Changed 5 years ago by mmacleod

Unfortunately I foolishly overwrote the original patch, so the best I can do is the above (new patch against older revision) which at least should make the changes clear.
Can you work with that?

Otherwise I will just quickly redo them against your branch, it is not a lot of changes.

comment:16 Changed 5 years ago by vadz

Yes, I should be able to apply this, thanks! I'll try to do this tonight.

I'll probably separate it into significant changes and the rest (e.g. "it's" vs "its"), hopefully this won't be a problem for you as you should be able to just start working on the github branch once this is applied.

comment:17 Changed 5 years ago by mmacleod

  • Blocking 9419 added

comment:18 Changed 5 years ago by mmacleod

  • Blocking 10466 added

comment:19 Changed 5 years ago by vadz

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:20 Changed 5 years ago by mmacleod

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.

Changed 5 years ago by mmacleod

comment:21 Changed 5 years ago by mmacleod

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:22 Changed 5 years ago by mmacleod

  • Blocking 8346 added

comment:23 Changed 5 years ago by vadz

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:24 Changed 5 years ago by mmacleod

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

Changed 5 years ago by mmacleod

comment:25 Changed 5 years ago by jens

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.

Changed 5 years ago by jens

make up-arrow visible again, make up-/down-arrows work for left and right aligned tabs, minimize free space after tabs

Changed 5 years ago by jens

make fixed height basically work (still needs some tweaking); more exact calculation of required width and height

comment:26 Changed 5 years ago by Hanmac

  • Cc hanmac@… added

comment:27 follow-up: Changed 5 years ago by vadz

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:28 follow-up: Changed 5 years ago by jens

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:29 in reply to: ↑ 28 ; follow-up: Changed 5 years ago by 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.

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:30 in reply to: ↑ 29 Changed 5 years ago by jens

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:31 follow-up: Changed 5 years ago by 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.

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:32 in reply to: ↑ 27 Changed 5 years ago by mmacleod

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:33 in reply to: ↑ 31 Changed 5 years ago by vadz

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.

Changed 5 years ago by jens

fix to calculate the tabs size instead of using fixed width and default height (apply first)

Changed 5 years ago by jens

fix drawing issues in tabart-classes introduced with the merge

comment:34 follow-up: Changed 5 years ago by 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.

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:35 in reply to: ↑ 34 Changed 5 years ago by mmacleod

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:36 follow-up: Changed 5 years ago by jens

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

Changed 5 years ago by jens

fix for fixed width/height notebook-tabs

comment:37 in reply to: ↑ 36 Changed 5 years ago by jens

Replying to jens:

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

I just attached a patch that should fix this.

Jens

comment:38 Changed 5 years ago by jens

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:39 Changed 5 years ago by vadz

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:40 Changed 5 years ago by mmacleod

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:41 Changed 5 years ago by jens

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.

Changed 5 years ago by mmacleod

comment:42 Changed 5 years ago by mmacleod

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

Changed 5 years ago by mmacleod

comment:43 Changed 5 years ago by mmacleod

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)

Changed 5 years ago by jens

reimplement the changes made for closing #14710

Changed 5 years ago by jens

tiny fixes for drawing issues in generic tabart

Changed 5 years ago by jens

build fixes needed on windows 7 with MinGW 4.7

comment:44 Changed 5 years ago by jens

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.

Changed 5 years ago by jens

better calculation of tab- and gapbox-size

comment:45 Changed 5 years ago by jens

The newest patch implements a better calculation of tab- and gapbox-size.

comment:46 follow-up: Changed 5 years ago by RedTide

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.

Changed 5 years ago by RedTide

GTK2 debug warnings

Changed 5 years ago by RedTide

GTK 3 backtrace

comment:47 in reply to: ↑ 46 ; follow-up: Changed 5 years ago by 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'.

comment:48 in reply to: ↑ 47 ; follow-up: Changed 5 years ago by jens

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:49 in reply to: ↑ 48 Changed 5 years ago by RedTide

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:50 Changed 5 years ago by RedTide

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:51 follow-up: Changed 5 years ago by vadz

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:52 in reply to: ↑ 51 Changed 5 years ago by RedTide

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:53 follow-up: Changed 5 years ago by 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).

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:54 in reply to: ↑ 53 ; follow-up: Changed 5 years ago by RedTide

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:55 in reply to: ↑ 54 ; follow-up: Changed 5 years ago by 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!

comment:56 in reply to: ↑ 55 Changed 5 years ago by RedTide

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:57 Changed 5 years ago by vadz

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:58 Changed 5 years ago by RedTide

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:59 follow-up: Changed 5 years ago by vadz

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

comment:60 in reply to: ↑ 59 ; follow-up: Changed 5 years ago by 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.

comment:61 in reply to: ↑ 60 Changed 5 years ago by RedTide

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:62 Changed 5 years ago by vadz

  • Milestone changed from 2.9.5 to 3.2

Sorry, this clearly won't happen for 3.0 :-(

comment:63 Changed 4 years ago by R.U.10

  • Blocking

(In #13787) FWIW, I updated my patch related to the changes in r71001. I did not work on the file tabartgtk.cpp inserted in r71002 so I guess the patch would work only on MSW.

comment:64 Changed 4 years ago by vadz

  • Blocking

(In #13787) All wxAUI issues will have to wait until we deal with the question of its refactoring/rewrite/replacement and this won't happen before 3.0.

comment:65 Changed 17 months ago by ngpaton

  • Blocking
Note: See TracTickets for help on using tickets.