Skip to content

Commit

Permalink
Bypass redundant cache lookup when using "Uncached" interface
Browse files Browse the repository at this point in the history
  • Loading branch information
@daschuer
daschuer committed Sep 22, 2023
1 parent bcc5d5e commit 40a0c16
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 98 deletions.
4 changes: 2 additions & 2 deletions src/library/basetracktablemodel.cpp
View file
Expand Up @@ -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);
Expand Down
105 changes: 43 additions & 62 deletions src/library/coverartcache.cpp
View file
Expand Up @@ -44,7 +44,7 @@ CoverArtCache::CoverArtCache() {
}

//static
void CoverArtCache::requestCover(
void CoverArtCache::requestCoverPrivate(
const QObject* pRequester,
const TrackPointer& pTrack,
const CoverInfo& coverInfo,
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -114,18 +117,31 @@ void CoverArtCache::requestUncachedCover(
}
pCache->tryLoadCover(
pRequester,
TrackPointer(),
pTrack,
coverInfo,
desiredWidth,
Loading::Default);
desiredWidth);
}

QPixmap CoverArtCache::tryLoadCover(
// static
void CoverArtCache::requestUncachedCover(
const QObject* pRequester,
const TrackPointer& pTrack,
int desiredWidth) {
VERIFY_OR_DEBUG_ASSERT(pTrack) {
return;
}
requestUncachedCoverPrivate(
pRequester,
pTrack,
pTrack->getCoverInfoWithLocation(),
desiredWidth);
}

void CoverArtCache::tryLoadCover(
const QObject* pRequester,
const TrackPointer& pTrack,
const CoverInfo& coverInfo,
int desiredWidth,
Loading loading) {
int desiredWidth) {
if (kLogger.traceEnabled()) {
kLogger.trace()
<< "requestCover"
Expand All @@ -136,48 +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<const QObject*, mixxx::cache_key_t> requestId = qMakePair(pRequester, requestedCacheKey);
if (m_runningRequests.contains(requestId)) {
return QPixmap();
}

// If this request comes from CoverDelegate (table view), it'll want to get
// a cropped cover which is ready to be drawn in the table view (cover art
// column). It's very important to keep the cropped covers in cache because
// it avoids having to rescale+crop it ALWAYS (which brings a lot of
// 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();
return;
}

if (kLogger.traceEnabled()) {
Expand All @@ -193,37 +178,33 @@ QPixmap CoverArtCache::tryLoadCover(
pRequester,
pTrack,
coverInfo,
desiredWidth,
loading == Loading::Default);
desiredWidth);
connect(watcher,
&QFutureWatcher<FutureResult>::finished,
this,
&CoverArtCache::coverLoaded);
watcher->setFuture(future);
return QPixmap();
return;
}

//static
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()) {
Expand Down Expand Up @@ -317,7 +298,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),
Expand Down
54 changes: 24 additions & 30 deletions src/library/coverartcache.h
View file
Expand Up @@ -15,15 +15,13 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
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,
Expand All @@ -32,33 +30,30 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
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;
};
Expand All @@ -68,8 +63,7 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
const QObject* pRequester,
TrackPointer pTrack,
CoverInfo coverInfo,
int desiredWidth,
bool emitSignals);
int desiredWidth);

private slots:
// Called when loadCover is complete in the main thread.
Expand All @@ -87,23 +81,23 @@ class CoverArtCache : public QObject, public Singleton<CoverArtCache> {
friend class Singleton<CoverArtCache>;

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<QPair<const QObject*, mixxx::cache_key_t>> m_runningRequests;
};

inline
QDebug operator<<(QDebug dbg, CoverArtCache::Loading loading) {
return dbg << static_cast<int>(loading);
}
4 changes: 2 additions & 2 deletions src/library/coverartdelegate.cpp
View file
Expand Up @@ -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<int>(option.rect.width() * scaleFactor));
} else {
// This is the fast path with an internal temporary track
CoverArtCache::requestCover(
CoverArtCache::requestUncachedCover(
this,
coverInfo,
static_cast<int>(option.rect.width() * scaleFactor));
Expand Down
4 changes: 2 additions & 2 deletions src/test/coverartcache_test.cpp
View file
Expand Up @@ -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());
}
Expand All @@ -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);
}
Expand Down

0 comments on commit 40a0c16

Please sign in to comment.