Merge pull request #12009 from daschuer/gh11131_2

CoverArtCache refactoring + Fix scrolling lag after updating Mixxx

CMakeLists.txt

@@ -1074,7 +1074,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

src/dialog/dlgreplacecuecolor.cpp

@@ -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;
@@ -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);

src/library/autodj/autodjfeature.cpp

@@ -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() {
@@ -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();
         }

src/library/autodj/autodjfeature.h

@@ -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();

src/library/basesqltablemodel.cpp

@@ -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::

src/library/basetracktablemodel.cpp

@@ -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;

src/library/basetracktablemodel.h

@@ -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)

src/library/coverart.cpp

@@ -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) {
@@ -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) &&

src/library/coverart.h

@@ -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;
     }
 
-    const QByteArray imageDigest() const {
+    const QByteArray& imageDigest() const {
         return m_imageDigest;
     }
     mixxx::cache_key_t cacheKey() const {
@@ -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;
 };