From 12a8da262a2110d0181e64f4d3b5d34073f135e0 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 28 Sep 2024 17:32:53 +0200 Subject: [PATCH] OverviewDelegate: implement lazy loading like in CoverArtDelegate --- src/library/basetracktablemodel.cpp | 26 +++--- src/library/basetracktablemodel.h | 2 +- src/library/overviewcache.cpp | 31 ++++++- src/library/overviewcache.h | 10 ++- .../tabledelegates/coverartdelegate.cpp | 3 - .../tabledelegates/overviewdelegate.cpp | 82 ++++++++++++++++--- src/library/tabledelegates/overviewdelegate.h | 24 ++++-- src/widget/wlibrarytableview.h | 2 +- src/widget/wtracktableview.cpp | 4 +- 9 files changed, 140 insertions(+), 44 deletions(-) diff --git a/src/library/basetracktablemodel.cpp b/src/library/basetracktablemodel.cpp index a78b40481d9..7bb8984df34 100644 --- a/src/library/basetracktablemodel.cpp +++ b/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& 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() { diff --git a/src/library/basetracktablemodel.h b/src/library/basetracktablemodel.h index 8ee4e07190a..b3bcee169b8 100644 --- a/src/library/basetracktablemodel.h +++ b/src/library/basetracktablemodel.h @@ -249,7 +249,7 @@ class BaseTrackTableModel : public QAbstractTableModel, public TrackModel { void slotRefreshCoverRows( const QList& rows); - void slotOverviewChanged(TrackId trackId); + void slotRefreshOverviewRows(const QList& rows); void slotRefreshAllRows(); diff --git a/src/library/overviewcache.cpp b/src/library/overviewcache.cpp index 69762f4cdd8..2736028020c 100644 --- a/src/library/overviewcache.cpp +++ b/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()); } diff --git a/src/library/overviewcache.h b/src/library/overviewcache.h index 0cefb11f745..a052f211e42 100644 --- a/src/library/overviewcache.h +++ b/src/library/overviewcache.h @@ -16,7 +16,12 @@ class OverviewCache : public QObject, public Singleton { 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 { void overviewReady( const QObject* pRequester, const TrackId trackId, - bool pixmapValid, - const QSize resizedToSize); // Currently only needed for debugging + bool pixmapValid); void overviewChanged(TrackId); diff --git a/src/library/tabledelegates/coverartdelegate.cpp b/src/library/tabledelegates/coverartdelegate.cpp index 8aa27a5da99..7f8aa5feaf1 100644 --- a/src/library/tabledelegates/coverartdelegate.cpp +++ b/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) { diff --git a/src/library/tabledelegates/overviewdelegate.cpp b/src/library/tabledelegates/overviewdelegate.cpp index 9f194fd67d4..431c67ce911 100644 --- a/src/library/tabledelegates/overviewdelegate.cpp +++ b/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( QStringLiteral("[Waveform]"), @@ -83,19 +84,65 @@ void OverviewDelegate::slotTypeControlChanged(double v) { m_pTableView->update(); } +void OverviewDelegate::emitOverviewRowsChanged(QSet&& trackIds) { + if (trackIds.isEmpty()) { + return; + } + + QList rows; + for (auto id : trackIds) { + const QList 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 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}); } } +/// Maybe request repaint via dataChanged() by BaseTrackTableModel +void OverviewDelegate::slotOverviewChanged(const TrackId trackId) { + // kLogger.info() << "slotOverviewChanged()" << trackId; + emitOverviewRowsChanged(QSet{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); diff --git a/src/library/tabledelegates/overviewdelegate.h b/src/library/tabledelegates/overviewdelegate.h index 2adda2248d6..219ccba1d75 100644 --- a/src/library/tabledelegates/overviewdelegate.h +++ b/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& 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&& trackIds); OverviewCache* const m_pCache; mixxx::OverviewType m_type; + bool m_inhibitLazyLoading; parented_ptr m_pTypeControl; WaveformSignalColors m_signalColors; - mutable QHash m_trackIdToRow; + mutable QSet m_cacheMissIds; }; diff --git a/src/widget/wlibrarytableview.h b/src/widget/wlibrarytableview.h index 6a63ecc2de5..ffe24ad2fee 100644 --- a/src/widget/wlibrarytableview.h +++ b/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); diff --git a/src/widget/wtracktableview.cpp b/src/widget/wtracktableview.cpp index 8cfda9a4c01..6890717cbc1 100644 --- a/src/widget/wtracktableview.cpp +++ b/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; } }