QQuickTableView: don't rebuild table upon querying content size

Overriding contentWidth/Height was done to be able to force build the
table early if the app needed to know the size of the table already at
Component.onCompleted (to e.g center the viewport on the center of
the table). But now that we have a forceLayout() function, it's better
to require that that function should be called before querying
contentWidth/Height at this stage.

By not building the table on the fly, we allow the application to
bind expressions directly to contentWidth/Height, without being concerned
about potential binding loops that can occur as a result of us
rebuilding the whole table behind his back. The benefit of this overshadows
the need to call forceLayout() explicit for some corner cases.

Note that we still redefine the contentWidth/Height properties in TableView
so that we can catch if the application sets an explicit contentWidth/Height
(which is tested by checkExplicitContentWidthAndHeight()).

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

4 files changed, 23 insertions, 57 deletions

diff --git a/examples/quick/tableview/gameoflife/main.qml b/examples/quick/tableview/gameoflife/main.qml
index 7ac26c477c..bf8c1151fe 100644
--- a/examples/quick/tableview/gameoflife/main.qml
+++ b/examples/quick/tableview/gameoflife/main.qml
@@ -98,6 +98,7 @@ ApplicationWindow {
 
         //! [scroll]
         Component.onCompleted: {
+            tableView.forceLayout()
             tableView.contentX = (tableView.contentWidth - tableView.width) / 2;
             tableView.contentY = (tableView.contentHeight - tableView.height) / 2;
         }
diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 4b2c26e3e6..1cc42c2a7a 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -502,15 +502,15 @@ void QQuickTableViewPrivate::updateContentWidth()
         if (currentRightColumn >= tableSize.width() - 1) {
             // We are at the last column, and can set the exact width
             if (currentWidth != q->implicitWidth())
-                q->setContentWidth(currentWidth);
+                q->QQuickFlickable::setContentWidth(currentWidth);
         } else if (currentWidth >= q->implicitWidth()) {
             // We are at the estimated width, but there are still more columns
-            q->setContentWidth(estimatedWidth);
+            q->QQuickFlickable::setContentWidth(estimatedWidth);
         } else {
             // Only set a new width if the new estimate is substantially different
             qreal diff = 1 - (estimatedWidth / q->implicitWidth());
             if (qAbs(diff) > thresholdBeforeAdjust)
-                q->setContentWidth(estimatedWidth);
+                q->QQuickFlickable::setContentWidth(estimatedWidth);
         }
     }
 }
@@ -539,15 +539,15 @@ void QQuickTableViewPrivate::updateContentHeight()
         if (currentBottomRow >= tableSize.height() - 1) {
             // We are at the last row, and can set the exact height
             if (currentHeight != q->implicitHeight())
-                q->setContentHeight(currentHeight);
+                q->QQuickFlickable::setContentHeight(currentHeight);
         } else if (currentHeight >= q->implicitHeight()) {
             // We are at the estimated height, but there are still more rows
-            q->setContentHeight(estimatedHeight);
+            q->QQuickFlickable::setContentHeight(estimatedHeight);
         } else {
             // Only set a new height if the new estimate is substantially different
             qreal diff = 1 - (estimatedHeight / q->implicitHeight());
             if (qAbs(diff) > thresholdBeforeAdjust)
-                q->setContentHeight(estimatedHeight);
+                q->QQuickFlickable::setContentHeight(estimatedHeight);
         }
     }
 }
@@ -1687,8 +1687,6 @@ QQuickTableView::QQuickTableView(QQuickItem *parent)
     : QQuickFlickable(*(new QQuickTableViewPrivate), parent)
 {
     setFlag(QQuickItem::ItemIsFocusScope);
-    connect(this, &QQuickFlickable::contentWidthChanged, this, &QQuickTableView::contentWidthOverrideChanged);
-    connect(this, &QQuickFlickable::contentHeightChanged, this, &QQuickTableView::contentHeightOverrideChanged);
 }
 
 int QQuickTableView::rows() const
@@ -1851,52 +1849,18 @@ void QQuickTableView::setReuseItems(bool reuse)
     emit reuseItemsChanged();
 }
 
-qreal QQuickTableView::explicitContentWidth() const
-{
-    Q_D(const QQuickTableView);
-
-    if (d->rebuildScheduled && d->explicitContentWidth.isNull) {
-        // The table is pending to be rebuilt. Since we don't
-        // know the contentWidth before this is done, we do the
-        // rebuild now, instead of waiting for the polish event.
-       d->updatePolishIfPossible();
-    }
-
-    return contentWidth();
-}
-
-void QQuickTableView::setExplicitContentWidth(qreal width)
+void QQuickTableView::setContentWidth(qreal width)
 {
     Q_D(QQuickTableView);
     d->explicitContentWidth = width;
-    if (width == contentWidth())
-        return;
-
-    setContentWidth(width);
+    QQuickFlickable::setContentWidth(width);
 }
 
