QQuickTableView: improve performance when scrolling with scrollbars

When flicking, the current implementation would load and unload
edges around the table until the new viewport was covered. The downside
of that strategy is that you if you move the viewport a long
distance in one go, you will need to load and unload edges hidden
outside the viewport until it catches up with the new viewport. It gets
even worse if you flick with a scrollbar, since then you can end up
flicking thousands of rows in one go. And this will keep tableview
busy loading and unloading edges for a "long" time.

This patch will fix this issue by checking how much the viewport
changes during a flick, and select a strategy based on that. So if the
viewport moves more than a page (which is the size of the viewport), it
will schedule a rebuild of the table from the viewports new location,
rather than trying to load and unload edges until it catches up.

Fixes: QTBUG-70704
Change-Id: I88909e118ec0759a7b7a305c19ccc6670af6263b
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

3 files changed, 167 insertions, 25 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index e9d0ab8f3a..2ae006be29 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -587,6 +587,16 @@ void QQuickTableViewPrivate::enforceTableAtOrigin()
     }
 }
 
+void QQuickTableViewPrivate::updateAverageEdgeSize()
+{
+    int bottomCell = loadedTable.bottom();
+    int rightCell = loadedTable.right();
+    qreal accRowSpacing = bottomCell * cellSpacing.height();
+    qreal accColumnSpacing = rightCell * cellSpacing.width();
+    averageEdgeSize.setHeight((loadedTableOuterRect.bottom() - accRowSpacing) / (bottomCell + 1));
+    averageEdgeSize.setWidth((loadedTableOuterRect.right() - accColumnSpacing) / (rightCell + 1));
+}
+
 void QQuickTableViewPrivate::syncLoadedTableRectFromLoadedTable()
 {
     QRectF topLeftRect = loadedTableItem(loadedTable.topLeft())->geometry();
@@ -1172,14 +1182,16 @@ void QQuickTableViewPrivate::processLoadRequest()
 
     syncLoadedTableFromLoadRequest();
     layoutTableEdgeFromLoadRequest();
-
     syncLoadedTableRectFromLoadedTable();
-    enforceTableAtOrigin();
-    updateContentWidth();
-    updateContentHeight();
+
+    if (rebuildState == RebuildState::Done) {
+        enforceTableAtOrigin();
+        updateContentWidth();
+        updateContentHeight();
+        drainReusePoolAfterLoadRequest();
+    }
 
     loadRequest.markAsDone();
-    drainReusePoolAfterLoadRequest();
 
     qCDebug(lcTableViewDelegateLifecycle()) << "request completed! Table:" << tableLayoutToString();
 }
@@ -1260,23 +1272,34 @@ void QQuickTableViewPrivate::beginRebuildTable()
     if (loadRequest.isActive())
         cancelLoadRequest();
 
+    calculateTableSize();
+
     QPoint topLeft;
     QPointF topLeftPos;
-    calculateTableSize();
 
     if (rebuildOptions & RebuildOption::All) {
+        qCDebug(lcTableViewDelegateLifecycle()) << "RebuildOption::All";
         releaseLoadedItems(QQmlTableInstanceModel::NotReusable);
-        topLeft = QPoint(0, 0);
-        topLeftPos = QPoint(0, 0);
     } else if (rebuildOptions & RebuildOption::ViewportOnly) {
-        // Rebuild the table without flicking the content view back to origin, and
-        // start building from the same top left item that is currently showing
-        // (unless it has been removed from the model).
+        qCDebug(lcTableViewDelegateLifecycle()) << "RebuildOption::ViewportOnly";
         releaseLoadedItems(reusableFlag);
-        topLeft = loadedTable.topLeft();
-        topLeftPos = loadedTableOuterRect.topLeft();
-        topLeft.setX(qMin(topLeft.x(), tableSize.width() - 1));
-        topLeft.setY(qMin(topLeft.y(), tableSize.height() - 1));
+
+        if (rebuildOptions & RebuildOption::CalculateNewTopLeftRow) {
+            const int newRow = int(viewportRect.y() / (averageEdgeSize.height() + cellSpacing.height()));
+            topLeft.ry() = qBound(0, newRow, tableSize.height() - 1);
+            topLeftPos.ry() = topLeft.y() * (averageEdgeSize.height() + cellSpacing.height());
+        } else {
+            topLeft.ry() = qBound(0, loadedTable.topLeft().y(), tableSize.height() - 1);
+            topLeftPos.ry() = loadedTableOuterRect.topLeft().y();
+        }
+        if (rebuildOptions & RebuildOption::CalculateNewTopLeftColumn) {
+            const int newColumn = int(viewportRect.x() / (averageEdgeSize.width() + cellSpacing.width()));
+            topLeft.rx() = qBound(0, newColumn, tableSize.width() - 1);
+            topLeftPos.rx() = topLeft.x() * (averageEdgeSize.width() + cellSpacing.width());
+        } else {
+            topLeft.rx() = qBound(0, loadedTable.topLeft().x(), tableSize.width() - 1);
+            topLeftPos.rx() = loadedTableOuterRect.topLeft().x();
+        }
     } else {
         Q_TABLEVIEW_UNREACHABLE(rebuildOptions);
     }
