Fix performance issues when handling layout changed in Quick item views

When the layout changes, we mark all rows as changed but do not track
where the individual rows get moved.

The only reason why one would want to track the moves is to persist
the current item selection across a layout change. But even the previous
code did not achieve that. I'll create a follow up patch to this one
that also implements this behavior as seen in Qt Widget item views.

Note that removing this code brings a tremendous performance win
on larger models. The repeated calls to _q_itemsMoved triggered O(n^2)
behavior in the number of top items in the model. Even with "only"
tens of thousands of items in the model, a layout change became very
costly and took seconds on a beefy modern desktop machine.

Calling _q_itemsMoved in a loop is bad because it:

- leads to O(N^2) behavior within QQmlChangeSet when merging the small
  moves into the item view's current change set
- potentially triggers tons of binding/property updates when the cached
  model indices are updated in _q_itemsMoved

Removing this slow path, I did not yet find a behavior change to the
previous code. Instead, it just does it all much faster.

Change-Id: I67fa99a1c5d8e05d17497d29391da9458bd9bdd0
Task-number: QTBUG-51638
Task-number: QTBUG-45674
Task-number: QTBUG-53677
Reviewed-by: Daniel Vrátil <daniel.vratil@kdab.com>
Reviewed-by: Robin Burchell <robin.burchell@viroteck.net>
(cherry picked from 84f61dd2d2b0140814b39a2c5238a6e31c49abd7)
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

4 files changed, 1 insertions, 43 deletions

diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp
index 21f89cd4ec..4a2cd57f55 100644
--- a/src/qml/types/qqmldelegatemodel.cpp
+++ b/src/qml/types/qqmldelegatemodel.cpp
@@ -1567,29 +1567,6 @@ bool QQmlDelegateModel::isDescendantOf(const QPersistentModelIndex& desc, const
     return false;
 }
 
-void QQmlDelegateModel::_q_layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint)
-{
-    Q_D(QQmlDelegateModel);
-    if (!d->m_complete)
-        return;
-
-    if (hint == QAbstractItemModel::VerticalSortHint) {
-        d->m_storedPersistentIndexes.clear();
-        if (!parents.isEmpty() && d->m_adaptorModel.rootIndex.isValid() &&  !isDescendantOf(d->m_adaptorModel.rootIndex, parents)) {
-            return;
-        }
-
-        for (int i = 0; i < d->m_count; ++i) {
-            const QModelIndex index = d->m_adaptorModel.aim()->index(i, 0, d->m_adaptorModel.rootIndex);
-            d->m_storedPersistentIndexes.append(index);
-        }
-    } else if (hint == QAbstractItemModel::HorizontalSortHint) {
-        // Ignored
-    } else {
-        // Triggers model reset, no preparations for that are needed
-    }
-}
-
 void QQmlDelegateModel::_q_layoutChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint)
 {
     Q_D(QQmlDelegateModel);
@@ -1601,19 +1578,7 @@ void QQmlDelegateModel::_q_layoutChanged(const QList<QPersistentModelIndex> &par
             return;
         }
 
-        for (int i = 0, c = d->m_storedPersistentIndexes.count(); i < c; ++i) {
-            const QPersistentModelIndex &index = d->m_storedPersistentIndexes.at(i);
-            if (i == index.row())
-                continue;
-
-            _q_itemsMoved(i, index.row(), 1);
-        }
-
-        d->m_storedPersistentIndexes.clear();
-
-        // layoutUpdate does not necessarily have any move changes, but it can
-        // also mean data changes. We can't detect what exactly has changed, so
-        // just emit it for all items
+        // mark all items as changed
         _q_itemsChanged(0, d->m_count, QVector<int>());
 
     } else if (hint == QAbstractItemModel::HorizontalSortHint) {
diff --git a/src/qml/types/qqmldelegatemodel_p.h b/src/qml/types/qqmldelegatemodel_p.h
index 6af052c1b4..6032d6be03 100644
--- a/src/qml/types/qqmldelegatemodel_p.h
+++ b/src/qml/types/qqmldelegatemodel_p.h
@@ -140,7 +140,6 @@ private Q_SLOTS:
     void _q_rowsRemoved(const QModelIndex &,int,int);
     void _q_rowsMoved(const QModelIndex &, int, int, const QModelIndex &, int);
     void _q_dataChanged(const QModelIndex&,const QModelIndex&,const QVector<int> &);
-    void _q_layoutAboutToBeChanged(const QList<QPersistentModelIndex>&, QAbstractItemModel::LayoutChangeHint);
     void _q_layoutChanged(const QList<QPersistentModelIndex>&, QAbstractItemModel::LayoutChangeHint);
 
 private:
diff --git a/src/qml/types/qqmldelegatemodel_p_p.h b/src/qml/types/qqmldelegatemodel_p_p.h
index 3a19163cbd..338348e5e3 100644
--- a/src/qml/types/qqmldelegatemodel_p_p.h
+++ b/src/qml/types/qqmldelegatemodel_p_p.h
@@ -331,8 +331,6 @@ public:
         };
         QQmlDelegateModelGroup *m_groups[Compositor::MaximumGroupCount];
     };
-
-    QList<QPersistentModelIndex> m_storedPersistentIndexes;
 };
 
 class QQmlPartsModel : public QQmlInstanceModel, public QQmlDelegateModelGroupEmitter
diff --git a/src/qml/util/qqmladaptormodel.cpp b/src/qml/util/qqmladaptormodel.cpp
index c61144dd8f..bf160fc3a6 100644
--- a/src/qml/util/qqmladaptormodel.cpp
+++ b/src/qml/util/qqmladaptormodel.cpp
@@ -464,8 +464,6 @@ public:
                                 vdm, SLOT(_q_rowsMoved(QModelIndex,int,int,QModelIndex,int)));
             QObject::disconnect(aim, SIGNAL(modelReset()),
                                 vdm, SLOT(_q_modelReset()));
-            QObject::disconnect(aim, SIGNAL(layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
-                                vdm, SLOT(_q_layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
             QObject::disconnect(aim, SIGNAL(layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
                                 vdm, SLOT(_q_layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
         }
@@ -922,8 +920,6 @@ void QQmlAdaptorModel::setModel(const QVariant &variant, QQmlDelegateModel *vdm,
                               vdm, QQmlDelegateModel, SLOT(_q_rowsMoved(QModelIndex,int,int,QModelIndex,int)));
             qmlobject_connect(model, QAbstractItemModel, SIGNAL(modelReset()),
                               vdm, QQmlDelegateModel, SLOT(_q_modelReset()));
-            qmlobject_connect(model, QAbstractItemModel, SIGNAL(layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
-                              vdm, QQmlDelegateModel, SLOT(_q_layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
             qmlobject_connect(model, QAbstractItemModel, SIGNAL(layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
                               vdm, QQmlDelegateModel, SLOT(_q_layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
         } else {