Displaced items were moving unnecessarily

They should only move if they actually change from the last set
position, and not if they are simply changing from their current item
position, as that is wrong during an animation.

This also cleans up some code for resetting the transition data.

Task-number: QTBUG-24586
Change-Id: I0a6635903975ebc40d5cf8398b943a9de92d4493
Reviewed-by: Martin Jones <martin.jones@nokia.com>

2 files changed, 129 insertions, 0 deletions

diff --git a/tests/auto/quick/qquicklistview/data/multipleDisplaced.qml b/tests/auto/quick/qquicklistview/data/multipleDisplaced.qml
new file mode 100644
index 0000000000..e315270360
--- /dev/null
+++ b/tests/auto/quick/qquicklistview/data/multipleDisplaced.qml
@@ -0,0 +1,78 @@
+import QtQuick 2.0
+
+Rectangle {
+    id: root
+    width: 500
+    height: 600
+
+    property int duration: 10
+    property int count: list.count
+
+    Component {
+        id: myDelegate
+        Rectangle {
+            id: wrapper
+
+            property string nameData: name
+
+            objectName: "wrapper"
+            height: 20
+            width: 240
+            Text { text: index }
+            Text {
+                x: 30
+                id: textName
+                objectName: "textName"
+                text: name
+            }
+            Text {
+                x: 200
+                text: wrapper.y
+            }
+            color: ListView.isCurrentItem ? "lightsteelblue" : "white"
+        }
+    }
+
+    ListView {
+        id: list
+
+        property var displaceTransitionsStarted: new Object()
+        property bool displaceTransitionsDone: false
+
+        objectName: "list"
+        focus: true
+        anchors.centerIn: parent
+        width: 240
+        height: 320
+        model: testModel
+        delegate: myDelegate
+
+        displaced: Transition {
+            id: transition
+            SequentialAnimation {
+                ScriptAction {
+                    script: {
+                        var name = transition.ViewTransition.item.nameData
+                        if (list.displaceTransitionsStarted[name] == undefined)
+                            list.displaceTransitionsStarted[name] = 0
+                        list.displaceTransitionsStarted[name] += 1
+                    }
+                }
+                NumberAnimation {
+                    properties: "x,y"
+                    duration: root.duration
+                    easing.type: Easing.OutBounce
+                    easing.amplitude: 10.0      // longer-lasting bounce to trigger bug
+                }
+                PropertyAction { target: list; property: "displaceTransitionsDone"; value: true }
+            }
+        }
+    }
+
+    Rectangle {
+        anchors.fill: list
+        color: "lightsteelblue"
+        opacity: 0.2
+    }
+}
+
diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
index c038faa0f1..037d9e9b7f 100644
--- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
@@ -183,6 +183,7 @@ private slots:
     void displacedTransitions_data();
     void multipleTransitions();
     void multipleTransitions_data();
+    void multipleDisplaced();
 
     void flickBeyondBounds();
 
@@ -5977,6 +5978,56 @@ void tst_QQuickListView::multipleTransitions_data()
             << true << true << false << false;
 }
 
+void tst_QQuickListView::multipleDisplaced()
+{
+    // multiple move() operations should only restart displace transitions for items that
+    // moved from previously set positions, and not those that have moved from their current
+    // item positions (which may e.g. still be changing from easing bounces in the last transition)
+
+    QmlListModel model;
+    for (int i = 0; i < 30; i++)
+        model.addItem("Original item" + QString::number(i), "");
+
+    QQuickView *canvas = createView();
+    QQmlContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+    ctxt->setContextProperty("testObject", new TestObject(canvas));
+    canvas->setSource(testFileUrl("multipleDisplaced.qml"));
+    canvas->show();
+    QTest::qWaitForWindowShown(canvas);
+
+    QQuickListView *listview = findItem<QQuickListView>(canvas->rootObject(), "list");
+    QTRY_VERIFY(listview != 0);
+    QQuickItem *contentItem = listview->contentItem();
+    QVERIFY(contentItem != 0);
+    QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false);
+
+    model.moveItems(12, 8, 1);
+    QTest::qWait(canvas->rootObject()->property("duration").toInt() / 2);
+    model.moveItems(8, 3, 1);
+    QTRY_VERIFY(listview->property("displaceTransitionsDone").toBool());
+
+    QVariantMap transitionsStarted = listview->property("displaceTransitionsStarted").toMap();
+    foreach (const QString &name, transitionsStarted.keys()) {
+        QVERIFY2(transitionsStarted[name] == 1,
+                 QTest::toString(QString("%1 was displaced %2 times").arg(name).arg(transitionsStarted[name].toInt())));
+    }
+
+    // verify all items moved to the correct final positions
+    QList<QQuickItem*> items = findItems<QQuickItem>(contentItem, "wrapper");
+    for (int i=0; i < model.count() && i < items.count(); ++i) {
+        QQuickItem *item = findItem<QQuickItem>(contentItem, "wrapper", i);
+        QVERIFY2(item, QTest::toString(QString("Item %1 not found").arg(i)));
+        QTRY_COMPARE(item->x(), 0.0);
+        QTRY_COMPARE(item->y(), i*20.0);
+        QQuickText *name = findItem<QQuickText>(contentItem, "textName", i);
+        QVERIFY(name != 0);
+        QTRY_COMPARE(name->text(), model.name(i));
+    }
+
+    delete canvas;
+}
+
 QList<int> tst_QQuickListView::toIntList(const QVariantList &list)
 {
     QList<int> ret;