@@ -1301,6 +1324,10 @@ void QQuickTableViewPrivate::layoutAfterLoadingInitialTable()
         // available yet for the calculation. So we do it now.
         relayoutTable();
     }
+
+    updateAverageEdgeSize();
+    updateContentWidth();
+    updateContentHeight();
 }
 
 void QQuickTableViewPrivate::loadInitialTopLeftItem(const QPoint &cell, const QPointF &pos)
@@ -1920,13 +1947,23 @@ void QQuickTableView::viewportMoved(Qt::Orientations orientation)
     Q_D(QQuickTableView);
     QQuickFlickable::viewportMoved(orientation);
 
-    // Calling polish() will schedule a polish event. But while the user is flicking, several
-    // mouse events will be handled before we get an updatePolish() call. And the updatePolish()
-    // call will only see the last mouse position. This results in a stuttering flick experience
-    // (especially on windows). We improve on this by calling updatePolish() directly. But this
-    // has the pitfall that we open up for recursive callbacks. E.g while inside updatePolish(), we
-    // load/unload items, and emit signals. The application can listen to those signals and set a
-    // new contentX/Y on the flickable. So we need to guard for this, to avoid unexpected behavior.
+    QQuickTableViewPrivate::RebuildOptions options = QQuickTableViewPrivate::RebuildOption::None;
+
+    // Check the viewport moved more than one page vertically
+    if (!d->viewportRect.intersects(QRectF(d->viewportRect.x(), contentY(), 1, height())))
+        options |= QQuickTableViewPrivate::RebuildOption::CalculateNewTopLeftRow;
+    // Check the viewport moved more than one page horizontally
+    if (!d->viewportRect.intersects(QRectF(contentX(), d->viewportRect.y(), width(), 1)))
+        options |= QQuickTableViewPrivate::RebuildOption::CalculateNewTopLeftColumn;
+
+    if (options) {
+        // When the viewport has moved more than one page vertically or horizontally, we switch
+        // strategy from refilling edges around the current table to instead rebuild the table
+        // from scratch inside the new viewport. This will greatly improve performance when flicking
+        // a long distance in one go, which can easily happen when dragging on scrollbars.
+        options |= QQuickTableViewPrivate::RebuildOption::ViewportOnly;
+        d->scheduleRebuildTable(options);
+    }
 
     if (d->rebuildScheduled) {
         // No reason to do anything, since we're about to rebuild the whole table anyway.
@@ -1936,6 +1973,13 @@ void QQuickTableView::viewportMoved(Qt::Orientations orientation)
         return;
     }
 
+    // Calling polish() will schedule a polish event. But while the user is flicking, several
+    // mouse events will be handled before we get an updatePolish() call. And the updatePolish()
+    // call will only see the last mouse position. This results in a stuttering flick experience
+    // (especially on windows). We improve on this by calling updatePolish() directly. But this
+    // has the pitfall that we open up for recursive callbacks. E.g while inside updatePolish(), we
+    // load/unload items, and emit signals. The application can listen to those signals and set a
+    // new contentX/Y on the flickable. So we need to guard for this, to avoid unexpected behavior.
     if (d->polishing)
         polish();
     else
diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h
index 4f5c819887..a4f829addd 100644
--- a/src/quick/items/qquicktableview_p_p.h
+++ b/src/quick/items/qquicktableview_p_p.h
@@ -182,7 +182,9 @@ public:
     enum class RebuildOption {
         None = 0,
         ViewportOnly = 0x1,
-        All = 0x2,
+        CalculateNewTopLeftRow = 0x2,
+        CalculateNewTopLeftColumn = 0x4,
+        All = 0x8,
     };
     Q_DECLARE_FLAGS(RebuildOptions, RebuildOption)
 
@@ -257,6 +259,8 @@ public:
     QQmlNullableValue<qreal> explicitContentWidth;
     QQmlNullableValue<qreal> explicitContentHeight;
 
+    QSizeF averageEdgeSize;
+
     const static QPoint kLeft;
     const static QPoint kRight;
     const static QPoint kUp;
@@ -289,6 +293,7 @@ public:
 
     void updateContentWidth();
     void updateContentHeight();
+    void updateAverageEdgeSize();
 
     void enforceTableAtOrigin();
     void syncLoadedTableRectFromLoadedTable();
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index 039fb91da0..60b938d127 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -117,6 +117,7 @@ private slots:
     void checkRowHeightProviderNotCallable();
     void checkForceLayoutFunction();
     void checkContentWidthAndHeight();
+    void checkPageFlicking();
     void checkExplicitContentWidthAndHeight();
     void checkContentXY();
     void noDelegate();
@@ -554,8 +555,15 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     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->polishScheduled);
+    QVERIFY(tableViewPrivate->rebuildScheduled);
+    WAIT_UNTIL_POLISHED;
+
     const int largeSizeCellCountInView = qCeil(tableView->width() / cellSizeLarge);
     const int columnCount = smallCellCount + largeSizeCellCountInView;
+    QCOMPARE(tableViewPrivate->loadedTable.left(), smallCellCount);
     QCOMPARE(tableViewPrivate->loadedTable.right(), columnCount - 1);
 
     const qreal firstHalfLength = smallCellCount * cellSizeSmall;
@@ -572,8 +580,20 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     // check that we then end up with the exact content width/height.
     const qreal secondHalfLength = largeCellCount * cellSizeLarge;
     const qreal expectedFullSize = (firstHalfLength + secondHalfLength) + accumulatedSpacing;
-    tableView->setContentX(expectedFullSize);
-    tableView->setContentY(expectedFullSize);
+
+    // If we flick more than one page at a time, tableview will jump to the new
+    // position and rebuild the table without loading the edges in-between. Which
+    // row and column that ends up as new top-left is then based on a prediction, and
+    // therefore unreliable. To avoid this to happen (which will also affect the
+    // reported size of the table), we flick to the end position in smaller chuncks.
+    QVERIFY(!tableViewPrivate->polishScheduled);
+    QVERIFY(!tableViewPrivate->rebuildScheduled);
+    int pages = qCeil((expectedFullSize - tableView->contentX()) / tableView->width());
+    for (int i = 0; i < pages; i++) {
+        tableView->setContentX(tableView->contentX() + tableView->width() - 1);
+        tableView->setContentY(tableView->contentY() + tableView->height() - 1);
+        QVERIFY(!tableViewPrivate->rebuildScheduled);
+    }
 
     QCOMPARE(tableView->contentWidth(), expectedFullSize);
     QCOMPARE(tableView->contentHeight(), expectedFullSize);
@@ -587,6 +607,79 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     QCOMPARE(tableView->contentHeight(), expectedFullSize);
 }
 
