Thanks, Martyn. You're right about the real culprit.
I implemented the fix a little more OOP-ish:
* Gave the Effect base class a variable for its dialog (if there is
one). Already had one for its parent.
* Then descendant Effects that create dialogs have them set it to
the one each creates, e.g., at the end of the dialog's constructor:
mEffect->SetDialog(this);
* Also moved the SetFocus() call into Effect::Preview(), so it
doesn't need to be duplicated in each descendant, and removed the
SetFocus() from each descendant's ::OnPreview().
* Then implemented your change for ProgressHide except rather than
overload it, just declared the arg to have a default NULL and then
SetEnabledWindow() on it only if non-NULL.
* >>" If somebody cancels the "Preparing preview" progress dialog then
things carry on anyway (but they do already) without the effect.
Probably the same for the "Mix and Render" progress dialog..."?>>
Added the arg to both ProgressHide calls in Effect::Preview(). I
think that handles the 'not ideal', because Mix-and-Render
doesn't have a controls dialog, so it's okay to re-enable the
Project window, right?
This involved 2-3 lines of changes in about 20 files. So the question is
whether to put all those changes into 1.3.5.
I posted a 1.3.5-rc4 Windows installer with these changes, built on
latest CVS (incl. locale updates), at
http://audacity.sourceforge.net/files/audacity-win-unicode-1.3.5-rc4.exe
, but NOT committed these progress-vs-modality changes to CVS.
If there's time to test them on Windows, should we put them into 1.3.5?
If not, we can just build the release from CVS. If so, even if more
problems show up, we can probably fix by 1.3.6.
- Vaughan
Martyn Shaw wrote:
> Yeah David, nice catch!
>
> I think V is slightly off, but helpful anyway (to me at least). I
> think the problem is in AudacityProject::ProgressHide(), a few lines
> up in Project.cpp to he indicates; the project window is getting
> enabled when Preview finishes (and I assume that means it's children
> as well), which isn't what we want in a effect. We want the effect
> dialog to be enabled (only).
>
> I have a fix here which is 'better than we have' but not ideal (see
> below).
>
> For an effect (it would need doing for all) I've tried putting
>
> void ExampleEffect::OnPreview(wxCommandEvent &event)
> {
> TransferDataFromWindow();
> - m_pEffect->Preview();
> + m_pEffect->Preview(this);
> this->SetFocus();
> }
>
> following the logic that in order to enable the correct dialog,
> Effect::Preview needs to know which one it is being called from. Then
> I followed this through with
>
> - void Effect::Preview()
> + void Effect::Preview(wxDialog * effectDialog)
> ...
> - GetActiveProject()->ProgressHide();
> + GetActiveProject()->ProgressHide(effectDialog);
>
> and an overloaded (correct term?)
>
> +void AudacityProject::ProgressHide(wxDialog * effectDialog)
> +{
> + ProgressHide(); // enable project window
> + SetEnabledWindow( effectDialog ); // and then set to the effect
> dialog again
> +}
>
> which enables the correct window again.
>
> Why is it 'not ideal'? I think there are (possibly) three progress
> windows that can open when you do a preview (if it's long enough).
> "Mix and Render", "Preparing preview" and the preview one.
>
> If somebody cancels the "Preparing preview" progress dialog then
> things carry on anyway (but they do already) without the effect.
> Probably the same for the "Mix and Render" progress dialog but I've
> ran out of brain, and time.
>
> I could commit the changes above in a day or two if they'd be useful,
> or maybe it's already fixed? (I see 66 unread emails in my inbox).
>
> Martyn
> Having an early night, due to a rugby tour for the last 2 days. I'm
> not entirely sure what happened.
>
> Vaughan Johnson wrote:
>
>> David Bailes wrote:
>>
>>> Hi,
>>>
>>> In an effects dialog, if you press preview, then the focus returns to
>>> the dialog (bug fixed), but the dialog becomes modeless, so allowing
>>> someone to do something in the main window (like deleting all the
>>> tracks) before pressing the ok button (crash).
>>>
>>>
>>>
>> Really good catch, David!
>>
>> I've debugged it to the extent of finding that it's not due to the
>> SetFocus() call. Instead, if you comment out line 424 of Effect.cpp:
>>
>> previewing = GetActiveProject()->ProgressUpdate(t);
>>
>> ...the modality problem does not happen. I haven't looked at OnPreview
>> in a long time, but with the original, where preview was only 3 seconds,
>> progress wasn't necessary, so I expect this code was added when a pref
>> was added to allow user-specified preview length.
>>
>> Anyway, Leland is, I think, the threaded progress expert. Maybe it's the
>> call to SetEnabledWindow() at line 3593 of Project.cpp that's switching
>> the modality?
>>
>> - Vaughan
>>
>> -------------------------------------------------------------------------
>> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
>> Don't miss this year's exciting event. There's still time to save $100.
>> Use priority code J8TL2D2.
>>
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone>> _______________________________________________
>> Audacity-devel mailing list
>>
Audacity-devel@...
>>
https://lists.sourceforge.net/lists/listinfo/audacity-devel>>
>>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
> Don't miss this year's exciting event. There's still time to save $100.
> Use priority code J8TL2D2.
>
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone> _______________________________________________
> Audacity-devel mailing list
>
Audacity-devel@...
>
https://lists.sourceforge.net/lists/listinfo/audacity-devel>
>
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone_______________________________________________
Audacity-devel mailing list
Audacity-devel@...
https://lists.sourceforge.net/lists/listinfo/audacity-devel
