[Icons] Author neutral System icon ink

The System theme derived icon ink from the platform palette, which rendered blue under the default palette. Author Icon/IconStrong/IconSoft explicitly: a black ladder in light mode, a white ladder in dark mode. Pin authorship, achromaticity and ordering in the render test.

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:

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);
 

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);

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

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

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<AppColor::Role, QColor> 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<AppColor::Role, QColor> 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<AppColor::Role, QColor> 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<int>(role);
+            EXPECT_EQ(system.value(role).hsvSaturation(), 0) << (dark ? "dark" : "light") << static_cast<int>(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)
 {

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());