From: Bea Lam Date: Wed, 4 Jan 2012 23:59:30 +0000 (+1000) Subject: Fix positioning issues and change content y repositioning behaviour X-Git-Tag: qt-v5.0.0-alpha1~715 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6b6d99c1a218f1c46ae0151a01575de6092ba9a7;p=profile%2Fivi%2Fqtdeclarative.git Fix positioning issues and change content y repositioning behaviour Refactor the code for re-positioning the visibleItems.first() in QSGItemView::applyModelChanges() and fix various positioning issues. The positioning behaviour for removing items at the start of the view has changed. This action will no longer cause the content y to move down; previously the content y would always move down unless another item was moving backwards to the first visible position. This will make it easier to implement built-in removal transitions for the views, since removed items cannot be animated if the content y jumps down past them. Additionally, moving items from before a GridView should not cause the top item in the view to move upwards and out of the view. This also adds additional remove tests and fixes the tst_QQuickGridView::moved() test which wasn't waiting for the polish event after setting the content y, which caused some of the tests to pass even though their test data was wrong. Change-Id: Idad11a73a18f88208e73a52111ed839458e05f2d Reviewed-by: Martin Jones --- diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index 582f462..b29c61f 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -173,7 +173,7 @@ public: virtual void repositionPackageItemAt(QQuickItem *item, int index); virtual void resetItemPosition(FxViewItem *item, FxViewItem *toItem); virtual void resetFirstItemPosition(); - virtual void moveItemBy(FxViewItem *item, qreal forwards, qreal backwards); + virtual void adjustFirstItem(qreal forwards, qreal backwards); virtual void createHighlight(); virtual void updateHighlight(); @@ -181,7 +181,7 @@ public: virtual void setPosition(qreal pos); virtual void layoutVisibleItems(); - bool applyInsertionChange(const QDeclarativeChangeSet::Insert &, FxViewItem *, InsertionsResult *); + virtual bool applyInsertionChange(const QDeclarativeChangeSet::Insert &insert, ChangeResult *changeResult, bool *newVisibleItemsFirst, QList *addedItems); virtual bool needsRefillForAddedOrRemovedIndex(int index) const; virtual qreal headerSize() const; @@ -598,11 +598,14 @@ void QQuickGridViewPrivate::resetFirstItemPosition() item->setPosition(0, 0); } -void QQuickGridViewPrivate::moveItemBy(FxViewItem *item, qreal forwards, qreal backwards) +void QQuickGridViewPrivate::adjustFirstItem(qreal forwards, qreal backwards) { + if (!visibleItems.count()) + return; + int moveCount = (forwards / rowSize()) - (backwards / rowSize()); - FxGridItemSG *gridItem = static_cast(item); + FxGridItemSG *gridItem = static_cast(visibleItems.first()); gridItem->setPosition(gridItem->colPos(), gridItem->rowPos() + ((moveCount / columns) * rowSize())); } @@ -783,8 +786,7 @@ void QQuickGridViewPrivate::initializeCurrentItem() { if (currentItem && currentIndex >= 0) { FxGridItemSG *gridItem = static_cast(currentItem); - if (gridItem) - gridItem->setPosition(colPosAt(currentIndex), rowPosAt(currentIndex)); + gridItem->setPosition(colPosAt(currentIndex), rowPosAt(currentIndex)); } } @@ -1768,7 +1770,7 @@ void QQuickGridView::moveCurrentIndexRight() } } -bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::Insert &change, FxViewItem *firstVisible, InsertionsResult *insertResult) +bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::Insert &change, ChangeResult *insertResult, bool *newVisibleItemsFirst, QList *addedItems) { Q_Q(QQuickGridView); @@ -1828,8 +1830,8 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In item->index += count; } - int prevAddedCount = insertResult->addedItems.count(); - if (firstVisible && rowPos < firstVisible->position()) { + int prevVisibleCount = visibleItems.count(); + if (insertResult->visiblePos.isValid() && rowPos < insertResult->visiblePos) { // Insert items before the visible item. int insertionIdx = index; int i = count - 1; @@ -1838,15 +1840,12 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In while (i >= 0) { if (rowPos > from && insertionIdx < visibleIndex) { // item won't be visible, just note the size for repositioning - insertResult->sizeAddedBeforeVisible += rowSize(); + insertResult->sizeChangesBeforeVisiblePos += rowSize(); } else { // item is before first visible e.g. in cache buffer FxViewItem *item = 0; - if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) { - if (item->index > modelIndex + i) - insertResult->movedBackwards.append(item); + if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) item->index = modelIndex + i; - } if (!item) item = createItem(modelIndex + i); if (!item) @@ -1854,10 +1853,9 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In item->item->setVisible(true); visibleItems.insert(insertionIdx, item); - if (!change.isMove()) { - insertResult->addedItems.append(item); - insertResult->sizeAddedBeforeVisible += rowSize(); - } + if (!change.isMove()) + addedItems->append(item); + insertResult->sizeChangesBeforeVisiblePos += rowSize(); } if (--colNum < 0 ) { @@ -1873,11 +1871,8 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In int to = buffer+tempPos+size()-1; while (i < count && rowPos <= to + rowSize()*(columns - (colPos/colSize()))/qreal(columns)) { FxViewItem *item = 0; - if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) { - if (item->index > modelIndex + i) - insertResult->movedBackwards.append(item); + if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) item->index = modelIndex + i; - } if (!item) item = createItem(modelIndex + i); if (!item) @@ -1885,8 +1880,12 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In item->item->setVisible(true); visibleItems.insert(index, item); + if (index == 0) + *newVisibleItemsFirst = true; if (!change.isMove()) - insertResult->addedItems.append(item); + addedItems->append(item); + insertResult->sizeChangesAfterVisiblePos += rowSize(); + if (++colNum >= columns) { colNum = 0; rowPos += rowSize(); @@ -1899,7 +1898,7 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In updateVisibleIndex(); - return insertResult->addedItems.count() > prevAddedCount; + return visibleItems.count() > prevVisibleCount; } bool QQuickGridViewPrivate::needsRefillForAddedOrRemovedIndex(int modelIndex) const diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 879c02f..8f46661 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -75,7 +75,7 @@ void QQuickItemViewChangeSet::applyChanges(const QDeclarativeChangeSet &changeSe pendingChanges.apply(changeSet); int moveId = -1; - int moveOffset; + int moveOffset = 0; foreach (const QDeclarativeChangeSet::Remove &r, changeSet.removes()) { itemCount -= r.count; @@ -1221,7 +1221,6 @@ void QQuickItemViewPrivate::updateCurrent(int modelIndex) { Q_Q(QQuickItemView); applyPendingChanges(); - if (!q->isComponentComplete() || !isValid() || modelIndex < 0 || modelIndex >= model->count()) { if (currentItem) { currentItem->attached->setIsCurrentItem(false); @@ -1425,57 +1424,25 @@ bool QQuickItemViewPrivate::applyModelChanges() updateUnrequestedIndexes(); moveReason = QQuickItemViewPrivate::Other; - int prevCount = itemCount; + FxViewItem *prevVisibleItemsFirst = visibleItems.count() ? visibleItems.first() : 0; + int prevItemCount = itemCount; + int prevVisibleItemsCount = visibleItems.count(); bool visibleAffected = false; bool viewportChanged = !currentChanges.pendingChanges.removes().isEmpty() || !currentChanges.pendingChanges.inserts().isEmpty(); - FxViewItem *firstVisible = firstVisibleItem(); - FxViewItem *origVisibleItemsFirst = visibleItems.count() ? visibleItems.first() : 0; - int firstItemIndex = firstVisible ? firstVisible->index : -1; - qreal removedBeforeFirstVisibleBy = 0; + FxViewItem *prevFirstVisible = firstVisibleItem(); + QDeclarativeNullableValue prevFirstVisiblePos; + if (prevFirstVisible) + prevFirstVisiblePos = prevFirstVisible->position(); const QVector &removals = currentChanges.pendingChanges.removes(); + ChangeResult removalResult(prevFirstVisiblePos); + int removedCount = 0; for (int i=0; i::Iterator it = visibleItems.begin(); - while (it != visibleItems.end()) { - FxViewItem *item = *it; - if (item->index == -1 || item->index < removals[i].index) { - // already removed, or before removed items - if (!visibleAffected && item->index < removals[i].index) - visibleAffected = true; - ++it; - } else if (item->index >= removals[i].index + removals[i].count) { - // after removed items - item->index -= removals[i].count; - ++it; - } else { - // removed item - visibleAffected = true; - if (!removals[i].isMove()) - item->attached->emitRemove(); - - if (item->attached->delayRemove() && !removals[i].isMove()) { - item->index = -1; - QObject::connect(item->attached, SIGNAL(delayRemoveChanged()), q, SLOT(destroyRemoved()), Qt::QueuedConnection); - ++it; - } else { - if (firstVisible && item->position() < firstVisible->position() && item != visibleItems.first()) - removedBeforeFirstVisibleBy += item->size(); - if (removals[i].isMove()) { - currentChanges.removedItems.insert(removals[i].moveKey(item->index), item); - } else { - if (item == firstVisible) - firstVisible = 0; - currentChanges.removedItems.insertMulti(QDeclarativeChangeSet::MoveKey(), item); - } - it = visibleItems.erase(it); - } - } - } + if (applyRemovalChange(removals[i], &removalResult, &removedCount)) + visibleAffected = true; if (!visibleAffected && needsRefillForAddedOrRemovedIndex(removals[i].index)) visibleAffected = true; } @@ -1483,59 +1450,47 @@ bool QQuickItemViewPrivate::applyModelChanges() updateVisibleIndex(); const QVector &insertions = currentChanges.pendingChanges.inserts(); - InsertionsResult insertResult; - bool allInsertionsBeforeVisible = true; - + ChangeResult insertionResult(prevFirstVisiblePos); + bool newVisibleItemsFirst = false; + QList newItems; for (int i=0; i= visibleIndex) - allInsertionsBeforeVisible = false; if (wasEmpty && !visibleItems.isEmpty()) resetFirstItemPosition(); itemCount += insertions[i].count; } - for (int i=0; iattached->emitAdd(); - - // if the first visible item has moved, ensure another one takes its place - // so that we avoid shifting all content forwards - // (if an item is removed from before the first visible, the first visible should not move upwards) - bool movedBackToFirstVisible = false; - if (firstVisible && firstItemIndex >= 0) { - for (int i=0; iindex == firstItemIndex) { - // an item has moved backwards up to the first visible's position - resetItemPosition(insertResult.movedBackwards[i], firstVisible); - insertResult.movedBackwards.removeAt(i); - movedBackToFirstVisible = true; - break; + for (int i=0; iattached->emitAdd(); + + // reposition visibleItems.first() correctly so that the content y doesn't jump + if (visibleItems.count() && removedCount != prevVisibleItemsCount) { + if (newVisibleItemsFirst && prevVisibleItemsFirst) + resetItemPosition(visibleItems.first(), prevVisibleItemsFirst); + + if (prevFirstVisible && prevVisibleItemsFirst == prevFirstVisible + && prevFirstVisible != visibleItems.first()) { + // the previous visibleItems.first() was also the first visible item, and it has been + // moved/removed, so move the new visibleItems.first() to the pos of the previous one + if (!newVisibleItemsFirst) + resetItemPosition(visibleItems.first(), prevFirstVisible); + + } else if (prevFirstVisiblePos.isValid()) { + qreal moveForwardsBy = 0; + qreal moveBackwardsBy = 0; + + // shift visibleItems.first() relative to the number of added/removed items + if (visibleItems.first()->position() > prevFirstVisiblePos) { + moveForwardsBy = insertionResult.sizeChangesAfterVisiblePos; + moveBackwardsBy = removalResult.sizeChangesAfterVisiblePos; + } else if (visibleItems.first()->position() < prevFirstVisiblePos) { + moveForwardsBy = removalResult.sizeChangesBeforeVisiblePos; + moveBackwardsBy = insertionResult.sizeChangesBeforeVisiblePos; } - } - if (!movedBackToFirstVisible && !allInsertionsBeforeVisible) { - // first visible item has moved forward, another visible item takes its place - FxViewItem *item = visibleItem(firstItemIndex); - if (item) - resetItemPosition(item, firstVisible); - } - } - - // Ensure we don't cause an ugly list scroll - if (firstVisible && visibleItems.count() && visibleItems.first() != firstVisible) { - // ensure first item is placed at correct postion if moving backward - // since it will be used to position all subsequent items - if (insertResult.movedBackwards.count() && origVisibleItemsFirst) - resetItemPosition(visibleItems.first(), origVisibleItemsFirst); - - // correct the first item position (unless it has already been fixed) - if (!movedBackToFirstVisible) { - qreal moveBackwardsBy = insertResult.sizeAddedBeforeVisible; - for (int i=0; isize(); - moveItemBy(visibleItems.first(), removedBeforeFirstVisibleBy, moveBackwardsBy); + adjustFirstItem(moveForwardsBy, moveBackwardsBy); } } @@ -1558,7 +1513,7 @@ bool QQuickItemViewPrivate::applyModelChanges() currentChanges.reset(); updateSections(); - if (prevCount != itemCount) + if (prevItemCount != itemCount) emit q->countChanged(); if (!visibleAffected) @@ -1570,6 +1525,58 @@ bool QQuickItemViewPrivate::applyModelChanges() return visibleAffected; } +bool QQuickItemViewPrivate::applyRemovalChange(const QDeclarativeChangeSet::Remove &removal, ChangeResult *insertResult, int *removedCount) +{ + Q_Q(QQuickItemView); + bool visibleAffected = false; + + QList::Iterator it = visibleItems.begin(); + while (it != visibleItems.end()) { + FxViewItem *item = *it; + if (item->index == -1 || item->index < removal.index) { + // already removed, or before removed items + if (!visibleAffected && item->index < removal.index) + visibleAffected = true; + ++it; + } else if (item->index >= removal.index + removal.count) { + // after removed items + item->index -= removal.count; + ++it; + } else { + // removed item + visibleAffected = true; + if (!removal.isMove()) + item->attached->emitRemove(); + + if (item->attached->delayRemove() && !removal.isMove()) { + item->index = -1; + QObject::connect(item->attached, SIGNAL(delayRemoveChanged()), q, SLOT(destroyRemoved()), Qt::QueuedConnection); + ++it; + } else { + if (insertResult->visiblePos.isValid()) { + if (item->position() < insertResult->visiblePos) { + // sizeRemovedBeforeFirstVisible measures the size between the visibleItems.first() + // and the firstVisible, so don't count it if removing visibleItems.first() + if (item != visibleItems.first()) + insertResult->sizeChangesBeforeVisiblePos += item->size(); + } else { + insertResult->sizeChangesAfterVisiblePos += item->size(); + } + } + if (removal.isMove()) { + currentChanges.removedItems.insert(removal.moveKey(item->index), item); + } else { + // track item so it is released later + currentChanges.removedItems.insertMulti(QDeclarativeChangeSet::MoveKey(), item); + (*removedCount)++; + } + it = visibleItems.erase(it); + } + } + } + return visibleAffected; +} + /* This may return 0 if the item is being created asynchronously. When the item becomes available, refill() will be called and the item diff --git a/src/quick/items/qquickitemview_p_p.h b/src/quick/items/qquickitemview_p_p.h index cfceca6..a81aa6f 100644 --- a/src/quick/items/qquickitemview_p_p.h +++ b/src/quick/items/qquickitemview_p_p.h @@ -99,12 +99,13 @@ class QQuickItemViewPrivate : public QQuickFlickablePrivate public: QQuickItemViewPrivate(); - struct InsertionsResult { - QList addedItems; - QList movedBackwards; - qreal sizeAddedBeforeVisible; + struct ChangeResult { + QDeclarativeNullableValue visiblePos; + qreal sizeChangesBeforeVisiblePos; + qreal sizeChangesAfterVisiblePos; - InsertionsResult() : sizeAddedBeforeVisible(0) {} + ChangeResult(const QDeclarativeNullableValue &p) + : visiblePos(p), sizeChangesBeforeVisiblePos(0), sizeChangesAfterVisiblePos(0) {} }; enum BufferMode { NoBuffer = 0x00, BufferBefore = 0x01, BufferAfter = 0x02 }; @@ -145,6 +146,7 @@ public: void positionViewAtIndex(int index, int mode); void applyPendingChanges(); bool applyModelChanges(); + bool applyRemovalChange(const QDeclarativeChangeSet::Remove &removal, ChangeResult *changeResult, int *removedCount); void checkVisible() const; @@ -236,11 +238,12 @@ protected: virtual void repositionPackageItemAt(QQuickItem *item, int index) = 0; virtual void resetItemPosition(FxViewItem *item, FxViewItem *toItem) = 0; virtual void resetFirstItemPosition() = 0; - virtual void moveItemBy(FxViewItem *item, qreal forwards, qreal backwards) = 0; + virtual void adjustFirstItem(qreal forwards, qreal backwards) = 0; virtual void layoutVisibleItems() = 0; virtual void changedVisibleIndex(int newIndex) = 0; - virtual bool applyInsertionChange(const QDeclarativeChangeSet::Insert &, FxViewItem *, InsertionsResult *) = 0; + virtual bool applyInsertionChange(const QDeclarativeChangeSet::Insert &insert, ChangeResult *changeResult, bool *newVisibleItemsFirst, QList *newItems) = 0; + virtual bool needsRefillForAddedOrRemovedIndex(int) const { return false; } virtual void initializeViewItem(FxViewItem *) {} diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp index b2e9c61..e2c4288 100644 --- a/src/quick/items/qquicklistview.cpp +++ b/src/quick/items/qquicklistview.cpp @@ -95,7 +95,7 @@ public: virtual void repositionPackageItemAt(QQuickItem *item, int index); virtual void resetItemPosition(FxViewItem *item, FxViewItem *toItem); virtual void resetFirstItemPosition(); - virtual void moveItemBy(FxViewItem *item, qreal forwards, qreal backwards); + virtual void adjustFirstItem(qreal forwards, qreal backwards); virtual void createHighlight(); virtual void updateHighlight(); @@ -103,7 +103,7 @@ public: virtual void setPosition(qreal pos); virtual void layoutVisibleItems(); - bool applyInsertionChange(const QDeclarativeChangeSet::Insert &, FxViewItem *firstVisible, InsertionsResult *); + virtual bool applyInsertionChange(const QDeclarativeChangeSet::Insert &insert, ChangeResult *changeResult, bool *newVisibleItemsFirst, QList *addedItems); virtual void updateSections(); QQuickItem *getSectionItem(const QString §ion); @@ -747,10 +747,12 @@ void QQuickListViewPrivate::resetFirstItemPosition() item->setPosition(0); } -void QQuickListViewPrivate::moveItemBy(FxViewItem *item, qreal forwards, qreal backwards) +void QQuickListViewPrivate::adjustFirstItem(qreal forwards, qreal backwards) { + if (!visibleItems.count()) + return; qreal diff = forwards - backwards; - static_cast(item)->setPosition(item->position() + diff); + static_cast(visibleItems.first())->setPosition(visibleItems.first()->position() + diff); } void QQuickListViewPrivate::createHighlight() @@ -2374,7 +2376,7 @@ void QQuickListView::updateSections() } } -bool QQuickListViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::Insert &change, FxViewItem *firstVisible, InsertionsResult *insertResult) +bool QQuickListViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::Insert &change, ChangeResult *insertResult, bool *newVisibleItemsFirst, QList *addedItems) { int modelIndex = change.index; int count = change.count; @@ -2414,8 +2416,8 @@ bool QQuickListViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In : visibleItems.last()->endPosition()+spacing; } - int prevAddedCount = insertResult->addedItems.count(); - if (firstVisible && pos < firstVisible->position()) { + int prevVisibleCount = visibleItems.count(); + if (insertResult->visiblePos.isValid() && pos < insertResult->visiblePos) { // Insert items before the visible item. int insertionIdx = index; int i = 0; @@ -2424,26 +2426,24 @@ bool QQuickListViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In for (i = count-1; i >= 0; --i) { if (pos > from && insertionIdx < visibleIndex) { // item won't be visible, just note the size for repositioning - insertResult->sizeAddedBeforeVisible += averageSize + spacing; + insertResult->sizeChangesBeforeVisiblePos += averageSize + spacing; pos -= averageSize + spacing; } else { // item is before first visible e.g. in cache buffer FxViewItem *item = 0; - if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) { - if (item->index > modelIndex + i) - insertResult->movedBackwards.append(item); + if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) item->index = modelIndex + i; - } if (!item) item = createItem(modelIndex + i); if (!item) return false; visibleItems.insert(insertionIdx, item); - if (!change.isMove()) { - insertResult->addedItems.append(item); - insertResult->sizeAddedBeforeVisible += item->size(); - } + if (insertionIdx == 0) + *newVisibleItemsFirst = true; + if (!change.isMove()) + addedItems->append(item); + insertResult->sizeChangesBeforeVisiblePos += item->size() + spacing; pos -= item->size() + spacing; } index++; @@ -2453,19 +2453,19 @@ bool QQuickListViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In int to = buffer+tempPos+size(); for (i = 0; i < count && pos <= to; ++i) { FxViewItem *item = 0; - if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) { - if (item->index > modelIndex + i) - insertResult->movedBackwards.append(item); + if (change.isMove() && (item = currentChanges.removedItems.take(change.moveKey(modelIndex + i)))) item->index = modelIndex + i; - } if (!item) item = createItem(modelIndex + i); if (!item) return false; visibleItems.insert(index, item); + if (index == 0) + *newVisibleItemsFirst = true; if (!change.isMove()) - insertResult->addedItems.append(item); + addedItems->append(item); + insertResult->sizeChangesAfterVisiblePos += item->size() + spacing; pos += item->size() + spacing; ++index; } @@ -2479,7 +2479,7 @@ bool QQuickListViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In updateVisibleIndex(); - return insertResult->addedItems.count() > prevAddedCount; + return visibleItems.count() > prevVisibleCount; } diff --git a/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp b/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp index fa42ddb..fe469b5 100644 --- a/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/qtquick2/qquickgridview/tst_qquickgridview.cpp @@ -73,6 +73,8 @@ private slots: void insertBeforeVisible(); void insertBeforeVisible_data(); void removed(); + void removed_more(); + void removed_more_data(); void addOrRemoveBeforeVisible(); void addOrRemoveBeforeVisible_data(); void clear(); @@ -629,6 +631,7 @@ void tst_QQuickGridView::insertBeforeVisible() int firstVisibleIndex = 20; // move to an index where the top item is not visible gridview->setContentY(firstVisibleIndex * 20.0); gridview->setCurrentIndex(firstVisibleIndex); + qApp->processEvents(); QTRY_COMPARE(gridview->currentIndex(), firstVisibleIndex); QQuickItem *item = findItem(contentItem, "wrapper", firstVisibleIndex); @@ -831,6 +834,181 @@ void tst_QQuickGridView::removed() delete canvas; } +void tst_QQuickGridView::removed_more() +{ + QFETCH(qreal, contentY); + QFETCH(int, removeIndex); + QFETCH(int, removeCount); + QFETCH(qreal, itemsOffsetAfterMove); + + QQuickText *name; + QQuickText *number; + 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(testFileUrl("gridview1.qml")); + qApp->processEvents(); + + QQuickGridView *gridview = findItem(canvas->rootObject(), "grid"); + QTRY_VERIFY(gridview != 0); + QQuickItem *contentItem = gridview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + gridview->setContentY(contentY); + QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false); + + model.removeItems(removeIndex, removeCount); + QTRY_COMPARE(gridview->property("count").toInt(), model.count()); + + // check visibleItems.first() is in correct position + QQuickItem *item0 = findItem(contentItem, "wrapper", 0); +// qApp->exec(); + QVERIFY(item0); + QCOMPARE(item0->y(), itemsOffsetAfterMove); + + int firstVisibleIndex = -1; + QList items = findItems(contentItem, "wrapper"); + for (int i=0; iy() >= contentY) { + QDeclarativeExpression e(qmlContext(items[i]), items[i], "index"); + firstVisibleIndex = e.evaluate().toInt(); + break; + } + } + QVERIFY2(firstVisibleIndex >= 0, QTest::toString(firstVisibleIndex)); + + // Confirm items positioned correctly and indexes correct + int itemCount = findItems(contentItem, "wrapper").count(); + for (int i = firstVisibleIndex; i < model.count() && i < itemCount; ++i) { + QQuickItem *item = findItem(contentItem, "wrapper", i); + QVERIFY2(item, QTest::toString(QString("Item %1 not found").arg(i))); + + QTRY_COMPARE(item->x(), (i%3)*80.0); + QTRY_COMPARE(item->y(), (i/3)*60.0 + itemsOffsetAfterMove); + + name = findItem(contentItem, "textName", i); + QVERIFY(name != 0); + QTRY_COMPARE(name->text(), model.name(i)); + number = findItem(contentItem, "textNumber", i); + QVERIFY(number != 0); + QTRY_COMPARE(number->text(), model.number(i)); + } + + delete canvas; +} + +void tst_QQuickGridView::removed_more_data() +{ + QTest::addColumn("contentY"); + QTest::addColumn("removeIndex"); + QTest::addColumn("removeCount"); + QTest::addColumn("itemsOffsetAfterMove"); + + QTest::newRow("remove 1, before visible items") + << 120.0 // show 6-23 + << 3 << 1 + << 0.0; + + QTest::newRow("remove multiple, all before visible items") + << 120.0 + << 1 << 3 + << 60.0; // removed top row, slide down by 1 row + + QTest::newRow("remove multiple, all before visible items, remove item 0") + << 120.0 + << 0 << 4 + << 60.0; // removed top row, slide down by 1 row + + + // remove 3,4,5 before the visible pos, first row moves down to just before the visible pos, + // items 6,7 are removed from view, item 8 slides up to original pos of item 6 (120px) + QTest::newRow("remove multiple, mix of items from before and within visible items") + << 120.0 + << 3 << 5 + << 60.0; // adjust for the 1 row removed before the visible + + QTest::newRow("remove multiple, mix of items from before and within visible items, remove item 0") + << 120.0 + << 0 << 8 + << 60.0 * 2; // adjust for the 2 rows removed before the visible + + + QTest::newRow("remove 1, from start of visible, content at start") + << 0.0 + << 0 << 1 + << 0.0; + + QTest::newRow("remove multiple, from start of visible, content at start") + << 0.0 + << 0 << 3 + << 0.0; + + QTest::newRow("remove 1, from start of visible, content not at start") + << 120.0 // show 6-23 + << 4 << 1 + << 0.0; + + QTest::newRow("remove multiple, from start of visible, content not at start") + << 120.0 // show 6-23 + << 4 << 3 + << 0.0; + + + QTest::newRow("remove 1, from middle of visible, content at start") + << 0.0 + << 10 << 1 + << 0.0; + + QTest::newRow("remove multiple, from middle of visible, content at start") + << 0.0 + << 10 << 5 + << 0.0; + + QTest::newRow("remove 1, from middle of visible, content not at start") + << 120.0 // show 6-23 + << 10 << 1 + << 0.0; + + QTest::newRow("remove multiple, from middle of visible, content not at start") + << 120.0 // show 6-23 + << 10 << 5 + << 0.0; + + + QTest::newRow("remove 1, after visible, content at start") + << 0.0 + << 16 << 1 + << 0.0; + + QTest::newRow("remove multiple, after visible, content at start") + << 0.0 + << 16 << 5 + << 0.0; + + QTest::newRow("remove 1, after visible, content not at start") + << 120.0 // show 6-23 + << 16+4 << 1 + << 0.0; + + QTest::newRow("remove multiple, after visible, content not at start") + << 120.0 // show 6-23 + << 16+4 << 5 + << 0.0; + + QTest::newRow("remove multiple, mix of items from within and after visible items") + << 120.0 // show 6-23 + << 20 << 5 + << 0.0; +} + void tst_QQuickGridView::addOrRemoveBeforeVisible() { // QTBUG-21588: ensure re-layout is done on grid after adding or removing @@ -956,6 +1134,11 @@ void tst_QQuickGridView::clear() void tst_QQuickGridView::moved() { + if (QTest::currentDataTag() == QLatin1String("move 1 forwards, from non-visible -> visible") + || QTest::currentDataTag() == QLatin1String("move 1 forwards, from non-visible -> visible (move first item)")) { + QSKIP("QTBUG-23455"); + } + QFETCH(qreal, contentY); QFETCH(int, from); QFETCH(int, to); @@ -989,9 +1172,9 @@ void tst_QQuickGridView::moved() QTRY_VERIFY(currentItem != 0); gridview->setContentY(contentY); - model.moveItems(from, to, count); + QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false); - // wait for items to move + model.moveItems(from, to, count); QTRY_COMPARE(QQuickItemPrivate::get(gridview)->polishScheduled, false); // Confirm items positioned correctly and indexes correct @@ -1030,20 +1213,26 @@ void tst_QQuickGridView::moved_data() // model starts with 30 items, each 80x60, in area 240x320 // 18 items should be visible at a time + // The first visible item should not move upwards and out of the view + // if items are moved/removed before it. + + QTest::newRow("move 1 forwards, within visible items") << 0.0 << 1 << 8 << 1 << 0.0; + // skipped QTBUG-23455 QTest::newRow("move 1 forwards, from non-visible -> visible") << 120.0 // show 6-23 << 1 << 23 << 1 - << 0.0; // only 1 item was removed from the 1st row, so it doesn't move down + << 0.0; + // skipped QTBUG-23455 QTest::newRow("move 1 forwards, from non-visible -> visible (move first item)") << 120.0 // // show 6-23 << 0 << 6 << 1 - << 0.0; // only 1 item was removed from the 1st row, so it doesn't move down + << 0.0; QTest::newRow("move 1 forwards, from visible -> non-visible") << 0.0 diff --git a/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp b/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp index 7312e44..98d4f49 100644 --- a/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp +++ b/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp @@ -85,8 +85,12 @@ private slots: void qAbstractItemModel_inserted_more_data(); void qListModelInterface_removed(); + void qListModelInterface_removed_more(); + void qListModelInterface_removed_more_data(); void qListModelInterface_package_removed(); void qAbstractItemModel_removed(); + void qAbstractItemModel_removed_more(); + void qAbstractItemModel_removed_more_data(); void qListModelInterface_moved(); void qListModelInterface_moved_data(); @@ -167,6 +171,7 @@ private: template void inserted(const QUrl &source); template void inserted_more(); template void removed(const QUrl &source, bool animated); + template void removed_more(const QUrl &source); template void moved(const QUrl &source); template void clear(const QUrl &source); template void sections(const QUrl &source); @@ -180,6 +185,7 @@ private: void dumpTree(QQuickItem *parent, int depth = 0); void inserted_more_data(); + void removed_more_data(); void moved_data(); }; @@ -677,7 +683,7 @@ void tst_QQuickListView::inserted(const QUrl &source) QQuickItem *item = findItem(contentItem, "wrapper", 0); QVERIFY(item); QCOMPARE(item->y(), 0.); - QVERIFY(listview->contentY() == 0); + QTRY_VERIFY(listview->contentY() == 0); delete canvas; delete testObject; @@ -870,6 +876,7 @@ void tst_QQuickListView::insertBeforeVisible() int firstVisibleIndex = 20; // move to an index where the top item is not visible listview->setContentY(firstVisibleIndex * 20.0); listview->setCurrentIndex(firstVisibleIndex); + qApp->processEvents(); QTRY_COMPARE(listview->currentIndex(), firstVisibleIndex); QQuickItem *item = findItem(contentItem, "wrapper", firstVisibleIndex); @@ -971,7 +978,7 @@ void tst_QQuickListView::removed(const QUrl &source, bool /* animated */) } // Remove first item (which is the current item); - model.removeItem(0); // post: top item starts at 20 + model.removeItem(0); QTRY_COMPARE(canvas->rootObject()->property("count").toInt(), model.count()); name = findItem(contentItem, "textName", 0); @@ -987,7 +994,7 @@ void tst_QQuickListView::removed(const QUrl &source, bool /* animated */) QQuickItem *item = findItem(contentItem, "wrapper", i); if (!item) qWarning() << "Item" << i << "not found"; QTRY_VERIFY(item); - QTRY_COMPARE(item->y(),i*20.0 + 20.0); + QTRY_COMPARE(item->y(),i*20.0); } // Remove items not visible @@ -1000,14 +1007,14 @@ void tst_QQuickListView::removed(const QUrl &source, bool /* animated */) QQuickItem *item = findItem(contentItem, "wrapper", i); if (!item) qWarning() << "Item" << i << "not found"; QTRY_VERIFY(item); - QTRY_COMPARE(item->y(),i*20.0+20.0); + QTRY_COMPARE(item->y(),i*20.0); } // Remove items before visible listview->setContentY(80); listview->setCurrentIndex(10); - model.removeItem(1); // post: top item will be at 40 + model.removeItem(1); // post: top item will be at 20 QTRY_COMPARE(canvas->rootObject()->property("count").toInt(), model.count()); // Confirm items positioned correctly @@ -1015,7 +1022,7 @@ void tst_QQuickListView::removed(const QUrl &source, bool /* animated */) QQuickItem *item = findItem(contentItem, "wrapper", i); if (!item) qWarning() << "Item" << i << "not found"; QTRY_VERIFY(item); - QTRY_COMPARE(item->y(),40+i*20.0); + QTRY_COMPARE(item->y(),20+i*20.0); } // Remove current index @@ -1026,7 +1033,7 @@ void tst_QQuickListView::removed(const QUrl &source, bool /* animated */) QTRY_COMPARE(listview->currentIndex(), 9); QTRY_VERIFY(listview->currentItem() != oldCurrent); - listview->setContentY(40); // That's the top now + listview->setContentY(20); // That's the top now // let transitions settle. QTest::qWait(300); @@ -1036,7 +1043,7 @@ void tst_QQuickListView::removed(const QUrl &source, bool /* animated */) QQuickItem *item = findItem(contentItem, "wrapper", i); if (!item) qWarning() << "Item" << i << "not found"; QTRY_VERIFY(item); - QTRY_COMPARE(item->y(),40+i*20.0); + QTRY_COMPARE(item->y(),20+i*20.0); } // remove current item beyond visible items. @@ -1105,6 +1112,184 @@ void tst_QQuickListView::removed(const QUrl &source, bool /* animated */) } template +void tst_QQuickListView::removed_more(const QUrl &source) +{ + QFETCH(qreal, contentY); + QFETCH(int, removeIndex); + QFETCH(int, removeCount); + QFETCH(qreal, itemsOffsetAfterMove); + + QQuickText *name; + QQuickText *number; + QQuickView *canvas = createView(); + canvas->show(); + + T model; + for (int i = 0; i < 30; i++) + model.addItem("Item" + QString::number(i), ""); + + QDeclarativeContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + TestObject *testObject = new TestObject; + ctxt->setContextProperty("testObject", testObject); + + canvas->setSource(source); + qApp->processEvents(); + + QQuickListView *listview = findItem(canvas->rootObject(), "list"); + QTRY_VERIFY(listview != 0); + QQuickItem *contentItem = listview->contentItem(); + QTRY_VERIFY(contentItem != 0); + + listview->setContentY(contentY); + QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false); + + // wait for refill (after refill, items above the firstVisibleIndex-1 should not be rendered) + int firstVisibleIndex = contentY / 20; + if (firstVisibleIndex - 2 >= 0) + QTRY_VERIFY(!findItem(contentItem, "textName", firstVisibleIndex - 2)); + + model.removeItems(removeIndex, removeCount); + QTRY_COMPARE(listview->property("count").toInt(), model.count()); + + // check visibleItems.first() is in correct position + QQuickItem *item0 = findItem(contentItem, "wrapper", 0); + QVERIFY(item0); + QCOMPARE(item0->y(), itemsOffsetAfterMove); + + QList items = findItems(contentItem, "wrapper"); + for (int i=0; iy() >= contentY) { + QDeclarativeExpression e(qmlContext(items[i]), items[i], "index"); + firstVisibleIndex = e.evaluate().toInt(); + break; + } + } + QVERIFY2(firstVisibleIndex >= 0, QTest::toString(firstVisibleIndex)); + + // Confirm items positioned correctly and indexes correct + int itemCount = findItems(contentItem, "wrapper").count(); + for (int i = firstVisibleIndex; i < model.count() && i < itemCount; ++i) { + QQuickItem *item = findItem(contentItem, "wrapper", i); + QVERIFY2(item, QTest::toString(QString("Item %1 not found").arg(i))); + QTRY_COMPARE(item->y(), i*20.0 + itemsOffsetAfterMove); + name = findItem(contentItem, "textName", i); + QVERIFY(name != 0); + QTRY_COMPARE(name->text(), model.name(i)); + number = findItem(contentItem, "textNumber", i); + QVERIFY(number != 0); + QTRY_COMPARE(number->text(), model.number(i)); + } + + delete canvas; + delete testObject; +} + +void tst_QQuickListView::removed_more_data() +{ + QTest::addColumn("contentY"); + QTest::addColumn("removeIndex"); + QTest::addColumn("removeCount"); + QTest::addColumn("itemsOffsetAfterMove"); + + QTest::newRow("remove 1, before visible items") + << 80.0 // show 4-19 + << 3 << 1 + << 20.0; // visible items slide down by 1 item so that first visible does not move + + QTest::newRow("remove multiple, all before visible items") + << 80.0 + << 1 << 3 + << 20.0 * 3; + + QTest::newRow("remove multiple, all before visible items, remove item 0") + << 80.0 + << 0 << 4 + << 20.0 * 4; + + // remove 1,2,3 before the visible pos, 0 moves down to just before the visible pos, + // items 4,5 are removed from view, item 6 slides up to original pos of item 4 (80px) + QTest::newRow("remove multiple, mix of items from before and within visible items") + << 80.0 + << 1 << 5 + << 20.0 * 3; // adjust for the 3 items removed before the visible + + QTest::newRow("remove multiple, mix of items from before and within visible items, remove item 0") + << 80.0 + << 0 << 6 + << 20.0 * 4; // adjust for the 3 items removed before the visible + + + QTest::newRow("remove 1, from start of visible, content at start") + << 0.0 + << 0 << 1 + << 0.0; + + QTest::newRow("remove multiple, from start of visible, content at start") + << 0.0 + << 0 << 3 + << 0.0; + + QTest::newRow("remove 1, from start of visible, content not at start") + << 80.0 // show 4-19 + << 4 << 1 + << 0.0; + + QTest::newRow("remove multiple, from start of visible, content not at start") + << 80.0 // show 4-19 + << 4 << 3 + << 0.0; + + + QTest::newRow("remove 1, from middle of visible, content at start") + << 0.0 + << 10 << 1 + << 0.0; + + QTest::newRow("remove multiple, from middle of visible, content at start") + << 0.0 + << 10 << 5 + << 0.0; + + QTest::newRow("remove 1, from middle of visible, content not at start") + << 80.0 // show 4-19 + << 10 << 1 + << 0.0; + + QTest::newRow("remove multiple, from middle of visible, content not at start") + << 80.0 // show 4-19 + << 10 << 5 + << 0.0; + + + QTest::newRow("remove 1, after visible, content at start") + << 0.0 + << 16 << 1 + << 0.0; + + QTest::newRow("remove multiple, after visible, content at start") + << 0.0 + << 16 << 5 + << 0.0; + + QTest::newRow("remove 1, after visible, content not at middle") + << 80.0 // show 4-19 + << 16+4 << 1 + << 0.0; + + QTest::newRow("remove multiple, after visible, content not at start") + << 80.0 // show 4-19 + << 16+4 << 5 + << 0.0; + + QTest::newRow("remove multiple, mix of items from within and after visible items") + << 80.0 + << 18 << 5 + << 0.0; +} + +template void tst_QQuickListView::clear(const QUrl &source) { QQuickView *canvas = createView(); @@ -1182,10 +1367,10 @@ void tst_QQuickListView::moved(const QUrl &source) QTRY_VERIFY(currentItem != 0); listview->setContentY(contentY); - model.moveItems(from, to, count); + QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false); - // wait for items to move - QTest::qWait(100); + model.moveItems(from, to, count); + QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false); QList items = findItems(contentItem, "wrapper"); int firstVisibleIndex = -1; @@ -4210,6 +4395,16 @@ void tst_QQuickListView::qListModelInterface_removed() removed(testFileUrl("listviewtest.qml"), true); } +void tst_QQuickListView::qListModelInterface_removed_more() +{ + removed_more(testFileUrl("listviewtest.qml")); +} + +void tst_QQuickListView::qListModelInterface_removed_more_data() +{ + removed_more_data(); +} + void tst_QQuickListView::qListModelInterface_package_removed() { removed(testFileUrl("listviewtest-package.qml"), false); @@ -4222,6 +4417,16 @@ void tst_QQuickListView::qAbstractItemModel_removed() removed(testFileUrl("listviewtest.qml"), true); } +void tst_QQuickListView::qAbstractItemModel_removed_more() +{ + removed_more(testFileUrl("listviewtest.qml")); +} + +void tst_QQuickListView::qAbstractItemModel_removed_more_data() +{ + removed_more_data(); +} + void tst_QQuickListView::qListModelInterface_moved() { moved(testFileUrl("listviewtest.qml")); @@ -4659,9 +4864,9 @@ void tst_QQuickListView::unrequestedVisibility() model.moveItems(17, 16, 1); QTRY_COMPARE(QQuickItemPrivate::get(leftview)->polishScheduled, false); - QVERIFY(item = findItem(leftContent, "wrapper", 5)); + QVERIFY(item = findItem(leftContent, "wrapper", 4)); QCOMPARE(item->isVisible(), false); - QVERIFY(item = findItem(leftContent, "wrapper", 6)); + QVERIFY(item = findItem(leftContent, "wrapper", 5)); QCOMPARE(item->isVisible(), true); QVERIFY(item = findItem(rightContent, "wrapper", 16)); QCOMPARE(item->isVisible(), true);