From 35d8d060b8621cfd17f92f9c632d99ceceb9acaa Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Wed, 25 Nov 2015 12:14:52 -0800 Subject: ListView: Sanitize visibleItems list after model insertions In QQuickListViewPrivate::applyInsertionChange(), we update the visibleItems list by first shifting the currently visible items and then we add as many items as the model was added and at the right position. We do this in such a way that we won't create items that will not be visible right away (and may be deleted right after by removeNonVisibleItems()). However, this may leave gaps in the item index sequence, and QQuickListView doesn't always recover gracefully from it. The purpose of this patch is to make sure those gaps are cleared right after inserting the new items. Since the insertions can happen in two different places (either before or after the first visible item) we need to update the visibleItems list accordingly. The way we sanitize visibleItems is by removing those items that lie beyond a possible index gap. If insertion happens before the first visible item, we'll remove all those items before the insertion point. If the insertion happens after the first visible item, we'll remove the items after the insertion point. Besides that, the logic for inserting before the visible position was wrong. As items are inserted bottom-up in that case, the insertion would start by just accounting for the item's size until the condition pos > from && insertionIdx < visibleIndex would become false only because 'pos' would be small enough. After that, the next loop run would start adding items before the 'from' position, which is wrong. Our fix is to move the condition outside the loop if the insertion index will be before the visible index and just account for the items' size in that case. Otherwise, the insertion happens as usual until pos < from. Change-Id: I35767cf6e9737bea1fe7677e580245fc7172710c Task-number: QTBUG-48870 Reviewed-by: Shawn Rutledge Reviewed-by: Robin Burchell --- tests/auto/quick/qquicklistview/BLACKLIST | 2 -- tests/auto/quick/qquicklistview/tst_qquicklistview.cpp | 18 ++++++++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) (limited to 'tests/auto/quick') diff --git a/tests/auto/quick/qquicklistview/BLACKLIST b/tests/auto/quick/qquicklistview/BLACKLIST index 02adeacf05..269696ce8c 100644 --- a/tests/auto/quick/qquicklistview/BLACKLIST +++ b/tests/auto/quick/qquicklistview/BLACKLIST @@ -2,5 +2,3 @@ * [enforceRange_withoutHighlight] osx -[QTBUG_48870_fastModelUpdates] -* diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp index ce2337ccb2..b3a6edb66a 100644 --- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp @@ -592,8 +592,7 @@ void tst_QQuickListView::inserted(const QUrl &source) QQuickItem *item = findItem(contentItem, "wrapper", 0); QVERIFY(item); - QCOMPARE(item->y(), 0.); - QTRY_COMPARE(listview->contentY(), qreal(0)); + QTRY_COMPARE(item->y() - listview->contentY(), 0.); delete window; delete testObject; @@ -644,7 +643,8 @@ void tst_QQuickListView::inserted_more(QQuickItemView::VerticalLayoutDirection v QTRY_COMPARE(listview->property("count").toInt(), model.count()); - + // FIXME This is NOT checking anything about visibleItems.first() +#if 0 // check visibleItems.first() is in correct position QQuickItem *item0 = findItem(contentItem, "wrapper", 0); QVERIFY(item0); @@ -652,6 +652,7 @@ void tst_QQuickListView::inserted_more(QQuickItemView::VerticalLayoutDirection v QCOMPARE(item0->y(), -item0->height() - itemsOffsetAfterMove); else QCOMPARE(item0->y(), itemsOffsetAfterMove); +#endif QList items = findItems(contentItem, "wrapper"); int firstVisibleIndex = -1; @@ -667,12 +668,21 @@ void tst_QQuickListView::inserted_more(QQuickItemView::VerticalLayoutDirection v // Confirm items positioned correctly and indexes correct QQuickText *name; QQuickText *number; + const qreal visibleFromPos = listview->contentY() - listview->displayMarginBeginning() - listview->cacheBuffer(); + const qreal visibleToPos = listview->contentY() + listview->height() + listview->displayMarginEnd() + listview->cacheBuffer(); for (int i = firstVisibleIndex; i < model.count() && i < items.count(); ++i) { QQuickItem *item = findItem(contentItem, "wrapper", i); QVERIFY2(item, QTest::toString(QString("Item %1 not found").arg(i))); qreal pos = i*20.0 + itemsOffsetAfterMove; if (verticalLayoutDirection == QQuickItemView::BottomToTop) - pos = -item0->height() - pos; + pos = -item->height() - pos; + // Items outside the visible area (including cache buffer) should be skipped + if (pos > visibleToPos || pos < visibleFromPos) { + QTRY_VERIFY2(QQuickItemPrivate::get(item)->culled || item->y() < visibleFromPos || item->y() > visibleToPos, + QTest::toString(QString("index %5, y %1, from %2, to %3, expected pos %4, culled %6"). + arg(item->y()).arg(visibleFromPos).arg(visibleToPos).arg(pos).arg(i).arg(bool(QQuickItemPrivate::get(item)->culled)))); + continue; + } QTRY_COMPARE(item->y(), pos); name = findItem(contentItem, "textName", i); QVERIFY(name != 0); -- cgit v1.2.3