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>

diff --git a/src/quick/items/qquickitemviewtransition.cpp b/src/quick/items/qquickitemviewtransition.cpp
index 54375bf..ac9375b 100644 (file)
--- a/src/quick/items/qquickitemviewtransition.cpp
+++ b/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;
 }
 

diff --git a/tests/auto/quick/qquickgridview/data/multipleTransitions.qml b/tests/auto/quick/qquickgridview/data/multipleTransitions.qml
index 909ec3a..f0d9320 100644 (file)
--- a/tests/auto/quick/qquickgridview/data/multipleTransitions.qml
+++ b/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
 

diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
index c0f2b02..b3e79df 100644 (file)
--- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
+++ b/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_VERIFY(gridview->property("runningMoveDisplaced").toBool());
+                QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false);
                 if (i == changes.count() - 1) {
                     QTRY_VERIFY(!gridview->property("runningMoveTargets").toBool());
                     QTRY_VERIFY(!gridview->property("runningMoveDisplaced").toBool());

diff --git a/tests/auto/quick/qquicklistview/data/multipleTransitions.qml b/tests/auto/quick/qquicklistview/data/multipleTransitions.qml
index 8264b42..68efeea 100644 (file)
--- a/tests/auto/quick/qquicklistview/data/multipleTransitions.qml
+++ b/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
 

diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
index 9195aab..8c5c25c 100644 (file)
--- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
+++ b/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_VERIFY(listview->property("runningMoveDisplaced").toBool());
+                QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false);
                 if (i == changes.count() - 1) {
                     QTRY_VERIFY(!listview->property("runningMoveTargets").toBool());
                     QTRY_VERIFY(!listview->property("runningMoveDisplaced").toBool());