From 9e7dbbeec0be2ce07c58edf84ce6e03d60946bce Mon Sep 17 00:00:00 2001 From: Richard Moe Gustavsen Date: Tue, 7 Nov 2017 14:56:03 +0100 Subject: QQuickListView/GridView: load items that results from model changes synchronously The current implementation created new items with default incubation mode, which is AsynchronousIfNested. But from reading the code, it seems like changes to the model were really expected to be handled synchronously, since there aren't any sections in the code that will recover from situations were requested items ends up incubating async. This is also backed by the fact that the second argument used to be a bool set to 'synchronous'. The fact that this was translated to AsynchronousIfNested later down the chain didn't seems to be taken into account. A bug with this is found in ListView when it's embedded inside an async Loader. In that case, all list items will be incubating async at startup, which is normally expected and fine. But if the model assigned to the ListView is modified before the loader has finished, async loading will also happen to the items created because of the change. And then the situation described above will occur. This patch will force any items loaded because of a model change to be synchronous. This seems to be in line with the synchronous logic that already exists. And its also quite acceptable, since changing the model before everything is completed is a corner case, and can naturally lead to some stuttering in the list view anyway. A potential for improvement on this logic is to try to recover whenever creating an item fails. But this takes a lot of work because of the way model changes are structured and stored, and can easily cause regressions. Since this is a corner case, it is probably not worth it. [ChangeLog][QtQml][ListView] Fixed a bug in ListView that sometimes make items end up with wrong geometry when changes to its model happens while the ListView is being loaded async (e.g if wrapped inside an async Loader). Task-number: QTBUG-61537 Change-Id: I8d6857beaf8ef98b02bb46282a1312749b0fb801 Reviewed-by: J-P Nurmi --- .../quick/qquicklistview/tst_qquicklistview.cpp | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'tests/auto/quick/qquicklistview/tst_qquicklistview.cpp') diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp index f06a118976..223f6004ff 100644 --- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp @@ -261,6 +261,7 @@ private slots: void releaseItems(); void QTBUG_34576_velocityZero(); + void QTBUG_61537_modelChangesAsync(); private: template void items(const QUrl &source); @@ -8694,6 +8695,33 @@ void tst_QQuickListView::QTBUG_34576_velocityZero() delete window; } +void tst_QQuickListView::QTBUG_61537_modelChangesAsync() +{ + // The purpose of this test if to check that any model changes that happens + // during start-up, while a loader higher up in the chain is still incubating + // async, will not fail. + QQuickView window; + window.setGeometry(0,0,640,480); + + QString filename(testFile("qtbug61537_modelChangesAsync.qml")); + window.setSource(QUrl::fromLocalFile(filename)); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + // The qml file will assign the listview to the 'listView' property once the + // loader is ready with async incubation. So we need to wait for it. + QObject *root = window.rootObject(); + QTRY_VERIFY(root->property("listView").value()); + QQuickListView *listView = root->property("listView").value(); + QVERIFY(listView); + + // Check that the number of delegates we expect to be visible in + // the listview matches the number of items we find if we count. + int reportedCount = listView->count(); + int actualCount = findItems(listView, "delegate").count(); + QCOMPARE(reportedCount, actualCount); +} + QTEST_MAIN(tst_QQuickListView) #include "tst_qquicklistview.moc" -- cgit v1.2.3