Merge pull request #10859 from ronso0/effect-chain-clear-empty-preset

Effects: allow clearing chains with empty '---' preset

src/effects/effectchain.cpp

@@ -356,13 +356,13 @@ void EffectChain::setControlLoadedPresetIndex(uint index) {
 
 void EffectChain::slotControlNextChainPreset(double value) {
     if (value > 0) {
-        loadChainPreset(presetAtIndex(presetIndex() + 1));
+        slotControlChainPresetSelector(1);
     }
 }
 
 void EffectChain::slotControlPrevChainPreset(double value) {
     if (value > 0) {
-        loadChainPreset(presetAtIndex(presetIndex() - 1));
+        slotControlChainPresetSelector(-1);
     }
 }
 

src/effects/effectsmanager.cpp

@@ -177,6 +177,8 @@ void EffectsManager::readEffectsXml() {
     }
     file.close();
 
+    // Note: QuickEffect chains are created only for existing main decks
+    // thus only for those the configured presets are requested
     QStringList deckStrings;
     for (auto it = m_quickEffectChains.begin(); it != m_quickEffectChains.end(); it++) {
         deckStrings << it.key();

src/effects/effectsmanager.h

@@ -88,6 +88,7 @@ class EffectsManager {
 
     QList<StandardEffectChainPointer> m_standardEffectChains;
     OutputEffectChainPointer m_outputEffectChain;
+    // These two store <deck group, effect chain pointer>
     QHash<QString, EqualizerEffectChainPointer> m_equalizerEffectChains;
     QHash<QString, QuickEffectChainPointer> m_quickEffectChains;
 

src/effects/presets/effectchainpreset.cpp

@@ -7,7 +7,8 @@
 EffectChainPreset::EffectChainPreset()
         : m_name(kNoEffectString),
           m_mixMode(EffectChainMixMode::DrySlashWet),
-          m_dSuper(0.0) {
+          m_dSuper(0.0),
+          m_readOnly(false) {
 }
 
 EffectChainPreset::EffectChainPreset(const QDomElement& chainElement) {
@@ -28,6 +29,8 @@ EffectChainPreset::EffectChainPreset(const QDomElement& chainElement) {
 
     m_dSuper = XmlParse::selectNodeDouble(chainElement, EffectXml::kChainSuperParameter);
 
+    m_readOnly = false;
+
     QDomElement effectsElement = XmlParse::selectElement(chainElement, EffectXml::kEffectsRoot);
     QDomNodeList effectList = effectsElement.childNodes();
 

src/effects/presets/effectchainpreset.h

@@ -40,6 +40,12 @@ class EffectChainPreset {
     double superKnob() const {
         return m_dSuper;
     }
+    void setReadOnly() {
+        m_readOnly = true;
+    }
+    bool isReadOnly() const {
+        return m_readOnly;
+    }
 
     const QList<EffectPresetPointer>& effectPresets() const {
         return m_effectPresets;
@@ -50,4 +56,5 @@ class EffectChainPreset {
     EffectChainMixMode::Type m_mixMode;
     double m_dSuper;
     QList<EffectPresetPointer> m_effectPresets;
+    bool m_readOnly;
 };

src/effects/presets/effectchainpresetmanager.cpp

@@ -123,9 +123,15 @@ void EffectChainPresetManager::importPreset() {
         EffectChainPresetPointer pPreset(
                 new EffectChainPreset(doc.documentElement()));
         if (!pPreset->isEmpty() && !pPreset->name().isEmpty()) {
+            // Don't allow '---' because that's the name of the internal empty preset
+            if (pPreset->name() == kNoEffectString) {
+                pPreset->setName(pPreset->name() +
+                        QStringLiteral(" (") + tr("imported") + QStringLiteral(")"));
+            }
+
             while (m_effectChainPresets.contains(pPreset->name())) {
                 pPreset->setName(pPreset->name() +
-                        QLatin1String(" (") + tr("duplicate") + QLatin1String(")"));
+                        QStringLiteral(" (") + tr("duplicate") + QStringLiteral(")"));
             }
 
             // An imported chain preset might contain an LV2 plugin that the user does not
@@ -227,10 +233,22 @@ void EffectChainPresetManager::renamePreset(const QString& oldName) {
     VERIFY_OR_DEBUG_ASSERT(m_effectChainPresets.contains(oldName)) {
         return;
     }
+    if (m_effectChainPresets.value(oldName)->isReadOnly()) {
+        QMessageBox msgBox;
+        msgBox.setText(tr("Effect chain preset can not be renamed"));
+        msgBox.setInformativeText(
+                tr("Effect chain preset \"%1\" is read-only and can not be renamed.")
+                        .arg(oldName));
+        msgBox.setIcon(QMessageBox::Warning);
+        msgBox.exec();
+        return;
+    }
 
     QString newName;
     QString errorText;
-    while (newName.isEmpty() || m_effectChainPresets.contains(newName)) {
+    // Don't allow '---' as new name either
+    while (newName.isEmpty() || m_effectChainPresets.contains(newName) ||
+            newName == kNoEffectString) {
         bool okay = false;
         newName = QInputDialog::getText(nullptr,
                 tr("Rename effect chain preset"),
@@ -246,6 +264,8 @@ void EffectChainPresetManager::renamePreset(const QString& oldName) {
 
         if (newName.isEmpty()) {
             errorText = tr("Effect chain preset name must not be empty.") + QStringLiteral("\n");
+        } else if (newName == kNoEffectString) {
+            errorText = tr("Invalid name \"%1\"").arg(newName) + QStringLiteral("\n");
         } else if (m_effectChainPresets.contains(newName)) {
             errorText =
                     tr("An effect chain preset named \"%1\" already exists.")
@@ -301,6 +321,16 @@ bool EffectChainPresetManager::deletePreset(const QString& chainPresetName) {
     VERIFY_OR_DEBUG_ASSERT(m_effectChainPresets.contains(chainPresetName)) {
         return false;
     }
+    if (m_effectChainPresets.value(chainPresetName)->isReadOnly()) {
+        QMessageBox msgBox;
+        msgBox.setText(tr("Effect chain preset can not be deleted"));
+        msgBox.setInformativeText(
+                tr("Effect chain preset \"%1\" is read-only and can not be deleted.")
+                        .arg(chainPresetName));
+        msgBox.setIcon(QMessageBox::Warning);
+        msgBox.exec();
+        return false;
+    }
     auto pressedButton = QMessageBox::question(nullptr,
             tr("Remove effect chain preset"),
             tr("Are you sure you want to delete the effect chain preset "
@@ -360,6 +390,18 @@ void EffectChainPresetManager::setQuickEffectPresetOrder(
                 m_effectChainPresets.value(chainPresetName));
     }
 
+    // Ensure empty '---' preset is the first list item
+    const auto& pEmptyPreset = m_effectChainPresets.value(kNoEffectString);
+    VERIFY_OR_DEBUG_ASSERT(pEmptyPreset) {
+        return;
+    }
+    int index = m_quickEffectChainPresetsSorted.indexOf(pEmptyPreset);
+    if (index == -1) { // not in list, re-add it
+        m_quickEffectChainPresetsSorted.prepend(pEmptyPreset);
+    } else if (index != 0) { // not first item, move to top
+        m_quickEffectChainPresetsSorted.move(index, 0);
+    }
+
     emit quickEffectChainPresetListUpdated();
 }
 
@@ -373,13 +415,19 @@ void EffectChainPresetManager::savePresetAndReload(EffectChainPointer pChainSlot
 bool EffectChainPresetManager::savePreset(EffectChainPresetPointer pPreset) {
     QString name;
     QString errorText;
-    while (name.isEmpty() || m_effectChainPresets.contains(name)) {
+    // Don't allow '---' because that's the name of the internal empty preset
+    // Clear initial name to avoid confusion.
+    QString presetName;
+    if (pPreset->name() != kNoEffectString) {
+        presetName = pPreset->name();
+    }
+    while (name.isEmpty() || m_effectChainPresets.contains(name) || name == kNoEffectString) {
         bool okay = false;
         name = QInputDialog::getText(nullptr,
                 tr("Save preset for effect chain"),
                 errorText + "\n" + tr("Name for new effect chain preset:"),
                 QLineEdit::Normal,
-                pPreset->name(),
+                presetName,
                 &okay)
                        .trimmed();
         if (!okay) {
@@ -388,6 +436,8 @@ bool EffectChainPresetManager::savePreset(EffectChainPresetPointer pPreset) {
 
         if (name.isEmpty()) {
             errorText = tr("Effect chain preset name must not be empty.") + QStringLiteral("\n");
+        } else if (name == kNoEffectString) {
+            errorText = tr("Invalid name \"%1\"").arg(name) + QStringLiteral("\n");
         } else if (m_effectChainPresets.contains(name)) {
             errorText =
                     tr("An effect chain preset named \"%1\" already exists.")
@@ -447,6 +497,11 @@ void EffectChainPresetManager::importUserPresets() {
         EffectChainPresetPointer pEffectChainPreset = loadPresetFromFile(
                 savedPresetsPath + kFolderDelimiter + filePath);
         if (pEffectChainPreset && !pEffectChainPreset->isEmpty()) {
+            // Don't allow '---' because that's the name of the internal empty preset
+            if (pEffectChainPreset->name() == kNoEffectString) {
+                pEffectChainPreset->setName(pEffectChainPreset->name() +
+                        QLatin1String(" (") + tr("imported") + QLatin1String(")"));
+            }
             m_effectChainPresets.insert(
                     pEffectChainPreset->name(), pEffectChainPreset);
         }
@@ -542,6 +597,11 @@ void EffectChainPresetManager::resetToDefaults() {
 }
 
 bool EffectChainPresetManager::savePresetXml(EffectChainPresetPointer pPreset) {
+    // Don't store the empty '---' preset in effects/chains.
+    // Shouldn't be possible via GUI anyway because the 'Update Preset' button
+    // is not shown in WEffectChainPresetButton if this preset is loaded.
+    DEBUG_ASSERT(pPreset->name() != kNoEffectString);
+
     QString path(m_pConfig->getSettingsPath() + kEffectChainPresetDirectory);
     QDir effectsChainsDir(path);
     if (!effectsChainsDir.exists()) {
@@ -591,7 +651,7 @@ EffectsXmlData EffectChainPresetManager::readEffectsXml(
         quickEffectPresets.insert(deckString, defaultQuickEffectChainPreset);
     }
 
-    // Reload state of standard chains
+    // Read state of standard chains
     QDomElement root = doc.documentElement();
     QDomElement rackElement = XmlParse::selectElement(root, EffectXml::kRack);
     QDomElement chainsElement =
@@ -668,10 +728,26 @@ EffectsXmlData EffectChainPresetManager::readEffectsXml(
 
     prependRemainingPresetsToLists();
 
+    // Create the empty '---' chain preset on each start.
+    // Its sole purpose is to eject the current QuickEffect chain presets via GUI.
+    // It will not be saved to effects/chains nor written to effects.xml
+    // except as identifier for QuickEffect chains.
+    // It will not be visible in the effects preferences.
+    EffectManifestPointer pEmptyChainManifest(new EffectManifest());
+    pEmptyChainManifest->setName(kNoEffectString);
+    // Required for the QuickEffect selector in DlgPrefEQ
+    pEmptyChainManifest->setShortName(kNoEffectString);
+    auto pEmptyChainPreset =
+            EffectChainPresetPointer(new EffectChainPreset(pEmptyChainManifest));
+    pEmptyChainPreset->setReadOnly();
+
+    m_effectChainPresets.insert(pEmptyChainPreset->name(), pEmptyChainPreset);
+    m_quickEffectChainPresetsSorted.prepend(pEmptyChainPreset);
+
     emit effectChainPresetListUpdated();
     emit quickEffectChainPresetListUpdated();
 
-    // Reload presets that were loaded into QuickEffects on last shutdown
+    // Read names of presets that were loaded into QuickEffects on last shutdown
     QDomElement quickEffectPresetsElement =
             XmlParse::selectElement(root, EffectXml::kQuickEffectChainPresets);
     QDomNodeList quickEffectNodeList =
@@ -683,6 +759,8 @@ EffectsXmlData EffectChainPresetManager::readEffectsXml(
             QString deckGroup = presetNameElement.attribute(QStringLiteral("group"));
             auto pPreset = m_effectChainPresets.value(presetNameElement.text());
             if (pPreset != nullptr) {
+                // Replace defaultQuickEffectChainPreset with pPreset
+                // for this deck group
                 quickEffectPresets.insert(deckGroup, pPreset);
             }
         }
@@ -717,6 +795,10 @@ void EffectChainPresetManager::saveEffectsXml(QDomDocument* pDoc, const EffectsX
     QDomElement quickEffectChainPresetListElement =
             pDoc->createElement(EffectXml::kQuickEffectList);
     for (const auto& pPreset : std::as_const(m_quickEffectChainPresetsSorted)) {
+        // Don't store the empty '---' in the QuickEffect preset list
+        if (pPreset->name() == kNoEffectString) {
+            continue;
+        }
         XmlParse::addElement(*pDoc,
                 quickEffectChainPresetListElement,
                 EffectXml::kChainPresetName,

src/preferences/dialog/dlgprefeffects.cpp

@@ -170,6 +170,12 @@ void DlgPrefEffects::loadChainPresetLists() {
 
     QStringList quickEffectChainPresetNames;
     for (const auto& pChainPreset : m_pChainPresetManager->getQuickEffectPresetsSorted()) {
+        // Don't show the empty '---' preset.
+        // After pushing the changed preferences list back to the preset manager
+        // it is re-added to the root list.
+        if (pChainPreset->name() == kNoEffectString) {
+            continue;
+        }
         quickEffectChainPresetNames << pChainPreset->name();
     }
     pModel = dynamic_cast<EffectChainPresetListModel*>(quickEffectListView->model());

src/preferences/dialog/dlgprefeq.cpp

@@ -302,10 +302,7 @@ void DlgPrefEQ::populateDeckQuickEffectBoxList(
         QString deckGroupName = PlayerManager::groupForDeck(deck);
         QString unitGroup = QuickEffectChain::formatEffectChainGroup(deckGroupName);
         EffectChainPointer pChain = m_pEffectsManager->getEffectChain(unitGroup);
-
-        // Add empty item at the top: no effect
-        box->addItem(kNoEffectString);
-        int i = 1;
+        int i = 0;
         for (const auto& pChainPreset : presetList) {
             box->addItem(pChainPreset->name());
             if (pChain->presetName() == pChainPreset->name()) {
@@ -492,8 +489,8 @@ void DlgPrefEQ::slotQuickEffectChangedOnDeck(int effectIndex) {
     EffectChainPointer pChain = m_pEffectsManager->getEffectChain(unitGroup);
     QList<EffectChainPresetPointer> presetList =
             m_pChainPresetManager->getQuickEffectPresetsSorted();
-    if (pChain && effectIndex > 0 && effectIndex <= presetList.size()) {
-        pChain->loadChainPreset(presetList[effectIndex - 1]);
+    if (pChain && effectIndex >= 0 && effectIndex < presetList.size()) {
+        pChain->loadChainPreset(presetList[effectIndex]);
     }
 }
 

src/widget/weffectchainpresetbutton.cpp

@@ -49,20 +49,23 @@ void WEffectChainPresetButton::populateMenu() {
     m_pMenu->clear();
 
     // Chain preset items
-    bool chainIsPreset = false;
+    bool presetIsReadOnly = true;
     for (const auto& pChainPreset : m_pChainPresetManager->getPresetsSorted()) {
         QString title = pChainPreset->name();
         if (title == m_pChain->presetName()) {
             title = QChar(0x2713) + // CHECK MARK
                     QChar(' ') + title;
-            chainIsPreset = true;
+            presetIsReadOnly = pChainPreset->isReadOnly();
         }
         m_pMenu->addAction(title, this, [this, pChainPreset]() {
             m_pChain->loadChainPreset(pChainPreset);
         });
     }
     m_pMenu->addSeparator();
-    if (chainIsPreset) {
+    // This prevents showing the Update button for the empty '---' preset, in case
+    // WEffectChainPresetButton and effect slot controls of a QuickEffect chain are
+    // exposed in a custom skin.
+    if (!presetIsReadOnly) {
         m_pMenu->addAction(tr("Update Preset"), this, [this]() {
             m_pChainPresetManager->updatePreset(m_pChain);
         });

src/widget/weffectchainpresetselector.cpp

@@ -36,7 +36,7 @@ void WEffectChainPresetSelector::setup(const QDomNode& node, const SkinContext&
     }
 
     auto chainPresetListUpdateSignal = &EffectChainPresetManager::effectChainPresetListUpdated;
-    auto pQuickEffectChain = dynamic_cast<QuickEffectChain*>(m_pChain.data());
+    auto pQuickEffectChain = qobject_cast<QuickEffectChain*>(m_pChain.data());
     if (pQuickEffectChain) {
         chainPresetListUpdateSignal = &EffectChainPresetManager::quickEffectChainPresetListUpdated;
         m_bQuickEffectChain = true;
@@ -84,10 +84,8 @@ void WEffectChainPresetSelector::populate() {
 }
 
 void WEffectChainPresetSelector::slotEffectChainPresetSelected(int index) {
-    Q_UNUSED(index);
     m_pChain->loadChainPreset(
             m_pChainPresetManager->getPreset(currentData().toString()));
-    setBaseTooltip(itemData(index, Qt::ToolTipRole).toString());
     // After selecting an effect move focus to the tracks table in order
     // to immediately allow keyboard shortcuts again.
     // TODO(ronso0) switch to previously focused (library?) widget instead
@@ -97,7 +95,7 @@ void WEffectChainPresetSelector::slotEffectChainPresetSelected(int index) {
 
 void WEffectChainPresetSelector::slotChainPresetChanged(const QString& name) {
     setCurrentIndex(findData(name));
-    setBaseTooltip(name);
+    setBaseTooltip(itemData(currentIndex(), Qt::ToolTipRole).toString());
 }
 
 bool WEffectChainPresetSelector::event(QEvent* pEvent) {