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 1 commit
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
16 changes: 13 additions & 3 deletions src/effects/effectmanifest.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,25 @@ class EffectManifest final {
m_metaknobDefault = metaknobDefault;
}

static QString backendTypeToTranslatedString(EffectBackendType type) {
switch (type) {
QString backendName() {
switch (m_backendType) {
case EffectBackendType::BuiltIn:
return QString("Built-in");
case EffectBackendType::LV2:
return QString("LV2");
default:
return QString("Unknown");
}
}
QString translatedBackendName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment: "Use this when showing the string in the GUI"

switch (m_backendType) {
case EffectBackendType::BuiltIn:
//: Used for effects that are built into Mixxx
return QObject::tr("Built-in");
case EffectBackendType::LV2:
return QString("LV2");
default:
return QString("");
return QString();
}
}
static EffectBackendType backendTypeFromString(const QString& name) {
Expand Down
4 changes: 0 additions & 4 deletions src/effects/effectsbackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ EffectsBackend::~EffectsBackend() {
m_effectIds.clear();
}

const EffectBackendType EffectsBackend::getType() const {
return m_type;
}

void EffectsBackend::registerEffect(const QString& id,
EffectManifestPointer pManifest,
EffectInstantiatorPointer pInstantiator) {
Expand Down
5 changes: 2 additions & 3 deletions src/effects/effectsbackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ class EffectsBackend : public QObject {
EffectsBackend(QObject* pParent, EffectBackendType type);
virtual ~EffectsBackend();

virtual const EffectBackendType getType() 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;
Expand All @@ -50,6 +48,8 @@ class EffectsBackend : public QObject {
new EffectProcessorInstantiator<EffectProcessorImpl>()));
}

EffectBackendType m_type;

private:
class RegisteredEffect {
public:
Expand All @@ -69,7 +69,6 @@ class EffectsBackend : public QObject {
EffectInstantiatorPointer m_pInitator;
};

EffectBackendType m_type;
QMap<QString, RegisteredEffect> m_registeredEffects;
QList<QString> m_effectIds;
};
Expand Down
7 changes: 3 additions & 4 deletions src/effects/effectsmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <QMetaType>
#include <QtAlgorithms>

#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.

remove the include from effectsmanager.h too


#include "engine/effects/engineeffectsmanager.h"
#include "effects/effectchainmanager.h"
#include "effects/effectsbackend.h"
Expand Down Expand Up @@ -69,10 +71,7 @@ EffectsManager::~EffectsManager() {
bool alphabetizeEffectManifests(EffectManifestPointer pManifest1,
EffectManifestPointer pManifest2) {
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(
EffectManifest::backendTypeToTranslatedString(pManifest1->backendType()),
EffectManifest::backendTypeToTranslatedString(pManifest2->backendType()));
// Add an exception for "Built-in" backends, to keep the Built-in effects in the beginning
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment here: "Sort built-in effects first before external plugins"

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also add a comment in defs.h where the EffectBackendType enum is declared explaining that the order that the values are declared in the enum class determines how they are sorted in the WEffectSelector and DlgPrefEffects.

int bNameComp = static_cast<int>(pManifest1->backendType()) - static_cast<int>(pManifest2->backendType());
return (bNameComp ? (bNameComp < 0) : (dNameComp < 0));
}

Expand Down
2 changes: 1 addition & 1 deletion src/effects/lv2/lv2backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ void LV2Backend::enumeratePlugins() {
continue;
}
LV2Manifest* lv2Manifest = new LV2Manifest(plug, m_properties);
lv2Manifest->getEffectManifest()->setBackendType(getType());
lv2Manifest->getEffectManifest()->setBackendType(m_type);
m_registeredEffects.insert(lv2Manifest->getEffectManifest()->id(),
lv2Manifest);
}
Expand Down
9 changes: 5 additions & 4 deletions src/preferences/dialog/dlgprefeffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ DlgPrefEffects::DlgPrefEffects(QWidget* pParent,
setupUi(this);

m_availableEffectsModel.resetFromEffectManager(pEffectsManager);
for (auto profile : m_availableEffectsModel.profiles()) {
for (auto& profile : m_availableEffectsModel.profiles()) {
EffectManifestPointer pManifest = profile->pManifest;

// Users are likely to have lots of external plugins installed and
// many of them are useless for DJing. To avoid cluttering the list
// shown in WEffectSelector, blacklist external plugins by default.
bool defaultValue = (pManifest->backendType() == EffectBackendType::BuiltIn);
bool visible = m_pConfig->getValue<bool>(ConfigKey("[Visible Effects]",
bool visible = m_pConfig->getValue<bool>(ConfigKey("[Visible " + pManifest->backendName() + " Effects]",
pManifest->id()), defaultValue);
profile->bIsVisible = visible;
m_pEffectsManager->setEffectVisibility(pManifest, visible);
Expand Down Expand Up @@ -62,7 +62,8 @@ void DlgPrefEffects::slotApply() {
for (EffectProfilePtr profile : m_availableEffectsModel.profiles()) {
EffectManifestPointer pManifest = profile->pManifest;
m_pEffectsManager->setEffectVisibility(pManifest, profile->bIsVisible);
m_pConfig->set(ConfigKey("[Visible Effects]", pManifest->id()), ConfigValue(profile->bIsVisible));
m_pConfig->set(ConfigKey("[Visible " + pManifest->backendName() + " Effects]", pManifest->id()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment here explaining why we put the backendName in the ConfigKey group.

ConfigValue(profile->bIsVisible));
}
}

Expand Down Expand Up @@ -90,5 +91,5 @@ void DlgPrefEffects::availableEffectsListItemSelected(const QModelIndex& selecte
effectAuthor->setText(pManifest->author());
effectDescription->setText(pManifest->description());
effectVersion->setText(pManifest->version());
effectType->setText(EffectManifest::backendTypeToTranslatedString(pManifest->backendType()));
effectType->setText(pManifest->translatedBackendName());
}
2 changes: 1 addition & 1 deletion src/preferences/effectsettingsmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ QVariant EffectSettingsModel::data(const QModelIndex& index, int role) const {
} else if (column == kColumnName && role == Qt::DisplayRole) {
return profile->pManifest->displayName();
} else if (column == kColumnType && role == Qt::DisplayRole) {
return EffectManifest::backendTypeToTranslatedString(profile->pManifest->backendType());
return profile->pManifest->translatedBackendName();
}
}

Expand Down