Merge pull request mixxxdj#11554 from daschuer/gh11260

Fix setting the wrong default cue color.
Swiftb0y · May 15, 2023 · ab83bef · ab83bef
2 parents c4090e4 + af2f087
commit ab83bef
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 27 deletions.
diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp
@@ -820,6 +820,16 @@ void CueControl::quantizeChanged(double v) {
     }
 }
 
+mixxx::RgbColor CueControl::colorFromConfig(const ConfigKey& configKey) {
+    auto hotcueColorPalette =
+            m_colorPaletteSettings.getHotcueColorPalette();
+    int colorIndex = m_pConfig->getValue(configKey, -1);
+    if (colorIndex < 0 || colorIndex >= hotcueColorPalette.size()) {
+        return hotcueColorPalette.defaultColor();
+    }
+    return hotcueColorPalette.at(colorIndex);
+};
+
 void CueControl::hotcueSet(HotcueControl* pControl, double value, HotcueSetMode mode) {
     //qDebug() << "CueControl::hotcueSet" << value;
 
@@ -896,35 +906,33 @@ void CueControl::hotcueSet(HotcueControl* pControl, double value, HotcueSetMode
 
     int hotcueIndex = pControl->getHotcueIndex();
 
-    CuePointer pCue = m_pLoadedTrack->createAndAddCue(
-            cueType,
-            hotcueIndex,
-            cueStartPosition,
-            cueEndPosition);
-
-    // TODO(XXX) deal with spurious signals
-    attachCue(pCue, pControl);
-
+    mixxx::RgbColor color = mixxx::PredefinedColorPalettes::kDefaultCueColor;
     if (cueType == mixxx::CueType::Loop) {
         ConfigKey autoLoopColorsKey("[Controls]", "auto_loop_colors");
         if (getConfig()->getValue(autoLoopColorsKey, false)) {
-            auto hotcueColorPalette =
-                    m_colorPaletteSettings.getHotcueColorPalette();
-            pCue->setColor(hotcueColorPalette.colorForHotcueIndex(hotcueIndex));
+            color = m_colorPaletteSettings.getHotcueColorPalette().colorForHotcueIndex(hotcueIndex);
         } else {
-            pCue->setColor(mixxx::PredefinedColorPalettes::kDefaultLoopColor);
+            color = colorFromConfig(ConfigKey("[Controls]", "LoopDefaultColorIndex"));
         }
     } else {
         ConfigKey autoHotcueColorsKey("[Controls]", "auto_hotcue_colors");
         if (getConfig()->getValue(autoHotcueColorsKey, false)) {
-            auto hotcueColorPalette =
-                    m_colorPaletteSettings.getHotcueColorPalette();
-            pCue->setColor(hotcueColorPalette.colorForHotcueIndex(hotcueIndex));
+            color = m_colorPaletteSettings.getHotcueColorPalette().colorForHotcueIndex(hotcueIndex);
         } else {
-            pCue->setColor(mixxx::PredefinedColorPalettes::kDefaultCueColor);
+            color = colorFromConfig(ConfigKey("[Controls]", "HotcueDefaultColorIndex"));
         }
     }
 
+    CuePointer pCue = m_pLoadedTrack->createAndAddCue(
+            cueType,
+            hotcueIndex,
+            cueStartPosition,
+            cueEndPosition,
+            color);
+
+    // TODO(XXX) deal with spurious signals
+    attachCue(pCue, pControl);
+
     if (cueType == mixxx::CueType::Loop) {
         setCurrentSavedLoopControlAndActivate(pControl);
     }
@@ -2631,6 +2639,7 @@ void HotcueControl::slotHotcueColorChangeRequest(double color) {
         qWarning() << "slotHotcueColorChanged got invalid value:" << color;
         return;
     }
+    // qDebug() << "HotcueControl::slotHotcueColorChangeRequest" << color;
     m_hotcueColor->setAndConfirm(color);
 }
 
@@ -2661,6 +2670,7 @@ void HotcueControl::setCue(const CuePointer& pCue) {
     Cue::StartAndEndPositions pos = pCue->getStartAndEndPosition();
     setPosition(pos.startPosition);
     setEndPosition(pos.endPosition);
+    // qDebug() << "HotcueControl::setCue";
     setColor(pCue->getColor());
     setStatus((pCue->getType() == mixxx::CueType::Invalid)
                     ? HotcueControl::Status::Empty
@@ -2675,6 +2685,7 @@ mixxx::RgbColor::optional_t HotcueControl::getColor() const {
 }
 
 void HotcueControl::setColor(mixxx::RgbColor::optional_t newColor) {
+    // qDebug() << "HotcueControl::setColor()" << newColor;
     if (newColor) {
         m_hotcueColor->set(*newColor);
     }

diff --git a/src/engine/controls/cuecontrol.h b/src/engine/controls/cuecontrol.h
@@ -287,6 +287,7 @@ class CueControl : public EngineControl {
     void seekOnLoad(mixxx::audio::FramePos seekOnLoadPosition);
     void setHotcueFocusIndex(int hotcueIndex);
     int getHotcueFocusIndex() const;
+    mixxx::RgbColor colorFromConfig(const ConfigKey& configKey);
 
     UserSettingsPointer m_pConfig;
     ColorPaletteSettings m_colorPaletteSettings;

diff --git a/src/test/cue_test.cpp b/src/test/cue_test.cpp
@@ -5,6 +5,7 @@
 #include "engine/engine.h"
 #include "test/mixxxtest.h"
 #include "util/color/color.h"
+#include "util/color/predefinedcolorpalettes.h"
 
 namespace mixxx {
 
@@ -13,7 +14,8 @@ TEST(CueTest, NewCueIsDirty) {
             mixxx::CueType::HotCue,
             1,
             mixxx::audio::kStartFramePos,
-            mixxx::audio::kInvalidFramePos);
+            mixxx::audio::kInvalidFramePos,
+            mixxx::PredefinedColorPalettes::kDefaultCueColor);
     EXPECT_TRUE(cue.isDirty());
 }
 

diff --git a/src/track/cue.cpp b/src/track/cue.cpp
@@ -97,13 +97,14 @@ Cue::Cue(
         mixxx::CueType type,
         int hotCueIndex,
         mixxx::audio::FramePos startPosition,
-        mixxx::audio::FramePos endPosition)
+        mixxx::audio::FramePos endPosition,
+        mixxx::RgbColor color)
         : m_bDirty(true), // not yet in database, needs to be saved
           m_type(type),
           m_startPosition(startPosition),
           m_endPosition(endPosition),
           m_iHotCue(hotCueIndex),
-          m_color(mixxx::PredefinedColorPalettes::kDefaultCueColor) {
+          m_color(color) {
     DEBUG_ASSERT(m_iHotCue == kNoHotCue || m_iHotCue >= mixxx::kFirstHotCueIndex);
     DEBUG_ASSERT(m_startPosition.isValid() || m_endPosition.isValid());
     DEBUG_ASSERT(!m_dbId.isValid());

diff --git a/src/track/cue.h b/src/track/cue.h
@@ -56,7 +56,8 @@ class Cue : public QObject {
             mixxx::CueType type,
             int hotCueIndex,
             mixxx::audio::FramePos startPosition,
-            mixxx::audio::FramePos endPosition);
+            mixxx::audio::FramePos endPosition,
+            mixxx::RgbColor color);
 
     ~Cue() override = default;
 

diff --git a/src/track/track.cpp b/src/track/track.cpp
@@ -912,7 +912,8 @@ void Track::setMainCuePosition(mixxx::audio::FramePos position) {
                     mixxx::CueType::MainCue,
                     Cue::kNoHotCue,
                     position,
-                    mixxx::audio::kInvalidFramePos));
+                    mixxx::audio::kInvalidFramePos,
+                    mixxx::PredefinedColorPalettes::kDefaultCueColor));
             // While this method could be called from any thread,
             // associated Cue objects should always live on the
             // same thread as their host, namely this->thread().
@@ -964,7 +965,8 @@ CuePointer Track::createAndAddCue(
         mixxx::CueType type,
         int hotCueIndex,
         mixxx::audio::FramePos startPosition,
-        mixxx::audio::FramePos endPosition) {
+        mixxx::audio::FramePos endPosition,
+        mixxx::RgbColor color) {
     VERIFY_OR_DEBUG_ASSERT(hotCueIndex == Cue::kNoHotCue ||
             hotCueIndex >= mixxx::kFirstHotCueIndex) {
         return CuePointer{};
@@ -976,7 +978,8 @@ CuePointer Track::createAndAddCue(
             type,
             hotCueIndex,
             startPosition,
-            endPosition));
+            endPosition,
+            color));
     // While this method could be called from any thread,
     // associated Cue objects should always live on the
     // same thread as their host, namely this->thread().

