QQuickTableView: don't recalculate content width while flicking

There are now three mechanisms in TableView that works together to
ensure that the table ends up edge-to-edge with the content view. They
are applied in the following order:

1. Adjust the content size, based on the predicted size of the table.
2. Adjust the origin and endExtend on the fly, if the content size is wrong.
3. Move the table directly to where it should be, in case we don't have
    time to wait for the origin to change.

We could have, strictly speaking, setteled with just one of them, but choose
to use them all at the same time for best flicking experience. Still, 1. and
2. sometimes step on each others feet when they both detect that something is
a bit off, and adjust.

So rather than adjusting the size of the content view every time we load a
new row or column, we just keep the first prediction. And then we leave all
later ajustments to 2. and 3. This turns out to be a more stable, and will
avoid some glitches that occur when flicking using a scrollbar, if several
mechanisms kick in at the same time.

Change-Id: Ib551a0bf8f6ee59ac9b3556b9462c91adb9cc80b
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
 
 
 

2 files changed, 36 insertions, 98 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 2b49200eaa..9583ef4231 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -305,9 +305,7 @@
     \l view, which means that the table's width could be larger or smaller than
     the viewport width. As a TableView cannot always know the exact width of
     the table without loading all columns in the model, the \c contentWidth is
-    usually an estimate based on the columns it has seen so far. This estimate
-    is recalculated whenever new columns are flicked into view, which means
-    that the content width can change dynamically.
+    usually an estimate based on the initially loaded table.
 
     If you know what the width of the table will be, assign a value to
     \c contentWidth, to avoid unnecessary calculations and updates to the
@@ -324,9 +322,7 @@
     \c view, which means that the table's height could be larger or smaller than the
     viewport height. As a TableView cannot always know the exact height of the
     table without loading all rows in the model, the \c contentHeight is
-    usually an estimate based on the rows it has seen so far. This estimate is
-    recalculated whenever new rows are flicked into view, which means that
-    the content height can change dynamically.
+    usually an estimate based on the initially loaded table.
 
     If you know what the height of the table will be, assign a
     value to \c contentHeight, to avoid unnecessary calculations and updates to
@@ -1539,19 +1535,6 @@ void QQuickTableViewPrivate::processLoadRequest()
     if (rebuildState == RebuildState::Done) {
         // Loading of this edge was not done as a part of a rebuild, but
         // instead as an incremental build after e.g a flick.
-        switch (loadRequest.edge()) {
-        case Qt::LeftEdge:
-        case Qt::TopEdge:
-            break;
-        case Qt::RightEdge:
-            updateAverageEdgeSize();
-            updateContentWidth();
-            break;
-        case Qt::BottomEdge:
-            updateAverageEdgeSize();
-            updateContentHeight();
-            break;
-        }
         updateExtents();
         drainReusePoolAfterLoadRequest();
     }
@@ -1838,9 +1821,23 @@ void QQuickTableViewPrivate::layoutAfterLoadingInitialTable()
         syncLoadedTableRectFromLoadedTable();
     }
 
-    updateAverageEdgeSize();
-    updateContentWidth();
-    updateContentHeight();
+    if (syncView || rebuildOptions.testFlag(RebuildOption::All)) {
+        // We try to limit how often we update the content size. The main reason is that is has a
+        // tendency to cause flicker in the viewport if it happens while flicking. But another just
+        // as valid reason is that we actually never really know what the size of the full table will
+        // ever be. Even if e.g spacing changes, and we normally would assume that the size of the table
+        // would increase accordingly, the model might also at some point have removed/hidden/resized
+        // rows/columns outside the viewport. This would also affect the size, but since we don't load
+        // rows or columns outside the viewport, this information is ignored. And even if we did, we
+        // might also have been fast-flicked to a new location at some point, and started a new rebuild
+        // there based on a new guesstimated top-left cell. Either way, changing the content size
+        // based on the currently visible row/columns/spacing can be really off. So instead of pretending
+        // that we know what the actual size of the table is, we just keep the first guesstimate.
+        updateAverageEdgeSize();
+        updateContentWidth();
+        updateContentHeight();
+    }
+
     updateExtents();
 }
 
@@ -1856,8 +1853,6 @@ void QQuickTableViewPrivate::unloadEdge(Qt::Edge edge)
             unloadItem(QPoint(column, r.key()));
         loadedColumns.remove(column);
         syncLoadedTableRectFromLoadedTable();
