Remove CreateUI; pass dialog factory to effect client instead

include/audacity/EffectInterface.h

@@ -42,6 +42,8 @@
 #ifndef __AUDACITY_EFFECTINTERFACE_H__
 #define __AUDACITY_EFFECTINTERFACE_H__
 
+#include <functional>
+
 #include "audacity/Types.h"
 #include "audacity/ComponentInterface.h"
 #include "audacity/ConfigInterface.h"
@@ -123,9 +125,6 @@ public:
 
    virtual void Preview() = 0;
 
-   //
-   virtual wxDialog *CreateUI(wxWindow *parent, EffectUIClientInterface *client) = 0;
-
    // Preset handling
    virtual RegistryPath GetUserPresetsGroup(const RegistryPath & name) = 0;
    virtual RegistryPath GetCurrentSettingsGroup() = 0;
@@ -144,6 +143,10 @@ AudacityCommand.
 class AUDACITY_DLL_API EffectClientInterface  /* not final */ : public EffectDefinitionInterface
 {
 public:
+   using EffectDialogFactory = std::function<
+      wxDialog* ( wxWindow*, EffectHostInterface*, EffectUIClientInterface* )
+   >;
+
    virtual ~EffectClientInterface() {};
 
    virtual bool SetHost(EffectHostInterface *host) = 0;
@@ -177,7 +180,10 @@ public:
    virtual size_t RealtimeProcess(int group, float **inBuf, float **outBuf, size_t numSamples) = 0;
    virtual bool RealtimeProcessEnd() = 0;
 
-   virtual bool ShowInterface(wxWindow *parent, bool forceModal = false) = 0;
+   virtual bool ShowInterface(
+      wxWindow *parent, const EffectDialogFactory &factory,
+      bool forceModal = false
+   ) = 0;
    // Some effects will use define params to define what parameters they take.
    // If they do, they won't need to implement Get or SetAutomation parameters.
    // since the Effect class can do it.  Or at least that is how things happen

src/BatchCommands.cpp

@@ -31,6 +31,7 @@ processing.  See also MacrosWindow and ApplyMacroDialog.
 #include "ProjectWindow.h"
 #include "commands/CommandManager.h"
 #include "effects/EffectManager.h"
+#include "effects/EffectUI.h"
 #include "FileNames.h"
 #include "Menus.h"
 #include "PluginManager.h"
@@ -452,7 +453,7 @@ wxString MacroCommands::PromptForParamsFor(const CommandID & command, const wxSt
 
    if (EffectManager::Get().SetEffectParameters(ID, params))
    {
-      if (EffectManager::Get().PromptUser(ID, parent))
+      if (EffectManager::Get().PromptUser(ID, EffectUI::DialogFactory, parent))
       {
          res = EffectManager::Get().GetEffectParameters(ID);
       }

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, false);
+      return Delegate(proxy, mUIParent, nullptr);
    }
    else
 #endif

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, false);
+      success = Delegate(proxy, mUIParent, nullptr);
    }
    else
 #endif

src/effects/Effect.cpp

@@ -478,7 +478,8 @@ bool Effect::RealtimeProcessEnd()
    return true;
 }
 
