From a2cb7e96ea057de604003ecdc70745a80e4b40e0 Mon Sep 17 00:00:00 2001 From: Lukas W Date: Sat, 10 Feb 2018 13:24:59 +0100 Subject: [PATCH] Fix VST sub-window creation glitches in project loading Fixes bugs where during project loading (observed with VST effects), empty widgets and sub-windows would be left floating around. These were caused by inconsistencies between the way VST UIs were created when loading a project and when adding an effect in an existing project. In some situations, this caused createUI to be called twice, leaving over multiple empty widgets. This commit refactors some code in order to avoid creating unnecessary sub- windows, which aren't needed with VST effects, but were still created, usually being invisible. All sub-window related code was moved out of VstPlugin into vestige.cpp, which is the only place where sub-window VSTs are actually used. A new sub-class of VstPlugin, VstInstrumentPlugin, now handles VST sub-windows and is used by vestigeInstrument. "guivisible" attribute loading was moved out of VstPlugin as well and is now done in VstEffectControls' and vestigeInstrument's loadSettings method respectively. This causes some minor code duplication unfortunately. Closes #4110 --- plugins/VstEffect/VstEffectControlDialog.cpp | 26 ++-- plugins/VstEffect/VstEffectControlDialog.h | 3 +- plugins/VstEffect/VstEffectControls.cpp | 14 +- plugins/VstEffect/VstEffectControls.h | 6 +- plugins/vestige/vestige.cpp | 62 ++++++++- plugins/vst_base/VstPlugin.cpp | 136 +++++-------------- plugins/vst_base/VstPlugin.h | 15 +- 7 files changed, 136 insertions(+), 126 deletions(-) diff --git a/plugins/VstEffect/VstEffectControlDialog.cpp b/plugins/VstEffect/VstEffectControlDialog.cpp index 34ad097c9..ad2666392 100644 --- a/plugins/VstEffect/VstEffectControlDialog.cpp +++ b/plugins/VstEffect/VstEffectControlDialog.cpp @@ -45,6 +45,7 @@ VstEffectControlDialog::VstEffectControlDialog( VstEffectControls * _ctl ) : EffectControlDialog( _ctl ), m_pluginWidget( NULL ), + m_plugin( NULL ), tbLabel( NULL ) { @@ -62,16 +63,10 @@ VstEffectControlDialog::VstEffectControlDialog( VstEffectControls * _ctl ) : embed_vst = m_plugin->embedMethod() != "none"; if (embed_vst) { - m_plugin->createUI( nullptr, true ); - m_pluginWidget = m_plugin->pluginWidget( false ); - -#ifdef LMMS_BUILD_WIN32 - if( !m_pluginWidget ) - { - m_pluginWidget = m_plugin->pluginWidget( false ); + if (! m_plugin->pluginWidget()) { + m_plugin->createUI(nullptr); } -#endif - + m_pluginWidget = m_plugin->pluginWidget(); } } @@ -79,7 +74,7 @@ VstEffectControlDialog::VstEffectControlDialog( VstEffectControls * _ctl ) : { setWindowTitle( m_plugin->name() ); - QPushButton * btn = new QPushButton( tr( "Show/hide" ) ); + QPushButton * btn = new QPushButton( tr( "Show/hide" )); if (embed_vst) { btn->setCheckable( true ); @@ -95,6 +90,7 @@ VstEffectControlDialog::VstEffectControlDialog( VstEffectControls * _ctl ) : btn->setMaximumWidth( 78 ); btn->setMinimumHeight( 24 ); btn->setMaximumHeight( 24 ); + m_togglePluginButton = btn; m_managePluginButton = new PixmapButton( this, "" ); m_managePluginButton->setCheckable( false ); @@ -295,6 +291,14 @@ void VstEffectControlDialog::togglePluginUI( bool checked ) return; } - m_plugin->toggleUI(); + if ( m_togglePluginButton->isChecked() != checked ) { + m_togglePluginButton->setChecked( checked ); + } + + if ( checked ) { + m_plugin->showUI(); + } else { + m_plugin->hideUI(); + } } diff --git a/plugins/VstEffect/VstEffectControlDialog.h b/plugins/VstEffect/VstEffectControlDialog.h index ddbbef878..e20915019 100644 --- a/plugins/VstEffect/VstEffectControlDialog.h +++ b/plugins/VstEffect/VstEffectControlDialog.h @@ -54,6 +54,7 @@ protected: private: QWidget * m_pluginWidget; + QPushButton * m_togglePluginButton; PixmapButton * m_openPresetButton; PixmapButton * m_rolLPresetButton; PixmapButton * m_rolRPresetButton; @@ -64,7 +65,7 @@ private: QLabel * tbLabel; -private slots: +public slots: void togglePluginUI( bool checked ); } ; diff --git a/plugins/VstEffect/VstEffectControls.cpp b/plugins/VstEffect/VstEffectControls.cpp index 92688545b..ef5a59463 100644 --- a/plugins/VstEffect/VstEffectControls.cpp +++ b/plugins/VstEffect/VstEffectControls.cpp @@ -40,7 +40,8 @@ VstEffectControls::VstEffectControls( VstEffect * _eff ) : m_subWindow( NULL ), knobFModel( NULL ), ctrHandle( NULL ), - lastPosInMenu (0) + lastPosInMenu (0), + m_vstGuiVisible ( true ) // m_presetLabel ( NULL ) { } @@ -64,6 +65,8 @@ void VstEffectControls::loadSettings( const QDomElement & _this ) m_effect->m_pluginMutex.lock(); if( m_effect->m_plugin != NULL ) { + m_vstGuiVisible = _this.attribute( "guivisible" ).toInt(); + m_effect->m_plugin->loadSettings( _this ); const QMap & dump = m_effect->m_plugin->parameterDump(); @@ -144,6 +147,15 @@ int VstEffectControls::controlCount() +EffectControlDialog *VstEffectControls::createView() +{ + auto dialog = new VstEffectControlDialog( this ); + dialog->togglePluginUI( m_vstGuiVisible ); + return dialog; +} + + + void VstEffectControls::managePlugin( void ) { diff --git a/plugins/VstEffect/VstEffectControls.h b/plugins/VstEffect/VstEffectControls.h index 7328f2f42..e4f099fd1 100644 --- a/plugins/VstEffect/VstEffectControls.h +++ b/plugins/VstEffect/VstEffectControls.h @@ -59,10 +59,7 @@ public: virtual int controlCount(); - virtual EffectControlDialog * createView() - { - return new VstEffectControlDialog( this ); - } + virtual EffectControlDialog * createView(); protected slots: @@ -96,6 +93,7 @@ private: friend class VstEffectControlDialog; friend class manageVSTEffectView; + bool m_vstGuiVisible; } ; diff --git a/plugins/vestige/vestige.cpp b/plugins/vestige/vestige.cpp index db9cf5bba..c54c6c3a8 100644 --- a/plugins/vestige/vestige.cpp +++ b/plugins/vestige/vestige.cpp @@ -24,6 +24,8 @@ #include "vestige.h" +#include + #include #include #include @@ -73,6 +75,57 @@ Plugin::Descriptor PLUGIN_EXPORT vestige_plugin_descriptor = } +class vstSubWin : public QMdiSubWindow +{ +public: + vstSubWin( QWidget * _parent ) : + QMdiSubWindow( _parent ) + { + setAttribute( Qt::WA_DeleteOnClose, false ); + setWindowFlags( Qt::WindowCloseButtonHint ); + } + + virtual ~vstSubWin() + { + } + + virtual void closeEvent( QCloseEvent * e ) + { + // ignore close-events - for some reason otherwise the VST GUI + // remains hidden when re-opening + hide(); + e->ignore(); + } +}; + + +class VstInstrumentPlugin : public VstPlugin +{ +public: + using VstPlugin::VstPlugin; + + void createUI( QWidget *parent ) override + { + Q_UNUSED(parent); + VstPlugin::createUI( nullptr ); + if ( embedMethod() != "none" ) { + m_pluginSubWindow.reset(new vstSubWin( gui->mainWindow()->workspace() )); + m_pluginSubWindow->setWidget(pluginWidget()); + } + } + + /// Overwrite editor() to return the sub window instead of the embed widget + /// itself. This makes toggleUI() and related functions toggle the + /// sub window's visibility. + QWidget* editor() override + { + return m_pluginSubWindow.get(); + } +private: + unique_ptr m_pluginSubWindow; +}; + + QPixmap * VestigeInstrumentView::s_artwork = NULL; QPixmap * manageVestigeInstrumentView::s_artwork = NULL; @@ -127,6 +180,12 @@ void vestigeInstrument::loadSettings( const QDomElement & _this ) { m_plugin->loadSettings( _this ); + if ( _this.attribute( "guivisible" ).toInt() ) { + m_plugin->showUI(); + } else { + m_plugin->hideUI(); + } + const QMap & dump = m_plugin->parameterDump(); paramCount = dump.size(); char paramStr[35]; @@ -267,7 +326,7 @@ void vestigeInstrument::loadFile( const QString & _file ) } m_pluginMutex.lock(); - m_plugin = new VstPlugin( m_pluginDLL ); + m_plugin = new VstInstrumentPlugin( m_pluginDLL ); if( m_plugin->failed() ) { m_pluginMutex.unlock(); @@ -278,6 +337,7 @@ void vestigeInstrument::loadFile( const QString & _file ) return; } + m_plugin->createUI(nullptr); m_plugin->showUI(); if( set_ch_name ) diff --git a/plugins/vst_base/VstPlugin.cpp b/plugins/vst_base/VstPlugin.cpp index 5c7504dd1..32230cd8d 100644 --- a/plugins/vst_base/VstPlugin.cpp +++ b/plugins/vst_base/VstPlugin.cpp @@ -62,28 +62,6 @@ #include "templates.h" #include "FileDialog.h" -class vstSubWin : public QMdiSubWindow -{ -public: - vstSubWin( QWidget * _parent ) : - QMdiSubWindow( _parent ) - { - setAttribute( Qt::WA_DeleteOnClose, false ); - } - - virtual ~vstSubWin() - { - } - - virtual void closeEvent( QCloseEvent * e ) - { - // ignore close-events - for some reason otherwise the VST GUI - // remains hidden when re-opening - hide(); - e->ignore(); - } -} ; - VstPlugin::VstPlugin( const QString & _plugin ) : m_plugin( _plugin ), @@ -124,7 +102,6 @@ VstPlugin::VstPlugin( const QString & _plugin ) : VstPlugin::~VstPlugin() { - delete m_pluginSubWindow; delete m_pluginWidget; } @@ -174,41 +151,8 @@ void VstPlugin::tryLoad( const QString &remoteVstPluginExecutable ) -void VstPlugin::hideEditor() -{ - QWidget * w = pluginWidget(); - if( w ) - { - w->hide(); - } -} - - - - -void VstPlugin::toggleEditor() -{ - QWidget * w = pluginWidget(); - if( w ) - { - w->setVisible( !w->isVisible() ); - } -} - - - - void VstPlugin::loadSettings( const QDomElement & _this ) { - if( _this.attribute( "guivisible" ).toInt() ) - { - showUI(); - } - else - { - hideUI(); - } - const int num_params = _this.attribute( "numparams" ).toInt(); // if it exists try to load settings chunk if( _this.hasAttribute( "chunk" ) ) @@ -286,7 +230,7 @@ void VstPlugin::toggleUI() } else if (pluginWidget()) { - toggleEditor(); + toggleEditorVisibility(); } } @@ -362,21 +306,9 @@ void VstPlugin::setParameterDump( const QMap & _pdump ) unlock(); } -QWidget *VstPlugin::pluginWidget(bool _top_widget) +QWidget *VstPlugin::pluginWidget() { - if ( m_embedMethod == "none" || !m_pluginWidget ) - { - return nullptr; - } - - if ( _top_widget && m_pluginWidget->parentWidget() == m_pluginSubWindow ) - { - return m_pluginSubWindow; - } - else - { - return m_pluginWidget; - } + return m_pluginWidget; } @@ -458,6 +390,10 @@ bool VstPlugin::processMessage( const message & _m ) } +QWidget *VstPlugin::editor() +{ + return m_pluginWidget; +} void VstPlugin::openPreset( ) @@ -579,15 +515,10 @@ void VstPlugin::showUI() } else if ( m_embedMethod != "headless" ) { - if (! pluginWidget()) { - createUI( NULL, false ); - } - - QWidget * w = pluginWidget(); - if( w ) - { - w->show(); + if (! editor()) { + qWarning() << "VstPlugin::showUI called before VstPlugin::createUI"; } + toggleEditorVisibility( true ); } } @@ -599,7 +530,7 @@ void VstPlugin::hideUI() } else if ( pluginWidget() != nullptr ) { - hideEditor(); + toggleEditorVisibility( false ); } } @@ -654,22 +585,39 @@ QByteArray VstPlugin::saveChunk() return a; } -void VstPlugin::createUI( QWidget * parent, bool isEffect ) +void VstPlugin::toggleEditorVisibility( int visible ) { + QWidget* w = editor(); + if ( ! w ) { + return; + } + + if ( visible < 0 ) { + visible = ! w->isVisible(); + } + w->setVisible( visible ); +} + +void VstPlugin::createUI( QWidget * parent ) +{ + if ( m_pluginWidget ) { + qWarning() << "VstPlugin::createUI called twice"; + m_pluginWidget->setParent( parent ); + return; + } + if( m_pluginWindowID == 0 ) { return; } QWidget* container = nullptr; - m_pluginSubWindow = new vstSubWin( gui->mainWindow()->workspace() ); - auto sw = m_pluginSubWindow.data(); #if QT_VERSION >= 0x050100 if (m_embedMethod == "qt" ) { QWindow* vw = QWindow::fromWinId(m_pluginWindowID); - container = QWidget::createWindowContainer(vw, sw ); + container = QWidget::createWindowContainer(vw, parent ); container->installEventFilter(this); } else #endif @@ -708,7 +656,7 @@ void VstPlugin::createUI( QWidget * parent, bool isEffect ) #ifdef LMMS_BUILD_LINUX if (m_embedMethod == "xembed" ) { - QX11EmbedContainer * embedContainer = new QX11EmbedContainer( sw ); + QX11EmbedContainer * embedContainer = new QX11EmbedContainer( parent ); connect(embedContainer, SIGNAL(clientIsEmbedded()), this, SLOT(handleClientEmbed())); embedContainer->embedClient( m_pluginWindowID ); container = embedContainer; @@ -716,29 +664,13 @@ void VstPlugin::createUI( QWidget * parent, bool isEffect ) #endif { qCritical() << "Unknown embed method" << m_embedMethod; - delete m_pluginSubWindow; return; } container->setFixedSize( m_pluginGeometry ); container->setWindowTitle( name() ); - if( parent == NULL ) - { - m_pluginWidget = container; - - sw->setWidget(container); - - if( isEffect ) - { - sw->setAttribute( Qt::WA_TranslucentBackground ); - sw->setWindowFlags( Qt::FramelessWindowHint ); - } - else - { - sw->setWindowFlags( Qt::WindowCloseButtonHint ); - } - }; + m_pluginWidget = container; container->setFixedSize( m_pluginGeometry ); } diff --git a/plugins/vst_base/VstPlugin.h b/plugins/vst_base/VstPlugin.h index f3d6bea8e..9e8b39771 100644 --- a/plugins/vst_base/VstPlugin.h +++ b/plugins/vst_base/VstPlugin.h @@ -54,8 +54,10 @@ public: return m_pluginWindowID != 0; } - void hideEditor(); - void toggleEditor(); + /// Same as pluginWidget(), but can be overwritten in sub-classes to modify + /// behavior the UI. This is used in VstInstrumentPlugin to wrap the VST UI + /// in a QMdiSubWindow + virtual QWidget* editor(); inline const QString & name() const { @@ -93,7 +95,7 @@ public: void setParameterDump( const QMap & _pdump ); - QWidget * pluginWidget( bool _top_widget = true ); + QWidget * pluginWidget(); virtual void loadSettings( const QDomElement & _this ); virtual void saveSettings( QDomDocument & _doc, QDomElement & _this ); @@ -103,9 +105,8 @@ public: return "vstplugin"; } - void toggleUI() override; - void createUI( QWidget *parent, bool isEffect ); + virtual void createUI(QWidget *parent); bool eventFilter(QObject *obj, QEvent *event); QString embedMethod() const; @@ -123,6 +124,7 @@ public slots: void showUI() override; void hideUI() override; + void toggleUI() override; void handleClientEmbed(); @@ -130,9 +132,10 @@ private: void loadChunk( const QByteArray & _chunk ); QByteArray saveChunk(); + void toggleEditorVisibility(int visible = -1); + QString m_plugin; QPointer m_pluginWidget; - QPointer m_pluginSubWindow; int m_pluginWindowID; QSize m_pluginGeometry; const QString m_embedMethod;