Bug#749659: audacity + wxWidgets 3.0 — Proposing patch

Wed Aug 20 21:17:42 UTC 2014

Thanks Olly, both for the initial patch (renaming of the constants) and 
for contacting both upstream teams (audacity and wxWidgets) to resolve 
the issues - the response of the wxWidgets guys has been very helpful to me.

Last weekend I was able to work on this bug. I managed to compile and 
run audacity with wxWidgets 3.0 successfully. However, there are still a 
few issues to resolve (mainly by wxWidgets upstream).

I tried to write code that works with both wx2.8 and wx3.0 without using 
preprocessor #ifdefs, but at some places it just couldn't be avoided.

As the patch has gotten way larger than I had expected, I feel the need 
to explain the changes:

Solved compilation issues:
* Allow wxWidgets 3.0 in various configure scripts.
* The file dialog was derived from an internal wx class that disappeared 
in wx3.0 (discussed in further detail below).
* In several places the types in interfaces changed between wxChar* and 
wxString. I've tried to modify the code so it works with both interfaces 
(thanks to implicit conversion; may look a bit weird in some places).
* Calls to "wxTheApp->SuspendIdleCallback()" are no longer supported. 
They were part of a workaround for a clipboard problem, which 
fortunately doesn't seem to be there any longer in wx3.0, so I 
deactivated it for wx3.0.
* AddPendingEvent() and ProcessEvent() now have the visibility 
"protected" in wxWindow. It has always been a bad idea to call them 
directly on a wxWindow object, but now it's explicitly forbidden. 
Instead, those functions should be called on the object returned by 
GetEventHandler().
* The class wxStandardPaths is now a singleton and has to be treated as 
such (no more explicit constructor calls).
* "wxLogWarning" is a macro now, so the "::" prefix doesn't work on it 
anymore.
* Several int types (which were actually used as enums) are now real 
enum types (e.g. wxRasterOperationMode, wxMouseButton) and have to be 
used as such.
* Apart from the FD constants (which have been fixed in Olly's patch 
already) there are some other constants which have received a prefix 
(e.g. wx* -> wxFONTFAMILY_*, wx* -> wxFONTSTYLE_*, wx* -> wxFONTWEIGHT_*).
* Signature changes in constructors of wxFlexGridSizer and wxIcon and 
wxSizeEvent
* Missing includes (missing header files were probably included 
indirectly in wx2.8 by chance)
* When deriving from the abstract class wxGridTableBase, different 
methods have to be implemented with wx3.0 (EndEdit with new signature 
and ApplyEdit) than before with wx2.8 (only EndEdit with old signature). 
Now both versions are implemented in parallel in the classes TimeEditor 
and ChoiceEditor (one version essentially being a wrapper of the other one).

Solved runtime issues:
* Segmentation fault: The LadspaEffectDialog receives EVT_TEXT events 
before it's properly initialized. To prevent this, a workaround was 
already in place, but was only active on Windows. It looks like this 
happens now on more platforms (including GTK). As the workaround doesn't 
do any harm, even if activated unnecessarily, I've simply activated it 
for all environments.
* GTK critical warning "IA__gtk_range_set_range: assertion 'min < max' 
failed" because of negative numbers as result of window size checking. 
Added a sanity check that straightens up the numbers in edge cases.
* GTK critical warning "IA__gdk_window_get_origin: assertion 
'GDK_IS_WINDOW (window)' failed": Received events of type wxSizeEvent on 
the main project window cause calls to "ClientToScreen" - which is not 
available until the window is first shown. So the class has to keep 
track of wxShowEvent events and inhibit those actions until the window 
is first shown. There should be a simpler way to do this, but neither 
IsShown() nor IsShownOnScreen() seem to help - both return true already 
before the window is actually created and before the ClientToScreen 
method starts working. I personally consider this a bug in wxWidgets 
(maybe I'm gonna file a bug report) - but nevertheless it's fixed in 
audacity now.
* The functions wxString::Format and wxString::Printf have become 
stricter about parameter types that don't match (format string vs. 
function parameters). So the bugs (that were already present in audacity 
before) become visible in wx3.0 as error messages. I've fixed all the 
ones that popped up during my testing, but there might be more of them 
that I just didn't happen to hit. At some point, all the calls to 
wxString::Format and wxString::Printf have to be checked systematically.