+void tst_QQuickTableView::checkPageFlicking()
+{
+    // Check that we rebuild the table instead of refilling edges, if the viewport moves
+    // more than a page (the size of TableView).
+    LOAD_TABLEVIEW("plaintableview.qml");
+
+    const int cellWidth = 100;
+    const int cellHeight = 50;
+    auto model = TestModelAsVariant(10000, 10000);
+
+    tableView->setModel(model);
+
+    WAIT_UNTIL_POLISHED;
+
+    // Sanity check startup table
+    QRect tableRect = tableViewPrivate->loadedTable;
+    QCOMPARE(tableRect.x(), 0);
+    QCOMPARE(tableRect.y(), 0);
+    QCOMPARE(tableRect.width(), tableView->width() / cellWidth);
+    QCOMPARE(tableRect.height(), tableView->height() / cellHeight);
+
+    // Since all cells have the same size, the average row/column
+    // size found by TableView should be exactly equal to this.
+    QCOMPARE(tableViewPrivate->averageEdgeSize.width(), cellWidth);
+    QCOMPARE(tableViewPrivate->averageEdgeSize.height(), cellHeight);
+
+    QVERIFY(!tableViewPrivate->rebuildScheduled);
+    QCOMPARE(tableViewPrivate->scheduledRebuildOptions, QQuickTableViewPrivate::RebuildOption::None);
+
+    // Flick 5000 columns to the right, and check that this triggers a
+    // rebuild, and that we end up at the expected top-left.
+    const int flickToColumn = 5000;
+    const qreal columnSpacing = tableView->columnSpacing();
+    const qreal flickToColumnInPixels = ((cellWidth + columnSpacing) * flickToColumn) - columnSpacing;
+    tableView->setContentX(flickToColumnInPixels);
+
+    QVERIFY(tableViewPrivate->rebuildScheduled);
+    QVERIFY(tableViewPrivate->scheduledRebuildOptions & QQuickTableViewPrivate::RebuildOption::ViewportOnly);
+    QVERIFY(tableViewPrivate->scheduledRebuildOptions & QQuickTableViewPrivate::RebuildOption::CalculateNewTopLeftColumn);
+    QVERIFY(!(tableViewPrivate->scheduledRebuildOptions & QQuickTableViewPrivate::RebuildOption::CalculateNewTopLeftRow));
+
+    WAIT_UNTIL_POLISHED;
+
+    tableRect = tableViewPrivate->loadedTable;
+    QCOMPARE(tableRect.x(), flickToColumn);
+    QCOMPARE(tableRect.y(), 0);
+    QCOMPARE(tableRect.width(), tableView->width() / cellWidth);
+    QCOMPARE(tableRect.height(), tableView->height() / cellHeight);
+
+    // Flick 5000 rows down as well. Since flicking down should only calculate a new row (but
+    // keep the current column), we deliberatly change the average width to check that it's
+    // actually ignored by the rebuild, and that the column stays the same.
+    tableViewPrivate->averageEdgeSize.rwidth() /= 2;
+
+    const int flickToRow = 5000;
+    const qreal rowSpacing = tableView->rowSpacing();
+    const qreal flickToRowInPixels = ((cellHeight + rowSpacing) * flickToRow) - rowSpacing;
+    tableView->setContentY(flickToRowInPixels);
+
+    QVERIFY(tableViewPrivate->rebuildScheduled);
+    QVERIFY(tableViewPrivate->scheduledRebuildOptions & QQuickTableViewPrivate::RebuildOption::ViewportOnly);
+    QVERIFY(!(tableViewPrivate->scheduledRebuildOptions & QQuickTableViewPrivate::RebuildOption::CalculateNewTopLeftColumn));
+    QVERIFY(tableViewPrivate->scheduledRebuildOptions & QQuickTableViewPrivate::RebuildOption::CalculateNewTopLeftRow);
+
+    WAIT_UNTIL_POLISHED;
+
+    tableRect = tableViewPrivate->loadedTable;
+    QCOMPARE(tableRect.x(), flickToRow);
+    QCOMPARE(tableRect.y(), flickToColumn);
+    QCOMPARE(tableRect.width(), tableView->width() / cellWidth);
+    QCOMPARE(tableRect.height(), tableView->height() / cellHeight);
+}
+
 void tst_QQuickTableView::checkExplicitContentWidthAndHeight()
 {
     // Check that you can set a custom contentWidth/Height, and that