prepareTransition() must cancel current transition

If it does not, itemX() or itemY() will still return the current
transition's 'to' position instead of the nextTransitionTo pos that was
scheduled for the next transition (which the item now has moved to
using the direct setPos() method).

Also refactor prepareTransition() to always move the item directly to
the nextTransitionTo if transition is not going ahead.

Also fix some broken test code.

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

src/quick/items/qquickitemviewtransition.cpp

@@ -408,14 +408,11 @@ bool QQuickViewItem::prepareTransition(const QRectF &viewBounds)
             doTransition = (nextTransitionType == QQuickItemViewTransitioner::AddTransition)
                     ? viewBounds.intersects(QRectF(nextTransitionTo.x(), nextTransitionTo.y(), item->width(), item->height()))
                     : viewBounds.intersects(QRectF(item->x(), item->y(), item->width(), item->height()));
-            if (!doTransition)
-                item->setPos(nextTransitionTo);
         } else {
+            // do transition if moving from or into visible area
             if (viewBounds.intersects(QRectF(item->x(), item->y(), item->width(), item->height()))
                     || viewBounds.intersects(QRectF(nextTransitionTo.x(), nextTransitionTo.y(), item->width(), item->height()))) {
                 doTransition = transitionWillChangePosition();
-            } else {
-                item->setPos(nextTransitionTo);
             }
         }
         break;
@@ -425,14 +422,16 @@ bool QQuickViewItem::prepareTransition(const QRectF &viewBounds)
             doTransition = viewBounds.isNull()
                     || viewBounds.intersects(QRectF(item->x(), item->y(), item->width(), item->height()))
                     || viewBounds.intersects(QRectF(nextTransitionTo.x(), nextTransitionTo.y(), item->width(), item->height()));
-            if (!doTransition)
-                item->setPos(nextTransitionTo);
         }
         break;
     }
 
-    if (!doTransition)
+    if (!doTransition) {
+        if (transition)
+            transition->cancel();
+        item->setPos(nextTransitionTo);
         resetTransitionData();
+    }
     return doTransition;
 }
 

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

