Notify about moves in the source model more efficiently.

Change-Id: I5ea2a2dddc1b39a3d2b405bda815f42df7c07c75
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>

4 files changed, 256 insertions, 21 deletions

diff --git a/src/widgets/itemviews/qsortfilterproxymodel.cpp b/src/widgets/itemviews/qsortfilterproxymodel.cpp
index 74999de426..90f5719782 100644
--- a/src/widgets/itemviews/qsortfilterproxymodel.cpp
+++ b/src/widgets/itemviews/qsortfilterproxymodel.cpp
@@ -218,6 +218,12 @@ public:
                                        int start, int end);
     void _q_sourceRowsRemoved(const QModelIndex &source_parent,
                               int start, int end);
+    void _q_sourceRowsAboutToBeMoved(const QModelIndex &sourceParent,
+                                     int sourceStart, int sourceEnd,
+                                     const QModelIndex &destParent, int dest);
+    void _q_sourceRowsMoved(const QModelIndex &sourceParent,
+                            int sourceStart, int sourceEnd,
+                            const QModelIndex &destParent, int dest);
     void _q_sourceColumnsAboutToBeInserted(const QModelIndex &source_parent,
                                            int start, int end);
     void _q_sourceColumnsInserted(const QModelIndex &source_parent,
@@ -226,6 +232,12 @@ public:
                                           int start, int end);
     void _q_sourceColumnsRemoved(const QModelIndex &source_parent,
                                  int start, int end);
+    void _q_sourceColumnsAboutToBeMoved(const QModelIndex &sourceParent,
+                                        int sourceStart, int sourceEnd,
+                                        const QModelIndex &destParent, int dest);
+    void _q_sourceColumnsMoved(const QModelIndex &sourceParent,
+                               int sourceStart, int sourceEnd,
+                               const QModelIndex &destParent, int dest);
 
     void _q_clearMapping();
 
@@ -1299,6 +1311,54 @@ void QSortFilterProxyModelPrivate::_q_sourceRowsRemoved(
     source_items_removed(source_parent, start, end, Qt::Vertical);
 }
 
