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 <martin.jones@nokia.com>

diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp
index 60514b2..8f8fac7 100644 (file)
--- 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<FxGridItemSG*>(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 (file)
--- a/src/quick/items/qquickitemview.cpp
+++ b/src/quick/items/qquickitemview.cpp
@@ -1426,8 +1426,11 @@ bool QQuickItemViewPrivate::applyModelChanges()
 
     FxViewItem *prevFirstVisible = firstVisibleItem();
     QDeclarativeNullableValue<qreal> 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<QDeclarativeChangeSet::Remove> &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 (file)
--- 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 (file)
--- 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<QQuickItem>(leftContent, "wrapper", 3));
     QCOMPARE(item->isVisible(), false);
-    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 4));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 5));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(rightContent, "wrapper", 9));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(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<QQuickItem>(leftContent, "wrapper", 1));
@@ -4113,55 +4129,59 @@ void tst_QQuickGridView::unrequestedVisibility()
 
     QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 3));
     QCOMPARE(item->isVisible(), false);
-    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 4));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 5));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(rightContent, "wrapper", 9));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(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<QQuickItem>(leftContent, "wrapper", 1));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 3));
     QCOMPARE(item->isVisible(), false);
-    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 2));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 5));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(rightContent, "wrapper", 9));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(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<QQuickItem>(leftContent, "wrapper", 1));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 3));
     QCOMPARE(item->isVisible(), false);
-    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 2));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 5));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(rightContent, "wrapper", 9));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(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<QQuickItem>(leftContent, "wrapper", 1));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 3));
     QCOMPARE(item->isVisible(), false);
-    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 2));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 5));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(rightContent, "wrapper", 9));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(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<QQuickItem>(leftContent, "wrapper", 1));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 3));
     QCOMPARE(item->isVisible(), false);
-    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 2));
+    QVERIFY(item = findItem<QQuickItem>(leftContent, "wrapper", 5));
     QCOMPARE(item->isVisible(), true);
     QVERIFY(item = findItem<QQuickItem>(rightContent, "wrapper", 9));
     QCOMPARE(item->isVisible(), true);