Skip to content

Commit

Permalink
refactor: initialize global static const color palettes in first use
Browse files Browse the repository at this point in the history
This will avoid the "static initialization order fiasco" by putting
all palettes behind a C++11 "magic static" that ensure that that
they are initialized before they're first used.
  • Loading branch information
Swiftb0y committed Aug 26, 2024
1 parent 3c6e23f commit 1c7afe6
Show file tree
Hide file tree
Showing 16 changed files with 424 additions and 438 deletions.
1 change: 1 addition & 0 deletions .codespellignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ ReflectIn
bufferIn
indexIn
allLocations
statics
6 changes: 3 additions & 3 deletions src/dialog/dlgreplacecuecolor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ DlgReplaceCueColor::DlgReplaceCueColor(
// Set up new color button
ColorPaletteSettings colorPaletteSettings(pConfig);
ColorPalette hotcuePalette = colorPaletteSettings.getHotcueColorPalette();
mixxx::RgbColor firstColor = mixxx::PredefinedColorPalettes::kDefaultCueColor;
mixxx::RgbColor firstColor = mixxx::predefinedcolorpalettes::kDefaultCueColor;
DEBUG_ASSERT(hotcuePalette.size() > 0);
if (hotcuePalette.size() > 0) { // Should always be true
firstColor = hotcuePalette.at(0);
Expand Down Expand Up @@ -116,7 +116,7 @@ DlgReplaceCueColor::DlgReplaceCueColor(
// Set up 'Current color' button
setButtonColor(pushButtonCurrentColor,
mixxx::RgbColor::toQColor(
mixxx::PredefinedColorPalettes::kDefaultCueColor));
mixxx::predefinedcolorpalettes::kDefaultCueColor));

// Update apply button when the current color comparison combobox is
// modified
Expand All @@ -134,7 +134,7 @@ DlgReplaceCueColor::DlgReplaceCueColor(
this);
m_pCurrentColorPickerAction->setObjectName("HotcueColorPickerAction");
m_pCurrentColorPickerAction->setSelectedColor(
mixxx::PredefinedColorPalettes::kDefaultCueColor);
mixxx::predefinedcolorpalettes::kDefaultCueColor);
connect(m_pCurrentColorPickerAction,
&WColorPickerAction::colorPicked,
this,
Expand Down
2 changes: 1 addition & 1 deletion src/engine/controls/cuecontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ void CueControl::hotcueSet(HotcueControl* pControl, double value, HotcueSetMode

int hotcueIndex = pControl->getHotcueIndex();

mixxx::RgbColor color = mixxx::PredefinedColorPalettes::kDefaultCueColor;
mixxx::RgbColor color = mixxx::predefinedcolorpalettes::kDefaultCueColor;
if (cueType == mixxx::CueType::Loop) {
ConfigKey autoLoopColorsKey("[Controls]", "auto_loop_colors");
if (getConfig()->getValue(autoLoopColorsKey, false)) {
Expand Down
13 changes: 8 additions & 5 deletions src/preferences/colorpaletteeditor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ void ColorPaletteEditor::initialize(
m_resetPalette = paletteName;
QString saveName = paletteName;

for (const ColorPalette& palette : mixxx::PredefinedColorPalettes::kPalettes) {
const auto& kPalettes = mixxx::predefinedcolorpalettes::get();

for (const ColorPalette& palette : kPalettes.palettes) {
if (paletteName == palette.getName()) {
saveName = paletteName + QStringLiteral(" (") + tr("Edited") + QChar(')');
ColorPaletteSettings colorPaletteSettings(m_pConfig);
Expand Down Expand Up @@ -206,7 +208,8 @@ void ColorPaletteEditor::slotRemoveColor() {
void ColorPaletteEditor::slotPaletteNameChanged(const QString& text) {
bool bPaletteIsReadOnly = false;
bool bPaletteExists = false;
for (const ColorPalette& palette : mixxx::PredefinedColorPalettes::kPalettes) {
const auto& kPalettes = mixxx::predefinedcolorpalettes::get();
for (const ColorPalette& palette : kPalettes.palettes) {
if (text == palette.getName()) {
bPaletteExists = true;
bPaletteIsReadOnly = true;
Expand All @@ -222,7 +225,7 @@ void ColorPaletteEditor::slotPaletteNameChanged(const QString& text) {
if (bPaletteExists) {
if (!m_pModel->isDirty()) {
bool bPaletteFound = false;
for (const ColorPalette& palette : mixxx::PredefinedColorPalettes::kPalettes) {
for (const ColorPalette& palette : kPalettes.palettes) {
if (text == palette.getName()) {
bPaletteFound = true;
m_pModel->setColorPalette(palette);
Expand All @@ -231,7 +234,7 @@ void ColorPaletteEditor::slotPaletteNameChanged(const QString& text) {
}
if (!bPaletteFound) {
m_pModel->setColorPalette(colorPaletteSettings.getColorPalette(
text, mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette));
text, kPalettes.defaultHotcueColorPalette));
}
}
}
Expand Down Expand Up @@ -277,7 +280,7 @@ void ColorPaletteEditor::slotResetButtonClicked() {
ColorPaletteSettings colorPaletteSettings(m_pConfig);
ColorPalette palette = colorPaletteSettings.getColorPalette(
m_resetPalette,
mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette);
mixxx::predefinedcolorpalettes::get().defaultHotcueColorPalette);
m_pModel->setColorPalette(palette);
slotUpdateButtons();
}
Expand Down
10 changes: 5 additions & 5 deletions src/preferences/colorpalettesettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ ColorPalette ColorPaletteSettings::getColorPalette(
}

// If we find a predefined palette with this name, return it
for (const ColorPalette& palette : mixxx::PredefinedColorPalettes::kPalettes) {
for (const ColorPalette& palette : mixxx::predefinedcolorpalettes::get().palettes) {
if (name == palette.getName()) {
return palette;
}
Expand Down Expand Up @@ -87,7 +87,7 @@ void ColorPaletteSettings::setColorPalette(const QString& name, const ColorPalet
return;
}

for (const ColorPalette& palette : mixxx::PredefinedColorPalettes::kPalettes) {
for (const ColorPalette& palette : mixxx::predefinedcolorpalettes::get().palettes) {
if (name == palette.getName()) {
qDebug() << "Color Palette" << name << "is a built-in palette, not writing to config!";
return;
Expand Down Expand Up @@ -131,7 +131,7 @@ ColorPalette ColorPaletteSettings::getHotcueColorPalette(
const QString& name) const {
return getColorPalette(
name,
mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette);
mixxx::predefinedcolorpalettes::get().defaultHotcueColorPalette);
}

void ColorPaletteSettings::setHotcueColorPalette(const ColorPalette& colorPalette) {
Expand All @@ -148,7 +148,7 @@ ColorPalette ColorPaletteSettings::getTrackColorPalette(
const QString& name) const {
return getColorPalette(
name,
mixxx::PredefinedColorPalettes::kDefaultTrackColorPalette);
mixxx::predefinedcolorpalettes::get().defaultTrackColorPalette);
}

ColorPalette ColorPaletteSettings::getTrackColorPalette() const {
Expand All @@ -170,7 +170,7 @@ ColorPalette ColorPaletteSettings::getKeyColorPalette(
const QString& name) const {
return getColorPalette(
name,
mixxx::PredefinedColorPalettes::kDefaultKeyColorPalette);
mixxx::predefinedcolorpalettes::get().defaultKeyColorPalette);
}

ColorPalette ColorPaletteSettings::getConfigKeyColorPalette() const {
Expand Down
21 changes: 8 additions & 13 deletions src/preferences/dialog/dlgprefcolors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void DlgPrefColors::slotUpdate() {
checkboxKeyColorsEnabled->setChecked(
m_pConfig->getValue(kKeyColorsEnabledConfigKey,
BaseTrackTableModel::kKeyColorsEnabledDefault));
for (const auto& palette : std::as_const(mixxx::PredefinedColorPalettes::kPalettes)) {
for (const auto& palette : mixxx::predefinedcolorpalettes::get().palettes) {
QString paletteName = palette.getName();
QString translatedName = QCoreApplication::translate(
"PredefinedColorPalettes", qPrintable(paletteName));
Expand Down Expand Up @@ -184,23 +184,19 @@ void DlgPrefColors::slotUpdate() {

// Set the default values for all the widgets
void DlgPrefColors::slotResetToDefaults() {
const auto& kPalettes = mixxx::predefinedcolorpalettes::get();
comboBoxHotcueColors->setCurrentText(QCoreApplication::translate(
"PredefinedColorPalettes",
qPrintable(
mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette
.getName())));
kPalettes.defaultHotcueColorPalette.getName())));
comboBoxTrackColors->setCurrentText(QCoreApplication::translate(
"PredefinedColorPalettes",
qPrintable(mixxx::PredefinedColorPalettes::kDefaultTrackColorPalette
.getName())));
qPrintable(kPalettes.defaultTrackColorPalette.getName())));
comboBoxKeyColors->setCurrentText(QCoreApplication::translate(
"PredefinedColorPalettes",
qPrintable(mixxx::PredefinedColorPalettes::kDefaultKeyColorPalette
.getName())));
comboBoxHotcueDefaultColor->setCurrentIndex(
mixxx::PredefinedColorPalettes::kDefaultTrackColorPalette.size());
comboBoxLoopDefaultColor->setCurrentIndex(
mixxx::PredefinedColorPalettes::kDefaultTrackColorPalette.size() - 1);
qPrintable(kPalettes.defaultKeyColorPalette.getName())));
comboBoxHotcueDefaultColor->setCurrentIndex(kPalettes.defaultTrackColorPalette.size());
comboBoxLoopDefaultColor->setCurrentIndex(kPalettes.defaultTrackColorPalette.size() - 1);
checkboxKeyColorsEnabled->setChecked(BaseTrackTableModel::kKeyColorsEnabledDefault);
}

Expand All @@ -213,8 +209,7 @@ void DlgPrefColors::slotApply() {
bool bTrackColorPaletteFound = false;
bool bKeyColorPaletteFound = false;

for (const auto& palette :
std::as_const(mixxx::PredefinedColorPalettes::kPalettes)) {
for (const auto& palette : mixxx::predefinedcolorpalettes::get().palettes) {
if (!bHotcueColorPaletteFound &&
hotcueColorPaletteName == palette.getName()) {
m_colorPaletteSettings.setHotcueColorPalette(palette);
Expand Down
5 changes: 3 additions & 2 deletions src/test/colorconfig_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ TEST_F(ColorConfigTest, LoadSavePalettes) {

TEST_F(ColorConfigTest, DefaultColorPalette) {
ColorPaletteSettings colorPaletteSettings(config());
ASSERT_EQ(mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette,
const auto& kPalettes = mixxx::predefinedcolorpalettes::get();
ASSERT_EQ(kPalettes.defaultHotcueColorPalette,
colorPaletteSettings.getHotcueColorPalette());
ASSERT_EQ(mixxx::PredefinedColorPalettes::kDefaultTrackColorPalette,
ASSERT_EQ(kPalettes.defaultTrackColorPalette,
colorPaletteSettings.getTrackColorPalette());
}
14 changes: 7 additions & 7 deletions src/test/colorpalette_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,25 @@
#include "test/mixxxtest.h"
#include "util/color/predefinedcolorpalettes.h"

class ColorPaletteTest : public MixxxTest {};
class ColorPaletteTest : public MixxxTest {
protected:
ColorPalette palette{mixxx::predefinedcolorpalettes::get().defaultHotcueColorPalette};
void SetUp() override {
ASSERT_TRUE(palette.size() >= 1);
}
};

TEST_F(ColorPaletteTest, NextColor) {
const ColorPalette palette = mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette;
ASSERT_TRUE(palette.size() >= 1);
ASSERT_EQ(palette.nextColor(palette.at(0)), palette.at(1));
ASSERT_EQ(palette.nextColor(palette.at(palette.size() - 1)), palette.at(0));
}

TEST_F(ColorPaletteTest, PreviousColor) {
const ColorPalette palette = mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette;
ASSERT_TRUE(palette.size() >= 1);
ASSERT_EQ(palette.previousColor(palette.at(1)), palette.at(0));
ASSERT_EQ(palette.previousColor(palette.at(0)), palette.at(palette.size() - 1));
}

TEST_F(ColorPaletteTest, NextAndPreviousColorRoundtrip) {
const ColorPalette palette = mixxx::PredefinedColorPalettes::kDefaultHotcueColorPalette;
ASSERT_TRUE(palette.size() >= 1);
ASSERT_EQ(palette.nextColor(palette.previousColor(palette.at(0))), palette.at(0));
ASSERT_EQ(palette.nextColor(palette.previousColor(palette.at(palette.size() - 1))), palette.at(palette.size() - 1));
ASSERT_EQ(palette.previousColor(palette.nextColor(palette.at(0))), palette.at(0));
Expand Down
2 changes: 1 addition & 1 deletion src/test/cue_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ TEST(CueTest, NewCueIsDirty) {
1,
mixxx::audio::kStartFramePos,
mixxx::audio::kInvalidFramePos,
mixxx::PredefinedColorPalettes::kDefaultCueColor);
mixxx::predefinedcolorpalettes::kDefaultCueColor);
EXPECT_TRUE(cue.isDirty());
}

Expand Down
6 changes: 3 additions & 3 deletions src/test/seratotagstest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,14 @@ TEST_F(SeratoTagsTest, SetIncompatibleCueInfos) {
}

TEST_F(SeratoTagsTest, CueColorConversionRoundtrip) {
for (const auto color : mixxx::PredefinedColorPalettes::
kSeratoTrackMetadataHotcueColorPalette) {
const auto& kPalettes = mixxx::predefinedcolorpalettes::get();
for (const auto color : kPalettes.seratoTrackMetadataHotcueColorPalette) {
const auto displayedColor = mixxx::SeratoStoredHotcueColor(color).toDisplayedColor();
const auto storedColor = mixxx::SeratoStoredHotcueColor::fromDisplayedColor(displayedColor);
EXPECT_EQ(color, storedColor.toQRgb());
}

for (const auto color : mixxx::PredefinedColorPalettes::kSeratoDJProHotcueColorPalette) {
for (const auto color : kPalettes.seratoDJProHotcueColorPalette) {
const auto storedColor = mixxx::SeratoStoredHotcueColor::fromDisplayedColor(color);
const auto displayedColor = storedColor.toDisplayedColor();
EXPECT_EQ(color, displayedColor);
Expand Down
2 changes: 1 addition & 1 deletion src/track/cue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Cue::Cue(
sampleRate)),
m_iHotCue(cueInfo.getHotCueIndex().value_or(kNoHotCue)),
m_label(cueInfo.getLabel()),
m_color(cueInfo.getColor().value_or(mixxx::PredefinedColorPalettes::kDefaultCueColor)) {
m_color(cueInfo.getColor().value_or(mixxx::predefinedcolorpalettes::kDefaultCueColor)) {
DEBUG_ASSERT(!m_dbId.isValid());
}

Expand Down
10 changes: 6 additions & 4 deletions src/track/serato/color.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ RgbColor::optional_t SeratoStoredHotcueColor::toDisplayedColor() const {
if (m_color == SeratoStoredColor::kNoColor) {
return RgbColor::nullopt();
}
const auto& kPalettes = mixxx::predefinedcolorpalettes::get();

return RgbColor::optional(getColorFromOtherPalette(
PredefinedColorPalettes::kSeratoTrackMetadataHotcueColorPalette,
PredefinedColorPalettes::kSeratoDJProHotcueColorPalette,
kPalettes.seratoTrackMetadataHotcueColorPalette,
kPalettes.seratoDJProHotcueColorPalette,
m_color));
}

Expand All @@ -92,9 +93,10 @@ SeratoStoredHotcueColor SeratoStoredHotcueColor::fromDisplayedColor(RgbColor::op
if (!color) {
return SeratoStoredHotcueColor(SeratoStoredColor::kNoColor);
}
const auto& kPalettes = mixxx::predefinedcolorpalettes::get();
return SeratoStoredHotcueColor(getColorFromOtherPalette(
PredefinedColorPalettes::kSeratoDJProHotcueColorPalette,
PredefinedColorPalettes::kSeratoTrackMetadataHotcueColorPalette,
kPalettes.seratoDJProHotcueColorPalette,
kPalettes.seratoTrackMetadataHotcueColorPalette,
*color));
}

Expand Down
2 changes: 1 addition & 1 deletion src/track/track.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ void Track::setMainCuePosition(mixxx::audio::FramePos position) {
Cue::kNoHotCue,
position,
mixxx::audio::kInvalidFramePos,
mixxx::PredefinedColorPalettes::kDefaultCueColor));
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().
Expand Down
4 changes: 2 additions & 2 deletions src/track/track.h
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,13 @@ class Track : public QObject {
int hotCueIndex,
mixxx::audio::FramePos startPosition,
mixxx::audio::FramePos endPosition,
mixxx::RgbColor color = mixxx::PredefinedColorPalettes::kDefaultCueColor);
mixxx::RgbColor color = mixxx::predefinedcolorpalettes::kDefaultCueColor);
CuePointer createAndAddCue(
mixxx::CueType type,
int hotCueIndex,
double startPositionSamples,
double endPositionSamples,
mixxx::RgbColor color = mixxx::PredefinedColorPalettes::kDefaultCueColor) {
mixxx::RgbColor color = mixxx::predefinedcolorpalettes::kDefaultCueColor) {
return createAndAddCue(type,
hotCueIndex,
mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
Expand Down
Loading

0 comments on commit 1c7afe6

Please sign in to comment.