diff options
-rw-r--r-- | src/quick/items/qquicktableview.cpp | 88 | ||||
-rw-r--r-- | src/quick/items/qquicktableview_p_p.h | 9 | ||||
-rw-r--r-- | tests/auto/quick/qquicktableview/tst_qquicktableview.cpp | 96 |
3 files changed, 153 insertions, 40 deletions
diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp index 89a1e1ce2a..3f5912e344 100644 --- a/src/quick/items/qquicktableview.cpp +++ b/src/quick/items/qquicktableview.cpp @@ -644,6 +644,15 @@ int QQuickTableViewPrivate::nextVisibleEdgeIndex(Qt::Edge edge, int startIndex) void QQuickTableViewPrivate::updateContentWidth() { + // Note that we actually never really know what the content size / size of the full table will + // 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. So the calculated content size should always + // be understood as a guesstimate, which sometimes can be really off (as a tradeoff for performance). + // When this is not acceptable, the user can always set a custom content size explicitly. Q_Q(QQuickTableView); if (syncHorizontally) { @@ -884,7 +893,7 @@ void QQuickTableViewPrivate::updateExtents() } } -void QQuickTableViewPrivate::updateAverageEdgeSize() +void QQuickTableViewPrivate::updateAverageColumnWidth() { if (explicitContentWidth.isValid()) { const qreal accColumnSpacing = (tableSize.width() - 1) * cellSpacing.width(); @@ -893,7 +902,10 @@ void QQuickTableViewPrivate::updateAverageEdgeSize() const qreal accColumnSpacing = (loadedColumns.count() - 1) * cellSpacing.width(); averageEdgeSize.setWidth((loadedTableOuterRect.width() - accColumnSpacing) / loadedColumns.count()); } +} +void QQuickTableViewPrivate::updateAverageRowHeight() +{ if (explicitContentHeight.isValid()) { const qreal accRowSpacing = (tableSize.height() - 1) * cellSpacing.height(); averageEdgeSize.setHeight((explicitContentHeight - accRowSpacing) / tableSize.height()); @@ -971,9 +983,15 @@ void QQuickTableViewPrivate::forceLayout() // the model is updated, but before we're notified about it. rebuildOptions = RebuildOption::All; } else { - rebuildOptions = checkForVisibilityChanges(); - if (!rebuildOptions) - rebuildOptions = RebuildOption::LayoutOnly; + // Resizing a column (or row) can result in the table going from being + // e.g completely inside the viewport to go outside. And in the latter + // case, the user needs to be able to scroll the viewport, also if + // flags such as Flickable.StopAtBounds is in use. So we need to + // update contentWidth/Height to support that case. + rebuildOptions = RebuildOption::LayoutOnly + | RebuildOption::CalculateNewContentWidth + | RebuildOption::CalculateNewContentHeight + | checkForVisibilityChanges(); } scheduleRebuildTable(rebuildOptions); @@ -1906,20 +1924,13 @@ void QQuickTableViewPrivate::layoutAfterLoadingInitialTable() relayoutTableItems(); syncLoadedTableRectFromLoadedTable(); - 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(); + if (rebuildOptions.testFlag(RebuildOption::CalculateNewContentWidth)) { + updateAverageColumnWidth(); updateContentWidth(); + } + + if (rebuildOptions.testFlag(RebuildOption::CalculateNewContentHeight)) { + updateAverageRowHeight(); updateContentHeight(); } @@ -2267,6 +2278,8 @@ void QQuickTableViewPrivate::syncRebuildOptions() if (rebuildOptions.testFlag(RebuildOption::All)) { rebuildOptions.setFlag(RebuildOption::ViewportOnly, false); rebuildOptions.setFlag(RebuildOption::LayoutOnly, false); + rebuildOptions.setFlag(RebuildOption::CalculateNewContentWidth); + rebuildOptions.setFlag(RebuildOption::CalculateNewContentHeight); } else if (rebuildOptions.testFlag(RebuildOption::ViewportOnly)) { rebuildOptions.setFlag(RebuildOption::LayoutOnly, false); } @@ -2365,10 +2378,15 @@ void QQuickTableViewPrivate::syncSyncView() syncHorizontally = syncView && assignedSyncDirection & Qt::Horizontal; syncVertically = syncView && assignedSyncDirection & Qt::Vertical; - if (syncHorizontally) + if (syncHorizontally) { q->setColumnSpacing(syncView->columnSpacing()); - if (syncVertically) + updateContentWidth(); + } + + if (syncVertically) { q->setRowSpacing(syncView->rowSpacing()); + updateContentHeight(); + } if (syncView && loadedItems.isEmpty() && !tableSize.isEmpty()) { // When we have a syncView, we can sometimes temporarily end up with no loaded items. @@ -2450,7 +2468,9 @@ void QQuickTableViewPrivate::modelUpdated(const QQmlChangeSet &changeSet, bool r Q_UNUSED(reset); Q_TABLEVIEW_ASSERT(!model->abstractItemModel(), ""); - scheduleRebuildTable(RebuildOption::ViewportOnly); + scheduleRebuildTable(RebuildOption::ViewportOnly + | RebuildOption::CalculateNewContentWidth + | RebuildOption::CalculateNewContentHeight); } void QQuickTableViewPrivate::rowsMovedCallback(const QModelIndex &parent, int, int, const QModelIndex &, int ) @@ -2474,7 +2494,7 @@ void QQuickTableViewPrivate::rowsInsertedCallback(const QModelIndex &parent, int if (parent != QModelIndex()) return; - scheduleRebuildTable(RebuildOption::ViewportOnly); + scheduleRebuildTable(RebuildOption::ViewportOnly | RebuildOption::CalculateNewContentHeight); } void QQuickTableViewPrivate::rowsRemovedCallback(const QModelIndex &parent, int, int) @@ -2482,7 +2502,7 @@ void QQuickTableViewPrivate::rowsRemovedCallback(const QModelIndex &parent, int, if (parent != QModelIndex()) return; - scheduleRebuildTable(RebuildOption::ViewportOnly); + scheduleRebuildTable(RebuildOption::ViewportOnly | RebuildOption::CalculateNewContentHeight); } void QQuickTableViewPrivate::columnsInsertedCallback(const QModelIndex &parent, int, int) @@ -2490,7 +2510,12 @@ void QQuickTableViewPrivate::columnsInsertedCallback(const QModelIndex &parent, if (parent != QModelIndex()) return; - scheduleRebuildTable(RebuildOption::ViewportOnly); + // Adding a column (or row) can result in the table going from being + // e.g completely inside the viewport to go outside. And in the latter + // case, the user needs to be able to scroll the viewport, also if + // flags such as Flickable.StopAtBounds is in use. So we need to + // update contentWidth to support that case. + scheduleRebuildTable(RebuildOption::ViewportOnly | RebuildOption::CalculateNewContentWidth); } void QQuickTableViewPrivate::columnsRemovedCallback(const QModelIndex &parent, int, int) @@ -2498,7 +2523,7 @@ void QQuickTableViewPrivate::columnsRemovedCallback(const QModelIndex &parent, i if (parent != QModelIndex()) return; - scheduleRebuildTable(RebuildOption::ViewportOnly); + scheduleRebuildTable(RebuildOption::ViewportOnly | RebuildOption::CalculateNewContentWidth); } void QQuickTableViewPrivate::layoutChangedCallback(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint) @@ -2529,6 +2554,9 @@ void QQuickTableViewPrivate::scheduleRebuildIfFastFlick() // 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. + // Note that we don't want to update the content size in this case, since first of all, the + // content size should logically not change as a result of flicking. But more importantly, updating + // the content size in combination with fast-flicking has a tendency to cause flicker in the viewport. // Check the viewport moved more than one page vertically if (!viewportRect.intersects(QRectF(viewportRect.x(), q->contentY(), 1, q->height()))) { @@ -2659,7 +2687,8 @@ void QQuickTableView::setRowSpacing(qreal spacing) return; d->cellSpacing.setHeight(spacing); - d->scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::LayoutOnly); + d->scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::LayoutOnly + | QQuickTableViewPrivate::RebuildOption::CalculateNewContentHeight); emit rowSpacingChanged(); } @@ -2677,7 +2706,8 @@ void QQuickTableView::setColumnSpacing(qreal spacing) return; d->cellSpacing.setWidth(spacing); - d->scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::LayoutOnly); + d->scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::LayoutOnly + | QQuickTableViewPrivate::RebuildOption::CalculateNewContentWidth); emit columnSpacingChanged(); } @@ -2693,7 +2723,8 @@ void QQuickTableView::setRowHeightProvider(const QJSValue &provider) return; d->rowHeightProvider = provider; - d->scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::ViewportOnly); + d->scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::ViewportOnly + | QQuickTableViewPrivate::RebuildOption::CalculateNewContentHeight); emit rowHeightProviderChanged(); } @@ -2709,7 +2740,8 @@ void QQuickTableView::setColumnWidthProvider(const QJSValue &provider) return; d->columnWidthProvider = provider; - d->scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::ViewportOnly); + d->scheduleRebuildTable(QQuickTableViewPrivate::RebuildOption::ViewportOnly + | QQuickTableViewPrivate::RebuildOption::CalculateNewContentWidth); emit columnWidthProviderChanged(); } diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h index fe4cf82b46..6c5abcf414 100644 --- a/src/quick/items/qquicktableview_p_p.h +++ b/src/quick/items/qquicktableview_p_p.h @@ -211,7 +211,9 @@ public: ViewportOnly = 0x2, CalculateNewTopLeftRow = 0x4, CalculateNewTopLeftColumn = 0x8, - All = 0x10, + CalculateNewContentWidth = 0x10, + CalculateNewContentHeight = 0x20, + All = 0x40, }; Q_DECLARE_FLAGS(RebuildOptions, RebuildOption) @@ -355,7 +357,8 @@ public: void updateContentWidth(); void updateContentHeight(); - void updateAverageEdgeSize(); + void updateAverageColumnWidth(); + void updateAverageRowHeight(); RebuildOptions checkForVisibilityChanges(); void forceLayout(); @@ -458,6 +461,8 @@ public: QPoint cell; }; +Q_DECLARE_OPERATORS_FOR_FLAGS(QQuickTableViewPrivate::RebuildOptions) + QT_END_NAMESPACE #endif diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp index b876ead301..6339e5a59d 100644 --- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp +++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp @@ -178,6 +178,8 @@ private slots: void delegateWithRequiredProperties(); void checkThatFetchMoreIsCalledWhenScrolledToTheEndOfTable(); void replaceModel(); + void checkContentSize_data(); + void checkContentSize(); }; tst_QQuickTableView::tst_QQuickTableView() @@ -590,6 +592,12 @@ void tst_QQuickTableView::checkForceLayoutEndUpDoingALayout() for (auto fxItem : tableViewPrivate->loadedItems) QCOMPARE(fxItem->item->height(), newDelegateSize); + + // Check that the content height has been updated as well + const qreal rowSpacing = tableView->rowSpacing(); + const qreal colSpacing = tableView->columnSpacing(); + QCOMPARE(tableView->contentWidth(), (10 * (newDelegateSize + colSpacing)) - colSpacing); + QCOMPARE(tableView->contentHeight(), (9 * (newDelegateSize + rowSpacing)) - rowSpacing); } void tst_QQuickTableView::checkForceLayoutDuringModelChange() @@ -1295,10 +1303,10 @@ void tst_QQuickTableView::checkSpacingValues() tableView->polish(); WAIT_UNTIL_POLISHED; - 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); + qreal expectedContentWidth = columnCount * (delegateWidth + tableView->columnSpacing()) - tableView->columnSpacing(); + qreal expectedContentHeight = rowCount * (delegateHeight + tableView->rowSpacing()) - tableView->rowSpacing(); + QCOMPARE(tableView->contentWidth(), expectedContentWidth); + QCOMPARE(tableView->contentHeight(), expectedContentHeight); // Valid spacing assignment tableView->setRowSpacing(42); @@ -1309,12 +1317,10 @@ void tst_QQuickTableView::checkSpacingValues() tableView->polish(); WAIT_UNTIL_POLISHED; - // 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); + expectedContentWidth = columnCount * (delegateWidth + tableView->columnSpacing()) - tableView->columnSpacing(); + expectedContentHeight = rowCount * (delegateHeight + tableView->rowSpacing()) - tableView->rowSpacing(); + QCOMPARE(tableView->contentWidth(), expectedContentWidth); + QCOMPARE(tableView->contentHeight(), expectedContentHeight); // Negative spacing is allowed, and can be used to eliminate double edges // in the grid if the delegate is a rectangle with a border. @@ -2775,6 +2781,76 @@ void tst_QQuickTableView::replaceModel() QCOMPARE(tableView->contentHeight(), 0); } +void tst_QQuickTableView::checkContentSize_data() +{ + QTest::addColumn<int>("rowCount"); + QTest::addColumn<int>("colCount"); + + QTest::newRow("4x4") << 4 << 4; + QTest::newRow("100x100") << 100 << 100; + QTest::newRow("0x0") << 0 << 0; +} + +void tst_QQuickTableView::checkContentSize() +{ + QFETCH(int, rowCount); + QFETCH(int, colCount); + + // Check that the content size is initially correct, and that + // it updates when we change e.g the model or spacing (QTBUG-87680) + LOAD_TABLEVIEW("plaintableview.qml"); + + TestModel model(rowCount, colCount); + tableView->setModel(QVariant::fromValue(&model)); + tableView->setRowSpacing(1); + tableView->setColumnSpacing(2); + + WAIT_UNTIL_POLISHED; + + const qreal delegateWidth = 100; + const qreal delegateHeight = 50; + qreal colSpacing = tableView->columnSpacing(); + qreal rowSpacing = tableView->rowSpacing(); + + // Check that content size has the exepected initial values + QCOMPARE(tableView->contentWidth(), colCount == 0 ? 0 : (colCount * (delegateWidth + colSpacing)) - colSpacing); + QCOMPARE(tableView->contentHeight(), rowCount == 0 ? 0 : (rowCount * (delegateHeight + rowSpacing)) - rowSpacing); + + // Set no spacing + rowSpacing = 0; + colSpacing = 0; + tableView->setRowSpacing(rowSpacing); + tableView->setColumnSpacing(colSpacing); + WAIT_UNTIL_POLISHED; + QCOMPARE(tableView->contentWidth(), colCount * delegateWidth); + QCOMPARE(tableView->contentHeight(), rowCount * delegateHeight); + + // Set typical spacing values + rowSpacing = 2; + colSpacing = 3; + tableView->setRowSpacing(rowSpacing); + tableView->setColumnSpacing(colSpacing); + WAIT_UNTIL_POLISHED; + QCOMPARE(tableView->contentWidth(), colCount == 0 ? 0 : (colCount * (delegateWidth + colSpacing)) - colSpacing); + QCOMPARE(tableView->contentHeight(), rowCount == 0 ? 0 : (rowCount * (delegateHeight + rowSpacing)) - rowSpacing); + + // Add a row and a column + model.insertRow(0); + model.insertColumn(0); + rowCount = model.rowCount(); + colCount = model.columnCount(); + WAIT_UNTIL_POLISHED; + QCOMPARE(tableView->contentWidth(), (colCount * (delegateWidth + colSpacing)) - colSpacing); + QCOMPARE(tableView->contentHeight(), (rowCount * (delegateHeight + rowSpacing)) - rowSpacing); + + // Remove a row (this should also make affect contentWidth if rowCount becomes 0) + model.removeRow(0); + rowCount = model.rowCount(); + WAIT_UNTIL_POLISHED; + QCOMPARE(tableView->contentWidth(), rowCount == 0 ? 0 : (colCount * (delegateWidth + colSpacing)) - colSpacing); + QCOMPARE(tableView->contentHeight(), rowCount == 0 ? 0 : (rowCount * (delegateHeight + rowSpacing)) - rowSpacing); +} + QTEST_MAIN(tst_QQuickTableView) #include "tst_qquicktableview.moc" |