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 <bea.lam@nokia.com>

diff --git a/src/declarative/items/qquickgridview.cpp b/src/declarative/items/qquickgridview.cpp
index a413314..f50c362 100644 (file)
--- 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<FxGridItemSG*>(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<FxGridItemSG*>(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<FxGridItemSG*>(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<FxGridItemSG*>(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<FxGridItemSG*>(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<FxGridItemSG*>(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 (file)
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 (file)
--- 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<QQuickGridView*>(canvas->rootObject());
+    QVERIFY(gridview != 0);
+
+    QQuickItem *contentItem = gridview->contentItem();
+    QVERIFY(contentItem != 0);
+
+    for (int i = 0; i < 10; ++i) {
+        QQuickItem *item = findItem<QQuickItem>(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<QQuickItem>(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<QQuickItem>(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<QQuickItem>(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()
 {