@@ -10,7 +10,7 @@ Rectangle {
     // interrupting transitions will still produce the correct result)
     property int timeBetweenActions: duration / 2
 
-    property int duration: 300
+    property int duration: 100
 
     property int count: grid.count
 

tests/auto/quick/qquickgridview/tst_qquickgridview.cpp

@@ -1274,8 +1274,6 @@ void tst_QQuickGridView::moved_data()
 
 void tst_QQuickGridView::multipleChanges()
 {
-    QSKIP("QTBUG-24523");
-
     QFETCH(int, startCount);
     QFETCH(QList<ListChange>, changes);
     QFETCH(int, newCount);
@@ -4817,17 +4815,15 @@ void tst_QQuickGridView::multipleTransitions()
 
     int timeBetweenActions = canvas->rootObject()->property("timeBetweenActions").toInt();
 
-    QList<QPair<QString, QString> > targetItems;
     for (int i=0; i<changes.count(); i++) {
         switch (changes[i].type) {
             case ListChange::Inserted:
             {
+                QList<QPair<QString, QString> > targetItems;
                 for (int j=changes[i].index; j<changes[i].index + changes[i].count; ++j)
                     targetItems << qMakePair(QString("new item %1").arg(j), QString::number(j));
                 model.insertItems(changes[i].index, targetItems);
                 QTRY_COMPARE(model.count(), gridview->count());
-                QTRY_VERIFY(gridview->property("runningAddTargets").toBool());
-                QTRY_VERIFY(gridview->property("runningAddDisplaced").toBool());
                 if (i == changes.count() - 1) {
                     QTRY_VERIFY(!gridview->property("runningAddTargets").toBool());
                     QTRY_VERIFY(!gridview->property("runningAddDisplaced").toBool());
@@ -4837,12 +4833,8 @@ void tst_QQuickGridView::multipleTransitions()
                 break;
             }
             case ListChange::Removed:
-                for (int j=changes[i].index; j<changes[i].index + changes[i].count; ++j)
-                    targetItems << qMakePair(model.name(i), model.number(i));
                 model.removeItems(changes[i].index, changes[i].count);
                 QTRY_COMPARE(model.count(), gridview->count());
-                QTRY_VERIFY(gridview->property("runningRemoveTargets").toBool());
-                QTRY_VERIFY(gridview->property("runningRemoveDisplaced").toBool());
                 if (i == changes.count() - 1) {
                     QTRY_VERIFY(!gridview->property("runningRemoveTargets").toBool());
                     QTRY_VERIFY(!gridview->property("runningRemoveDisplaced").toBool());
@@ -4851,11 +4843,8 @@ void tst_QQuickGridView::multipleTransitions()
                 }
                 break;
             case ListChange::Moved:
-                for (int j=changes[i].index; j<changes[i].index + changes[i].count; ++j)
-                    targetItems << qMakePair(model.name(i), model.number(i));
                 model.moveItems(changes[i].index, changes[i].to, changes[i].count);
-                QTRY_VERIFY(gridview->property("runningMoveTargets").toBool());
+                QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false);
-                QTRY_VERIFY(gridview->property("runningMoveDisplaced").toBool());
                 if (i == changes.count() - 1) {
                     QTRY_VERIFY(!gridview->property("runningMoveTargets").toBool());
                     QTRY_VERIFY(!gridview->property("runningMoveDisplaced").toBool());

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

@@ -10,7 +10,7 @@ Rectangle {
     // interrupting transitions will still produce the correct result)
     property int timeBetweenActions: duration / 2
 
-    property int duration: 300
+    property int duration: 100
 
     property int count: list.count
 

tests/auto/quick/qquicklistview/tst_qquicklistview.cpp

@@ -5779,8 +5779,6 @@ void tst_QQuickListView::displacedTransitions_data()
 
 void tst_QQuickListView::multipleTransitions()
 {
-    QSKIP("QTBUG-24523");
-
     // Tests that if you interrupt a transition in progress with another action that
     // cancels the previous transition, the resulting items are still placed correctly.
 
@@ -5829,17 +5827,15 @@ void tst_QQuickListView::multipleTransitions()
 
     int timeBetweenActions = canvas->rootObject()->property("timeBetweenActions").toInt();
 
-    QList<QPair<QString, QString> > targetItems;
     for (int i=0; i<changes.count(); i++) {
         switch (changes[i].type) {
             case ListChange::Inserted:
             {
+                QList<QPair<QString, QString> > targetItems;
                 for (int j=changes[i].index; j<changes[i].index + changes[i].count; ++j)
                     targetItems << qMakePair(QString("new item %1").arg(j), QString::number(j));
                 model.insertItems(changes[i].index, targetItems);
                 QTRY_COMPARE(model.count(), listview->count());
-                QTRY_VERIFY(listview->property("runningAddTargets").toBool());
-                QTRY_VERIFY(listview->property("runningAddDisplaced").toBool());
                 if (i == changes.count() - 1) {
                     QTRY_VERIFY(!listview->property("runningAddTargets").toBool());
                     QTRY_VERIFY(!listview->property("runningAddDisplaced").toBool());
@@ -5849,12 +5845,8 @@ void tst_QQuickListView::multipleTransitions()
                 break;
             }
             case ListChange::Removed:
-                for (int j=changes[i].index; j<changes[i].index + changes[i].count; ++j)
-                    targetItems << qMakePair(model.name(i), model.number(i));
                 model.removeItems(changes[i].index, changes[i].count);
                 QTRY_COMPARE(model.count(), listview->count());
-                QTRY_VERIFY(listview->property("runningRemoveTargets").toBool());
-                QTRY_VERIFY(listview->property("runningRemoveDisplaced").toBool());
                 if (i == changes.count() - 1) {
                     QTRY_VERIFY(!listview->property("runningRemoveTargets").toBool());
                     QTRY_VERIFY(!listview->property("runningRemoveDisplaced").toBool());
@@ -5863,11 +5855,8 @@ void tst_QQuickListView::multipleTransitions()
                 }
                 break;
             case ListChange::Moved:
-                for (int j=changes[i].index; j<changes[i].index + changes[i].count; ++j)
-                    targetItems << qMakePair(model.name(i), model.number(i));
                 model.moveItems(changes[i].index, changes[i].to, changes[i].count);
-                QTRY_VERIFY(listview->property("runningMoveTargets").toBool());
+                QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false);
-                QTRY_VERIFY(listview->property("runningMoveDisplaced").toBool());
                 if (i == changes.count() - 1) {
                     QTRY_VERIFY(!listview->property("runningMoveTargets").toBool());
                     QTRY_VERIFY(!listview->property("runningMoveDisplaced").toBool());