diff --git a/src/track/track.h b/src/track/track.h
@@ -11,6 +11,7 @@
 #include "track/cueinfoimporter.h"
 #include "track/track_decl.h"
 #include "track/trackrecord.h"
+#include "util/color/predefinedcolorpalettes.h"
 #include "util/compatibility/qmutex.h"
 #include "util/fileaccess.h"
 #include "util/memory.h"
@@ -298,18 +299,21 @@ class Track : public QObject {
             mixxx::CueType type,
             int hotCueIndex,
             mixxx::audio::FramePos startPosition,
-            mixxx::audio::FramePos endPosition);
+            mixxx::audio::FramePos endPosition,
+            mixxx::RgbColor color = mixxx::PredefinedColorPalettes::kDefaultCueColor);
     CuePointer createAndAddCue(
             mixxx::CueType type,
             int hotCueIndex,
             double startPositionSamples,
-            double endPositionSamples) {
+            double endPositionSamples,
+            mixxx::RgbColor color = mixxx::PredefinedColorPalettes::kDefaultCueColor) {
         return createAndAddCue(type,
                 hotCueIndex,
                 mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
                         startPositionSamples),
                 mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
-                        endPositionSamples));
+                        endPositionSamples),
+                color);
     }
     CuePointer findCueByType(mixxx::CueType type) const; // NOTE: Cannot be used for hotcues.
     CuePointer findCueById(DbId id) const;

diff --git a/src/util/color/colorpalette.h b/src/util/color/colorpalette.h
@@ -45,6 +45,10 @@ class ColorPalette final {
         return m_colorList.end();
     }
 
+    mixxx::RgbColor defaultColor() const {
+        return m_colorList.last();
+    }
+
     QString getName() const {
         return m_name;
     }