XRC handler for wxAuiManager and wxAuiPaneInfo

[RedTide]

This patch adds an XRC handler for wxAuiManager and wxAuiPaneInfo.
It was done using the #10996 made by Steve Lamerton, with some small changes added by me.
To make it works I've used the following steps:

patch -p0 < xrcreorg-new-4.patch
patch -p0 < xrc_aui.patch
cd ./build/bakefiles
bakefile_gen
cd ../../
aclocal -I ./build/aclocal
autoconf
mkdir buildgtk
cd buildgtk
../configure --enable-debug >build.txt
make

[vadz]

(In #10996) I've (finally!) started working on applying this patch and hope to apply it soon (== before 2.9.5). Please let me know if anybody sees any problems with it. The latest version of the patch can be found at  https://github.com/vadz/wxWidgets/tree/xrc-reorg

[steve_lamerton]

(In #10996) Still looks good to me, I look forward to seeing this committed!

[vadz]

(In #10996) One thing I wonder about: would it better to have all wxXmlResourceHandler methods forwarding to GetImpl() inline in wx/xrc/xmlreshandler.h? I'm not sure either way and I don't seem to get any noticeable difference in the library sizes whether I do it like this or leave the original patch alone. I think I'd still prefer keeping them inline just because it's easier to update/maintain like this, what do you think?

[vadz]

(In #10996) I think I'm quite happy with this patch, it's really extremely simple, thanks a lot for paring it down to just this.

The only thing I'd like to do before applying it would be to add a test of wxRichTextCtrl to the xrc sample (of course, this will require linking the sample with the richtext library), just to check that it does work as expected. If you have time to do it, it would be great, otherwise I'll try to do it myself soon.

Thanks again!

[steve_lamerton]

(In #10996) With regards to inlining I don't really have a preference either way. I think in one of the earlier versions of the patch they may well have been done that way. It is easy enough to swap it over.

I'll try and get a basic example added to the sample in the next day or two.

[vadz]

(In #10996) Actually #12058 already updates the xrc sample to use ribbon XRC handler so there is no need to do anything more, thanks. I'll commit this soon.

[vadz]

Unfortunately there are too many problems with this patch for me to apply it right now -- I tried but ran out of time, so all I can do is to attach here my work-in-progress version of it and hope that someone else can finish it later.

Here is the list of problems I noticed:

1. It doesn't link because the code in xh_aui.cpp uses wxXmlNode directly but "aui" library doesn't link with "xml" one, so this fails in DLL builds. For the "classname", IsOfClass() should have been used instead but I'm not sure about the "name" nor about the logic here.
2. The new handler is not documented in docs/doxygen/overviews/xrc_format.h and I don't understand how it works well enough to do it myself.
3. We don't need a new sample, we should just add a menu item allowing to load a frame using AUI to the existing "xrc" one. I started doing this but didn't finish because I couldn't test it anyhow because of the linking problem.
4. Maybe most important: I'm not sure if it makes much sense to have this handler and wxAuiNotebookXmlHandler one. IMHO they should be merged. What do you think, Steven?

[vadz]

Unfinished VZ's version of the patch

[VZ]

(In #10996) (In [72727]) Refactor wxXRC to allow defining handlers outside of xrc library.

Split wxXmlResourceHandler into an ABC and the real implementation to allow
referencing the ABC in the core library itself but without pulling in all of
the XRC into it. This also allows defining XRC handlers, which only depend on
this ABC and not the xrc library, in other libraries, such as richtext, as
demonstrated by the now enabled wxRichTextXMLHandler.

Closes #10996.

[RedTide]

Hi Vadim and Steven.

1. Sorry for this but I didn't know about this problem, also after checking all libs dependencies at  http://docs.wxwidgets.org/trunk/page_libs.html where it seems that wxAUI depends from wxXML lib.
The wxXmlNode is used to determine the parent node of the current one, to check if the AUI manager can be used in the specified window, where classname is the window class (i.e. wxPanel) and name is the name of the window (i.e. myPanel) where the manager is inserted in.

2. Because of the blocking/WIP 10996 I wanted to wait to document it, waiting for a final version but I'll do it ASAP.
All XRC properties reflects the wxAuiPaneInfo ones (I think I'll have to add also some for wxAuiManager, at least its styles), just using a non camel case naming convention as in other handlers for the XML tags, i.e. CaptionVisible become caption_visible, DefaultPane default_pane and so on (lower case using underscores).

3. I wasn't sure if doing that way we had to link the sample to aui lib other than to the XRC lib, so I thought that the samples for non core/adv wx libs had to be separated...

4. I think they should be merged also because the incoming 'dynamic notebooks' branch merge, but maybe is not that important (AFAIK wxAuiNotebook will be just a class definition for compatibility because the new incoming AUI logic).
Anyway, do we need to wait this merge to test this handler properly?

[steve_lamerton]

I definitely think that they should be merged, it makes sense to have just a single handler for the various wxAUI bits and pieces, especially with the work in the dynamic notebooks branch.

[vadz]

Replying to RedTide:

Hi Vadim and Steven.

1. Sorry for this but I didn't know about this problem, also after checking all libs dependencies at  http://docs.wxwidgets.org/trunk/page_libs.html where it seems that wxAUI depends from wxXML lib.

This is wrong (I'll correct it), it doesn't depend on it and IMO shouldn't.

The wxXmlNode is used to determine the parent node of the current one, to check if the AUI manager can be used in the specified window, where classname is the window class (i.e. wxPanel) and name is the name of the window (i.e. myPanel) where the manager is inserted in.

I don't know the details but this doesn't seem to be the right way to do it. The other handlers never look up the parent, instead they remember the tag they're currently in and check it, see e.g. wxNotebook handler. I think this one should do the same thing.

3. I wasn't sure if doing that way we had to link the sample to aui lib other than to the XRC lib, so I thought that the samples for non core/adv wx libs had to be separated...

No, there is no particular problem with linking the sample to aui.

4. I think they should be merged also because the incoming 'dynamic notebooks' branch merge, but maybe is not that important (AFAIK wxAuiNotebook will be just a class definition for compatibility because the new incoming AUI logic).
Anyway, do we need to wait this merge to test this handler properly?

I don't think so, the branch is not supposed to change the external API of wxAUI (much), so the handler code should continue to work. And I think it'd still make sense to define wxAuiNotebook in XRC even after this merge (Malcolm, please correct me if I'm wrong). At any rate, the other classes (i.e. wxAuiManager, wxAuiPaneInfo) would remain the same.

[mmacleod]

No you're right. wxAuiNotebook becomes a lot lighter internally and passes most the work off to a manager instead, but from an external (and therefore XRC) perspective its still the same class and isn't changing or going anywhere, so shouldn't affect this at all.

[RedTide]

OK, so I can make the necessary changes, I think for the next week..
What about the wxAuiNotebook handler? I saw a previous commit so how to merge it?
Is wxAuiToolBar the only missing control to add?

[vadz]

Replying to RedTide:

OK, so I can make the necessary changes, I think for the next week..

TIA!

What about the wxAuiNotebook handler? I saw a previous commit so how to merge it?

I think you should remove xh_auinotbk files and merge their contents/functionality into xh_aui.

[RedTide]

I'm going to finish the patch but I have a problem: wxAuiToolBar has no Create() function, so it can't be used with XRC as is, so I add a small diff to the current trunk to add the missing function, I'm just not sure about the initialization code, so I hope someone can review it, thanks.

[VZ]

(In [72785]) Add wxAuiToolBar::Create().

Implement two-step creation of wxAuiToolBar to allow doing it from XRC.

See #13520.

[RedTide]

Here it is my current patch version.

Changes:

- Removed the checks to the wxAuiManager managed window and the wxXML link problem
- Merged Steve's wxAuiNotebook code and Vadim's changes (wxVector)
- Added wxAuiToolBar control
- Doxygen documentation

There are some problems with wxAuiToolBar:

in some situations, changing the style and moving the mouse pointer over it, can cause a crash (segfault), same when clicking on a dropdown arrow button
(maybe this is related to a not handled wxMenu; the toolbar can set a tool button as dropdown but not associate a menu to it if not from an event handler.)

The toolbar does not appears immediatly with the correct size
(in the most cases drag it and dock it again make it to fit with correct size).

Since we can't use wxXmlNode directly, I have some difficult to add some features like adding dropdown button and wxControl derived tools like in wxToolBar handler.

Where can I add GetAuiManager() function documentation?

Any suggestion/code corrections will be appreciated, TIA.

[RedTide]

Looks like the problem with wxAuiToolBar can be related to #10036 (actually my demo use it inside a wxPanel in a xrcdemo choicebookpage).
I think it's not good for a custom toolbar like this to have such limitation, so I met Hanmac on IRC (see #14152 and the mentioned below #14145) and we agree to work on his #14152 patch to solve the issues, including the dropdown one.

I think also should be nice to have the latter to be applied (maybe adding also a wxDEPRECATE to wxAuiManager::UnInit() ?), so I can also remove from this patch the need to UnInit() managers.

About the wxXmlNode limitation I wonder if adding some wxXmlNode *wxXmlResourceHandler::GetParamNode( wxObject *object, const wxString &childNodeName) could be useful in those cases where we need to handle situations like described in my previous comment (wxToolBar dropdown and wxControl derived tool objects).

What do you suggest to do?

[VZ]

(In [72816]) Get rid of wxAuiToolBar::m_style and just use base class m_windowStyle.

This fixes an assert exposed by the addition of wxAuiToolBar::Create() in
r72785: as m_style was not initialized before, calling GetWindowStyle() from
wxControl::Create() returned wrong flags.

Fix this by just removing m_style completely, there doesn't seem to be any
need for it nor for overriding GetWindowStyleFlag().

See #13520.

[RedTide]

What about to remove temporarily wxAuiToolBar support due to its current issues to be able to apply this patch in current trunk, or is better to wait later when it will be fixed?
(other than I think that we need also some additional feature in wxXmlResourceHandler to be able to retrieve nested nodes like it happens with <dropdown> tag for wxToolBar).
Let me know also about #14145, because if it will be applied, I could also remove the automatic UnInit() in wxAuiXmlHandler.

[RedTide]

Final version

[RedTide]

wxGTK2 appearance

[RedTide]

wxGTK3 appearance

[RedTide]

wxMSW appearance

[RedTide]

The attacked patch is my current (and I think final) version of the patch, just removed wxAuiToolBar because the above issues and added a wxToolBar in the sample demo.

[RedTide]

Removed the 'work-needed' bit: all the issues in comment 7 should be fixed and it can be applied as it is.
wxAuiToolBar could be added later when fixed and if #14145 will be applied, the duplicate code to UnInit() managers can be removed.