From 8eb0e48a051771fe99bce75497e803d98d9253ad Mon Sep 17 00:00:00 2001 From: Martin Jones Date: Thu, 10 Nov 2011 08:53:04 +1000 Subject: [PATCH] GridView sometimes lays out one less column than expected If the cellWidth/cellHeight was not a whole number the wrapping was unreliable. Calulate column positions more robustly, i.e. avoid comparing values subject to rounding errors. Task-number: QTBUG-21846 Change-Id: Ic3a90b36d542ce8af49461bd524e4405c74aece5 Reviewed-by: Bea Lam --- src/declarative/items/qquickgridview.cpp | 72 +++++++++++--------- .../declarative/qquickgridview/data/unaligned.qml | 15 ++++ .../qquickgridview/tst_qquickgridview.cpp | 60 ++++++++++++++++ 3 files changed, 114 insertions(+), 33 deletions(-) create mode 100644 tests/auto/declarative/qquickgridview/data/unaligned.qml diff --git a/src/declarative/items/qquickgridview.cpp b/src/declarative/items/qquickgridview.cpp index a413314..f50c362 100644 --- a/src/declarative/items/qquickgridview.cpp +++ b/src/declarative/items/qquickgridview.cpp @@ -398,16 +398,17 @@ FxViewItem *QQuickGridViewPrivate::newViewItem(int modelIndex, QQuickItem *item) bool QQuickGridViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool doBuffer) { - int colPos = colPosAt(visibleIndex); - int rowPos = rowPosAt(visibleIndex); + qreal colPos = colPosAt(visibleIndex); + qreal rowPos = rowPosAt(visibleIndex); if (visibleItems.count()) { FxGridItemSG *lastItem = static_cast(visibleItems.last()); rowPos = lastItem->rowPos(); - colPos = lastItem->colPos() + colSize(); - if (colPos > colSize() * (columns-1)) { - colPos = 0; + int colNum = qFloor((lastItem->colPos()+colSize()/2) / colSize()); + if (++colNum >= columns) { + colNum = 0; rowPos += rowSize(); } + colPos = colNum * colSize(); } int modelIndex = findLastVisibleIndex(); @@ -432,7 +433,7 @@ bool QQuickGridViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d rowPos = rowPosAt(visibleIndex); } - int colNum = colPos / colSize(); + int colNum = qFloor((colPos+colSize()/2) / colSize()); FxGridItemSG *item = 0; bool changed = false; @@ -442,30 +443,32 @@ bool QQuickGridViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d break; item->setPosition(colPos, rowPos); visibleItems.append(item); - colPos += colSize(); - colNum++; - if (colPos > colSize() * (columns-1)) { - colPos = 0; + if (++colNum >= columns) { colNum = 0; rowPos += rowSize(); } + colPos = colNum * colSize(); ++modelIndex; changed = true; if (doBuffer) // never buffer more than one item per frame break; } + // Find first column if (visibleItems.count()) { FxGridItemSG *firstItem = static_cast(visibleItems.first()); rowPos = firstItem->rowPos(); - colPos = firstItem->colPos() - colSize(); - if (colPos < 0) { - colPos = colSize() * (columns - 1); + colNum = qFloor((firstItem->colPos()+colSize()/2) / colSize()); + if (--colNum < 0) { + colNum = columns - 1; rowPos -= rowSize(); } + } else { + colNum = qFloor((colPos+colSize()/2) / colSize()); } - colNum = colPos / colSize(); + // Prepend + colPos = colNum * colSize(); while (visibleIndex > 0 && rowPos + rowSize() - 1 >= fillFrom - rowSize()*(colNum+1)/(columns+1)){ // qDebug() << "refill: prepend item" << visibleIndex-1 << "top pos" << rowPos << colPos; if (!(item = static_cast(createItem(visibleIndex-1)))) @@ -473,13 +476,11 @@ bool QQuickGridViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool d --visibleIndex; item->setPosition(colPos, rowPos); visibleItems.prepend(item); - colPos -= colSize(); - colNum--; - if (colPos < 0) { - colPos = colSize() * (columns - 1); + if (--colNum < 0) { colNum = columns-1; rowPos -= rowSize(); } + colPos = colNum * colSize(); changed = true; if (doBuffer) // never buffer more than one item per frame break; @@ -529,7 +530,8 @@ void QQuickGridViewPrivate::visibleItemsChanged() void QQuickGridViewPrivate::updateViewport() { Q_Q(QQuickGridView); - columns = (int)qMax((flow == QQuickGridView::LeftToRight ? q->width() : q->height()) / colSize(), qreal(1.)); + qreal length = flow == QQuickGridView::LeftToRight ? q->width() : q->height(); + columns = (int)qMax((length + colSize()/2) / colSize(), qreal(1.)); QQuickItemViewPrivate::updateViewport(); } @@ -546,11 +548,11 @@ void QQuickGridViewPrivate::layoutVisibleItems() } for (int i = 1; i < visibleItems.count(); ++i) { FxGridItemSG *item = static_cast(visibleItems.at(i)); - colPos += colSize(); - if (colPos > colSize() * (columns-1)) { - colPos = 0; + if (++col >= columns) { + col = 0; rowPos += rowSize(); } + colPos = col * colSize(); item->setPosition(colPos, rowPos); } } @@ -1763,22 +1765,25 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In } qreal tempPos = isRightToLeftTopToBottom() ? -position()-size()+q->width()+1 : position(); - int colPos = 0; - int rowPos = 0; + qreal colPos = 0; + qreal rowPos = 0; + int colNum = 0; if (visibleItems.count()) { if (index < visibleItems.count()) { FxGridItemSG *gridItem = static_cast(visibleItems.at(index)); colPos = gridItem->colPos(); rowPos = gridItem->rowPos(); + colNum = qFloor((colPos+colSize()/2) / colSize()); } else { // appending items to visible list FxGridItemSG *gridItem = static_cast(visibleItems.at(index-1)); - colPos = gridItem->colPos() + colSize(); rowPos = gridItem->rowPos(); - if (colPos > colSize() * (columns-1)) { - colPos = 0; + colNum = qFloor((gridItem->colPos()+colSize()/2) / colSize()); + if (++colNum >= columns) { + colNum = 0; rowPos += rowSize(); } + colPos = colNum * colSize(); } } @@ -1815,11 +1820,12 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In insertResult->sizeAddedBeforeVisible += rowSize(); } } - colPos -= colSize(); - if (colPos < 0) { - colPos = colSize() * (columns - 1); + + if (--colNum < 0 ) { + colNum = columns - 1; rowPos -= rowSize(); } + colPos = colNum * colSize(); index++; i--; } @@ -1839,11 +1845,11 @@ bool QQuickGridViewPrivate::applyInsertionChange(const QDeclarativeChangeSet::In visibleItems.insert(index, item); if (!change.isMove()) insertResult->addedItems.append(item); - colPos += colSize(); - if (colPos > colSize() * (columns-1)) { - colPos = 0; + if (++colNum >= columns) { + colNum = 0; rowPos += rowSize(); } + colPos = colNum * colSize(); ++index; ++i; } diff --git a/tests/auto/declarative/qquickgridview/data/unaligned.qml b/tests/auto/declarative/qquickgridview/data/unaligned.qml new file mode 100644 index 0000000..445400e --- /dev/null +++ b/tests/auto/declarative/qquickgridview/data/unaligned.qml @@ -0,0 +1,15 @@ +import QtQuick 2.0 + +GridView { + width: 400 + height: 200 + cellWidth: width/9 + cellHeight: height/2 + + model: testModel + delegate: Rectangle { + objectName: "wrapper"; width: 10; height: 10; color: "green" + Text { text: index } + } +} + diff --git a/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp b/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp index 7a399eb..7174232 100644 --- a/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/declarative/qquickgridview/tst_qquickgridview.cpp @@ -114,6 +114,7 @@ private slots: void creationContext(); void snapToRow_data(); void snapToRow(); + void unaligned(); private: QQuickView *createView(); @@ -3343,6 +3344,65 @@ void tst_QQuickGridView::snapToRow() delete canvas; } +void tst_QQuickGridView::unaligned() +{ + QQuickView *canvas = createView(); + canvas->show(); + + TestModel model; + for (int i = 0; i < 10; i++) + model.addItem("Item" + QString::number(i), ""); + + QDeclarativeContext *ctxt = canvas->rootContext(); + ctxt->setContextProperty("testModel", &model); + + canvas->setSource(QUrl::fromLocalFile(TESTDATA("unaligned.qml"))); + qApp->processEvents(); + + QQuickGridView *gridview = qobject_cast(canvas->rootObject()); + QVERIFY(gridview != 0); + + QQuickItem *contentItem = gridview->contentItem(); + QVERIFY(contentItem != 0); + + for (int i = 0; i < 10; ++i) { + QQuickItem *item = findItem(contentItem, "wrapper", i); + if (!item) qWarning() << "Item" << i << "not found"; + QVERIFY(item); + QCOMPARE(item->x(), qreal((i%9)*gridview->cellWidth())); + QCOMPARE(item->y(), qreal((i/9)*gridview->cellHeight())); + } + + // appending + for (int i = 10; i < 18; ++i) { + model.addItem("Item" + QString::number(i), ""); + QQuickItem *item = 0; + QTRY_VERIFY(item = findItem(contentItem, "wrapper", i)); + QCOMPARE(item->x(), qreal((i%9)*gridview->cellWidth())); + QCOMPARE(item->y(), qreal((i/9)*gridview->cellHeight())); + } + + // inserting + for (int i = 0; i < 10; ++i) { + model.insertItem(i, "Item" + QString::number(i), ""); + QQuickItem *item = 0; + QTRY_VERIFY(item = findItem(contentItem, "wrapper", i)); + QCOMPARE(item->x(), qreal((i%9)*gridview->cellWidth())); + QCOMPARE(item->y(), qreal((i/9)*gridview->cellHeight())); + } + + // removing + model.removeItems(7, 10); + qApp->processEvents(); + for (int i = 0; i < 18; ++i) { + QQuickItem *item = 0; + QTRY_VERIFY(item = findItem(contentItem, "wrapper", i)); + QCOMPARE(item->x(), qreal(i%9)*gridview->cellWidth()); + QCOMPARE(item->y(), qreal(i/9)*gridview->cellHeight()); + } + + delete canvas; +} QQuickView *tst_QQuickGridView::createView() { -- 1.7.2.5