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

Effect Blacklisting #1674

Merged
merged 20 commits into from
May 29, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
2 changes: 2 additions & 0 deletions build/depends.py
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,9 @@ def sources(self, build):
"preferences/broadcastsettings.cpp",
"preferences/broadcastsettings_legacy.cpp",
"preferences/broadcastsettingsmodel.cpp",
"preferences/effectsettingsmodel.cpp",
"preferences/broadcastprofile.cpp",
"preferences/effectprofile.cpp",
"preferences/upgrade.cpp",
"preferences/dlgpreferencepage.cpp",

Expand Down
8 changes: 8 additions & 0 deletions src/effects/effectmanifest.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ class EffectManifest final {
}
}

const QString& backendName() const {
return m_backendName;
}
void setBackendName(const QString& name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this

m_backendName = name;
}

const QString& author() const {
return m_author;
}
Expand Down Expand Up @@ -133,6 +140,7 @@ class EffectManifest final {
QString m_id;
QString m_name;
QString m_shortName;
QString m_backendName;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove m_backendName

QString m_author;
QString m_version;
QString m_description;
Expand Down
21 changes: 20 additions & 1 deletion src/effects/effectsbackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@
#include "effects/effectsbackend.h"
#include "effects/effectsmanager.h"

EffectsBackend::EffectsBackend(QObject* pParent, QString name)
EffectsBackend::EffectsBackend(UserSettingsPointer pConfig,
QObject* pParent,
QString name)
Copy link
Contributor

Choose a reason for hiding this comment

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

replace the QString here with a BackendType value

: QObject(pParent),
m_pConfig(pConfig),
m_name(name) {
}

EffectsBackend::~EffectsBackend() {
m_registeredEffects.clear();
m_visibleEffectIds.clear();
m_effectIds.clear();
}

Expand All @@ -25,7 +29,14 @@ void EffectsBackend::registerEffect(const QString& id,
return;
}

const bool visible = m_pConfig->getValue<bool>(ConfigKey("[Visible Effects]",
pManifest->id()), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

User preference code should be in the preference dialog classes, not scattered throughout the rest of the code. Reading the configuration values should be handled by DlgPrefEffects on initialization, which should make calls to EffectsManager::setEffectVisibility.

pManifest->setBackendName(m_name);
Copy link
Contributor

Choose a reason for hiding this comment

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

pManifest->setBackendType


m_registeredEffects[id] = RegisteredEffect(pManifest, pInstantiator);
if (visible) {
addVisibleEffect(id);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this.

m_effectIds.append(id);
emit(effectRegistered(pManifest));
}
Expand All @@ -42,6 +53,14 @@ EffectManifestPointer EffectsBackend::getManifest(const QString& effectId) const
return m_registeredEffects[effectId].manifest();
}

bool EffectsBackend::getVisibility(const QString& effectId) const {
return m_visibleEffectIds.contains(effectId);
}

void EffectsBackend::addVisibleEffect(const QString& effectId) {
m_visibleEffectIds.append(effectId);
}

bool EffectsBackend::canInstantiateEffect(const QString& effectId) const {
return m_registeredEffects.contains(effectId);
}
Expand Down
9 changes: 7 additions & 2 deletions src/effects/effectsbackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <QSet>
#include <QString>

#include "preferences/usersettings.h"
#include "effects/effect.h"
#include "effects/effectinstantiator.h"

Expand All @@ -19,14 +20,16 @@ class EffectProcessor;
class EffectsBackend : public QObject {
Q_OBJECT
public:
EffectsBackend(QObject* pParent, QString name);
EffectsBackend(UserSettingsPointer pConfig, QObject* pParent, QString name);
virtual ~EffectsBackend();

virtual const QString getName() const;

// returns a list sorted like it should be displayed in the GUI
virtual const QList<QString> getEffectIds() const;
virtual EffectManifestPointer getManifest(const QString& effectId) const;
virtual bool getVisibility(const QString& effectId) const;
virtual void addVisibleEffect(const QString& effectId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove these functions.

virtual bool canInstantiateEffect(const QString& effectId) const;
virtual EffectPointer instantiateEffect(
EffectsManager* pEffectsManager, const QString& effectId);
Expand All @@ -47,6 +50,8 @@ class EffectsBackend : public QObject {
EffectInstantiatorPointer(
new EffectProcessorInstantiator<EffectProcessorImpl>()));
}

UserSettingsPointer m_pConfig;
Copy link
Contributor

Choose a reason for hiding this comment

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

No longer needed.


private:
class RegisteredEffect {
Expand All @@ -61,7 +66,6 @@ class EffectsBackend : public QObject {

EffectManifestPointer manifest() const { return m_pManifest; };
EffectInstantiatorPointer initiator() const { return m_pInitator; };

Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep this blank line to separate private and public members.

private:
EffectManifestPointer m_pManifest;
EffectInstantiatorPointer m_pInitator;
Expand All @@ -70,6 +74,7 @@ class EffectsBackend : public QObject {
QString m_name;
QMap<QString, RegisteredEffect> m_registeredEffects;
QList<QString> m_effectIds;
QList<QString> m_visibleEffectIds;
Copy link
Contributor

Choose a reason for hiding this comment

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

EffectsBackend should not track which effects are visible, only EffectsManager.

};

#endif /* EFFECTSBACKEND_H */
34 changes: 29 additions & 5 deletions src/effects/effectsmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ EffectsManager::~EffectsManager() {

bool alphabetizeEffectManifests(EffectManifestPointer pManifest1,
EffectManifestPointer pManifest2) {
return QString::localeAwareCompare(pManifest1->displayName(), pManifest2->displayName()) < 0;

Copy link
Contributor

Choose a reason for hiding this comment

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

remove this blank line

int dNameComp = QString::localeAwareCompare(pManifest1->displayName(), pManifest2->displayName());
Copy link
Contributor

@Be-ing Be-ing May 29, 2018

Choose a reason for hiding this comment

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

What do b and d before NameComp mean here? Backend and... ? Let's use variable names that explain themselves better. How about "backendNameComparison" and "displayNameComparison"?

int bNameComp = QString::localeAwareCompare(pManifest1->backendName(), pManifest2->backendName());
// Add an exception for "Native" backends, to keep the Native effects in the beginning
return (bNameComp ? (bNameComp > 0) : (dNameComp < 0));
}

void EffectsManager::addEffectsBackend(EffectsBackend* pBackend) {
Expand All @@ -79,7 +83,11 @@ void EffectsManager::addEffectsBackend(EffectsBackend* pBackend) {

QList<QString> backendEffects = pBackend->getEffectIds();
for (const QString& effectId : backendEffects) {
m_availableEffectManifests.append(pBackend->getManifest(effectId));
const EffectManifestPointer pManifest = pBackend->getManifest(effectId);
m_availableEffectManifests.append(pManifest);
if (pBackend->getVisibility(effectId)) {
setEffectVisibility(pManifest, true);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this.

}

m_pNumEffectsAvailable->forceSet(m_availableEffectManifests.size());
Expand All @@ -95,9 +103,9 @@ void EffectsManager::addEffectsBackend(EffectsBackend* pBackend) {
}

void EffectsManager::slotBackendRegisteredEffect(EffectManifestPointer pManifest) {
auto insertion_point = qLowerBound(m_availableEffectManifests.begin(),
m_availableEffectManifests.end(),
pManifest, alphabetizeEffectManifests);
auto insertion_point = std::lower_bound(m_availableEffectManifests.begin(),
m_availableEffectManifests.end(),
pManifest, alphabetizeEffectManifests);
m_availableEffectManifests.insert(insertion_point, pManifest);
m_pNumEffectsAvailable->forceSet(m_availableEffectManifests.size());
}
Expand Down Expand Up @@ -310,6 +318,22 @@ EffectButtonParameterSlotPointer EffectsManager::getEffectButtonParameterSlot(
return pParameterSlot;
}

void EffectsManager::setEffectVisibility(EffectManifestPointer pManifest, bool visible) {
if (visible && !m_visibleEffectManifests.contains(pManifest)) {
auto insertion_point = std::lower_bound(m_visibleEffectManifests.begin(),
m_visibleEffectManifests.end(),
pManifest, alphabetizeEffectManifests);
m_visibleEffectManifests.insert(insertion_point, pManifest);
emit visibleEffectsUpdated();
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I thought you needed parentheses around the signal name: emit(visibleEffectsUpdated())

} else if (!visible) {
m_visibleEffectManifests.removeOne(pManifest);
emit visibleEffectsUpdated();
}
}

bool EffectsManager::getEffectVisibility(EffectManifestPointer pManifest) {
return m_visibleEffectManifests.contains(pManifest);
}

void EffectsManager::setup() {
// These controls are used inside EQ Effects
Expand Down
12 changes: 12 additions & 0 deletions src/effects/effectsmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include <QScopedPointer>
#include <QPair>

#include <algorithm>
Copy link
Contributor

Choose a reason for hiding this comment

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

This #include is only needed in the .cpp file, so move it there.


#include "preferences/usersettings.h"
#include "control/controlpotmeter.h"
#include "control/controlpushbutton.h"
Expand Down Expand Up @@ -79,6 +81,9 @@ class EffectsManager : public QObject {
inline const QList<EffectManifestPointer>& getAvailableEffectManifests() const {
return m_availableEffectManifests;
};
inline const QList<EffectManifestPointer>& getVisibleEffectManifests() const {
return m_visibleEffectManifests;
};
const QList<EffectManifestPointer> getAvailableEffectManifestsFiltered(
EffectManifestFilterFnc filter) const;
bool isEQ(const QString& effectId) const;
Expand All @@ -88,6 +93,9 @@ class EffectsManager : public QObject {
EffectManifestPointer getEffectManifest(const QString& effectId) const;
EffectPointer instantiateEffect(const QString& effectId);

void setEffectVisibility(EffectManifestPointer pManifest, bool visibility);
bool getEffectVisibility(EffectManifestPointer pManifest);

// Temporary, but for setting up all the default EffectChains and EffectRacks
void setup();

Expand All @@ -101,6 +109,7 @@ class EffectsManager : public QObject {
signals:
// TODO() Not connected. Can be used when we implement effect PlugIn loading at runtime
void availableEffectsUpdated(EffectManifestPointer);
void visibleEffectsUpdated();

private slots:
void slotBackendRegisteredEffect(EffectManifestPointer pManifest);
Expand All @@ -118,6 +127,7 @@ class EffectsManager : public QObject {
EffectChainManager* m_pEffectChainManager;
QList<EffectsBackend*> m_effectsBackends;
QList<EffectManifestPointer> m_availableEffectManifests;
QList<EffectManifestPointer> m_visibleEffectManifests;

EngineEffectsManager* m_pEngineEffectsManager;

Expand All @@ -132,6 +142,8 @@ class EffectsManager : public QObject {

bool m_underDestruction;

UserSettingsPointer m_pConfig;
Copy link
Contributor

Choose a reason for hiding this comment

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

No longer needed.


DISALLOW_COPY_AND_ASSIGN(EffectsManager);
};

Expand Down
13 changes: 11 additions & 2 deletions src/effects/lv2/lv2backend.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#include "effects/lv2/lv2backend.h"
#include "effects/lv2/lv2manifest.h"

LV2Backend::LV2Backend(QObject* pParent)
: EffectsBackend(pParent, tr("LV2")) {
LV2Backend::LV2Backend(UserSettingsPointer pConfig,
QObject* pParent)
: EffectsBackend(pConfig, pParent, tr("LV2")) {
m_pWorld = lilv_world_new();
initializeProperties();
lilv_world_load_all(m_pWorld);
Expand All @@ -27,6 +28,14 @@ void LV2Backend::enumeratePlugins() {
continue;
}
LV2Manifest* lv2Manifest = new LV2Manifest(plug, m_properties);

const bool visible = m_pConfig->getValue<bool>(ConfigKey("[Visible Effects]",
lv2Manifest->getEffectManifest()->id()), false);
lv2Manifest->getEffectManifest()->setBackendName(this->getName());
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace this->getName() with simply getName()

if (visible) {
addVisibleEffect(lv2Manifest->getEffectManifest()->id());
}

m_registeredEffects.insert(lv2Manifest->getEffectManifest()->id(),
lv2Manifest);
}
Expand Down
4 changes: 3 additions & 1 deletion src/effects/lv2/lv2backend.h
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
#ifndef LV2BACKEND_H
#define LV2BACKEND_H

#include "preferences/usersettings.h"
#include "effects/effectsbackend.h"
#include "effects/lv2/lv2manifest.h"
#include <lilv-0/lilv/lilv.h>

class LV2Backend : public EffectsBackend {
Q_OBJECT
public:
LV2Backend(QObject* pParent=NULL);
LV2Backend(UserSettingsPointer pConfig,
QObject* pParent = nullptr);
virtual ~LV2Backend();

void enumeratePlugins();
Expand Down
4 changes: 2 additions & 2 deletions src/effects/native/nativebackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
#include "effects/native/metronomeeffect.h"
#include "effects/native/tremoloeffect.h"

NativeBackend::NativeBackend(QObject* pParent)
: EffectsBackend(pParent, tr("Native")) {
NativeBackend::NativeBackend(UserSettingsPointer pConfig, QObject* pParent)
: EffectsBackend(pConfig, pParent, tr("Native")) {
// Keep this list in a reasonable order
// Mixing EQs
registerEffect<Bessel4LVMixEQEffect>();
Expand Down
2 changes: 1 addition & 1 deletion src/effects/native/nativebackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
class NativeBackend : public EffectsBackend {
Q_OBJECT
public:
NativeBackend(QObject* pParent=NULL);
NativeBackend(UserSettingsPointer pConfig, QObject* pParent = nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Conventionally the QObject* pParent argument to a constructor is the first argument.

virtual ~NativeBackend();

private:
Expand Down
4 changes: 2 additions & 2 deletions src/mixxx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,10 @@ void MixxxMainWindow::initialize(QApplication* pApp, const CmdlineArgs& args) {

// Create effect backends. We do this after creating EngineMaster to allow
// effect backends to refer to controls that are produced by the engine.
NativeBackend* pNativeBackend = new NativeBackend(m_pEffectsManager);
NativeBackend* pNativeBackend = new NativeBackend(pConfig, m_pEffectsManager);
m_pEffectsManager->addEffectsBackend(pNativeBackend);
#ifdef __LILV__
LV2Backend* pLV2Backend = new LV2Backend(m_pEffectsManager);
LV2Backend* pLV2Backend = new LV2Backend(pConfig, m_pEffectsManager);
m_pEffectsManager->addEffectsBackend(pLV2Backend);
#else
LV2Backend* pLV2Backend = nullptr;
Expand Down
Loading