QQuickTableView: don't position the table contents outside the viewport

If you call positionViewAtCell(row, Qt.AlignTop) for the last row
in the table, the row will be aligned to the top of the view, as
requested. But this looks really wrong , since it will cause the
table to be flicked to a position that causes it to overshoot
by a distance close to the height of the whole view, effectively
leaving a big empty gap at the bottom. This looks really buggy.

This patch will correct this behavior by ensuring that we never
flick the table to an "invalid" position while positioning
table at a cell using the requested alignment. Instead we
clamp the table to the edges of the viewport.

Fixes: QTBUG-100680
Change-Id: Id3003df784a0128df28ee2e78e2456e1fa1e11e8
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
(cherry picked from commit c3d909ec286962c507e346a8d490b905cdd266f8)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

3 files changed, 219 insertions, 55 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 665246af66..f1c20563d2 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -2350,6 +2350,20 @@ void QQuickTableViewPrivate::processRebuildTable()
             return;
     }
 
+    if (rebuildState == RebuildState::CancelOvershootBottomRight) {
+        cancelOvershootBottomRight();
+        loadAndUnloadVisibleEdges();
+        if (!moveToNextRebuildState())
+            return;
+    }
+
+    if (rebuildState == RebuildState::CancelOvershootTopLeft) {
+        cancelOvershootTopLeft();
+        loadAndUnloadVisibleEdges();
+        if (!moveToNextRebuildState())
+            return;
+    }
+
     const bool preload = (rebuildOptions & RebuildOption::All
                           && reusableFlag == QQmlTableInstanceModel::Reusable);
 
@@ -2684,6 +2698,58 @@ void QQuickTableViewPrivate::adjustViewportYAccordingToAlignment()
     syncViewportRect();
 }
 
