diff options
author | Gabriel de Dietrich <gabriel.dedietrich@theqtcompany.com> | 2016-02-01 17:30:57 -0800 |
---|---|---|
committer | Gabriel de Dietrich <gabriel.dedietrich@theqtcompany.com> | 2016-02-13 00:15:45 +0000 |
commit | 0fab5761d5428aa708edd66e40fc3c449adc4b11 (patch) | |
tree | f8d7f3f150535a1e249412de8780f3c302148947 | |
parent | 04936e8704e3b0da60bf3ee79571c70e60f069c5 (diff) |
GridView: Sanitize visible items after model insertion
This is basically the same patch as 35d8d060b8621cfd, but this
time for GridView instead of ListView.
The major difference is that GridView seems to do the right thing
when the model insertions happen after the visible index. That
explains the missing part of the patch in applyInsertionChange()
as compared to the same function in QQuickListView.
Also, QQuickGridView auto-tests are a bit more robust than their
ListView counterpart. So no changes were necessary to existing
test cases.
Task-number: QTBUG-48870
Change-Id: I19b24c4d84a1a4cef4fdb3ddd3381d0c6b93a76a
Reviewed-by: Robin Burchell <robin.burchell@viroteck.net>
-rw-r--r-- | src/quick/items/qquickgridview.cpp | 51 | ||||
-rw-r--r-- | tests/auto/quick/qquickgridview/data/qtbug48870.qml | 30 | ||||
-rw-r--r-- | tests/auto/quick/qquickgridview/tst_qquickgridview.cpp | 38 |
3 files changed, 106 insertions, 13 deletions
diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index f00baf0dae..8d84144519 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -2381,11 +2381,12 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QQmlChangeSet::Change &ch int i = count - 1; int from = tempPos - buffer - displayMarginBeginning; - while (i >= 0) { - if (rowPos > from && insertionIdx < visibleIndex) { - // item won't be visible, just note the size for repositioning - insertResult->countChangeBeforeVisible++; - } else { + if (rowPos > from && insertionIdx < visibleIndex) { + // items won't be visible, just note the size for repositioning + insertResult->countChangeBeforeVisible += count; + insertResult->sizeChangesBeforeVisiblePos += ((count + columns - 1) / columns) * rowSize(); + } else { + while (i >= 0) { // item is before first visible e.g. in cache buffer FxViewItem *item = 0; if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) @@ -2401,19 +2402,40 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QQmlChangeSet::Change &ch insertResult->changedFirstItem = true; if (!change.isMove()) { addedItems->append(item); - item->transitionNextReposition(transitioner, QQuickItemViewTransitioner::AddTransition, true); + if (transitioner) + item->transitionNextReposition(transitioner, QQuickItemViewTransitioner::AddTransition, true); + else + item->moveTo(QPointF(colPos, rowPos), true); } insertResult->sizeChangesBeforeVisiblePos += rowSize(); + + if (--colNum < 0 ) { + colNum = columns - 1; + rowPos -= rowSize(); + } + colPos = colNum * colSize(); + index++; + i--; } + } - if (--colNum < 0 ) { - colNum = columns - 1; - rowPos -= rowSize(); + // There may be gaps in the index sequence of visibleItems because + // of the index shift/update done before the insertion just above. + // Find if there is any... + int firstOkIdx = -1; + for (int i = 0; i <= insertionIdx && i < visibleItems.count() - 1; i++) { + if (visibleItems.at(i)->index + 1 != visibleItems.at(i + 1)->index) { + firstOkIdx = i + 1; + break; } - colPos = colNum * colSize(); - index++; - i--; } + // ... and remove all the items before that one + for (int i = 0; i < firstOkIdx; i++) { + FxViewItem *nvItem = visibleItems.takeFirst(); + addedItems->removeOne(nvItem); + removeItem(nvItem); + } + } else { int i = 0; int to = buffer+displayMarginEnd+tempPos+size()-1; @@ -2438,7 +2460,10 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QQmlChangeSet::Change &ch movingIntoView->append(MovedItem(item, change.moveKey(item->index))); } else { addedItems->append(item); - item->transitionNextReposition(transitioner, QQuickItemViewTransitioner::AddTransition, true); + if (transitioner) + item->transitionNextReposition(transitioner, QQuickItemViewTransitioner::AddTransition, true); + else + item->moveTo(QPointF(colPos, rowPos), true); } insertResult->sizeChangesAfterVisiblePos += rowSize(); diff --git a/tests/auto/quick/qquickgridview/data/qtbug48870.qml b/tests/auto/quick/qquickgridview/data/qtbug48870.qml new file mode 100644 index 0000000000..7c0783fbb5 --- /dev/null +++ b/tests/auto/quick/qquickgridview/data/qtbug48870.qml @@ -0,0 +1,30 @@ +import QtQuick 2.6 + +Rectangle { + width: 500 + height: 500 + color: "blue" + + GridView { + id: view + objectName: "view" + anchors.fill: parent + model: testModel + cellWidth: 150 + cellHeight: 150 + readonly property int columns: Math.floor(width / cellWidth) + + delegate: Rectangle { + width: GridView.view.cellWidth + height: GridView.view.cellHeight + color: (row & 1) != (col & 1) ? "green" : "red" + readonly property int row: index / view.columns + readonly property int col: index % view.columns + + Text { + anchors.centerIn: parent + text: "Item " + index + } + } + } +} diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index 3699bef56d..4edfb0ed3b 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -41,6 +41,7 @@ #include <QtQml/qqmlincubator.h> #include <QtQml/qqmlcontext.h> #include <QtQuick/private/qquickitem_p.h> +#include <QtQuick/private/qquickitemview_p_p.h> #include <QtQuick/private/qquickgridview_p.h> #include <QtQuick/private/qquicktext_p.h> #include <QtQml/private/qqmllistmodel_p.h> @@ -210,6 +211,7 @@ private slots: void contentHeightWithDelayRemove(); void QTBUG_45640(); + void QTBUG_48870_fastModelUpdates(); private: QList<int> toIntList(const QVariantList &list); @@ -6566,6 +6568,42 @@ void tst_QQuickGridView::QTBUG_45640() delete window; } +void tst_QQuickGridView::QTBUG_48870_fastModelUpdates() +{ + StressTestModel model; + + QScopedPointer<QQuickView> window(createView()); + QQmlContext *ctxt = window->rootContext(); + ctxt->setContextProperty("testModel", &model); + + window->setSource(testFileUrl("qtbug48870.qml")); + window->show(); + QVERIFY(QTest::qWaitForWindowExposed(window.data())); + + QQuickGridView *view = findItem<QQuickGridView>(window->rootObject(), "view"); + QTRY_VERIFY(view != 0); + + QQuickItemViewPrivate *priv = QQuickItemViewPrivate::get(view); + bool nonUnique; + FxViewItem *item = Q_NULLPTR; + int expectedIdx; + QVERIFY(testVisibleItems(priv, &nonUnique, &item, &expectedIdx)); + + for (int i = 0; i < 10; i++) { + QTest::qWait(100); + QVERIFY2(testVisibleItems(priv, &nonUnique, &item, &expectedIdx), + qPrintable(!item ? QString("Unexpected null item") + : nonUnique ? QString("Non-unique item at %1 and %2").arg(item->index).arg(expectedIdx) + : QString("Found index %1, expected index is %3").arg(item->index).arg(expectedIdx))); + if (i % 3 != 0) { + if (i & 1) + flick(window.data(), QPoint(100, 200), QPoint(100, 0), 100); + else + flick(window.data(), QPoint(100, 200), QPoint(100, 400), 100); + } + } +} + QTEST_MAIN(tst_QQuickGridView) #include "tst_qquickgridview.moc" |