From 0d84aaf05c7306c8e39bac7acd7c85dc04358b17 Mon Sep 17 00:00:00 2001 From: Alberto Mardegan Date: Mon, 21 Nov 2016 22:35:37 +0300 Subject: QQuickItemView: avoid wrong repositioning of removed items If all the items currently in the view are being removed, lastVisibleIndex will be -1. This caused an unwanted repositioning of all the deleted items, which got moved to a wrong location. Therefore, when all visible items are removed, we should avoid recomputing any item's position. Task-number: QTBUG-57225 Change-Id: I9909748a9cccb5e6a3726306e250921ce69fcba9 Reviewed-by: J-P Nurmi Reviewed-by: Shawn Rutledge --- .../quick/qquickgridview/tst_qquickgridview.cpp | 31 ++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'tests/auto/quick/qquickgridview/tst_qquickgridview.cpp') diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index 2b14842658..7333cc3ceb 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -153,6 +153,7 @@ private slots: void multipleTransitions(); void multipleTransitions_data(); void multipleDisplaced(); + void regression_QTBUG_57225(); void inserted_leftToRight_RtL_TtB(); void inserted_leftToRight_RtL_TtB_data(); @@ -5798,6 +5799,36 @@ void tst_QQuickGridView::multipleDisplaced() delete window; } +void tst_QQuickGridView::regression_QTBUG_57225() +{ + // deleting all visible items should not cause a repositioning of said items. + + QaimModel model; + for (int i = 0; i < 20; i++) + model.addItem("Original item" + QString::number(i), ""); + + QQuickView *window = createView(); + QQmlContext *ctxt = window->rootContext(); + ctxt->setContextProperty("testModel", &model); + window->setSource(testFileUrl("qtbug57225.qml")); + window->show(); + QVERIFY(QTest::qWaitForWindowExposed(window)); + + QQuickGridView *gridview = findItem(window->rootObject(), "grid"); + QVERIFY(gridview != 0); + QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false); + + model.removeItems(0, 8); + QTRY_VERIFY(gridview->property("animationDone").toBool()); + + // verify that none of the removed items has moved to a negative position + QPoint minimumPosition = gridview->property("minimumPosition").toPoint(); + QVERIFY(minimumPosition.x() >= 0); + QVERIFY(minimumPosition.y() >= 0); + + delete window; +} + void tst_QQuickGridView::cacheBuffer() { QQuickView *window = createView(); -- cgit v1.2.3 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 --- .../quick/qquickgridview/tst_qquickgridview.cpp | 34 ++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) (limited to 'tests/auto/quick/qquickgridview/tst_qquickgridview.cpp') 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