From 1379c77fce38a81d80bb76d2266e3886b42c2cdd Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 18 Jan 2026 10:37:12 +0100 Subject: [PATCH] Tracks: avoid re-sorting table after track removal (hide, remove, purge) Implemented for track models that don't have a position column (playlists) as that still requires rebuilding the row cache --- src/library/basesqltablemodel.cpp | 54 ++++++++++++++++++- src/library/basesqltablemodel.h | 1 + src/library/basetracktablemodel.cpp | 8 ++- src/library/basetracktablemodel.h | 2 + src/library/dao/trackdao.cpp | 4 +- src/library/dao/trackdao.h | 1 + src/library/trackcollection.cpp | 2 +- src/library/trackmodel.h | 2 + .../trackset/crate/cratetablemodel.cpp | 4 +- 9 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/library/basesqltablemodel.cpp b/src/library/basesqltablemodel.cpp index 8f35c7bd0425..4d5fdaf853a4 100644 --- a/src/library/basesqltablemodel.cpp +++ b/src/library/basesqltablemodel.cpp @@ -844,7 +844,59 @@ void BaseSqlTableModel::hideTracks(const QModelIndexList& indices) { // TODO(rryan) : do not select, instead route event to BTC and notify from // there. - select(); //Repopulate the data model. + + QSet tracksRemovedSet = QSet(trackIds.begin(), trackIds.end()); + removeTrackRows(tracksRemovedSet); +} + +void BaseSqlTableModel::removeTrackRows(const QSet& trackIdsToRemove) { + // This is called after hiding, purging or removing tracks from a track set. + // The purpose is to keep the tracks view constant after after these + // operations by avoiding pointless/confusing re-sorting of the model, + // which happens when we call select(), which rebuilds the row cache. + // We assume metadata of remaining tracks hasn't changed, we can simply + // remove the respective track rows. + + // However, if this is a playlist model, track removal likely also changes + // the tracks' positions in the playlist. We need to get the tracks' new + // positions from the database, so let's do a select(). + // FIXME Update position without re-sorting + if (hasPositionColumn()) { + select(); + return; + } + + // For other models we can now remove all track rows. + QVector rowInfos = m_rowInfo; + TrackId2Rows trackIdToRows; + TrackPos2Row trackPosToRows; // remains empty + + QMutableListIterator it(rowInfos); + while (it.hasNext()) { + const RowInfo& rowInfo = it.next(); + if (trackIdsToRemove.contains(rowInfo.trackId)) { + it.remove(); + } + } + + // Recreate TrackId2Rows, taken from select() + trackIdToRows.reserve(rowInfos.size()); + for (int i = 0; i < rowInfos.size(); ++i) { + const RowInfo& rowInfo = rowInfos[i]; + if (rowInfo.row == -1) { + // We've reached the end of valid rows. Resize rowInfo to cut off + // this and all further elements. + rowInfos.resize(i); + break; + } + trackIdToRows[rowInfo.trackId].push_back(i); + } + + clearRows(); + replaceRows( + std::move(rowInfos), + std::move(trackIdToRows), + std::move(trackPosToRows)); } QList BaseSqlTableModel::getTrackRefs( diff --git a/src/library/basesqltablemodel.h b/src/library/basesqltablemodel.h index 07926513f8bf..b327fef7d30e 100644 --- a/src/library/basesqltablemodel.h +++ b/src/library/basesqltablemodel.h @@ -67,6 +67,7 @@ class BaseSqlTableModel : public BaseTrackTableModel { int columnIndexFromSortColumnId(TrackModel::SortColumnId sortColumn) const override; void hideTracks(const QModelIndexList& indices) override; + void removeTrackRows(const QSet& trackIdsToRemove) override; void select() override; diff --git a/src/library/basetracktablemodel.cpp b/src/library/basetracktablemodel.cpp index bef134cddc5b..093d8f6d96ff 100644 --- a/src/library/basetracktablemodel.cpp +++ b/src/library/basetracktablemodel.cpp @@ -107,9 +107,9 @@ BaseTrackTableModel::BaseTrackTableModel( m_trackPlayedColor(QColor(WTrackTableView::kDefaultTrackPlayedColor)), m_trackMissingColor(QColor(WTrackTableView::kDefaultTrackMissingColor)) { connect(&pTrackCollectionManager->internalCollection()->getTrackDAO(), - &TrackDAO::forceModelUpdate, + &TrackDAO::tracksRemoved, this, - &BaseTrackTableModel::slotRefreshAllRows); + &BaseTrackTableModel::slotTracksRemoved); connect(&PlayerInfo::instance(), &PlayerInfo::trackChanged, this, @@ -989,6 +989,10 @@ void BaseTrackTableModel::slotRefreshAllRows() { select(); } +void BaseTrackTableModel::slotTracksRemoved(const QSet& trackIds) { + removeTrackRows(trackIds); +} + void BaseTrackTableModel::emitDataChangedForMultipleRowsInColumn( const QList& rows, int column, diff --git a/src/library/basetracktablemodel.h b/src/library/basetracktablemodel.h index 7947e783b4ed..02cd41af3b4c 100644 --- a/src/library/basetracktablemodel.h +++ b/src/library/basetracktablemodel.h @@ -248,6 +248,8 @@ class BaseTrackTableModel : public QAbstractTableModel, public TrackModel { void slotRefreshAllRows(); + void slotTracksRemoved(const QSet& trackIds); + void slotCoverFound( const QObject* pRequester, const CoverInfo& coverInfo, diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index cdd5ba2a4440..437c14ab7beb 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -1111,9 +1111,9 @@ void TrackDAO::afterPurgingTracks( #else QSet tracksRemovedSet = QSet::fromList(trackIds); #endif + // Notify BaseTrackCache it should remove tracks and track models + // that they should update their cache as well. emit tracksRemoved(tracksRemovedSet); - // notify trackmodels that they should update their cache as well. - emit forceModelUpdate(); } namespace { diff --git a/src/library/dao/trackdao.h b/src/library/dao/trackdao.h index 16412f1abf98..88037a152a89 100644 --- a/src/library/dao/trackdao.h +++ b/src/library/dao/trackdao.h @@ -116,6 +116,7 @@ class TrackDAO : public QObject, public virtual DAO, public virtual GlobalTrackC void progressVerifyTracksOutside(const QString& path); void progressCoverArt(const QString& file); void forceModelUpdate(); + void removeTrackRows(const QSet& trackIds); public slots: // Slots to inform the TrackDAO about changes that diff --git a/src/library/trackcollection.cpp b/src/library/trackcollection.cpp index 55a85efe6f13..d3f551a4599c 100644 --- a/src/library/trackcollection.cpp +++ b/src/library/trackcollection.cpp @@ -41,7 +41,7 @@ TrackCollection::TrackCollection( connect(&m_trackDao, &TrackDAO::tracksRemoved, this, - &TrackCollection::tracksRemoved, + &TrackCollection::tracksRemoved, // unused /*signal-to-signal*/ Qt::DirectConnection); connect(&m_trackDao, &TrackDAO::forceModelUpdate, diff --git a/src/library/trackmodel.h b/src/library/trackmodel.h index 85d212a15e3c..4c6e60932319 100644 --- a/src/library/trackmodel.h +++ b/src/library/trackmodel.h @@ -243,6 +243,8 @@ class TrackModel { virtual void select() { } + virtual void removeTrackRows(const QSet&) {}; + /// This is an interface to stop any potentially running /// model population when switching models in WTrackTableView. /// Only implemented in ProxyTrackModel. diff --git a/src/library/trackset/crate/cratetablemodel.cpp b/src/library/trackset/crate/cratetablemodel.cpp index b660fa159ae2..3d95a764a793 100644 --- a/src/library/trackset/crate/cratetablemodel.cpp +++ b/src/library/trackset/crate/cratetablemodel.cpp @@ -218,7 +218,9 @@ void CrateTableModel::removeTracks(const QModelIndexList& indices) { return; } - select(); + // Now remove the track rows + QSet tracksRemovedSet = QSet(trackIds.begin(), trackIds.end()); + removeTrackRows(tracksRemovedSet); } QString CrateTableModel::modelKey(bool noSearch) const {