diff --git a/src/control/convert.h b/src/control/convert.h new file mode 100644 index 000000000000..a9016b66362e --- /dev/null +++ b/src/control/convert.h @@ -0,0 +1,43 @@ +#pragma once + +#include "util/color/rgbcolor.h" +#include "util/assert.h" + +namespace mixxx { + +namespace control { + +constexpr double kInvalidRgbColor = -1.0; + +inline double doubleFromRgbColor(RgbColor color) { + return static_cast(color); +} + +inline double doubleFromRgbColor(RgbColor::optional_t color) { + if (color) { + return doubleFromRgbColor(*color); + } else { + return kInvalidRgbColor; + } +} + +inline RgbColor::optional_t doubleToRgbColor(double value) { + if (value >= 0) { + const auto code = static_cast(value); + // If value is out of range then unused bits will be + // discarded when converting into an RgbColor. Nevertheless + // check with a debug assertion that the given value + // is in range as expected. Out of range values indicate + // programming errors in other components. + DEBUG_ASSERT(RgbColor::isValidCode(code)); + return RgbColor::optional(code); + } else { + // < 0 or NaN (if non-signalling) + DEBUG_ASSERT(value == kInvalidRgbColor); + return RgbColor::nullopt(); + } +} + +} // namespace control + +} // namespace mixxx diff --git a/src/library/basesqltablemodel.cpp b/src/library/basesqltablemodel.cpp index 3c8be415f648..4f9422296d0b 100644 --- a/src/library/basesqltablemodel.cpp +++ b/src/library/basesqltablemodel.cpp @@ -24,6 +24,7 @@ #include "util/duration.h" #include "util/assert.h" #include "util/performancetimer.h" +#include "util/platform.h" #include "widget/wlibrarytableview.h" namespace { @@ -724,14 +725,9 @@ QVariant BaseSqlTableModel::data(const QModelIndex& index, int role) const { switch (role) { case Qt::ToolTipRole: if (column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_COLOR)) { - const auto color = mixxx::RgbColor::optional(value); - if (color) { - value = mixxx::toQColor(color).name(); - } else { - value = QString(); - } + value = mixxx::RgbColor::toQString(mixxx::RgbColor::fromQVariant(value)); } - [[fallthrough]]; + M_FALLTHROUGH_INTENDED; case Qt::DisplayRole: if (column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_DURATION)) { int duration = value.toInt(); diff --git a/src/library/basetrackcache.cpp b/src/library/basetrackcache.cpp index 5057986dc1ac..29d7e7dbfd49 100644 --- a/src/library/basetrackcache.cpp +++ b/src/library/basetrackcache.cpp @@ -416,7 +416,7 @@ void BaseTrackCache::getTrackValueForColumn(TrackPointer pTrack, } else if (fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_BPM_LOCK) == column) { trackValue.setValue(pTrack->isBpmLocked()); } else if (fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_COLOR) == column) { - trackValue.setValue(toQVariant(pTrack->getColor())); + trackValue.setValue(mixxx::RgbColor::toQVariant(pTrack->getColor())); } else if (fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_COVERART_LOCATION) == column) { trackValue.setValue(pTrack->getCoverInfo().coverLocation); } else if (fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_COVERART_HASH) == column || diff --git a/src/library/colordelegate.cpp b/src/library/colordelegate.cpp index 434d66671189..f92018b0b9f1 100644 --- a/src/library/colordelegate.cpp +++ b/src/library/colordelegate.cpp @@ -13,7 +13,7 @@ ColorDelegate::ColorDelegate(QTableView* pTableView) } void ColorDelegate::paintItem(QPainter* painter, const QStyleOptionViewItem& option, const QModelIndex& index) const { - const auto color = mixxx::RgbColor::optional(index.data()); + const auto color = mixxx::RgbColor::fromQVariant(index.data()); if (!color) { // Filter out track color that is hidden @@ -23,7 +23,7 @@ void ColorDelegate::paintItem(QPainter* painter, const QStyleOptionViewItem& opt return; } - painter->fillRect(option.rect, toQColor(color)); + painter->fillRect(option.rect, mixxx::RgbColor::toQColor(color)); // Paint transparent highlight if row is selected if (option.state & QStyle::State_Selected) { diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index fc86727d0578..0af22d784a8f 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -463,7 +463,7 @@ namespace { pTrackLibraryQuery->bindValue(":tracknumber", track.getTrackNumber()); pTrackLibraryQuery->bindValue(":tracktotal", track.getTrackTotal()); pTrackLibraryQuery->bindValue(":filetype", track.getType()); - pTrackLibraryQuery->bindValue(":color", toQVariant(track.getColor())); + pTrackLibraryQuery->bindValue(":color", mixxx::RgbColor::toQVariant(track.getColor())); pTrackLibraryQuery->bindValue(":comment", track.getComment()); pTrackLibraryQuery->bindValue(":url", track.getURL()); pTrackLibraryQuery->bindValue(":duration", track.getDuration()); @@ -985,7 +985,7 @@ bool setTrackTotal(const QSqlRecord& record, const int column, bool setTrackColor(const QSqlRecord& record, const int column, TrackPointer pTrack) { - pTrack->setColor(mixxx::RgbColor::optional(record.value(column))); + pTrack->setColor(mixxx::RgbColor::fromQVariant(record.value(column))); return false; } diff --git a/src/test/rgbcolor_test.cpp b/src/test/rgbcolor_test.cpp index 92b6ff0a4d2a..95f715a09f90 100644 --- a/src/test/rgbcolor_test.cpp +++ b/src/test/rgbcolor_test.cpp @@ -3,63 +3,105 @@ #include #include "test/mixxxtest.h" +#include "control/convert.h" namespace mixxx { -TEST(RgbColorTest, OptionalRgbColorFromInvalidQColor) { - EXPECT_FALSE(RgbColor::optional(QColor())); - EXPECT_EQ(RgbColor::nullopt(), RgbColor::optional(QColor())); +TEST(RgbColorTest, fromInvalidQColor) { + EXPECT_FALSE(RgbColor::fromQColor(QColor())); + EXPECT_EQ(RgbColor::nullopt(), RgbColor::fromQColor(QColor())); } -TEST(RgbColorTest, OptionalRgbColorFromQColorWithoutAlpha) { - EXPECT_TRUE(RgbColor::optional(QColor::fromRgb(0x000000))); - EXPECT_EQ(0x000000, *RgbColor::optional(QColor::fromRgb(0x000000))); - EXPECT_TRUE(RgbColor::optional(QColor::fromRgb(0xFF0000))); - EXPECT_EQ(RgbColor::optional(0xFF0000), RgbColor::optional(QColor::fromRgb(0xFF0000))); - EXPECT_TRUE(RgbColor::optional(QColor::fromRgb(0x00FF00))); - EXPECT_EQ(RgbColor::optional(0x00FF00), RgbColor::optional(QColor::fromRgb(0x00FF00))); - EXPECT_TRUE(RgbColor::optional(QColor::fromRgb(0x0000FF))); - EXPECT_EQ(RgbColor::optional(0x0000FF), RgbColor::optional(QColor::fromRgb(0x0000FF))); - EXPECT_TRUE(RgbColor::optional(QColor::fromRgb(0xFFFFFF))); - EXPECT_EQ(RgbColor::optional(0xFFFFFF), RgbColor::optional(QColor::fromRgb(0xFFFFFF))); +TEST(RgbColorTest, fromQColorWithoutAlpha) { + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgb(0x000000))); + EXPECT_EQ(RgbColor::optional(0x000000), RgbColor::fromQColor(QColor::fromRgb(0x000000))); + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgb(0xFF0000))); + EXPECT_EQ(RgbColor::optional(0xFF0000), RgbColor::fromQColor(QColor::fromRgb(0xFF0000))); + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgb(0x00FF00))); + EXPECT_EQ(RgbColor::optional(0x00FF00), RgbColor::fromQColor(QColor::fromRgb(0x00FF00))); + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgb(0x0000FF))); + EXPECT_EQ(RgbColor::optional(0x0000FF), RgbColor::fromQColor(QColor::fromRgb(0x0000FF))); + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgb(0xFFFFFF))); + EXPECT_EQ(RgbColor::optional(0xFFFFFF), RgbColor::fromQColor(QColor::fromRgb(0xFFFFFF))); } -TEST(RgbColorTest, OptionalRgbColorFromQColorWithAlpha) { - EXPECT_TRUE(RgbColor::optional(QColor::fromRgba(0xAA000000))); - EXPECT_EQ(RgbColor::optional(0x000000), RgbColor::optional(QColor::fromRgba(0xAA000000))); - EXPECT_TRUE(RgbColor::optional(QColor::fromRgba(0xAAFF0000))); - EXPECT_EQ(RgbColor::optional(0xFF0000), RgbColor::optional(QColor::fromRgba(0xAAFF0000))); - EXPECT_TRUE(RgbColor::optional(QColor::fromRgba(0xAA00FF00))); - EXPECT_EQ(RgbColor::optional(0x00FF00), RgbColor::optional(QColor::fromRgba(0xAA00FF00))); - EXPECT_TRUE(RgbColor::optional(QColor::fromRgba(0xAA0000FF))); - EXPECT_EQ(RgbColor::optional(0x0000FF), RgbColor::optional(QColor::fromRgba(0xAA0000FF))); - EXPECT_TRUE(RgbColor::optional(QColor::fromRgba(0xAAFFFFFF))); - EXPECT_EQ(RgbColor::optional(0xFFFFFF), RgbColor::optional(QColor::fromRgba(0xAAFFFFFF))); +TEST(RgbColorTest, fromQColorWithAlpha) { + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgba(0xAA000000))); + EXPECT_EQ(RgbColor::optional(0x000000), RgbColor::fromQColor(QColor::fromRgba(0xAA000000))); + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgba(0xAAFF0000))); + EXPECT_EQ(RgbColor::optional(0xFF0000), RgbColor::fromQColor(QColor::fromRgba(0xAAFF0000))); + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgba(0xAA00FF00))); + EXPECT_EQ(RgbColor::optional(0x00FF00), RgbColor::fromQColor(QColor::fromRgba(0xAA00FF00))); + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgba(0xAA0000FF))); + EXPECT_EQ(RgbColor::optional(0x0000FF), RgbColor::fromQColor(QColor::fromRgba(0xAA0000FF))); + EXPECT_TRUE(RgbColor::fromQColor(QColor::fromRgba(0xAAFFFFFF))); + EXPECT_EQ(RgbColor::optional(0xFFFFFF), RgbColor::fromQColor(QColor::fromRgba(0xAAFFFFFF))); } -TEST(RgbColorTest,RgbColorToQColor) { - EXPECT_EQ(QColor::fromRgb(0x123456), toQColor(RgbColor(0x123456))); +TEST(RgbColorTest, toQColor) { + EXPECT_EQ(QColor::fromRgb(0x123456), + RgbColor::toQColor(RgbColor(0x123456))); } -TEST(RgbColorTest, OptionalRgbColorToQColor) { - EXPECT_EQ(QColor(), toQColor(RgbColor::nullopt())); +TEST(RgbColorTest, toQColorOptional) { + EXPECT_EQ(QColor(), + RgbColor::toQColor(RgbColor::nullopt())); EXPECT_EQ(QColor::fromRgba(0xAABBCCDD), - toQColor(RgbColor::nullopt(), QColor::fromRgba(0xAABBCCDD))); + RgbColor::toQColor(RgbColor::nullopt(), QColor::fromRgba(0xAABBCCDD))); EXPECT_EQ(QColor::fromRgb(0x123456), - toQColor(RgbColor::optional(QColor::fromRgba(0xAA123456)))); + RgbColor::toQColor(RgbColor::fromQColor(QColor::fromRgba(0xAA123456)))); EXPECT_EQ(QColor::fromRgb(0x123456), - toQColor(RgbColor::optional(QColor::fromRgba(0xAA123456)), QColor::fromRgba(0xAABBCCDD))); + RgbColor::toQColor(RgbColor::fromQColor(QColor::fromRgba(0xAA123456)), QColor::fromRgba(0xAABBCCDD))); } -TEST(RgbColorTest, RgbColorToQVariant) { - EXPECT_EQ(QVariant(), toQVariant(RgbColor::nullopt())); - EXPECT_EQ(QVariant(0x123456), toQVariant(RgbColor(0x123456))); +TEST(RgbColorTest, toQVariant) { + EXPECT_EQ(QVariant(0x123456), + RgbColor::toQVariant(RgbColor(0x123456))); } -TEST(RgbColorTest, OptionalRgbColorToQVariant) { - EXPECT_EQ(QVariant(), toQVariant(RgbColor::nullopt())); +TEST(RgbColorTest, toQVariantOptional) { + EXPECT_EQ(QVariant(), + RgbColor::toQVariant(RgbColor::nullopt())); EXPECT_EQ(QVariant(0x123456), - toQVariant(RgbColor::optional(QColor::fromRgba(0xAA123456)))); + RgbColor::toQVariant(RgbColor::fromQColor(QColor::fromRgba(0xAA123456)))); +} + +TEST(RgbColorTest, toQString) { + EXPECT_EQ(QString("#123456"), + RgbColor::toQString(RgbColor(0x123456))); +} + +TEST(RgbColorTest, toQStringOptional) { + EXPECT_EQ(QString(), + RgbColor::toQString(RgbColor::nullopt())); + EXPECT_EQ(QString("None"), + RgbColor::toQString(RgbColor::nullopt(), QStringLiteral("None"))); + EXPECT_EQ(QString("#123456"), + RgbColor::toQString(RgbColor::fromQColor(QColor::fromRgba(0xAA123456)))); + EXPECT_EQ(QString("#123456"), + RgbColor::toQString(RgbColor::fromQColor(QColor::fromRgba(0xAA123456)), QStringLiteral("None"))); +} + +TEST(RgbColorTest, fromControlValue) { + EXPECT_EQ(RgbColor::nullopt(), + control::doubleToRgbColor(control::kInvalidRgbColor)); + EXPECT_EQ(RgbColor::optional(0), + control::doubleToRgbColor(0)); + EXPECT_EQ(RgbColor::optional(0xFEDCBA), + control::doubleToRgbColor(0xFEDCBA)); +} + +TEST(RgbColorTest, toControlValue) { + EXPECT_EQ(control::kInvalidRgbColor, + control::doubleFromRgbColor(RgbColor::nullopt())); + EXPECT_EQ(0.0, + control::doubleFromRgbColor(RgbColor(0))); + EXPECT_EQ(0.0, + control::doubleFromRgbColor(RgbColor::optional(0))); + EXPECT_EQ(16702650.0, + control::doubleFromRgbColor(RgbColor::optional(0xFEDCBA))); + EXPECT_EQ(16702650.0, + control::doubleFromRgbColor(RgbColor::optional(0xFEDCBA))); } } // namespace mixxx diff --git a/src/util/color/rgbcolor.h b/src/util/color/rgbcolor.h index f7f79b4367c3..03c4c7fc5736 100644 --- a/src/util/color/rgbcolor.h +++ b/src/util/color/rgbcolor.h @@ -20,7 +20,7 @@ class RgbColor { // without an an alpha channel. // We are using a separate typedef, because QRgb implicitly // includes an alpha channel whereas this type does not! - typedef quint32 code_t; + typedef QRgb code_t; static constexpr code_t validateCode(code_t code) { return code & kRgbCodeMask; @@ -36,10 +36,6 @@ class RgbColor { explicit constexpr RgbColor(code_t code) : m_code(validateCode(code)) { } - // Explicit conversion from QColor. - RgbColor(QColor anyColor, code_t codeIfInvalid) - : m_code(anyColorToCode(anyColor, codeIfInvalid)) { - } // Implicit conversion to a color code. constexpr operator code_t() const { @@ -64,85 +60,180 @@ class RgbColor { static constexpr optional_t optional(RgbColor color) { return std::make_optional(color); } - static optional_t optional(QColor color) { - if (color.isValid()) { - return optional(validateCode(color.rgb())); + + /////////////////////////////////////////////////////////////////// + // Conversion functions from/to Qt types + /////////////////////////////////////////////////////////////////// + + // Explicit conversion of both non-optional and optional + // RgbColor values to/from QColor as overloaded free functions. + + static QColor toQColor(RgbColor color) { + return QColor::fromRgb(color); + } + + static QColor toQColor( + RgbColor::optional_t optional, + const QColor& defaultColor = QColor()) { + if (optional) { + return toQColor(*optional); } else { - return nullopt(); + return defaultColor; } } - static optional_t optional(const QVariant& varCode) { - if (varCode.isNull()) { - return nullopt(); + + static RgbColor::optional_t fromQColor( + const QColor& color, + RgbColor::optional_t defaultColor = RgbColor::nullopt()) { + if (color.isValid()) { + return RgbColor::optional( + RgbColor::validateCode(color.rgb())); } else { - DEBUG_ASSERT(varCode.canConvert(QMetaType::UInt)); - bool ok = false; - const auto code = varCode.toUInt(&ok); - VERIFY_OR_DEBUG_ASSERT(ok) { - return nullopt(); - } - return optional(static_cast(code)); + return defaultColor; } } - protected: - // Bitmask of valid codes = 0x00RRGGBB - static constexpr code_t kRgbCodeMask = 0x00FFFFFF; + // Explicit conversion of both non-optional and optional + // RgbColor values to/from QString in the format #RRGGBB, + // e.g. for tool tips or settings. + + static QString toQString( + mixxx::RgbColor color) { + return toQColor(color).name(QColor::HexRgb); + } - static code_t anyColorToCode(QColor anyColor, code_t codeIfInvalid) { - if (anyColor.isValid()) { - // Strip alpha channel bits! - return validateCode(anyColor.rgb()); + static QString toQString( + mixxx::RgbColor::optional_t color, + const QString& defaultString = QString()) { + if (color) { + return toQString(*color); } else { - return codeIfInvalid; + return defaultString; } } - code_t m_code; -}; + static RgbColor::optional_t fromQString( + const QString& hexCode, + RgbColor::optional_t defaultColor = RgbColor::nullopt()) { + const auto color = QColor(hexCode); + if (color.isValid()) { + return fromQColor(color); + } else { + return defaultColor; + } + } -inline bool operator!=(RgbColor lhs, RgbColor rhs) { - return !(lhs == rhs); -} + // Explicit conversion of both non-optional and optional + // RgbColor values to QVariant as overloaded free functions. + // + // The default versions encode the internal color code as + // an unsigned integer. The `Color` and `String` variants + // use the corresponding mapping to QColor and QString + // respectively. + + static QVariant toQVariant( + RgbColor color) { + return QVariant(static_cast(color)); + } -// Explicit conversion of both non-optional and optional -// RgbColor values to QColor as overloaded free functions. + static QVariant toQVariantColor( + RgbColor color) { + return QVariant(toQColor(color)); + } -inline QColor toQColor(RgbColor color) { - return QColor::fromRgb(color); -} + static QVariant toQVariantString( + RgbColor color) { + return QVariant(toQString(color)); + } -inline QColor toQColor(RgbColor::optional_t optional, QColor defaultColor = QColor()) { - if (optional) { - return toQColor(*optional); - } else { - return defaultColor; + static QVariant toQVariant( + optional_t optional, + const QVariant& defaultVariant = QVariant()) { + if (optional) { + return toQVariant(*optional); + } else { + return defaultVariant; + } } -} -// Explicit conversion of both non-optional and optional -// RgbColor values to QVariant as overloaded free functions. + static QVariant toQVariantColor( + optional_t optional, + const QVariant& defaultVariant = QVariant()) { + if (optional) { + return toQVariantColor(*optional); + } else { + return defaultVariant; + } + } -inline QVariant toQVariant(RgbColor color) { - return QVariant(static_cast(color)); -} + static QVariant toQVariantString( + optional_t optional, + const QVariant& defaultVariant = QVariant()) { + if (optional) { + return toQVariantString(*optional); + } else { + return defaultVariant; + } + } -inline QVariant toQVariant(RgbColor::optional_t optional) { - if (optional) { - return toQVariant(*optional); - } else { - return QVariant(); + static optional_t fromQVariant( + const QVariant& varCode, + optional_t defaultColor = nullopt()) { + if (varCode.isNull()) { + return defaultColor; + } + VERIFY_OR_DEBUG_ASSERT(varCode.canConvert(QMetaType::UInt)) { + return defaultColor; + } + const auto value = varCode.value(); + return RgbColor::optional(value); + } + + static optional_t fromQVariantColor( + const QVariant& varColor, + optional_t defaultColor = nullopt()) { + if (varColor.isNull()) { + return defaultColor; + } + VERIFY_OR_DEBUG_ASSERT(varColor.canConvert(QMetaType::QColor)) { + return defaultColor; + } + const auto value = varColor.value(); + return fromQColor(value); } + + static optional_t fromQVariantString( + const QVariant& varString, + optional_t defaultColor = nullopt()) { + if (varString.isNull()) { + return defaultColor; + } + VERIFY_OR_DEBUG_ASSERT(varString.canConvert(QMetaType::QString)) { + return defaultColor; + } + const auto value = varString.value(); + return fromQString(value); + } + + protected: + // Bitmask of valid codes = 0x00RRGGBB + static constexpr code_t kRgbCodeMask = 0x00FFFFFF; + + code_t m_code; +}; + +inline bool operator!=(RgbColor lhs, RgbColor rhs) { + return !(lhs == rhs); } // Debug output stream operators inline QDebug operator<<(QDebug dbg, RgbColor color) { - return dbg << toQColor(color); + return dbg << RgbColor::toQString(color).toLatin1().constData(); } -inline QDebug operator<<(QDebug dbg, RgbColor::optional_t optional) { - return dbg << toQColor(optional); +inline QDebug operator<<(QDebug dbg, RgbColor::optional_t optionalColor) { + return dbg << RgbColor::toQString(optionalColor).toLatin1().constData(); } } // namespace mixxx diff --git a/src/widget/wtracktableview.cpp b/src/widget/wtracktableview.cpp index 141ef03832d7..aff9ac98e5a7 100644 --- a/src/widget/wtracktableview.cpp +++ b/src/widget/wtracktableview.cpp @@ -1093,7 +1093,7 @@ void WTrackTableView::contextMenuEvent(QContextMenuEvent* event) { // Get color of first selected track int column = trackModel->fieldIndex(LIBRARYTABLE_COLOR); QModelIndex index = indices.at(0).sibling(indices.at(0).row(), column); - mixxx::RgbColor::optional_t trackColor = mixxx::RgbColor::optional(index.data()); + auto trackColor = mixxx::RgbColor::fromQVariant(index.data()); // Check if all other selected tracks have the same color bool multipleTrackColors = false; @@ -1101,9 +1101,8 @@ void WTrackTableView::contextMenuEvent(QContextMenuEvent* event) { int row = indices.at(i).row(); QModelIndex index = indices.at(i).sibling(row, column); - mixxx::RgbColor::optional_t otherTrackColor = mixxx::RgbColor::optional(index.data()); - if (trackColor != otherTrackColor) { - trackColor = std::nullopt; + if (trackColor != mixxx::RgbColor::fromQVariant(index.data())) { + trackColor = mixxx::RgbColor::nullopt(); multipleTrackColors = true; break; } @@ -1996,7 +1995,7 @@ void WTrackTableView::slotColorPicked(PredefinedColorPointer pColor) { // TODO: This should be done in a thread for large selections for (const auto& index : selectedTrackIndices) { TrackPointer track = trackModel->getTrack(index); - track->setColor(mixxx::RgbColor::optional(pColor->m_defaultRgba)); + track->setColor(mixxx::RgbColor::fromQColor(pColor->m_defaultRgba)); } m_pMenu->hide();