-bool Effect::ShowInterface(wxWindow *parent, bool forceModal)
+bool Effect::ShowInterface(wxWindow *parent,
+   const EffectDialogFactory &factory, bool forceModal)
 {
    if (!IsInteractive())
    {
@@ -494,13 +495,14 @@ bool Effect::ShowInterface(wxWindow *parent, bool forceModal)
 
    if (mClient)
    {
-      return mClient->ShowInterface(parent, forceModal);
+      return mClient->ShowInterface(parent, factory, forceModal);
    }
 
    // mUIDialog is null
    auto cleanup = valueRestorer( mUIDialog );
-
-   mUIDialog = CreateUI(parent, this);
+   
+   if ( factory )
+      mUIDialog = factory(parent, this, this);
    if (!mUIDialog)
    {
       return false;
@@ -731,21 +733,6 @@ void Effect::Preview()
    Preview(false);
 }
 
-#include "EffectUI.h"
-wxDialog *Effect::CreateUI(wxWindow *parent, EffectUIClientInterface *client)
-{
-   Destroy_ptr<EffectUIHost> dlg
-      { safenew EffectUIHost{ parent, this, client} };
-
-   if (dlg->Initialize())
-   {
-      // release() is safe because parent will own it
-      return dlg.release();
-   }
-
-   return NULL;
-}
-
 RegistryPath Effect::GetUserPresetsGroup(const RegistryPath & name)
 {
    RegistryPath group = wxT("UserPresets");
@@ -1114,7 +1101,7 @@ bool Effect::DoEffect(wxWindow *parent,
                       TrackList *list,
                       TrackFactory *factory,
                       NotifyingSelectedRegion &selectedRegion,
-                      bool shouldPrompt /* = true */)
+                      const EffectDialogFactory &dialogFactory)
 {
    wxASSERT(selectedRegion.duration() >= 0.0);
 
@@ -1200,9 +1187,9 @@ bool Effect::DoEffect(wxWindow *parent,
    // Prompting will be bypassed when applying an effect that has already
    // been configured, e.g. repeating the last effect on a different selection.
    // Prompting may call Effect::Preview
-   if ( shouldPrompt &&
+   if ( dialogFactory &&
       IsInteractive() &&
-      !ShowInterface(parent, IsBatchProcessing()) )
+      !ShowInterface(parent, dialogFactory, IsBatchProcessing()) )
    {
       return false;
    }
@@ -1231,13 +1218,14 @@ bool Effect::DoEffect(wxWindow *parent,
    return returnVal;
 }
 
-bool Effect::Delegate( Effect &delegate, wxWindow *parent, bool shouldPrompt)
+bool Effect::Delegate(
+   Effect &delegate, wxWindow *parent, const EffectDialogFactory &factory )
 {
    NotifyingSelectedRegion region;
    region.setTimes( mT0, mT1 );
 
    return delegate.DoEffect( parent, mProjectRate, mTracks, mFactory,
-      region, shouldPrompt );
+      region, factory );
 }
 
 // All legacy effects should have this overridden

src/effects/Effect.h

@@ -16,6 +16,7 @@
 
 #include "../Experimental.h"
 
+#include <functional>
 #include <set>
 
 #include <wx/defs.h>
@@ -141,7 +142,8 @@ class AUDACITY_DLL_API Effect /* not final */ : public wxEvtHandler,
                                        size_t numSamples) override;
    bool RealtimeProcessEnd() override;
 
-   bool ShowInterface(wxWindow *parent, bool forceModal = false) override;
+   bool ShowInterface( wxWindow *parent,
+      const EffectDialogFactory &factory, bool forceModal = false) override;
 
    bool GetAutomationParameters(CommandParameters & parms) override;
    bool SetAutomationParameters(CommandParameters & parms) override;
@@ -179,8 +181,6 @@ class AUDACITY_DLL_API Effect /* not final */ : public wxEvtHandler,
 
    void Preview() override;
 
-   wxDialog *CreateUI(wxWindow *parent, EffectUIClientInterface *client) override;
-
    RegistryPath GetUserPresetsGroup(const RegistryPath & name) override;
    RegistryPath GetCurrentSettingsGroup() override;
    RegistryPath GetFactoryDefaultsGroup() override;
@@ -256,11 +256,13 @@ class AUDACITY_DLL_API Effect /* not final */ : public wxEvtHandler,
    // Returns true on success.  Will only operate on tracks that
    // 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,
-                 TrackFactory *factory, NotifyingSelectedRegion &selectedRegion,
-                 bool shouldPrompt = true);
+      TrackFactory *factory, NotifyingSelectedRegion &selectedRegion,
+      const EffectDialogFactory &dialogFactory );
 
-   bool Delegate( Effect &delegate, wxWindow *parent, bool shouldPrompt);
+   bool Delegate( Effect &delegate,
+      wxWindow *parent, const EffectDialogFactory &factory );
 
    virtual bool IsHidden();
 

src/effects/EffectManager.cpp

@@ -31,9 +31,7 @@ effects.
 
 #include "../widgets/AudacityMessageBox.h"
 
-#if defined(EXPERIMENTAL_EFFECTS_RACK)
 #include "EffectUI.h"
-#endif
 
 #include "../ShuttleGetDefinition.h"
 #include "../commands/CommandContext.h"
@@ -234,7 +232,7 @@ bool EffectManager::DoEffect(const PluginID & ID,
                              AudacityProject &project,
                              double projectRate,
                              TrackList *list,
-                             TrackFactory *factory,
+                             TrackFactory *trackFactory,
                              NotifyingSelectedRegion &selectedRegion,
                              bool shouldPrompt /* = true */)
 
