diff options
author | Fabian Kosmale <fabian.kosmale@qt.io> | 2023-04-17 12:48:56 +0200 |
---|---|---|
committer | Fabian Kosmale <fabian.kosmale@qt.io> | 2023-04-20 09:26:38 +0200 |
commit | 37c25c6e74f4f74d7cca8f5b0f12a40ec0354f7e (patch) | |
tree | 1d4eab8eaec5a8363071a5b3f68711ca9f7d75dc | |
parent | db04cfb2d00c68e7c63bd183c118ecd77f0e4e68 (diff) |
QQuickItemView: Skip instantiating delegates if size is 0
If the area of a (List|Grid)View is 0, then instantiating delegates is
pointless, as they couldn't be shown anyway. However, our current logic
could not handle this case well, and would end up instantiating a
delegate for every delegate entry if their size also ended up being 0 -
you can after all fit infinitely many 0 sized items into a zero sized
container.
Detect this situation in QQuickItemViewPrivate::refill and the
applyInsertionChange implementations. Note that we only exit early if
there are no visible items and the view is zero-sized; if there are
visible items, we still want to ensure that they are removed after all.
We also need to adjust a few tests which had zero sized views to no
longer be zero sized; otherwise they wouldn't have created their
delegates in time.
[ChangeLog][QtQuick][ListView][Important Behavior Change] If a ListView
has size zero, it won't instantiate any delegates until its size becomes
non-zero.
Pick-to: 6.5
Fixes: QTBUG-110625
Fixes: QTBUG-89568
Fixes: QTBUG-51773
Change-Id: Ibe0e6fa5f01784016882522c120d2fee38df285b
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
12 files changed, 73 insertions, 3 deletions
diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index 000f0ca887..2b0f70ab10 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -2358,6 +2358,9 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QQmlChangeSet::Change &ch { Q_Q(QQuickGridView); + if (q->size().isEmpty()) + return false; + int modelIndex = change.index; int count = change.count; diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index ba6c1534d3..fea7dba2c3 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -1763,6 +1763,8 @@ void QQuickItemViewPrivate::refill(qreal from, qreal to) Q_Q(QQuickItemView); if (!model || !model->isValid() || !q->isComponentComplete()) return; + if (q->size().isEmpty() && visibleItems.isEmpty()) + return; if (!model->count()) { updateHeader(); updateFooter(); diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp index dc6d952ddd..afcddd59fd 100644 --- a/src/quick/items/qquicklistview.cpp +++ b/src/quick/items/qquicklistview.cpp @@ -3628,12 +3628,16 @@ void QQuickListViewPrivate::updateSectionCriteria() bool QQuickListViewPrivate::applyInsertionChange(const QQmlChangeSet::Change &change, ChangeResult *insertResult, QList<FxViewItem *> *addedItems, QList<MovedItem> *movingIntoView) { + Q_Q(QQuickListView); #if QT_CONFIG(quick_viewtransitions) Q_UNUSED(movingIntoView) #endif int modelIndex = change.index; int count = change.count; + if (q->size().isEmpty() && visibleItems.isEmpty()) + return false; + qreal tempPos = isContentFlowReversed() ? -position()-size() : position(); int index = visibleItems.size() ? mapFromModel(modelIndex) : 0; qreal lastVisiblePos = buffer + displayMarginEnd + tempPos + size(); diff --git a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp index e522f280a3..6fd173fb0c 100644 --- a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp +++ b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp @@ -1549,6 +1549,8 @@ void tst_qqmllistmodel::modify_through_delegate() " ListElement { name: \"Doe\"; age: 33 }\n" " }\n" " ListView {\n" + " height: 100\n" \ + " width: 100\n" \ " model: testModel\n" " delegate: Item {\n" " Component.onCompleted: model.age = 18;\n" diff --git a/tests/auto/quick/qquicklistview/data/objectModelCulling.qml b/tests/auto/quick/qquicklistview/data/objectModelCulling.qml index c0a70ec485..9ca79ec8e4 100644 --- a/tests/auto/quick/qquicklistview/data/objectModelCulling.qml +++ b/tests/auto/quick/qquicklistview/data/objectModelCulling.qml @@ -3,6 +3,7 @@ import QtQuick 2.12 import QtQml.Models 2.12 Item { + anchors.fill: parent ObjectModel { id: model1 diff --git a/tests/auto/quick/qquicklistview/data/snapOneItemResize.qml b/tests/auto/quick/qquicklistview/data/snapOneItemResize.qml index 7ecc833a64..4603cd9f34 100644 --- a/tests/auto/quick/qquicklistview/data/snapOneItemResize.qml +++ b/tests/auto/quick/qquicklistview/data/snapOneItemResize.qml @@ -2,6 +2,7 @@ import QtQuick 2.0 ListView { id: list + anchors.fill: parent currentIndex: 5 snapMode: ListView.SnapOneItem orientation: ListView.Horizontal diff --git a/tests/auto/quick/qquicklistview2/data/areaZeroView.qml b/tests/auto/quick/qquicklistview2/data/areaZeroView.qml new file mode 100644 index 0000000000..e0329f4e83 --- /dev/null +++ b/tests/auto/quick/qquicklistview2/data/areaZeroView.qml @@ -0,0 +1,22 @@ +import QtQuick + +Window { + width: 600 + height: 600 + visible: true + property int delegateCreationCounter: 0 + + ListView { + id: lv + anchors.fill: parent + model: 6000 + + delegate: Rectangle { + width: ListView.view.width + height: ListView.view.width / 6 + color: "white" + border.width: 1 + Component.onCompleted: ++delegateCreationCounter + } + } +} diff --git a/tests/auto/quick/qquicklistview2/data/boundDelegateComponent.qml b/tests/auto/quick/qquicklistview2/data/boundDelegateComponent.qml index 6fb84279b0..5e4de1a08d 100644 --- a/tests/auto/quick/qquicklistview2/data/boundDelegateComponent.qml +++ b/tests/auto/quick/qquicklistview2/data/boundDelegateComponent.qml @@ -6,6 +6,8 @@ Item { objectName: "outer" ListView { id: listView + width: 100 + height: 100 model: 1 property string foo: "foo" delegate: Text { @@ -16,6 +18,8 @@ Item { ListView { id: listView2 + width: 100 + height: 100 model: 1 delegate: Text { required property int index @@ -49,6 +53,8 @@ Item { } ListView { + width: 100 + height: 100 id: innerListView model: listModel delegate: innerComponent diff --git a/tests/auto/quick/qquicklistview2/data/innerRequired.qml b/tests/auto/quick/qquicklistview2/data/innerRequired.qml index b45181c0d2..c0862cec0d 100644 --- a/tests/auto/quick/qquicklistview2/data/innerRequired.qml +++ b/tests/auto/quick/qquicklistview2/data/innerRequired.qml @@ -25,6 +25,8 @@ Item { ListView { id: listView model: myModel + width: 100 + height: 100 delegate: AnotherDelegate { age: model.age text: model.noise diff --git a/tests/auto/quick/qquicklistview2/data/metaSequenceAsModel.qml b/tests/auto/quick/qquicklistview2/data/metaSequenceAsModel.qml index 2b4651feaa..461450239f 100644 --- a/tests/auto/quick/qquicklistview2/data/metaSequenceAsModel.qml +++ b/tests/auto/quick/qquicklistview2/data/metaSequenceAsModel.qml @@ -2,6 +2,8 @@ import QtQuick ListView { id: view + width: 100 + height: 100 property list<rect> rects: [ Qt.rect(1, 2, 3, 4), Qt.rect(5, 6, 7, 8) ] property list<string> texts diff --git a/tests/auto/quick/qquicklistview2/data/sectionBoundComponent.qml b/tests/auto/quick/qquicklistview2/data/sectionBoundComponent.qml index dbb966ae28..74ab6b59fa 100644 --- a/tests/auto/quick/qquicklistview2/data/sectionBoundComponent.qml +++ b/tests/auto/quick/qquicklistview2/data/sectionBoundComponent.qml @@ -2,7 +2,9 @@ pragma ComponentBehavior: Bound import QtQuick ListView { id: view - model: ListModel { + width: 100 + height: 100 + model: ListModel { ListElement { name: "foo"; age: 42 } ListElement { name: "bar"; age: 13 } } diff --git a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp index 684a12160b..c83040fb11 100644 --- a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp +++ b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp @@ -54,6 +54,7 @@ private slots: void highlightWithBound(); void sectionIsCompatibleWithBoundComponents(); void sectionGeometryChange(); + void areaZeroviewDoesNotNeedlesslyPopulateWholeModel(); private: void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to); @@ -334,7 +335,7 @@ void tst_QQuickListView2::boundDelegateComponent() QVERIFY2(c.isReady(), qPrintable(c.errorString())); QTest::ignoreMessage( - QtWarningMsg, qPrintable(QLatin1String("%1:12: ReferenceError: index is not defined") + QtWarningMsg, qPrintable(QLatin1String("%1:14: ReferenceError: index is not defined") .arg(url.toString()))); QScopedPointer<QObject> o(c.create()); @@ -365,7 +366,7 @@ void tst_QQuickListView2::boundDelegateComponent() for (int i = 0; i < 3; ++i) { QTest::ignoreMessage( QtWarningMsg, - qPrintable(QLatin1String("%1:47:21: ReferenceError: model is not defined") + qPrintable(QLatin1String("%1:51:21: ReferenceError: model is not defined") .arg(url.toString()))); } @@ -988,6 +989,28 @@ void tst_QQuickListView2::sectionGeometryChange() QTRY_COMPARE(element1->y(), section1->y() + section1->height()); } +void tst_QQuickListView2::areaZeroviewDoesNotNeedlesslyPopulateWholeModel() +{ + QTest::failOnWarning(QRegularExpression(".*")); + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("areaZeroView.qml")); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + std::unique_ptr<QObject> root(c.create()); + QVERIFY(root); + auto delegateCreationCounter = [&]() { + return root->property("delegateCreationCounter").toInt(); + }; + // wait for onComplete to be settled + QTRY_VERIFY(delegateCreationCounter() != 0); + auto view = qobject_cast<QQuickListView *>(qmlContext(root.get())->objectForName("lv")); + QVERIFY(view); + QCOMPARE(view->count(), 6'000); + // we use 100, which is < 6000, but larger than the actual expected value + // that's to give the test some leniency in case the ListView implementation + // changes in the future to instantiate a few more items outside of the viewport + QVERIFY(delegateCreationCounter() < 100); +} + QTEST_MAIN(tst_QQuickListView2) #include "tst_qquicklistview2.moc" |