From 5ddfcd7ead665907e40e1e62ddba308813023868 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 25 Oct 2026 00:36:14 +0200 Subject: [PATCH] [Pictures] Watch the custom picture folder instead of polling (#7635) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The local picture loader walked the whole custom picture tree every ten seconds on the GUI thread. Watch the root and every subdirectory with QFileSystemWatcher, debounce bursts of events into a single 500 ms refresh, and keep a five-minute fallback poll for filesystems where watching fails. Add an explicit-path constructor and indexedPaths() so the indexing can be tested without SettingsCache, and emit indexChanged() after a rebuild. Verification: cockatrice built. Co-authored-by: Lukas Brübach --- .../card_picture_loader_local.cpp | 75 ++++++++++++++++--- .../card_picture_loader_local.h | 30 +++++++- 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp index 1437e23f4..fde6830c2 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -19,35 +20,62 @@ #include #include -static constexpr int REFRESH_INTERVAL_MS = 10 * 1000; +static constexpr int REFRESH_DEBOUNCE_MS = 500; +// Watching can fail on network and FUSE mounts; the slow poll still picks those +// changes up without the old ten-second full rescan. +static constexpr int FALLBACK_REFRESH_INTERVAL_MS = 5 * 60 * 1000; CardPictureLoaderLocal::CardPictureLoaderLocal(QObject *parent) - : QObject(parent), picsPath(SettingsCache::instance().paths().getPicsPath()), - customPicsPath(SettingsCache::instance().paths().getCustomPicsPath()) + : CardPictureLoaderLocal(parent, + SettingsCache::instance().paths().getPicsPath(), + SettingsCache::instance().paths().getCustomPicsPath()) { // Hook up signals to settings connect(&SettingsCache::instance().paths(), &PathsSettings::picsPathChanged, this, &CardPictureLoaderLocal::picsPathChanged); +} + +CardPictureLoaderLocal::CardPictureLoaderLocal(QObject *parent, + const QString &_picsPath, + const QString &_customPicsPath) + : QObject(parent), picsPath(_picsPath), customPicsPath(_customPicsPath), + folderWatcher(new QFileSystemWatcher(this)), indexRefreshTimer(new QTimer(this)), + fallbackRefreshTimer(new QTimer(this)) +{ + indexRefreshTimer->setSingleShot(true); + indexRefreshTimer->setInterval(REFRESH_DEBOUNCE_MS); + connect(indexRefreshTimer, &QTimer::timeout, this, &CardPictureLoaderLocal::refreshIndex); + + connect(folderWatcher, &QFileSystemWatcher::directoryChanged, indexRefreshTimer, qOverload<>(&QTimer::start)); + connect(folderWatcher, &QFileSystemWatcher::fileChanged, indexRefreshTimer, qOverload<>(&QTimer::start)); + + fallbackRefreshTimer->setInterval(FALLBACK_REFRESH_INTERVAL_MS); + connect(fallbackRefreshTimer, &QTimer::timeout, this, &CardPictureLoaderLocal::refreshIndex); + fallbackRefreshTimer->start(); refreshIndex(); - - refreshTimer = new QTimer(this); - connect(refreshTimer, &QTimer::timeout, this, &CardPictureLoaderLocal::refreshIndex); - refreshTimer->start(REFRESH_INTERVAL_MS); } void CardPictureLoaderLocal::refreshIndex() { customFolderIndex.clear(); + QSet directories; + + QDir customDir(customPicsPath); + if (customDir.exists()) { + directories.insert(customDir.absolutePath()); + } QDirIterator it(customPicsPath, QDirIterator::Subdirectories | QDirIterator::FollowSymlinks); // Recursively check all subdirectories of the CUSTOM folder while (it.hasNext()) { - QString thisPath(it.next()); - QFileInfo thisFileInfo(thisPath); + const QString thisPath = it.next(); + const QFileInfo thisFileInfo = it.fileInfo(); - if (thisFileInfo.isFile()) { + if (thisFileInfo.isDir()) { + directories.insert(thisFileInfo.absoluteFilePath()); + } else if (thisFileInfo.isFile()) { // We don't know which name is the correctedName because there might be '.'s in the cardName. // Just add all possibilities to be sure. customFolderIndex.insert(thisFileInfo.baseName(), thisFileInfo.absoluteFilePath()); @@ -55,8 +83,34 @@ void CardPictureLoaderLocal::refreshIndex() } } + updateWatchedDirectories(directories); + qCDebug(CardPictureLoaderLocalLog) << "Finished indexing local image folder CUSTOM; map now has" << customFolderIndex.size() << "entries."; + emit indexChanged(); +} + +void CardPictureLoaderLocal::updateWatchedDirectories(const QSet &directories) +{ + const QSet stale = watchedDirectories - directories; + if (!stale.isEmpty()) { + folderWatcher->removePaths(QStringList(stale.begin(), stale.end())); + } + + const QSet added = directories - watchedDirectories; + if (!added.isEmpty()) { + const QStringList failed = folderWatcher->addPaths(QStringList(added.begin(), added.end())); + if (!failed.isEmpty()) { + qCDebug(CardPictureLoaderLocalLog) << "Could not watch custom picture directories:" << failed; + } + } + + watchedDirectories = directories; +} + +QStringList CardPictureLoaderLocal::indexedPaths(const QString &baseName) const +{ + return customFolderIndex.values(baseName); } QImage CardPictureLoaderLocal::tryLoad(const ExactCard &toLoad) const @@ -145,4 +199,5 @@ void CardPictureLoaderLocal::picsPathChanged() { picsPath = SettingsCache::instance().paths().getPicsPath(); customPicsPath = SettingsCache::instance().paths().getCustomPicsPath(); + refreshIndex(); } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.h index 009897acc..35190366d 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_local.h @@ -5,10 +5,12 @@ #include #include #include +#include #include #include class ExactCard; +class QFileSystemWatcher; class QTimer; inline Q_LOGGING_CATEGORY(CardPictureLoaderLocalLog, "card_picture_loader.local"); @@ -40,6 +42,18 @@ public: */ explicit CardPictureLoaderLocal(QObject *parent); + /** + * @brief Constructs the loader against explicit folders without SettingsCache. + * + * Used by tests and by callers that already resolved the configured paths. + */ + CardPictureLoaderLocal(QObject *parent, const QString &picsPath, const QString &customPicsPath); + + /** + * @brief Returns every indexed file whose base name matches @p baseName. + */ + [[nodiscard]] QStringList indexedPaths(const QString &baseName) const; + /** * @brief Attempts to load a card image from local disk or custom folders. * @param toLoad ExactCard object representing the card to load. @@ -54,7 +68,10 @@ private: QString customPicsPath; ///< Path to custom card image folder QMultiHash customFolderIndex; ///< Multimap from cardName to file paths in CUSTOM folder - QTimer *refreshTimer; ///< Timer for periodic folder index refresh + QFileSystemWatcher *folderWatcher; ///< Watches the custom folder tree for changes + QTimer *indexRefreshTimer; ///< Debounces bursts of filesystem events + QTimer *fallbackRefreshTimer; ///< Slow poll for filesystems without change notification + QSet watchedDirectories; ///< Directories currently registered with folderWatcher /** * @brief Rebuilds the index of the custom image folder. @@ -65,6 +82,11 @@ private: */ void refreshIndex(); + /** + * @brief Keeps the watcher registered on exactly @p directories. + */ + void updateWatchedDirectories(const QSet &directories); + /** * @brief Attempts to load a card image from disk given its set and name info. * @param setName Corrected short name of the card's set. @@ -82,6 +104,12 @@ private: const QString &collectorNumber, const QString &providerId) const; +signals: + /** + * @brief Emitted after the custom folder index has been rebuilt. + */ + void indexChanged(); + private slots: /** * @brief Updates internal paths when the user changes picture settings.