-        updateAverageEdgeSize();
-        updateContentWidth();
         break; }
     case Qt::TopEdge:
     case Qt::BottomEdge: {
@@ -1866,8 +1861,6 @@ void QQuickTableViewPrivate::unloadEdge(Qt::Edge edge)
             unloadItem(QPoint(c.key(), row));
         loadedRows.remove(row);
         syncLoadedTableRectFromLoadedTable();
-        updateAverageEdgeSize();
-        updateContentHeight();
         break; }
     }
 
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index d03f08ec6a..fbe56abda5 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -563,7 +563,7 @@ void tst_QQuickTableView::checkForceLayoutFunction()
 
 void tst_QQuickTableView::checkContentWidthAndHeight()
 {
-    // Check that contentWidth/Height reports the correct size of the the
+    // Check that contentWidth/Height reports the correct size of the
     // table, based on knowledge of the rows and columns that has been loaded.
     LOAD_TABLEVIEW("contentwidthheight.qml");
 
@@ -574,11 +574,7 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
 
     const int tableSize = 100;
     const int cellSizeSmall = 100;
-    const int cellSizeLarge = 200;
     const int spacing = 1;
-    const int smallCellCount = 20;
-    const int largeCellCount = tableSize - smallCellCount;
-    const qreal accumulatedSpacing = ((tableSize - 1) * spacing);
     auto model = TestModelAsVariant(tableSize, tableSize);
 
     tableView->setModel(model);
@@ -591,71 +587,12 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     QCOMPARE(tableViewPrivate->averageEdgeSize.width(), cellSizeSmall);
     QCOMPARE(tableViewPrivate->averageEdgeSize.height(), cellSizeSmall);
 
-    // Flick in 5 more rows and columns, but not so far that we start loading in
-    // the ones that are bigger. Loading in more rows and columns of the same
-    // size as the initial ones should not change the first prediction.
-    qreal flickTo = ((cellSizeSmall + spacing) * 5);
-    tableView->setContentX(flickTo);
-    tableView->setContentY(flickTo);
+    // Flick to the end, and check that content width/height stays unchanged
+    tableView->setContentX(tableView->contentWidth() - tableView->width());
+    tableView->setContentY(tableView->contentHeight() - tableView->height());
 
     QCOMPARE(tableView->contentWidth(), expectedSizeInit);
     QCOMPARE(tableView->contentHeight(), expectedSizeInit);
-    QCOMPARE(tableViewPrivate->averageEdgeSize.width(), cellSizeSmall);
-    QCOMPARE(tableViewPrivate->averageEdgeSize.height(), cellSizeSmall);
-
-    // Flick to row and column 20 (smallCellCount), since there the row and
-    // column sizes increases with 100. Check that TableView then adjusts
-    // contentWidth and contentHeight accordingly.
-    flickTo = ((cellSizeSmall + spacing) * smallCellCount) - spacing;
-    tableView->setContentX(flickTo);
-    tableView->setContentY(flickTo);
-
-    // Since we move the viewport more than a page, tableview
-    // will jump to the new position and do a rebuild.
-    QVERIFY(tableViewPrivate->scheduledRebuildOptions);
-    WAIT_UNTIL_POLISHED;
-
-    // Check that the average cell size is now matching the
-    // large cells since they fill up the whole view.
-    QCOMPARE(tableViewPrivate->averageEdgeSize.width(), cellSizeLarge);
-    QCOMPARE(tableViewPrivate->averageEdgeSize.height(), cellSizeLarge);
-
-    const int largeSizeCellCountInView = qCeil(tableView->width() / cellSizeLarge);
-    const int columnCount = smallCellCount + largeSizeCellCountInView;
-    QCOMPARE(tableViewPrivate->leftColumn(), smallCellCount);
-    QCOMPARE(tableViewPrivate->rightColumn(), columnCount - 1);
-
-    const qreal firstHalfLength = smallCellCount * cellSizeSmall;
-    const qreal secondHalfOneScreenLength = largeSizeCellCountInView * cellSizeLarge;
-    const qreal lengthAfterFlick = firstHalfLength + secondHalfOneScreenLength;
-
-    // Check that loadedTableOuterRect has been calculated correct thus far
-    const qreal spacingAfterFlick = (smallCellCount + largeSizeCellCountInView - 1) * spacing;
-    QCOMPARE(tableViewPrivate->loadedTableOuterRect.left(), flickTo + spacing);
-    QCOMPARE(tableViewPrivate->loadedTableOuterRect.right(), lengthAfterFlick + spacingAfterFlick);
-    QCOMPARE(tableViewPrivate->loadedTableOuterRect.top(), flickTo + spacing);
-    QCOMPARE(tableViewPrivate->loadedTableOuterRect.bottom(), lengthAfterFlick + spacingAfterFlick);
-
-    // At this point, we should have the exact content width/height set, because
-    // TableView knows where the large cells start in the viewport, and how many
-    // columns that remain in the model. It will assume that the rest of the the
-    // columns have the same average size as the ones currently inside the viewport.
-    const qreal expectedContentSize = (smallCellCount * cellSizeSmall) + (largeCellCount * cellSizeLarge) + accumulatedSpacing;
-    QCOMPARE(tableView->contentWidth(), expectedContentSize);
-    QCOMPARE(tableView->contentHeight(), expectedContentSize);
-
-    // Flick to the end (row/column 100, and overshoot a bit), and
-    // check that we then end up with the exact content width/height.
-    const qreal secondHalfLength = largeCellCount * cellSizeLarge;
-    const qreal expectedFullSize = (firstHalfLength + secondHalfLength) + accumulatedSpacing;
-    const qreal overshoot = 100;
-    const qreal endPosX = expectedFullSize - tableView->width() + overshoot;
-    const qreal endPosY = expectedFullSize - tableView->height() + overshoot;
-    tableView->setContentX(endPosX);
-    tableView->setContentY(endPosY);
-
-    QCOMPARE(tableView->contentWidth(), expectedFullSize);
-    QCOMPARE(tableView->contentHeight(), expectedFullSize);
 
     // Flick back to start
     tableView->setContentX(0);
@@ -667,7 +604,7 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     QVERIFY(tableViewPrivate->scheduledRebuildOptions);
     WAIT_UNTIL_POLISHED;
 
-    // We should now have the same content width/height as when we started
+    // We should still have the same content width/height as when we started
     QCOMPARE(tableView->contentWidth(), expectedSizeInit);
     QCOMPARE(tableView->contentHeight(), expectedSizeInit);
 }
