Merge pull request #2902 from Swiftb0y/enable-multple-cues-per-color-…

…autoassign

Allow assigning more than one Hotcue to a Color in a Colorpalette

CMakeLists.txt

@@ -867,6 +867,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
   src/util/mac.cpp
   src/util/movinginterquartilemean.cpp
   src/util/performancetimer.cpp
+  src/util/rangelist.cpp
   src/util/readaheadsamplebuffer.cpp
   src/util/rlimit.cpp
   src/util/rotary.cpp
@@ -1517,6 +1518,7 @@ add_executable(mixxx-test
   src/test/portmidicontroller_test.cpp
   src/test/portmidienumeratortest.cpp
   src/test/queryutiltest.cpp
+  src/test/rangelist_test.cpp
   src/test/readaheadmanager_test.cpp
   src/test/replaygaintest.cpp
   src/test/rescalertest.cpp

src/preferences/colorpaletteeditor.cpp

@@ -186,7 +186,7 @@ void ColorPaletteEditor::slotTableViewDoubleClicked(const QModelIndex& index) {
 }
 
 void ColorPaletteEditor::slotAddColor() {
-    m_pModel->appendRow(kDefaultPaletteColor);
+    m_pModel->appendRow(kDefaultPaletteColor, {});
     m_pTableView->scrollToBottom();
     m_pTableView->setCurrentIndex(
             m_pModel->index(m_pModel->rowCount() - 1, 0));

src/preferences/colorpaletteeditormodel.cpp

@@ -1,5 +1,14 @@
 #include "preferences/colorpaletteeditormodel.h"
 
+#include <util/assert.h>
+#include <util/rangelist.h>
+
+#include <QList>
+#include <QMap>
+#include <QMultiMap>
+#include <algorithm>
+
+#include "engine/controls/cuecontrol.h"
 #include "moc_colorpaletteeditormodel.cpp"
 
 namespace {
@@ -10,6 +19,13 @@ QIcon toQIcon(const QColor& color) {
     return QIcon(pixmap);
 }
 
+HotcueIndexListItem* toHotcueIndexListItem(QStandardItem* pFrom) {
+    VERIFY_OR_DEBUG_ASSERT(pFrom->type() == QStandardItem::UserType) {
+        return nullptr;
+    }
+    return static_cast<HotcueIndexListItem*>(pFrom);
+}
+
 } // namespace
 
 ColorPaletteEditorModel::ColorPaletteEditorModel(QObject* parent)
@@ -53,25 +69,39 @@ bool ColorPaletteEditorModel::dropMimeData(const QMimeData* data, Qt::DropAction
 bool ColorPaletteEditorModel::setData(const QModelIndex& modelIndex, const QVariant& value, int role) {
     setDirty(true);
     if (modelIndex.isValid() && modelIndex.column() == 1) {
-        bool ok;
-        int hotcueIndex = value.toInt(&ok);
+        const bool initialAttemptSuccessful = QStandardItemModel::setData(modelIndex, value, role);
 
-        // Make sure that the value is valid
-        if (!ok || hotcueIndex <= 0 || hotcueIndex > rowCount()) {
-            return QStandardItemModel::setData(modelIndex, QVariant(), role);
+        const auto* pHotcueIndexListItem = toHotcueIndexListItem(itemFromIndex(modelIndex));
+        VERIFY_OR_DEBUG_ASSERT(pHotcueIndexListItem) {
+            return false;
         }
 
-        // Make sure there is no other row with the same hotcue index
-        for (int i = 0; i < rowCount(); i++) {
-            QModelIndex otherModelIndex = index(i, 1);
-            QVariant otherValue = data(otherModelIndex);
-            int otherHotcueIndex = otherValue.toInt(&ok);
-            if (ok && otherHotcueIndex == hotcueIndex) {
-                QStandardItemModel::setData(otherModelIndex, QVariant(), role);
+        auto hotcueIndexList = pHotcueIndexListItem->getHotcueIndexList();
+
+        // make sure no index is outside of range
+        DEBUG_ASSERT(std::is_sorted(hotcueIndexList.cbegin(), hotcueIndexList.cend()));
+        auto endUpper = std::upper_bound(
+                hotcueIndexList.begin(), hotcueIndexList.end(), NUM_HOT_CUES);
+        hotcueIndexList.erase(endUpper, hotcueIndexList.end());
+        auto endLower = std::upper_bound(hotcueIndexList.begin(), hotcueIndexList.end(), 0);
+        hotcueIndexList.erase(hotcueIndexList.begin(), endLower);
+
+        for (int i = 0; i < rowCount(); ++i) {
+            auto* pHotcueIndexListItem = toHotcueIndexListItem(item(i, 1));
+
+            if (pHotcueIndexListItem == nullptr) {
+                continue;
+            }
+
+            if (i == modelIndex.row()) {
+                pHotcueIndexListItem->setHotcueIndexList(hotcueIndexList);
+            } else {
+                pHotcueIndexListItem->removeIndicies(hotcueIndexList);
             }
         }
-    }
 
+        return initialAttemptSuccessful;
+    }
     return QStandardItemModel::setData(modelIndex, value, role);
 }
 
@@ -84,62 +114,119 @@ void ColorPaletteEditorModel::setColor(int row, const QColor& color) {
     setDirty(true);
 }
 
-void ColorPaletteEditorModel::appendRow(const QColor& color, int hotcueIndex) {
+void ColorPaletteEditorModel::appendRow(
+        const QColor& color, const QList<int>& hotcueIndicies) {
     QStandardItem* pColorItem = new QStandardItem(toQIcon(color), color.name());
     pColorItem->setEditable(false);
     pColorItem->setDropEnabled(false);
 
-    QString hotcueIndexStr;
-    if (hotcueIndex >= 0) {
-        hotcueIndexStr = QString::number(hotcueIndex + 1);
-    }
-
-    QStandardItem* pHotcueIndexItem = new QStandardItem(hotcueIndexStr);
+    QStandardItem* pHotcueIndexItem = new HotcueIndexListItem(hotcueIndicies);
     pHotcueIndexItem->setEditable(true);
     pHotcueIndexItem->setDropEnabled(false);
 
-    QStandardItemModel::appendRow(QList<QStandardItem*>{pColorItem, pHotcueIndexItem});
+    QStandardItemModel::appendRow(
+            QList<QStandardItem*>{pColorItem, pHotcueIndexItem});
 }
 
 void ColorPaletteEditorModel::setColorPalette(const ColorPalette& palette) {
     // Remove all rows
     removeRows(0, rowCount());
 
     // Make a map of hotcue indices
-    QMap<int, int> hotcueColorIndicesMap;
+    QMultiMap<int, int> hotcueColorIndicesMap;
     QList<int> colorIndicesByHotcue = palette.getIndicesByHotcue();
     for (int i = 0; i < colorIndicesByHotcue.size(); i++) {
         int colorIndex = colorIndicesByHotcue.at(i);
-        hotcueColorIndicesMap.insert(colorIndex, i);
+        hotcueColorIndicesMap.insert(colorIndex, i + 1);
     }
 
     for (int i = 0; i < palette.size(); i++) {
         QColor color = mixxx::RgbColor::toQColor(palette.at(i));
-        int colorIndex = hotcueColorIndicesMap.value(i, kNoHotcueIndex);
+        QList<int> colorIndex = hotcueColorIndicesMap.values(i);
         appendRow(color, colorIndex);
     }
 
     setDirty(false);
 }
 
-ColorPalette ColorPaletteEditorModel::getColorPalette(const QString& name) const {
+ColorPalette ColorPaletteEditorModel::getColorPalette(
+        const QString& name) const {
     QList<mixxx::RgbColor> colors;
     QMap<int, int> hotcueColorIndices;
     for (int i = 0; i < rowCount(); i++) {
         QStandardItem* pColorItem = item(i, 0);
-        QStandardItem* pHotcueIndexItem = item(i, 1);
-        mixxx::RgbColor::optional_t color = mixxx::RgbColor::fromQString(pColorItem->text());
+
+        const auto* pHotcueIndexItem = toHotcueIndexListItem(item(i, 1));
+        if (!pHotcueIndexItem) {
+            continue;
+        }
+
+        mixxx::RgbColor::optional_t color =
+                mixxx::RgbColor::fromQString(pColorItem->text());
+
         if (color) {
+            const QList<int> hotcueIndexes = pHotcueIndexItem->getHotcueIndexList();
             colors << *color;
 
-            bool ok;
-            int hotcueIndex = pHotcueIndexItem->text().toInt(&ok);
-            if (ok) {
-                hotcueColorIndices.insert(hotcueIndex - 1, colors.size() - 1);
+            for (int index : hotcueIndexes) {
+                hotcueColorIndices.insert(index - 1, colors.size() - 1);
             }
         }
     }
     // If we have a non consequitive list of hotcue indexes, indexes are shifted down
     // due to the sorting nature of QMap. This is intended, this way we have a color for every hotcue.
     return ColorPalette(name, colors, hotcueColorIndices.values());
 }
+
+HotcueIndexListItem::HotcueIndexListItem(const QList<int>& hotcueList)
+        : QStandardItem(), m_hotcueIndexList(hotcueList) {
+    std::sort(m_hotcueIndexList.begin(), m_hotcueIndexList.end());
+}
+QVariant HotcueIndexListItem::data(int role) const {
+    switch (role) {
+    case Qt::DisplayRole:
+    case Qt::EditRole: {
+        return QVariant(mixxx::stringifyRangeList(m_hotcueIndexList));
+        break;
+    }
+    default:
+        return QStandardItem::data(role);
+        break;
+    }
+}
+
+void HotcueIndexListItem::setData(const QVariant& value, int role) {
+    switch (role) {
+    case Qt::EditRole: {
+        const QList<int> newHotcueIndicies = mixxx::parseRangeList(value.toString());
+
+        if (m_hotcueIndexList != newHotcueIndicies) {
+            m_hotcueIndexList = newHotcueIndicies;
+            emitDataChanged();
+        }
+        break;
+    }
+    default:
+        QStandardItem::setData(value, role);
+        break;
+    }
+}
+
+void HotcueIndexListItem::removeIndicies(const QList<int>& otherIndicies) {
+    DEBUG_ASSERT(std::is_sorted(otherIndicies.cbegin(), otherIndicies.cend()));
+    DEBUG_ASSERT(std::is_sorted(m_hotcueIndexList.cbegin(), m_hotcueIndexList.cend()));
+
+    QList<int> hotcueIndiciesWithOthersRemoved;
+    hotcueIndiciesWithOthersRemoved.reserve(m_hotcueIndexList.size());
+
+    std::set_difference(m_hotcueIndexList.cbegin(),
+            m_hotcueIndexList.cend(),
+            otherIndicies.cbegin(),
+            otherIndicies.cend(),
+            std::back_inserter(hotcueIndiciesWithOthersRemoved));
+
+    if (m_hotcueIndexList != hotcueIndiciesWithOthersRemoved) {
+        m_hotcueIndexList = hotcueIndiciesWithOthersRemoved;
+        emitDataChanged();
+    }
+}

src/preferences/colorpaletteeditormodel.h

@@ -1,23 +1,23 @@
 #pragma once
+#include <QStandardItem>
 #include <QStandardItemModel>
+#include <QVariant>
 
 #include "util/color/colorpalette.h"
 
 // Model that is used by the QTableView of the ColorPaletteEditor.
-// Takes of displaying palette colors and provides a getter/setter for
+// Takes care of displaying palette colors and provides a getter/setter for
 // ColorPalette instances.
 class ColorPaletteEditorModel : public QStandardItemModel {
     Q_OBJECT
   public:
-    static constexpr int kNoHotcueIndex = -1;
-
     ColorPaletteEditorModel(QObject* parent = nullptr);
 
     bool dropMimeData(const QMimeData* data, Qt::DropAction action, int row, int column, const QModelIndex& parent) override;
     bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole) override;
 
     void setColor(int row, const QColor& color);
-    void appendRow(const QColor& color, int hotcueIndex = kNoHotcueIndex);
+    void appendRow(const QColor& color, const QList<int>& hotcueIndicies);
 
     void setDirty(bool bDirty) {
         if (m_bDirty == bDirty) {
@@ -46,3 +46,27 @@ class ColorPaletteEditorModel : public QStandardItemModel {
     bool m_bEmpty;
     bool m_bDirty;
 };
+
+class HotcueIndexListItem : public QStandardItem {
+  public:
+    HotcueIndexListItem(const QList<int>& hotcueList = {});
+
+    void setData(const QVariant& value, int role = Qt::UserRole + 1) override;
+    QVariant data(int role = Qt::UserRole + 1) const override;
+
+    int type() const override {
+        return QStandardItem::UserType;
+    };
+
+    const QList<int>& getHotcueIndexList() const {
+        return m_hotcueIndexList;
+    }
+    void setHotcueIndexList(const QList<int>& list) {
+        m_hotcueIndexList = QList(list);
+    }
+
+    void removeIndicies(const QList<int>& otherIndicies);
+
+  private:
+    QList<int> m_hotcueIndexList;
+};

src/test/rangelist_test.cpp

@@ -0,0 +1,66 @@
+#include "util/rangelist.h"
+
+#include <gtest/gtest.h>
+
+#include <QString>
+#include <algorithm>
+
+#include "test/mixxxtest.h"
+
+void roundTripTestStr(const QString& in, const QString* out = nullptr) {
+    const QString roundTrip = mixxx::stringifyRangeList(mixxx::parseRangeList(in));
+    if (out == nullptr) {
+        EXPECT_QSTRING_EQ(in, roundTrip);
+    } else {
+        EXPECT_QSTRING_EQ(*out, roundTrip);
+    }
+}
+
+void roundTripTestList(const QList<int>& in, const QList<int>* out = nullptr) {
+    const QList<int> roundTrip = mixxx::parseRangeList(mixxx::stringifyRangeList(in));
+    if (out == nullptr) {
+        EXPECT_EQ(in, roundTrip);
+    } else {
+        EXPECT_EQ(*out, roundTrip);
+    }
+}
+
+TEST(DisplayIntListTest, ListEmpty) {
+    roundTripTestList({});
+    roundTripTestStr("");
+}
+
+TEST(DisplayIntListTest, smallContinousRangeIsSeparate) {
+    const QList<int> list = mixxx::parseRangeList(QStringLiteral("1 - 2"));
+    EXPECT_EQ(list, QList({1, 2}));
+    EXPECT_QSTRING_EQ("1, 2", mixxx::stringifyRangeList(list));
+}
+
+TEST(DisplayIntListTest, whiteSpaceAreIgnored) {
+    EXPECT_EQ(QList<int>({1, 2, 3, 4}), mixxx::parseRangeList("   1,2  ,  3, 4"));
+}
+
+TEST(DisplayIntListTest, trailingCommaIsIgnored) {
+    EXPECT_EQ(QList<int>({1}), mixxx::parseRangeList("1,"));
+}
+TEST(DisplayIntListTest, largeRangesAreExpanded) {
+    EXPECT_EQ(QList<int>({1, 2, 3, 4, 5, 6, 7}), mixxx::parseRangeList("1 - 7"));
+}
+
+TEST(DisplayIntListTest, duplicateValuesAreIgnored) {
+    EXPECT_EQ(QList<int>({1, 2, 3}), mixxx::parseRangeList("1, 1, 1, 1, 2, 2, 3"));
+}
+
+TEST(DisplayIntListTest, resultingListIsSortedAscending) {
+    const auto list = mixxx::parseRangeList("4, 2, 3, 1, 6, 5");
+    EXPECT_TRUE(std::is_sorted(list.cbegin(), list.cend()));
+}
+TEST(DisplayIntListTest, consequitiveValuesAreRanges) {
+    EXPECT_QSTRING_EQ("1 - 4", mixxx::stringifyRangeList(QList<int>({1, 2, 3, 4})));
+}
+TEST(DisplayIntListTest, adjacentRangeAndLiteralGetsCollapsed) {
+    EXPECT_EQ(QList<int>({1, 2, 3, 4, 5}), mixxx::parseRangeList("1, 2 - 4, 5"));
+}
+TEST(DisplayIntListTest, overLappingRangesGetUnionized) {
+    EXPECT_EQ(QList<int>({1, 2, 3, 4}), mixxx::parseRangeList("1 - 3, 2 - 4"));
+}