-qreal QQuickTableView::explicitContentHeight() const
-{
-    Q_D(const QQuickTableView);
-
-    if (d->rebuildScheduled && d->explicitContentHeight.isNull) {
-        // The table is pending to be rebuilt. Since we don't
-        // know the contentHeight before this is done, we do the
-        // rebuild now, instead of waiting for the polish event.
-       d->updatePolishIfPossible();
-    }
-
-    return contentHeight();
-}
-
-void QQuickTableView::setExplicitContentHeight(qreal height)
+void QQuickTableView::setContentHeight(qreal height)
 {
     Q_D(QQuickTableView);
     d->explicitContentHeight = height;
-    if (height == contentHeight())
-        return;
-
-    setContentHeight(height);
+    QQuickFlickable::setContentHeight(height);
 }
 
 void QQuickTableView::forceLayout()
diff --git a/src/quick/items/qquicktableview_p.h b/src/quick/items/qquicktableview_p.h
index 0a2eff8fc1..6ba91d16a1 100644
--- a/src/quick/items/qquicktableview_p.h
+++ b/src/quick/items/qquicktableview_p.h
@@ -77,8 +77,8 @@ class Q_QUICK_PRIVATE_EXPORT QQuickTableView : public QQuickFlickable
     Q_PROPERTY(QVariant model READ model WRITE setModel NOTIFY modelChanged)
     Q_PROPERTY(QQmlComponent *delegate READ delegate WRITE setDelegate NOTIFY delegateChanged)
     Q_PROPERTY(bool reuseItems READ reuseItems WRITE setReuseItems NOTIFY reuseItemsChanged)
-    Q_PROPERTY(qreal contentWidth READ explicitContentWidth WRITE setExplicitContentWidth NOTIFY contentWidthOverrideChanged)
-    Q_PROPERTY(qreal contentHeight READ explicitContentHeight WRITE setExplicitContentWidth NOTIFY contentHeightOverrideChanged)
+    Q_PROPERTY(qreal contentWidth READ contentWidth WRITE setContentWidth NOTIFY contentWidthChanged)
+    Q_PROPERTY(qreal contentHeight READ contentHeight WRITE setContentWidth NOTIFY contentHeightChanged)
 
 public:
     QQuickTableView(QQuickItem *parent = nullptr);
@@ -107,10 +107,8 @@ public:
     bool reuseItems() const;
     void setReuseItems(bool reuseItems);
 
-    qreal explicitContentWidth() const;
-    void setExplicitContentWidth(qreal width);
-    qreal explicitContentHeight() const;
-    void setExplicitContentHeight(qreal height);
+    void setContentWidth(qreal width);
+    void setContentHeight(qreal height);
 
     Q_INVOKABLE void forceLayout();
 
@@ -126,8 +124,6 @@ Q_SIGNALS:
     void modelChanged();
     void delegateChanged();
     void reuseItemsChanged();
-    void contentWidthOverrideChanged();
-    void contentHeightOverrideChanged();
 
 protected:
     void geometryChanged(const QRectF &newGeometry, const QRectF &oldGeometry) override;
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index c7fd51f72c..dfcce6afdb 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -538,13 +538,14 @@ void tst_QQuickTableView::checkContentWidthAndHeight()
     tableView->setContentY(flickTo);
 
     const int largeSizeCellCountInView = qCeil(tableView->width() / cellSizeLarge);
-    const int bottomRow = smallCellCount + largeSizeCellCountInView - 1;
-    QCOMPARE(tableViewPrivate->loadedTable.right(), bottomRow);
+    const int columnCount = smallCellCount + largeSizeCellCountInView;
+    QCOMPARE(tableViewPrivate->loadedTable.right(), columnCount - 1);
 
     const qreal firstHalfLength = smallCellCount * cellSizeSmall;
     const qreal secondHalfOneScreenLength = largeSizeCellCountInView * cellSizeLarge;
     const qreal lengthAfterFlick = firstHalfLength + secondHalfOneScreenLength;
-    const qreal averageCellSize = lengthAfterFlick / (smallCellCount + largeSizeCellCountInView);
+
+    const qreal averageCellSize = lengthAfterFlick / columnCount;
     const qreal expectedSizeHalf = (tableSize * averageCellSize) + accumulatedSpacing;
 
     QCOMPARE(tableView->contentWidth(), expectedSizeHalf);
@@ -580,6 +581,10 @@ void tst_QQuickTableView::checkExplicitContentWidthAndHeight()
     QCOMPARE(tableView->contentWidth(), 1000);
     QCOMPARE(tableView->contentHeight(), 1000);
 
+    auto model = TestModelAsVariant(100, 100);
+    tableView->setModel(model);
+    WAIT_UNTIL_POLISHED;
+
     // Flick somewhere. It should not affect the contentWidth/Height
     tableView->setContentX(500);
     tableView->setContentY(500);