@@ -1303,8 +1240,11 @@ void tst_QQuickTableView::checkSpacingValues()
 
     tableView->polish();
     WAIT_UNTIL_POLISHED;
-    QCOMPARE(tableView->contentWidth(), columnCount * (delegateWidth + tableView->columnSpacing()) - tableView->columnSpacing());
-    QCOMPARE(tableView->contentHeight(), rowCount * (delegateHeight + tableView->rowSpacing()) - tableView->rowSpacing());
+
+    const qreal expectedInitialContentWidth = columnCount * (delegateWidth + tableView->columnSpacing()) - tableView->columnSpacing();
+    const qreal expectedInitialContentHeight = rowCount * (delegateHeight + tableView->rowSpacing()) - tableView->rowSpacing();
+    QCOMPARE(tableView->contentWidth(), expectedInitialContentWidth);
+    QCOMPARE(tableView->contentHeight(), expectedInitialContentHeight);
 
     // Valid spacing assignment
     tableView->setRowSpacing(42);
@@ -1314,8 +1254,13 @@ void tst_QQuickTableView::checkSpacingValues()
 
     tableView->polish();
     WAIT_UNTIL_POLISHED;
-    QCOMPARE(tableView->contentWidth(), columnCount * (delegateWidth + tableView->columnSpacing()) - tableView->columnSpacing());
-    QCOMPARE(tableView->contentHeight(), rowCount * (delegateHeight + tableView->rowSpacing()) - tableView->rowSpacing());
+
+    // Even after changing spacing, TableView will keep the initial guesstimated content size. The
+    // reason is that deciding the content size based on the currently visible row/columns/spacing
+    // will almost always be at a little bit wrong at best. So instead of pretending that TableView
+    // knows what the size of the full table is, it sticks with the first guesstimate.
+    QCOMPARE(tableView->contentWidth(), expectedInitialContentWidth);
+    QCOMPARE(tableView->contentHeight(), expectedInitialContentHeight);
 
     // Invalid assignments (should ignore)
     tableView->setRowSpacing(-1);