From f05964318767b8ae5e7f21eaa97b6bfbe86dbcc5 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Fri, 13 Jun 2025 14:54:43 +0200 Subject: [PATCH] [Picture Loader] Consider local images, remove some unused variables. (#5985) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Consider local images, remove some unused variables. * Move checking for local card images outside of card download loop, add NOT found debug line. --------- Co-authored-by: Lukas Brübach --- .../picture_loader/picture_loader_worker.cpp | 29 +---- .../ui/picture_loader/picture_loader_worker.h | 19 +--- .../picture_loader_worker_work.cpp | 107 ++++++++++++++---- .../picture_loader_worker_work.h | 12 +- .../deck_preview_deck_tags_display_widget.h | 2 + 5 files changed, 102 insertions(+), 67 deletions(-) diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp index 8ef0818dd..3f6cc8fa3 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp @@ -15,17 +15,8 @@ // Card back returned by gatherer when card is not found QStringList PictureLoaderWorker::md5Blacklist = QStringList() << "db0c48db407a907c16ade38de048a441"; -PictureLoaderWorker::PictureLoaderWorker() - : QObject(nullptr), picsPath(SettingsCache::instance().getPicsPath()), - customPicsPath(SettingsCache::instance().getCustomPicsPath()), - picDownload(SettingsCache::instance().getPicDownload()), downloadRunning(false), loadQueueRunning(false), - overrideAllCardArtWithPersonalPreference(SettingsCache::instance().getOverrideAllCardArtWithPersonalPreference()) +PictureLoaderWorker::PictureLoaderWorker() : QObject(nullptr), picDownload(SettingsCache::instance().getPicDownload()) { - connect(&SettingsCache::instance(), SIGNAL(picsPathChanged()), this, SLOT(picsPathChanged())); - connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged())); - connect(&SettingsCache::instance(), &SettingsCache::overrideAllCardArtWithPersonalPreferenceChanged, this, - &PictureLoaderWorker::setOverrideAllCardArtWithPersonalPreference); - networkManager = new QNetworkAccessManager(this); // We need a timeout to ensure requests don't hang indefinitely in case of // cache corruption, see related Qt bug: https://bugreports.qt.io/browse/QTBUG-111397 @@ -213,24 +204,6 @@ void PictureLoaderWorker::cleanStaleEntries() } } -void PictureLoaderWorker::picDownloadChanged() -{ - QMutexLocker locker(&mutex); - picDownload = SettingsCache::instance().getPicDownload(); -} - -void PictureLoaderWorker::picsPathChanged() -{ - QMutexLocker locker(&mutex); - picsPath = SettingsCache::instance().getPicsPath(); - customPicsPath = SettingsCache::instance().getCustomPicsPath(); -} - -void PictureLoaderWorker::setOverrideAllCardArtWithPersonalPreference(bool _overrideAllCardArtWithPersonalPreference) -{ - overrideAllCardArtWithPersonalPreference = _overrideAllCardArtWithPersonalPreference; -} - void PictureLoaderWorker::clearNetworkCache() { networkManager->cache()->clear(); diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h index 2e0735dbb..8e3339df5 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h @@ -29,9 +29,9 @@ class PictureLoaderWorker : public QObject public: explicit PictureLoaderWorker(); ~PictureLoaderWorker() override; - void queueRequest(const QUrl &url, PictureLoaderWorkerWork *worker); - void enqueueImageLoad(const CardInfoPtr &card); + void enqueueImageLoad(const CardInfoPtr &card); // Starts a thread for the image to be loaded + void queueRequest(const QUrl &url, PictureLoaderWorkerWork *worker); // Queues network requests for load threads void clearNetworkCache(); public slots: @@ -43,22 +43,15 @@ private: static QStringList md5Blacklist; QThread *pictureLoaderThread; - QString picsPath, customPicsPath; - QList loadQueue; - QMutex mutex; QNetworkAccessManager *networkManager; QNetworkDiskCache *cache; QHash> redirectCache; // Stores redirect and timestamp QString cacheFilePath; // Path to persistent storage static constexpr int CacheTTLInDays = 30; // TODO: Make user configurable - QList cardsToDownload; - PictureToLoad cardBeingLoaded; PictureToLoad cardBeingDownloaded; - bool picDownload, downloadRunning, loadQueueRunning; + bool picDownload; QQueue> requestLoadQueue; - QList> requestQueue; QTimer requestTimer; // Timer for processing delayed requests - bool overrideAllCardArtWithPersonalPreference; void cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl); QUrl getCachedRedirect(const QUrl &originalUrl) const; @@ -66,13 +59,7 @@ private: void saveRedirectCache() const; void cleanStaleEntries(); -private slots: - void picDownloadChanged(); - void picsPathChanged(); - void setOverrideAllCardArtWithPersonalPreference(bool _overrideAllCardArtWithPersonalPreference); - signals: - void startLoadQueue(); void imageLoaded(CardInfoPtr card, const QImage &image); void imageLoadQueued(const QUrl &url, PictureLoaderWorkerWork *worker); void imageLoadSuccessful(const QUrl &url, PictureLoaderWorkerWork *worker); diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp b/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp index bc9787767..048f3b1f7 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp @@ -16,16 +16,27 @@ QStringList PictureLoaderWorkerWork::md5Blacklist = QStringList() << "db0c48db407a907c16ade38de048a441"; PictureLoaderWorkerWork::PictureLoaderWorkerWork(PictureLoaderWorker *_worker, const CardInfoPtr &toLoad) - : QThread(nullptr), worker(_worker), cardToDownload(toLoad) + : QThread(nullptr), worker(_worker), cardToDownload(toLoad), picsPath(SettingsCache::instance().getPicsPath()), + customPicsPath(SettingsCache::instance().getCustomPicsPath()), + overrideAllCardArtWithPersonalPreference(SettingsCache::instance().getOverrideAllCardArtWithPersonalPreference()) + { + // Hook up signals to the orchestrator connect(this, &PictureLoaderWorkerWork::requestImageDownload, worker, &PictureLoaderWorker::queueRequest, Qt::QueuedConnection); connect(this, &PictureLoaderWorkerWork::imageLoaded, worker, &PictureLoaderWorker::imageLoadedSuccessfully, Qt::QueuedConnection); + + // Hook up signals to settings + connect(&SettingsCache::instance(), SIGNAL(picsPathChanged()), this, SLOT(picsPathChanged())); + connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged())); + connect(&SettingsCache::instance(), &SettingsCache::overrideAllCardArtWithPersonalPreferenceChanged, this, + &PictureLoaderWorkerWork::setOverrideAllCardArtWithPersonalPreference); + pictureLoaderThread = new QThread; pictureLoaderThread->start(QThread::LowPriority); moveToThread(pictureLoaderThread); - startNextPicDownload(); + startWork(); } PictureLoaderWorkerWork::~PictureLoaderWorkerWork() @@ -33,32 +44,66 @@ PictureLoaderWorkerWork::~PictureLoaderWorkerWork() pictureLoaderThread->deleteLater(); } -bool PictureLoaderWorkerWork::cardImageExistsOnDisk(QString &setName, QString &correctedCardname) +void PictureLoaderWorkerWork::startWork() +{ + QString setName = cardToDownload.getSetName(); + QString cardName = cardToDownload.getCard()->getName(); + QString correctedCardName = cardToDownload.getCard()->getCorrectedName(); + + qCDebug(PictureLoaderWorkerLog).nospace() + << "[card: " << cardName << " set: " << setName << "]: Trying to load picture"; + + // FIXME: This is a hack so that to keep old Cockatrice behavior + // (ignoring provider ID) when the "override all card art with personal + // preference" is set. + // + // Figure out a proper way to integrate the two systems at some point. + // + // Note: need to go through a member for + // overrideAllCardArtWithPersonalPreference as reading from the + // SettingsCache instance from the PictureLoaderWorker thread could + // cause race conditions. + // + // XXX: Reading from the CardDatabaseManager instance from the + // PictureLoaderWorker thread might not be safe either + bool searchCustomPics = overrideAllCardArtWithPersonalPreference || + CardDatabaseManager::getInstance()->isProviderIdForPreferredPrinting( + cardName, cardToDownload.getCard()->getPixmapCacheKey()); + if (!(searchCustomPics && cardImageExistsOnDisk(setName, correctedCardName, searchCustomPics))) { + startNextPicDownload(); + } +} + +bool PictureLoaderWorkerWork::cardImageExistsOnDisk(const QString &setName, + const QString &correctedCardName, + const bool searchCustomPics) { QImage image; QImageReader imgReader; imgReader.setDecideFormatFromContent(true); QList picsPaths = QList(); - QDirIterator it(SettingsCache::instance().getCustomPicsPath(), - QDirIterator::Subdirectories | QDirIterator::FollowSymlinks); - // Recursively check all subdirectories of the CUSTOM folder - while (it.hasNext()) { - QString thisPath(it.next()); - QFileInfo thisFileInfo(thisPath); + if (searchCustomPics) { + QDirIterator it(customPicsPath, QDirIterator::Subdirectories | QDirIterator::FollowSymlinks); - if (thisFileInfo.isFile() && - (thisFileInfo.fileName() == correctedCardname || thisFileInfo.completeBaseName() == correctedCardname || - thisFileInfo.baseName() == correctedCardname)) { - picsPaths << thisPath; // Card found in the CUSTOM directory, somewhere + // Recursively check all subdirectories of the CUSTOM folder + while (it.hasNext()) { + QString thisPath(it.next()); + QFileInfo thisFileInfo(thisPath); + + if (thisFileInfo.isFile() && + (thisFileInfo.fileName() == correctedCardName || thisFileInfo.completeBaseName() == correctedCardName || + thisFileInfo.baseName() == correctedCardName)) { + picsPaths << thisPath; // Card found in the CUSTOM directory, somewhere + } } } if (!setName.isEmpty()) { - picsPaths << SettingsCache::instance().getPicsPath() + "/" + setName + "/" + correctedCardname + picsPaths << picsPath + "/" + setName + "/" + correctedCardName // We no longer store downloaded images there, but don't just ignore // stuff that old versions have put there. - << SettingsCache::instance().getPicsPath() + "/downloadedPics/" + setName + "/" + correctedCardname; + << picsPath + "/downloadedPics/" + setName + "/" + correctedCardName; } // Iterates through the list of paths, searching for images with the desired @@ -67,27 +112,28 @@ bool PictureLoaderWorkerWork::cardImageExistsOnDisk(QString &setName, QString &c for (const auto &_picsPath : picsPaths) { imgReader.setFileName(_picsPath); if (imgReader.read(&image)) { - qCDebug(PictureLoaderWorkerWorkLog).nospace() - << "PictureLoader: [card: " << correctedCardname << " set: " << setName << "]: Picture found on disk."; + qCDebug(PictureLoaderWorkerLog).nospace() + << "[card: " << correctedCardName << " set: " << setName << "]: Picture found on disk."; imageLoaded(cardToDownload.getCard(), image); return true; } imgReader.setFileName(_picsPath + ".full"); if (imgReader.read(&image)) { - qCDebug(PictureLoaderWorkerWorkLog).nospace() << "PictureLoader: [card: " << correctedCardname - << " set: " << setName << "]: Picture.full found on disk."; + qCDebug(PictureLoaderWorkerLog).nospace() + << "[card: " << correctedCardName << " set: " << setName << "]: Picture.full found on disk."; imageLoaded(cardToDownload.getCard(), image); return true; } imgReader.setFileName(_picsPath + ".xlhq"); if (imgReader.read(&image)) { - qCDebug(PictureLoaderWorkerWorkLog).nospace() << "PictureLoader: [card: " << correctedCardname - << " set: " << setName << "]: Picture.xlhq found on disk."; + qCDebug(PictureLoaderWorkerLog).nospace() + << "[card: " << correctedCardName << " set: " << setName << "]: Picture.xlhq found on disk."; imageLoaded(cardToDownload.getCard(), image); return true; } } - + qCDebug(PictureLoaderWorkerLog).nospace() + << "[card: " << correctedCardName << " set: " << setName << "]: Picture NOT found on disk."; return false; } @@ -229,6 +275,23 @@ void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply) reply->deleteLater(); } +void PictureLoaderWorkerWork::picDownloadChanged() +{ + picDownload = SettingsCache::instance().getPicDownload(); +} + +void PictureLoaderWorkerWork::picsPathChanged() +{ + picsPath = SettingsCache::instance().getPicsPath(); + customPicsPath = SettingsCache::instance().getCustomPicsPath(); +} + +void PictureLoaderWorkerWork::setOverrideAllCardArtWithPersonalPreference( + bool _overrideAllCardArtWithPersonalPreference) +{ + overrideAllCardArtWithPersonalPreference = _overrideAllCardArtWithPersonalPreference; +} + bool PictureLoaderWorkerWork::imageIsBlackListed(const QByteArray &picData) { QString md5sum = QCryptographicHash::hash(picData, QCryptographicHash::Md5).toHex(); diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h b/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h index bbc1564ec..e1089b080 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h @@ -26,21 +26,31 @@ class PictureLoaderWorkerWork : public QThread public: explicit PictureLoaderWorkerWork(PictureLoaderWorker *worker, const CardInfoPtr &toLoad); ~PictureLoaderWorkerWork() override; + void startWork(); PictureLoaderWorker *worker; PictureToLoad cardToDownload; + public slots: void picDownloadFinished(QNetworkReply *reply); void picDownloadFailed(); private: + QString picsPath, customPicsPath; + bool overrideAllCardArtWithPersonalPreference; static QStringList md5Blacklist; QThread *pictureLoaderThread; QNetworkAccessManager *networkManager; bool picDownload, downloadRunning, loadQueueRunning; + void startNextPicDownload(); - bool cardImageExistsOnDisk(QString &setName, QString &correctedCardName); + bool cardImageExistsOnDisk(const QString &setName, const QString &correctedCardName, bool searchCustomPics); bool imageIsBlackListed(const QByteArray &); +private slots: + void picDownloadChanged(); + void picsPathChanged(); + void setOverrideAllCardArtWithPersonalPreference(bool _overrideAllCardArtWithPersonalPreference); + signals: void startLoadQueue(); void imageLoaded(CardInfoPtr card, const QImage &image); diff --git a/cockatrice/src/client/ui/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h b/cockatrice/src/client/ui/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h index cf8917d14..3b18c2232 100644 --- a/cockatrice/src/client/ui/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h +++ b/cockatrice/src/client/ui/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h @@ -6,6 +6,8 @@ #include +inline bool confirmOverwriteIfExists(QWidget *parent, const QString &filePath); + class DeckPreviewWidget; class DeckPreviewDeckTagsDisplayWidget : public QWidget {