diff --git a/src/library/coverartcache.cpp b/src/library/coverartcache.cpp index dd82760cb173..c8af7a5b5a16 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,12 +53,16 @@ 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 @@ -69,7 +73,7 @@ void CoverArtCache::requestTrackCover( VERIFY_OR_DEBUG_ASSERT(pTrack) { return; } - requestCover( + requestCoverPrivate( pRequester, pTrack, pTrack->getCoverInfoWithLocation(), @@ -80,7 +84,7 @@ void CoverArtCache::requestTrackCover( 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 +108,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 +119,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 +154,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 +174,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 +187,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 +201,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,10 +307,8 @@ void CoverArtCache::coverLoaded() { m_runningRequests.remove(qMakePair(res.pRequester, res.requestedCacheKey)); - if (res.signalWhenDone) { - emit coverFound( - res.pRequester, - std::move(res.coverArt), - pixmap); - } + emit coverFound( + res.pRequester, + std::move(res.coverArt), + pixmap); } diff --git a/src/library/coverartcache.h b/src/library/coverartcache.h index 331e51c089a4..f7cd7573a048 100644 --- a/src/library/coverartcache.h +++ b/src/library/coverartcache.h @@ -17,7 +17,7 @@ class CoverArtCache : public QObject, public Singleton { const QObject* pRequester, const CoverInfo& coverInfo, int desiredWidth = 0) { - requestCover(pRequester, TrackPointer(), coverInfo, desiredWidth); + requestCoverPrivate(pRequester, TrackPointer(), coverInfo, desiredWidth); } static void requestTrackCover( @@ -32,33 +32,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 = 0); // 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 +65,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 +83,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); + + static void requestUncachedCoverPrivate( const QObject* pRequester, const TrackPointer& /*optional*/ pTrack, const CoverInfo& coverInfo, int desiredWidth = 0); - QPixmap tryLoadCover( + 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/test/coverartcache_test.cpp b/src/test/coverartcache_test.cpp index fd3b7b3643a1..a6989b6c3644 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_EQ(CoverImageUtils::calculateDigest(img), res.coverArt.imageDigest()); EXPECT_TRUE(res.coverArt.coverLocation.isNull()); @@ -48,7 +48,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_EQ(CoverImageUtils::calculateDigest(img), res.coverArt.imageDigest()); EXPECT_QSTRING_EQ(info.coverLocation, res.coverArt.coverLocation);