Some issues that I came across can be considered wxWidgets' "fault" 
(general problems with wxWidgets 3.0), but nevertheless affect 
audacity's functionality. I isolated them and reported them to the 
wxWidgets bug tracker:

Unresolved wxWidgets upstream issues:
* Dialog size: The Fit() method to resize a dialog to make all elements 
fit doesn't seem to work properly in wxWidgets 3.0 (GTK) on some Desktop 
Environments. This causes several audacity dialogs (e.g. the file 
recovery dialog on start-up) to lack elements. Reported to wxWidgets 
upstream: http://trac.wxwidgets.org/ticket/16440
* GTK critical warning "os_bar_hide: assertion 'OS_IS_BAR (bar)' failed" 
(and similar ones) when closing the program (affecting only downstream 
Ubuntu, not Debian sid): http://trac.wxwidgets.org/ticket/16438#ticket

Now, specifically about the issue with the wxFileDialog that has been 
discussed here previously: Unfortunately, different from what has been 
stated here before, it's not only about the added button - although that 
was the original intention of the copy. Since the moment the copy from 
wxWidgets was made, wxWidget's version and audacity's version have 
diverged further. This means that there is a conflict between a 
cleaner/more sustainable solution (by using only the proper wxWidgets 
interfaces) and maintaining all the functionality on all platforms (by 
keeping the copies - with the implication of having to add an additional 
copy of the outdated wxGenericFileDialog!!).

So I tried to figure out what exactly the real differences are. From the 
differences I guess that wxWidgets 2.6.3 was the base for the copy (it's 
the one with the least number of different lines overall). So I made a 
diff and (with the additional help of audacity's SVN) tried to figure 
out what purpose those changes serve. The good news is: For the GTK 
version we can go for the clean solution using only wxWidgets 
interfaces. All changes can either be re-done in a clean way (using only 
the interfaces) or are already built into wxWidgets 3.0:
* The extra button can be added via 
"wxFileDialog::SetExtraControlCreator()", as suggested in the wxWidgets 
upstream bug.
* Code that tries to expand all "expanders" before showing the dialog: I 
tried it and didn't see any difference between activated and deactivated 
the code. According to the comments I guess the original purpose was to 
expand the "Browse for other folders..." in older versions of the GTK 
FileChooser. But I haven't seen that old dialog version for ages, I 
don't think we have to worry about it.
* Fix of a memory leak in the original wxWidgets code: The leak is fixed 
in wx3.0 (they now use the wxGtkString that frees the strings received 
by GTK functions after its usage).
* Updated calls, variable names and macros to wx2.8 conventions: Nothing 
we have to worry about, if we use don't mess with wxWidgets internals.
* Deactivation of the automatic overwrite dialog: In wx3.0 that behavior 
can now be controlled via the wxFD_OVERWRITE_PROMPT style flag.
* Code that activates the "Enter" key as a way to execute the default 
action: This is done automatically in wx3.0.

The implementation in the attached patch provides a platform-independent 
file dialog that has all the features of the platform-specific old GTK 
version. Whether or not that implementation will be suitable for Windows 
and Mac is up to upstream, maybe they decide to stick with their own 
version for those platforms. I'm not completely done with interpreting 
all the changes, but it seems like the Windows version additionally 
tries to deal with some problems with long file filters that are not 
handled properly by the native Windows dialog prior to Windows 7 and the 
Mac version additionally tries to improve some accessibility features. 
In my opinion, if anywhere, those improvements should be in the 
wxWidgets code itself.

As I had to patch some "configure.in" files and those don't seem to be 
passed to "autoconf" automatically by the build system, I provided an 
additional patch for the "configure" files (output of "autoconf"). But 
maybe a better solution would be to generate those files automatically 
during the build by calling "autoconf".

Right now to build the package you have to apply all 3 patches (in this 
order): wx-fd-constants.patch, wx30.patch and wx30-autoconf-results.patch.

Cheers,
Martin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: wx30.patch
Type: text/x-diff
Size: 43195 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20140820/08790626/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wx30-autoconf-results.patch
Type: text/x-diff
Size: 369366 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20140820/08790626/attachment-0003.patch>