TableView: switch to use TableView.cellWidth/cellHeight

This is logically more correct since TableView will override
width / height anyway to make the delegates fit into the table if
they are not as wide/tall as the widest column/row.
And it gets even more problematic when we recycle delegates, since
in that case we need to keep the original width binding of the
delegate to calulate the size of new columns. And this all
fits better by using attached properties instead.

Change-Id: Ia5f2acd2bfc45f3fb160c3782191ad8da9f780e6
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: J-P Nurmi <jpnurmi@qt.io>

7 files changed, 61 insertions, 28 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index b3802b7694..9080ccad18 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -495,8 +495,8 @@ void QQuickTableViewPrivate::calculateColumnWidthsAfterRebuilding()
     for (int column = loadedTable.left(); column <= loadedTable.right(); ++column) {
         qreal columnWidth = 0;
         for (int row = loadedTable.top(); row <= loadedTable.bottom(); ++row) {
-            auto const item = loadedTableItem(QPoint(column, row));
-            columnWidth = qMax(columnWidth, item->geometry().width());
+            if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, row))->item))
+                columnWidth = qMax(columnWidth, attached->cellWidth());
         }
 
         if (columnWidth <= 0)
@@ -522,8 +522,8 @@ void QQuickTableViewPrivate::calculateRowHeightsAfterRebuilding()
     for (int row = loadedTable.top(); row <= loadedTable.bottom(); ++row) {
         qreal rowHeight = 0;
         for (int column = loadedTable.left(); column <= loadedTable.right(); ++column) {
-            auto const item = loadedTableItem(QPoint(column, row));
-            rowHeight = qMax(rowHeight, item->geometry().height());
+            if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, row))->item))
+                rowHeight = qMax(rowHeight, attached->cellHeight());
         }
 
         if (rowHeight <= 0)
@@ -553,8 +553,8 @@ void QQuickTableViewPrivate::calculateColumnWidth(int column)
 
     qreal columnWidth = 0;
     for (int row = loadedTable.top(); row <= loadedTable.bottom(); ++row) {
-        auto const item = loadedTableItem(QPoint(column, row));
-        columnWidth = qMax(columnWidth, item->geometry().width());
+        if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, row))->item))
+            columnWidth = qMax(columnWidth, attached->cellWidth());
     }
 
     if (columnWidth <= 0)
@@ -576,8 +576,8 @@ void QQuickTableViewPrivate::calculateRowHeight(int row)
 
     qreal rowHeight = 0;
     for (int column = loadedTable.left(); column <= loadedTable.right(); ++column) {
-        auto const item = loadedTableItem(QPoint(column, row));
-        rowHeight = qMax(rowHeight, item->geometry().height());
+        if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, row))->item))
+            rowHeight = qMax(rowHeight, attached->cellHeight());
     }
 
     if (rowHeight <= 0)
@@ -661,8 +661,10 @@ qreal QQuickTableViewPrivate::columnWidth(int column)
     }
 
     // If we have an item loaded at column, return the width of the item.
-    if (column >= loadedTable.left() && column <= loadedTable.right())
-        return loadedTableItem(QPoint(column, 0))->geometry().width();
+    if (column >= loadedTable.left() && column <= loadedTable.right()) {
+        if (auto attached = getAttachedObject(loadedTableItem(QPoint(column, 0))->item))
+            return attached->cellWidth();
+    }
 
     return -1;
 }
@@ -695,8 +697,10 @@ qreal QQuickTableViewPrivate::rowHeight(int row)
     }
 
     // If we have an item loaded at row, return the height of the item.
-    if (row >= loadedTable.top() && row <= loadedTable.bottom())
-        return loadedTableItem(QPoint(0, row))->geometry().height();
+    if (row >= loadedTable.top() && row <= loadedTable.bottom()) {
+        if (auto attached = getAttachedObject(loadedTableItem(QPoint(0, row))->item))
+            return attached->cellHeight();
+    }
 
     return -1;
 }
@@ -839,16 +843,24 @@ void QQuickTableViewPrivate::layoutHorizontalEdge(Qt::Edge tableEdge)
 void QQuickTableViewPrivate::layoutTopLeftItem()
 {
     // ###todo: support starting with other top-left items than 0,0
-    Q_TABLEVIEW_ASSERT(loadRequest.firstCell() == QPoint(0, 0), loadRequest.toString());
-    auto topLeftItem = loadedTableItem(QPoint(0, 0));
+    const QPoint cell = loadRequest.firstCell();
+    Q_TABLEVIEW_ASSERT(cell == QPoint(0, 0), loadRequest.toString());
+    auto topLeftItem = loadedTableItem(cell);
     auto item = topLeftItem->item;
 
-    if (item->width() <= 0)
-        item->setWidth(kDefaultColumnWidth);
-    if (item->height() <= 0)
-        item->setHeight(kDefaultRowHeight);
+    qreal width = 0;
+    qreal height = 0;
+    if (auto attached = getAttachedObject(loadedTableItem(cell)->item)) {
+        width = attached->cellWidth();
+        height = attached->cellHeight();
+    }
+    if (width <= 0)
+        width = kDefaultColumnWidth;
+    if (height <= 0)
+        height = kDefaultRowHeight;
 
     item->setPosition(QPoint(tableMargins.left(), tableMargins.top()));
+    item->setSize(QSizeF(width, height));
     topLeftItem->setVisible(true);
     qCDebug(lcTableViewDelegateLifecycle) << "geometry:" << topLeftItem->geometry();
 }
