From d2af5c95f4057a5eaf755ce7149e749e005beac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= <44814898+BruebachL@users.noreply.github.com> Date: Sun, 8 Nov 2026 14:50:35 +0100 Subject: [PATCH] Address review findings --- cockatrice/src/interface/theme_config.cpp | 13 +-- cockatrice/src/interface/theme_config.h | 10 ++- cockatrice/src/interface/theme_icon.cpp | 11 ++- cockatrice/src/interface/theme_icon.h | 10 +-- cockatrice/src/interface/theme_icon_ink.cpp | 44 +++++++--- cockatrice/src/interface/theme_icon_ink.h | 8 +- cockatrice/src/interface/theme_manager.cpp | 11 ++- .../themes/Fabric/palette-default-dark.toml | 2 +- .../themes/Fabric/palette-default-light.toml | 2 +- .../themes/Fusion/palette-default-dark.toml | 2 +- .../themes/Fusion/palette-default-light.toml | 2 +- .../themes/Leather/palette-default-dark.toml | 2 +- .../themes/Leather/palette-default-light.toml | 2 +- .../themes/Plasma/palette-default-dark.toml | 2 +- .../themes/Plasma/palette-default-light.toml | 2 +- .../themes/System/palette-default-dark.toml | 20 ++--- .../themes/System/palette-default-light.toml | 20 ++--- .../VelvetMarble/palette-default-dark.toml | 2 +- .../VelvetMarble/palette-default-light.toml | 2 +- .../theme_icon_render_test.cpp | 84 +++++++++++-------- 20 files changed, 154 insertions(+), 97 deletions(-) diff --git a/cockatrice/src/interface/theme_config.cpp b/cockatrice/src/interface/theme_config.cpp index 6abf63157..848be2231 100644 --- a/cockatrice/src/interface/theme_config.cpp +++ b/cockatrice/src/interface/theme_config.cpp @@ -458,17 +458,18 @@ QColor defaultAppColor(AppColor::Role role, bool dark) switch (role) { case AppColor::AccentStrong: case AppColor::AccentSoft: - // The neutral icon family is palette-derived like the accent, so a - // theme without an authored icon family keeps following the platform - // instead of pinning one scheme's ink. + // The neutral icon family is palette-derived like the accent: a theme + // that leaves it unauthored (System, and every theme written before the + // roles existed) keeps the live platform palette's WindowText instead + // of pinning one scheme's ink. case AppColor::Icon: case AppColor::IconStrong: case AppColor::IconSoft: return {}; // The semantic family follows the same chroma ladder as the authored - // themes. A theme without stored values (System) has no per-surface - // palette to measure against, so these are authored against the - // representative platform surfaces the System theme runs on. + // themes. A theme that authors none of it has no per-surface palette to + // measure against, so these are authored against the representative + // platform surfaces the System theme runs on. case AppColor::Positive: return QColor(dark ? QStringLiteral("#6dd58c") : QStringLiteral("#1a7d3a")); case AppColor::PositiveStrong: diff --git a/cockatrice/src/interface/theme_config.h b/cockatrice/src/interface/theme_config.h index fb052ad1e..68734e762 100644 --- a/cockatrice/src/interface/theme_config.h +++ b/cockatrice/src/interface/theme_config.h @@ -63,11 +63,13 @@ Q_ENUM_NS(Role) * * Accent roles and the neutral Icon family return an invalid QColor because * ThemeManager derives them from the palette: the accent from the highlight, - * the icon ink from the neutral text color plus the highlight. Semantic roles + * the icon ink from the live WindowText held to the label floor. Semantic roles * return an opaque default that suits the scheme, so themes written before the - * role existed still render it. A theme with no authored icon family keeps - * deriving its icon ink from the platform palette instead of pinning one - * scheme's colors. + * role existed still render it. The System theme deliberately leaves the + * neutral family unauthored: the palette file it picks cannot know the platform + * scheme, so a fixed ladder would be invisible on the other one. It derives + * neutral ink from the live platform palette instead of pinning one scheme's + * colors. */ QColor defaultAppColor(AppColor::Role role, bool dark); diff --git a/cockatrice/src/interface/theme_icon.cpp b/cockatrice/src/interface/theme_icon.cpp index cb37f6b77..f0c9ef84f 100644 --- a/cockatrice/src/interface/theme_icon.cpp +++ b/cockatrice/src/interface/theme_icon.cpp @@ -64,10 +64,19 @@ AppColor::Role emphasisRole(AppColor::Role role, IconEmphasis emphasis) * * The fill needs the surface scheme to choose the badge glyph ink, and it cannot * read it from the tone: an authored quiet level is a dark color on the dark - * scheme too. + * scheme too. The theme's resolved scheme is the same source the palette file + * and the default app colors are chosen from, so the fill follows it instead of + * re-deriving the scheme from the live Window lightness, which can disagree + * when a theme pins a scheme the platform palette does not match. */ ThemeIconFill::Scheme activeScheme() { + if (themeManager != nullptr) { + return themeManager->isDarkModeActive() ? ThemeIconFill::Scheme::Dark : ThemeIconFill::Scheme::Light; + } + + // No manager (test binaries link the icon engine without the app): fall + // back to the live palette, which is the only scheme signal left. const QPalette &palette = qApp->palette(); return palette.color(QPalette::Active, QPalette::Window).lightness() < 128 ? ThemeIconFill::Scheme::Dark : ThemeIconFill::Scheme::Light; diff --git a/cockatrice/src/interface/theme_icon.h b/cockatrice/src/interface/theme_icon.h index 8f33b1f67..82500292a 100644 --- a/cockatrice/src/interface/theme_icon.h +++ b/cockatrice/src/interface/theme_icon.h @@ -104,11 +104,11 @@ public: * hue and saturation stay the author's. * * Neutral label roles (WindowText, Text) resolve to icon ink rather than the - * raw palette color: ThemeIconInk::glyphInk derives it from the accent and - * holds the glyph to a contrast floor, because a 2px glyph in body-text ink - * reads thinner than the text beside it. A theme that authors the neutral - * icon roles overrides that derivation. Semantic roles keep the hue the call - * site asked for. + * raw palette color: an unauthored neutral family derives it from the live + * WindowText and holds the glyph to a contrast floor, because a 2px glyph in + * body-text ink reads thinner than the text beside it. A theme that authors + * the neutral icon roles overrides that derivation. Semantic roles keep the + * hue the call site asked for. * * Emphasis selects the family's authored variant, so the palette editor's * rows are the whole source of truth. A caller that names a variant diff --git a/cockatrice/src/interface/theme_icon_ink.cpp b/cockatrice/src/interface/theme_icon_ink.cpp index 0b5364674..7ed458f70 100644 --- a/cockatrice/src/interface/theme_icon_ink.cpp +++ b/cockatrice/src/interface/theme_icon_ink.cpp @@ -177,26 +177,50 @@ double targetLuminance(const QColor &surface, double ratio) * * Luminance is monotone in HSV value at a fixed hue and saturation, so a * bisection finds the boundary: the closest tone to the surface that still - * clears the floor, on either scheme. Returns an invalid color when even full - * value cannot reach the floor at that saturation. + * clears the floor, on either scheme. The direction depends on the surface: + * toward black (a light surface) the floor is a luminance ceiling, so the + * search keeps the highest passing value; toward white (a dark surface) it is a + * floor, so the search keeps the lowest passing value. Returns an invalid color + * when the tone cannot reach the floor at that saturation at all. */ QColor solveValue(int hue, int saturation, const QColor &surface, double floor) { + const bool towardBlack = darkerPoleWins(surface); const double target = targetLuminance(surface, floor + CONTRAST_ROUNDING_MARGIN); - if (ThemeIconInk::relativeLuminance(QColor::fromHsv(hue, saturation, 255)) < target) { + // Toward black a tone is legible at or below the target luminance, toward + // white at or above it. Testing the wrong side is what let a saturated + // violet at full value look "too dark" on a light surface and fall back to + // the unchanged secondary. + const auto passes = [hue, saturation, target, towardBlack](int value) { + const double luminance = ThemeIconInk::relativeLuminance(QColor::fromHsv(hue, saturation, value)); + return towardBlack ? luminance <= target : luminance >= target; + }; + + if (passes(255)) { + // Toward black, full value is the palest tone: if even that clears the + // floor, the whole ramp does and the surface wants the authored tone. + // Toward white it is the palest tone but also the most legible one, so + // the search still starts from the failing end. + if (towardBlack) { + return QColor::fromHsv(hue, saturation, 255).toRgb(); + } + } else if (!towardBlack) { + // Toward white the floor needs the palest tone; no darker value can + // reach it at this saturation. return {}; } - int low = 0; - int high = 255; + + int passing = towardBlack ? 0 : 255; + int failing = towardBlack ? 255 : 0; for (int search = 0; search < CONTRAST_SEARCH_STEPS; ++search) { - const int middle = (low + high) / 2; - if (ThemeIconInk::relativeLuminance(QColor::fromHsv(hue, saturation, middle)) >= target) { - high = middle; + const int middle = (passing + failing) / 2; + if (passes(middle)) { + passing = middle; } else { - low = middle; + failing = middle; } } - return QColor::fromHsv(hue, saturation, high).toRgb(); + return QColor::fromHsv(hue, saturation, passing).toRgb(); } /** diff --git a/cockatrice/src/interface/theme_icon_ink.h b/cockatrice/src/interface/theme_icon_ink.h index 760c3b8de..dfa585722 100644 --- a/cockatrice/src/interface/theme_icon_ink.h +++ b/cockatrice/src/interface/theme_icon_ink.h @@ -24,7 +24,9 @@ * on. The chroma is what makes it read as an icon rather than as leftover text; * the floor keeps a theme with a weak text color from producing a glyph that is * not legible. Neither step invents a color: both are derived from the palette - * the theme already ships. + * the theme already ships. An unauthored neutral family stops at the floor: the + * live platform palette is the only source that knows the actual scheme, so a + * chroma pinned in a palette file could belong to the other one. * * The rule only applies to the neutral label roles. A semantic tint * (`AppColor::Positive`, `QPalette::Highlight`, `QPalette::Link`) is a hue the @@ -104,7 +106,9 @@ double defaultAccentMix(); * @param accent The theme accent, normally `QPalette::Highlight`. * @param accentMix How much of the accent color to borrow. Defaults to * defaultAccentMix(); previews and tests sweep it to compare - * strengths, production callers do not pass it. + * strengths. Production passes 0 for an unauthored neutral + * family, where the live platform palette is the only source + * that knows the actual scheme. * @return The ink to resolve `currentColor` to, as an RGB color. */ QColor glyphInk(const QColor &text, const QColor &surface, const QColor &accent, double accentMix = defaultAccentMix()); diff --git a/cockatrice/src/interface/theme_manager.cpp b/cockatrice/src/interface/theme_manager.cpp index e277d9050..1ad0309b2 100644 --- a/cockatrice/src/interface/theme_manager.cpp +++ b/cockatrice/src/interface/theme_manager.cpp @@ -656,14 +656,17 @@ QColor ThemeManager::appColor(AppColor::Role role) const case AppColor::IconStrong: case AppColor::IconSoft: { // Legacy fallback for a theme written before the icon roles - // existed, or one with no authored icon family. The three levels - // are the ink rule this series shipped, applied once here so - // authored roles never get recomputed at paint time. + // existed, or one that deliberately leaves the neutral family + // unauthored (System). No accent chroma is borrowed here: the + // palette file that would carry it cannot know the platform scheme, + // so neutral ink is the live WindowText held to the label floor. + // The three levels are the ink rule this series shipped, applied + // once here so authored roles never get recomputed at paint time. const QPalette &palette = qApp->palette(); const QColor text = palette.color(QPalette::Active, QPalette::WindowText); const QColor surface = ThemeIconInk::harderSurface(palette.color(QPalette::Active, QPalette::Window), palette.color(QPalette::Active, QPalette::Button), text); - const QColor ink = ThemeIconInk::glyphInk(text, surface, accent); + const QColor ink = ThemeIconInk::glyphInk(text, surface, accent, 0.0); switch (role) { case AppColor::IconStrong: return ThemeIconInk::forEmphasis(ink, surface, IconEmphasis::Primary); diff --git a/cockatrice/themes/Fabric/palette-default-dark.toml b/cockatrice/themes/Fabric/palette-default-dark.toml index bd7945be9..419859844 100644 --- a/cockatrice/themes/Fabric/palette-default-dark.toml +++ b/cockatrice/themes/Fabric/palette-default-dark.toml @@ -79,4 +79,4 @@ PositiveStrong = #30b758 PositiveSoft = #84a98f Negative = #ed8077 NegativeStrong = #ff7367 -NegativeSoft = #be9794 \ No newline at end of file +NegativeSoft = #be9794 diff --git a/cockatrice/themes/Fabric/palette-default-light.toml b/cockatrice/themes/Fabric/palette-default-light.toml index 2b10981e7..95973457f 100644 --- a/cockatrice/themes/Fabric/palette-default-light.toml +++ b/cockatrice/themes/Fabric/palette-default-light.toml @@ -79,4 +79,4 @@ PositiveStrong = #006b22 PositiveSoft = #40634b Negative = #b0251d NegativeStrong = #ba0a00 -NegativeSoft = #7c504d \ No newline at end of file +NegativeSoft = #7c504d diff --git a/cockatrice/themes/Fusion/palette-default-dark.toml b/cockatrice/themes/Fusion/palette-default-dark.toml index b367fe12b..adc992a89 100644 --- a/cockatrice/themes/Fusion/palette-default-dark.toml +++ b/cockatrice/themes/Fusion/palette-default-dark.toml @@ -79,4 +79,4 @@ PositiveStrong = #32bf5c PositiveSoft = #8ab196 Negative = #f8867c NegativeStrong = #ff8075 -NegativeSoft = #c79e9b \ No newline at end of file +NegativeSoft = #c79e9b diff --git a/cockatrice/themes/Fusion/palette-default-light.toml b/cockatrice/themes/Fusion/palette-default-light.toml index 89439d145..5e2caa4b8 100644 --- a/cockatrice/themes/Fusion/palette-default-light.toml +++ b/cockatrice/themes/Fusion/palette-default-light.toml @@ -79,4 +79,4 @@ PositiveStrong = #007e28 PositiveSoft = #4c765a Negative = #cf2c23 NegativeStrong = #da0c00 -NegativeSoft = #935f5c \ No newline at end of file +NegativeSoft = #935f5c diff --git a/cockatrice/themes/Leather/palette-default-dark.toml b/cockatrice/themes/Leather/palette-default-dark.toml index 479463a38..6782bb7c4 100644 --- a/cockatrice/themes/Leather/palette-default-dark.toml +++ b/cockatrice/themes/Leather/palette-default-dark.toml @@ -79,4 +79,4 @@ PositiveStrong = #2dab52 PositiveSoft = #7c9f86 Negative = #df7970 NegativeStrong = #ff5e50 -NegativeSoft = #b38e8b \ No newline at end of file +NegativeSoft = #b38e8b diff --git a/cockatrice/themes/Leather/palette-default-light.toml b/cockatrice/themes/Leather/palette-default-light.toml index a2e9b8bf3..e405881cd 100644 --- a/cockatrice/themes/Leather/palette-default-light.toml +++ b/cockatrice/themes/Leather/palette-default-light.toml @@ -79,4 +79,4 @@ PositiveStrong = #006621 PositiveSoft = #3d5f48 Negative = #a7231c NegativeStrong = #b10a00 -NegativeSoft = #764c4a \ No newline at end of file +NegativeSoft = #764c4a diff --git a/cockatrice/themes/Plasma/palette-default-dark.toml b/cockatrice/themes/Plasma/palette-default-dark.toml index ce8d84a09..de36f449a 100644 --- a/cockatrice/themes/Plasma/palette-default-dark.toml +++ b/cockatrice/themes/Plasma/palette-default-dark.toml @@ -79,4 +79,4 @@ PositiveStrong = #2ba44f PositiveSoft = #779881 Negative = #d5736b NegativeStrong = #ff4f40 -NegativeSoft = #ab8885 \ No newline at end of file +NegativeSoft = #ab8885 diff --git a/cockatrice/themes/Plasma/palette-default-light.toml b/cockatrice/themes/Plasma/palette-default-light.toml index d1c973f70..b93002e25 100644 --- a/cockatrice/themes/Plasma/palette-default-light.toml +++ b/cockatrice/themes/Plasma/palette-default-light.toml @@ -79,4 +79,4 @@ PositiveStrong = #006420 PositiveSoft = #3c5d47 Negative = #a4231b NegativeStrong = #ae0900 -NegativeSoft = #744b48 \ No newline at end of file +NegativeSoft = #744b48 diff --git a/cockatrice/themes/System/palette-default-dark.toml b/cockatrice/themes/System/palette-default-dark.toml index 11f898d2a..b26bbd268 100644 --- a/cockatrice/themes/System/palette-default-dark.toml +++ b/cockatrice/themes/System/palette-default-dark.toml @@ -1,21 +1,19 @@ # The System theme derives all of its colors from the platform palette # (e.g. qt6ct's custom color scheme on Linux), so it must NOT ship palette -# overrides. Only the identity, neutral-ink and semantic app colors are -# provided here, so buttons keep their classic greens while the actual palette -# stays native to the OS. -# The neutral icon family is authored instead of derived: deriving it from the -# platform palette picked up the scheme's accent chroma, which painted neutral -# chrome blue. Neutral ink stays neutral here, black on the light scheme and -# white on the dark one, regardless of what the platform's accent carries. +# overrides. Only the identity and semantic app colors are provided here, so +# buttons keep their classic greens while the actual palette stays native to +# the OS. +# The neutral icon family is deliberately not authored: the palette file is +# picked from the theme's scheme, which on Qt < 6.5 is always light and on newer +# Qt can be unknown, so a fixed black-or-white ladder would be invisible on a +# platform palette of the other scheme. Neutral ink derives from the live +# WindowText instead. [AppColors] AccentStrong = #ff148c3c AccentSoft = #ff78c850 -Icon = #ffe0e0e0 -IconStrong = #ffffffff -IconSoft = #ffb0b0b0 Positive = #ff6dd58c PositiveStrong = #ff32c05c PositiveSoft = #ff8ab196 Negative = #ffff8a80 NegativeStrong = #ffff8378 -NegativeSoft = #ffc79e9b \ No newline at end of file +NegativeSoft = #ffc79e9b diff --git a/cockatrice/themes/System/palette-default-light.toml b/cockatrice/themes/System/palette-default-light.toml index f2eb9ab31..6d6d7ebf4 100644 --- a/cockatrice/themes/System/palette-default-light.toml +++ b/cockatrice/themes/System/palette-default-light.toml @@ -1,21 +1,19 @@ # The System theme derives all of its colors from the platform palette # (e.g. qt6ct's custom color scheme on Linux), so it must NOT ship palette -# overrides. Only the identity, neutral-ink and semantic app colors are -# provided here, so buttons keep their classic greens while the actual palette -# stays native to the OS. -# The neutral icon family is authored instead of derived: deriving it from the -# platform palette picked up the scheme's accent chroma, which painted neutral -# chrome blue. Neutral ink stays neutral here, black on the light scheme and -# white on the dark one, regardless of what the platform's accent carries. +# overrides. Only the identity and semantic app colors are provided here, so +# buttons keep their classic greens while the actual palette stays native to +# the OS. +# The neutral icon family is deliberately not authored: the palette file is +# picked from the theme's scheme, which on Qt < 6.5 is always light and on newer +# Qt can be unknown, so a fixed black-or-white ladder would be invisible on a +# platform palette of the other scheme. Neutral ink derives from the live +# WindowText instead. [AppColors] AccentStrong = #ff148c3c AccentSoft = #ff78c850 -Icon = #ff1a1a1a -IconStrong = #ff000000 -IconSoft = #ff5f5f5f Positive = #ff1a7d3a PositiveStrong = #ff007e28 PositiveSoft = #ff4c7659 Negative = #ffb3261e NegativeStrong = #ffda0b00 -NegativeSoft = #ff935f5c \ No newline at end of file +NegativeSoft = #ff935f5c diff --git a/cockatrice/themes/VelvetMarble/palette-default-dark.toml b/cockatrice/themes/VelvetMarble/palette-default-dark.toml index 69b5f3953..1fb24eb65 100644 --- a/cockatrice/themes/VelvetMarble/palette-default-dark.toml +++ b/cockatrice/themes/VelvetMarble/palette-default-dark.toml @@ -79,4 +79,4 @@ PositiveStrong = #2ba650 PositiveSoft = #789a82 Negative = #d8756c NegativeStrong = #ff5344 -NegativeSoft = #ad8a86 \ No newline at end of file +NegativeSoft = #ad8a86 diff --git a/cockatrice/themes/VelvetMarble/palette-default-light.toml b/cockatrice/themes/VelvetMarble/palette-default-light.toml index 44626ff46..1599d71fc 100644 --- a/cockatrice/themes/VelvetMarble/palette-default-light.toml +++ b/cockatrice/themes/VelvetMarble/palette-default-light.toml @@ -79,4 +79,4 @@ PositiveStrong = #006721 PositiveSoft = #3e6049 Negative = #aa241c NegativeStrong = #b40a00 -NegativeSoft = #784d4b \ No newline at end of file +NegativeSoft = #784d4b diff --git a/tests/theme_migration/theme_icon_render_test.cpp b/tests/theme_migration/theme_icon_render_test.cpp index cdf9a3c11..93203be6c 100644 --- a/tests/theme_migration/theme_icon_render_test.cpp +++ b/tests/theme_migration/theme_icon_render_test.cpp @@ -80,16 +80,18 @@ int countOpaquePixels(const QPixmap &pixmap, const QColor &expected) /** * @brief The ink the engine resolves a neutral glyph to, for @p palette. * - * Neutral glyphs are not painted with WindowText verbatim: theme_icon_ink - * borrows accent chroma and holds the result to a contrast floor. Tests that - * pin the rendered color ask the rule instead of restating it. + * Neutral glyphs are not painted with WindowText verbatim: theme_icon_ink holds + * the platform text color to a contrast floor. Tests that pin the rendered color + * ask the rule instead of restating it. No accent chroma is borrowed: this is + * the fallback an unauthored neutral family takes, so it works whatever scheme + * the platform palette actually is. */ QColor expectedNeutralInk(const QPalette &palette) { const QColor text = palette.color(QPalette::Active, QPalette::WindowText); const QColor surface = ThemeIconInk::harderSurface(palette.color(QPalette::Active, QPalette::Window), palette.color(QPalette::Active, QPalette::Button), text); - return ThemeIconInk::glyphInk(text, surface, palette.color(QPalette::Active, QPalette::Highlight)); + return ThemeIconInk::glyphInk(text, surface, palette.color(QPalette::Active, QPalette::Highlight), 0.0); } /** @@ -327,10 +329,16 @@ TEST_F(ThemeIconRenderTest, SelectedPaletteTintUsesHighlightedText) qApp->setPalette(palette); const IconTint tint(QPalette::WindowText); - // The normal ink is derived from the active text color, but it is icon ink - // rather than the text color itself. - EXPECT_NE(tint.resolve(QIcon::Normal), activeText); - EXPECT_EQ(tint.resolve(QIcon::Normal), expectedNeutralInk(qApp->palette())); + // The normal ink is the accent-free neutral fallback: the platform text + // color held to the label floor. A text color that already clears the floor + // is the ink verbatim, so this pins the rule and the floor, not a shift. + const QColor normal = tint.resolve(QIcon::Normal); + EXPECT_EQ(normal, expectedNeutralInk(qApp->palette())); + const QPalette resolved = qApp->palette(); + const QColor surface = ThemeIconInk::harderSurface(resolved.color(QPalette::Active, QPalette::Window), + resolved.color(QPalette::Active, QPalette::Button), + resolved.color(QPalette::Active, QPalette::WindowText)); + EXPECT_GE(ThemeIconInk::contrastRatio(normal, surface), ThemeIconInk::labelContrastFloor()); // A selected row paints on the highlight, so the ink is the palette's // highlighted text rather than the icon ink. EXPECT_EQ(tint.resolve(QIcon::Selected), selectedText); @@ -1194,9 +1202,9 @@ InkPalette shippedPalette(const QString &theme, bool dark) // A theme may author only [AppColors] (System does), so appColors alone do // not mean it ships a palette: the keys have to be present. The authored // neutral-family assertions therefore cover only themes with a palette; - // System's authored neutral family is checked for authorship and ordering - // by ShippedPalettesAuthorTheEmphasisLadder, and its semantic fallbacks - // against representative platform surfaces by + // System deliberately leaves its neutral family unauthored, which + // ShippedPalettesAuthorTheEmphasisLadder pins, and its semantic fallbacks + // are measured against representative platform surfaces by // DefaultSemanticFamilyMeetsTheLadderOnPlatformSurfaces. if (config.colors.isEmpty()) { return {}; @@ -1211,9 +1219,9 @@ InkPalette shippedPalette(const QString &theme, bool dark) /** * @brief The [AppColors] a shipped theme authors, even when it ships no palette. * - * System deliberately has no palette of its own, but it still authors every - * app-color role, including the neutral icon family that used to be derived - * from the platform palette. + * System deliberately has no palette of its own, but it still authors its + * semantic roles. The neutral icon family stays unauthored there so it follows + * the live platform palette; see ShippedPalettesAuthorTheEmphasisLadder. */ QMap shippedAppColors(const QString &theme, bool dark) { @@ -1257,6 +1265,29 @@ TEST(IconInk, ContrastFloorPicksThePoleItCanReach) EXPECT_GE(ThemeIconInk::contrastRatio(solved, background), 4.5); } +/** + * @brief Primary stays louder than secondary on a light surface. + * + * A saturated blue or violet at full value is already dark enough to clear the + * label floor on a light surface. When solveValue() read the target as a + * minimum in both directions it rejected that tone, walked the whole saturation + * ladder and fell back to the unchanged secondary, so primary was never + * distinguishable. #5a3ee2 on #efeff4 is the shipped Plasma accent's shape. + */ +TEST(IconInk, PrimaryRaisesAChromaticToneOnALightSurface) +{ + const QColor surface(QStringLiteral("#efeff4")); + const QColor violet(QStringLiteral("#5a3ee2")); + ASSERT_GE(ThemeIconInk::contrastRatio(violet, surface), ThemeIconInk::labelContrastFloor()) + << "the fixture must already be legible, so the fallback would be a no-op"; + + const QColor primary = ThemeIconInk::forEmphasis(violet, surface, IconEmphasis::Primary); + ASSERT_TRUE(primary.isValid()); + EXPECT_GE(ThemeIconInk::contrastRatio(primary, surface), ThemeIconInk::labelContrastFloor()); + EXPECT_GT(primary.hsvSaturation(), violet.hsvSaturation()) << "primary fell back to the unchanged secondary"; + EXPECT_NEAR(primary.hue(), violet.hue(), 2); +} + /** * @brief A green or red accent must not tint neutral chrome. * @@ -1675,30 +1706,17 @@ TEST(IconInk, ShippedPalettesAuthorTheEmphasisLadder) EXPECT_GT(checked, 0) << "no shipped palette was loadable"; // The System theme ships no palette of its own, so its families have no - // authored surfaces to measure against. It still authors the neutral family - // instead of deriving it: deriving picked up the platform accent's chroma, - // which painted neutral chrome blue on a default Linux scheme. + // authored surfaces to measure against. It nevertheless authors the + // semantic family, while the neutral family stays unauthored on purpose: + // the palette file is chosen from the theme's scheme, which is always light + // on Qt < 6.5 and can be unknown on newer Qt, so one fixed ladder would be + // invisible against a platform palette of the other scheme. for (const bool dark : {false, true}) { const QMap system = shippedAppColors(QStringLiteral("System"), dark); EXPECT_TRUE(system.contains(AppColor::PositiveStrong)); EXPECT_TRUE(system.contains(AppColor::NegativeSoft)); for (const AppColor::Role role : {AppColor::Icon, AppColor::IconStrong, AppColor::IconSoft}) { - ASSERT_TRUE(system.contains(role)) << (dark ? "dark" : "light") << static_cast(role); - EXPECT_EQ(system.value(role).hsvSaturation(), 0) << (dark ? "dark" : "light") << static_cast(role); - } - - // Strong is the loudest level a neutral family can reach: the darkest - // tone on the light scheme, the palest on the dark one. Quiet recedes - // toward the surface on both. - const double secondary = ThemeIconInk::relativeLuminance(system.value(AppColor::Icon)); - const double primary = ThemeIconInk::relativeLuminance(system.value(AppColor::IconStrong)); - const double quiet = ThemeIconInk::relativeLuminance(system.value(AppColor::IconSoft)); - if (dark) { - EXPECT_GT(primary, secondary) << "dark"; - EXPECT_GT(secondary, quiet) << "dark"; - } else { - EXPECT_LT(primary, secondary) << "light"; - EXPECT_LT(secondary, quiet) << "light"; + EXPECT_FALSE(system.contains(role)) << (dark ? "dark" : "light") << static_cast(role); } } }