Fix inserting before the visible area into cache buffer

Items being inserted after the visible index must be created, even if
they aren't in view (e.g. are in the cache buffer) otherwise they will
not appear at the correct index in the visibleItems list.

Task-number: QTBUG-22772
Change-Id: I235dc766a6abf4988872bb70aa40cdc767df8c96
Reviewed-by: Martin Jones <martin.jones@nokia.com>

diff --git a/src/declarative/items/qquickgridview.cpp b/src/declarative/items/qquickgridview.cpp
index f50c362..51131ab 100644 (file)
--- a/src/declarative/items/qquickgridview.cpp
+++ b/src/declarative/items/qquickgridview.cpp
@@ -1802,9 +1802,11 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In
         int from = tempPos - buffer;
 
         while (i >= 0) {
-            if (rowPos > from) {
+            if (rowPos > from && insertionIdx < visibleIndex) {
+                // item won't be visible, just note the size for repositioning
                 insertResult->sizeAddedBeforeVisible += rowSize();
             } else {
+                // item is before first visible e.g. in cache buffer
                 FxViewItem *item = 0;
                 if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) {
                     if (item->index > modelIndex + i)

diff --git a/src/declarative/items/qquicklistview.cpp b/src/declarative/items/qquicklistview.cpp
index 4fd4e97..60164d5 100644 (file)
--- a/src/declarative/items/qquicklistview.cpp
+++ b/src/declarative/items/qquicklistview.cpp
@@ -2384,10 +2384,12 @@ bool QQuickListViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In
         int from = tempPos - buffer;
 
         for (i = count-1; i >= 0; --i) {
-            if (pos > from) {
-                insertResult->sizeAddedBeforeVisible += averageSize;
-                pos -= averageSize;
+            if (pos > from && insertionIdx < visibleIndex) {
+                // item won't be visible, just note the size for repositioning
+                insertResult->sizeAddedBeforeVisible += averageSize + spacing;
+                pos -= averageSize + spacing;
             } else {
+                // item is before first visible e.g. in cache buffer
                 FxViewItem *item = 0;
                 if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) {
                     if (item->index > modelIndex + i)

diff --git a/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp b/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp
index bdcb27f..1dbacb1 100644 (file)
--- a/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp
+++ b/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp
@@ -71,6 +71,8 @@ private slots:
     void inserted();
     void inserted_more();
     void inserted_more_data();
+    void insertBeforeVisible();
+    void insertBeforeVisible_data();
     void removed();
     void addOrRemoveBeforeVisible();
     void addOrRemoveBeforeVisible_data();
@@ -597,6 +599,94 @@ void tst_QQuickGridView::inserted_more_data()
             << 0.0;
 }
 
+void tst_QQuickGridView::insertBeforeVisible()
+{
+    QFETCH(int, insertIndex);
+    QFETCH(int, insertCount);
+    QFETCH(int, cacheBuffer);
+
+    QQuickText *name;
+    QQuickView *canvas = createView();
+    canvas->show();
+
+    TestModel model;
+    for (int i = 0; i < 30; i++)
+        model.addItem("Item" + QString::number(i), "");
+
+    QDeclarativeContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+    ctxt->setContextProperty("testRightToLeft", QVariant(false));
+    ctxt->setContextProperty("testTopToBottom", QVariant(false));
+    canvas->setSource(QUrl::fromLocalFile(TESTDATA("gridview1.qml")));
+    qApp->processEvents();
+
+    QQuickGridView *gridview = findItem<QQuickGridView>(canvas->rootObject(), "grid");
+    QTRY_VERIFY(gridview != 0);
+    QQuickItem *contentItem = gridview->contentItem();
+    QTRY_VERIFY(contentItem != 0);
+
+    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);
+    gridview->setCurrentIndex(firstVisibleIndex);
+    qApp->processEvents();
+    QTRY_COMPARE(gridview->currentIndex(), firstVisibleIndex);
+    QQuickItem *item = findItem<QQuickItem>(contentItem, "wrapper", firstVisibleIndex);
+    QVERIFY(item);
+    QCOMPARE(item->y(), gridview->contentY());
+
+    QList<QPair<QString, QString> > newData;
+    for (int i=0; i<insertCount; i++)
+        newData << qMakePair(QString("value %1").arg(i), QString::number(i));
+    model.insertItems(insertIndex, newData);
+    QTRY_COMPARE(gridview->property("count").toInt(), model.count());
+
+    // now, moving to the top of the view should position the inserted items correctly
+    int itemsOffsetAfterMove = (insertCount / 3) * -60.0;
+    gridview->setCurrentIndex(0);
+    QTRY_COMPARE(gridview->currentIndex(), 0);
+    QTRY_COMPARE(gridview->contentY(), 0.0 + itemsOffsetAfterMove);
+
+    // Confirm items positioned correctly and indexes correct
+    int itemCount = findItems<QQuickItem>(contentItem, "wrapper").count();
+    for (int i = 0; i < model.count() && i < itemCount; ++i) {
+        item = findItem<QQuickItem>(contentItem, "wrapper", i);
+        QVERIFY2(item, QTest::toString(QString("Item %1 not found").arg(i)));
+        QCOMPARE(item->x(), (i%3)*80.0);
+        QCOMPARE(item->y(), (i/3)*60.0 + itemsOffsetAfterMove);
+        name = findItem<QQuickText>(contentItem, "textName", i);
+        QVERIFY(name != 0);
+        QTRY_COMPARE(name->text(), model.name(i));
+    }
+
+    delete canvas;
+}
+
+void tst_QQuickGridView::insertBeforeVisible_data()
+{
+    QTest::addColumn<int>("insertIndex");
+    QTest::addColumn<int>("insertCount");
+    QTest::addColumn<int>("cacheBuffer");
+
+    QTest::newRow("insert 1 at 0, 0 buffer") << 0 << 1 << 0;
+    QTest::newRow("insert 1 at 0, 100 buffer") << 0 << 1 << 100;
+    QTest::newRow("insert 1 at 0, 500 buffer") << 0 << 1 << 500;
+
+    QTest::newRow("insert 1 at 1, 0 buffer") << 1 << 1 << 0;
+    QTest::newRow("insert 1 at 1, 100 buffer") << 1 << 1 << 100;
+    QTest::newRow("insert 1 at 1, 500 buffer") << 1 << 1 << 500;
+
+    QTest::newRow("insert multiple at 0, 0 buffer") << 0 << 6 << 0;
+    QTest::newRow("insert multiple at 0, 100 buffer") << 0 << 6 << 100;
+    QTest::newRow("insert multiple at 0, 500 buffer") << 0 << 6 << 500;
+
+    QTest::newRow("insert multiple at 1, 0 buffer") << 1 << 6 << 0;
+    QTest::newRow("insert multiple at 1, 100 buffer") << 1 << 6 << 100;
+    QTest::newRow("insert multiple at 1, 500 buffer") << 1 << 6 << 500;
+}
+
 void tst_QQuickGridView::removed()
 {
     QQuickView *canvas = createView();

diff --git a/tests/auto/declarative/qquicklistview/tst_qquicklistview.cpp b/tests/auto/declarative/qquicklistview/tst_qquicklistview.cpp
index b2e6645..f7830b7 100644 (file)
--- a/tests/auto/declarative/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/declarative/qquicklistview/tst_qquicklistview.cpp
@@ -96,6 +96,8 @@ private slots:
     void qListModelInterface_clear();
     void qAbstractItemModel_clear();
 
+    void insertBeforeVisible();
+    void insertBeforeVisible_data();
     void swapWithFirstItem();
     void itemList();
     void currentIndex_delayedItemCreation();
@@ -818,6 +820,96 @@ void tst_QQuickListView::inserted_more_data()
             << 0.0;
 }
 
+void tst_QQuickListView::insertBeforeVisible()
+{
+    QFETCH(int, insertIndex);
+    QFETCH(int, insertCount);
+    QFETCH(int, cacheBuffer);
+
+    QQuickText *name;
+    QQuickView *canvas = createView();
+    canvas->show();
+
+    TestModel model;
+    for (int i = 0; i < 30; i++)
+        model.addItem("Item" + QString::number(i), "");
+
+    QDeclarativeContext *ctxt = canvas->rootContext();
+    ctxt->setContextProperty("testModel", &model);
+
+    TestObject *testObject = new TestObject;
+    ctxt->setContextProperty("testObject", testObject);
+
+    canvas->setSource(QUrl::fromLocalFile(TESTDATA("listviewtest.qml")));
+    qApp->processEvents();
+
+    QQuickListView *listview = findItem<QQuickListView>(canvas->rootObject(), "list");
+    QTRY_VERIFY(listview != 0);
+    QQuickItem *contentItem = listview->contentItem();
+    QTRY_VERIFY(contentItem != 0);
+
+    listview->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
+    listview->setContentY(firstVisibleIndex * 20.0);
+    listview->setCurrentIndex(firstVisibleIndex);
+    qApp->processEvents();
+    QTRY_COMPARE(listview->currentIndex(), firstVisibleIndex);
+    QQuickItem *item = findItem<QQuickItem>(contentItem, "wrapper", firstVisibleIndex);
+    QVERIFY(item);
+    QCOMPARE(item->y(), listview->contentY());
+
+    QList<QPair<QString, QString> > newData;
+    for (int i=0; i<insertCount; i++)
+        newData << qMakePair(QString("value %1").arg(i), QString::number(i));
+    model.insertItems(insertIndex, newData);
+    QTRY_COMPARE(listview->property("count").toInt(), model.count());
+
+    // now, moving to the top of the view should position the inserted items correctly
+    int itemsOffsetAfterMove = -(insertCount * 20);
+    listview->setCurrentIndex(0);
+    QTRY_COMPARE(listview->currentIndex(), 0);
+    QTRY_COMPARE(listview->contentY(), 0.0 + itemsOffsetAfterMove);
+
+    // Confirm items positioned correctly and indexes correct
+    int itemCount = findItems<QQuickItem>(contentItem, "wrapper").count();
+    for (int i = 0; i < model.count() && i < itemCount; ++i) {
+        item = findItem<QQuickItem>(contentItem, "wrapper", i);
+        QVERIFY2(item, QTest::toString(QString("Item %1 not found").arg(i)));
+        QTRY_COMPARE(item->y(), i*20.0 + itemsOffsetAfterMove);
+        name = findItem<QQuickText>(contentItem, "textName", i);
+        QVERIFY(name != 0);
+        QTRY_COMPARE(name->text(), model.name(i));
+    }
+
+    delete canvas;
+    delete testObject;
+}
+
+void tst_QQuickListView::insertBeforeVisible_data()
+{
+    QTest::addColumn<int>("insertIndex");
+    QTest::addColumn<int>("insertCount");
+    QTest::addColumn<int>("cacheBuffer");
+
+    QTest::newRow("insert 1 at 0, 0 buffer") << 0 << 1 << 0;
+    QTest::newRow("insert 1 at 0, 100 buffer") << 0 << 1 << 100;
+    QTest::newRow("insert 1 at 0, 500 buffer") << 0 << 1 << 500;
+
+    QTest::newRow("insert 1 at 1, 0 buffer") << 1 << 1 << 0;
+    QTest::newRow("insert 1 at 1, 100 buffer") << 1 << 1 << 100;
+    QTest::newRow("insert 1 at 1, 500 buffer") << 1 << 1 << 500;
+
+    QTest::newRow("insert multiple at 0, 0 buffer") << 0 << 3 << 0;
+    QTest::newRow("insert multiple at 0, 100 buffer") << 0 << 3 << 100;
+    QTest::newRow("insert multiple at 0, 500 buffer") << 0 << 3 << 500;
+
+    QTest::newRow("insert multiple at 1, 0 buffer") << 1 << 3 << 0;
+    QTest::newRow("insert multiple at 1, 100 buffer") << 1 << 3 << 100;
+    QTest::newRow("insert multiple at 1, 500 buffer") << 1 << 3 << 500;
+}
+
 template <class T>
 void tst_QQuickListView::removed(bool animated)
 {