[Icons] Add palette-aware themed icon engine

A QPixmap snapshot cannot follow palette changes, so expose a QIcon whose
engine resolves the `currentColor` sentinel from the live palette on every
pixmap request. Icons stay legible across light/dark switches without
recreating actions or buttons.

- Resolve the asset, its bytes and its intrinsic size once, and drop them on
  ThemeManager::themeChanged/paletteChanged. A repaint does no file I/O and
  no theme.cfg parse; the palette color stays part of the render cache key.
- Centre the glyph on QRectF(rect).center(): QRect::center() floors and uses
  right = left + width - 1, which shifted every 16x16 item-view glyph up and
  left by a logical pixel and clipped its last row and column.
- Take a raster override's intrinsic size from QImageReader instead of the
  24x24 SVG default, so a non-square override reports the box it draws.
- Set the device pixel ratio before inserting into the render cache, so a
  cache hit no longer detaches the pixmap on every paint.

cockatrice/src/interface/theme_icon.cpp

@@ -10,6 +10,8 @@
 #include <QHash>
 #include <QIODevice>
 #include <QIconEngine>
+#include <QImageReader>
+#include <QObject>
 #include <QPaintDevice>
 #include <QPainter>
 #include <QPointF>
@@ -78,6 +80,11 @@ QPixmap renderSvg(const QByteArray &data, const QSize &size, qreal devicePixelRa
     return pixmap;
 }
 
+bool hasSentinel(const QByteArray &svg)
+{
+    return !svg.isEmpty() && QString::fromUtf8(svg).contains(QStringLiteral("currentColor"), Qt::CaseInsensitive);
+}
+
 QString cacheKey(const QString &path, const QByteArray &color, const QSize &size, qreal devicePixelRatio)
 {
     return path + QLatin1Char('|') + QString::fromLatin1(color) + QLatin1Char('|') + QString::number(size.width()) +
@@ -88,15 +95,25 @@ QString cacheKey(const QString &path, const QByteArray &color, const QSize &size
  * @brief QIcon engine rendering a `theme:` SVG with `currentColor` resolved on demand.
  *
  * Qt's SVG renderer has no ambient color, so the sentinel is substituted from
- * the current palette on every pixmap request. The render cache is keyed by
- * resolved path, palette color, pixel size and DPR, so a theme change produces
- * new keys instead of reusing art built under the previous palette.
+ * the current palette on every pixmap request. Asset resolution, the file bytes
+ * and the intrinsic size are cached until the theme changes: a repaint then
+ * does no file I/O at all. The render cache is keyed by resolved path, palette
+ * color, pixel size and DPR, so a palette change produces new keys instead of
+ * reusing art built under the previous palette.
  */
-class ThemeSvgIconEngine final : public QIconEngine
+class ThemeSvgIconEngine final : public QObject, public QIconEngine
 {
 public:
     ThemeSvgIconEngine(QString _prefix, QPalette::ColorRole _role) : prefix(std::move(_prefix)), role(_role)
     {
+        if (themeManager) {
+            // A theme switch changes the resolved asset (scheme-qualified
+            // variants, raster overrides) and a palette change invalidates
+            // every cached colour, so both drop the caches instead of
+            // re-reading theme.cfg and the assets on every repaint.
+            connect(themeManager, &ThemeManager::themeChanged, this, &ThemeSvgIconEngine::invalidate);
+            connect(themeManager, &ThemeManager::paletteChanged, this, &ThemeSvgIconEngine::invalidate);
+        }
     }
 
     QIconEngine *clone() const override
@@ -113,12 +130,14 @@ public:
         }
         const QSize pixelSize(qRound(logicalSize.width() * devicePixelRatio),
                               qRound(logicalSize.height() * devicePixelRatio));
-        QPixmap pixmap = render(pixelSize, devicePixelRatio, mode);
+        const QPixmap pixmap = render(pixelSize, devicePixelRatio, mode);
         if (!pixmap.isNull()) {
-            pixmap.setDevicePixelRatio(devicePixelRatio);
+            // QRect::center() would floor and drop the last row/column of a
+            // 16x16 glyph; the center has to be fractional so glyphs stay
+            // unshifted and unsclipped in item views.
             const QSizeF contentSize = pixmap.deviceIndependentSize();
             const QRectF targetRect(
-                QPointF(rect.center()) - QPointF(contentSize.width() / 2.0, contentSize.height() / 2.0), contentSize);
+                QRectF(rect).center() - QPointF(contentSize.width() / 2.0, contentSize.height() / 2.0), contentSize);
             painter->drawPixmap(targetRect, pixmap, QRectF(pixmap.rect()));
         }
     }
