diff --git a/cockatrice/src/client/settings/cache_settings.cpp b/cockatrice/src/client/settings/cache_settings.cpp index b40e26377..fb66a6ebb 100644 --- a/cockatrice/src/client/settings/cache_settings.cpp +++ b/cockatrice/src/client/settings/cache_settings.cpp @@ -6,7 +6,6 @@ #include "shortcuts_settings.h" #include -#include #include #include #include @@ -48,32 +47,14 @@ Q_GLOBAL_STATIC(SettingsCache, settingsCache) SettingsCache &SettingsCache::instance() { - SettingsCache &cache = *settingsCache; // returns a QT managed singleton reference - // The release channels own QNetworkAccessManagers, which must be destroyed - // while the application object is alive. ~SettingsCache() runs during - // static teardown, after QApplication is gone, so hook the application's - // shutdown instead. Retried here because the first access may predate the - // application object. - cache.installShutdownHook(); - return cache; + return *settingsCache; // returns a QT managed singleton reference } -void SettingsCache::installShutdownHook() +SettingsCache::~SettingsCache() { - if (shutdownHookInstalled) { - return; + for (ReleaseChannel *releaseChannel : releaseChannels) { + delete releaseChannel; } - QCoreApplication *application = QCoreApplication::instance(); - if (application == nullptr) { - return; - } - connect(application, &QCoreApplication::aboutToQuit, this, [this] { - for (ReleaseChannel *releaseChannel : std::as_const(releaseChannels)) { - delete releaseChannel; - } - releaseChannels.clear(); - }); - shutdownHookInstalled = true; } QString SettingsCache::getDataPath() diff --git a/cockatrice/src/client/settings/cache_settings.h b/cockatrice/src/client/settings/cache_settings.h index a6610ee0d..472aac44f 100644 --- a/cockatrice/src/client/settings/cache_settings.h +++ b/cockatrice/src/client/settings/cache_settings.h @@ -91,19 +91,15 @@ private: QString themeName; QList releaseChannels; - bool shutdownHookInstalled = false; bool isPortableBuild; - // Connects the aboutToQuit handler that deletes the release channels while - // the application object is still alive; a no-op before it exists. - void installShutdownHook(); - [[nodiscard]] QString getSafeConfigPath(QString configEntry, QString defaultPath) const; [[nodiscard]] QString getSafeConfigFilePath(QString configEntry, QString defaultPath) const; void loadPaths(); public: SettingsCache(); + ~SettingsCache() override; QString getDataPath(); QString getSettingsPath(); [[nodiscard]] QString getCachePath() const; diff --git a/cockatrice/src/interface/pixel_map_generator.cpp b/cockatrice/src/interface/pixel_map_generator.cpp index 037293af9..f024ba580 100644 --- a/cockatrice/src/interface/pixel_map_generator.cpp +++ b/cockatrice/src/interface/pixel_map_generator.cpp @@ -4,7 +4,6 @@ #include "theme_icon.h" #include "theme_manager.h" -#include #include #include #include @@ -436,19 +435,39 @@ QIcon UserLevelPixmapGenerator::generateIconWithColorOverride(int height, QMap UserLevelPixmapGenerator::iconCache; -QPixmap DropdownIconPixmapGenerator::generatePixmap(int height, bool expanded) +QPixmap LockPixmapGenerator::generatePixmap(int height) { - const QString key = QString::number(expanded) + ":" + QString::number(height); + + int key = height; + if (pmCache.contains(key)) { + return pmCache.value(key); + } + + const QString resolved = themeManager->assetPath(QStringLiteral("icons/lock")); + QPixmap pixmap = loadSentinelSvgPixmap(resolved, QPalette::WindowText, QSize(height, height)); + if (pixmap.isNull()) { + pixmap = tryLoadImage(QStringLiteral("theme:") + resolved, QSize(height, height), true); + } + pmCache.insert(key, pixmap); + return pixmap; +} + +QMap LockPixmapGenerator::pmCache; + +QPixmap DropdownIconPixmapGenerator::generatePixmap(int height, bool expanded) +{ + QString key = QString::number(expanded) + ":" + QString::number(height); if (pmCache.contains(key)) { return pmCache.value(key); } - // Render through the themed icon engine: it keeps the glyph's aspect ratio, - // follows the palette, and renders at the device pixel ratio instead of - // handing a DPR-1 bitmap to a label that would upscale and blur it on a 2x - // display. const QString prefix = QStringLiteral("icons/") + (expanded ? "dropdown_expanded" : "dropdown_collapsed"); - const QPixmap pixmap = loadThemedIcon(prefix).pixmap(QSize(height, height), qApp->devicePixelRatio()); + const QString resolved = themeManager->assetPath(prefix); + QPixmap pixmap = loadSentinelSvgPixmap(resolved, QPalette::WindowText, QSize(height, height)); + if (pixmap.isNull()) { + pixmap = tryLoadImage(QStringLiteral("theme:") + resolved, QSize(height, height), true); + } + pmCache.insert(key, pixmap); return pixmap; } @@ -520,6 +539,7 @@ void clearPixmapGeneratorCaches() PingPixmapGenerator::clear(); CountryPixmapGenerator::clear(); UserLevelPixmapGenerator::clear(); + LockPixmapGenerator::clear(); DropdownIconPixmapGenerator::clear(); ManaSymbolPixmapGenerator::clear(); } diff --git a/cockatrice/src/interface/pixel_map_generator.h b/cockatrice/src/interface/pixel_map_generator.h index 951f4231b..299f72609 100644 --- a/cockatrice/src/interface/pixel_map_generator.h +++ b/cockatrice/src/interface/pixel_map_generator.h @@ -106,6 +106,19 @@ public: } }; +class LockPixmapGenerator +{ +private: + static QMap pmCache; + +public: + static QPixmap generatePixmap(int height); + static void clear() + { + pmCache.clear(); + } +}; + class DropdownIconPixmapGenerator { private: diff --git a/cockatrice/src/interface/theme_icon.cpp b/cockatrice/src/interface/theme_icon.cpp index 038a2fba0..a355b5174 100644 --- a/cockatrice/src/interface/theme_icon.cpp +++ b/cockatrice/src/interface/theme_icon.cpp @@ -10,8 +10,6 @@ #include #include #include -#include -#include #include #include #include @@ -80,11 +78,6 @@ 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()) + @@ -95,25 +88,15 @@ 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. 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. + * 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. */ -class ThemeSvgIconEngine final : public QObject, public QIconEngine +class ThemeSvgIconEngine final : 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 @@ -130,14 +113,12 @@ public: } const QSize pixelSize(qRound(logicalSize.width() * devicePixelRatio), qRound(logicalSize.height() * devicePixelRatio)); - const QPixmap pixmap = render(pixelSize, devicePixelRatio, mode); + QPixmap pixmap = render(pixelSize, devicePixelRatio, mode); if (!pixmap.isNull()) { - // 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. + pixmap.setDevicePixelRatio(devicePixelRatio); const QSizeF contentSize = pixmap.deviceIndependentSize(); const QRectF targetRect( - QRectF(rect).center() - QPointF(contentSize.width() / 2.0, contentSize.height() / 2.0), contentSize); + QPointF(rect.center()) - QPointF(contentSize.width() / 2.0, contentSize.height() / 2.0), contentSize); painter->drawPixmap(targetRect, pixmap, QRectF(pixmap.rect())); } } @@ -156,7 +137,11 @@ public: // Qt < 6.8 passes the device-dependent size to the engine. const QSize pixelSize = logicalSize; #endif - return render(pixelSize, scale, mode); + QPixmap pixmap = render(pixelSize, scale, mode); + if (!pixmap.isNull()) { + pixmap.setDevicePixelRatio(scale); + } + return pixmap; } QSize actualSize(const QSize &size, QIcon::Mode, QIcon::State) override @@ -165,41 +150,26 @@ public: } private: - void invalidate() - { - hasResolvedAsset = false; - hasIntrinsicSize = false; - cache.clear(); - } - void ensureResolved() const { - if (hasResolvedAsset && (!themeManager || themeManager->getCurrentThemePath() == resolvedThemePath)) { - return; + 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; } - 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) { - 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(); - } + const QByteArray svg = readSvg(resolvedPath); + QSvgRenderer renderer(svg); + intrinsicSizeCache = renderer.isValid() ? renderer.defaultSize() : QSize(24, 24); if (intrinsicSizeCache.isEmpty()) { intrinsicSizeCache = QSize(24, 24); } @@ -216,44 +186,35 @@ 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(); - if (!resolvedHasSentinel) { + const QString themePath = resolvedThemePath; + const bool darkMode = resolvedDarkMode; + + const QByteArray svg = readSvg(resolvedPath); + if (svg.isEmpty() || !QString::fromUtf8(svg).contains(QStringLiteral("currentColor"), Qt::CaseInsensitive)) { // Authored art carries its own colors: use Qt's normal loader so // raster overrides and non-SVG assets keep working. - QPixmap pixmap = fromAsset(pixelSize, mode); - if (!pixmap.isNull()) { - pixmap.setDevicePixelRatio(devicePixelRatio); + 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; } const QByteArray color = sentinelColor(mode, role); - const QString key = cacheKey(resolvedPath, color, pixelSize, devicePixelRatio); + const QString key = cacheKey(resolvedPath, color, pixelSize, devicePixelRatio) + QLatin1Char('|') + themePath + + (darkMode ? QLatin1Char('d') : QLatin1Char('l')); const auto cached = cache.constFind(key); if (cached != cache.constEnd()) { return cached.value(); } - 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); - } + QPixmap pixmap = renderSvg(resolveSvgColorSentinel(svg, color), pixelSize, devicePixelRatio); if (cache.size() >= 64) { cache.clear(); } @@ -266,11 +227,9 @@ private: mutable QHash cache; mutable QString resolvedPath; mutable QString resolvedThemePath; - mutable QByteArray resolvedSvg; mutable QSize intrinsicSizeCache; - mutable bool resolvedAssetIsSvg = false; - mutable bool resolvedHasSentinel = false; - mutable bool hasResolvedAsset = false; + mutable bool resolvedDarkMode = false; + mutable bool hasResolvedPath = false; mutable bool hasIntrinsicSize = false; }; } // namespace @@ -287,7 +246,7 @@ QPixmap loadSentinelSvgPixmap(const QString &resolvedPath, QPalette::ColorRole r } const QByteArray svg = readSvg(resolvedPath); - if (!hasSentinel(svg)) { + if (svg.isEmpty() || !QString::fromUtf8(svg).contains(QStringLiteral("currentColor"), Qt::CaseInsensitive)) { return {}; } diff --git a/cockatrice/src/interface/widgets/printing_selector/printing_selector_card_overlay_widget.cpp b/cockatrice/src/interface/widgets/printing_selector/printing_selector_card_overlay_widget.cpp index 5d535397f..6512c049d 100644 --- a/cockatrice/src/interface/widgets/printing_selector/printing_selector_card_overlay_widget.cpp +++ b/cockatrice/src/interface/widgets/printing_selector/printing_selector_card_overlay_widget.cpp @@ -14,7 +14,6 @@ #include #include -#include #include #include #include @@ -24,9 +23,6 @@ #include #include #include -#include -#include -#include #include #include #include @@ -411,56 +407,12 @@ void PrintingSelectorCardOverlayWidget::customMenu(QPoint point) previewSourceMenu = nullptr; } -/** - * @brief Composes the pin badge: the glyph over a rounded palette backdrop. - * - * The badge floats on card art rather than on a palette surface, so a bare glyph - * colored WindowText is not enough: a thin dark stroke disappears on dark art in a - * light palette, and a thin light stroke disappears on light art in a dark palette. - * A Window-filled rounded rect with a WindowText outline gives the glyph a solid - * backing, and the outline guarantees the badge edge contrasts with the art even when - * the fill matches it. Both roles follow the palette, like the glyph. - * - * @param logicalSize The badge's size in logical pixels. - * @param devicePixelRatio The label's device pixel ratio, applied to the pixmap so it - * stays crisp on HiDPI displays. - * @param palette The palette the glyph's WindowText resolves against. - */ -static QPixmap composePinBadgePixmap(const QSize &logicalSize, qreal devicePixelRatio, const QPalette &palette) -{ - // Inset the glyph so the backdrop reads as a badge around it, not as a border. - const int inset = qRound(logicalSize.width() * 0.16); - const QSize glyphSize = logicalSize - QSize(inset * 2, inset * 2); - const QPixmap glyph = loadThemedIcon(QStringLiteral("icons/pin")).pixmap(glyphSize, devicePixelRatio); - if (glyph.isNull()) { - // Let the caller fall back to the textual badge instead of drawing an empty panel. - return QPixmap(); - } - - QPixmap badge(logicalSize * devicePixelRatio); - badge.setDevicePixelRatio(devicePixelRatio); - badge.fill(Qt::transparent); - - QPainter painter(&badge); - painter.setRenderHint(QPainter::Antialiasing); - - // Half-pixel inset keeps the 1.5px outline inside the pixmap instead of clipping it. - const QRectF box(QPointF(0.5, 0.5), QSizeF(logicalSize) - QSizeF(1, 1)); - painter.setPen(QPen(palette.color(QPalette::WindowText), 1.5)); - painter.setBrush(palette.color(QPalette::Window)); - painter.drawRoundedRect(box, 8, 8); - painter.drawPixmap(QPointF(inset, inset), glyph); - - return badge; -} - /** * @brief Initializes the pin badge overlay and loads a DPI-aware icon pixmap. * - * The glyph is drawn onto a rounded palette backdrop so the badge stays readable on - * card art, and it is rasterized at the label's device pixel ratio so it stays crisp on - * HiDPI displays. It is re-rendered when the palette or theme changes so the badge - * follows the surrounding UI instead of freezing at its first colors. + * The icon is asked for at the label's device pixel ratio so it stays crisp on HiDPI + * displays, and re-rendered when the palette or theme changes so the badge follows the + * surrounding UI instead of freezing at its first colors. * If the SVG cannot be read, a textual fallback badge is created instead. */ void PrintingSelectorCardOverlayWidget::initializePinBadge() @@ -476,8 +428,10 @@ void PrintingSelectorCardOverlayWidget::initializePinBadge() } bool pinLoaded = false; + // Ask QIcon for a 64×64 logical pixmap; QIcon applies the target DPR itself. const QSize targetSize(64, 64); - const QPixmap pinPix = composePinBadgePixmap(targetSize, pinBadge->devicePixelRatioF(), pinBadge->palette()); + const QPixmap pinPix = + loadThemedIcon(QStringLiteral("icons/pin")).pixmap(targetSize, pinBadge->devicePixelRatioF()); if (!pinPix.isNull()) { pinBadge->setPixmap(pinPix); pinBadge->setFixedSize(targetSize); diff --git a/cockatrice/src/interface/widgets/server/games_model.cpp b/cockatrice/src/interface/widgets/server/games_model.cpp index 217a62abc..315d51d9e 100644 --- a/cockatrice/src/interface/widgets/server/games_model.cpp +++ b/cockatrice/src/interface/widgets/server/games_model.cpp @@ -1,7 +1,6 @@ #include "games_model.h" #include "../../../client/settings/cache_settings.h" -#include "../../theme_icon.h" #include "../interface/pixel_map_generator.h" #include "libcockatrice/protocol/pb/serverinfo_user.pb.h" #include "user/user_list_proxy.h" @@ -189,10 +188,7 @@ QVariant GamesModel::data(const QModelIndex &index, int role) const return result.join(", "); } case Qt::DecorationRole: { - // The themed icon follows the palette, so the lock also - // switches to HighlightedText on a selected row, and the - // item view paints it at the device pixel ratio. - return gameentry.with_password() ? loadThemedIcon(QStringLiteral("icons/lock")) : QVariant(); + return gameentry.with_password() ? QIcon(LockPixmapGenerator::generatePixmap(13)) : QVariant(); case Qt::TextAlignmentRole: return Qt::AlignLeft; default: diff --git a/libcockatrice_utility/libcockatrice/utility/svg_recolor.cpp b/libcockatrice_utility/libcockatrice/utility/svg_recolor.cpp index e25a31ba5..7072c1586 100644 --- a/libcockatrice_utility/libcockatrice/utility/svg_recolor.cpp +++ b/libcockatrice_utility/libcockatrice/utility/svg_recolor.cpp @@ -1,41 +1,15 @@ #include "svg_recolor.h" -#include - -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 +#include QByteArray resolveSvgColorSentinel(const QByteArray &svg, const QByteArray &color) { - // 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) { + const QString source = QString::fromUtf8(svg); + if (!source.contains(QStringLiteral("currentColor"), Qt::CaseInsensitive)) { return svg; } - 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; + QString resolved = source; + resolved.replace(QStringLiteral("currentColor"), QString::fromLatin1(color), Qt::CaseInsensitive); + return resolved.toUtf8(); } diff --git a/libcockatrice_utility/libcockatrice/utility/svg_recolor.h b/libcockatrice_utility/libcockatrice/utility/svg_recolor.h index 56d609b87..767d46120 100644 --- a/libcockatrice_utility/libcockatrice/utility/svg_recolor.h +++ b/libcockatrice_utility/libcockatrice/utility/svg_recolor.h @@ -16,13 +16,8 @@ * (for example a QPalette role serialized to #RRGGBB) and receive SVG bytes * that are safe to hand to QSvgRenderer. * - * 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 svg UTF-8 SVG source. Every `currentColor` occurrence, in any case, + * is replaced. * @param color Concrete SVG color (for example "#ff00ff"), as ASCII bytes. * @return The recolored SVG source; the input is left untouched. */ diff --git a/tests/icon_assets/icon-baseline.txt b/tests/icon_assets/icon-baseline.txt new file mode 100644 index 000000000..01a3f8f15 --- /dev/null +++ b/tests/icon_assets/icon-baseline.txt @@ -0,0 +1,9 @@ +# Icon rule baseline (ratchet). +# +# One violation per line: . +# Rules: xml, viewbox, editor-metadata, hardcoded-color, color-filename, no-token, physical-size, raster. +# +# This file may only shrink. When an icon is fixed, delete its line(s) in the +# same change; the test fails while a fixed entry remains. Never add new lines. +# +# The UI glyph set is fully migrated; this list is intentionally empty. diff --git a/tests/icon_assets/icon_assets_test.cpp b/tests/icon_assets/icon_assets_test.cpp index f0b06d5cc..93681d59c 100644 --- a/tests/icon_assets/icon_assets_test.cpp +++ b/tests/icon_assets/icon_assets_test.cpp @@ -13,12 +13,18 @@ namespace { const QString ICONS_RELATIVE_PATH = QStringLiteral("cockatrice/resources/icons"); +const QString BASELINE_RELATIVE_PATH = QStringLiteral("tests/icon_assets/icon-baseline.txt"); QString iconsDir() { return QDir(QStringLiteral(COCKATRICE_SOURCE_DIR)).absoluteFilePath(ICONS_RELATIVE_PATH); } +QString baselinePath() +{ + return QDir(QStringLiteral(COCKATRICE_SOURCE_DIR)).absoluteFilePath(BASELINE_RELATIVE_PATH); +} + /** @brief True for content art that is allowed to carry full color. */ bool isContentArt(const QString &relativePath) { @@ -214,6 +220,26 @@ QSet collectViolations() return violations; } +/** @brief Parses the ratchet file; returns false with an error when it cannot be read. */ +bool readBaseline(QSet &entries, QString &error) +{ + QFile file(baselinePath()); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { + error = QStringLiteral("Cannot read %1").arg(baselinePath()); + return false; + } + + const QStringList lines = QString::fromUtf8(file.readAll()).split(QLatin1Char('\n')); + for (const QString &line : lines) { + const QString trimmed = line.trimmed(); + if (trimmed.isEmpty() || trimmed.startsWith(QLatin1Char('#'))) { + continue; + } + entries.insert(trimmed); + } + return true; +} + QString formatSet(const QSet &entries) { QStringList lines(entries.begin(), entries.end()); @@ -224,18 +250,44 @@ QString formatSet(const QSet &entries) } // namespace /** - * @brief Every UI glyph follows the palette-token rules. + * @brief The icon ratchet: no new violations, and fixed icons must leave the baseline. * - * The legacy set was paid down icon by icon, so no baseline grandfathers - * violations: a new violation fails right here. Scheme variants - * (`*-dark.svg`/`*-light.svg`) and mana art are exempt inside collectViolations(). + * Existing UI glyphs are grandfathered in tests/icon_assets/icon-baseline.txt. Fixing an icon + * without deleting its baseline lines fails on purpose, so the list can only shrink. */ -TEST(IconAssets, AssetsFollowTheRules) +TEST(IconAssets, BaselineRatchet) { ASSERT_TRUE(QDir::isAbsolutePath(iconsDir())); ASSERT_TRUE(QDir(iconsDir()).exists()) << "Missing icon directory: " << iconsDir().toStdString(); - const QSet violations = collectViolations(); - EXPECT_TRUE(violations.isEmpty()) << "Icon rule violations. Fix the icon; do not add exceptions:\n" - << formatSet(violations).toStdString(); + QSet baseline; + QString error; + ASSERT_TRUE(readBaseline(baseline, error)) << error.toStdString(); + + const QSet actual = collectViolations(); + + const QSet newViolations = actual - baseline; + const QSet staleBaseline = baseline - actual; + + EXPECT_TRUE(newViolations.isEmpty()) << "New icon rule violations. Fix the icon; do not add exceptions:\n" + << formatSet(newViolations).toStdString(); + + EXPECT_TRUE(staleBaseline.isEmpty()) << "These baseline entries are fixed or gone; delete them from " + << BASELINE_RELATIVE_PATH.toStdString() << ":\n" + << formatSet(staleBaseline).toStdString(); +} + +/** @brief Guards against duplicate or malformed baseline entries. */ +TEST(IconAssets, BaselineIsWellFormed) +{ + QSet baseline; + QString error; + ASSERT_TRUE(readBaseline(baseline, error)) << error.toStdString(); + EXPECT_TRUE(baseline.isEmpty()) << "The legacy baseline is fully paid down; fix new violations instead"; + + static const QRegularExpression entryPattern(QStringLiteral( + R"(^(xml|viewbox|editor-metadata|hardcoded-color|color-filename|no-token|physical-size|raster) \S+$)")); + for (const QString &entry : baseline) { + EXPECT_TRUE(entryPattern.match(entry).hasMatch()) << "Malformed baseline entry: " << entry.toStdString(); + } } diff --git a/tests/picture_loader_benchmark/settings_cache_mock.cpp b/tests/picture_loader_benchmark/settings_cache_mock.cpp index 13e831b62..70b08644c 100644 --- a/tests/picture_loader_benchmark/settings_cache_mock.cpp +++ b/tests/picture_loader_benchmark/settings_cache_mock.cpp @@ -144,6 +144,10 @@ SettingsCache::SettingsCache() loadPaths(); } +// The real destructor deletes the release channels; this mock never creates any, +// but the vtable still needs the definition. +SettingsCache::~SettingsCache() = default; + void SettingsCache::loadPaths() { QString dataPath = getDataPath(); diff --git a/tests/svg_recolor_test.cpp b/tests/svg_recolor_test.cpp index abe1e8d1a..62990c322 100644 --- a/tests/svg_recolor_test.cpp +++ b/tests/svg_recolor_test.cpp @@ -43,19 +43,6 @@ TEST(SvgRecolor, DoesNotModifyTheInput) EXPECT_EQ(svg, original); } -TEST(SvgRecolor, PreservesBytesThatAreNotValidUtf8) -{ - QByteArray svg = ""; - svg.append("\xff\xfe"); - svg.append(R"()"); - - 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"()"; diff --git a/tests/theme_migration/theme_icon_render_test.cpp b/tests/theme_migration/theme_icon_render_test.cpp index f12db5b16..5d0c13e3f 100644 --- a/tests/theme_migration/theme_icon_render_test.cpp +++ b/tests/theme_migration/theme_icon_render_test.cpp @@ -28,10 +28,7 @@ void ensureGuiApplication() static int argc = 1; static char name[] = "theme_icon_test"; static char *argv[] = {name, nullptr}; - // Leaked on purpose: a function-local static QApplication is destroyed before Qt's - // own library statics at exit, and the network module's destructors then run - // against a torn-down QObject layer (SEGV under AddressSanitizer). - static QApplication *app = new QApplication(argc, argv); + static QApplication app(argc, argv); Q_UNUSED(app); }