@@ -256,12 +254,12 @@ bool EffectManager::DoEffect(const PluginID & ID,
    (void)project;
 #endif
 
-   bool res = effect->DoEffect(parent,
-                               projectRate,
-                               list,
-                               factory,
-                               selectedRegion,
-                               shouldPrompt);
+   bool res = effect->DoEffect( parent,
+      projectRate,
+      list,
+      trackFactory,
+      selectedRegion,
+      shouldPrompt ?  EffectUI::DialogFactory : nullptr );
 
    return res;
 }
@@ -516,14 +514,17 @@ bool EffectManager::SetEffectParameters(const PluginID & ID, const wxString & pa
    return false;
 }
 
-bool EffectManager::PromptUser(const PluginID & ID, wxWindow *parent)
+bool EffectManager::PromptUser(
+   const PluginID & ID,
+   const EffectClientInterface::EffectDialogFactory &factory, wxWindow *parent)
 {
    bool result = false;
    Effect *effect = GetEffect(ID);
 
    if (effect)
    {
-      result = effect->ShowInterface( parent, effect->IsBatchProcessing() );
+      result = effect->ShowInterface(
+         parent, factory, effect->IsBatchProcessing() );
       return result;
    }
 

src/effects/EffectManager.h

@@ -18,6 +18,7 @@
 #include <vector>
 
 #include <unordered_map>
+#include "audacity/EffectInterface.h"
 #include "audacity/Types.h"
 
 class AudacityCommand;
@@ -120,7 +121,9 @@ public:
    bool SupportsAutomation(const PluginID & ID);
    wxString GetEffectParameters(const PluginID & ID);
    bool SetEffectParameters(const PluginID & ID, const wxString & params);
-   bool PromptUser(const PluginID & ID, wxWindow *parent);
+   bool PromptUser( const PluginID & ID,
+      const EffectClientInterface::EffectDialogFactory &factory,
+      wxWindow *parent );
    bool HasPresets(const PluginID & ID);
    wxString GetPreset(const PluginID & ID, const wxString & params, wxWindow * parent);
    wxString GetDefaultPreset(const PluginID & ID);

src/effects/EffectUI.cpp

@@ -376,7 +376,8 @@ void EffectRack::OnEditor(wxCommandEvent & evt)
    }
 
    auto pEffect = mEffects[index];
-   pEffect->ShowInterface( GetParent(), pEffect->IsBatchProcessing() );
+   pEffect->ShowInterface( GetParent(), EffectUI::DialogFactory,
+      pEffect->IsBatchProcessing() );
 }
 
 void EffectRack::OnUp(wxCommandEvent & evt)
@@ -1809,3 +1810,22 @@ void EffectUIHost::CleanupRealtime()
       mInitialized = false;
    }
 }
+
+wxDialog *EffectUI::DialogFactory( wxWindow *parent, EffectHostInterface *pHost,
+   EffectUIClientInterface *client)
+{
+   auto pEffect = dynamic_cast< Effect* >( pHost );
+   if ( ! pEffect )
+      return nullptr;
+
+   Destroy_ptr<EffectUIHost> dlg{
+      safenew EffectUIHost{ parent, pEffect, client} };
+   
+   if (dlg->Initialize())
+   {
+      // release() is safe because parent will own it
+      return dlg.release();
+   }
+   
+   return nullptr;
+};

src/effects/EffectUI.h

@@ -212,4 +212,9 @@ private:
    DECLARE_EVENT_TABLE()
 };
 
+namespace  EffectUI {
+   wxDialog *DialogFactory( wxWindow *parent, EffectHostInterface *pHost,
+      EffectUIClientInterface *client);
+}
+
 #endif // __AUDACITY_EFFECTUI_H__

src/effects/NoiseReduction.cpp

@@ -459,10 +459,13 @@ bool EffectNoiseReduction::CheckWhetherSkipEffect()
    return false;
 }
 
