Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CoverArtCache refactoring + Fix scrolling lag after updating Mixxx #12009

Merged
merged 14 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,6 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/util/db/dbconnectionpool.cpp
src/util/db/dbconnectionpooled.cpp
src/util/db/dbconnectionpooler.cpp
src/util/db/dbid.cpp
src/util/db/fwdsqlquery.cpp
src/util/db/fwdsqlqueryselectresult.cpp
src/util/db/sqlite.cpp
Expand Down
4 changes: 2 additions & 2 deletions src/dialog/dlgreplacecuecolor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ void DlgReplaceCueColor::slotApply() {
continue;
}
CueDatabaseRow row = {DbId(selectQuery.value(idColumn)),
TrackId(selectQuery.value(trackIdColumn).toInt()),
TrackId(selectQuery.value(trackIdColumn)),
*color};
rows << row;
trackIds << row.trackId;
Expand Down Expand Up @@ -393,7 +393,7 @@ void DlgReplaceCueColor::slotApply() {
break;
}
query.bindValue(":id", row.id.toVariant());
query.bindValue(":track_id", row.trackId.value());
query.bindValue(":track_id", row.trackId.toVariant());
query.bindValue(":current_color", mixxx::RgbColor::toQVariant(row.color));
if (!query.exec()) {
LOG_FAILED_QUERY(query);
Expand Down
11 changes: 6 additions & 5 deletions src/library/autodj/autodjfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ bool AutoDJFeature::dragMoveAccept(const QUrl& url) {
}

// Add a crate to the auto-DJ queue.
void AutoDJFeature::slotAddCrateToAutoDj(int iCrateId) {
m_pTrackCollection->updateAutoDjCrate(CrateId(iCrateId), true);
void AutoDJFeature::slotAddCrateToAutoDj(CrateId crateId) {
m_pTrackCollection->updateAutoDjCrate(crateId, true);
}

void AutoDJFeature::slotRemoveCrateFromAutoDj() {
Expand Down Expand Up @@ -301,9 +301,10 @@ void AutoDJFeature::onRightClickChild(const QPoint& globalPos,
Crate crate;
while (nonAutoDjCrates.populateNext(&crate)) {
auto pAction = std::make_unique<QAction>(crate.getName(), &crateMenu);
int iCrateId = crate.getId().value();
connect(pAction.get(), &QAction::triggered,
this, [this, iCrateId] { slotAddCrateToAutoDj(iCrateId); });
auto crateId = crate.getId();
connect(pAction.get(), &QAction::triggered, this, [this, crateId] {
slotAddCrateToAutoDj(crateId);
});
crateMenu.addAction(pAction.get());
pAction.release();
}
Expand Down
2 changes: 1 addition & 1 deletion src/library/autodj/autodjfeature.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class AutoDJFeature : public LibraryFeature {

private slots:
// Add a crate to the auto-DJ queue.
void slotAddCrateToAutoDj(int iCrateId);
void slotAddCrateToAutoDj(CrateId crateId);

// Implements the context-menu item.
void slotRemoveCrateFromAutoDj();
Expand Down
2 changes: 1 addition & 1 deletion src/library/basesqltablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ CoverInfo BaseSqlTableModel::getCoverInfo(const QModelIndex& index) const {
fieldIndex(ColumnCache::
COLUMN_LIBRARYTABLE_COVERART_COLOR))
.data());
if (coverInfo.hasImage()) {
if (coverInfo.hasCacheKey()) {
coverInfo.type = static_cast<CoverInfo::Type>(
index.sibling(index.row(),
fieldIndex(ColumnCache::
Expand Down
22 changes: 13 additions & 9 deletions src/library/basetracktablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -546,16 +546,20 @@ QVariant BaseTrackTableModel::composeCoverArtToolTipHtml(
unsigned int absoluteHeightOfCoverartToolTip = static_cast<int>(
pViewScreen->availableGeometry().height() *
kRelativeHeightOfCoverartToolTip);
// Get image from cover art cache
CoverArtCache* pCache = CoverArtCache::instance();
QPixmap pixmap = QPixmap(absoluteHeightOfCoverartToolTip,
absoluteHeightOfCoverartToolTip); // Height also used as default for the width, in assumption that covers are squares
pixmap = pCache->tryLoadCover(this,
getCoverInfo(index),
absoluteHeightOfCoverartToolTip,
CoverArtCache::Loading::NoSignal);
const auto coverInfo = getCoverInfo(index);
if (!coverInfo.hasImage()) {
return QPixmap();
}
QPixmap pixmap = CoverArtCache::getCachedCover(
coverInfo,
absoluteHeightOfCoverartToolTip);
if (pixmap.isNull()) {
// Cache miss -> Don't show a tooltip
// Cache miss -> Don't show a tooltip, refresh cache
// Height used for the width, in assumption that covers are squares
CoverArtCache::requestUncachedCover(
nullptr,
coverInfo,
absoluteHeightOfCoverartToolTip);
return QVariant();
}
QByteArray data;
Expand Down
2 changes: 1 addition & 1 deletion src/library/basetracktablemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class BaseTrackTableModel : public QAbstractTableModel, public TrackModel {
/// Return the raw data value at the given index.
///
/// Expected types by ColumnCache field (pass-through = not validated):
/// COLUMN_LIBRARYTABLE_ID: DbId::value_type (pass-through)
/// COLUMN_LIBRARYTABLE_ID: int (pass-through)
/// COLUMN_LIBRARYTABLE_ARTIST: QString (pass-through)
/// COLUMN_LIBRARYTABLE_TITLE: QString (pass-through)
/// COLUMN_LIBRARYTABLE_ALBUM: QString (pass-through)
Expand Down
30 changes: 3 additions & 27 deletions src/library/coverart.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,15 @@ CoverInfoRelative::CoverInfoRelative()
m_legacyHash(defaultLegacyHash()) {
}

void CoverInfoRelative::setImage(
void CoverInfoRelative::setImageDigest(
const QImage& image) {
color = CoverImageUtils::extractBackgroundColor(image);
m_imageDigest = CoverImageUtils::calculateDigest(image);
DEBUG_ASSERT(image.isNull() == m_imageDigest.isEmpty());
m_legacyHash = calculateLegacyHash(image);
DEBUG_ASSERT(image.isNull() == (m_legacyHash == defaultLegacyHash()));
DEBUG_ASSERT(image.isNull() != hasImage());
DEBUG_ASSERT(image.isNull() != hasCacheKey());
DEBUG_ASSERT(image.isNull() == (type == NONE));
}

bool operator==(const CoverInfoRelative& lhs, const CoverInfoRelative& rhs) {
Expand Down Expand Up @@ -172,31 +173,6 @@ CoverInfo::LoadedImage CoverInfo::loadImage(TrackPointer pTrack) const {
return loadedImage;
}

bool CoverInfo::refreshImageDigest(
const QImage& loadedImage) {
if (!imageDigest().isEmpty()) {
// Assume that a non-empty digest has been calculated from
// the corresponding image. Otherwise we would refresh all
// digests over and over again.
// Invalid legacy hash values are ignored. These will only
// be refreshed when opened with a previous version.
return false;
}
QImage image = loadedImage;
if (image.isNull()) {
image = loadImage(TrackPointer()).image;
}
if (image.isNull() && type != CoverInfo::NONE) {
kLogger.warning()
<< "Resetting cover info"
<< *this;
reset();
return true;
}
setImage(image);
return true;
}

bool operator==(const CoverInfo& lhs, const CoverInfo& rhs) {
return static_cast<const CoverInfoRelative&>(lhs) ==
static_cast<const CoverInfoRelative&>(rhs) &&
Expand Down
20 changes: 9 additions & 11 deletions src/library/coverart.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,25 @@ class CoverInfoRelative {
*this = CoverInfoRelative();
}

void setImage(
const QImage& image = QImage());
void setImageDigest(
const QImage& image);

void setImageDigest(
QByteArray imageDigest,
quint16 legacyHash = defaultLegacyHash()) {
m_imageDigest = std::move(imageDigest);
m_legacyHash = legacyHash;
}

bool hasCacheKey() const {
return !m_imageDigest.isEmpty() || m_legacyHash != defaultLegacyHash();
}

bool hasImage() const {
return cacheKey() != CoverImageUtils::defaultCacheKey();
return type != NONE;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
}

const QByteArray imageDigest() const {
const QByteArray& imageDigest() const {
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
return m_imageDigest;
}
mixxx::cache_key_t cacheKey() const {
Expand Down Expand Up @@ -178,13 +183,6 @@ class CoverInfo : public CoverInfoRelative {
};
LoadedImage loadImage(TrackPointer pTrack = {}) const;

/// Verify the image digest and update it if necessary.
/// If the corresponding image has already been loaded it
/// could be provided as a parameter to avoid reloading
/// if actually needed.
bool refreshImageDigest(
const QImage& loadedImage = QImage());

QString trackLocation;
};

Expand Down
Loading
Loading