From d290cb3a499a0c3a71ab1f63cbd2fc45b0f5835f Mon Sep 17 00:00:00 2001 From: Bea Lam Date: Mon, 27 Feb 2012 16:26:05 +1000 Subject: [PATCH] Fix when animating items that are already moving The view must transition displaced/moved items that are currently transitioning to another position; check against the current transition-to position, not just the current item position. Task-number: QTBUG-24522 Change-Id: Icf1c290f76ceb8c93716f1562ae0bc5a75445b78 Reviewed-by: Martin Jones --- src/quick/items/qquickitemviewtransition.cpp | 20 ++++++++++---- src/quick/items/qquickitemviewtransition_p.h | 1 + .../qquickgridview/data/multipleTransitions.qml | 28 ++++++++++++++++++- .../qtquick2/qquickgridview/tst_qquickgridview.cpp | 32 ++++++++++++++++++---- .../qquicklistview/data/multipleTransitions.qml | 27 ++++++++++++++++++ .../qtquick2/qquicklistview/tst_qquicklistview.cpp | 28 ++++++++++++++++--- 6 files changed, 121 insertions(+), 15 deletions(-) diff --git a/src/quick/items/qquickitemviewtransition.cpp b/src/quick/items/qquickitemviewtransition.cpp index abff768..3febc9b 100644 --- a/src/quick/items/qquickitemviewtransition.cpp +++ b/src/quick/items/qquickitemviewtransition.cpp @@ -383,6 +383,9 @@ bool QQuickViewItem::prepareTransition(const QRectF &viewBounds) if (nextTransitionType != QQuickItemViewTransitioner::NoTransition && !nextTransitionToSet) moveTo(item->pos()); + // For move transitions (both target and displaced) and displaced transitions of other + // types, only run the transition if the item is actually moving to another position. + switch (nextTransitionType) { case QQuickItemViewTransitioner::NoTransition: { @@ -394,14 +397,14 @@ bool QQuickViewItem::prepareTransition(const QRectF &viewBounds) } case QQuickItemViewTransitioner::AddTransition: case QQuickItemViewTransitioner::RemoveTransition: - // For Add targets, do transition if item is moving into visible area - // For Remove targets, do transition if item is currently in visible area if (viewBounds.isNull()) { if (isTransitionTarget) doTransition = true; else - doTransition = (nextTransitionTo != item->pos()); + doTransition = transitionWillChangePosition(); } else if (isTransitionTarget) { + // For Add targets, do transition if item is moving into visible area + // For Remove targets, do transition if item is currently in visible area 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())); @@ -410,7 +413,7 @@ bool QQuickViewItem::prepareTransition(const QRectF &viewBounds) } else { if (viewBounds.intersects(QRectF(item->x(), item->y(), item->width(), item->height())) || viewBounds.intersects(QRectF(nextTransitionTo.x(), nextTransitionTo.y(), item->width(), item->height()))) { - doTransition = (nextTransitionTo != item->pos()); + doTransition = transitionWillChangePosition(); } else { item->setPos(nextTransitionTo); } @@ -418,7 +421,7 @@ bool QQuickViewItem::prepareTransition(const QRectF &viewBounds) break; case QQuickItemViewTransitioner::MoveTransition: // do transition if moving from or into visible area - if (nextTransitionTo != item->pos()) { + if (transitionWillChangePosition()) { 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())); @@ -472,6 +475,13 @@ void QQuickViewItem::setNextTransition(QQuickItemViewTransitioner::TransitionTyp isTransitionTarget = isTargetItem; } +bool QQuickViewItem::transitionWillChangePosition() const +{ + if (transitionRunning() && transition->m_toPos != nextTransitionTo) + return true; + return nextTransitionTo != item->pos(); +} + void QQuickViewItem::finishedTransition() { nextTransitionToSet = false; diff --git a/src/quick/items/qquickitemviewtransition_p.h b/src/quick/items/qquickitemviewtransition_p.h index 1ebc52c..57ea85b 100644 --- a/src/quick/items/qquickitemviewtransition_p.h +++ b/src/quick/items/qquickitemviewtransition_p.h @@ -152,6 +152,7 @@ private: friend class QQuickItemViewTransitioner; friend class QQuickItemViewTransitionJob; void setNextTransition(QQuickItemViewTransitioner::TransitionType, bool isTargetItem); + bool transitionWillChangePosition() const; void finishedTransition(); void resetTransitionData(); }; diff --git a/tests/auto/qtquick2/qquickgridview/data/multipleTransitions.qml b/tests/auto/qtquick2/qquickgridview/data/multipleTransitions.qml index 45b86e2..909ec3a 100644 --- a/tests/auto/qtquick2/qquickgridview/data/multipleTransitions.qml +++ b/tests/auto/qtquick2/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: 100 + property int duration: 300 property int count: grid.count @@ -46,6 +46,8 @@ Rectangle { property bool runningAddDisplaced: false property bool runningMoveTargets: false property bool runningMoveDisplaced: false + property bool runningRemoveTargets: false + property bool runningRemoveDisplaced: false objectName: "grid" width: 240 @@ -103,6 +105,30 @@ Rectangle { ScriptAction { script: grid.runningMoveDisplaced = false } } } + + remove: Transition { + id: removeTargets + SequentialAnimation { + ScriptAction { script: grid.runningRemoveTargets = true } + ParallelAnimation { + NumberAnimation { properties: "x"; to: removeTargets_transitionTo.x; duration: root.duration } + NumberAnimation { properties: "y"; to: removeTargets_transitionTo.y; duration: root.duration } + } + ScriptAction { script: grid.runningRemoveTargets = false } + } + } + + removeDisplaced: Transition { + id: removeDisplaced + SequentialAnimation { + ScriptAction { script: grid.runningRemoveDisplaced = true } + ParallelAnimation { + NumberAnimation { properties: "x"; from: removeDisplaced_transitionFrom.x; duration: root.duration } + NumberAnimation { properties: "y"; from: removeDisplaced_transitionFrom.y; duration: root.duration } + } + ScriptAction { script: grid.runningRemoveDisplaced = false } + } + } } Rectangle { diff --git a/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp b/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp index 6d755a6..077cd2b 100644 --- a/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp @@ -4776,12 +4776,15 @@ void tst_QQuickGridView::multipleTransitions() QFETCH(int, initialCount); QFETCH(qreal, contentY); QFETCH(QList, changes); + QFETCH(bool, rippleAddDisplaced); // add transitions on the left, moves on the right QPointF addTargets_transitionFrom(-50, -50); QPointF addDisplaced_transitionFrom(-50, 50); QPointF moveTargets_transitionFrom(50, -50); QPointF moveDisplaced_transitionFrom(50, 50); + QPointF removeTargets_transitionTo(-100, 300); + QPointF removeDisplaced_transitionFrom(100, 300); QmlListModel model; for (int i = 0; i < initialCount; i++) @@ -4794,8 +4797,12 @@ void tst_QQuickGridView::multipleTransitions() ctxt->setContextProperty("addDisplaced_transitionFrom", addDisplaced_transitionFrom); ctxt->setContextProperty("moveTargets_transitionFrom", moveTargets_transitionFrom); ctxt->setContextProperty("moveDisplaced_transitionFrom", moveDisplaced_transitionFrom); + ctxt->setContextProperty("removeTargets_transitionTo", removeTargets_transitionTo); + ctxt->setContextProperty("removeDisplaced_transitionFrom", removeDisplaced_transitionFrom); + ctxt->setContextProperty("rippleAddDisplaced", rippleAddDisplaced); canvas->setSource(testFileUrl("multipleTransitions.qml")); canvas->show(); + QTest::qWaitForWindowShown(canvas); QQuickGridView *gridview = findItem(canvas->rootObject(), "grid"); QTRY_VERIFY(gridview != 0); @@ -4803,6 +4810,11 @@ void tst_QQuickGridView::multipleTransitions() QVERIFY(contentItem != 0); QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false); + if (contentY != 0) { + gridview->setContentY(contentY); + QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false); + } + int timeBetweenActions = canvas->rootObject()->property("timeBetweenActions").toInt(); QList > targetItems; @@ -4894,18 +4906,21 @@ void tst_QQuickGridView::multipleTransitions_data() QTest::addColumn("initialCount"); QTest::addColumn("contentY"); QTest::addColumn >("changes"); + QTest::addColumn("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::insert(0, 1) - << ListChange::move(0, 3, 1) - ); + << ListChange::insert(0, 1) + << ListChange::move(0, 3, 1) + ) + << 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::move(1, 10, 3) << ListChange::insert(0, 1) - ); + ) + << 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() @@ -4913,7 +4928,14 @@ void tst_QQuickGridView::multipleTransitions_data() << ListChange::setContentY(160.0) << ListChange::setContentY(0.0) << ListChange::insert(0, 1) - ); + ) + << false; + + QTest::newRow("insert then remove same index, with ripple effect on add displaced") << 20 << 0.0 << (QList() + << ListChange::insert(1, 1) + << ListChange::remove(1, 1) + ) + << true; } void tst_QQuickGridView::cacheBuffer() diff --git a/tests/auto/qtquick2/qquicklistview/data/multipleTransitions.qml b/tests/auto/qtquick2/qquicklistview/data/multipleTransitions.qml index 3e32485..8264b42 100644 --- a/tests/auto/qtquick2/qquicklistview/data/multipleTransitions.qml +++ b/tests/auto/qtquick2/qquicklistview/data/multipleTransitions.qml @@ -45,6 +45,8 @@ Rectangle { property bool runningAddDisplaced: false property bool runningMoveTargets: false property bool runningMoveDisplaced: false + property bool runningRemoveTargets: false + property bool runningRemoveDisplaced: false objectName: "list" focus: true @@ -70,6 +72,7 @@ Rectangle { id: addDisplaced SequentialAnimation { ScriptAction { script: list.runningAddDisplaced = true } + PauseAnimation { duration: rippleAddDisplaced ? addDisplaced.ViewTransition.index * root.duration/10 : 0 } ParallelAnimation { NumberAnimation { properties: "x"; from: addDisplaced_transitionFrom.x; duration: root.duration } NumberAnimation { properties: "y"; from: addDisplaced_transitionFrom.y; duration: root.duration } @@ -101,6 +104,30 @@ Rectangle { ScriptAction { script: list.runningMoveDisplaced = false } } } + + remove: Transition { + id: removeTargets + SequentialAnimation { + ScriptAction { script: list.runningRemoveTargets = true } + ParallelAnimation { + NumberAnimation { properties: "x"; to: removeTargets_transitionTo.x; duration: root.duration } + NumberAnimation { properties: "y"; to: removeTargets_transitionTo.y; duration: root.duration } + } + ScriptAction { script: list.runningRemoveTargets = false } + } + } + + removeDisplaced: Transition { + id: removeDisplaced + SequentialAnimation { + ScriptAction { script: list.runningRemoveDisplaced = true } + ParallelAnimation { + NumberAnimation { properties: "x"; from: removeDisplaced_transitionFrom.x; duration: root.duration } + NumberAnimation { properties: "y"; from: removeDisplaced_transitionFrom.y; duration: root.duration } + } + ScriptAction { script: list.runningRemoveDisplaced = false } + } + } } Rectangle { diff --git a/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp b/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp index a834f1a..8f1527f 100644 --- a/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp @@ -5787,12 +5787,14 @@ void tst_QQuickListView::multipleTransitions() QFETCH(int, initialCount); QFETCH(qreal, contentY); QFETCH(QList, changes); + QFETCH(bool, rippleAddDisplaced); - // add transitions on the left, moves on the right QPointF addTargets_transitionFrom(-50, -50); QPointF addDisplaced_transitionFrom(-50, 50); QPointF moveTargets_transitionFrom(50, -50); QPointF moveDisplaced_transitionFrom(50, 50); + QPointF removeTargets_transitionTo(-100, 300); + QPointF removeDisplaced_transitionFrom(100, 300); QmlListModel model; for (int i = 0; i < initialCount; i++) @@ -5807,6 +5809,9 @@ void tst_QQuickListView::multipleTransitions() ctxt->setContextProperty("addDisplaced_transitionFrom", addDisplaced_transitionFrom); ctxt->setContextProperty("moveTargets_transitionFrom", moveTargets_transitionFrom); ctxt->setContextProperty("moveDisplaced_transitionFrom", moveDisplaced_transitionFrom); + ctxt->setContextProperty("removeTargets_transitionTo", removeTargets_transitionTo); + ctxt->setContextProperty("removeDisplaced_transitionFrom", removeDisplaced_transitionFrom); + ctxt->setContextProperty("rippleAddDisplaced", rippleAddDisplaced); canvas->setSource(testFileUrl("multipleTransitions.qml")); canvas->show(); QTest::qWaitForWindowShown(canvas); @@ -5817,6 +5822,11 @@ void tst_QQuickListView::multipleTransitions() QVERIFY(contentItem != 0); QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false); + if (contentY != 0) { + listview->setContentY(contentY); + QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false); + } + int timeBetweenActions = canvas->rootObject()->property("timeBetweenActions").toInt(); QList > targetItems; @@ -5897,18 +5907,21 @@ void tst_QQuickListView::multipleTransitions_data() QTest::addColumn("initialCount"); QTest::addColumn("contentY"); QTest::addColumn >("changes"); + QTest::addColumn("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::insert(0, 1) << ListChange::move(0, 3, 1) - ); + ) + << 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::move(1, 10, 3) << ListChange::insert(0, 1) - ); + ) + << 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() @@ -5916,7 +5929,14 @@ void tst_QQuickListView::multipleTransitions_data() << ListChange::setContentY(80.0) << ListChange::setContentY(0.0) << ListChange::insert(0, 1) - ); + ) + << false; + + QTest::newRow("insert then remove same index, with ripple effect on add displaced") << 20 << 0.0 << (QList() + << ListChange::insert(1, 1) + << ListChange::remove(1, 1) + ) + << true; } QList tst_QQuickListView::toIntList(const QVariantList &list) -- 2.7.4