diff options
author | Martin Jones <martin.jones@nokia.com> | 2012-03-07 15:26:33 +1000 |
---|---|---|
committer | Qt by Nokia <qt-info@nokia.com> | 2012-03-07 08:42:21 +0100 |
commit | b1a301884bb88ff62268437f436d6f0fc38efe2b (patch) | |
tree | 8278b5519a599d96dfdd85a755f746378eb43f45 | |
parent | 22a53bcd69d39a5ea128d53231e9e51455a98cc4 (diff) |
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 <bea.lam@nokia.com>
-rw-r--r-- | src/quick/items/qquickgridview.cpp | 4 | ||||
-rw-r--r-- | src/quick/items/qquicklistview.cpp | 27 | ||||
-rw-r--r-- | tests/auto/quick/qquickgridview/tst_qquickgridview.cpp | 6 | ||||
-rw-r--r-- | tests/auto/quick/qquicklistview/data/flickBeyondBoundsBug.qml | 43 | ||||
-rw-r--r-- | tests/auto/quick/qquicklistview/tst_qquicklistview.cpp | 39 |
5 files changed, 105 insertions, 14 deletions
diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index 45fbc9f93e..a78ab4c4e5 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -1884,6 +1884,10 @@ void QQuickGridView::viewportMoved() FxGridItemSG *item = static_cast<FxGridItemSG*>(d->visibleItems.at(i)); item->item->setVisible(item->rowPos() + d->rowSize() >= from && item->rowPos() <= to); } + if (d->currentItem) { + FxGridItemSG *item = static_cast<FxGridItemSG*>(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 5e9a685cf5..a2f687c86b 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<FxListItemSG*>(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 0171872aab..66c98cda84 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 0000000000..0a1b1a1b64 --- /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 32f329f45b..dcc2e9dd16 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 <class T> void items(const QUrl &source, bool forceLayout); template <class T> 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<QQuickListView>(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<QQuickItem>(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<QQuickItem>(contentItem, "wrapper").count(), 2); + for (int i = 0; i < 2; ++i) { + QQuickItem *item = findItem<QQuickItem>(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) |