Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unify [effect chain],loaded_chain_preset control (all 0-indexed now) #12561

Merged
merged 6 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions src/effects/effectchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,23 @@ void EffectChain::loadChainPreset(EffectChainPresetPointer pChainPreset) {
setControlLoadedPresetIndex(presetIndex());
}

bool EffectChain::isEmpty() {
for (const auto& pEffectSlot : std::as_const(m_effectSlots)) {
if (pEffectSlot->isLoaded()) {
return false;
}
}
return true;
}

bool EffectChain::isEmptyPlaceholderPresetLoaded() {
return isEmpty() && presetName() == kNoEffectString;
}

void EffectChain::loadEmptyNamelessPreset() {
loadChainPreset(m_pChainPresetManager->createEmptyNamelessChainPreset());
}

void EffectChain::sendParameterUpdate() {
EffectsRequest* pRequest = new EffectsRequest();
pRequest->type = EffectsRequest::SET_EFFECT_CHAIN_PARAMETERS;
Expand Down Expand Up @@ -309,7 +326,7 @@ EffectSlotPointer EffectChain::getEffectSlot(unsigned int slotNumber) {
}

void EffectChain::slotControlClear(double v) {
for (EffectSlotPointer pEffectSlot : std::as_const(m_effectSlots)) {
for (const auto& pEffectSlot : std::as_const(m_effectSlots)) {
pEffectSlot->slotClear(v);
}
}
Expand Down Expand Up @@ -337,18 +354,16 @@ void EffectChain::slotControlChainPresetSelector(double value) {
}

void EffectChain::slotControlLoadedChainPresetRequest(double value) {
// subtract 1 to make the ControlObject 1-indexed like other ControlObjects
int index = static_cast<int>(value) - 1;
int index = static_cast<int>(value);
if (index < 0 || index >= numPresets()) {
return;
}
// loadChainPreset calls setAndConfirm
loadChainPreset(presetAtIndex(index));
}

void EffectChain::setControlLoadedPresetIndex(uint index) {
// add 1 to make the ControlObject 1-indexed like other ControlObjects
m_pControlLoadedChainPreset->setAndConfirm(index + 1);
void EffectChain::setControlLoadedPresetIndex(int index) {
m_pControlLoadedChainPreset->setAndConfirm(index);
}

void EffectChain::slotControlNextChainPreset(double value) {
Expand Down Expand Up @@ -418,6 +433,9 @@ void EffectChain::disableForInputChannel(const ChannelHandleAndGroup& handleGrou
}

int EffectChain::presetIndex() const {
// 0-indexed, 0 is the empty '---' preset.
// This can be -1 if the name is not found in the presets list,
// which is default state of standard effect chains.
return m_pChainPresetManager->presetIndex(m_presetName);
}

Expand Down
8 changes: 7 additions & 1 deletion src/effects/effectchain.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ class EffectChain : public QObject {

virtual void loadChainPreset(EffectChainPresetPointer pPreset);

bool isEmpty();

bool isEmptyPlaceholderPresetLoaded();

void loadEmptyNamelessPreset();

public slots:
void slotControlClear(double value);

Expand Down Expand Up @@ -135,7 +141,7 @@ class EffectChain : public QObject {
std::unique_ptr<ControlPushButton> m_pControlNextChainPreset;
std::unique_ptr<ControlPushButton> m_pControlPrevChainPreset;

void setControlLoadedPresetIndex(uint index);
void setControlLoadedPresetIndex(int index);

// These COs do not affect how the effects are processed;
// they are defined here for skins and controller mappings to communicate
Expand Down
8 changes: 8 additions & 0 deletions src/effects/effectslot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,14 @@ void EffectSlot::loadEffectInner(const EffectManifestPointer pManifest,
return;
}

// Don't load an effect into the '---' preset. The preset would remain
// selected in WEffectChainPresetSelector and WEffectChainPresetButton and
// therefore couldn't be used to clear the chain.
// Instead, load an empty, nameless preset, then load the desired effect.
if (m_pChain->isEmptyPlaceholderPresetLoaded()) {
m_pChain->loadEmptyNamelessPreset();
}

m_pManifest = pManifest;
addToEngine();

Expand Down
55 changes: 50 additions & 5 deletions src/effects/presets/effectchainpresetmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ EffectChainPresetPointer loadPresetFromFile(const QString& filePath) {
return pEffectChainPreset;
}

EffectChainPresetPointer createEmptyChainPreset() {
EffectChainPresetPointer createEmptyReadOnlyChainPreset() {
EffectManifestPointer pEmptyManifest(new EffectManifest());
pEmptyManifest->setName(kNoEffectString);
// Center the Super knob, eliminates the colored (bipolar) knob ring
Expand Down Expand Up @@ -394,6 +394,20 @@ void EffectChainPresetManager::setPresetOrder(
m_effectChainPresets.value(chainPresetName));
}

// After having changed the presets order in DlgPrefEffects, we received the
// new lists. The '---' was not displayed there, so it's not in the list.
// Make sure the empty preset '---' is the first item in the list.
const auto& pEmptyPreset = m_effectChainPresets.value(kNoEffectString);
VERIFY_OR_DEBUG_ASSERT(pEmptyPreset) {
return;
}
int index = m_effectChainPresetsSorted.indexOf(pEmptyPreset);
if (index == -1) { // not in list, re-add it
m_effectChainPresetsSorted.prepend(pEmptyPreset);
} else if (index != 0) { // not first item, move to top
m_effectChainPresetsSorted.move(index, 0);
}

emit effectChainPresetListUpdated();
}

Expand All @@ -409,7 +423,9 @@ void EffectChainPresetManager::setQuickEffectPresetOrder(
m_effectChainPresets.value(chainPresetName));
}

// Ensure empty '---' preset is the first list item
// After having changed the presets order in DlgPrefEffects, we received the
// new lists. The '---' was not displayed there, so it's not in the list.
// Make sure the empty preset '---' is the first item in the list.
const auto& pEmptyPreset = m_effectChainPresets.value(kNoEffectString);
VERIFY_OR_DEBUG_ASSERT(pEmptyPreset) {
return;
Expand Down Expand Up @@ -612,8 +628,9 @@ void EffectChainPresetManager::resetToDefaults() {
prependRemainingPresetsToLists();

// Re-add the empty chain preset
EffectChainPresetPointer pEmptyChainPreset = createEmptyChainPreset();
EffectChainPresetPointer pEmptyChainPreset = createEmptyReadOnlyChainPreset();
m_effectChainPresets.insert(pEmptyChainPreset->name(), pEmptyChainPreset);
m_effectChainPresetsSorted.prepend(pEmptyChainPreset);
m_quickEffectChainPresetsSorted.prepend(pEmptyChainPreset);

emit effectChainPresetListUpdated();
Expand Down Expand Up @@ -660,6 +677,13 @@ bool EffectChainPresetManager::savePresetXml(EffectChainPresetPointer pPreset) {
return success;
}

// static
EffectChainPresetPointer EffectChainPresetManager::createEmptyNamelessChainPreset() {
auto pPreset = EffectChainPresetPointer::create(EffectChainPreset());
pPreset->setName("");
return pPreset;
}

EffectManifestPointer EffectChainPresetManager::getDefaultEqEffect() {
EffectManifestPointer pDefaultEqEffect = m_pBackendManager->getManifest(
BiquadFullKillEQEffect::getId(), EffectBackendType::BuiltIn);
Expand Down Expand Up @@ -705,6 +729,10 @@ EffectsXmlData EffectChainPresetManager::readEffectsXml(
QDomElement chainElement = chainNode.toElement();
EffectChainPresetPointer pPreset =
EffectChainPresetPointer::create(chainElement);
// Shouldn't happen, see EffectSlot::loadEffectInner
VERIFY_OR_DEBUG_ASSERT(pPreset->name() != kNoEffectString) {
pPreset->setName("");
}
standardEffectChainPresets.append(pPreset);
}
}
Expand Down Expand Up @@ -782,8 +810,9 @@ EffectsXmlData EffectChainPresetManager::readEffectsXml(
// It will not be visible in the effects preferences.
// Note: we also need to take care of this preset in resetToDefaults() and
// setQuickEffectPresetOrder(), both called from DlgPrefEffects
EffectChainPresetPointer pEmptyChainPreset = createEmptyChainPreset();
EffectChainPresetPointer pEmptyChainPreset = createEmptyReadOnlyChainPreset();
m_effectChainPresets.insert(pEmptyChainPreset->name(), pEmptyChainPreset);
m_effectChainPresetsSorted.prepend(pEmptyChainPreset);
m_quickEffectChainPresetsSorted.prepend(pEmptyChainPreset);

emit effectChainPresetListUpdated();
Expand Down Expand Up @@ -896,6 +925,17 @@ void EffectChainPresetManager::saveEffectsXml(QDomDocument* pDoc, const EffectsX
QDomElement chainsElement = pDoc->createElement(EffectXml::kChainsRoot);
rackElement.appendChild(chainsElement);
for (const auto& pPreset : std::as_const(data.standardEffectChainPresets)) {
// Don't store the empty '---' preset.
if (pPreset->name() == kNoEffectString) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we don't want to store the noeffect, shouldn't we skip it (continue) instead of setting the name empty and adding it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(as you did below)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this only as safeguard to not dump loaded effects in the (unexpected) case that there are effects loaded despite the '---' preset name.
I can turn it into a VERIFY_OR_DEBUG_ASSERT.

Copy link
Member Author

@ronso0 ronso0 Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by d9b104b

// It must not have any effects loaded. If it has, clear the name.
// See readEffectsXml() for explanation.
VERIFY_OR_DEBUG_ASSERT(pPreset->isEmpty()) {
pPreset->setName("");
}
else {
continue;
}
}
chainsElement.appendChild(pPreset->toXml(pDoc));
}

Expand All @@ -909,6 +949,11 @@ void EffectChainPresetManager::saveEffectsXml(QDomDocument* pDoc, const EffectsX
QDomElement chainPresetListElement =
pDoc->createElement(EffectXml::kChainPresetList);
for (const auto& pPreset : std::as_const(m_effectChainPresetsSorted)) {
// Don't store the empty '---' preset in the main preset list,
// it won't be exported anyway.
if (pPreset->name() == kNoEffectString) {
continue;
}
XmlParse::addElement(*pDoc,
chainPresetListElement,
EffectXml::kChainPresetName,
Expand All @@ -920,7 +965,7 @@ 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
// Same here, don't store the empty '---' preset
if (pPreset->name() == kNoEffectString) {
continue;
}
Expand Down
2 changes: 2 additions & 0 deletions src/effects/presets/effectchainpresetmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ class EffectChainPresetManager : public QObject {
EffectManifestPointer getDefaultEqEffect();
EffectChainPresetPointer getDefaultQuickEffectPreset();

static EffectChainPresetPointer createEmptyNamelessChainPreset();

EffectsXmlData readEffectsXml(const QDomDocument& doc, const QStringList& deckStrings);
EffectXmlDataSingleDeck readEffectsXmlSingleDeck(
const QDomDocument& doc, const QString& deckString);
Expand Down
10 changes: 7 additions & 3 deletions src/preferences/dialog/dlgprefeffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,20 @@ void DlgPrefEffects::clearChainInfoDisableButtons() {
void DlgPrefEffects::loadChainPresetLists() {
QStringList chainPresetNames;
for (const auto& pChainPreset : m_pChainPresetManager->getPresetsSorted()) {
// Don't show the empty '---' preset.
// After pushing the changed preferences list back to the preset manager
// it is re-added to the base list.
if (pChainPreset->name() == kNoEffectString) {
continue;
}
chainPresetNames << pChainPreset->name();
}
auto* pModel = dynamic_cast<EffectChainPresetListModel*>(chainListView->model());
pModel->setStringList(chainPresetNames);

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.
// Same here, don't show the empty '---' preset.
if (pChainPreset->name() == kNoEffectString) {
continue;
}
Expand Down
Loading