refilled items should be moved immediately

refill() functionality should reposition items immediately, else
removeNonVisibleItems() sees different positions from those added in
addVisibleItems() if an item is animating.

Change-Id: Ib9904e08bf92b18fd4b712270c0ab69e9a113e04
Reviewed-by: Martin Jones <martin.jones@nokia.com>

src/quick/items/qquickgridview.cpp

@@ -120,8 +120,8 @@ class FxGridItemSG : public FxViewItem
                 return itemX() + view->cellWidth();
         }
     }
-    void setPosition(qreal col, qreal row) {
-        moveTo(pointForPosition(col, row));
+    void setPosition(qreal col, qreal row, bool immediate = false) {
+        moveTo(pointForPosition(col, row), immediate);
     }
     bool contains(qreal x, qreal y) const {
         return (x >= itemX() && x < itemX() + view->cellWidth() &&
@@ -483,7 +483,7 @@ bool QQuickGridViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d
         if (!(item = static_cast<FxGridItemSG*>(createItem(modelIndex, doBuffer))))
             break;
         if (!transitioner || !transitioner->canTransition(QQuickItemViewTransitioner::PopulateTransition, true)) // pos will be set by layoutVisibleItems()
-            item->setPosition(colPos, rowPos);
+            item->setPosition(colPos, rowPos, true);
         item->item->setVisible(!doBuffer);
         visibleItems.append(item);
         if (++colNum >= columns) {
@@ -521,7 +521,7 @@ bool QQuickGridViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d
             break;
         --visibleIndex;
         if (!transitioner || !transitioner->canTransition(QQuickItemViewTransitioner::PopulateTransition, true)) // pos will be set by layoutVisibleItems()
-            item->setPosition(colPos, rowPos);
+            item->setPosition(colPos, rowPos, true);
         item->item->setVisible(!doBuffer);
         visibleItems.prepend(item);
         if (--colNum < 0) {

src/quick/items/qquickitemview.cpp

@@ -73,10 +73,10 @@ qreal FxViewItem::itemY() const
     return transitionableItem ? transitionableItem->itemY() : item->y();
 }
 
-void FxViewItem::moveTo(const QPointF &pos)
+void FxViewItem::moveTo(const QPointF &pos, bool immediate)
 {
     if (transitionableItem)
-        transitionableItem->moveTo(pos);
+        transitionableItem->moveTo(pos, immediate);
     else
         item->setPos(pos);
 }

src/quick/items/qquickitemview_p_p.h

@@ -66,7 +66,7 @@ class FxViewItem
     qreal itemX() const;
     qreal itemY() const;
 
-    void moveTo(const QPointF &pos);
+    void moveTo(const QPointF &pos, bool immediate);
     void setVisible(bool visible);
 
     QQuickItemViewTransitioner::TransitionType scheduledTransitionType() const;

src/quick/items/qquickitemviewtransition.cpp

@@ -365,13 +365,18 @@ qreal QQuickItemViewTransitionableItem::itemY() const
         return item->y();
 }
 
-void QQuickItemViewTransitionableItem::moveTo(const QPointF &pos)
+void QQuickItemViewTransitionableItem::moveTo(const QPointF &pos, bool immediate)
 {
-    if (transitionScheduledOrRunning()) {
+    if (immediate || !transitionScheduledOrRunning()) {
+        if (immediate) {
+            if (transition)
+                transition->cancel();
+            resetTransitionData();
+        }
+        item->setPos(pos);
+    } else {
         nextTransitionTo = pos;
         nextTransitionToSet = true;
-    } else {
-        item->setPos(pos);
     }
 }
 

src/quick/items/qquickitemviewtransition_p.h

@@ -132,7 +132,7 @@ class QQuickItemViewTransitionableItem
     qreal itemX() const;
     qreal itemY() const;
 
-    void moveTo(const QPointF &pos);
+    void moveTo(const QPointF &pos, bool immediate = false);
 
     bool transitionScheduledOrRunning() const;
     bool transitionRunning() const;

src/quick/items/qquicklistview.cpp

@@ -292,7 +292,7 @@ class FxListItemSG : public FxViewItem
                     : itemX() + item->width());
         }
     }
-    void setPosition(qreal pos) {
+    void setPosition(qreal pos, bool immediate = false) {
         // position the section immediately even if there is a transition
         if (section()) {
             if (view->orientation() == QQuickListView::Vertical) {
@@ -304,7 +304,7 @@ class FxListItemSG : public FxViewItem
                     section()->setX(pos);
             }
         }
-        moveTo(pointForPosition(pos));
+        moveTo(pointForPosition(pos), immediate);
     }
     void setSize(qreal size) {
         if (view->orientation() == QQuickListView::Vertical)
@@ -638,7 +638,7 @@ bool QQuickListViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d
         if (!(item = static_cast<FxListItemSG*>(createItem(modelIndex, doBuffer))))
             break;
         if (!transitioner || !transitioner->canTransition(QQuickItemViewTransitioner::PopulateTransition, true)) // pos will be set by layoutVisibleItems()
-            item->setPosition(pos);
+            item->setPosition(pos, true);
         item->item->setVisible(!doBuffer);
         pos += item->size() + spacing;
         visibleItems.append(item);
@@ -658,7 +658,7 @@ bool QQuickListViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d
         --visibleIndex;
         visiblePos -= item->size() + spacing;
         if (!transitioner || !transitioner->canTransition(QQuickItemViewTransitioner::PopulateTransition, true)) // pos will be set by layoutVisibleItems()
-            item->setPosition(visiblePos);
+            item->setPosition(visiblePos, true);
         item->item->setVisible(!doBuffer);
         visibleItems.prepend(item);
         changed = true;

tests/auto/quick/qquickgridview/data/multipleTransitions.qml

@@ -60,6 +60,7 @@ Rectangle {
 
         add: Transition {
             id: addTargets
+            enabled: enableAddTransitions
             SequentialAnimation {
                 ScriptAction { script: grid.runningAddTargets = true }
                 ParallelAnimation {
@@ -72,6 +73,7 @@ Rectangle {
 
         addDisplaced: Transition {
             id: addDisplaced
+            enabled: enableAddTransitions
             SequentialAnimation {
                 ScriptAction { script: grid.runningAddDisplaced = true }
                 ParallelAnimation {
@@ -84,6 +86,7 @@ Rectangle {
 
         move: Transition {
             id: moveTargets
+            enabled: enableMoveTransitions
             SequentialAnimation {
                 ScriptAction { script: grid.runningMoveTargets = true }
                 ParallelAnimation {
@@ -96,6 +99,7 @@ Rectangle {
 
         moveDisplaced: Transition {
             id: moveDisplaced
+            enabled: enableMoveTransitions
             SequentialAnimation {
                 ScriptAction { script: grid.runningMoveDisplaced = true }
                 ParallelAnimation {
@@ -108,6 +112,7 @@ Rectangle {
 
         remove: Transition {
             id: removeTargets
+            enabled: enableRemoveTransitions
             SequentialAnimation {
                 ScriptAction { script: grid.runningRemoveTargets = true }
                 ParallelAnimation {
@@ -120,6 +125,7 @@ Rectangle {
 
         removeDisplaced: Transition {
             id: removeDisplaced
+            enabled: enableRemoveTransitions
             SequentialAnimation {
                 ScriptAction { script: grid.runningRemoveDisplaced = true }
                 ParallelAnimation {

tests/auto/quick/qquickgridview/tst_qquickgridview.cpp

@@ -4795,6 +4795,9 @@ void tst_QQuickGridView::multipleTransitions()
     QFETCH(int, initialCount);
     QFETCH(qreal, contentY);
     QFETCH(QList<ListChange>, changes);
+    QFETCH(bool, enableAddTransitions);
+    QFETCH(bool, enableMoveTransitions);
+    QFETCH(bool, enableRemoveTransitions);
     QFETCH(bool, rippleAddDisplaced);
 
     // add transitions on the left, moves on the right
@@ -4818,6 +4821,9 @@ void tst_QQuickGridView::multipleTransitions()
     ctxt->setContextProperty("moveDisplaced_transitionFrom", moveDisplaced_transitionFrom);
     ctxt->setContextProperty("removeTargets_transitionTo", removeTargets_transitionTo);
     ctxt->setContextProperty("removeDisplaced_transitionFrom", removeDisplaced_transitionFrom);
+    ctxt->setContextProperty("enableAddTransitions", enableAddTransitions);
+    ctxt->setContextProperty("enableMoveTransitions", enableMoveTransitions);
+    ctxt->setContextProperty("enableRemoveTransitions", enableRemoveTransitions);
     ctxt->setContextProperty("rippleAddDisplaced", rippleAddDisplaced);
     canvas->setSource(testFileUrl("multipleTransitions.qml"));
     canvas->show();
@@ -4901,8 +4907,8 @@ void tst_QQuickGridView::multipleTransitions()
     for (int i=firstVisibleIndex; i < model.count() && i < itemCount; ++i) {
         QQuickItem *item = findItem<QQuickItem>(contentItem, "wrapper", i);
         QVERIFY2(item, QTest::toString(QString("Item %1 not found").arg(i)));
-        QCOMPARE(item->x(), (i%3)*80.0);
-        QCOMPARE(item->y(), (i/3)*60.0);
+        QTRY_COMPARE(item->x(), (i%3)*80.0);
+        QTRY_COMPARE(item->y(), (i/3)*60.0);
         QQuickText *name = findItem<QQuickText>(contentItem, "textName", i);
         QVERIFY(name != 0);
         QTRY_COMPARE(name->text(), model.name(i));
@@ -4916,21 +4922,24 @@ void tst_QQuickGridView::multipleTransitions_data()
     QTest::addColumn<int>("initialCount");
     QTest::addColumn<qreal>("contentY");
     QTest::addColumn<QList<ListChange> >("changes");
+    QTest::addColumn<bool>("enableAddTransitions");
+    QTest::addColumn<bool>("enableMoveTransitions");
+    QTest::addColumn<bool>("enableRemoveTransitions");
     QTest::addColumn<bool>("rippleAddDisplaced");
 
     // the added item and displaced items should move to final dest correctly
     QTest::newRow("add item, then move it immediately") << 10 << 0.0 << (QList<ListChange>()
              << ListChange::insert(0, 1)
              << ListChange::move(0, 3, 1)
              )
-             << false;
+             << true << true << true << false;
 
     // items affected by the add should change from move to add transition
     QTest::newRow("move, then insert item before the moved item") << 20 << 0.0 << (QList<ListChange>()
             << ListChange::move(1, 10, 3)
             << ListChange::insert(0, 1)
             )
-            << false;
+            << true << true << true << false;
 
     // items should be placed correctly if you trigger a transition then refill for that index
     QTest::newRow("add at 0, flick down, flick back to top and add at 0 again") << 20 << 0.0 << (QList<ListChange>()
@@ -4939,13 +4948,31 @@ void tst_QQuickGridView::multipleTransitions_data()
             << ListChange::setContentY(0.0)
             << ListChange::insert(0, 1)
             )
-            << false;
+            << true << true << true << false;
 
     QTest::newRow("insert then remove same index, with ripple effect on add displaced") << 20 << 0.0 << (QList<ListChange>()
             << ListChange::insert(1, 1)
             << ListChange::remove(1, 1)
             )
-            << true;
+            << true << true << true << true;
+
+    // if item is removed while undergoing a displaced transition, all other items should end up at their correct positions,
+    // even if a remove-displace transition is not present to re-animate them
+    QTest::newRow("insert then remove, with remove disabled") << 20 << 0.0 << (QList<ListChange>()
+            << ListChange::insert(0, 1)
+            << ListChange::remove(2, 1)
+            )
+            << true << true << false << false;
+
+    // if last item is not flush with the edge of the view, it should still be refilled in correctly after a
+    // remove has changed the position of where it will move to
+    QTest::newRow("insert twice then remove, with remove disabled") << 20 << 0.0 << (QList<ListChange>()
+            << ListChange::setContentY(-10.0)
+            << ListChange::insert(0, 1)
+            << ListChange::insert(0, 1)
+            << ListChange::remove(2, 1)
+            )
+            << true << true << false << false;
 }
 
 void tst_QQuickGridView::cacheBuffer()

tests/auto/quick/qquicklistview/data/multipleTransitions.qml

@@ -58,6 +58,7 @@ Rectangle {
 
         add: Transition {
             id: addTargets
+            enabled: enableAddTransitions
             SequentialAnimation {
                 ScriptAction { script: list.runningAddTargets = true }
                 ParallelAnimation {
@@ -70,6 +71,7 @@ Rectangle {
 
         addDisplaced: Transition {
             id: addDisplaced
+            enabled: enableAddTransitions
             SequentialAnimation {
                 ScriptAction { script: list.runningAddDisplaced = true }
                 PauseAnimation { duration: rippleAddDisplaced ? addDisplaced.ViewTransition.index * root.duration/10 : 0 }
@@ -83,6 +85,7 @@ Rectangle {
 
         move: Transition {
             id: moveTargets
+            enabled: enableMoveTransitions
             SequentialAnimation {
                 ScriptAction { script: list.runningMoveTargets = true }
                 ParallelAnimation {
@@ -95,6 +98,7 @@ Rectangle {
 
         moveDisplaced: Transition {
             id: moveDisplaced
+            enabled: enableMoveTransitions
             SequentialAnimation {
                 ScriptAction { script: list.runningMoveDisplaced = true }
                 ParallelAnimation {
@@ -107,6 +111,7 @@ Rectangle {
 
         remove: Transition {
             id: removeTargets
+            enabled: enableRemoveTransitions
             SequentialAnimation {
                 ScriptAction { script: list.runningRemoveTargets = true }
                 ParallelAnimation {
@@ -119,6 +124,7 @@ Rectangle {
 
         removeDisplaced: Transition {
             id: removeDisplaced
+            enabled: enableRemoveTransitions
             SequentialAnimation {
                 ScriptAction { script: list.runningRemoveDisplaced = true }
                 ParallelAnimation {

tests/auto/quick/qquicklistview/tst_qquicklistview.cpp

@@ -5807,6 +5807,9 @@ void tst_QQuickListView::multipleTransitions()
     QFETCH(int, initialCount);
     QFETCH(qreal, contentY);
     QFETCH(QList<ListChange>, changes);
+    QFETCH(bool, enableAddTransitions);
+    QFETCH(bool, enableMoveTransitions);
+    QFETCH(bool, enableRemoveTransitions);
     QFETCH(bool, rippleAddDisplaced);
 
     QPointF addTargets_transitionFrom(-50, -50);
@@ -5831,6 +5834,9 @@ void tst_QQuickListView::multipleTransitions()
     ctxt->setContextProperty("moveDisplaced_transitionFrom", moveDisplaced_transitionFrom);
     ctxt->setContextProperty("removeTargets_transitionTo", removeTargets_transitionTo);
     ctxt->setContextProperty("removeDisplaced_transitionFrom", removeDisplaced_transitionFrom);
+    ctxt->setContextProperty("enableAddTransitions", enableAddTransitions);
+    ctxt->setContextProperty("enableMoveTransitions", enableMoveTransitions);
+    ctxt->setContextProperty("enableRemoveTransitions", enableRemoveTransitions);
     ctxt->setContextProperty("rippleAddDisplaced", rippleAddDisplaced);
     canvas->setSource(testFileUrl("multipleTransitions.qml"));
     canvas->show();
@@ -5918,21 +5924,24 @@ void tst_QQuickListView::multipleTransitions_data()
     QTest::addColumn<int>("initialCount");
     QTest::addColumn<qreal>("contentY");
     QTest::addColumn<QList<ListChange> >("changes");
+    QTest::addColumn<bool>("enableAddTransitions");
+    QTest::addColumn<bool>("enableMoveTransitions");
+    QTest::addColumn<bool>("enableRemoveTransitions");
     QTest::addColumn<bool>("rippleAddDisplaced");
 
     // the added item and displaced items should move to final dest correctly
     QTest::newRow("add item, then move it immediately") << 10 << 0.0 << (QList<ListChange>()
             << ListChange::insert(0, 1)
             << ListChange::move(0, 3, 1)
             )
-            << false;
+            << true << true << true << false;
 
     // items affected by the add should change from move to add transition
     QTest::newRow("move, then insert item before the moved item") << 20 << 0.0 << (QList<ListChange>()
             << ListChange::move(1, 10, 3)
             << ListChange::insert(0, 1)
             )
-            << false;
+            << true << true << true << false;
 
     // items should be placed correctly if you trigger a transition then refill for that index
     QTest::newRow("add at 0, flick down, flick back to top and add at 0 again") << 20 << 0.0 << (QList<ListChange>()
@@ -5941,13 +5950,31 @@ void tst_QQuickListView::multipleTransitions_data()
             << ListChange::setContentY(0.0)
             << ListChange::insert(0, 1)
             )
-            << false;
+            << true << true << true << false;
 
     QTest::newRow("insert then remove same index, with ripple effect on add displaced") << 20 << 0.0 << (QList<ListChange>()
             << ListChange::insert(1, 1)
             << ListChange::remove(1, 1)
             )
-            << true;
+            << true << true << true << true;
+
+    // if item is removed while undergoing a displaced transition, all other items should end up at their correct positions,
+    // even if a remove-displace transition is not present to re-animate them
+    QTest::newRow("insert then remove, with remove disabled") << 20 << 0.0 << (QList<ListChange>()
+            << ListChange::insert(0, 1)
+            << ListChange::remove(2, 1)
+            )
+            << true << true << false << false;
+
+    // if last item is not flush with the edge of the view, it should still be refilled in correctly after a
+    // remove has changed the position of where it will move to
+    QTest::newRow("insert twice then remove, with remove disabled") << 20 << 0.0 << (QList<ListChange>()
+            << ListChange::setContentY(-10.0)
+            << ListChange::insert(0, 1)
+            << ListChange::insert(0, 1)
+            << ListChange::remove(2, 1)
+            )
+            << true << true << false << false;
 }
 
 QList<int> tst_QQuickListView::toIntList(const QVariantList &list)