From 1410bb31a16d41a9ad9d1a9f5b0caf7300fb0e26 Mon Sep 17 00:00:00 2001 From: Nature Is Frequency Date: Fri, 9 Feb 2024 23:52:18 +0000 Subject: [PATCH 1/2] Enable/Disable Menu items based on plugin loaded Per @Trinitou suggestions, a plugin specific menu item should only be enabled (available to execute by user) This fixes the underlying cause of the crashes reported in #40 and #42 Therefore this also reverts the now obsolete change in previous merged pull request #43 Upon load change we update the menu items Tab indentation set to 3 to be consistent with existing code Note: Follows the QT memory ownership model, and the QAction pointers lifetime is handled by their parents Testing: Launch with no plugin loaded, see menu items are greyed out (disabled), user cannot select these items (as intended) therefore code is not executed and app doesn't crash Launch with plugin loaded, see menu items are available and executed when user clicks, and confirm still no crashes Images taken for testing --- host/main-window.cc | 35 ++++++++++++++++++++++++++++++----- host/main-window.hh | 10 ++++++++++ host/plugin-host.cc | 11 +++++------ 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/host/main-window.cc b/host/main-window.cc index feee8a1..79cc827 100644 --- a/host/main-window.cc +++ b/host/main-window.cc @@ -46,7 +46,9 @@ void MainWindow::createMenu() { QMenu *fileMenu = menuBar->addMenu(tr("File")); // TODO: fileMenu->addAction(tr("Load plugin")); - connect(fileMenu->addAction(tr("Load Native Plugin Preset")), + + _loadPluginPresetAction = fileMenu->addAction(tr("Load Native Plugin Preset")); + connect(_loadPluginPresetAction, &QAction::triggered, this, &MainWindow::loadNativePluginPreset); @@ -62,11 +64,15 @@ void MainWindow::createMenu() { &Application::quit); auto windowsMenu = menuBar->addMenu("Windows"); - connect(windowsMenu->addAction(tr("Show Parameters")), + + _showPluginParametersAction = windowsMenu->addAction(tr("Show Parameters")); + connect(_showPluginParametersAction, &QAction::triggered, this, &MainWindow::showPluginParametersWindow); - connect(windowsMenu->addAction(tr("Show Quick Controls")), + + _showPluginQuickControlsAction = windowsMenu->addAction(tr("Show Quick Controls")); + connect(_showPluginQuickControlsAction, &QAction::triggered, this, &MainWindow::showPluginQuickControlsWindow); @@ -76,11 +82,15 @@ void MainWindow::createMenu() { dialog.exec(); }); menuBar->addSeparator(); - connect(windowsMenu->addAction(tr("Toggle Plugin Window Visibility")), + + _togglePluginWindowVisibilityAction = windowsMenu->addAction(tr("Toggle Plugin Window Visibility")); + connect(_togglePluginWindowVisibilityAction, &QAction::triggered, this, &MainWindow::togglePluginWindowVisibility); - connect(windowsMenu->addAction(tr("Recreate Plugin Window")), + + _recreatePluginWindowAction = windowsMenu->addAction(tr("Recreate Plugin Window")); + connect(_recreatePluginWindowAction, &QAction::triggered, this, &MainWindow::recreatePluginWindow); @@ -88,6 +98,16 @@ void MainWindow::createMenu() { QMenu *helpMenu = menuBar->addMenu(tr("Help")); connect( helpMenu->addAction(tr("About")), &QAction::triggered, this, &MainWindow::showAboutDialog); + + updateMenuItems(); +} + +void MainWindow::updateMenuItems() { + _loadPluginPresetAction->setEnabled(_pluginLoaded); + _showPluginParametersAction->setEnabled(_pluginLoaded); + _showPluginQuickControlsAction->setEnabled(_pluginLoaded); + _togglePluginWindowVisibilityAction->setEnabled(_pluginLoaded); + _recreatePluginWindowAction->setEnabled(_pluginLoaded); } void MainWindow::showSettingsDialog() { @@ -117,6 +137,11 @@ void MainWindow::resizePluginView(int width, int height) { adjustSize(); } +void MainWindow::onPluginLoadChange(bool const i_pluginLoaded) { + _pluginLoaded = i_pluginLoaded; + updateMenuItems(); +} + void MainWindow::loadNativePluginPreset() { auto file = QFileDialog::getOpenFileName(this, tr("Load Plugin Native Preset")); if (file.isEmpty()) diff --git a/host/main-window.hh b/host/main-window.hh index 1dcfa07..260407e 100644 --- a/host/main-window.hh +++ b/host/main-window.hh @@ -25,6 +25,7 @@ public: void showPluginParametersWindow(); void showPluginQuickControlsWindow(); void resizePluginView(int width, int height); + void onPluginLoadChange(bool i_pluginLoaded); void showPluginWindow() { _pluginViewWidget->show(); } @@ -41,11 +42,20 @@ private: void togglePluginWindowVisibility(); void recreatePluginWindow(); void showAboutDialog(); + void updateMenuItems(); Application &_application; QWindow *_pluginViewWindow = nullptr; QWidget *_pluginViewWidget = nullptr; + QAction *_loadPluginPresetAction = nullptr; + QAction *_showPluginParametersAction = nullptr; + QAction *_showPluginQuickControlsAction = nullptr; + QAction *_togglePluginWindowVisibilityAction = nullptr; + QAction *_recreatePluginWindowAction = nullptr; + + bool _pluginLoaded = false; + PluginParametersWidget *_pluginParametersWidget = nullptr; PluginQuickControlsWidget *_pluginRemoteControlsWidget = nullptr; }; diff --git a/host/plugin-host.cc b/host/plugin-host.cc index fa45677..175f826 100644 --- a/host/plugin-host.cc +++ b/host/plugin-host.cc @@ -147,12 +147,17 @@ bool PluginHost::load(const QString &path, int pluginIndex) { scanParams(); scanQuickControls(); + + Application::instance().mainWindow()->onPluginLoadChange(true); + return true; } void PluginHost::unload() { checkForMainThread(); + Application::instance().mainWindow()->onPluginLoadChange(false); + if (!_library.isLoaded()) return; @@ -1171,12 +1176,6 @@ void PluginHost::remoteControlsSuggestPage(clap_id page_id) noexcept { bool PluginHost::loadNativePluginPreset(const std::string &path) { checkForMainThread(); - if(!_plugin) - { - std::cerr << "called with a null clap_plugin pointer!" << std::endl; - return false; - } - if (!_plugin->canUsePresetLoad()) return false; From 7017523b570e6f43925fe85a9070269bd04c3e4e Mon Sep 17 00:00:00 2001 From: Nature Is Frequency Date: Mon, 12 Feb 2024 13:56:27 +0000 Subject: [PATCH 2/2] Use signal/slot approach for pluginLoadedChanged Per @Trinitou feedback: 1. pluginLoadedChanged is now a signal of PluginHost 2. Connect MainWindow:: updatePluginMenuItems to this signal 3. Rename updateMenuItems to updatePluginMenuItems (so it specifies it's these are plugin related menu items) 4. Remove i_prefix in function parameter to follow code style 5. Use default argument false for updatePluginMenuItems --- host/main-window.cc | 22 ++++++++++------------ host/main-window.hh | 5 +---- host/plugin-host.cc | 4 ++-- host/plugin-host.hh | 1 + 4 files changed, 14 insertions(+), 18 deletions(-) diff --git a/host/main-window.cc b/host/main-window.cc index 79cc827..4442761 100644 --- a/host/main-window.cc +++ b/host/main-window.cc @@ -99,15 +99,18 @@ void MainWindow::createMenu() { connect( helpMenu->addAction(tr("About")), &QAction::triggered, this, &MainWindow::showAboutDialog); - updateMenuItems(); + updatePluginMenuItems(); + + assert(_application.engine()); + connect(&_application.engine()->pluginHost(), &PluginHost::pluginLoadedChanged, this, &MainWindow::updatePluginMenuItems); } -void MainWindow::updateMenuItems() { - _loadPluginPresetAction->setEnabled(_pluginLoaded); - _showPluginParametersAction->setEnabled(_pluginLoaded); - _showPluginQuickControlsAction->setEnabled(_pluginLoaded); - _togglePluginWindowVisibilityAction->setEnabled(_pluginLoaded); - _recreatePluginWindowAction->setEnabled(_pluginLoaded); +void MainWindow::updatePluginMenuItems(bool const pluginLoaded /* = false */ ) { + _loadPluginPresetAction->setEnabled(pluginLoaded); + _showPluginParametersAction->setEnabled(pluginLoaded); + _showPluginQuickControlsAction->setEnabled(pluginLoaded); + _togglePluginWindowVisibilityAction->setEnabled(pluginLoaded); + _recreatePluginWindowAction->setEnabled(pluginLoaded); } void MainWindow::showSettingsDialog() { @@ -137,11 +140,6 @@ void MainWindow::resizePluginView(int width, int height) { adjustSize(); } -void MainWindow::onPluginLoadChange(bool const i_pluginLoaded) { - _pluginLoaded = i_pluginLoaded; - updateMenuItems(); -} - void MainWindow::loadNativePluginPreset() { auto file = QFileDialog::getOpenFileName(this, tr("Load Plugin Native Preset")); if (file.isEmpty()) diff --git a/host/main-window.hh b/host/main-window.hh index 260407e..6bb2f85 100644 --- a/host/main-window.hh +++ b/host/main-window.hh @@ -25,7 +25,6 @@ public: void showPluginParametersWindow(); void showPluginQuickControlsWindow(); void resizePluginView(int width, int height); - void onPluginLoadChange(bool i_pluginLoaded); void showPluginWindow() { _pluginViewWidget->show(); } @@ -42,7 +41,7 @@ private: void togglePluginWindowVisibility(); void recreatePluginWindow(); void showAboutDialog(); - void updateMenuItems(); + void updatePluginMenuItems(bool pluginLoaded = false); Application &_application; QWindow *_pluginViewWindow = nullptr; @@ -54,8 +53,6 @@ private: QAction *_togglePluginWindowVisibilityAction = nullptr; QAction *_recreatePluginWindowAction = nullptr; - bool _pluginLoaded = false; - PluginParametersWidget *_pluginParametersWidget = nullptr; PluginQuickControlsWidget *_pluginRemoteControlsWidget = nullptr; }; diff --git a/host/plugin-host.cc b/host/plugin-host.cc index 175f826..30d0540 100644 --- a/host/plugin-host.cc +++ b/host/plugin-host.cc @@ -148,7 +148,7 @@ bool PluginHost::load(const QString &path, int pluginIndex) { scanParams(); scanQuickControls(); - Application::instance().mainWindow()->onPluginLoadChange(true); + pluginLoadedChanged(true); return true; } @@ -156,7 +156,7 @@ bool PluginHost::load(const QString &path, int pluginIndex) { void PluginHost::unload() { checkForMainThread(); - Application::instance().mainWindow()->onPluginLoadChange(false); + pluginLoadedChanged(false); if (!_library.isLoaded()) return; diff --git a/host/plugin-host.hh b/host/plugin-host.hh index 7f36db7..0a69fa9 100644 --- a/host/plugin-host.hh +++ b/host/plugin-host.hh @@ -92,6 +92,7 @@ signals: void quickControlsPagesChanged(); void quickControlsSelectedPageChanged(); void paramAdjusted(clap_id paramId); + void pluginLoadedChanged(bool pluginLoaded); protected: /////////////////////////