Implement proper support for layoutChange in QQmlDelegateModel

Current implementation is treating model layoutChange the same way as modelReset,
which causes problems when using ListView on top of a QSortFilterProxyModel. The
model emits layoutChanged whenever a data within sorting column change. Treating
it as modelReset leads to poor performance on large models and caused UI issues,
because the scrolling position is reset every time.

This patch implements proper handling for layoutChanged signals by first handling
all items moves and then simulating dataChange for all items.

This fixes regression from Qt 5.1 introduced by Change I16b859d9

Task-number: QTBUG-37983
Task-number: QTBUG-34391
Change-Id: I6d3873b7b87e7f0e8fc0c1ed5dc80c6f8fdf6c22
Reviewed-by: Albert Astals Cid <albert.astals@canonical.com>
Reviewed-by: Michael Zanetti <michael.zanetti@canonical.com>
Reviewed-by: Alan Alpert <aalpert@blackberry.com>
Reviewed-by: Gunnar Sletta <gunnar.sletta@jollamobile.com>

diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp
index be479ee..4591d42 100644 (file)
--- a/src/qml/types/qqmldelegatemodel.cpp
+++ b/src/qml/types/qqmldelegatemodel.cpp
@@ -1521,9 +1521,64 @@ void QQmlDelegateModel::_q_dataChanged(const QModelIndex &begin, const QModelInd
         _q_itemsChanged(begin.row(), end.row() - begin.row() + 1, roles);
 }
 
-void QQmlDelegateModel::_q_layoutChanged()
+void QQmlDelegateModel::_q_layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint)
 {
-    _q_modelReset();
+    Q_D(QQmlDelegateModel);
+    if (!d->m_complete)
+        return;
+
+    if (hint == QAbstractItemModel::VerticalSortHint) {
+        d->m_storedPersistentIndexes.clear();
+        if (!parents.contains(d->m_adaptorModel.rootIndex))
+            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);
+    if (!d->m_complete)
+        return;
+
+    if (hint == QAbstractItemModel::VerticalSortHint) {
+        if (!parents.contains(d->m_adaptorModel.rootIndex))
+            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;
+
+            QVector<Compositor::Insert> inserts;
+            QVector<Compositor::Remove> removes;
+            d->m_compositor.listItemsMoved(&d->m_adaptorModel, i, index.row(), 1, &removes, &inserts);
+            if (!removes.isEmpty() || !inserts.isEmpty()) {
+                d->itemsMoved(removes, inserts);
+            }
+        }
+
+        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
+        _q_itemsChanged(0, d->m_count, QVector<int>());
+
+    } else if (hint == QAbstractItemModel::HorizontalSortHint) {
+        // Ignored
+    } else {
+        // We don't know what's going on, so reset the model
+        _q_modelReset();
+    }
 }
 
 QQmlDelegateModelAttached *QQmlDelegateModel::qmlAttachedProperties(QObject *obj)

diff --git a/src/qml/types/qqmldelegatemodel_p.h b/src/qml/types/qqmldelegatemodel_p.h
index 51f846e..0b67179 100644 (file)
--- a/src/qml/types/qqmldelegatemodel_p.h
+++ b/src/qml/types/qqmldelegatemodel_p.h
@@ -139,7 +139,8 @@ 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_layoutChanged();
+    void _q_layoutAboutToBeChanged(const QList<QPersistentModelIndex>&, QAbstractItemModel::LayoutChangeHint);
+    void _q_layoutChanged(const QList<QPersistentModelIndex>&, QAbstractItemModel::LayoutChangeHint);
 
 private:
     Q_DISABLE_COPY(QQmlDelegateModel)

diff --git a/src/qml/types/qqmldelegatemodel_p_p.h b/src/qml/types/qqmldelegatemodel_p_p.h
index 7da089c..d64f641 100644 (file)
--- a/src/qml/types/qqmldelegatemodel_p_p.h
+++ b/src/qml/types/qqmldelegatemodel_p_p.h
@@ -331,6 +331,8 @@ 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 d38e5ac..c1c8bfa 100644 (file)
--- a/src/qml/util/qqmladaptormodel.cpp
+++ b/src/qml/util/qqmladaptormodel.cpp
@@ -467,8 +467,10 @@ public:
                                 vdm, SLOT(_q_rowsMoved(QModelIndex,int,int,QModelIndex,int)));
             QObject::disconnect(aim, SIGNAL(modelReset()),
                                 vdm, SLOT(_q_modelReset()));
-            QObject::disconnect(aim, SIGNAL(layoutChanged()),
-                                vdm, SLOT(_q_layoutChanged()));
+            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)));
         }
 
         const_cast<VDMAbstractItemModelDataType *>(this)->release();
@@ -920,8 +922,10 @@ 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(layoutChanged()),
-                              vdm, QQmlDelegateModel, SLOT(_q_layoutChanged()));
+            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 {
             accessors = new VDMObjectDelegateDataType;
         }