OverviewDelegate: implement lazy loading like in CoverArtDelegate

src/library/basetracktablemodel.cpp

@@ -520,7 +520,7 @@ QAbstractItemDelegate* BaseTrackTableModel::delegateForColumn(
                 new CoverArtDelegate(pTableView);
         // WLibraryTableView -> CoverArtDelegate
         connect(pTableView,
-                &WLibraryTableView::onlyCachedCoverArt,
+                &WLibraryTableView::onlyCachedCoversAndOverviews,
                 pCoverArtDelegate,
                 &CoverArtDelegate::slotInhibitLazyLoading);
         // CoverArtDelegate -> BaseTrackTableModel
@@ -534,13 +534,13 @@ QAbstractItemDelegate* BaseTrackTableModel::delegateForColumn(
     } else if (index == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_WAVESUMMARYHEX)) {
         auto* pOverviewDelegate = new OverviewDelegate(pTableView);
         connect(pOverviewDelegate,
-                &OverviewDelegate::overviewReady,
+                &OverviewDelegate::overviewRowsChanged,
                 this,
-                &BaseTrackTableModel::slotOverviewChanged);
-        connect(pOverviewDelegate,
-                &OverviewDelegate::overviewChanged,
-                this,
-                &BaseTrackTableModel::slotOverviewChanged);
+                &BaseTrackTableModel::slotRefreshOverviewRows);
+        connect(pTableView,
+                &WLibraryTableView::onlyCachedCoversAndOverviews,
+                pOverviewDelegate,
+                &OverviewDelegate::slotInhibitLazyLoading);
         return pOverviewDelegate;
     }
     return nullptr;
@@ -1217,17 +1217,15 @@ void BaseTrackTableModel::slotRefreshCoverRows(
     emitDataChangedForMultipleRowsInColumn(rows, column);
 }
 
