From 62b1bc426929c5f990f00bee7d3de8a0024ed7c5 Mon Sep 17 00:00:00 2001 From: Alberto Mardegan Date: Mon, 21 Nov 2016 22:41:33 +0300 Subject: QQuickItemView: always honor the removeDisplaced animation The animation was not being performed if the delayRemove attached property was changed by the handler of the remove() attached signal. We need to run the delayed transitions not only if we have an animation for the target item, but also if we have an animation for the items being displaced. (The flag variables can safely be obtained outside of the for loop, given that their value should not change during the loop iteration) Task-number: QTBUG-57225 Change-Id: I8c138677d7dcdf63e0932ec5cf7738c0caeb2ab8 Reviewed-by: J-P Nurmi Reviewed-by: Shawn Rutledge --- src/quick/items/qquickitemview.cpp | 12 +++++++- .../auto/quick/qquickgridview/data/qtbug57225.qml | 3 +- .../quick/qquickgridview/tst_qquickgridview.cpp | 34 ++++++++++++++++++++-- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 8f3fe9f0f2..66438ee37d 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -1246,16 +1246,26 @@ void QQuickItemViewPrivate::itemGeometryChanged(QQuickItem *item, QQuickGeometry void QQuickItemView::destroyRemoved() { Q_D(QQuickItemView); + + bool hasRemoveTransition = false; + bool hasRemoveTransitionAsTarget = false; + if (d->transitioner) { + hasRemoveTransition = d->transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, false); + hasRemoveTransitionAsTarget = d->transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, true); + } + for (QList::Iterator it = d->visibleItems.begin(); it != d->visibleItems.end();) { FxViewItem *item = *it; if (item->index == -1 && (!item->attached || item->attached->delayRemove() == false)) { - if (d->transitioner && d->transitioner->canTransition(QQuickItemViewTransitioner::RemoveTransition, true)) { + if (hasRemoveTransitionAsTarget) { // don't remove from visibleItems until next layout() d->runDelayedRemoveTransition = true; QObject::disconnect(item->attached, SIGNAL(delayRemoveChanged()), this, SLOT(destroyRemoved())); ++it; } else { + if (hasRemoveTransition) + d->runDelayedRemoveTransition = true; d->releaseItem(item); it = d->visibleItems.erase(it); } diff --git a/tests/auto/quick/qquickgridview/data/qtbug57225.qml b/tests/auto/quick/qquickgridview/data/qtbug57225.qml index 7f5e67822c..3871e5d273 100644 --- a/tests/auto/quick/qquickgridview/data/qtbug57225.qml +++ b/tests/auto/quick/qquickgridview/data/qtbug57225.qml @@ -52,6 +52,7 @@ Rectangle { GridView { id: grid + property int displaceTransitionsDone: 0 property bool animationDone: false property point minimumPosition: Qt.point(0, 0) @@ -73,8 +74,8 @@ Rectangle { properties: "x,y" duration: root.duration easing.type: Easing.OutBounce - easing.amplitude: 10.0 // longer-lasting bounce to trigger bug } + ScriptAction { script: grid.displaceTransitionsDone += 1 } } } diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index 7333cc3ceb..388ecc2ab8 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -154,6 +154,7 @@ private slots: void multipleTransitions_data(); void multipleDisplaced(); void regression_QTBUG_57225(); + void regression_QTBUG_57225_data(); void inserted_leftToRight_RtL_TtB(); void inserted_leftToRight_RtL_TtB_data(); @@ -5801,10 +5802,15 @@ void tst_QQuickGridView::multipleDisplaced() void tst_QQuickGridView::regression_QTBUG_57225() { + QFETCH(int, initialCount); + QFETCH(int, removeIndex); + QFETCH(int, removeCount); + QFETCH(int, expectedDisplaceTransitions); + // deleting all visible items should not cause a repositioning of said items. QaimModel model; - for (int i = 0; i < 20; i++) + for (int i = 0; i < initialCount; i++) model.addItem("Original item" + QString::number(i), ""); QQuickView *window = createView(); @@ -5818,7 +5824,7 @@ void tst_QQuickGridView::regression_QTBUG_57225() QVERIFY(gridview != 0); QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false); - model.removeItems(0, 8); + model.removeItems(removeIndex, removeCount); QTRY_VERIFY(gridview->property("animationDone").toBool()); // verify that none of the removed items has moved to a negative position @@ -5826,9 +5832,33 @@ void tst_QQuickGridView::regression_QTBUG_57225() QVERIFY(minimumPosition.x() >= 0); QVERIFY(minimumPosition.y() >= 0); + // wait some more time to let the displaced transition happen + QTest::qWait(window->rootObject()->property("duration").toInt()); + QTRY_VERIFY2(gridview->property("displaceTransitionsDone").toInt() >= expectedDisplaceTransitions, + QByteArray::number(gridview->property("displaceTransitionsDone").toInt()).constData()); + delete window; } +void tst_QQuickGridView::regression_QTBUG_57225_data() +{ + QTest::addColumn("initialCount"); + QTest::addColumn("removeIndex"); + QTest::addColumn("removeCount"); + QTest::addColumn("expectedDisplaceTransitions"); + + // no displace transitions should happen + QTest::newRow("remove all visible items") << + 20 << 0 << 8 << 0; + + // check that the removal animation is performed + QTest::newRow("remove items in between") << + 20 << 1 << 2 << 3; + + QTest::newRow("remove items in between - 2") << + 20 << 2 << 3 << 1; +} + void tst_QQuickGridView::cacheBuffer() { QQuickView *window = createView(); -- cgit v1.2.3