diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_request_status_display_widget.cpp b/cockatrice/src/client/ui/picture_loader/picture_loader_request_status_display_widget.cpp index 3647eb71e..a792087d5 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_request_status_display_widget.cpp +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_request_status_display_widget.cpp @@ -2,23 +2,22 @@ PictureLoaderRequestStatusDisplayWidget::PictureLoaderRequestStatusDisplayWidget(QWidget *parent, const QUrl &_url, - PictureLoaderWorkerWork *worker) + const CardInfoPtr &card, + const QString &setName) : QWidget(parent) { layout = new QHBoxLayout(this); - if (worker->cardToDownload.getCard()) { - name = new QLabel(this); - name->setText(worker->cardToDownload.getCard()->getName()); - setShortname = new QLabel(this); - setShortname->setText(worker->cardToDownload.getSetName()); - providerId = new QLabel(this); - providerId->setText(worker->cardToDownload.getCard()->getProperty("uuid")); + name = new QLabel(this); + name->setText(card->getName()); + setShortname = new QLabel(this); + setShortname->setText(setName); + providerId = new QLabel(this); + providerId->setText(card->getProperty("uuid")); - layout->addWidget(name); - layout->addWidget(setShortname); - layout->addWidget(providerId); - } + layout->addWidget(name); + layout->addWidget(setShortname); + layout->addWidget(providerId); startTime = new QLabel(QDateTime::currentDateTime().toString(), this); elapsedTime = new QLabel("0", this); diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_request_status_display_widget.h b/cockatrice/src/client/ui/picture_loader/picture_loader_request_status_display_widget.h index 01eee1b48..46a809e47 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_request_status_display_widget.h +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_request_status_display_widget.h @@ -10,8 +10,10 @@ class PictureLoaderRequestStatusDisplayWidget : public QWidget { Q_OBJECT public: - PictureLoaderRequestStatusDisplayWidget(QWidget *parent, const QUrl &url, PictureLoaderWorkerWork *worker); - PictureLoaderWorkerWork *worker; + PictureLoaderRequestStatusDisplayWidget(QWidget *parent, + const QUrl &url, + const CardInfoPtr &card, + const QString &setName); void setFinished() { diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_status_bar.cpp b/cockatrice/src/client/ui/picture_loader/picture_loader_status_bar.cpp index 67b50a1f2..a2beeac1f 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_status_bar.cpp +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_status_bar.cpp @@ -38,15 +38,14 @@ void PictureLoaderStatusBar::cleanOldEntries() } } -void PictureLoaderStatusBar::addQueuedImageLoad(const QUrl &url, PictureLoaderWorkerWork *worker) +void PictureLoaderStatusBar::addQueuedImageLoad(const QUrl &url, const CardInfoPtr &card, const QString &setName) { - loadLog->addSettingsWidget(new PictureLoaderRequestStatusDisplayWidget(loadLog, url, worker)); + loadLog->addSettingsWidget(new PictureLoaderRequestStatusDisplayWidget(loadLog, url, card, setName)); progressBar->setMaximum(progressBar->maximum() + 1); } -void PictureLoaderStatusBar::addSuccessfulImageLoad(const QUrl &url, PictureLoaderWorkerWork *worker) +void PictureLoaderStatusBar::addSuccessfulImageLoad(const QUrl &url) { - Q_UNUSED(worker) progressBar->setValue(progressBar->value() + 1); for (PictureLoaderRequestStatusDisplayWidget *statusDisplayWidget : loadLog->popup->findChildren()) { diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_status_bar.h b/cockatrice/src/client/ui/picture_loader/picture_loader_status_bar.h index 6a507cbb0..6f5111a34 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_status_bar.h +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_status_bar.h @@ -15,8 +15,8 @@ public: explicit PictureLoaderStatusBar(QWidget *parent); public slots: - void addQueuedImageLoad(const QUrl &url, PictureLoaderWorkerWork *worker); - void addSuccessfulImageLoad(const QUrl &url, PictureLoaderWorkerWork *worker); + void addQueuedImageLoad(const QUrl &url, const CardInfoPtr &card, const QString &setName); + void addSuccessfulImageLoad(const QUrl &url); void cleanOldEntries(); private: 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 b0ab0b4c0..c7378fb27 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp @@ -12,7 +12,10 @@ #include #include -PictureLoaderWorker::PictureLoaderWorker() : QObject(nullptr), picDownload(SettingsCache::instance().getPicDownload()) +static constexpr int MAX_REQUESTS_PER_SEC = 10; + +PictureLoaderWorker::PictureLoaderWorker() + : QObject(nullptr), picDownload(SettingsCache::instance().getPicDownload()), requestQuota(MAX_REQUESTS_PER_SEC) { networkManager = new QNetworkAccessManager(this); // We need a timeout to ensure requests don't hang indefinitely in case of @@ -48,7 +51,7 @@ PictureLoaderWorker::PictureLoaderWorker() : QObject(nullptr), picDownload(Setti connect(this, &PictureLoaderWorker::imageLoadEnqueued, this, &PictureLoaderWorker::handleImageLoadEnqueued); - connect(&requestTimer, &QTimer::timeout, this, &PictureLoaderWorker::processQueuedRequests); + connect(&requestTimer, &QTimer::timeout, this, &PictureLoaderWorker::resetRequestQuota); requestTimer.setInterval(1000); requestTimer.start(); } @@ -69,7 +72,8 @@ void PictureLoaderWorker::queueRequest(const QUrl &url, PictureLoaderWorkerWork makeRequest(url, worker); } else { requestLoadQueue.append(qMakePair(url, worker)); - emit imageLoadQueued(url, worker); + emit imageLoadQueued(url, worker->cardToDownload.getCard(), worker->cardToDownload.getSetName()); + processQueuedRequests(); } } @@ -78,7 +82,7 @@ QNetworkReply *PictureLoaderWorker::makeRequest(const QUrl &url, PictureLoaderWo // Check for cached redirects QUrl cachedRedirect = getCachedRedirect(url); if (!cachedRedirect.isEmpty()) { - emit imageLoadSuccessful(url, worker); + emit imageLoadSuccessful(url); return makeRequest(cachedRedirect, worker); } @@ -95,22 +99,34 @@ QNetworkReply *PictureLoaderWorker::makeRequest(const QUrl &url, PictureLoaderWo return reply; } +void PictureLoaderWorker::resetRequestQuota() +{ + requestQuota = MAX_REQUESTS_PER_SEC; + processQueuedRequests(); +} + +/** + * Keeps processing requests from the queue until it is empty or until the quota runs out. + */ void PictureLoaderWorker::processQueuedRequests() { - for (int i = 0; i < 10; i++) { - processSingleRequest(); + while (requestQuota > 0 && processSingleRequest()) { + --requestQuota; } } /** - * Immediately processes a single queued request + * Immediately processes a single queued request. No-ops if the load queue is empty + * @return If a request was processed */ -void PictureLoaderWorker::processSingleRequest() +bool PictureLoaderWorker::processSingleRequest() { if (!requestLoadQueue.isEmpty()) { auto request = requestLoadQueue.takeFirst(); makeRequest(request.first, request.second); + return true; } + return false; } void PictureLoaderWorker::enqueueImageLoad(const CardInfoPtr &card) @@ -132,7 +148,7 @@ void PictureLoaderWorker::handleImageLoadEnqueued(const CardInfoPtr &card) // try to load image from local first QImage image = localLoader->tryLoad(card); if (!image.isNull()) { - imageLoadedSuccessfully(card, image); + handleImageLoaded(card, image); } else { // queue up to load image from remote only after local loading failed new PictureLoaderWorkerWork(this, card); @@ -140,10 +156,9 @@ void PictureLoaderWorker::handleImageLoadEnqueued(const CardInfoPtr &card) } /** - * Called when image loading is done - * Contrary to the name, this is called on both success and failure. Failures are indicated by an empty QImage. + * Called when image loading is done. Failures are indicated by an empty QImage. */ -void PictureLoaderWorker::imageLoadedSuccessfully(const CardInfoPtr &card, const QImage &image) +void PictureLoaderWorker::handleImageLoaded(const CardInfoPtr &card, const QImage &image) { currentlyLoading.remove(card); emit imageLoaded(card, image); 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 2f0a737ad..f5b0a3507 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h @@ -38,8 +38,8 @@ public: public slots: QNetworkReply *makeRequest(const QUrl &url, PictureLoaderWorkerWork *workThread); void processQueuedRequests(); - void processSingleRequest(); - void imageLoadedSuccessfully(const CardInfoPtr &card, const QImage &image); + bool processSingleRequest(); + void handleImageLoaded(const CardInfoPtr &card, const QImage &image); void cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl); void removedCachedUrl(const QUrl &url); @@ -52,7 +52,9 @@ private: static constexpr int CacheTTLInDays = 30; // TODO: Make user configurable bool picDownload; QQueue> requestLoadQueue; - QTimer requestTimer; // Timer for processing delayed requests + + int requestQuota; + QTimer requestTimer; // Timer for refreshing request quota PictureLoaderLocal *localLoader; QSet currentlyLoading; // for deduplication purposes @@ -63,13 +65,14 @@ private: void cleanStaleEntries(); private slots: + void resetRequestQuota(); void handleImageLoadEnqueued(const CardInfoPtr &card); signals: void imageLoadEnqueued(const CardInfoPtr &card); void imageLoaded(CardInfoPtr card, const QImage &image); - void imageLoadQueued(const QUrl &url, PictureLoaderWorkerWork *worker); - void imageLoadSuccessful(const QUrl &url, PictureLoaderWorkerWork *worker); + void imageLoadQueued(const QUrl &url, const CardInfoPtr &card, const QString &setName); + void imageLoadSuccessful(const QUrl &url); }; #endif // PICTURE_LOADER_WORKER_H 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 712f68dc1..38c8f14cb 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 @@ -22,7 +22,7 @@ PictureLoaderWorkerWork::PictureLoaderWorkerWork(const PictureLoaderWorker *work connect(this, &PictureLoaderWorkerWork::requestImageDownload, worker, &PictureLoaderWorker::queueRequest); connect(this, &PictureLoaderWorkerWork::urlRedirected, worker, &PictureLoaderWorker::cacheRedirect); connect(this, &PictureLoaderWorkerWork::cachedUrlInvalidated, worker, &PictureLoaderWorker::removedCachedUrl); - connect(this, &PictureLoaderWorkerWork::imageLoaded, worker, &PictureLoaderWorker::imageLoadedSuccessfully); + connect(this, &PictureLoaderWorkerWork::imageLoaded, worker, &PictureLoaderWorker::handleImageLoaded); // Hook up signals to settings connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged()));