From a026f5ad75df6f93c87ad53461abd0b8d61fb8ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 15 May 2023 01:12:48 +0200 Subject: [PATCH 1/5] Fix setting the wrong default cue color. This PR fixes also the issue that cues are shown with wrong dafault for a spit of a second in --- src/engine/controls/cuecontrol.cpp | 49 ++++++++++++++++++++---------- src/test/cue_test.cpp | 4 ++- src/track/cue.cpp | 5 +-- src/track/cue.h | 3 +- src/track/track.cpp | 9 ++++-- src/track/track.h | 10 ++++-- 6 files changed, 54 insertions(+), 26 deletions(-) diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index f1272d47a61..467f403917b 100644 --- a/src/engine/controls/cuecontrol.cpp +++ b/src/engine/controls/cuecontrol.cpp @@ -896,35 +896,49 @@ 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; + auto hotcueColorPalette = + m_colorPaletteSettings.getHotcueColorPalette(); 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 = hotcueColorPalette.colorForHotcueIndex(hotcueIndex); } else { - pCue->setColor(mixxx::PredefinedColorPalettes::kDefaultLoopColor); + int loopDefaultColorIndex = m_pConfig->getValue( + ConfigKey("[Controls]", "LoopDefaultColorIndex"), -1); + if (loopDefaultColorIndex < 0 || loopDefaultColorIndex >= hotcueColorPalette.size()) { + loopDefaultColorIndex = hotcueColorPalette.size() - + 1; // default to last color (orange) + } + color = hotcueColorPalette.at(loopDefaultColorIndex); } } else { ConfigKey autoHotcueColorsKey("[Controls]", "auto_hotcue_colors"); if (getConfig()->getValue(autoHotcueColorsKey, false)) { - auto hotcueColorPalette = - m_colorPaletteSettings.getHotcueColorPalette(); - pCue->setColor(hotcueColorPalette.colorForHotcueIndex(hotcueIndex)); + color = hotcueColorPalette.colorForHotcueIndex(hotcueIndex); } else { - pCue->setColor(mixxx::PredefinedColorPalettes::kDefaultCueColor); + int hotcueDefaultColorIndex = m_pConfig->getValue( + ConfigKey("[Controls]", "HotcueDefaultColorIndex"), -1); + if (hotcueDefaultColorIndex < 0 || + hotcueDefaultColorIndex >= hotcueColorPalette.size()) { + hotcueDefaultColorIndex = hotcueColorPalette.size() - + 1; // default to last color (orange) + } + color = hotcueColorPalette.at(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 +2645,7 @@ void HotcueControl::slotHotcueColorChangeRequest(double color) { qWarning() << "slotHotcueColorChanged got invalid value:" << color; return; } + qDebug() << "HotcueControl::slotHotcueColorChangeRequest" << color; m_hotcueColor->setAndConfirm(color); } @@ -2661,6 +2676,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 +2691,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/test/cue_test.cpp b/src/test/cue_test.cpp index cf32dfe25c2..86cad1d0253 100644 --- 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 index 8bf7bb95baa..7c515410bfb 100644 --- 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 index 69536439bce..49af203cee2 100644 --- 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 index f470cf93859..d73054b7d98 100644 --- 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 index cd0fbb5b91c..e3a960446be 100644 --- 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; From 069f17f9fc80d17878e60f2c2e2cf58bdd1b6430 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Mon, 15 May 2023 14:50:44 +0200 Subject: [PATCH 2/5] feat(color): make last color default of palette --- src/util/color/colorpalette.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/color/colorpalette.h b/src/util/color/colorpalette.h index 48a8d0c3c8d..3205a9f7008 100644 --- 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; } From 42fc91c58c0e5a27242dd1a6558478402b387778 Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Mon, 15 May 2023 14:52:42 +0200 Subject: [PATCH 3/5] refactor(cuecontrol): cleanup color logic on hotcue creation --- src/engine/controls/cuecontrol.cpp | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index 467f403917b..8005ec10980 100644 --- a/src/engine/controls/cuecontrol.cpp +++ b/src/engine/controls/cuecontrol.cpp @@ -900,32 +900,27 @@ void CueControl::hotcueSet(HotcueControl* pControl, double value, HotcueSetMode auto hotcueColorPalette = m_colorPaletteSettings.getHotcueColorPalette(); + auto colorFromConfig = [this, &hotcueColorPalette](const ConfigKey& configKey) { + int colorIndex = m_pConfig->getValue(configKey, -1); + if (colorIndex < 0 || colorIndex >= hotcueColorPalette.size()) { + return hotcueColorPalette.defaultColor(); + } + return hotcueColorPalette.at(colorIndex); + }; + if (cueType == mixxx::CueType::Loop) { ConfigKey autoLoopColorsKey("[Controls]", "auto_loop_colors"); if (getConfig()->getValue(autoLoopColorsKey, false)) { color = hotcueColorPalette.colorForHotcueIndex(hotcueIndex); } else { - int loopDefaultColorIndex = m_pConfig->getValue( - ConfigKey("[Controls]", "LoopDefaultColorIndex"), -1); - if (loopDefaultColorIndex < 0 || loopDefaultColorIndex >= hotcueColorPalette.size()) { - loopDefaultColorIndex = hotcueColorPalette.size() - - 1; // default to last color (orange) - } - color = hotcueColorPalette.at(loopDefaultColorIndex); + color = colorFromConfig(ConfigKey("[Controls]", "LoopDefaultColorIndex")); } } else { ConfigKey autoHotcueColorsKey("[Controls]", "auto_hotcue_colors"); if (getConfig()->getValue(autoHotcueColorsKey, false)) { color = hotcueColorPalette.colorForHotcueIndex(hotcueIndex); } else { - int hotcueDefaultColorIndex = m_pConfig->getValue( - ConfigKey("[Controls]", "HotcueDefaultColorIndex"), -1); - if (hotcueDefaultColorIndex < 0 || - hotcueDefaultColorIndex >= hotcueColorPalette.size()) { - hotcueDefaultColorIndex = hotcueColorPalette.size() - - 1; // default to last color (orange) - } - color = hotcueColorPalette.at(hotcueDefaultColorIndex); + color = colorFromConfig(ConfigKey("[Controls]", "HotcueDefaultColorIndex")); } } From 2851f5b8bb8d94eaf3c4306e2694442d26a993ec Mon Sep 17 00:00:00 2001 From: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com> Date: Mon, 15 May 2023 14:55:18 +0200 Subject: [PATCH 4/5] refactor(cuecontrol): comment out debug prints --- src/engine/controls/cuecontrol.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index 8005ec10980..5e6b6f7ca1d 100644 --- a/src/engine/controls/cuecontrol.cpp +++ b/src/engine/controls/cuecontrol.cpp @@ -2640,7 +2640,7 @@ void HotcueControl::slotHotcueColorChangeRequest(double color) { qWarning() << "slotHotcueColorChanged got invalid value:" << color; return; } - qDebug() << "HotcueControl::slotHotcueColorChangeRequest" << color; + // qDebug() << "HotcueControl::slotHotcueColorChangeRequest" << color; m_hotcueColor->setAndConfirm(color); } @@ -2671,7 +2671,7 @@ void HotcueControl::setCue(const CuePointer& pCue) { Cue::StartAndEndPositions pos = pCue->getStartAndEndPosition(); setPosition(pos.startPosition); setEndPosition(pos.endPosition); - qDebug() << "HotcueControl::setCue"; + // qDebug() << "HotcueControl::setCue"; setColor(pCue->getColor()); setStatus((pCue->getType() == mixxx::CueType::Invalid) ? HotcueControl::Status::Empty @@ -2686,7 +2686,7 @@ mixxx::RgbColor::optional_t HotcueControl::getColor() const { } void HotcueControl::setColor(mixxx::RgbColor::optional_t newColor) { - qDebug() << "HotcueControl::::setColor()" << newColor; + // qDebug() << "HotcueControl::setColor()" << newColor; if (newColor) { m_hotcueColor->set(*newColor); } From af2f087df46b947abb2c4a19a88a743afc4de0a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 15 May 2023 17:40:11 +0200 Subject: [PATCH 5/5] Make colorFromConfig a normal member function --- src/engine/controls/cuecontrol.cpp | 25 ++++++++++++------------- src/engine/controls/cuecontrol.h | 1 + 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index 5e6b6f7ca1d..790aa0e39d3 100644 --- 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; @@ -897,28 +907,17 @@ void CueControl::hotcueSet(HotcueControl* pControl, double value, HotcueSetMode int hotcueIndex = pControl->getHotcueIndex(); mixxx::RgbColor color = mixxx::PredefinedColorPalettes::kDefaultCueColor; - auto hotcueColorPalette = - m_colorPaletteSettings.getHotcueColorPalette(); - - auto colorFromConfig = [this, &hotcueColorPalette](const ConfigKey& configKey) { - int colorIndex = m_pConfig->getValue(configKey, -1); - if (colorIndex < 0 || colorIndex >= hotcueColorPalette.size()) { - return hotcueColorPalette.defaultColor(); - } - return hotcueColorPalette.at(colorIndex); - }; - if (cueType == mixxx::CueType::Loop) { ConfigKey autoLoopColorsKey("[Controls]", "auto_loop_colors"); if (getConfig()->getValue(autoLoopColorsKey, false)) { - color = hotcueColorPalette.colorForHotcueIndex(hotcueIndex); + color = m_colorPaletteSettings.getHotcueColorPalette().colorForHotcueIndex(hotcueIndex); } else { color = colorFromConfig(ConfigKey("[Controls]", "LoopDefaultColorIndex")); } } else { ConfigKey autoHotcueColorsKey("[Controls]", "auto_hotcue_colors"); if (getConfig()->getValue(autoHotcueColorsKey, false)) { - color = hotcueColorPalette.colorForHotcueIndex(hotcueIndex); + color = m_colorPaletteSettings.getHotcueColorPalette().colorForHotcueIndex(hotcueIndex); } else { color = colorFromConfig(ConfigKey("[Controls]", "HotcueDefaultColorIndex")); } diff --git a/src/engine/controls/cuecontrol.h b/src/engine/controls/cuecontrol.h index 6e9bd8baf85..2a4326a1822 100644 --- 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;