+void QQuickTableViewPrivate::cancelOvershootBottomRight()
+{
+    // After doing a layout with positioning specified, the table might end up in an
+    // overshooting state (meaning that the table is dragged beyond the bounds
+    // of the viewport, with the result that it will bounce back in once you touch it).
+    // This happens because the layouting might try to e.g position the last row in the
+    // model in the center of the viewport, which will leave a big blank space from the
+    // last row towards the bottom.
+    // This space will be detected by updateExtents(), which will adjust the extents so
+    // that the superfluous blank area ends up as an overshoot. But we shouldn't leave the
+    // table in an overshooting state after a rebuild, so we clamp it here so thatit's
+    // either aligned to the top/left or to the bottom/right of the viewport.
+    // An exception is if we're not rebuilding because of positionViewAtCell(), since
+    // the app is allowed to set contentX and contentY at start-up. A second exception is
+    // if this view is a sync child, since then we can end up with extra space at the end
+    // if our model has fewer rows/columns than the syncView.
+    const qreal blankSpaceRight = viewportRect.right() - loadedTableOuterRect.right();
+    const qreal blankSpaceBottom = viewportRect.bottom() - loadedTableOuterRect.bottom();
+    const bool positionVertically = rebuildOptions.testFlag(RebuildOption::PositionViewAtRow);
+    const bool positionHorizontally = rebuildOptions.testFlag(RebuildOption::PositionViewAtColumn);
+
+    if (positionVertically && !syncVertically && viewportRect.top() > 0 && blankSpaceBottom > 0) {
+        qCDebug(lcTableViewDelegateLifecycle()) << "cancelling overshoot at bottom:" << blankSpaceBottom;
+        setLocalViewportY(viewportRect.y() - blankSpaceBottom);
+        syncViewportRect();
+    }
+
+    if (positionHorizontally && !syncHorizontally && viewportRect.left() > 0 && blankSpaceRight > 0) {
+        qCDebug(lcTableViewDelegateLifecycle()) << "cancelling overshoot at right:" << blankSpaceRight;
+        setLocalViewportX(viewportRect.x() - blankSpaceRight);
+        syncViewportRect();
+    }
+}
+
+void QQuickTableViewPrivate::cancelOvershootTopLeft()
+{
+    const bool positionVertically = rebuildOptions.testFlag(RebuildOption::PositionViewAtRow);
+    const bool positionHorizontally = rebuildOptions.testFlag(RebuildOption::PositionViewAtColumn);
+
+    if (positionVertically && !syncVertically && viewportRect.top() < 0) {
+        qCDebug(lcTableViewDelegateLifecycle()) << "cancelling overshoot at top:" << viewportRect.top();
+        setLocalViewportY(0);
+        syncViewportRect();
+    }
+
+    if (positionHorizontally && !syncHorizontally && viewportRect.left() < 0) {
+        qCDebug(lcTableViewDelegateLifecycle()) << "cancelling overshoot at left:" << viewportRect.left();
+        setLocalViewportX(0);
+        syncViewportRect();
+    }
+}
+
 void QQuickTableViewPrivate::unloadEdge(Qt::Edge edge)
 {
     Q_Q(QQuickTableView);
diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h
index 77b492b707..6fd05c5508 100644
--- a/src/quick/items/qquicktableview_p_p.h
+++ b/src/quick/items/qquicktableview_p_p.h
@@ -202,6 +202,8 @@ public:
         VerifyTable,
         LayoutTable,
         LoadAndUnloadAfterLayout,
+        CancelOvershootBottomRight,
+        CancelOvershootTopLeft,
         PreloadColumns,
         PreloadRows,
         MovePreloadedItemsToPool,
@@ -434,6 +436,9 @@ public:
     void adjustViewportXAccordingToAlignment();
     void adjustViewportYAccordingToAlignment();
 
+    void cancelOvershootBottomRight();
+    void cancelOvershootTopLeft();
+
     void scheduleRebuildTable(QQuickTableViewPrivate::RebuildOptions options);
 
     QTypeRevision resolveImportVersion();
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index 8c9680f305..2f0e6890e0 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -192,6 +192,10 @@ private slots:
     void positionViewAtRow();
     void positionViewAtColumn_data();
     void positionViewAtColumn();
+    void positionViewAtRowClamped_data();
+    void positionViewAtRowClamped();
+    void positionViewAtColumnClamped_data();
+    void positionViewAtColumnClamped();
     void itemAtCell_data();
     void itemAtCell();
     void leftRightTopBottomProperties_data();
@@ -3138,48 +3142,30 @@ void tst_QQuickTableView::positionViewAtRow_data()
     QTest::newRow("AlignTop 1") << 1 << Qt::AlignTop << 0. << 50.;
     QTest::newRow("AlignTop 50") << 50 << Qt::AlignTop << 0. << -1.;
     QTest::newRow("AlignTop 0") << 0 << Qt::AlignTop << 0. << -1.;
-    QTest::newRow("AlignTop 99") << 99 << Qt::AlignTop << 0. << -1.;
-
-    QTest::newRow("AlignTop 0") << 0 << Qt::AlignTop << -10. << 0.;
     QTest::newRow("AlignTop 1") << 1 << Qt::AlignTop << -10. << 0.;
     QTest::newRow("AlignTop 1") << 1 << Qt::AlignTop << -10. << 50.;
     QTest::newRow("AlignTop 50") << 50 << Qt::AlignTop << -10. << -1.;
-    QTest::newRow("AlignTop 0") << 0 << Qt::AlignTop << -10. << -1.;
-    QTest::newRow("AlignTop 99") << 99 << Qt::AlignTop << -10. << -1.;
 
-    QTest::newRow("AlignBottom 0") << 0 << Qt::AlignBottom << 0. << 0.;
-    QTest::newRow("AlignBottom 1") << 1 << Qt::AlignBottom << 0. << 0.;
-    QTest::newRow("AlignBottom 1") << 1 << Qt::AlignBottom << 0. << 50.;
     QTest::newRow("AlignBottom 50") << 50 << Qt::AlignBottom << 0. << -1.;
-    QTest::newRow("AlignBottom 0") << 0 << Qt::AlignBottom << 0. << -1.;
+    QTest::newRow("AlignBottom 98") << 98 << Qt::AlignBottom << 0. << -1.;
     QTest::newRow("AlignBottom 99") << 99 << Qt::AlignBottom << 0. << -1.;
+    QTest::newRow("AlignBottom 50") << 40 << Qt::AlignBottom << 10. << -1.;
+    QTest::newRow("AlignBottom 40") << 50 << Qt::AlignBottom << -10. << -1.;
+    QTest::newRow("AlignBottom 98") << 98 << Qt::AlignBottom << 10. << -1.;
+    QTest::newRow("AlignBottom 99") << 99 << Qt::AlignBottom << -10. << -1.;
 
-    QTest::newRow("AlignBottom 0") << 0 << Qt::AlignBottom << 10. << 0.;
-    QTest::newRow("AlignBottom 1") << 1 << Qt::AlignBottom << 10. << 0.;
-    QTest::newRow("AlignBottom 1") << 1 << Qt::AlignBottom << 10. << 50.;
-    QTest::newRow("AlignBottom 50") << 50 << Qt::AlignBottom << 10. << -1.;
-    QTest::newRow("AlignBottom 0") << 0 << Qt::AlignBottom << 10. << -1.;
-    QTest::newRow("AlignBottom 99") << 99 << Qt::AlignBottom << 10. << -1.;
-
-    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << 0. << 0.;
-    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << 0. << 0.;
-    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << 0. << 50.;
+    QTest::newRow("AlignCenter 40") << 40 << Qt::AlignCenter << 0. << -1.;
     QTest::newRow("AlignCenter 50") << 50 << Qt::AlignCenter << 0. << -1.;
-    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << 0. << -1.;
-    QTest::newRow("AlignCenter 99") << 99 << Qt::AlignCenter << 0. << -1.;
-
-    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << -10. << 0.;
-    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << -10. << 0.;
-    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << -10. << 50.;
+    QTest::newRow("AlignCenter 40") << 40 << Qt::AlignCenter << 10. << -1.;
     QTest::newRow("AlignCenter 50") << 50 << Qt::AlignCenter << -10. << -1.;
-    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << -10. << -1.;
-    QTest::newRow("AlignCenter 99") << 99 << Qt::AlignCenter << -10. << -1.;
 }
 
 void tst_QQuickTableView::positionViewAtRow()
 {
     // Check that positionViewAtRow actually flicks the view
-    // to the right position so that the row becomes visible
+    // to the right position so that the row becomes visible.
+    // For this test, we only check cells that can be placed exactly
+    // according to the given alignment.
     QFETCH(int, row);
     QFETCH(Qt::AlignmentFlag, alignment);
     QFETCH(qreal, offset);
@@ -3229,48 +3215,27 @@ void tst_QQuickTableView::positionViewAtColumn_data()
     QTest::newRow("AlignLeft 1") << 1 << Qt::AlignLeft << 0. << 50.;
     QTest::newRow("AlignLeft 50") << 50 << Qt::AlignLeft << 0. << -1.;
     QTest::newRow("AlignLeft 0") << 0 << Qt::AlignLeft << 0. << -1.;
-    QTest::newRow("AlignLeft 99") << 99 << Qt::AlignLeft << 0. << -1.;
-
-    QTest::newRow("AlignLeft 0") << 0 << Qt::AlignLeft << -10. << 0.;
     QTest::newRow("AlignLeft 1") << 1 << Qt::AlignLeft << -10. << 0.;
     QTest::newRow("AlignLeft 1") << 1 << Qt::AlignLeft << -10. << 50.;
     QTest::newRow("AlignLeft 50") << 50 << Qt::AlignLeft << -10. << -1.;
-    QTest::newRow("AlignLeft 0") << 0 << Qt::AlignLeft << -10. << -1.;
-    QTest::newRow("AlignLeft 99") << 99 << Qt::AlignLeft << -10. << -1.;
 
-    QTest::newRow("AlignRight 0") << 0 << Qt::AlignRight << 0. << 0.;
-    QTest::newRow("AlignRight 1") << 1 << Qt::AlignRight << 0. << 0.;
-    QTest::newRow("AlignRight 1") << 1 << Qt::AlignRight << 0. << 50.;
     QTest::newRow("AlignRight 50") << 50 << Qt::AlignRight << 0. << -1.;
-    QTest::newRow("AlignRight 0") << 0 << Qt::AlignRight << 0. << -1.;
     QTest::newRow("AlignRight 99") << 99 << Qt::AlignRight << 0. << -1.;
-
-    QTest::newRow("AlignRight 0") << 0 << Qt::AlignRight << 10. << 0.;
-    QTest::newRow("AlignRight 1") << 1 << Qt::AlignRight << 10. << 0.;
-    QTest::newRow("AlignRight 1") << 1 << Qt::AlignRight << 10. << 50.;
     QTest::newRow("AlignRight 50") << 50 << Qt::AlignRight << 10. << -1.;
-    QTest::newRow("AlignRight 0") << 0 << Qt::AlignRight << 10. << -1.;
-    QTest::newRow("AlignRight 99") << 99 << Qt::AlignRight << 10. << -1.;
+    QTest::newRow("AlignRight 99") << 99 << Qt::AlignRight << -10. << -1.;
 
-    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << 0. << 0.;
-    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << 0. << 0.;
-    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << 0. << 50.;
+    QTest::newRow("AlignCenter 40") << 50 << Qt::AlignCenter << 0. << -1.;
     QTest::newRow("AlignCenter 50") << 50 << Qt::AlignCenter << 0. << -1.;
-    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << 0. << -1.;
-    QTest::newRow("AlignCenter 99") << 99 << Qt::AlignCenter << 0. << -1.;
-
-    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << -10. << 0.;
-    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << -10. << 0.;
-    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << -10. << 50.;
+    QTest::newRow("AlignCenter 40") << 50 << Qt::AlignCenter << 10. << -1.;
     QTest::newRow("AlignCenter 50") << 50 << Qt::AlignCenter << -10. << -1.;
-    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << -10. << -1.;
-    QTest::newRow("AlignCenter 99") << 99 << Qt::AlignCenter << -10. << -1.;
 }
 
 void tst_QQuickTableView::positionViewAtColumn()
 {
     // Check that positionViewAtColumn actually flicks the view
-    // to the right position so that the row becomes visible
+    // to the right position so that the row becomes visible.
+    // For this test, we only check cells that can be placed exactly
+    // according to the given alignment.
     QFETCH(int, column);
     QFETCH(Qt::AlignmentFlag, alignment);
     QFETCH(qreal, offset);
@@ -3308,6 +3273,134 @@ void tst_QQuickTableView::positionViewAtColumn()
     }
 }
 