-bool EffectNoiseReduction::ShowInterface( wxWindow *parent, bool )
+bool EffectNoiseReduction::ShowInterface(
+   wxWindow *parent, const EffectDialogFactory &, bool forceModal)
 {
    // to do: use forceModal correctly
 
+   // Doesn't use the factory but substitutes its own dialog
+
    // We may want to twiddle the levels if we are setting
    // from an automation dialog, the only case in which we can
    // get here without any wavetracks.

src/effects/NoiseReduction.h

@@ -38,7 +38,8 @@ public:
 
 //   using Effect::TrackProgress;
 
-   bool ShowInterface( wxWindow *parent, bool forceModal ) override;
+   bool ShowInterface( wxWindow *parent,
+      const EffectDialogFactory &factory, bool forceModal = false) override;
 
    bool Init() override;
    bool CheckWhetherSkipEffect() override;

src/effects/NoiseRemoval.cpp

@@ -149,7 +149,8 @@ bool EffectNoiseRemoval::CheckWhetherSkipEffect()
    return (mLevel == 0);
 }
 
-bool EffectNoiseRemoval::ShowInterface( wxWindow *parent, bool /* forceModal */ )
+bool EffectNoiseRemoval::ShowInterface(
+   wxWindow *parent, const EffectDialogFactory &, bool forceModal  /* forceModal */ )
 {
    // to do: use forceModal correctly
    NoiseRemovalDialog dlog(this, parent);

src/effects/NoiseRemoval.h

@@ -53,7 +53,8 @@ public:
 
    // Effect implementation
 
-   bool ShowInterface( wxWindow *parent, bool forceModal ) override;
+   bool ShowInterface( wxWindow *parent,
+      const EffectDialogFactory &factory, bool forceModal = false) override;
    bool Init() override;
    bool CheckWhetherSkipEffect() override;
    bool Process() override;

src/effects/VST/VSTEffect.cpp

@@ -1601,7 +1601,8 @@ bool VSTEffect::RealtimeProcessEnd()
 /// provided by the effect, so it will not work with all effects since they don't
 /// all provide the information (kn0ck0ut is one).
 ///
-bool VSTEffect::ShowInterface(wxWindow *parent, bool forceModal)
+bool VSTEffect::ShowInterface(
+   wxWindow *parent, const EffectDialogFactory &factory, bool forceModal)
 {
    if (mDialog)
    {
@@ -1624,7 +1625,8 @@ bool VSTEffect::ShowInterface(wxWindow *parent, bool forceModal)
       ProcessInitialize(0, NULL);
    }
 
-   mDialog = mHost->CreateUI(parent, this);
+   if ( factory )
+      mDialog = factory(parent, mHost, this);
    if (!mDialog)
    {
       return false;

src/effects/VST/VSTEffect.h

@@ -151,7 +151,8 @@ class VSTEffect final : public wxEvtHandler,
                                        size_t numSamples) override;
    bool RealtimeProcessEnd() override;
 
-   bool ShowInterface(wxWindow *parent, bool forceModal = false) override;
+   bool ShowInterface( wxWindow *parent,
+      const EffectDialogFactory &factory, bool forceModal = false) override;
 
    bool GetAutomationParameters(CommandParameters & parms) override;
    bool SetAutomationParameters(CommandParameters & parms) override;

src/effects/audiounits/AudioUnitEffect.cpp

@@ -1428,7 +1428,8 @@ bool AudioUnitEffect::RealtimeProcessEnd()
    return true;
 }
 
-bool AudioUnitEffect::ShowInterface(wxWindow *parent, bool forceModal)
+bool AudioUnitEffect::ShowInterface(
+   wxWindow *parent, const EffectDialogFactory &factory, bool forceModal)
 {
    if (mDialog)
    {
@@ -1440,7 +1441,8 @@ bool AudioUnitEffect::ShowInterface(wxWindow *parent, bool forceModal)
    // mDialog is null
    auto cleanup = valueRestorer( mDialog );
 
-   mDialog = mHost->CreateUI(parent, this);
+   if ( factory )
+      mDialog = factory(parent, mHost, this);
    if (!mDialog)
    {
       return false;

src/effects/audiounits/AudioUnitEffect.h

@@ -101,7 +101,8 @@ public:
                                        size_t numSamples) override;
    bool RealtimeProcessEnd() override;
 
-   bool ShowInterface(wxWindow *parent, bool forceModal = false) override;
+   bool ShowInterface( wxWindow *parent,
+      const EffectDialogFactory &factory, bool forceModal = false) override;
 
    bool GetAutomationParameters(CommandParameters & parms) override;
    bool SetAutomationParameters(CommandParameters & parms) override;

src/effects/ladspa/LadspaEffect.cpp

@@ -1070,7 +1070,8 @@ bool LadspaEffect::RealtimeProcessEnd()
    return true;
 }
 
-bool LadspaEffect::ShowInterface(wxWindow *parent, bool forceModal)
+bool LadspaEffect::ShowInterface(
+   wxWindow *parent, const EffectDialogFactory &factory, bool forceModal)
 {
    if (mDialog)
    {
@@ -1082,7 +1083,8 @@ bool LadspaEffect::ShowInterface(wxWindow *parent, bool forceModal)
    // mDialog is null
    auto cleanup = valueRestorer( mDialog );
 
-   mDialog = mHost->CreateUI(parent, this);
+   if ( factory )
+      mDialog = factory(parent, mHost, this);
    if (!mDialog)
    {
       return false;

src/effects/ladspa/LadspaEffect.h

@@ -99,7 +99,8 @@ public:
                                        size_t numSamples) override;
    bool RealtimeProcessEnd() override;
 
-   bool ShowInterface(wxWindow *parent, bool forceModal = false) override;
+   bool ShowInterface( wxWindow *parent,
+      const EffectDialogFactory &factory, bool forceModal = false) override;
 
    bool GetAutomationParameters(CommandParameters & parms) override;
    bool SetAutomationParameters(CommandParameters & parms) override;

src/effects/lv2/LV2Effect.cpp

@@ -1439,7 +1439,8 @@ bool LV2Effect::RealtimeProcessEnd()
    return true;
 }
 
-bool LV2Effect::ShowInterface(wxWindow *parent, bool forceModal)
+bool LV2Effect::ShowInterface(
+   wxWindow *parent, const EffectDialogFactory &factory, bool forceModal)
 {
    if (mDialog)
    {
@@ -1453,7 +1454,8 @@ bool LV2Effect::ShowInterface(wxWindow *parent, bool forceModal)
    // mDialog is null
    auto cleanup = valueRestorer(mDialog);
 
-   mDialog = mHost->CreateUI(parent, this);
+   if ( factory )
+      mDialog = factory(parent, mHost, this);
    if (!mDialog)
    {
       return false;

src/effects/lv2/LV2Effect.h

@@ -305,7 +305,8 @@ public:
    size_t RealtimeProcess(int group, float **inbuf, float **outbuf, size_t numSamples) override;
    bool RealtimeProcessEnd() override;
 
-   bool ShowInterface(wxWindow *parent, bool forceModal = false) override;
+   bool ShowInterface( wxWindow *parent,
+      const EffectDialogFactory &factory, bool forceModal = false) override;
 
    bool GetAutomationParameters(CommandParameters & parms) override;
    bool SetAutomationParameters(CommandParameters & parms) override;

src/effects/nyquist/Nyquist.cpp

@@ -976,10 +976,11 @@ finish:
    return success;
 }
 
-bool NyquistEffect::ShowInterface(wxWindow *parent, bool forceModal)
+bool NyquistEffect::ShowInterface(
+   wxWindow *parent, const EffectDialogFactory &factory, bool forceModal)
 {
    // Show the normal (prompt or effect) interface
-   bool res = Effect::ShowInterface(parent, forceModal);
+   bool res = Effect::ShowInterface(parent, factory, forceModal);
 
    // Remember if the user clicked debug
    mDebug = (mUIResultID == eDebugID);
@@ -995,7 +996,7 @@ bool NyquistEffect::ShowInterface(wxWindow *parent, bool forceModal)
 
    effect.SetCommand(mInputCmd);
    effect.mDebug = (mUIResultID == eDebugID);
-   bool result = Delegate(effect, parent, true);
+   bool result = Delegate(effect, parent, factory);
    mT0 = effect.mT0;
    mT1 = effect.mT1;
    return result;

src/effects/nyquist/Nyquist.h

@@ -99,7 +99,8 @@ public:
    bool Init() override;
    bool CheckWhetherSkipEffect() override;
    bool Process() override;
-   bool ShowInterface(wxWindow *parent, bool forceModal = false) override;
+   bool ShowInterface( wxWindow *parent,
+      const EffectDialogFactory &factory, bool forceModal = false) override;
    void PopulateOrExchange(ShuttleGui & S) override;
    bool TransferDataToWindow() override;
    bool TransferDataFromWindow() override;