From 451ae420c8454c3f6a157019646781fa97af3b2e Mon Sep 17 00:00:00 2001 From: Méven Car Date: Sat, 13 Jun 2026 16:11:44 +0000 Subject: Replace the itemsChanged disconnect/connect dance with a guard The updater must not react to its own setData() changes, or it would resolve the roles again. Instead of disconnecting slotItemsChanged() around every setData() and reconnecting (a dozen times), add a setModelData() helper that sets a guard while calling setData(), and make slotItemsChanged() return early while it is set. The view still gets itemsChanged and repaints. This also drops a stray disconnect of itemsMoved (never reconnected) that a hover-sequence block did while duplicating the itemsChanged connection. --- src/kitemviews/kfileitemmodelrolesupdater.cpp | 65 +++++++++++---------------- 1 file changed, 26 insertions(+), 39 deletions(-) (limited to 'src/kitemviews/kfileitemmodelrolesupdater.cpp') diff --git a/src/kitemviews/kfileitemmodelrolesupdater.cpp b/src/kitemviews/kfileitemmodelrolesupdater.cpp index 33cf6341c..6f86e1b38 100644 --- a/src/kitemviews/kfileitemmodelrolesupdater.cpp +++ b/src/kitemviews/kfileitemmodelrolesupdater.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -491,6 +492,12 @@ void KFileItemModelRolesUpdater::slotItemsChanged(const KItemRangeList &itemRang { Q_UNUSED(roles) + // Ignore changes this updater applied itself through setModelData(); they + // are already resolved and must not trigger another round of resolving. + if (m_applyingChangesToModel) { + return; + } + // Find out if slotItemsChanged() has been done recently. If that is the // case, resolving the roles is postponed until a timer has exceeded // to prevent expensive repeated updates if files are updated frequently. @@ -568,9 +575,7 @@ void KFileItemModelRolesUpdater::slotGotPreview(const KFileItem &item, const QPi data.insert("iconPixmap", transformPreviewPixmap(pixmap)); data.insert("supportsSequencing", m_previewJob->handlesSequences()); - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); Q_EMIT previewJobFinished(); // For unit testing applyResolvedRoles(index, ResolveAll, item); @@ -590,9 +595,7 @@ void KFileItemModelRolesUpdater::slotPreviewFailed(const KFileItem &item) QHash data; data.insert("iconPixmap", QPixmap()); - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); applyResolvedRoles(index, ResolveAll, item); m_finishedItems.insert(item); @@ -635,9 +638,7 @@ void KFileItemModelRolesUpdater::slotHoverSequenceGotPreview(const KFileItem &it } if (wap >= 0.0f) { data["hoverSequenceWraparoundPoint"] = wap; - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); } // For hover sequence previews we never load index 0, because that's just the regular preview @@ -652,9 +653,7 @@ void KFileItemModelRolesUpdater::slotHoverSequenceGotPreview(const KFileItem &it pixmaps.append(scaledPixmap); data["hoverSequencePixmaps"] = QVariant::fromValue(pixmaps); - disconnect(m_model, &KFileItemModel::itemsMoved, this, &KFileItemModelRolesUpdater::slotItemsMoved); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); const auto loadedIt = std::find(m_hoverSequenceLoadedItems.begin(), m_hoverSequenceLoadedItems.end(), item); if (loadedIt == m_hoverSequenceLoadedItems.end()) { @@ -793,13 +792,11 @@ void KFileItemModelRolesUpdater::resolveNextPendingRoles() data.insert("iconPixmap", QPixmap()); data.insert("hoverSequencePixmaps", QVariant::fromValue(QVector())); - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); for (int index = 0; index <= m_model->count(); ++index) { if (m_model->data(index).contains("iconPixmap") || m_model->data(index).contains("hoverSequencePixmaps")) { - m_model->setData(index, data); + setModelData(index, data); } } - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); } m_clearPreviews = false; } @@ -856,10 +853,8 @@ void KFileItemModelRolesUpdater::applyChangedBalooRolesForItem(const KFileItem & data.insert(it.key(), it.value()); } - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); const int index = m_model->index(item); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); #else #ifndef Q_CC_MSVC Q_UNUSED(item) @@ -885,9 +880,7 @@ void KFileItemModelRolesUpdater::slotDirectoryContentsCountReceived(const QStrin data.insert("isExpandable", count > 0); } - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); } } } @@ -1067,9 +1060,7 @@ void KFileItemModelRolesUpdater::loadNextHoverSequencePreview() if (!data.contains("hoverSequencePixmaps")) { // The pixmap at index 0 isn't used ("iconPixmap" will be used instead) data.insert("hoverSequencePixmaps", QVariant::fromValue(QVector() << QPixmap())); - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); } const QVector pixmaps = data["hoverSequencePixmaps"].value>(); @@ -1189,6 +1180,12 @@ void KFileItemModelRolesUpdater::updateChangedItems() } } +void KFileItemModelRolesUpdater::setModelData(int index, const QHash &data) +{ + const QScopedValueRollback guard(m_applyingChangesToModel, true); + m_model->setData(index, data); +} + void KFileItemModelRolesUpdater::applySortRole(int index) { QHash data; @@ -1208,9 +1205,7 @@ void KFileItemModelRolesUpdater::applySortRole(int index) data = rolesData(item, index); } - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); } void KFileItemModelRolesUpdater::applySortProgressToModel() @@ -1253,9 +1248,7 @@ bool KFileItemModelRolesUpdater::applyResolvedRoles(int index, ResolveHint hint, data.insert("hoverSequencePixmaps", QVariant::fromValue(QVector())); } - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); return true; } @@ -1292,9 +1285,7 @@ void KFileItemModelRolesUpdater::startDirectorySizeCounting(const KFileItem &ite data.insert("size", -2); // invalid size, -1 means size unknown - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); // Use KIO flags to exclude dot entries at the source to simplify counting KIO::ListJob::ListFlags flags = KIO::ListJob::ListFlag::ExcludeDotAndDotDot; @@ -1342,9 +1333,7 @@ void KFileItemModelRolesUpdater::startDirectorySizeCounting(const KFileItem &ite } if (!newData.isEmpty()) { - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); - m_model->setData(index, newData); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, newData); } }); return; @@ -1522,11 +1511,9 @@ void KFileItemModelRolesUpdater::trimHoverSequenceLoadedItems() void KFileItemModelRolesUpdater::resetSizeData(const int index, const int size) { - disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); auto data = m_model->data(index); data.insert("size", size); - m_model->setData(index, data); - connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + setModelData(index, data); } void KFileItemModelRolesUpdater::recountDirectoryItems(const QList &directories) -- cgit v1.3.1