From 1686a82f3859ce2a0f48774ded21bf9817f5abc9 Mon Sep 17 00:00:00 2001 From: Bea Lam Date: Tue, 8 Nov 2011 16:11:26 +1000 Subject: [PATCH] GridView should re-layout if add/remove before visible index Unlike ListView, GridView must redo its layout if an item is added or removed from before the visible items, since it affects the column/row layout. Task-number: QTBUG-21588 Change-Id: Id333bc653033751c45d127973e94fae4580c55b0 Reviewed-by: Bea Lam --- src/declarative/items/qquickgridview.cpp | 8 ++ src/declarative/items/qquickitemview.cpp | 19 +++-- src/declarative/items/qquickitemview_p_p.h | 1 + .../qquickgridview/tst_qquickgridview.cpp | 87 ++++++++++++++++++++ 4 files changed, 107 insertions(+), 8 deletions(-) diff --git a/src/declarative/items/qquickgridview.cpp b/src/declarative/items/qquickgridview.cpp index 2793032..a413314 100644 --- a/src/declarative/items/qquickgridview.cpp +++ b/src/declarative/items/qquickgridview.cpp @@ -182,6 +182,7 @@ public: virtual void setPosition(qreal pos); virtual void layoutVisibleItems(); bool applyInsertionChange(const QDeclarativeChangeSet::Insert &, FxViewItem *, InsertionsResult *); + virtual bool needsRefillForAddedOrRemovedIndex(int index) const; virtual qreal headerSize() const; virtual qreal footerSize() const; @@ -1853,6 +1854,13 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In return insertResult->addedItems.count() > prevAddedCount; } +bool QQuickGridViewPrivate::needsRefillForAddedOrRemovedIndex(int modelIndex) const +{ + // If we add or remove items before visible items, a layout may be + // required to ensure item 0 is in the first column. + return modelIndex < visibleIndex; +} + /*! \qmlmethod QtQuick2::GridView::positionViewAtIndex(int index, PositionMode mode) diff --git a/src/declarative/items/qquickitemview.cpp b/src/declarative/items/qquickitemview.cpp index 923db474..79787d7 100644 --- a/src/declarative/items/qquickitemview.cpp +++ b/src/declarative/items/qquickitemview.cpp @@ -1413,7 +1413,7 @@ bool QQuickItemViewPrivate::applyModelChanges() moveReason = QQuickItemViewPrivate::Other; int prevCount = itemCount; - bool removedVisible = false; + bool visibleAffected = false; bool viewportChanged = !currentChanges.pendingChanges.removes().isEmpty() || !currentChanges.pendingChanges.inserts().isEmpty(); @@ -1432,8 +1432,8 @@ bool QQuickItemViewPrivate::applyModelChanges() FxViewItem *item = *it; if (item->index == -1 || item->index < removals[i].index) { // already removed, or before removed items - if (item->index < removals[i].index && !removedVisible) - removedVisible = true; + if (!visibleAffected && item->index < removals[i].index) + visibleAffected = true; ++it; } else if (item->index >= removals[i].index + removals[i].count) { // after removed items @@ -1441,7 +1441,7 @@ bool QQuickItemViewPrivate::applyModelChanges() ++it; } else { // removed item - removedVisible = true; + visibleAffected = true; if (!removals[i].isMove()) item->attached->emitRemove(); @@ -1463,20 +1463,22 @@ bool QQuickItemViewPrivate::applyModelChanges() } } } - + if (!visibleAffected && needsRefillForAddedOrRemovedIndex(removals[i].index)) + visibleAffected = true; } if (!removals.isEmpty()) updateVisibleIndex(); const QVector &insertions = currentChanges.pendingChanges.inserts(); - bool addedVisible = false; InsertionsResult insertResult; bool allInsertionsBeforeVisible = true; for (int i=0; i= visibleIndex) allInsertionsBeforeVisible = false; if (wasEmpty && !visibleItems.isEmpty()) @@ -1542,7 +1544,8 @@ bool QQuickItemViewPrivate::applyModelChanges() if (prevCount != itemCount) emit q->countChanged(); - bool visibleAffected = removedVisible || addedVisible || !currentChanges.pendingChanges.changes().isEmpty(); + if (!visibleAffected) + visibleAffected = !currentChanges.pendingChanges.changes().isEmpty(); if (!visibleAffected && viewportChanged) updateViewport(); diff --git a/src/declarative/items/qquickitemview_p_p.h b/src/declarative/items/qquickitemview_p_p.h index 398de84..ca4c0ce 100644 --- a/src/declarative/items/qquickitemview_p_p.h +++ b/src/declarative/items/qquickitemview_p_p.h @@ -240,6 +240,7 @@ protected: virtual void layoutVisibleItems() = 0; virtual void changedVisibleIndex(int newIndex) = 0; virtual bool applyInsertionChange(const QDeclarativeChangeSet::Insert &, FxViewItem *, InsertionsResult *) = 0; + virtual bool needsRefillForAddedOrRemovedIndex(int) const { return false; } virtual void initializeViewItem(FxViewItem *) {} virtual void initializeCurrentItem() {} diff --git a/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp b/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp index 9492605..a190d87 100644 --- a/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp @@ -72,6 +72,8 @@ private slots: void inserted_more(); void inserted_more_data(); void removed(); + void addOrRemoveBeforeVisible(); + void addOrRemoveBeforeVisible_data(); void clear(); void moved(); void moved_data(); @@ -740,6 +742,91 @@ void tst_QQuickGridView::removed() delete canvas; } +void tst_QQuickGridView::addOrRemoveBeforeVisible() +{ + // QTBUG-21588: ensure re-layout is done on grid after adding or removing + // items from before the visible area + + QFETCH(bool, doAdd); + QFETCH(qreal, newTopContentY); + + QQuickView *canvas = createView(); + canvas->show(); + + TestModel model; + for (int i = 0; i < 30; i++) + model.addItem("Item" + QString::number(i), ""); + + QDeclarativeContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + ctxt->setContextProperty("testRightToLeft", QVariant(false)); + ctxt->setContextProperty("testTopToBottom", QVariant(false)); + canvas->setSource(QUrl::fromLocalFile(TESTDATA("gridview1.qml"))); + + QQuickGridView *gridview = findItem(canvas->rootObject(), "grid"); + QTRY_VERIFY(gridview != 0); + QQuickItem *contentItem = gridview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + QQuickText *name = findItem(contentItem, "textName", 0); + QTRY_COMPARE(name->text(), QString("Item0")); + + gridview->setCurrentIndex(0); + qApp->processEvents(); + + // scroll down until item 0 is no longer drawn + // (bug not triggered if we just move using content y, since that doesn't + // refill and change the visible items) + gridview->setCurrentIndex(24); + qApp->processEvents(); + + QTRY_COMPARE(gridview->currentIndex(), 24); + QTRY_COMPARE(gridview->contentY(), 220.0); + + QTest::qWait(100); // wait for refill to complete + QTRY_VERIFY(!findItem(contentItem, "wrapper", 0)); // 0 shouldn't be visible + + if (doAdd) { + model.insertItem(0, "New Item", "New Item number"); + QTRY_COMPARE(gridview->count(), 31); + } else { + model.removeItem(0); + QTRY_COMPARE(gridview->count(), 29); + } + + // scroll back up and item 0 should be gone + gridview->setCurrentIndex(0); + qApp->processEvents(); + QTRY_COMPARE(gridview->currentIndex(), 0); + QTRY_COMPARE(gridview->contentY(), newTopContentY); + + name = findItem(contentItem, "textName", 0); + if (doAdd) + QCOMPARE(name->text(), QString("New Item")); + else + QCOMPARE(name->text(), QString("Item1")); + + // Confirm items positioned correctly + int itemCount = findItems(contentItem, "wrapper").count(); + for (int i = 0; i < model.count() && i < itemCount; ++i) { + QTRY_VERIFY(findItem(contentItem, "wrapper", i)); + QQuickItem *item = findItem(contentItem, "wrapper", i); + QTRY_VERIFY(item->x() == (i%3)*80); + QTRY_VERIFY(item->y() == (i/3)*60 + newTopContentY); + } + + delete canvas; +} + +void tst_QQuickGridView::addOrRemoveBeforeVisible_data() +{ + QTest::addColumn("doAdd"); + QTest::addColumn("newTopContentY"); + + QTest::newRow("add") << true << -60.0; + QTest::newRow("remove") << false << 0.0; +} + void tst_QQuickGridView::clear() { QQuickView *canvas = createView(); -- 1.7.2.5