From ed74ec4c40f1476c545bcaacb12fe3a607172035 Mon Sep 17 00:00:00 2001 From: Bea Lam Date: Mon, 5 Mar 2012 18:05:40 +1000 Subject: 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 --- src/quick/items/qquickgridview.cpp | 8 ++--- src/quick/items/qquickitemview.cpp | 4 +-- src/quick/items/qquickitemview_p_p.h | 2 +- src/quick/items/qquickitemviewtransition.cpp | 13 +++++--- src/quick/items/qquickitemviewtransition_p.h | 2 +- src/quick/items/qquicklistview.cpp | 8 ++--- .../qquickgridview/data/multipleTransitions.qml | 6 ++++ .../quick/qquickgridview/tst_qquickgridview.cpp | 39 ++++++++++++++++++---- .../qquicklistview/data/multipleTransitions.qml | 6 ++++ .../quick/qquicklistview/tst_qquicklistview.cpp | 35 ++++++++++++++++--- 10 files changed, 97 insertions(+), 26 deletions(-) diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index 7c4f95faed..78899b27e5 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -120,8 +120,8 @@ public: 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(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) { diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index aafddb40c8..c6f45aaf0b 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/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); } diff --git a/src/quick/items/qquickitemview_p_p.h b/src/quick/items/qquickitemview_p_p.h index 216f10aa23..dfc0a8bc7e 100644 --- a/src/quick/items/qquickitemview_p_p.h +++ b/src/quick/items/qquickitemview_p_p.h @@ -66,7 +66,7 @@ public: 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; diff --git a/src/quick/items/qquickitemviewtransition.cpp b/src/quick/items/qquickitemviewtransition.cpp index 7e804ae5bf..d9dce49349 100644 --- a/src/quick/items/qquickitemviewtransition.cpp +++ b/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); } } diff --git a/src/quick/items/qquickitemviewtransition_p.h b/src/quick/items/qquickitemviewtransition_p.h index d50b056593..a4babdca05 100644 --- a/src/quick/items/qquickitemviewtransition_p.h +++ b/src/quick/items/qquickitemviewtransition_p.h @@ -132,7 +132,7 @@ public: 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; diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp index 0b9d1c2f7e..9db2060d89 100644 --- a/src/quick/items/qquicklistview.cpp +++ b/src/quick/items/qquicklistview.cpp @@ -292,7 +292,7 @@ public: : 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 @@ public: 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(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; diff --git a/tests/auto/quick/qquickgridview/data/multipleTransitions.qml b/tests/auto/quick/qquickgridview/data/multipleTransitions.qml index f0d932082b..cfe0be7b1e 100644 --- a/tests/auto/quick/qquickgridview/data/multipleTransitions.qml +++ b/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 { diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index 66c98cda84..04bdfc8ac3 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -4795,6 +4795,9 @@ void tst_QQuickGridView::multipleTransitions() QFETCH(int, initialCount); QFETCH(qreal, contentY); QFETCH(QList, 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(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(contentItem, "textName", i); QVERIFY(name != 0); QTRY_COMPARE(name->text(), model.name(i)); @@ -4916,6 +4922,9 @@ void tst_QQuickGridView::multipleTransitions_data() QTest::addColumn("initialCount"); QTest::addColumn("contentY"); QTest::addColumn >("changes"); + QTest::addColumn("enableAddTransitions"); + QTest::addColumn("enableMoveTransitions"); + QTest::addColumn("enableRemoveTransitions"); QTest::addColumn("rippleAddDisplaced"); // the added item and displaced items should move to final dest correctly @@ -4923,14 +4932,14 @@ void tst_QQuickGridView::multipleTransitions_data() << 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::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() @@ -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::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::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::setContentY(-10.0) + << ListChange::insert(0, 1) + << ListChange::insert(0, 1) + << ListChange::remove(2, 1) + ) + << true << true << false << false; } void tst_QQuickGridView::cacheBuffer() diff --git a/tests/auto/quick/qquicklistview/data/multipleTransitions.qml b/tests/auto/quick/qquicklistview/data/multipleTransitions.qml index 68efeea2ec..c0e888c6c6 100644 --- a/tests/auto/quick/qquicklistview/data/multipleTransitions.qml +++ b/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 { diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp index dcc2e9dd16..c038faa0f1 100644 --- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp @@ -5807,6 +5807,9 @@ void tst_QQuickListView::multipleTransitions() QFETCH(int, initialCount); QFETCH(qreal, contentY); QFETCH(QList, 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,6 +5924,9 @@ void tst_QQuickListView::multipleTransitions_data() QTest::addColumn("initialCount"); QTest::addColumn("contentY"); QTest::addColumn >("changes"); + QTest::addColumn("enableAddTransitions"); + QTest::addColumn("enableMoveTransitions"); + QTest::addColumn("enableRemoveTransitions"); QTest::addColumn("rippleAddDisplaced"); // the added item and displaced items should move to final dest correctly @@ -5925,14 +5934,14 @@ void tst_QQuickListView::multipleTransitions_data() << 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::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() @@ -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::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::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::setContentY(-10.0) + << ListChange::insert(0, 1) + << ListChange::insert(0, 1) + << ListChange::remove(2, 1) + ) + << true << true << false << false; } QList tst_QQuickListView::toIntList(const QVariantList &list) -- cgit v1.2.3