+void QSortFilterProxyModelPrivate::_q_sourceRowsAboutToBeMoved(
+    const QModelIndex &sourceParent, int sourceStart, int sourceEnd, const QModelIndex &destParent, int dest)
+{
+    Q_Q(QSortFilterProxyModel);
+    // Because rows which are contiguous in the source model might not be contiguous
+    // in the proxy due to sorting, the best thing we can do here is be specific about what
+    // parents are having their children changed.
+    // Optimize: Emit move signals if the proxy is not sorted. Will need to account for rows
+    // being filtered out though.
+
+    saved_persistent_indexes.clear();
+
+    QList<QPersistentModelIndex> parents;
+    parents << q->mapFromSource(sourceParent);
+    if (sourceParent != destParent)
+      parents << q->mapFromSource(destParent);
+    emit q->layoutAboutToBeChanged(parents);
+    if (persistent.indexes.isEmpty())
+        return;
+    saved_persistent_indexes = store_persistent_indexes();
+}
+
+void QSortFilterProxyModelPrivate::_q_sourceRowsMoved(
+    const QModelIndex &sourceParent, int sourceStart, int sourceEnd, const QModelIndex &destParent, int dest)
+{
+    Q_Q(QSortFilterProxyModel);
+
+    // Optimize: We only need to clear and update the persistent indexes which are children of
+    // sourceParent or destParent
+    qDeleteAll(source_index_mapping);
+    source_index_mapping.clear();
+
+    update_persistent_indexes(saved_persistent_indexes);
+    saved_persistent_indexes.clear();
+
+    if (dynamic_sortfilter && update_source_sort_column()) {
+        //update_source_sort_column might have created wrong mapping so we have to clear it again
+        qDeleteAll(source_index_mapping);
+        source_index_mapping.clear();
+    }
+
+    QList<QPersistentModelIndex> parents;
+    parents << q->mapFromSource(sourceParent);
+    if (sourceParent != destParent)
+      parents << q->mapFromSource(destParent);
+    emit q->layoutChanged(parents);
+}
+
 void QSortFilterProxyModelPrivate::_q_sourceColumnsAboutToBeInserted(
     const QModelIndex &source_parent, int start, int end)
 {
@@ -1355,6 +1415,47 @@ void QSortFilterProxyModelPrivate::_q_sourceColumnsRemoved(
     proxy_sort_column = q->mapFromSource(model->index(0,source_sort_column, source_parent)).column();
 }
 
+void QSortFilterProxyModelPrivate::_q_sourceColumnsAboutToBeMoved(
+    const QModelIndex &sourceParent, int sourceStart, int sourceEnd, const QModelIndex &destParent, int dest)
+{
+    Q_Q(QSortFilterProxyModel);
+
+    saved_persistent_indexes.clear();
+
+    QList<QPersistentModelIndex> parents;
+    parents << q->mapFromSource(sourceParent);
+    if (sourceParent != destParent)
+      parents << q->mapFromSource(destParent);
+    emit q->layoutAboutToBeChanged(parents);
+
+    if (persistent.indexes.isEmpty())
+        return;
+    saved_persistent_indexes = store_persistent_indexes();
+}
+
+void QSortFilterProxyModelPrivate::_q_sourceColumnsMoved(
+    const QModelIndex &sourceParent, int sourceStart, int sourceEnd, const QModelIndex &destParent, int dest)
+{
+    Q_Q(QSortFilterProxyModel);
+
+    qDeleteAll(source_index_mapping);
+    source_index_mapping.clear();
+
+    update_persistent_indexes(saved_persistent_indexes);
+    saved_persistent_indexes.clear();
+
+    if (dynamic_sortfilter && update_source_sort_column()) {
+        qDeleteAll(source_index_mapping);
+        source_index_mapping.clear();
+    }
+
+    QList<QPersistentModelIndex> parents;
+    parents << q->mapFromSource(sourceParent);
+    if (sourceParent != destParent)
+      parents << q->mapFromSource(destParent);
+    emit q->layoutChanged(parents);
+}
+
 /*!
     \since 4.1
     \class QSortFilterProxyModel
@@ -1576,22 +1677,22 @@ void QSortFilterProxyModel::setSourceModel(QAbstractItemModel *sourceModel)
     disconnect(d->model, SIGNAL(columnsRemoved(QModelIndex,int,int)),
                this, SLOT(_q_sourceColumnsRemoved(QModelIndex,int,int)));
 
-    disconnect(d->model, SIGNAL(layoutAboutToBeChanged()),
-               this, SLOT(_q_sourceLayoutAboutToBeChanged()));
-
-    disconnect(d->model, SIGNAL(layoutChanged()),
-               this, SLOT(_q_sourceLayoutChanged()));
-
     disconnect(d->model, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)),
-               this, SLOT(_q_sourceLayoutAboutToBeChanged()));
+               this, SLOT(_q_sourceRowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)));
 
     disconnect(d->model, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)),
-               this, SLOT(_q_sourceLayoutChanged()));
+               this, SLOT(_q_sourceRowsMoved(QModelIndex,int,int,QModelIndex,int)));
 
     disconnect(d->model, SIGNAL(columnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)),
-               this, SLOT(_q_sourceLayoutAboutToBeChanged()));
+               this, SLOT(_q_sourceColumnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)));
 
     disconnect(d->model, SIGNAL(columnsMoved(QModelIndex,int,int,QModelIndex,int)),
+               this, SLOT(_q_sourceColumnsMoved(QModelIndex,int,int,QModelIndex,int)));
+
+    disconnect(d->model, SIGNAL(layoutAboutToBeChanged()),
+               this, SLOT(_q_sourceLayoutAboutToBeChanged()));
+
+    disconnect(d->model, SIGNAL(layoutChanged()),
                this, SLOT(_q_sourceLayoutChanged()));
 
     disconnect(d->model, SIGNAL(modelAboutToBeReset()), this, SLOT(_q_sourceAboutToBeReset()));
@@ -1629,22 +1730,22 @@ void QSortFilterProxyModel::setSourceModel(QAbstractItemModel *sourceModel)
     connect(d->model, SIGNAL(columnsRemoved(QModelIndex,int,int)),
             this, SLOT(_q_sourceColumnsRemoved(QModelIndex,int,int)));
 
-    connect(d->model, SIGNAL(layoutAboutToBeChanged()),
-            this, SLOT(_q_sourceLayoutAboutToBeChanged()));
-
-    connect(d->model, SIGNAL(layoutChanged()),
-            this, SLOT(_q_sourceLayoutChanged()));
-
     connect(d->model, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)),
-            this, SLOT(_q_sourceLayoutAboutToBeChanged()));
+            this, SLOT(_q_sourceRowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)));
 
     connect(d->model, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)),
-            this, SLOT(_q_sourceLayoutChanged()));
+            this, SLOT(_q_sourceRowsMoved(QModelIndex,int,int,QModelIndex,int)));
 
     connect(d->model, SIGNAL(columnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)),
-            this, SLOT(_q_sourceLayoutAboutToBeChanged()));
+            this, SLOT(_q_sourceColumnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)));
 
     connect(d->model, SIGNAL(columnsMoved(QModelIndex,int,int,QModelIndex,int)),
+            this, SLOT(_q_sourceColumnsMoved(QModelIndex,int,int,QModelIndex,int)));
+
+    connect(d->model, SIGNAL(layoutAboutToBeChanged()),
+            this, SLOT(_q_sourceLayoutAboutToBeChanged()));
+
+    connect(d->model, SIGNAL(layoutChanged()),
             this, SLOT(_q_sourceLayoutChanged()));
 
     connect(d->model, SIGNAL(modelAboutToBeReset()), this, SLOT(_q_sourceAboutToBeReset()));
diff --git a/src/widgets/itemviews/qsortfilterproxymodel.h b/src/widgets/itemviews/qsortfilterproxymodel.h
index 68d92be841..1ce84ec11e 100644
--- a/src/widgets/itemviews/qsortfilterproxymodel.h
+++ b/src/widgets/itemviews/qsortfilterproxymodel.h
@@ -185,10 +185,14 @@ private:
     Q_PRIVATE_SLOT(d_func(), void _q_sourceRowsInserted(const QModelIndex &source_parent, int start, int end))
     Q_PRIVATE_SLOT(d_func(), void _q_sourceRowsAboutToBeRemoved(const QModelIndex &source_parent, int start, int end))
     Q_PRIVATE_SLOT(d_func(), void _q_sourceRowsRemoved(const QModelIndex &source_parent, int start, int end))
+    Q_PRIVATE_SLOT(d_func(), void _q_sourceRowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int))
+    Q_PRIVATE_SLOT(d_func(), void _q_sourceRowsMoved(QModelIndex,int,int,QModelIndex,int))
     Q_PRIVATE_SLOT(d_func(), void _q_sourceColumnsAboutToBeInserted(const QModelIndex &source_parent, int start, int end))
     Q_PRIVATE_SLOT(d_func(), void _q_sourceColumnsInserted(const QModelIndex &source_parent, int start, int end))
     Q_PRIVATE_SLOT(d_func(), void _q_sourceColumnsAboutToBeRemoved(const QModelIndex &source_parent, int start, int end))
     Q_PRIVATE_SLOT(d_func(), void _q_sourceColumnsRemoved(const QModelIndex &source_parent, int start, int end))
+    Q_PRIVATE_SLOT(d_func(), void _q_sourceColumnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int))
+    Q_PRIVATE_SLOT(d_func(), void _q_sourceColumnsMoved(QModelIndex,int,int,QModelIndex,int))
     Q_PRIVATE_SLOT(d_func(), void _q_clearMapping())
 };
 
diff --git a/tests/auto/other/modeltest/tst_modeltest.cpp b/tests/auto/other/modeltest/tst_modeltest.cpp
index 434537a81f..e5c227d167 100644
--- a/tests/auto/other/modeltest/tst_modeltest.cpp
+++ b/tests/auto/other/modeltest/tst_modeltest.cpp
@@ -207,8 +207,8 @@ public:
     , storePersistentFailureCount(0)
     , checkPersistentFailureCount(0)
     {
-        connect(m_proxy, SIGNAL(layoutAboutToBeChanged()), SLOT(storePersistent()));
-        connect(m_proxy, SIGNAL(layoutChanged()), SLOT(checkPersistent()));
+        connect(m_proxy, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), SLOT(storePersistent()));
+        connect(m_proxy, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)), SLOT(checkPersistent()));
     }
 
 public slots:
@@ -235,7 +235,7 @@ public slots:
 
     void storePersistent()
     {
-        // This method is called from layoutAboutToBeChanged. Persistent indexes should be valid
+        // This method is called from rowsAboutToBeMoved. Persistent indexes should be valid
         foreach(const QModelIndex &idx, m_persistentProxyIndexes)
             if (!idx.isValid()) {
                 qWarning("%s: persistentProxyIndexes contains invalid index", Q_FUNC_INFO);
diff --git a/tests/auto/widgets/itemviews/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/widgets/itemviews/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
index fb5fe579a6..1d4bb5616f 100644
--- a/tests/auto/widgets/itemviews/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/widgets/itemviews/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
@@ -148,6 +148,7 @@ private slots:
     void filteredColumns();
 
     void testParentLayoutChanged();
+    void moveSourceRows();
 
 protected:
     void buildHierarchy(const QStringList &data, QAbstractItemModel *model);
@@ -3276,5 +3277,134 @@ void tst_QSortFilterProxyModel::testParentLayoutChanged()
 
 }
 
+class SignalArgumentChecker : public QObject
+{
+    Q_OBJECT
+public:
+    SignalArgumentChecker(QAbstractItemModel *model, QAbstractProxyModel *proxy, QObject *parent = 0)
+      : QObject(parent), m_model(model), m_proxy(proxy)
+    {
+        connect(model, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), SLOT(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)));
+        connect(model, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)), SLOT(rowsMoved(QModelIndex,int,int,QModelIndex,int)));
+        connect(proxy, SIGNAL(layoutAboutToBeChanged(QList<QPersistentModelIndex>)), SLOT(layoutAboutToBeChanged(QList<QPersistentModelIndex>)));
+        connect(proxy, SIGNAL(layoutChanged(QList<QPersistentModelIndex>)), SLOT(layoutChanged(QList<QPersistentModelIndex>)));
+    }
+
+private slots:
+    void rowsAboutToBeMoved(const QModelIndex &source, int, int, const QModelIndex &destination, int)
+    {
+        m_p1PersistentBefore = source;
+        m_p2PersistentBefore = destination;
+        m_p2FirstProxyChild = m_proxy->index(0, 0, m_proxy->mapFromSource(destination));
+    }
+
+    void rowsMoved(const QModelIndex &source, int, int, const QModelIndex &destination, int)
+    {
+        m_p1PersistentAfter = source;
+        m_p2PersistentAfter = destination;
+    }
+
+    void layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents)
+    {
+        QVERIFY(m_p1PersistentBefore.isValid());
+        QVERIFY(m_p2PersistentBefore.isValid());
+        QCOMPARE(parents.size(), 2);
+        QVERIFY(parents.first() != parents.at(1));
+        QVERIFY(parents.contains(m_proxy->mapFromSource(m_p1PersistentBefore)));
+        QVERIFY(parents.contains(m_proxy->mapFromSource(m_p2PersistentBefore)));
+    }
+
+    void layoutChanged(const QList<QPersistentModelIndex> &parents)
+    {
+        QVERIFY(m_p1PersistentAfter.isValid());
+        QVERIFY(m_p2PersistentAfter.isValid());
+        QCOMPARE(parents.size(), 2);
+        QVERIFY(parents.first() != parents.at(1));
+        QVERIFY(parents.contains(m_proxy->mapFromSource(m_p1PersistentAfter)));
+        QVERIFY(parents.contains(m_proxy->mapFromSource(m_p2PersistentAfter)));
+
+        // In the source model, the rows were moved to row 1 in the parent.
+        // m_p2FirstProxyChild was created with row 0 in the proxy.
+        // The moved rows in the proxy do not appear at row 1 because of sorting.
+        // Sorting causes them to appear at row 0 instead, pushing what used to
+        // be row 0 in the proxy down by two rows.
+        QCOMPARE(m_p2FirstProxyChild.row(), 2);
+    }
+
+private:
+    QAbstractItemModel *m_model;
+    QAbstractProxyModel *m_proxy;
+    QPersistentModelIndex m_p1PersistentBefore;
+    QPersistentModelIndex m_p2PersistentBefore;
+    QPersistentModelIndex m_p1PersistentAfter;
+    QPersistentModelIndex m_p2PersistentAfter;
+
+    QPersistentModelIndex m_p2FirstProxyChild;
+};
+
+void tst_QSortFilterProxyModel::moveSourceRows()
+{
+    qRegisterMetaType<QList<QPersistentModelIndex> >();
+
+    DynamicTreeModel model;
+
+    {
+        ModelInsertCommand insertCommand(&model);
+        insertCommand.setStartRow(0);
+        insertCommand.setEndRow(9);
+        insertCommand.doCommand();
+    }
+    {
+        ModelInsertCommand insertCommand(&model);
+        insertCommand.setAncestorRowNumbers(QList<int>() << 2);
+        insertCommand.setStartRow(0);
+        insertCommand.setEndRow(9);
+        insertCommand.doCommand();
+    }
+    {
+        ModelInsertCommand insertCommand(&model);
+        insertCommand.setAncestorRowNumbers(QList<int>() << 5);
+        insertCommand.setStartRow(0);
+        insertCommand.setEndRow(9);
+        insertCommand.doCommand();
+    }
+
+    QSortFilterProxyModel proxy;
+    proxy.setDynamicSortFilter(true);
+    proxy.sort(0, Qt::AscendingOrder);
+
+    // We need to check the arguments at emission time
+    SignalArgumentChecker checker(&model, &proxy);
+
+    proxy.setSourceModel(&model);
+
+    QSignalSpy modelBeforeSpy(&model, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)));
+    QSignalSpy modelAfterSpy(&model, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)));
+    QSignalSpy proxyBeforeMoveSpy(m_proxy, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)));
+    QSignalSpy proxyAfterMoveSpy(m_proxy, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)));
+    QSignalSpy proxyBeforeParentLayoutSpy(&proxy, SIGNAL(layoutAboutToBeChanged(QList<QPersistentModelIndex>)));
+    QSignalSpy proxyAfterParentLayoutSpy(&proxy, SIGNAL(layoutChanged(QList<QPersistentModelIndex>)));
+
+    {
+        ModelMoveCommand moveCommand(&model, 0);
+        moveCommand.setAncestorRowNumbers(QList<int>() << 2);
+        moveCommand.setDestAncestors(QList<int>() << 5);
+        moveCommand.setStartRow(3);
+        moveCommand.setEndRow(4);
+        moveCommand.setDestRow(1);
+        moveCommand.doCommand();
+    }
+
+    // Proxy notifies layout change
+    QCOMPARE(modelBeforeSpy.size(), 1);
+    QCOMPARE(proxyBeforeParentLayoutSpy.size(), 1);
+    QCOMPARE(modelAfterSpy.size(), 1);
+    QCOMPARE(proxyAfterParentLayoutSpy.size(), 1);
+
+    // But it doesn't notify a move.
+    QCOMPARE(proxyBeforeMoveSpy.size(), 0);
+    QCOMPARE(proxyAfterMoveSpy.size(), 0);
+}
+
 QTEST_MAIN(tst_QSortFilterProxyModel)
 #include "tst_qsortfilterproxymodel.moc"