@@ -137,11 +156,7 @@ public:
         // Qt < 6.8 passes the device-dependent size to the engine.
         const QSize pixelSize = logicalSize;
 #endif
-        QPixmap pixmap = render(pixelSize, scale, mode);
-        if (!pixmap.isNull()) {
-            pixmap.setDevicePixelRatio(scale);
-        }
-        return pixmap;
+        return render(pixelSize, scale, mode);
     }
 
     QSize actualSize(const QSize &size, QIcon::Mode, QIcon::State) override
@@ -150,26 +165,41 @@ public:
     }
 
 private:
+    void invalidate()
+    {
+        hasResolvedAsset = false;
+        hasIntrinsicSize = false;
+        cache.clear();
+    }
+
     void ensureResolved() const
     {
-        const QString themePath = themeManager->getCurrentThemePath();
-        const bool darkMode = themeManager->isDarkModeActive();
-        if (!hasResolvedPath || themePath != resolvedThemePath || darkMode != resolvedDarkMode) {
-            resolvedPath = themeManager->assetPath(prefix);
-            resolvedThemePath = themePath;
-            resolvedDarkMode = darkMode;
-            hasResolvedPath = true;
-            hasIntrinsicSize = false;
+        if (hasResolvedAsset && (!themeManager || themeManager->getCurrentThemePath() == resolvedThemePath)) {
+            return;
         }
+        resolvedThemePath = themeManager ? themeManager->getCurrentThemePath() : QString();
+        resolvedPath = themeManager ? themeManager->assetPath(prefix) : QString();
+        resolvedAssetIsSvg = resolvedPath.endsWith(QStringLiteral(".svg"), Qt::CaseInsensitive);
+        resolvedSvg = resolvedAssetIsSvg ? readSvg(resolvedPath) : QByteArray();
+        resolvedHasSentinel = hasSentinel(resolvedSvg);
+        hasResolvedAsset = true;
+        hasIntrinsicSize = false;
     }
 
     QSize intrinsicSize() const
     {
         ensureResolved();
         if (!hasIntrinsicSize) {
-            const QByteArray svg = readSvg(resolvedPath);
-            QSvgRenderer renderer(svg);
-            intrinsicSizeCache = renderer.isValid() ? renderer.defaultSize() : QSize(24, 24);
+            if (resolvedAssetIsSvg) {
+                QSvgRenderer renderer(resolvedSvg);
+                intrinsicSizeCache = renderer.isValid() ? renderer.defaultSize() : QSize();
+            } else {
+                // A theme may override a glyph with a raster; take its real
+                // aspect ratio instead of assuming the 24x24 SVG default, so
+                // actualSize() and the rendered pixmap agree.
+                QImageReader reader(QStringLiteral("theme:") + resolvedPath);
+                intrinsicSizeCache = reader.size();
+            }
             if (intrinsicSizeCache.isEmpty()) {
                 intrinsicSizeCache = QSize(24, 24);
             }
@@ -186,35 +216,44 @@ private:
         return intrinsicSize().scaled(size, Qt::KeepAspectRatio);
     }
 
+    QPixmap fromAsset(const QSize &pixelSize, QIcon::Mode mode) const
+    {
+        QPixmap pixmap(QStringLiteral("theme:") + resolvedPath);
+        if (!pixelSize.isEmpty() && pixmap.size() != pixelSize) {
+            pixmap = pixmap.scaled(pixelSize, Qt::KeepAspectRatio, Qt::SmoothTransformation);
+        }
+        if (mode == QIcon::Disabled && !pixmap.isNull()) {
+            pixmap = QApplication::style()->generatedIconPixmap(QIcon::Disabled, pixmap, nullptr);
+        }
+        return pixmap;
+    }
+
     QPixmap render(const QSize &pixelSize, qreal devicePixelRatio, QIcon::Mode mode) const
     {
         ensureResolved();
-        const QString themePath = resolvedThemePath;
-        const bool darkMode = resolvedDarkMode;
-
-        const QByteArray svg = readSvg(resolvedPath);
-        if (svg.isEmpty() || !QString::fromUtf8(svg).contains(QStringLiteral("currentColor"), Qt::CaseInsensitive)) {
+        if (!resolvedHasSentinel) {
             // Authored art carries its own colors: use Qt's normal loader so
             // raster overrides and non-SVG assets keep working.
-            QPixmap pixmap(QStringLiteral("theme:") + resolvedPath);
-            if (!pixelSize.isEmpty() && pixmap.size() != pixelSize) {
-                pixmap = pixmap.scaled(pixelSize, Qt::KeepAspectRatio, Qt::SmoothTransformation);
-            }
-            if (mode == QIcon::Disabled && !pixmap.isNull()) {
-                pixmap = QApplication::style()->generatedIconPixmap(QIcon::Disabled, pixmap, nullptr);
+            QPixmap pixmap = fromAsset(pixelSize, mode);
+            if (!pixmap.isNull()) {
+                pixmap.setDevicePixelRatio(devicePixelRatio);
             }
             return pixmap;
         }
 
         const QByteArray color = sentinelColor(mode, role);
-        const QString key = cacheKey(resolvedPath, color, pixelSize, devicePixelRatio) + QLatin1Char('|') + themePath +
-                            (darkMode ? QLatin1Char('d') : QLatin1Char('l'));
+        const QString key = cacheKey(resolvedPath, color, pixelSize, devicePixelRatio);
         const auto cached = cache.constFind(key);
         if (cached != cache.constEnd()) {
             return cached.value();
         }
 
-        QPixmap pixmap = renderSvg(resolveSvgColorSentinel(svg, color), pixelSize, devicePixelRatio);
+        QPixmap pixmap = renderSvg(resolveSvgColorSentinel(resolvedSvg, color), pixelSize, devicePixelRatio);
+        if (!pixmap.isNull()) {
+            // Set the ratio before caching: assigning it to a cached pixmap
+            // afterwards detaches (deep-copies) it on every paint.
+            pixmap.setDevicePixelRatio(devicePixelRatio);
+        }
         if (cache.size() >= 64) {
             cache.clear();
         }
@@ -227,9 +266,11 @@ private:
     mutable QHash<QString, QPixmap> cache;
     mutable QString resolvedPath;
     mutable QString resolvedThemePath;
+    mutable QByteArray resolvedSvg;
     mutable QSize intrinsicSizeCache;
-    mutable bool resolvedDarkMode = false;
-    mutable bool hasResolvedPath = false;
+    mutable bool resolvedAssetIsSvg = false;
+    mutable bool resolvedHasSentinel = false;
+    mutable bool hasResolvedAsset = false;
     mutable bool hasIntrinsicSize = false;
 };
 } // namespace
@@ -246,7 +287,7 @@ QPixmap loadSentinelSvgPixmap(const QString &resolvedPath, QPalette::ColorRole r
     }
 
     const QByteArray svg = readSvg(resolvedPath);
-    if (svg.isEmpty() || !QString::fromUtf8(svg).contains(QStringLiteral("currentColor"), Qt::CaseInsensitive)) {
+    if (!hasSentinel(svg)) {
         return {};
     }
 

libcockatrice_utility/libcockatrice/utility/svg_recolor.cpp

@@ -1,15 +1,41 @@
 #include "svg_recolor.h"
 
-#include <QString>
+#include <QByteArrayView>
+
+namespace
+{
+constexpr QByteArrayView SENTINEL = "currentColor";
+
+/// Case-insensitive byte search for the sentinel, starting at @p from.
+/// QByteArray::indexOf() only matches case-sensitively for QByteArrayView, and
+/// its char overload cannot match a word.
+qsizetype findSentinel(const QByteArray &bytes, qsizetype from)
+{
+    for (qsizetype i = from; i + SENTINEL.size() <= bytes.size(); ++i) {
+        if (bytes.sliced(i, SENTINEL.size()).compare(SENTINEL, Qt::CaseInsensitive) == 0) {
+            return i;
+        }
+    }
+    return -1;
+}
+} // namespace
 
 QByteArray resolveSvgColorSentinel(const QByteArray &svg, const QByteArray &color)
 {
-    const QString source = QString::fromUtf8(svg);
-    if (!source.contains(QStringLiteral("currentColor"), Qt::CaseInsensitive)) {
+    // The sentinel and the replacement are both ASCII, so stay in bytes: a
+    // QString round trip would decode the whole document (and mangle bytes that
+    // are not valid UTF-8) even on the no-op path.
+    qsizetype pos = findSentinel(svg, 0);
+    if (pos < 0) {
         return svg;
     }
 
-    QString resolved = source;
-    resolved.replace(QStringLiteral("currentColor"), QString::fromLatin1(color), Qt::CaseInsensitive);
-    return resolved.toUtf8();
+    QByteArray resolved = svg;
+    while (pos >= 0) {
+        resolved.replace(pos, SENTINEL.size(), color);
+        // Skip past the inserted color: a caller's color never contains the
+        // sentinel, and this keeps the scan linear.
+        pos = findSentinel(resolved, pos + color.size());
+    }
+    return resolved;
 }

libcockatrice_utility/libcockatrice/utility/svg_recolor.h

@@ -16,8 +16,13 @@
  * (for example a QPalette role serialized to #RRGGBB) and receive SVG bytes
  * that are safe to hand to QSvgRenderer.
  *
- * @param svg UTF-8 SVG source. Every `currentColor` occurrence, in any case,
- *            is replaced.
+ * Matching is a plain, case-insensitive substring search, not a CSS parse: an
+ * identifier that merely starts with the sentinel (for example
+ * `id="currentColorStop"`) is rewritten too. That is acceptable for the assets
+ * this project authors, and keeps the resolver dependency-free.
+ *
+ * @param svg SVG source bytes, expected to be UTF-8. Invalid UTF-8 is preserved
+ *            byte-for-byte, since the resolver never decodes the document.
  * @param color Concrete SVG color (for example "#ff00ff"), as ASCII bytes.
  * @return The recolored SVG source; the input is left untouched.
  */

tests/svg_recolor_test.cpp

@@ -43,6 +43,19 @@ TEST(SvgRecolor, DoesNotModifyTheInput)
     EXPECT_EQ(svg, original);
 }
 
+TEST(SvgRecolor, PreservesBytesThatAreNotValidUtf8)
+{
+    QByteArray svg = "<svg>";
+    svg.append("\xff\xfe");
+    svg.append(R"(<path fill="currentColor"/></svg>)");
+
+    const QByteArray result = resolveSvgColorSentinel(svg, "#ff8800");
+
+    EXPECT_TRUE(result.contains("\xff\xfe"));
+    EXPECT_FALSE(result.contains("currentColor"));
+    EXPECT_EQ(result.count("#ff8800"), 1);
+}
+
 TEST(SvgRecolor, NoSentinelIsANoOp)
 {
     const QByteArray svg = R"(<svg><path fill="#abcdef"/></svg>)";