From b1a301884bb88ff62268437f436d6f0fc38efe2b Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Wed, 7 Mar 2012 15:26:33 +1000 Subject: [PATCH] ListView can freeze if flicked beyond its bounds. If the delegate's size changes in componentComplete and all items are flicked out of view, an incorrect jump calculation in addVisibleItems() resulted in a new delegate being created in the wrong position, and retriggering the jump calculation, which resulted in a new delegate being created in the wrong position, and retriggering the jump... Also fixed currentItem visibility. Change-Id: Iad5f211c4fc5eed9c009d51a0ce3b58181a7b36e Reviewed-by: Bea Lam --- src/quick/items/qquickgridview.cpp | 4 ++ src/quick/items/qquicklistview.cpp | 25 +++++------ .../quick/qquickgridview/tst_qquickgridview.cpp | 6 +++ .../qquicklistview/data/flickBeyondBoundsBug.qml | 43 ++++++++++++++++++++ .../quick/qquicklistview/tst_qquicklistview.cpp | 39 ++++++++++++++++++ 5 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 tests/auto/quick/qquicklistview/data/flickBeyondBoundsBug.qml diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index 45fbc9f..a78ab4c 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -1884,6 +1884,10 @@ void QQuickGridView::viewportMoved() FxGridItemSG *item = static_cast(d->visibleItems.at(i)); item->item->setVisible(item->rowPos() + d->rowSize() >= from && item->rowPos() <= to); } + if (d->currentItem) { + FxGridItemSG *item = static_cast(d->currentItem); + item->item->setVisible(item->rowPos() + d->rowSize() >= from && item->rowPos() <= to); + } if (d->hData.flicking || d->vData.flicking || d->hData.moving || d->vData.moving) d->moveReason = QQuickGridViewPrivate::Mouse; diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp index 5e9a685..a2f687c 100644 --- a/src/quick/items/qquicklistview.cpp +++ b/src/quick/items/qquicklistview.cpp @@ -615,20 +615,17 @@ bool QQuickListViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d // We've jumped more than a page. Estimate which items are now // visible and fill from there. int count = (fillFrom - itemEnd) / (averageSize + spacing); - for (int i = 0; i < visibleItems.count(); ++i) - releaseItem(visibleItems.at(i)); - visibleItems.clear(); - modelIndex += count; - if (modelIndex >= model->count()) { - count -= modelIndex - model->count() + 1; - modelIndex = model->count() - 1; - } else if (modelIndex < 0) { - count -= modelIndex; - modelIndex = 0; - } - visibleIndex = modelIndex; - visiblePos = itemEnd + count * (averageSize + spacing); - itemEnd = visiblePos; + int newModelIdx = qBound(0, modelIndex + count, model->count()); + count = newModelIdx - modelIndex; + if (count) { + for (int i = 0; i < visibleItems.count(); ++i) + releaseItem(visibleItems.at(i)); + visibleItems.clear(); + modelIndex = newModelIdx; + visibleIndex = modelIndex; + visiblePos = itemEnd + count * (averageSize + spacing); + itemEnd = visiblePos; + } } bool changed = false; @@ -2553,6 +2550,8 @@ void QQuickListView::viewportMoved() FxViewItem *item = static_cast(d->visibleItems.at(i)); item->item->setVisible(item->endPosition() >= from && item->position() <= to); } + if (d->currentItem) + d->currentItem->item->setVisible(d->currentItem->endPosition() >= from && d->currentItem->position() <= to); if (d->hData.flicking || d->vData.flicking || d->hData.moving || d->vData.moving) d->moveReason = QQuickListViewPrivate::Mouse; diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index 0171872..66c98cd 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -1773,6 +1773,12 @@ void tst_QQuickGridView::currentIndex() QVERIFY(!gridview->highlightItem()); QVERIFY(!gridview->currentItem()); + // moving currentItem out of view should make it invisible + gridview->setCurrentIndex(0); + QTRY_VERIFY(gridview->currentItem()->isVisible()); + gridview->setContentY(200); + QTRY_VERIFY(!gridview->currentItem()->isVisible()); + delete canvas; } diff --git a/tests/auto/quick/qquicklistview/data/flickBeyondBoundsBug.qml b/tests/auto/quick/qquicklistview/data/flickBeyondBoundsBug.qml new file mode 100644 index 0000000..0a1b1a1 --- /dev/null +++ b/tests/auto/quick/qquicklistview/data/flickBeyondBoundsBug.qml @@ -0,0 +1,43 @@ +import QtQuick 2.0 + +Rectangle { + id: root + width: 240 + height: 320 + color: "#ffffff" + + Component { + id: myDelegate + Rectangle { + id: wrapper + objectName: "wrapper" + height: column.height + Column { + id: column + Text { + text: "index: " + index + ", delegate A" + Component.onCompleted: height = index % 2 ? 30 : 20 + } + Text { + x: 200 + text: wrapper.y + height: 25 + } + } + color: ListView.isCurrentItem ? "lightsteelblue" : "#EEEEEE" + } + } + ListView { + id: list + objectName: "list" + focus: true + width: 240 + height: 320 + model: 2 + delegate: myDelegate + highlightMoveSpeed: 1000 + highlightResizeSpeed: 1000 + cacheBuffer: 400 + } + Text { anchors.bottom: parent.bottom; text: list.contentY } +} diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp index 32f329f..dcc2e9d 100644 --- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp @@ -184,6 +184,8 @@ private slots: void multipleTransitions(); void multipleTransitions_data(); + void flickBeyondBounds(); + private: template void items(const QUrl &source, bool forceLayout); template void changed(const QUrl &source, bool forceLayout); @@ -2351,6 +2353,11 @@ void tst_QQuickListView::currentIndex() QVERIFY(!listview->highlightItem()); QVERIFY(!listview->currentItem()); + listview->setCurrentIndex(0); + QTRY_VERIFY(listview->currentItem()->isVisible()); + listview->setContentY(200); + QTRY_VERIFY(!listview->currentItem()->isVisible()); + delete canvas; } @@ -5994,6 +6001,38 @@ void tst_QQuickListView::matchItemLists(const QVariantList &itemLists, const QLi } } +void tst_QQuickListView::flickBeyondBounds() +{ + QQuickView *canvas = createView(); + + canvas->setSource(testFileUrl("flickBeyondBoundsBug.qml")); + canvas->show(); + qApp->processEvents(); + + QQuickListView *listview = findItem(canvas->rootObject(), "list"); + QTRY_VERIFY(listview != 0); + + QQuickItem *contentItem = listview->contentItem(); + QTRY_VERIFY(contentItem != 0); + QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false); + + // Flick view up beyond bounds + flick(canvas, QPoint(10, 10), QPoint(10, -1000), 180); + QTRY_VERIFY(findItems(contentItem, "wrapper").count() == 0); + + // We're really testing that we don't get stuck in a loop, + // but also confirm items positioned correctly. + QTRY_COMPARE(findItems(contentItem, "wrapper").count(), 2); + for (int i = 0; i < 2; ++i) { + QQuickItem *item = findItem(contentItem, "wrapper", i); + if (!item) qWarning() << "Item" << i << "not found"; + QTRY_VERIFY(item); + QTRY_VERIFY(item->y() == i*45); + } + + delete canvas; +} + QTEST_MAIN(tst_QQuickListView) -- 1.7.2.5