-void BaseTrackTableModel::slotOverviewChanged(TrackId trackId) {
+void BaseTrackTableModel::slotRefreshOverviewRows(const QList<int>& rows) {
+    if (rows.isEmpty()) {
+        return;
+    }
     const int column = fieldIndex(LIBRARYTABLE_WAVESUMMARYHEX);
     VERIFY_OR_DEBUG_ASSERT(column >= 0) {
         return;
     }
-
-    const auto rows = getTrackRows(trackId);
-    for (int row : rows) {
-        QModelIndex idx = index(row, column);
-        emit dataChanged(idx, idx);
-    }
+    emitDataChangedForMultipleRowsInColumn(rows, column);
 }
 
 void BaseTrackTableModel::slotRefreshAllRows() {

src/library/basetracktablemodel.h

@@ -249,7 +249,7 @@ class BaseTrackTableModel : public QAbstractTableModel, public TrackModel {
     void slotRefreshCoverRows(
             const QList<int>& rows);
 
-    void slotOverviewChanged(TrackId trackId);
+    void slotRefreshOverviewRows(const QList<int>& rows);
 
     void slotRefreshAllRows();
 

src/library/overviewcache.cpp

@@ -100,7 +100,32 @@ void OverviewCache::onTrackSummaryChanged(TrackId trackId) {
     emit overviewChanged(trackId);
 }
 
-QPixmap OverviewCache::requestOverview(
+QPixmap OverviewCache::requestCachedOverview(
+        mixxx::OverviewType type,
+        const TrackId trackId,
+        const QObject* pRequester,
+        const QSize desiredSize) {
+    if (!trackId.isValid()) {
+        return QPixmap();
+    }
+
+    if (m_currentlyLoading.contains(trackId)) {
+        return QPixmap();
+    }
+
+    if (m_tracksWithoutOverview.contains(trackId)) {
+        return QPixmap();
+    }
+
+    kLogger.info() << "requestCachedOverview()" << trackId << pRequester << desiredSize;
+
+    const QString cacheKey = pixmapCacheKey(trackId, desiredSize, type);
+    QPixmap pixmap;
+    QPixmapCache::find(cacheKey, &pixmap);
+    return pixmap;
+}
+
+QPixmap OverviewCache::requestUncachedOverview(
         mixxx::OverviewType type,
         const WaveformSignalColors& signalColors,
         const TrackId trackId,
@@ -120,9 +145,9 @@ QPixmap OverviewCache::requestOverview(
 
     kLogger.info() << "requestOverview()" << trackId << pRequester << desiredSize;
 
-    // request overview
     const QString cacheKey = pixmapCacheKey(trackId, desiredSize, type);
     QPixmap pixmap;
+    // Maybe it has been cached since the request for cached image?
     if (QPixmapCache::find(cacheKey, &pixmap)) {
         return pixmap;
     }
@@ -229,5 +254,5 @@ void OverviewCache::overviewPrepared() {
     }
     m_currentlyLoading.remove(res.trackId);
 
-    emit overviewReady(res.requester, res.trackId, !pixmap.isNull(), res.resizedToSize);
+    emit overviewReady(res.requester, res.trackId, !pixmap.isNull());
 }

src/library/overviewcache.h

@@ -16,7 +16,12 @@ class OverviewCache : public QObject, public Singleton<OverviewCache> {
   public:
     void onTrackSummaryChanged(TrackId);
 
-    QPixmap requestOverview(
+    QPixmap requestCachedOverview(
+            const mixxx::OverviewType type,
+            const TrackId trackId,
+            const QObject* pRequester,
+            const QSize desiredSize);
+    QPixmap requestUncachedOverview(
             const mixxx::OverviewType type,
             const WaveformSignalColors& signalColors,
             const TrackId trackId,
@@ -44,8 +49,7 @@ class OverviewCache : public QObject, public Singleton<OverviewCache> {
     void overviewReady(
             const QObject* pRequester,
             const TrackId trackId,
-            bool pixmapValid,
-            const QSize resizedToSize); // Currently only needed for debugging
+            bool pixmapValid);
 
     void overviewChanged(TrackId);
 

src/library/tabledelegates/coverartdelegate.cpp

@@ -111,9 +111,6 @@ void CoverArtDelegate::paintItem(
     paintItemBackground(painter, option, index);
 
     CoverInfo coverInfo = m_pTrackModel->getCoverInfo(index);
-    VERIFY_OR_DEBUG_ASSERT(m_pTrackModel) {
-        return;
-    }
     bool drewPixmap = false;
     if (coverInfo.hasImage()) {
         VERIFY_OR_DEBUG_ASSERT(m_pCache) {

src/library/tabledelegates/overviewdelegate.cpp

@@ -6,6 +6,7 @@
 #include "library/trackmodel.h"
 #include "moc_overviewdelegate.cpp"
 #include "util/logger.h"
+#include "util/make_const_iterator.h"
 #include "widget/wlibrary.h"
 
 namespace {
@@ -39,7 +40,8 @@ OverviewDelegate::OverviewDelegate(QTableView* pTableView)
         : TableItemDelegate(pTableView),
           m_pTrackModel(asTrackModel(pTableView)),
           m_pCache(OverviewCache::instance()),
-          m_type(mixxx::OverviewType::RGB) {
+          m_type(mixxx::OverviewType::RGB),
+          m_inhibitLazyLoading(false) {
     VERIFY_OR_DEBUG_ASSERT(m_pCache) {
         kLogger.warning() << "Caching of overviews is not available";
         return;
@@ -58,8 +60,7 @@ OverviewDelegate::OverviewDelegate(QTableView* pTableView)
     connect(m_pCache,
             &OverviewCache::overviewChanged,
             this,
-            &OverviewDelegate::overviewChanged,
-            Qt::DirectConnection); // signal-to-signal
+            &OverviewDelegate::slotOverviewChanged);
 
     m_pTypeControl = make_parented<ControlProxy>(
             QStringLiteral("[Waveform]"),
@@ -83,19 +84,65 @@ void OverviewDelegate::slotTypeControlChanged(double v) {
     m_pTableView->update();
 }
 
+void OverviewDelegate::emitOverviewRowsChanged(QSet<TrackId>&& trackIds) {
+    if (trackIds.isEmpty()) {
+        return;
+    }
+
+    QList<int> rows;
+    for (auto id : trackIds) {
+        const QList<int> idRows = m_pTrackModel->getTrackRows(id);
+        rows.append(idRows);
+    }
+    if (rows.isEmpty()) {
+        // For some reason this can happen during startup
+        return;
+    }
+    // Sort in ascending order...
+    std::sort(rows.begin(), rows.end());
+    // ...and then deduplicate...
+    constErase(
+            &rows,
+            make_const_iterator(
+                    rows, std::unique(rows.begin(), rows.end())),
+            rows.constEnd());
+    // ...before emitting the signal.
+    DEBUG_ASSERT(!rows.isEmpty());
+    emit overviewRowsChanged(std::move(rows));
+}
+
+void OverviewDelegate::slotInhibitLazyLoading(bool inhibitLazyLoading) {
+    m_inhibitLazyLoading = inhibitLazyLoading;
+    if (m_inhibitLazyLoading || m_cacheMissIds.isEmpty()) {
+        return;
+    }
+    // If we can request non-cache covers now, request updates
+    // for all rows that were cache misses since the last time.
+    // Reset the member variable before mutating the aggregated
+    // rows list (-> implicit sharing) and emitting a signal that
+    // in turn may trigger new signals for CoverArtDelegate!
+    QSet<TrackId> staleIds = std::move(m_cacheMissIds);
+    DEBUG_ASSERT(m_cacheMissIds.isEmpty());
+    emitOverviewRowsChanged(std::move(staleIds));
+}
+
 /// Maybe request repaint via dataChanged() by BaseTrackTableModel
 void OverviewDelegate::slotOverviewReady(const QObject* pRequester,
         const TrackId trackId,
-        bool pixmapValid,
-        const QSize resizedToSize) {
-    Q_UNUSED(resizedToSize);
-    // kLogger.info() << "slotOverviewReady()" << trackId << pixmap << resizedToSize;
+        bool pixmapValid) {
+    // kLogger.info() << "slotOverviewReady()" << trackId << "pixmap valid:" << pixmapValid;
 
     if (pRequester == this && pixmapValid) {
-        emit overviewReady(trackId);
+        emitOverviewRowsChanged(QSet<TrackId>{trackId});
     }
 }
 
+/// Maybe request repaint via dataChanged() by BaseTrackTableModel
+void OverviewDelegate::slotOverviewChanged(const TrackId trackId) {
+    // kLogger.info() << "slotOverviewChanged()" << trackId;
+    emitOverviewRowsChanged(QSet<TrackId>{trackId});
+}
+
 void OverviewDelegate::paintItem(QPainter* painter,
         const QStyleOptionViewItem& option,
         const QModelIndex& index) const {
@@ -107,12 +154,25 @@ void OverviewDelegate::paintItem(QPainter* painter,
 
     const TrackId trackId(m_pTrackModel->getTrackId(index));
     const double scaleFactor = m_pTableView->devicePixelRatioF();
-    QPixmap pixmap = m_pCache->requestOverview(m_type,
-            m_signalColors,
+    QPixmap pixmap = m_pCache->requestCachedOverview(m_type,
             trackId,
             this,
             option.rect.size() * scaleFactor);
-    if (!pixmap.isNull()) {
+    if (pixmap.isNull()) {
+        // Cache miss
+        if (m_inhibitLazyLoading) {
+            // We are requesting cache-only covers and got a cache
+            // miss. Record this row so that when we switch to requesting
+            // non-cache we can request an update.
+            m_cacheMissIds.insert(trackId);
+        } else {
+            pixmap = m_pCache->requestUncachedOverview(m_type,
+                    m_signalColors,
+                    trackId,
+                    this,
+                    option.rect.size() * scaleFactor);
+        }
+    } else {
         // We have a cached pixmap, paint it
         pixmap.setDevicePixelRatio(scaleFactor);
         painter->drawPixmap(option.rect, pixmap);

src/library/tabledelegates/overviewdelegate.h

@@ -26,25 +26,37 @@ class OverviewDelegate : public TableItemDelegate {
             const QModelIndex& index) const final;
 
   signals:
-    void overviewReady(TrackId trackId);
-
-    void overviewChanged(TrackId trackId);
+    void overviewRowsChanged(const QList<int>& rows);
+
+  public slots:
+    // Advise the delegate to temporarily inhibit lazy loading
+    // of overview images and to only display those images
+    // that have already been cached.
+    //
+    // It is useful to handle cases when the user scroll down
+    // very fast or when they hold an arrow key. In this case
+    // it is NOT desirable to start multiple expensive file
+    // system operations for loading and scaling images that
+    // are not even displayed after scrolling beyond them.
+    void slotInhibitLazyLoading(bool inhibitLazyLoading);
 
   private slots:
     void slotTypeControlChanged(double v);
     void slotOverviewReady(const QObject* pRequester,
             const TrackId trackId,
-            bool pixmapValid,
-            const QSize resizedToSize);
+            bool pixmapValid);
+    void slotOverviewChanged(const TrackId trackId);
 
   protected:
     TrackModel* const m_pTrackModel;
 
   private:
+    void emitOverviewRowsChanged(QSet<TrackId>&& trackIds);
     OverviewCache* const m_pCache;
     mixxx::OverviewType m_type;
+    bool m_inhibitLazyLoading;
     parented_ptr<ControlProxy> m_pTypeControl;
     WaveformSignalColors m_signalColors;
 
-    mutable QHash<TrackId, int> m_trackIdToRow;
+    mutable QSet<TrackId> m_cacheMissIds;
 };

src/widget/wlibrarytableview.h

@@ -58,7 +58,7 @@ class WLibraryTableView : public QTableView, public virtual LibraryView {
     void loadTrack(TrackPointer pTrack);
     void loadTrackToPlayer(TrackPointer pTrack, const QString& group, bool play = false);
     void trackSelected(TrackPointer pTrack);
-    void onlyCachedCoverArt(bool);
+    void onlyCachedCoversAndOverviews(bool);
     void scrollValueChanged(int);
     FocusWidget setLibraryFocus(FocusWidget newFocus);
 

src/widget/wtracktableview.cpp

@@ -89,7 +89,7 @@ void WTrackTableView::enableCachedOnly() {
     if (!m_loadCachedOnly) {
         // don't try to load and search covers, drawing only
         // covers which are already in the QPixmapCache.
-        emit onlyCachedCoverArt(true);
+        emit onlyCachedCoversAndOverviews(true);
         m_loadCachedOnly = true;
     }
     m_lastUserAction = mixxx::Time::elapsed();
@@ -153,7 +153,7 @@ void WTrackTableView::slotGuiTick50ms(double /*unused*/) {
 
         // This allows CoverArtDelegate to request that we load covers from disk
         // (as opposed to only serving them from cache).
-        emit onlyCachedCoverArt(false);
+        emit onlyCachedCoversAndOverviews(false);
         m_loadCachedOnly = false;
     }
 }