From ed8e4de3aebe4440cd067552830f6e9bc66fcc3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= <44814898+BruebachL@users.noreply.github.com> Date: Sat, 7 Nov 2026 09:39:50 +0100 Subject: [PATCH] [Icons] Keep the System semantic ladder on the text floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit System authors no palette of its own, so its semantic family is fixed data measured against whatever surfaces the platform provides. The ladder that landed with the authored roles reused the old contrast-ordered values: PositiveSoft and NegativeSoft sat at 2.76–3.90:1 on common light and dark platform surfaces, and the strong level was less saturated than the secondary, which is the inversion the chroma rule removes. Re-author the six default semantic colors per scheme at the same 4.60 target the shipped themes use, against representative platform surfaces, and mirror them in System's palette files. The new DefaultSemanticFamilyMeetsTheLadderOnPlatformSurfaces test holds the floor and the saturation order there, and shippedPalette's early return now says why System is outside the authored-palette assertions. --- cockatrice/src/interface/theme_config.cpp | 14 ++++-- cockatrice/src/interface/theme_icon.h | 9 ++-- .../themes/System/palette-default-dark.toml | 8 ++-- .../themes/System/palette-default-light.toml | 10 ++-- .../theme_icon_render_test.cpp | 47 ++++++++++++++++++- .../theme_migration/theme_migration_test.cpp | 18 +++---- 6 files changed, 78 insertions(+), 28 deletions(-) diff --git a/cockatrice/src/interface/theme_config.cpp b/cockatrice/src/interface/theme_config.cpp index f5a3cff7e..1a1c2e098 100644 --- a/cockatrice/src/interface/theme_config.cpp +++ b/cockatrice/src/interface/theme_config.cpp @@ -465,18 +465,22 @@ QColor defaultAppColor(AppColor::Role role, bool dark) 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. case AppColor::Positive: - return QColor(dark ? QStringLiteral("#6dd58c") : QStringLiteral("#1b7f3b")); + return QColor(dark ? QStringLiteral("#6dd58c") : QStringLiteral("#1a7d3a")); case AppColor::PositiveStrong: - return QColor(dark ? QStringLiteral("#99e2ae") : QStringLiteral("#104a22")); + return QColor(dark ? QStringLiteral("#32c05c") : QStringLiteral("#007e28")); case AppColor::PositiveSoft: - return QColor(dark ? QStringLiteral("#5e8873") : QStringLiteral("#5d7f6f")); + return QColor(dark ? QStringLiteral("#8ab196") : QStringLiteral("#4c7659")); case AppColor::Negative: return QColor(dark ? QStringLiteral("#ff8a80") : QStringLiteral("#b3261e")); case AppColor::NegativeStrong: - return QColor(dark ? QStringLiteral("#ffb3ac") : QStringLiteral("#7c1b15")); + return QColor(dark ? QStringLiteral("#ff8378") : QStringLiteral("#da0b00")); case AppColor::NegativeSoft: - return QColor(dark ? QStringLiteral("#9b7678") : QStringLiteral("#9a6c6f")); + return QColor(dark ? QStringLiteral("#c79e9b") : QStringLiteral("#935f5c")); } return {}; diff --git a/cockatrice/src/interface/theme_icon.h b/cockatrice/src/interface/theme_icon.h index 1bca9fd7f..8f33b1f67 100644 --- a/cockatrice/src/interface/theme_icon.h +++ b/cockatrice/src/interface/theme_icon.h @@ -84,7 +84,7 @@ public: IconTint(QPalette::ColorRole role, IconEmphasis emphasis = IconEmphasis::Secondary); // implicit for role callers IconTint(AppColor::Role role, IconEmphasis emphasis = IconEmphasis::Secondary); // implicit for semantic callers - /** @brief A neutral glyph in the accent tone, for the surface's primary action. */ + /** @brief A neutral glyph at the family's primary level, for the surface's primary action. */ static IconTint primary(); /** @brief A dimmed neutral glyph, for a control that labels rather than acts. */ @@ -93,9 +93,10 @@ public: /** * @brief Resolves the concrete color for an icon mode. * - * A disabled application color resolves to the disabled foreground, so an - * unavailable control does not keep its semantic color. A selected palette - * color keeps the WindowText -> HighlightedText swap. + * A disabled application color resolves to the derived disabled ink, the + * quiet level receded toward the surface, so an unavailable control does not + * keep its semantic color. A selected palette color keeps the + * WindowText -> HighlightedText swap. * * Application colors are returned verbatim: the theme author stores one * value per emphasis level, so the paint path does not rewrite it. The diff --git a/cockatrice/themes/System/palette-default-dark.toml b/cockatrice/themes/System/palette-default-dark.toml index ae821b739..18f834b52 100644 --- a/cockatrice/themes/System/palette-default-dark.toml +++ b/cockatrice/themes/System/palette-default-dark.toml @@ -9,8 +9,8 @@ AccentStrong = #ff148c3c AccentSoft = #ff78c850 Positive = #ff6dd58c -PositiveStrong = #ff99e2ae -PositiveSoft = #ff5e8873 +PositiveStrong = #ff32c05c +PositiveSoft = #ff8ab196 Negative = #ffff8a80 -NegativeStrong = #ffffb3ac -NegativeSoft = #ff9b7678 +NegativeStrong = #ffff8378 +NegativeSoft = #ffc79e9b \ No newline at end of file diff --git a/cockatrice/themes/System/palette-default-light.toml b/cockatrice/themes/System/palette-default-light.toml index da4dac445..374ae87c8 100644 --- a/cockatrice/themes/System/palette-default-light.toml +++ b/cockatrice/themes/System/palette-default-light.toml @@ -8,9 +8,9 @@ [AppColors] AccentStrong = #ff148c3c AccentSoft = #ff78c850 -Positive = #ff1b7f3b -PositiveStrong = #ff104a22 -PositiveSoft = #ff5d7f6f +Positive = #ff1a7d3a +PositiveStrong = #ff007e28 +PositiveSoft = #ff4c7659 Negative = #ffb3261e -NegativeStrong = #ff7c1b15 -NegativeSoft = #ff9a6c6f +NegativeStrong = #ffda0b00 +NegativeSoft = #ff935f5c \ No newline at end of file diff --git a/tests/theme_migration/theme_icon_render_test.cpp b/tests/theme_migration/theme_icon_render_test.cpp index c157ecf05..299ebd717 100644 --- a/tests/theme_migration/theme_icon_render_test.cpp +++ b/tests/theme_migration/theme_icon_render_test.cpp @@ -1196,7 +1196,10 @@ InkPalette shippedPalette(const QString &theme, bool dark) .arg(theme, dark ? QStringLiteral("dark") : QStringLiteral("light"))); const PaletteConfig config = PaletteConfig::fromFile(file); // A theme may author only [AppColors] (System does), so appColors alone do - // not mean it ships a palette: the keys have to be present. + // 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. if (config.colors.isEmpty()) { return {}; } @@ -1760,6 +1763,48 @@ TEST(IconInk, DisabledStaysBelowQuietOnEveryShippedTheme) EXPECT_GT(checked, 0) << "no shipped palette was loadable"; } +/** + * @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. + */ +TEST(IconInk, DefaultSemanticFamilyMeetsTheLadderOnPlatformSurfaces) +{ + struct Scheme + { + bool dark; + QColor window; + QColor button; + }; + const QList schemes = { + {false, QColor(QStringLiteral("#f0f0f0")), QColor(QStringLiteral("#f5f5f5"))}, + {true, QColor(QStringLiteral("#2b2b2b")), QColor(QStringLiteral("#3c3c3c"))}, + }; + + for (const Scheme &scheme : schemes) { + for (const auto &[base, strong, soft] : { + std::tuple{AppColor::Positive, AppColor::PositiveStrong, AppColor::PositiveSoft}, + std::tuple{AppColor::Negative, AppColor::NegativeStrong, AppColor::NegativeSoft}}) { + const QColor secondary = defaultAppColor(base, scheme.dark); + const QColor primary = defaultAppColor(strong, scheme.dark); + const QColor quiet = defaultAppColor(soft, scheme.dark); + ASSERT_TRUE(secondary.isValid() && primary.isValid() && quiet.isValid()); + + for (const QColor &surface : {scheme.window, scheme.button}) { + EXPECT_GE(ThemeIconInk::contrastRatio(secondary, surface), ThemeIconInk::labelContrastFloor()); + EXPECT_GE(ThemeIconInk::contrastRatio(primary, surface), ThemeIconInk::labelContrastFloor()); + EXPECT_GE(ThemeIconInk::contrastRatio(quiet, surface), ThemeIconInk::labelContrastFloor()); + } + EXPECT_LT(quiet.hsvSaturation(), secondary.hsvSaturation()); + EXPECT_GT(primary.hsvSaturation(), secondary.hsvSaturation()); + } + } +} + /** * @brief Every emphasis keeps the selected-row swap to HighlightedText. */ diff --git a/tests/theme_migration/theme_migration_test.cpp b/tests/theme_migration/theme_migration_test.cpp index 0e55e966d..deff913f6 100644 --- a/tests/theme_migration/theme_migration_test.cpp +++ b/tests/theme_migration/theme_migration_test.cpp @@ -151,18 +151,18 @@ TEST_F(ThemeMigrationTest, IsIdempotent) TEST(ThemeAppColorTest, SemanticDefaultsFollowTheScheme) { - EXPECT_EQ(defaultAppColor(AppColor::Positive, false), QColor(QStringLiteral("#1b7f3b"))); + EXPECT_EQ(defaultAppColor(AppColor::Positive, false), QColor(QStringLiteral("#1a7d3a"))); EXPECT_EQ(defaultAppColor(AppColor::Positive, true), QColor(QStringLiteral("#6dd58c"))); - EXPECT_EQ(defaultAppColor(AppColor::PositiveStrong, false), QColor(QStringLiteral("#104a22"))); - EXPECT_EQ(defaultAppColor(AppColor::PositiveStrong, true), QColor(QStringLiteral("#99e2ae"))); - EXPECT_EQ(defaultAppColor(AppColor::PositiveSoft, false), QColor(QStringLiteral("#5d7f6f"))); - EXPECT_EQ(defaultAppColor(AppColor::PositiveSoft, true), QColor(QStringLiteral("#5e8873"))); + EXPECT_EQ(defaultAppColor(AppColor::PositiveStrong, false), QColor(QStringLiteral("#007e28"))); + EXPECT_EQ(defaultAppColor(AppColor::PositiveStrong, true), QColor(QStringLiteral("#32c05c"))); + EXPECT_EQ(defaultAppColor(AppColor::PositiveSoft, false), QColor(QStringLiteral("#4c7659"))); + EXPECT_EQ(defaultAppColor(AppColor::PositiveSoft, true), QColor(QStringLiteral("#8ab196"))); EXPECT_EQ(defaultAppColor(AppColor::Negative, false), QColor(QStringLiteral("#b3261e"))); EXPECT_EQ(defaultAppColor(AppColor::Negative, true), QColor(QStringLiteral("#ff8a80"))); - EXPECT_EQ(defaultAppColor(AppColor::NegativeStrong, false), QColor(QStringLiteral("#7c1b15"))); - EXPECT_EQ(defaultAppColor(AppColor::NegativeStrong, true), QColor(QStringLiteral("#ffb3ac"))); - EXPECT_EQ(defaultAppColor(AppColor::NegativeSoft, false), QColor(QStringLiteral("#9a6c6f"))); - EXPECT_EQ(defaultAppColor(AppColor::NegativeSoft, true), QColor(QStringLiteral("#9b7678"))); + EXPECT_EQ(defaultAppColor(AppColor::NegativeStrong, false), QColor(QStringLiteral("#da0b00"))); + 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.