diff --git a/cockatrice/src/interface/theme_config.cpp b/cockatrice/src/interface/theme_config.cpp index 1a1c2e098..6abf63157 100644 --- a/cockatrice/src/interface/theme_config.cpp +++ b/cockatrice/src/interface/theme_config.cpp @@ -459,8 +459,8 @@ QColor defaultAppColor(AppColor::Role role, bool dark) case AppColor::AccentStrong: case AppColor::AccentSoft: // The neutral icon family is palette-derived like the accent, so a - // theme without a palette of its own (System) keeps following the - // platform instead of pinning one scheme's ink. + // theme without an authored icon family keeps following the platform + // instead of pinning one scheme's ink. case AppColor::Icon: case AppColor::IconStrong: case AppColor::IconSoft: diff --git a/cockatrice/src/interface/theme_config.h b/cockatrice/src/interface/theme_config.h index 4d2cb74ee..fb052ad1e 100644 --- a/cockatrice/src/interface/theme_config.h +++ b/cockatrice/src/interface/theme_config.h @@ -65,9 +65,9 @@ Q_ENUM_NS(Role) * ThemeManager derives them from the palette: the accent from the highlight, * the icon ink from the neutral text color plus the highlight. Semantic roles * return an opaque default that suits the scheme, so themes written before the - * role existed still render it. A theme with no palette of its own (the System - * theme) keeps deriving its icon ink from the platform palette instead of - * pinning one scheme's colors. + * 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. */ QColor defaultAppColor(AppColor::Role role, bool dark); diff --git a/cockatrice/src/interface/theme_manager.cpp b/cockatrice/src/interface/theme_manager.cpp index 90e6f8e8c..e277d9050 100644 --- a/cockatrice/src/interface/theme_manager.cpp +++ b/cockatrice/src/interface/theme_manager.cpp @@ -656,8 +656,8 @@ 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 palette of its own (System). The three - // levels are the ink rule this series shipped, applied once here so + // 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. const QPalette &palette = qApp->palette(); const QColor text = palette.color(QPalette::Active, QPalette::WindowText); diff --git a/cockatrice/themes/System/palette-default-dark.toml b/cockatrice/themes/System/palette-default-dark.toml index 18f834b52..11f898d2a 100644 --- a/cockatrice/themes/System/palette-default-dark.toml +++ b/cockatrice/themes/System/palette-default-dark.toml @@ -1,13 +1,18 @@ # 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 and semantic app colors are provided here, so -# buttons keep their classic greens while the actual palette stays native to -# the OS. -# Icon, IconStrong and IconSoft are deliberately absent: a theme with no -# palette of its own derives its icon ink from the platform 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. [AppColors] AccentStrong = #ff148c3c AccentSoft = #ff78c850 +Icon = #ffe0e0e0 +IconStrong = #ffffffff +IconSoft = #ffb0b0b0 Positive = #ff6dd58c PositiveStrong = #ff32c05c PositiveSoft = #ff8ab196 diff --git a/cockatrice/themes/System/palette-default-light.toml b/cockatrice/themes/System/palette-default-light.toml index 374ae87c8..f2eb9ab31 100644 --- a/cockatrice/themes/System/palette-default-light.toml +++ b/cockatrice/themes/System/palette-default-light.toml @@ -1,13 +1,18 @@ # 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 and semantic app colors are provided here, so -# buttons keep their classic greens while the actual palette stays native to -# the OS. -# Icon, IconStrong and IconSoft are deliberately absent: a theme with no -# palette of its own derives its icon ink from the platform 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. [AppColors] AccentStrong = #ff148c3c AccentSoft = #ff78c850 +Icon = #ff1a1a1a +IconStrong = #ff000000 +IconSoft = #ff5f5f5f Positive = #ff1a7d3a PositiveStrong = #ff007e28 PositiveSoft = #ff4c7659 diff --git a/tests/theme_migration/theme_icon_render_test.cpp b/tests/theme_migration/theme_icon_render_test.cpp index 4547a4e2a..cdf9a3c11 100644 --- a/tests/theme_migration/theme_icon_render_test.cpp +++ b/tests/theme_migration/theme_icon_render_test.cpp @@ -1194,8 +1194,10 @@ 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 fixed semantic family is checked against representative - // platform surfaces by DefaultSemanticFamilyMeetsTheLadderOnPlatformSurfaces. + // System's authored neutral family is checked for authorship and ordering + // by ShippedPalettesAuthorTheEmphasisLadder, and its semantic fallbacks + // against representative platform surfaces by + // DefaultSemanticFamilyMeetsTheLadderOnPlatformSurfaces. if (config.colors.isEmpty()) { return {}; } @@ -1209,8 +1211,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 its - * semantic roles; the icon roles stay unset there so they follow the platform. + * 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. */ QMap shippedAppColors(const QString &theme, bool dark) { @@ -1627,7 +1630,7 @@ TEST(IconInk, ShippedPalettesAuthorTheEmphasisLadder) for (const bool dark : {false, true}) { const InkPalette palette = shippedPalette(theme, dark); if (!palette.text.isValid()) { - continue; // System ships no palette and derives its icon ink. + continue; // System ships no palette, so it has no surfaces to measure. } ++checked; @@ -1671,14 +1674,33 @@ TEST(IconInk, ShippedPalettesAuthorTheEmphasisLadder) } EXPECT_GT(checked, 0) << "no shipped palette was loadable"; - // The System theme is the deliberate exception: it authors its semantic - // levels but leaves the neutral family to the platform palette. - const QMap system = shippedAppColors(QStringLiteral("System"), false); - EXPECT_TRUE(system.contains(AppColor::PositiveStrong)); - EXPECT_TRUE(system.contains(AppColor::NegativeSoft)); - EXPECT_FALSE(system.contains(AppColor::Icon)); - EXPECT_FALSE(system.contains(AppColor::IconStrong)); - EXPECT_FALSE(system.contains(AppColor::IconSoft)); + // 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. + 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"; + } + } } /** @@ -1759,11 +1781,11 @@ TEST(IconInk, DisabledStaysBelowQuietOnEveryShippedTheme) /** * @brief The System fallback family keeps the ladder on a platform palette. * - * System authors no neutral roles and follows the OS surfaces, so its semantic - * family is the one shipped ladder that cannot be measured against its own - * window and button. These are the representative extremes of a platform - * palette, and the fixed values have to clear the same floor there that the - * five authored themes clear on their own surfaces. + * System authors no palette and follows the OS surfaces, so its families + * cannot be measured against its own window and button. These are the + * representative extremes of a platform palette, and the fixed values have to + * clear the same floor there that the five authored themes clear on their own + * surfaces. */ TEST(IconInk, DefaultSemanticFamilyMeetsTheLadderOnPlatformSurfaces) { diff --git a/tests/theme_migration/theme_migration_test.cpp b/tests/theme_migration/theme_migration_test.cpp index deff913f6..ebc6fd47d 100644 --- a/tests/theme_migration/theme_migration_test.cpp +++ b/tests/theme_migration/theme_migration_test.cpp @@ -163,9 +163,9 @@ TEST(ThemeAppColorTest, SemanticDefaultsFollowTheScheme) EXPECT_EQ(defaultAppColor(AppColor::NegativeStrong, true), QColor(QStringLiteral("#ff8378"))); EXPECT_EQ(defaultAppColor(AppColor::NegativeSoft, false), QColor(QStringLiteral("#935f5c"))); EXPECT_EQ(defaultAppColor(AppColor::NegativeSoft, true), QColor(QStringLiteral("#c79e9b"))); - // Accent and neutral icon roles are derived from the palette, so they have - // no static default. A theme with no palette (System) keeps following the - // platform instead of pinning one scheme's ink. + // Accent and neutral icon roles have no static default: an older theme + // written before the roles existed falls back to deriving them from its + // palette rather than pinning one scheme's ink. for (const bool dark : {false, true}) { EXPECT_FALSE(defaultAppColor(AppColor::AccentStrong, dark).isValid()); EXPECT_FALSE(defaultAppColor(AppColor::AccentSoft, dark).isValid());