diff --git a/src/quick/items/qquicktableview_p.h b/src/quick/items/qquicktableview_p.h
index 8cc5800d7e..0589d83f50 100644
--- a/src/quick/items/qquicktableview_p.h
+++ b/src/quick/items/qquicktableview_p.h
@@ -141,6 +141,8 @@ class Q_QUICK_PRIVATE_EXPORT QQuickTableViewAttached : public QObject
     Q_OBJECT
 
     Q_PROPERTY(QQuickTableView *tableView READ tableView NOTIFY tableViewChanged)
+    Q_PROPERTY(qreal cellWidth READ cellWidth WRITE setCellWidth NOTIFY cellWidthChanged)
+    Q_PROPERTY(qreal cellHeight READ cellHeight WRITE setCellHeight NOTIFY cellHeightChanged)
     Q_PROPERTY(int row READ row NOTIFY rowChanged)
     Q_PROPERTY(int column READ column NOTIFY columnChanged)
 
@@ -156,6 +158,22 @@ public:
         Q_EMIT tableViewChanged();
     }
 
+    qreal cellWidth() const { return m_cellSize.width(); }
+    void setCellWidth(qreal newWidth) {
+        if (newWidth == m_cellSize.width())
+            return;
+        m_cellSize.setWidth(newWidth);
+        Q_EMIT cellWidthChanged();
+    }
+
+    qreal cellHeight() const { return m_cellSize.height(); }
+    void setCellHeight(qreal newHeight) {
+        if (newHeight == m_cellSize.height())
+            return;
+        m_cellSize.setHeight(newHeight);
+        Q_EMIT cellHeightChanged();
+    }
+
     int row() const { return m_row; }
     void setRow(int newRow) {
         if (newRow == m_row)
@@ -174,6 +192,8 @@ public:
 
 Q_SIGNALS:
     void tableViewChanged();
+    void cellWidthChanged();
+    void cellHeightChanged();
     void rowChanged();
     void columnChanged();
 
@@ -181,6 +201,7 @@ private:
     QPointer<QQuickTableView> m_tableview;
     int m_row = -1;
     int m_column = -1;
+    QSizeF m_cellSize;
 };
 
 QT_END_NAMESPACE
diff --git a/tests/auto/quick/qquicktableview/data/countingtableview.qml b/tests/auto/quick/qquicktableview/data/countingtableview.qml
index 1e8e9f43fb..d8315be54f 100644
--- a/tests/auto/quick/qquicktableview/data/countingtableview.qml
+++ b/tests/auto/quick/qquicktableview/data/countingtableview.qml
@@ -64,8 +64,8 @@ Item {
         id: tableViewDelegate
         Rectangle {
             objectName: "tableViewDelegate"
-            implicitWidth: 100
-            implicitHeight: 50
+            TableView.cellWidth: 100
+            TableView.cellHeight: 50
             color: "lightgray"
             border.width: 1
             Text {
diff --git a/tests/auto/quick/qquicktableview/data/plaintableview.qml b/tests/auto/quick/qquicktableview/data/plaintableview.qml
index a073941faa..7668f0ca01 100644
--- a/tests/auto/quick/qquicktableview/data/plaintableview.qml
+++ b/tests/auto/quick/qquicktableview/data/plaintableview.qml
@@ -66,8 +66,8 @@ Item {
         id: tableViewDelegate
         Rectangle {
             objectName: "tableViewDelegate"
-            implicitWidth: delegateWidth
-            implicitHeight: delegateHeight
+            TableView.cellWidth: delegateWidth
+            TableView.cellHeight: delegateHeight
             color: "lightgray"
             border.width: 1
             Text {
diff --git a/tests/manual/tableview/abstracttablemodel/main.qml b/tests/manual/tableview/abstracttablemodel/main.qml
index bff851a9f2..c448815a7c 100644
--- a/tests/manual/tableview/abstracttablemodel/main.qml
+++ b/tests/manual/tableview/abstracttablemodel/main.qml
@@ -76,8 +76,8 @@ Window {
         Component {
             id: tableViewDelegate
             Rectangle {
-                width: TableView.column % 3 ? 80 : 50
-                height: TableView.row % 3 ? 80 : 50
+                TableView.cellWidth: TableView.column % 3 ? 80 : 50
+                TableView.cellHeight: TableView.row % 3 ? 80 : 50
 
                 Text {
                     anchors.centerIn: parent
diff --git a/tests/manual/tableview/listmodel/main.qml b/tests/manual/tableview/listmodel/main.qml
index f4def85455..b5a1ab8379 100644
--- a/tests/manual/tableview/listmodel/main.qml
+++ b/tests/manual/tableview/listmodel/main.qml
@@ -72,8 +72,8 @@ Window {
             delegate: Component {
                 Rectangle {
                     id: tableDelegate
-                    width: 100
-                    height: 50
+                    TableView.cellWidth: 100
+                    TableView.cellHeight: 50
 
                     Text {
                         anchors.centerIn: parent
diff --git a/tests/manual/tableview/storagemodel/main.qml b/tests/manual/tableview/storagemodel/main.qml
index fbeb5f6115..f76da374e3 100644
--- a/tests/manual/tableview/storagemodel/main.qml
+++ b/tests/manual/tableview/storagemodel/main.qml
@@ -60,8 +60,8 @@ Window {
         rowSpacing: 1
         delegate: Rectangle {
             id: tableDelegate
-            width: displayText.implicitWidth + 8
-            height: displayText.implicitHeight + 14
+            TableView.cellWidth: displayText.implicitWidth + 8
+            TableView.cellHeight: displayText.implicitHeight + 14
 
             Text {
                 id: displayText