Effect Blacklisting #1674

kshitij98 · 2018-05-20T12:25:16Z

Implemented a functionality to hide effects through the effects tab in Preferences. LV2 Effects are hidden by default.

A few things are still left to be done:

Add a signal to connect visibility mode update to effect selector.
Debug segmentation fault triggered in EffectChain::refreshAllEffects() on clicking OK in preferences.

https://bugs.launchpad.net/mixxx/+bug/1653140

Be-ing · 2018-05-20T12:47:41Z

Could you run git rebase -i upstream/lv2_support2 and squash the last code cleanup commit into the commit before it? (Be sure you have fetched the latest updates from upstream first.) You'll need to use the --force option with git push after that.

Be-ing · 2018-05-20T12:56:50Z

src/effects/effectmanifest.h

+
+    void setVisibility(const bool value) {
+        m_isVisible = value;
+    }


Nothing else in the effect manifest is changed during the runtime of Mixxx. I think it would be better for EffectsManager or EffectsBackend to hold the information of which effects are blacklisted.

I think it would be better to store which effects are hidden in EffectsManager than in EffectsBackend.

Be-ing · 2018-05-20T12:59:53Z

src/effects/native/nativebackend.h

@@ -6,7 +6,7 @@
 class NativeBackend : public EffectsBackend {
    Q_OBJECT
  public:
-    NativeBackend(QObject* pParent=NULL);
+    NativeBackend(UserSettingsPointer pConfig, QObject* pParent = nullptr);


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

Be-ing · 2018-05-20T13:03:22Z

src/widget/weffectselector.cpp

    const QList<EffectManifestPointer> availableEffectManifests =
            m_pEffectsManager->getAvailableEffectManifests();
    QFontMetrics metrics(font());

    for (int i = 0; i < availableEffectManifests.size(); ++i) {
        const EffectManifestPointer pManifest = availableEffectManifests.at(i);
+        if (!pManifest->isVisible())
+            continue;


I don't think WEffectSelector should be responsible for determining which effects to show. Calling m_pEffectsManager->getAvailableEffectManifests() above should take care of that for WEffectSelector. I think the only change needed to WEffectSelector is to call WEffectSelector::populate again when a signal is received from changing the blacklisted effects in the preferences.

Be-ing · 2018-05-20T13:18:07Z

src/preferences/dialog/dlgprefeffects.cpp

+    // Highlight first row
+    availableEffectsList->selectRow(0);
+
+    availableEffectsList->setColumnWidth(0, 50);


The width of the first column cuts off the "Visible" column header text for me (Qt5 with 2.5 scale factor)

Be-ing · 2018-05-20T13:20:30Z

src/preferences/dialog/dlgprefeffectsdlg.ui

@@ -43,28 +43,62 @@
      </attribute>
      <layout class="QHBoxLayout" name="horizontalLayout">
       <item>
-        <widget class="QListWidget" name="availableEffectsList">
-         <property name="sizePolicy">
+        <widget class="QTableView" name="availableEffectsList">


It would be helpful to add a column here for the type of effect (built-in or LV2).

Be-ing · 2018-05-20T13:23:36Z

How should we sort the list of effects in the preferences? Maybe put an alphabetized list of built-in effects first then the alphabetized list of LV2 effects?

Be-ing · 2018-05-20T13:27:30Z

This is a good start, but I think the code could use some refactoring so the visibility of effects isn't stored on the EffectManifest.

Be-ing · 2018-05-21T11:41:58Z

I don't understand the purpose of the EffectProfile class. It seems to be a thin wrapper around the EffectManifest class. It looks like it was copied and pasted from the broadcasting preferences then modified at the price of overcomplicating the code for this case.

kshitij98 · 2018-05-21T12:07:49Z

The purpose of EffectProfile class was to store the intermediate visibility settings of each effect. It allows us to cancel without applying the new visibility settings.

kshitij98 · 2018-05-21T12:15:06Z

I think we can shift the visibility settings (applied and intermediate both) to EffectsManager.

Be-ing · 2018-05-21T12:27:03Z

src/preferences/dialog/dlgprefeffects.cpp

+    for (EffectProfilePtr profile : m_pAvailableEffectsModel->profiles()) {
+        EffectManifest* pManifest = profile->getManifest();
+        pManifest->setVisibility(profile->isVisible());
+        m_pConfig->set(ConfigKey("[Visible Effects]", pManifest->id()), ConfigValue(profile->isVisible()));


I was initially thinking of using a comma or semicolon separated list for the hidden effects, but this approach has simpler code and is more robust in case the effect name has the delimiter in it. 👍

Be-ing · 2018-05-21T23:06:26Z

The purpose of EffectProfile class was to store the intermediate visibility settings of each effect. It allows us to cancel without applying the new visibility settings.

I think we can simplify this class by making it a private struct of EffectSettingsModel:

struct EffectProfile {
    EffectManifestPointer pManifest;
    bool bIsVisible;
}

kshitij98 · 2018-05-23T23:03:37Z

I have made all the required changes. Please review.

I think we can simplify this class by making it a private struct of EffectSettingsModel:

struct EffectProfile {
    EffectManifestPointer pManifest;
    bool bIsVisible;
}

We will anyway have to define accessors and setter functions. So I think we will just be shifting the code from EffectProfile.cpp to EffectSettingsModel.cpp and it is better as it is.

Debug segmentation fault triggered in EffectChain::refreshAllEffects() on clicking OK in preferences.

I confirm that the segmentation fault is in the lv2 branch itself and hence is out of the scope of this PR.

Be-ing · 2018-05-24T22:58:07Z

We will anyway have to define accessors and setter functions.

That's not necessary, just use the public members of the struct directly.

Be-ing

This is better, but there's still more refactoring to do. EffectsBackend shouldn't have anything to do with effect visibility. That should all be handled by DlgPrefEffects and EffectsManager.

Be-ing · 2018-05-23T21:56:37Z

src/effects/lv2/lv2backend.cpp

@@ -31,7 +31,10 @@ void LV2Backend::enumeratePlugins() {

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


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

Be-ing · 2018-05-24T22:56:13Z

src/effects/effectsbackend.h

@@ -73,6 +74,7 @@ class EffectsBackend : public QObject {
    QString m_name;
    QMap<QString, RegisteredEffect> m_registeredEffects;
    QList<QString> m_effectIds;
+    QList<QString> m_visibleEffectIds;


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

Be-ing · 2018-05-24T23:04:26Z

src/effects/effectsbackend.cpp


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


Remove this.

Be-ing · 2018-05-24T23:04:47Z

src/effects/effectsmanager.cpp

@@ -81,6 +85,9 @@ void EffectsManager::addEffectsBackend(EffectsBackend* pBackend) {
    for (const QString& effectId : backendEffects) {
        const EffectManifestPointer pManifest = pBackend->getManifest(effectId);
        m_availableEffectManifests.append(pManifest);
+        if (pBackend->getVisibility(effectId)) {
+            setEffectVisibility(pManifest, true);
+        }


Remove this.

Be-ing · 2018-05-24T23:08:00Z

src/preferences/effectprofile.cpp

+EffectProfile::EffectProfile(EffectManifestPointer pManifest,
+                             bool visibility,
+                             QObject* parent)
+    : QObject(parent),


Does this need to be a QObject subclass? I think a simple struct will work.

Be-ing · 2018-05-24T23:33:05Z

src/effects/effectsbackend.cpp

@@ -30,9 +31,12 @@ void EffectsBackend::registerEffect(const QString& id,

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


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.

Be-ing · 2018-05-24T23:38:44Z

src/effects/effectsmanager.cpp

+                                                m_visibleEffectManifests.end(),
+                                                pManifest, alphabetizeEffectManifests);
+        m_visibleEffectManifests.insert(insertion_point, pManifest);
+        emit visibleEffectsUpdated();


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

Be-ing · 2018-05-24T23:40:10Z

src/preferences/effectsettingsmodel.cpp

@@ -96,6 +99,8 @@ QVariant EffectSettingsModel::headerData(int section, Qt::Orientation orientatio
                return tr("Visible");
            } else if(section == kColumnName) {
                return tr("Name");
+            } else if(section == kColumnType) {


space after "if": } else if (

Be-ing · 2018-05-24T23:40:23Z

src/preferences/effectsettingsmodel.cpp

@@ -109,6 +114,9 @@ Qt::ItemFlags EffectSettingsModel::flags(const QModelIndex& index) const {
    if(index.column() == kColumnName)
        return QAbstractItemModel::flags(index) | Qt::ItemIsSelectable;

+    if(index.column() == kColumnType)


space after "if"

Be-ing · 2018-05-24T23:40:58Z

src/preferences/effectsettingsmodel.cpp

@@ -59,29 +60,31 @@ int EffectSettingsModel::rowCount(const QModelIndex& parent) const {

 int EffectSettingsModel::columnCount(const QModelIndex& parent) const {
    Q_UNUSED(parent);
-    return 2;
+    return 3;


move this magic number to the anonymous namespace at the top of the file

into effect_blacklisting

kshitij98 · 2018-05-28T09:06:58Z

I have made all the requested changes. Please review.

Be-ing

Almost done :)

Be-ing · 2018-05-28T11:31:44Z

src/effects/effectmanifest.h

    QString m_backendName;
-    // QString m_backendName;


remove this

Be-ing · 2018-05-28T11:32:46Z

src/effects/defs.h

@@ -11,9 +11,10 @@ enum class EffectEnableState {
    Enabling
 };

-enum class EffectType {
+enum class BackendType {


It's not immediately clear what "BackendType" means without context. I think "EffectType" is a better name. If you don't like that, then how about "EffectBackendType"?

Yes, EffectBackendType seems to be better than EffectType to me. I think EffectType should denote Phaser, compressor, etc...

I have changed it to EffectBackendType

Good point. "EffectBackendType" works well.

Be-ing · 2018-05-28T11:38:22Z

src/preferences/dialog/dlgprefeffects.cpp

@@ -19,7 +19,7 @@ DlgPrefEffects::DlgPrefEffects(QWidget* pParent,
        EffectManifestPointer pManifest = profile->pManifest;

        // Blacklisted Non-builtin effects by default


Let's elaborate on this comment to explain why we are choosing to do this. Replace the comment with:
"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."

Be-ing · 2018-05-28T11:40:46Z

src/effects/effectmanifest.h

@@ -59,11 +59,15 @@ class EffectManifest final {
        }
    }

+    const BackendType& backendType() const {
+        return m_backendType;
+    }
    const QString& backendName() const {
        return m_backendName;
    }
    void setBackendName(const QString& name) {


remove this

Be-ing · 2018-05-28T11:43:21Z

src/effects/effectsbackend.cpp

@@ -3,7 +3,8 @@
 #include "effects/effectsbackend.h"
 #include "effects/effectsmanager.h"

-EffectsBackend::EffectsBackend(QObject* pParent, QString name)
+EffectsBackend::EffectsBackend(QObject* pParent,
+                               QString name)


replace the QString here with a BackendType value

Be-ing · 2018-05-28T12:12:48Z

src/effects/effectsbackend.cpp

@@ -25,6 +26,8 @@ void EffectsBackend::registerEffect(const QString& id,
        return;
    }

+    pManifest->setBackendName(m_name);


pManifest->setBackendType

Be-ing · 2018-05-28T12:13:22Z

src/effects/effectsbackend.h

@@ -61,7 +62,6 @@ class EffectsBackend : public QObject {

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


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

Be-ing · 2018-05-28T14:45:59Z

src/preferences/dialog/dlgprefeffects.cpp

        return;
-    }
-    QString effectId = pCurrent->data(Qt::UserRole).toString();
+
    EffectManifestPointer pManifest;
    EffectsBackend* pBackend;
    m_pEffectsManager->getEffectManifestAndBackend(effectId, &pManifest, &pBackend);


GitHub won't let me comment directly on the lines below because you haven't modified them yet. Anyway, use effectType->setText(EffectManifest::backendTypeToTranslatedString(pManifest->backendType()) below instead of pBackend->getName().

I noticed an issue with the EffectsManager::getEffectManifestAndBackend and EffectsManager::getEffectManifest functions. If two backends each have an effect that happens to have the same ID string, there will be conflict between them and only the effect in the backend that happened to be registered first will be available to the rest of Mixxx. Currently this is a hypothetical issue because LV2 effects are identified by a URL which wouldn't conflict with the ID strings for built-in effects (assuming a standards-compliant LV2 plugin, which we cannot assume because we do not control the LV2 plugins), so fixing this is not a big priority and does not need to be done in this PR. For now, removing DlgPrefEffects' dependency on EffectsManager::getEffectManifestAndBackend is enough. I think we should keep this issue in mind going forward with the refactoring though. Effects should be identified by both their EffectType and a QString, not a QString alone.

I've removed the dependency on EffectsManager::getEffectManifestAndBackend in the last commit.

I noticed the same a while ago. I think we can create a hash function using the EffectType and the Effect Id, and use it rather than just the Effect Id.

We may consider adding an equality operator to the EffectManifest class that compares both the ID string and the EffectBackendType.

Be-ing · 2018-05-28T14:48:59Z

src/preferences/effectsettingsmodel.cpp

+        } else if (column == kColumnName && role == Qt::DisplayRole) {
+            return profile->pManifest->displayName();
+        } else if (column == kColumnType && role == Qt::DisplayRole) {
+            return profile->pManifest->backendName();


Use EffectManifest::backendTypeToTranslatedString

Be-ing · 2018-05-28T15:00:31Z

src/preferences/dialog/dlgprefeffects.cpp

+}
+
+DlgPrefEffects::~DlgPrefEffects() {
+    delete m_pAvailableEffectsModel;


Does this need to be a pointer? How about making it a plain member variable?

kshitij98 · 2018-05-29T00:44:09Z

I have made the requested changes.

Currently backendTypeToTranslatedString returns QString("") for Unknown backends. We can set it to tr("Unknown"), but for consistency in Effect Info in Effects Preferences, I've returned an empty string.

Be-ing · 2018-05-29T02:07:07Z

src/effects/effectsmanager.cpp

-    int bNameComp = QString::localeAwareCompare(pManifest1->backendName(), pManifest2->backendName());
+    int bNameComp = QString::localeAwareCompare(
+                        EffectManifest::backendTypeToTranslatedString(pManifest1->backendType()), 
+                        EffectManifest::backendTypeToTranslatedString(pManifest2->backendType()));


The results of this could change depending on how "Built-in" is translated. Let's use static_cast<int> to compare the EffectBackendTypes instead. This way the sorting order will match the order the types are declared in the enum class.

Be-ing · 2018-05-29T02:12:12Z

src/preferences/dialog/dlgprefeffects.cpp

-    } else {
-        effectType->clear();
-    }
+    effectType->setText(EffectManifest::backendTypeToTranslatedString(pManifest->backendType()));


Hmm, now that I see this line, it looks unnecessarily verbose. Let's change backendTypeToTranslatedString to a non-static public method of EffectManifest that reads the manifest's m_backendType member so this line could be reduced to:
effectType->setText(pManifest->backendTypeToTranslatedString());

I think we can change the function name to something like translatedBackendName() also.
Because anyway we are using backendTypeToTranslatedString() as an accessor function and translatedBackendName() seems more intuitive to me.

Be-ing · 2018-05-29T02:13:34Z

src/preferences/dialog/dlgprefeffects.cpp

-    m_pAvailableEffectsModel->resetFromEffectManager(pEffectsManager);
-    for (auto profile : m_pAvailableEffectsModel->profiles()) {
+    m_availableEffectsModel.resetFromEffectManager(pEffectsManager);
+    for (auto profile : m_availableEffectsModel.profiles()) {


auto& profile to prevent copying

Be-ing · 2018-05-29T02:16:09Z

src/effects/lv2/lv2backend.cpp

@@ -27,7 +27,7 @@ void LV2Backend::enumeratePlugins() {
            continue;
        }
        LV2Manifest* lv2Manifest = new LV2Manifest(plug, m_properties);
-        lv2Manifest->getEffectManifest()->setBackendName(getName());
+        lv2Manifest->getEffectManifest()->setBackendType(getType());


It is a little odd to use getType() here when you could simply access m_type directly by making it a protected member of EffectsBackend.

Be-ing · 2018-05-29T02:22:30Z

src/effects/defs.h

@@ -11,9 +11,10 @@ enum class EffectEnableState {
    Enabling
 };

-enum class EffectType {
+enum class BackendType {


Good point. "EffectBackendType" works well.

Be-ing · 2018-05-29T02:42:01Z

src/effects/defs.h

+enum class EffectBackendType {
+    BuiltIn,
+    LV2,
+    Unknown


Do we need this Unknown value? Can we remove it?

I added Unknown to handle the case of a manifest not having a backend rather than handling it while setting the effectType in dlgprefeffects. I think we should keep it.

When would an EffectManifest ever not have a backend?

EffectManifest would always have a backend currently. But because the backendName is not set in its constructor but through setBackendName() function, I think we should set it to Unknown for the time being rather than setting it to the first value (Built-in) in the EffectBackendType enum.

Oh, I thought of a use case for this Unknown value. When we parse an XML file specifying an effect chain preset and use the EffectManifest::backendTypeFromString function, we can use the Unknown return value to determine that the effect cannot be loaded.

Be-ing · 2018-05-29T02:49:13Z

src/effects/effectsmanager.h

@@ -8,6 +8,8 @@
 #include <QScopedPointer>
 #include <QPair>

+#include <algorithm>


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

Be-ing · 2018-05-29T03:18:50Z

src/preferences/dialog/dlgprefeffects.cpp

+    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));


Now that we have noticed the potential for name collisions using just the ID string of EffectManifests, I see that problem could also occur here. I propose adding the EffectBackendType converted to a string to the group of the ConfigKey, for example "[Visible Built-in Effects]". We'll need a new public method of EffectManifest, let's call it backendTypeToUntranslatedString, so the logic works regardless of the user's locale (which the user could change between different times Mixxx is run).

Be-ing

Let's add some comments to explain why some not-so-obvious decisions were made to future developers (including ourselves looking at this code in the future).

Be-ing · 2018-05-29T12:51:01Z

src/effects/effectsmanager.cpp

-    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


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

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.

Be-ing · 2018-05-29T12:51:52Z

src/preferences/dialog/dlgprefeffects.cpp

@@ -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()), 


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

Be-ing · 2018-05-29T13:00:16Z

src/effects/effectsmanager.cpp

@@ -3,6 +3,8 @@
 #include <QMetaType>
 #include <QtAlgorithms>

+#include <algorithm>


remove the include from effectsmanager.h too

Be-ing · 2018-05-29T13:05:35Z

src/effects/effectmanifest.h

+                return QString("Unknown");
+        }
+    }
+    QString translatedBackendName() {


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

Be-ing · 2018-05-29T13:06:45Z

src/effects/defs.h

+enum class EffectBackendType {
+    BuiltIn,
+    LV2,
+    Unknown


Oh, I thought of a use case for this Unknown value. When we parse an XML file specifying an effect chain preset and use the EffectManifest::backendTypeFromString function, we can use the Unknown return value to determine that the effect cannot be loaded.

Be-ing · 2018-05-29T13:12:45Z

The tests fail to compile:

In file included from src/test/baseeffecttest.cpp:4:
src/test/baseeffecttest.h: In constructor ‘TestEffectBackend::TestEffectBackend()’:
src/test/baseeffecttest.h:22:61: error: no matching function for call to ‘EffectsBackend::EffectsBackend(NULL, const char [12])’
     TestEffectBackend() : EffectsBackend(NULL, "TestBackend") {
                                                             ^
In file included from src/test/baseeffecttest.h:13,
                 from src/test/baseeffecttest.cpp:4:
src/effects/effectsbackend.h:24:5: note: candidate: ‘EffectsBackend::EffectsBackend(QObject*, EffectBackendType)’
     EffectsBackend(QObject* pParent, EffectBackendType type);
     ^~~~~~~~~~~~~~
src/effects/effectsbackend.h:24:5: note:   no known conversion for argument 2 from ‘const char [12]’ to ‘EffectBackendType’

Use the test=1 option with scons to build the tests.

Be-ing · 2018-05-29T13:17:47Z

src/effects/effectsmanager.cpp

    int bNameComp = static_cast<int>(pManifest1->backendType()) - static_cast<int>(pManifest2->backendType());
+    int dNameComp = QString::localeAwareCompare(pManifest1->displayName(), pManifest2->displayName());


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"?

Be-ing · 2018-05-29T13:21:21Z

src/preferences/dialog/dlgprefeffects.cpp

+
+        // Effects from different backends can have same Effect IDs.
+        // Add backend name to group to uniquely identify those effects.
+        // Use untranslated value to keep the group language independent.


Be-ing · 2018-05-29T13:57:27Z

Looks good to me! I'll wait for Travis and AppVeyor to finish before merging. Thank you for rescuing the LV2 branch from rotting further. I am glad we can finally release it.

Effect Blacklisting

kshitij98 · 2018-07-21T06:17:45Z

https://bugs.launchpad.net/mixxx/+bug/1782868

Effect Slots need to be unloaded if a loaded effect is blacklisted.

Be-ing reviewed May 20, 2018

View reviewed changes

Be-ing added the effects label May 20, 2018

Be-ing added this to the 2.2.0 milestone May 20, 2018

Be-ing reviewed May 21, 2018

View reviewed changes

kshitij98 added 5 commits May 21, 2018 18:47

Added hidden visibility mode for LV2 effects

6d61872

Blacklisted hidden effects from menu

9caa1f7

effectsmanager.cpp: Added visibility check

02cf7dd

Integrated config and fixed blacklisting effects

566db03

Updated Effect Blacklisting Preferences

036f0e2

kshitij98 force-pushed the effect_blacklisting branch from d62acfb to 036f0e2 Compare May 21, 2018 13:25

kshitij98 added 3 commits May 24, 2018 03:19

Shifted visibility logic to EffectsManager

92acfe0

pParent as the first argument convention

94ff53a

Added "Type" column in Available Effects List

260bfcd

Added support for Qt5 (setResizeMode/setSectionResizeMode)

72b1f91

Be-ing suggested changes May 24, 2018

View reviewed changes

Coding standards conformance

eb992a3

Be-ing mentioned this pull request May 27, 2018

LV2 packaging fixes #1672

Merged

kshitij98 added 3 commits May 28, 2018 06:51

Renamed "Native" to "Built-in"

cd39efb

Merge branch 'effect_blacklisting' of https://github.com/kshitij98/mixxx

4e3468c

into effect_blacklisting

Added BackendType enum

b4efad1

Be-ing suggested changes May 28, 2018

View reviewed changes

kshitij98 added 3 commits May 29, 2018 02:31

Changed "BackendType" to "EffectBackendType"

e250f10

Updated blacklisting comment

5bbe893

Removed backendName QString from EffectManifest

ff90165

kshitij98 force-pushed the effect_blacklisting branch from b321c71 to ff90165 Compare May 29, 2018 00:41

Be-ing reviewed May 29, 2018

View reviewed changes

Be-ing suggested changes May 29, 2018

View reviewed changes

kshitij98 force-pushed the effect_blacklisting branch from 7f4582c to c3079c3 Compare May 29, 2018 12:42

Created Backend specific visibility group

b87b2bb

kshitij98 force-pushed the effect_blacklisting branch from c3079c3 to b87b2bb Compare May 29, 2018 12:44

Be-ing suggested changes May 29, 2018

View reviewed changes

Be-ing reviewed May 29, 2018

View reviewed changes

Updated TestEffectBackend

99bb100

kshitij98 force-pushed the effect_blacklisting branch from bbae2ba to 99bb100 Compare May 29, 2018 13:48

Be-ing merged commit 9a456ed into mixxxdj:lv2_support2 May 29, 2018

ronso0 added a commit to ronso0/mixxx that referenced this pull request Jul 19, 2018

Merge pull request mixxxdj#1674 from kshitij98/effect_blacklisting

3e96599

Effect Blacklisting

mixxxbot mentioned this pull request Aug 23, 2022

blacklist effects #8739

Closed

		@@ -30,9 +31,12 @@ void EffectsBackend::registerEffect(const QString& id,

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

		@@ -19,7 +19,7 @@ DlgPrefEffects::DlgPrefEffects(QWidget* pParent,
		EffectManifestPointer pManifest = profile->pManifest;

		// Blacklisted Non-builtin effects by default

		@@ -61,7 +62,6 @@ class EffectsBackend : public QObject {

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

		int bNameComp = static_cast<int>(pManifest1->backendType()) - static_cast<int>(pManifest2->backendType());
		int dNameComp = QString::localeAwareCompare(pManifest1->displayName(), pManifest2->displayName());

Effect Blacklisting #1674

Effect Blacklisting #1674

Conversation

kshitij98 commented May 20, 2018 • edited Loading

Be-ing commented May 20, 2018 • edited Loading

Be-ing May 20, 2018 • edited Loading

Choose a reason for hiding this comment

Be-ing May 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Be-ing commented May 20, 2018

Be-ing commented May 20, 2018

Be-ing commented May 21, 2018 • edited Loading

kshitij98 commented May 21, 2018

kshitij98 commented May 21, 2018

Choose a reason for hiding this comment

Be-ing commented May 21, 2018

kshitij98 commented May 23, 2018 • edited Loading

Be-ing commented May 24, 2018

Be-ing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshitij98 commented May 28, 2018

Be-ing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshitij98 commented May 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshitij98 May 29, 2018 • edited Loading

Choose a reason for hiding this comment

Be-ing May 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Be-ing May 29, 2018 • edited Loading

Choose a reason for hiding this comment

Be-ing left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Be-ing commented May 29, 2018

Be-ing May 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshitij98 commented May 20, 2018 •

edited

Loading

Be-ing commented May 20, 2018 •

edited

Loading

Be-ing May 20, 2018 •

edited

Loading

Be-ing May 21, 2018 •

edited

Loading

Be-ing commented May 21, 2018 •

edited

Loading

kshitij98 commented May 23, 2018 •

edited

Loading

kshitij98 May 29, 2018 •

edited

Loading

Be-ing May 29, 2018 •

edited

Loading

Be-ing May 29, 2018 •

edited

Loading

Be-ing left a comment •

edited

Loading

Be-ing May 29, 2018 •

edited

Loading