+void tst_QQuickTableView::positionViewAtRowClamped_data()
+{
+    QTest::addColumn<int>("row");
+    QTest::addColumn<Qt::AlignmentFlag>("alignment");
+    QTest::addColumn<qreal>("offset");
+    QTest::addColumn<qreal>("contentYStartPos");
+
+    QTest::newRow("AlignTop 0") << 0 << Qt::AlignTop << -10. << 0.;
+    QTest::newRow("AlignTop 0") << 0 << Qt::AlignTop << -10. << -1.;
+    QTest::newRow("AlignTop 99") << 99 << Qt::AlignTop << 0. << -1.;
+    QTest::newRow("AlignTop 99") << 99 << Qt::AlignTop << -10. << -1.;
+
+    QTest::newRow("AlignBottom 0") << 0 << Qt::AlignBottom << 0. << 0.;
+    QTest::newRow("AlignBottom 1") << 1 << Qt::AlignBottom << 0. << 0.;
+    QTest::newRow("AlignBottom 1") << 1 << Qt::AlignBottom << 0. << 50.;
+    QTest::newRow("AlignBottom 0") << 0 << Qt::AlignBottom << 0. << -1.;
+
+    QTest::newRow("AlignBottom 0") << 0 << Qt::AlignBottom << 10. << 0.;
+    QTest::newRow("AlignBottom 1") << 1 << Qt::AlignBottom << 10. << 0.;
+    QTest::newRow("AlignBottom 1") << 1 << Qt::AlignBottom << 10. << 50.;
+    QTest::newRow("AlignBottom 0") << 0 << Qt::AlignBottom << 10. << -1.;
+    QTest::newRow("AlignBottom 99") << 99 << Qt::AlignBottom << 10. << -1.;
+
+    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << 0. << 0.;
+    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << 0. << 0.;
+    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << 0. << 50.;
+    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << 0. << -1.;
+    QTest::newRow("AlignCenter 99") << 99 << Qt::AlignCenter << 0. << -1.;
+
+    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << -10. << 0.;
+    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << -10. << 0.;
+    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << -10. << 50.;
+    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << -10. << -1.;
+    QTest::newRow("AlignCenter 99") << 99 << Qt::AlignCenter << -10. << -1.;
+}
+
+void tst_QQuickTableView::positionViewAtRowClamped()
+{
+    // Check that positionViewAtRow actually flicks the table to the
+    // right position so that the row becomes visible. For this test, we
+    // only test cells that cannot be placed exactly at the given alignment,
+    // because it would cause the table to overshoot. Instead the
+    // table should be flicked to the edge of the viewport, close to the
+    // requested alignment.
+    QFETCH(int, row);
+    QFETCH(Qt::AlignmentFlag, alignment);
+    QFETCH(qreal, offset);
+    QFETCH(qreal, contentYStartPos);
+
+    LOAD_TABLEVIEW("plaintableview.qml");
+    auto model = TestModelAsVariant(100, 100);
+    tableView->setModel(model);
+    if (contentYStartPos >= 0)
+        tableView->setContentY(contentYStartPos);
+
+    WAIT_UNTIL_POLISHED;
+
+    tableView->positionViewAtRow(row, alignment, offset);
+
+    WAIT_UNTIL_POLISHED;
+
+    QCOMPARE(tableView->contentY(), row < 50 ? 0 : tableView->contentHeight() - tableView->height());
+}
+
+void tst_QQuickTableView::positionViewAtColumnClamped_data()
+{
+    QTest::addColumn<int>("column");
+    QTest::addColumn<Qt::AlignmentFlag>("alignment");
+    QTest::addColumn<qreal>("offset");
+    QTest::addColumn<qreal>("contentXStartPos");
+
+    QTest::newRow("AlignLeft 0") << 0 << Qt::AlignLeft << -10. << 0.;
+    QTest::newRow("AlignLeft 0") << 0 << Qt::AlignLeft << -10. << -1.;
+    QTest::newRow("AlignLeft 99") << 99 << Qt::AlignLeft << 0. << -1.;
+    QTest::newRow("AlignLeft 99") << 99 << Qt::AlignLeft << -10. << -1.;
+
+    QTest::newRow("AlignRight 0") << 0 << Qt::AlignRight << 0. << 0.;
+    QTest::newRow("AlignRight 1") << 1 << Qt::AlignRight << 0. << 0.;
+    QTest::newRow("AlignRight 1") << 1 << Qt::AlignRight << 0. << 50.;
+    QTest::newRow("AlignRight 0") << 0 << Qt::AlignRight << 0. << -1.;
+
+    QTest::newRow("AlignRight 0") << 0 << Qt::AlignRight << 10. << 0.;
+    QTest::newRow("AlignRight 1") << 1 << Qt::AlignRight << 10. << 0.;
+    QTest::newRow("AlignRight 1") << 1 << Qt::AlignRight << 10. << 50.;
+    QTest::newRow("AlignRight 0") << 0 << Qt::AlignRight << 10. << -1.;
+    QTest::newRow("AlignRight 99") << 99 << Qt::AlignRight << 10. << -1.;
+
+    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << 0. << 0.;
+    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << 0. << 0.;
+    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << 0. << 50.;
+    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << 0. << -1.;
+    QTest::newRow("AlignCenter 99") << 99 << Qt::AlignCenter << 0. << -1.;
+
+    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << -10. << 0.;
+    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << -10. << 0.;
+    QTest::newRow("AlignCenter 1") << 1 << Qt::AlignCenter << -10. << 50.;
+    QTest::newRow("AlignCenter 0") << 0 << Qt::AlignCenter << -10. << -1.;
+    QTest::newRow("AlignCenter 99") << 99 << Qt::AlignCenter << -10. << -1.;
+}
+
+void tst_QQuickTableView::positionViewAtColumnClamped()
+{
+    // Check that positionViewAtColumn actually flicks the table to the
+    // right position so that the column becomes visible. For this test, we
+    // only test cells that cannot be placed exactly at the given alignment,
+    // because it would cause the table to overshoot. Instead the
+    // table should be flicked to the edge of the viewport, close to the
+    // requested alignment.
+    QFETCH(int, column);
+    QFETCH(Qt::AlignmentFlag, alignment);
+    QFETCH(qreal, offset);
+    QFETCH(qreal, contentXStartPos);
+
+    LOAD_TABLEVIEW("plaintableview.qml");
+    auto model = TestModelAsVariant(100, 100);
+    tableView->setModel(model);
+    if (contentXStartPos >= 0)
+        tableView->setContentX(contentXStartPos);
+
+    WAIT_UNTIL_POLISHED;
+
+    tableView->positionViewAtColumn(column, alignment, offset);
+
+    WAIT_UNTIL_POLISHED;
+
+    QCOMPARE(tableView->contentX(), column < 50 ? 0 : tableView->contentWidth() - tableView->width());
+}
+
 void tst_QQuickTableView::itemAtCell_data()
 {
     QTest::addColumn<QPoint>("cell");