From bb26b2f2c4a0213d44434927dbaf33a34c125d48 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Mon, 6 Jan 2020 10:09:18 -0500 Subject: [PATCH] Require a ProjectWindow as ancestor of effect dialog... ... by passing parent as reference, not pointer, and testing in the dialog factory function. This is important so that we know the lifetime of an effect dialog, even when it is non-modal, is bounded by the lifetime of the associated project. --- include/audacity/EffectInterface.h | 5 +++-- src/BatchCommandDialog.cpp | 2 +- src/BatchCommands.cpp | 3 ++- src/BatchCommands.h | 3 ++- src/BatchProcessDialog.cpp | 2 +- src/effects/ChangePitch.cpp | 2 +- src/effects/ChangeTempo.cpp | 2 +- src/effects/Effect.cpp | 6 +++--- src/effects/Effect.h | 6 +++--- src/effects/EffectManager.cpp | 4 ++-- src/effects/EffectManager.h | 2 +- src/effects/EffectUI.cpp | 16 ++++++++++++---- src/effects/EffectUI.h | 2 +- src/effects/NoiseReduction.cpp | 8 ++++---- src/effects/NoiseReduction.h | 2 +- src/effects/NoiseRemoval.cpp | 4 ++-- src/effects/NoiseRemoval.h | 2 +- src/effects/VST/VSTEffect.cpp | 2 +- src/effects/VST/VSTEffect.h | 2 +- src/effects/audiounits/AudioUnitEffect.cpp | 2 +- src/effects/audiounits/AudioUnitEffect.h | 2 +- src/effects/ladspa/LadspaEffect.cpp | 2 +- src/effects/ladspa/LadspaEffect.h | 2 +- src/effects/lv2/LV2Effect.cpp | 2 +- src/effects/lv2/LV2Effect.h | 2 +- src/effects/nyquist/Nyquist.cpp | 2 +- src/effects/nyquist/Nyquist.h | 2 +- 27 files changed, 51 insertions(+), 40 deletions(-) diff --git a/include/audacity/EffectInterface.h b/include/audacity/EffectInterface.h index a4f14357a..2db9ff1a7 100755 --- a/include/audacity/EffectInterface.h +++ b/include/audacity/EffectInterface.h @@ -142,7 +142,8 @@ class AUDACITY_DLL_API EffectClientInterface /* not final */ : public EffectDef { public: using EffectDialogFactory = std::function< - wxDialog* ( wxWindow*, EffectHostInterface*, EffectUIClientInterface* ) + wxDialog* ( wxWindow &parent, + EffectHostInterface*, EffectUIClientInterface* ) >; virtual ~EffectClientInterface() {}; @@ -179,7 +180,7 @@ public: virtual bool RealtimeProcessEnd() = 0; virtual bool ShowInterface( - wxWindow *parent, const EffectDialogFactory &factory, + wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false ) = 0; // Some effects will use define params to define what parameters they take. diff --git a/src/BatchCommandDialog.cpp b/src/BatchCommandDialog.cpp index 976ff3b50..f6fa6be36 100644 --- a/src/BatchCommandDialog.cpp +++ b/src/BatchCommandDialog.cpp @@ -218,7 +218,7 @@ void MacroCommandDialog::OnEditParams(wxCommandEvent & WXUNUSED(event)) auto command = mInternalCommandName; wxString params = mParameters->GetValue(); - params = MacroCommands::PromptForParamsFor(command, params, this).Trim(); + params = MacroCommands::PromptForParamsFor(command, params, *this).Trim(); mParameters->SetValue(params); mParameters->Refresh(); diff --git a/src/BatchCommands.cpp b/src/BatchCommands.cpp index cc8278199..37a80a698 100644 --- a/src/BatchCommands.cpp +++ b/src/BatchCommands.cpp @@ -438,7 +438,8 @@ wxString MacroCommands::GetCurrentParamsFor(const CommandID & command) return EffectManager::Get().GetEffectParameters(ID); } -wxString MacroCommands::PromptForParamsFor(const CommandID & command, const wxString & params, wxWindow *parent) +wxString MacroCommands::PromptForParamsFor( + const CommandID & command, const wxString & params, wxWindow &parent) { const PluginID & ID = EffectManager::Get().GetEffectByIdentifier(command); diff --git a/src/BatchCommands.h b/src/BatchCommands.h index 61ee72b6f..80fa12674 100644 --- a/src/BatchCommands.h +++ b/src/BatchCommands.h @@ -95,7 +95,8 @@ class MacroCommands final { static wxArrayStringEx GetNamesOfDefaultMacros(); static wxString GetCurrentParamsFor(const CommandID & command); - static wxString PromptForParamsFor(const CommandID & command, const wxString & params, wxWindow *parent); + static wxString PromptForParamsFor( + const CommandID & command, const wxString & params, wxWindow &parent); static wxString PromptForPresetFor(const CommandID & command, const wxString & params, wxWindow *parent); // These commands do depend on the command list. diff --git a/src/BatchProcessDialog.cpp b/src/BatchProcessDialog.cpp index 0fcecfb39..cadec9b51 100644 --- a/src/BatchProcessDialog.cpp +++ b/src/BatchProcessDialog.cpp @@ -1104,7 +1104,7 @@ void MacrosWindow::OnEditCommandParams(wxCommandEvent & WXUNUSED(event)) auto command = mMacroCommands.GetCommand(item); wxString params = mMacroCommands.GetParams(item); - params = MacroCommands::PromptForParamsFor(command, params, this).Trim(); + params = MacroCommands::PromptForParamsFor(command, params, *this).Trim(); Raise(); mMacroCommands.DeleteFromMacro(item); diff --git a/src/effects/ChangePitch.cpp b/src/effects/ChangePitch.cpp index 9463f9c63..e9d617ea6 100644 --- a/src/effects/ChangePitch.cpp +++ b/src/effects/ChangePitch.cpp @@ -215,7 +215,7 @@ bool EffectChangePitch::Process() proxy.mProxyEffectName = XO("High Quality Pitch Change"); proxy.setParameters(1.0, pitchRatio); - return Delegate(proxy, mUIParent, nullptr); + return Delegate(proxy, *mUIParent, nullptr); } else #endif diff --git a/src/effects/ChangeTempo.cpp b/src/effects/ChangeTempo.cpp index 87e2a31e5..fb9f008dd 100644 --- a/src/effects/ChangeTempo.cpp +++ b/src/effects/ChangeTempo.cpp @@ -200,7 +200,7 @@ bool EffectChangeTempo::Process() EffectSBSMS proxy; proxy.mProxyEffectName = XO("High Quality Tempo Change"); proxy.setParameters(tempoRatio, 1.0); - success = Delegate(proxy, mUIParent, nullptr); + success = Delegate(proxy, *mUIParent, nullptr); } else #endif diff --git a/src/effects/Effect.cpp b/src/effects/Effect.cpp index 9ab545591..1c1d71359 100644 --- a/src/effects/Effect.cpp +++ b/src/effects/Effect.cpp @@ -478,7 +478,7 @@ bool Effect::RealtimeProcessEnd() return true; } -bool Effect::ShowInterface(wxWindow *parent, +bool Effect::ShowInterface(wxWindow &parent, const EffectDialogFactory &factory, bool forceModal) { if (!IsInteractive()) @@ -1091,7 +1091,7 @@ void Effect::SetBatchProcessing(bool start) } } -bool Effect::DoEffect(wxWindow *parent, +bool Effect::DoEffect(wxWindow &parent, double projectRate, TrackList *list, TrackFactory *factory, @@ -1214,7 +1214,7 @@ bool Effect::DoEffect(wxWindow *parent, } bool Effect::Delegate( - Effect &delegate, wxWindow *parent, const EffectDialogFactory &factory ) + Effect &delegate, wxWindow &parent, const EffectDialogFactory &factory ) { NotifyingSelectedRegion region; region.setTimes( mT0, mT1 ); diff --git a/src/effects/Effect.h b/src/effects/Effect.h index 3942fa3a3..7bfa1c7f8 100644 --- a/src/effects/Effect.h +++ b/src/effects/Effect.h @@ -142,7 +142,7 @@ class AUDACITY_DLL_API Effect /* not final */ : public wxEvtHandler, size_t numSamples) override; bool RealtimeProcessEnd() override; - bool ShowInterface( wxWindow *parent, + bool ShowInterface( wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false) override; bool GetAutomationParameters(CommandParameters & parms) override; @@ -255,12 +255,12 @@ class AUDACITY_DLL_API Effect /* not final */ : public wxEvtHandler, // have the "selected" flag set to true, which is consistent with // Audacity's standard UI. // Create a user interface only if the supplied function is not null. - /* not virtual */ bool DoEffect(wxWindow *parent, double projectRate, TrackList *list, + /* not virtual */ bool DoEffect(wxWindow &parent, double projectRate, TrackList *list, TrackFactory *factory, NotifyingSelectedRegion &selectedRegion, const EffectDialogFactory &dialogFactory ); bool Delegate( Effect &delegate, - wxWindow *parent, const EffectDialogFactory &factory ); + wxWindow &parent, const EffectDialogFactory &factory ); virtual bool IsHidden(); diff --git a/src/effects/EffectManager.cpp b/src/effects/EffectManager.cpp index 0d7d2bd08..885cc6773 100644 --- a/src/effects/EffectManager.cpp +++ b/src/effects/EffectManager.cpp @@ -326,7 +326,7 @@ bool EffectManager::SetEffectParameters(const PluginID & ID, const wxString & pa bool EffectManager::PromptUser( const PluginID & ID, - const EffectClientInterface::EffectDialogFactory &factory, wxWindow *parent) + const EffectClientInterface::EffectDialogFactory &factory, wxWindow &parent) { bool result = false; Effect *effect = GetEffect(ID); @@ -342,7 +342,7 @@ bool EffectManager::PromptUser( if (command) { - result = command->PromptUser(parent); + result = command->PromptUser(&parent); return result; } diff --git a/src/effects/EffectManager.h b/src/effects/EffectManager.h index d206971ab..ea3f3be3e 100644 --- a/src/effects/EffectManager.h +++ b/src/effects/EffectManager.h @@ -107,7 +107,7 @@ public: bool SetEffectParameters(const PluginID & ID, const wxString & params); bool PromptUser( const PluginID & ID, const EffectClientInterface::EffectDialogFactory &factory, - wxWindow *parent ); + wxWindow &parent ); bool HasPresets(const PluginID & ID); wxString GetPreset(const PluginID & ID, const wxString & params, wxWindow * parent); wxString GetDefaultPreset(const PluginID & ID); diff --git a/src/effects/EffectUI.cpp b/src/effects/EffectUI.cpp index 15a6bb6ff..cf0253c1a 100644 --- a/src/effects/EffectUI.cpp +++ b/src/effects/EffectUI.cpp @@ -19,6 +19,7 @@ #include "Effect.h" #include "EffectManager.h" #include "../ProjectHistory.h" +#include "../ProjectWindowBase.h" #include "../TrackPanelAx.h" #include "RealtimeEffectManager.h" @@ -377,7 +378,7 @@ void EffectRack::OnEditor(wxCommandEvent & evt) } auto pEffect = mEffects[index]; - pEffect->ShowInterface( GetParent(), EffectUI::DialogFactory, + pEffect->ShowInterface( *GetParent(), EffectUI::DialogFactory, pEffect->IsBatchProcessing() ); } @@ -1812,15 +1813,22 @@ void EffectUIHost::CleanupRealtime() } } -wxDialog *EffectUI::DialogFactory( wxWindow *parent, EffectHostInterface *pHost, +wxDialog *EffectUI::DialogFactory( wxWindow &parent, EffectHostInterface *pHost, EffectUIClientInterface *client) { auto pEffect = dynamic_cast< Effect* >( pHost ); if ( ! pEffect ) return nullptr; + // Make sure there is an associated project, whose lifetime will + // govern the lifetime of the dialog, even when the dialog is + // non-modal, as for realtime effects + auto project = FindProjectFromWindow(&parent); + if ( !project ) + return nullptr; + Destroy_ptr dlg{ - safenew EffectUIHost{ parent, pEffect, client} }; + safenew EffectUIHost{ &parent, pEffect, client} }; if (dlg->Initialize()) { @@ -1913,7 +1921,7 @@ wxDialog *EffectUI::DialogFactory( wxWindow *parent, EffectHostInterface *pHost, EffectRack::Get( context.project ).Add(effect); } #endif - success = effect->DoEffect(&window, + success = effect->DoEffect(window, rate, &tracks, &trackFactory, diff --git a/src/effects/EffectUI.h b/src/effects/EffectUI.h index b51cf70b2..fac23d5ed 100644 --- a/src/effects/EffectUI.h +++ b/src/effects/EffectUI.h @@ -216,7 +216,7 @@ class CommandContext; namespace EffectUI { - wxDialog *DialogFactory( wxWindow *parent, EffectHostInterface *pHost, + wxDialog *DialogFactory( wxWindow &parent, EffectHostInterface *pHost, EffectUIClientInterface *client); /** Run an effect given the plugin ID */ diff --git a/src/effects/NoiseReduction.cpp b/src/effects/NoiseReduction.cpp index 71291f6f9..d6b81f145 100644 --- a/src/effects/NoiseReduction.cpp +++ b/src/effects/NoiseReduction.cpp @@ -210,7 +210,7 @@ public: ~Settings() {} bool PromptUser(EffectNoiseReduction *effect, - wxWindow *parent, bool bHasProfile, bool bAllowTwiddleSettings); + wxWindow &parent, bool bHasProfile, bool bAllowTwiddleSettings); bool PrefsIO(bool read); bool Validate(EffectNoiseReduction *effect) const; @@ -460,7 +460,7 @@ bool EffectNoiseReduction::CheckWhetherSkipEffect() } bool EffectNoiseReduction::ShowInterface( - wxWindow *parent, const EffectDialogFactory &, bool forceModal) + wxWindow &parent, const EffectDialogFactory &, bool forceModal) { // to do: use forceModal correctly @@ -474,11 +474,11 @@ bool EffectNoiseReduction::ShowInterface( } bool EffectNoiseReduction::Settings::PromptUser -(EffectNoiseReduction *effect, wxWindow *parent, +(EffectNoiseReduction *effect, wxWindow &parent, bool bHasProfile, bool bAllowTwiddleSettings) { EffectNoiseReduction::Dialog dlog - (effect, this, parent, bHasProfile, bAllowTwiddleSettings); + (effect, this, &parent, bHasProfile, bAllowTwiddleSettings); dlog.CentreOnParent(); dlog.ShowModal(); diff --git a/src/effects/NoiseReduction.h b/src/effects/NoiseReduction.h index 105d1dccf..f289765fe 100644 --- a/src/effects/NoiseReduction.h +++ b/src/effects/NoiseReduction.h @@ -38,7 +38,7 @@ public: // using Effect::TrackProgress; - bool ShowInterface( wxWindow *parent, + bool ShowInterface( wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false) override; bool Init() override; diff --git a/src/effects/NoiseRemoval.cpp b/src/effects/NoiseRemoval.cpp index fd2a6c790..c05f18ed1 100644 --- a/src/effects/NoiseRemoval.cpp +++ b/src/effects/NoiseRemoval.cpp @@ -150,10 +150,10 @@ bool EffectNoiseRemoval::CheckWhetherSkipEffect() } bool EffectNoiseRemoval::ShowInterface( - wxWindow *parent, const EffectDialogFactory &, bool forceModal /* forceModal */ ) + wxWindow &parent, const EffectDialogFactory &, bool forceModal /* forceModal */ ) { // to do: use forceModal correctly - NoiseRemovalDialog dlog(this, parent); + NoiseRemovalDialog dlog(this, &parent); dlog.mSensitivity = mSensitivity; dlog.mGain = -mNoiseGain; dlog.mFreq = mFreqSmoothingHz; diff --git a/src/effects/NoiseRemoval.h b/src/effects/NoiseRemoval.h index ada089852..478629daa 100644 --- a/src/effects/NoiseRemoval.h +++ b/src/effects/NoiseRemoval.h @@ -53,7 +53,7 @@ public: // Effect implementation - bool ShowInterface( wxWindow *parent, + bool ShowInterface( wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false) override; bool Init() override; bool CheckWhetherSkipEffect() override; diff --git a/src/effects/VST/VSTEffect.cpp b/src/effects/VST/VSTEffect.cpp index 1905cc089..904b0389b 100644 --- a/src/effects/VST/VSTEffect.cpp +++ b/src/effects/VST/VSTEffect.cpp @@ -1602,7 +1602,7 @@ bool VSTEffect::RealtimeProcessEnd() /// all provide the information (kn0ck0ut is one). /// bool VSTEffect::ShowInterface( - wxWindow *parent, const EffectDialogFactory &factory, bool forceModal) + wxWindow &parent, const EffectDialogFactory &factory, bool forceModal) { if (mDialog) { diff --git a/src/effects/VST/VSTEffect.h b/src/effects/VST/VSTEffect.h index 1831cb325..f31306f98 100644 --- a/src/effects/VST/VSTEffect.h +++ b/src/effects/VST/VSTEffect.h @@ -151,7 +151,7 @@ class VSTEffect final : public wxEvtHandler, size_t numSamples) override; bool RealtimeProcessEnd() override; - bool ShowInterface( wxWindow *parent, + bool ShowInterface( wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false) override; bool GetAutomationParameters(CommandParameters & parms) override; diff --git a/src/effects/audiounits/AudioUnitEffect.cpp b/src/effects/audiounits/AudioUnitEffect.cpp index 56fa25107..96a28e73f 100644 --- a/src/effects/audiounits/AudioUnitEffect.cpp +++ b/src/effects/audiounits/AudioUnitEffect.cpp @@ -1429,7 +1429,7 @@ bool AudioUnitEffect::RealtimeProcessEnd() } bool AudioUnitEffect::ShowInterface( - wxWindow *parent, const EffectDialogFactory &factory, bool forceModal) + wxWindow &parent, const EffectDialogFactory &factory, bool forceModal) { if (mDialog) { diff --git a/src/effects/audiounits/AudioUnitEffect.h b/src/effects/audiounits/AudioUnitEffect.h index 0db9e41d0..754a960be 100644 --- a/src/effects/audiounits/AudioUnitEffect.h +++ b/src/effects/audiounits/AudioUnitEffect.h @@ -101,7 +101,7 @@ public: size_t numSamples) override; bool RealtimeProcessEnd() override; - bool ShowInterface( wxWindow *parent, + bool ShowInterface( wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false) override; bool GetAutomationParameters(CommandParameters & parms) override; diff --git a/src/effects/ladspa/LadspaEffect.cpp b/src/effects/ladspa/LadspaEffect.cpp index 9c9391208..6a18481a0 100644 --- a/src/effects/ladspa/LadspaEffect.cpp +++ b/src/effects/ladspa/LadspaEffect.cpp @@ -1071,7 +1071,7 @@ bool LadspaEffect::RealtimeProcessEnd() } bool LadspaEffect::ShowInterface( - wxWindow *parent, const EffectDialogFactory &factory, bool forceModal) + wxWindow &parent, const EffectDialogFactory &factory, bool forceModal) { if (mDialog) { diff --git a/src/effects/ladspa/LadspaEffect.h b/src/effects/ladspa/LadspaEffect.h index 4e75d4d04..0cd7847ad 100644 --- a/src/effects/ladspa/LadspaEffect.h +++ b/src/effects/ladspa/LadspaEffect.h @@ -99,7 +99,7 @@ public: size_t numSamples) override; bool RealtimeProcessEnd() override; - bool ShowInterface( wxWindow *parent, + bool ShowInterface( wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false) override; bool GetAutomationParameters(CommandParameters & parms) override; diff --git a/src/effects/lv2/LV2Effect.cpp b/src/effects/lv2/LV2Effect.cpp index ff8903f14..777b97c25 100755 --- a/src/effects/lv2/LV2Effect.cpp +++ b/src/effects/lv2/LV2Effect.cpp @@ -1440,7 +1440,7 @@ bool LV2Effect::RealtimeProcessEnd() } bool LV2Effect::ShowInterface( - wxWindow *parent, const EffectDialogFactory &factory, bool forceModal) + wxWindow &parent, const EffectDialogFactory &factory, bool forceModal) { if (mDialog) { diff --git a/src/effects/lv2/LV2Effect.h b/src/effects/lv2/LV2Effect.h index f07a8bf3a..c3e0628e7 100755 --- a/src/effects/lv2/LV2Effect.h +++ b/src/effects/lv2/LV2Effect.h @@ -305,7 +305,7 @@ public: size_t RealtimeProcess(int group, float **inbuf, float **outbuf, size_t numSamples) override; bool RealtimeProcessEnd() override; - bool ShowInterface( wxWindow *parent, + bool ShowInterface( wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false) override; bool GetAutomationParameters(CommandParameters & parms) override; diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp index 91d5ad743..1e53463a5 100644 --- a/src/effects/nyquist/Nyquist.cpp +++ b/src/effects/nyquist/Nyquist.cpp @@ -977,7 +977,7 @@ finish: } bool NyquistEffect::ShowInterface( - wxWindow *parent, const EffectDialogFactory &factory, bool forceModal) + wxWindow &parent, const EffectDialogFactory &factory, bool forceModal) { // Show the normal (prompt or effect) interface bool res = Effect::ShowInterface(parent, factory, forceModal); diff --git a/src/effects/nyquist/Nyquist.h b/src/effects/nyquist/Nyquist.h index 6d7c9d613..b1ac01af4 100644 --- a/src/effects/nyquist/Nyquist.h +++ b/src/effects/nyquist/Nyquist.h @@ -99,7 +99,7 @@ public: bool Init() override; bool CheckWhetherSkipEffect() override; bool Process() override; - bool ShowInterface( wxWindow *parent, + bool ShowInterface( wxWindow &parent, const EffectDialogFactory &factory, bool forceModal = false) override; void PopulateOrExchange(ShuttleGui & S) override; bool TransferDataToWindow() override;