From 279e4ae7dd9c0b60c2e4dccb31363eb6bac04918 Mon Sep 17 00:00:00 2001 From: Bea Lam Date: Wed, 25 Jan 2012 17:24:05 +1000 Subject: [PATCH] Fixes for removing before visible Account for removing multiple items over visible and non-visible items and calculate these using the index of the first visible item rather than the visibleIndex (which can be above the visible position). Also, don't take the changeBeforeVisible into account if the first visible is already being adjusted, otherwise it moves back one more row than it should. Change-Id: Iea7247102e06697d10eb41301ca689c0cdc35ece Reviewed-by: Martin Jones --- src/quick/items/qquickgridview.cpp | 5 +- src/quick/items/qquickitemview.cpp | 14 ++++-- .../qquickgridview/data/unrequestedItems.qml | 12 +++-- .../qtquick2/qquickgridview/tst_qquickgridview.cpp | 50 ++++++++++++++------ 4 files changed, 55 insertions(+), 26 deletions(-) diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index 60514b2..8f8fac7 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -609,9 +609,8 @@ void QQuickGridViewPrivate::adjustFirstItem(qreal forwards, qreal backwards, int return; int moveCount = (forwards - backwards) / rowSize(); - - if (changeBeforeVisible) - moveCount += (changeBeforeVisible%columns) - (columns - 1); + if (moveCount == 0 && changeBeforeVisible != 0) + moveCount += (changeBeforeVisible % columns) - (columns - 1); FxGridItemSG *gridItem = static_cast(visibleItems.first()); gridItem->setPosition(gridItem->colPos(), gridItem->rowPos() + ((moveCount / columns) * rowSize())); diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 6577617..3b264ab 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -1426,8 +1426,11 @@ bool QQuickItemViewPrivate::applyModelChanges() FxViewItem *prevFirstVisible = firstVisibleItem(); QDeclarativeNullableValue prevViewPos; - if (prevFirstVisible) + int prevFirstVisibleIndex = -1; + if (prevFirstVisible) { prevViewPos = prevFirstVisible->position(); + prevFirstVisibleIndex = prevFirstVisible->index; + } qreal prevVisibleItemsFirstPos = visibleItems.count() ? visibleItems.first()->position() : 0.0; const QVector &removals = currentChanges.pendingChanges.removes(); @@ -1442,6 +1445,12 @@ bool QQuickItemViewPrivate::applyModelChanges() visibleAffected = true; if (!visibleAffected && needsRefillForAddedOrRemovedIndex(removals[i].index)) visibleAffected = true; + if (prevFirstVisibleIndex >= 0 && removals[i].index < prevFirstVisibleIndex) { + if (removals[i].index + removals[i].count < prevFirstVisibleIndex) + removalResult.changeBeforeVisible -= removals[i].count; + else + removalResult.changeBeforeVisible -= (prevFirstVisibleIndex - removals[i].index); + } } if (!removals.isEmpty()) { updateVisibleIndex(); @@ -1557,9 +1566,6 @@ bool QQuickItemViewPrivate::applyRemovalChange(const QDeclarativeChangeSet::Remo } } - if (removal.index + removal.count < visibleIndex) - removeResult->changeBeforeVisible -= removal.count; - return visibleAffected; } diff --git a/tests/auto/qtquick2/qquickgridview/data/unrequestedItems.qml b/tests/auto/qtquick2/qquickgridview/data/unrequestedItems.qml index ffd52d2..79f845f 100644 --- a/tests/auto/qtquick2/qquickgridview/data/unrequestedItems.qml +++ b/tests/auto/qtquick2/qquickgridview/data/unrequestedItems.qml @@ -14,8 +14,10 @@ Item { Package.name: "left" height: 80 width: 60 - Text { - text: index + Column { + Text { text: index } + Text { text: name } + Text { text: leftWrapper.x + ", " + leftWrapper.y } } color: ListView.isCurrentItem ? "lightsteelblue" : "white" } @@ -25,8 +27,10 @@ Item { Package.name: "right" height: 80 width: 60 - Text { - text: index + Column { + Text { text: index } + Text { text: name } + Text { text: rightWrapper.x + ", " + rightWrapper.y } } color: ListView.isCurrentItem ? "lightsteelblue" : "white" } diff --git a/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp b/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp index 32934b5..303f4bd 100644 --- a/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp @@ -628,8 +628,8 @@ void tst_QQuickGridView::insertBeforeVisible() gridview->setCacheBuffer(cacheBuffer); // trigger a refill (not just setting contentY) so that the visibleItems grid is updated - int firstVisibleIndex = 20; // move to an index where the top item is not visible - gridview->setContentY(firstVisibleIndex * 20.0); + int firstVisibleIndex = 12; // move to an index where the top item is not visible + gridview->setContentY(firstVisibleIndex/3 * 60.0); gridview->setCurrentIndex(firstVisibleIndex); qApp->processEvents(); @@ -914,6 +914,11 @@ void tst_QQuickGridView::removed_more_data() QTest::newRow("remove 1, before visible items") << 120.0 // show 6-23 + << 2 << 1 + << 0.0 << "Item7"; + + QTest::newRow("remove 1, before visible position") + << 120.0 // show 6-23 << 3 << 1 << 0.0 << "Item7"; @@ -932,6 +937,16 @@ void tst_QQuickGridView::removed_more_data() << 1 << 7 << 120.0 << "Item13"; + QTest::newRow("remove one row before visible, content y not on item border") + << 100.0 + << 0 << 3 + << 60.0 << "Item6"; // 1 row removed + + QTest::newRow("remove mix of visible/non-visible") + << 120.0 // show 6-23 + << 2 << 3 + << 60.0 << "Item6"; // 1 row removed + // remove 3,4,5 before the visible pos, first row moves down to just before the visible pos, // items 6,7 are removed from view, item 8 slides up to original pos of item 6 (120px) @@ -4091,14 +4106,15 @@ void tst_QQuickGridView::unrequestedVisibility() QVERIFY(item = findItem(leftContent, "wrapper", 3)); QCOMPARE(item->isVisible(), false); - QVERIFY(item = findItem(leftContent, "wrapper", 4)); + QVERIFY(item = findItem(leftContent, "wrapper", 5)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 9)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 10)); QCOMPARE(item->isVisible(), false); - model.moveItems(19, 1, 1); + // move a non-visible item into view + model.moveItems(10, 9, 1); QTRY_COMPARE(QQuickItemPrivate::get(leftview)->polishScheduled, false); QTRY_VERIFY(item = findItem(leftContent, "wrapper", 1)); @@ -4113,55 +4129,59 @@ void tst_QQuickGridView::unrequestedVisibility() QVERIFY(item = findItem(leftContent, "wrapper", 3)); QCOMPARE(item->isVisible(), false); - QVERIFY(item = findItem(leftContent, "wrapper", 4)); + QVERIFY(item = findItem(leftContent, "wrapper", 5)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 9)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 10)); QCOMPARE(item->isVisible(), false); - model.moveItems(3, 4, 1); + // move a visible item out of view + model.moveItems(5, 3, 1); QTRY_COMPARE(QQuickItemPrivate::get(leftview)->polishScheduled, false); - QVERIFY(item = findItem(leftContent, "wrapper", 1)); + QVERIFY(item = findItem(leftContent, "wrapper", 3)); QCOMPARE(item->isVisible(), false); - QVERIFY(item = findItem(leftContent, "wrapper", 2)); + QVERIFY(item = findItem(leftContent, "wrapper", 5)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 9)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 10)); QCOMPARE(item->isVisible(), false); - model.moveItems(4, 5, 1); + // move a non-visible item into view + model.moveItems(3, 5, 1); QTRY_COMPARE(QQuickItemPrivate::get(leftview)->polishScheduled, false); - QVERIFY(item = findItem(leftContent, "wrapper", 1)); + QVERIFY(item = findItem(leftContent, "wrapper", 3)); QCOMPARE(item->isVisible(), false); - QVERIFY(item = findItem(leftContent, "wrapper", 2)); + QVERIFY(item = findItem(leftContent, "wrapper", 5)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 9)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 10)); QCOMPARE(item->isVisible(), false); + // move a visible item out of view model.moveItems(9, 10, 1); QTRY_COMPARE(QQuickItemPrivate::get(leftview)->polishScheduled, false); - QVERIFY(item = findItem(leftContent, "wrapper", 1)); + QVERIFY(item = findItem(leftContent, "wrapper", 3)); QCOMPARE(item->isVisible(), false); - QVERIFY(item = findItem(leftContent, "wrapper", 2)); + QVERIFY(item = findItem(leftContent, "wrapper", 5)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 9)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 10)); QCOMPARE(item->isVisible(), false); + // move a non-visible item into view model.moveItems(10, 9, 1); QTRY_COMPARE(QQuickItemPrivate::get(leftview)->polishScheduled, false); - QVERIFY(item = findItem(leftContent, "wrapper", 1)); + QVERIFY(item = findItem(leftContent, "wrapper", 3)); QCOMPARE(item->isVisible(), false); - QVERIFY(item = findItem(leftContent, "wrapper", 2)); + QVERIFY(item = findItem(leftContent, "wrapper", 5)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 9)); QCOMPARE(item->isVisible(), true); -- 1.7.2.5