Don't move content forwards when items are moved down

Previously, if items moved down within a view, the content position
would effectively drop down. E.g. for a (0,5,3) move that moved 3
items from 0 to 5, the content y would move to the position of index
3, since it became the new first item. However, this makes it
difficult to move transitions for move() operations in these cases
since these items do not move (since the content position moves
instead). With this fix, the content position does not move, and items
will always move if they are moved.

Note this behaviour was previously implemented for backwards
movements, e.g. a (5,0,3) move but was not enabled for a forwards
(0,5,3) move.

Change-Id: I1c5a19e3c36347a4aa0cf6e31c975967a7eeada9
Reviewed-on: http://codereview.qt-project.org/5576
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Martin Jones <martin.jones@nokia.com>

src/declarative/items/qsggridview.cpp

@@ -574,6 +574,8 @@ void QSGGridViewPrivate::repositionPackageItemAt(QSGItem *item, int index)
 
 void QSGGridViewPrivate::resetItemPosition(FxViewItem *item, FxViewItem *toItem)
 {
+    if (item == toItem)
+        return;
     FxGridItemSG *toGridItem = static_cast<FxGridItemSG*>(toItem);
     static_cast<FxGridItemSG*>(item)->setPosition(toGridItem->colPos(), toGridItem->rowPos());
 }

src/declarative/items/qsgitemview.cpp

@@ -1489,16 +1489,27 @@ bool QSGItemViewPrivate::applyModelChanges()
     for (int i=0; i<addedItems.count(); ++i)
         addedItems.at(i)->attached->emitAdd();
 
-    // if first visible item is moving but another item is moving up to replace it,
-    // do this positioning now to avoid shifting all content forwards
+    // if the first visible item has moved, ensure another one takes its place
+    // so that we avoid shifting all content forwards
+    // (don't use items from removedBeforeFirstVisible - if an item is removed from
+    // before the first visible, the first visible should not move upwards)
     if (firstVisible && firstItemIndex >= 0) {
+        bool found = false;
         for (int i=0; i<movedBackwards.count(); i++) {
             if (movedBackwards[i]->index == firstItemIndex) {
+                // an item has moved backwards up to the first visible's position
                 resetItemPosition(movedBackwards[i], firstVisible);
                 movedBackwards.removeAt(i);
+                found = true;
                 break;
             }
         }
+        if (!found) {
+            // first visible item has moved forward, another item takes its place
+            FxViewItem *item = visibleItem(firstItemIndex);
+            if (item)
+                resetItemPosition(item, firstVisible);
+        }
     }
 
     // Ensure we don't cause an ugly list scroll

src/declarative/items/qsglistview.cpp

@@ -716,6 +716,8 @@ void QSGListViewPrivate::repositionPackageItemAt(QSGItem *item, int index)
 
 void QSGListViewPrivate::resetItemPosition(FxViewItem *item, FxViewItem *toItem)
 {
+    if (item == toItem)
+        return;
     static_cast<FxListItemSG*>(item)->setPosition(toItem->position());
 }
 

tests/auto/declarative/qsggridview/tst_qsggridview.cpp

@@ -737,7 +737,12 @@ void tst_QSGGridView::moved_data()
     QTest::newRow("move multiple forwards, within visible items")
             << 0.0
             << 0 << 5 << 3
-            << 60.0;    // moved 3 items (i.e. 1 row) down
+            << 0.0;
+
+    QTest::newRow("move multiple forwards, before visible items")
+            << 120.0     // show 6-23
+            << 3 << 4 << 3      // 3, 4, 5 move to after 6
+            << 60.0;      // row of 3,4,5 has moved down
 
     QTest::newRow("move multiple forwards, from non-visible -> visible")
             << 120.0     // show 6-23
@@ -757,7 +762,7 @@ void tst_QSGGridView::moved_data()
     QTest::newRow("move multiple forwards, from visible -> non-visible (move first item)")
             << 0.0
             << 0 << 16 << 3
-            << 60.0;
+            << 0.0;
 
 
     QTest::newRow("move multiple backwards, within visible items")

tests/auto/declarative/qsglistview/tst_qsglistview.cpp

@@ -981,7 +981,7 @@ void tst_QSGListView::moved_data()
     QTest::newRow("move 1 forwards, from visible -> non-visible (move first item)")
             << 0.0
             << 0 << 16 << 1
-            << 20.0;
+            << 0.0;
 
 
     QTest::newRow("move 1 backwards, within visible items")
@@ -1018,7 +1018,12 @@ void tst_QSGListView::moved_data()
     QTest::newRow("move multiple forwards, within visible items")
             << 0.0
             << 0 << 5 << 3
-            << 20.0 * 3;
+            << 0.0;
+
+    QTest::newRow("move multiple forwards, before visible items")
+            << 140.0     // show 7-22
+            << 4 << 5 << 3      // 4,5,6 move to below 7
+            << 20.0 * 3;      // 4,5,6 moved down
 
     QTest::newRow("move multiple forwards, from non-visible -> visible")
             << 80.0     // show 4-19
@@ -1038,7 +1043,7 @@ void tst_QSGListView::moved_data()
     QTest::newRow("move multiple forwards, from visible -> non-visible (move first item)")
             << 0.0
             << 0 << 16 << 3
-            << 20.0 * 3;
+            << 0.0;
 
 
     QTest::newRow("move multiple backwards, within visible items")