diff --git a/src/library/basetracktablemodel.cpp b/src/library/basetracktablemodel.cpp index 52dac3a7bf58..0156ef65f7b7 100644 --- a/src/library/basetracktablemodel.cpp +++ b/src/library/basetracktablemodel.cpp @@ -554,8 +554,8 @@ QVariant BaseTrackTableModel::composeCoverArtToolTipHtml( coverInfo, absoluteHeightOfCoverartToolTip); if (pixmap.isNull()) { - // Cache miss -> Don't show a tooltip - CoverArtCache::requestCover( + // Cache miss -> Don't show a tooltip, refresh cache + CoverArtCache::requestUncachedCover( nullptr, coverInfo, absoluteHeightOfCoverartToolTip); diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index dd82760cb173..98f8fb6b84c7 100644 --- a/src/library/coverartcache.cpp +++ b/src/library/coverartcache.cpp @@ -44,7 +44,7 @@ CoverArtCache::CoverArtCache() { } //static -void CoverArtCache::requestCover( +void CoverArtCache::requestCoverPrivate( const QObject* pRequester, const TrackPointer& pTrack, const CoverInfo& coverInfo, @@ -53,34 +53,36 @@ void CoverArtCache::requestCover( VERIFY_OR_DEBUG_ASSERT(pCache) { return; } + QPixmap pixmap = CoverArtCache::getCachedCover(coverInfo, desiredWidth); + if (!pixmap.isNull()) { + emit pCache->coverFound(pRequester, coverInfo, pixmap); + return; + } pCache->tryLoadCover( pRequester, pTrack, coverInfo, - desiredWidth, - Loading::Default); + desiredWidth); } //static void CoverArtCache::requestTrackCover( const QObject* pRequester, - const TrackPointer& pTrack, - int desiredWidth) { + const TrackPointer& pTrack) { VERIFY_OR_DEBUG_ASSERT(pTrack) { return; } - requestCover( + requestCoverPrivate( pRequester, pTrack, - pTrack->getCoverInfoWithLocation(), - desiredWidth); + pTrack->getCoverInfoWithLocation()); } // static QPixmap CoverArtCache::getCachedCover( const CoverInfo& coverInfo, int desiredWidth) { - if (coverInfo.type == CoverInfo::NONE) { + if (!coverInfo.hasImage()) { return QPixmap(); } const mixxx::cache_key_t requestedCacheKey = coverInfo.cacheKey(); @@ -104,8 +106,9 @@ QPixmap CoverArtCache::getCachedCover( } // static -void CoverArtCache::requestUncachedCover( +void CoverArtCache::requestUncachedCoverPrivate( const QObject* pRequester, + const TrackPointer& pTrack, const CoverInfo& coverInfo, int desiredWidth) { CoverArtCache* pCache = CoverArtCache::instance(); @@ -114,18 +117,31 @@ void CoverArtCache::requestUncachedCover( } pCache->tryLoadCover( pRequester, - TrackPointer(), + pTrack, coverInfo, - desiredWidth, - Loading::Default); + desiredWidth); +} + +// static +void CoverArtCache::requestUncachedCover( + const QObject* pRequester, + const TrackPointer& pTrack, + int desiredWidth) { + VERIFY_OR_DEBUG_ASSERT(pTrack) { + return; + } + requestUncachedCoverPrivate( + pRequester, + pTrack, + pTrack->getCoverInfoWithLocation(), + desiredWidth); } -QPixmap CoverArtCache::tryLoadCover( +void CoverArtCache::tryLoadCover( const QObject* pRequester, const TrackPointer& pTrack, const CoverInfo& coverInfo, - int desiredWidth, - Loading loading) { + int desiredWidth) { if (kLogger.traceEnabled()) { kLogger.trace() << "requestCover" @@ -136,20 +152,17 @@ QPixmap CoverArtCache::tryLoadCover( DEBUG_ASSERT(!pTrack || pTrack->getLocation() == coverInfo.trackLocation); - const auto requestedCacheKey = coverInfo.cacheKey(); - - if (coverInfo.type == CoverInfo::NONE) { - if (loading == Loading::Default) { - emit coverFound(pRequester, coverInfo, QPixmap()); - } - return QPixmap(); + if (!coverInfo.hasImage()) { + emit coverFound(pRequester, coverInfo, QPixmap()); + return; } + const auto requestedCacheKey = coverInfo.cacheKey(); // keep a list of trackIds for which a future is currently running // to avoid loading the same picture again while we are loading it QPair requestId = qMakePair(pRequester, requestedCacheKey); if (m_runningRequests.contains(requestId)) { - return QPixmap(); + return; } // If this request comes from CoverDelegate (table view), it'll want to get @@ -159,27 +172,6 @@ QPixmap CoverArtCache::tryLoadCover( // performance issues). QString cacheKey = pixmapCacheKey(requestedCacheKey, desiredWidth); - QPixmap pixmap; - if (QPixmapCache::find(cacheKey, &pixmap)) { - if (kLogger.traceEnabled()) { - kLogger.trace() - << "requestCover cache hit" - << coverInfo - << loading; - } - if (loading == Loading::Default) { - emit coverFound(pRequester, coverInfo, pixmap); - } - return pixmap; - } - - if (loading == Loading::CachedOnly) { - if (kLogger.traceEnabled()) { - kLogger.trace() << "requestCover cache miss"; - } - return QPixmap(); - } - if (kLogger.traceEnabled()) { kLogger.trace() << "requestCover starting future for" @@ -193,14 +185,13 @@ QPixmap CoverArtCache::tryLoadCover( pRequester, pTrack, coverInfo, - desiredWidth, - loading == Loading::Default); + desiredWidth); connect(watcher, &QFutureWatcher::finished, this, &CoverArtCache::coverLoaded); watcher->setFuture(future); - return QPixmap(); + return; } //static @@ -208,22 +199,19 @@ CoverArtCache::FutureResult CoverArtCache::loadCover( const QObject* pRequester, TrackPointer pTrack, CoverInfo coverInfo, - int desiredWidth, - bool signalWhenDone) { + int desiredWidth) { if (kLogger.traceEnabled()) { kLogger.trace() << "loadCover" << coverInfo - << desiredWidth - << signalWhenDone; + << desiredWidth; } DEBUG_ASSERT(!pTrack || pTrack->getLocation() == coverInfo.trackLocation); auto res = FutureResult( pRequester, - coverInfo.cacheKey(), - signalWhenDone); + coverInfo.cacheKey()); CoverInfo::LoadedImage loadedImage = coverInfo.loadImage(pTrack); if (!loadedImage.image.isNull()) { @@ -317,7 +305,7 @@ void CoverArtCache::coverLoaded() { m_runningRequests.remove(qMakePair(res.pRequester, res.requestedCacheKey)); - if (res.signalWhenDone) { + if (res.pRequester) { emit coverFound( res.pRequester, std::move(res.coverArt), diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h index 331e51c089a4..db219494da03 100644 --- a/src/library/coverartcache.h +++ b/src/library/coverartcache.h @@ -15,15 +15,13 @@ class CoverArtCache : public QObject, public Singleton { public: static void requestCover( const QObject* pRequester, - const CoverInfo& coverInfo, - int desiredWidth = 0) { - requestCover(pRequester, TrackPointer(), coverInfo, desiredWidth); + const CoverInfo& coverInfo) { + requestCoverPrivate(pRequester, TrackPointer(), coverInfo); } static void requestTrackCover( const QObject* pRequester, - const TrackPointer& pTrack, - int desiredWidth = 0); + const TrackPointer& pTrack); static QPixmap getCachedCover( const CoverInfo& coverInfo, @@ -32,33 +30,30 @@ class CoverArtCache : public QObject, public Singleton { static void requestUncachedCover( const QObject* pRequester, const CoverInfo& coverInfo, - int desiredWidth); + int desiredWidth) { + requestUncachedCoverPrivate(pRequester, TrackPointer(), coverInfo, desiredWidth); + } - enum class Loading { - CachedOnly, - NoSignal, - Default, // signal when done - }; + static void requestUncachedCover( + const QObject* pRequester, + const TrackPointer& pTrack, + int desiredWidth); // Only public for testing struct FutureResult { FutureResult() : pRequester(nullptr), - requestedCacheKey(CoverImageUtils::defaultCacheKey()), - signalWhenDone(false) { + requestedCacheKey(CoverImageUtils::defaultCacheKey()) { } FutureResult( const QObject* pRequestorArg, - mixxx::cache_key_t requestedCacheKeyArg, - bool signalWhenDoneArg) + mixxx::cache_key_t requestedCacheKeyArg) : pRequester(pRequestorArg), - requestedCacheKey(requestedCacheKeyArg), - signalWhenDone(signalWhenDoneArg) { + requestedCacheKey(requestedCacheKeyArg) { } const QObject* pRequester; mixxx::cache_key_t requestedCacheKey; - bool signalWhenDone; CoverArt coverArt; }; @@ -68,8 +63,7 @@ class CoverArtCache : public QObject, public Singleton { const QObject* pRequester, TrackPointer pTrack, CoverInfo coverInfo, - int desiredWidth, - bool emitSignals); + int desiredWidth); private slots: // Called when loadCover is complete in the main thread. @@ -87,23 +81,23 @@ class CoverArtCache : public QObject, public Singleton { friend class Singleton; private: - static void requestCover( + static void requestCoverPrivate( const QObject* pRequester, const TrackPointer& /*optional*/ pTrack, const CoverInfo& coverInfo, - int desiredWidth = 0); + int desiredWidth = 0); // <= 0: original size - QPixmap tryLoadCover( + static void requestUncachedCoverPrivate( + const QObject* pRequester, + const TrackPointer& /*optional*/ pTrack, + const CoverInfo& coverInfo, + int desiredWidth); + + void tryLoadCover( const QObject* pRequester, const TrackPointer& pTrack, const CoverInfo& info, - int desiredWidth, - Loading loading); + int desiredWidth); QSet> m_runningRequests; }; - -inline -QDebug operator<<(QDebug dbg, CoverArtCache::Loading loading) { - return dbg << static_cast(loading); -} diff --git a/src/library/coverartdelegate.cpp b/src/library/coverartdelegate.cpp index 9fac4ccbacc7..6c1d43f71edd 100644 --- a/src/library/coverartdelegate.cpp +++ b/src/library/coverartdelegate.cpp @@ -130,13 +130,13 @@ void CoverArtDelegate::paintItem( // This happens if we have the legacy hash // The CoverArtCache will take care of the update const auto pTrack = loadTrackByLocation(coverInfo.trackLocation); - CoverArtCache::requestTrackCover( + CoverArtCache::requestUncachedCover( this, pTrack, static_cast(option.rect.width() * scaleFactor)); } else { // This is the fast path with an internal temporary track - CoverArtCache::requestCover( + CoverArtCache::requestUncachedCover( this, coverInfo, static_cast(option.rect.width() * scaleFactor)); diff --git a/src/test/coverartcache_test.cpp b/src/test/coverartcache_test.cpp index 52c7f2470e09..be9017c103df 100644 --- a/src/test/coverartcache_test.cpp +++ b/src/test/coverartcache_test.cpp @@ -29,7 +29,7 @@ class CoverArtCacheTest : public LibraryTest, public CoverArtCache { info.trackLocation = trackLocation; CoverArtCache::FutureResult res; - res = CoverArtCache::loadCover(nullptr, TrackPointer(), info, 0, false); + res = CoverArtCache::loadCover(nullptr, TrackPointer(), info, 0); EXPECT_EQ(img, res.coverArt.loadedImage.image); EXPECT_TRUE(res.coverArt.coverLocation.isNull()); } @@ -47,7 +47,7 @@ class CoverArtCacheTest : public LibraryTest, public CoverArtCache { info.trackLocation = trackLocation; CoverArtCache::FutureResult res; - res = CoverArtCache::loadCover(nullptr, TrackPointer(), info, 0, false); + res = CoverArtCache::loadCover(nullptr, TrackPointer(), info, 0); EXPECT_EQ(img, res.coverArt.loadedImage.image); EXPECT_QSTRING_EQ(info.coverLocation, res.coverArt.coverLocation); }