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, 152 insertions, 41 deletions
diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp index 8f0c783c80..9a0a21cccd 100644 --- a/src/quick/items/qquicktableview.cpp +++ b/src/quick/items/qquicktableview.cpp @@ -782,6 +782,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) { @@ -1022,7 +1031,7 @@ void QQuickTableViewPrivate::updateExtents() } } -void QQuickTableViewPrivate::updateAverageEdgeSize() +void QQuickTableViewPrivate::updateAverageColumnWidth() { if (explicitContentWidth.isValid()) { const qreal accColumnSpacing = (tableSize.width() - 1) * cellSpacing.width(); @@ -1031,7 +1040,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()); @@ -1121,9 +1133,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); @@ -2116,20 +2134,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(); } @@ -2575,6 +2586,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); } @@ -2679,10 +2692,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. @@ -2774,7 +2792,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 ) @@ -2798,7 +2818,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) @@ -2806,7 +2826,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) @@ -2814,7 +2834,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) @@ -2822,7 +2847,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) @@ -2853,6 +2878,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()))) { @@ -2991,7 +3019,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(); } @@ -3009,7 +3038,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(); } @@ -3025,7 +3055,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(); } @@ -3041,7 +3072,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 876a175d87..6a8c5fa7fa 100644 --- a/src/quick/items/qquicktableview_p_p.h +++ b/src/quick/items/qquicktableview_p_p.h @@ -213,8 +213,10 @@ public: ViewportOnly = 0x4, CalculateNewTopLeftRow = 0x8, CalculateNewTopLeftColumn = 0x10, - PositionViewAtRow = 0x20, - PositionViewAtColumn = 0x40, + CalculateNewContentWidth = 0x20, + CalculateNewContentHeight = 0x40, + PositionViewAtRow = 0x80, + PositionViewAtColumn = 0x100, }; Q_DECLARE_FLAGS(RebuildOptions, RebuildOption) @@ -371,7 +373,8 @@ public: void updateContentWidth(); void updateContentHeight(); - void updateAverageEdgeSize(); + void updateAverageColumnWidth(); + void updateAverageRowHeight(); RebuildOptions checkForVisibilityChanges(); void forceLayout(); diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp index 6453180999..0375b74386 100644 --- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp +++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp @@ -188,6 +188,8 @@ private slots: void itemAtCell(); void leftRightTopBottomProperties_data(); void leftRightTopBottomProperties(); + void checkContentSize_data(); + void checkContentSize(); }; tst_QQuickTableView::tst_QQuickTableView() @@ -600,6 +602,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() @@ -1305,10 +1313,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); @@ -1319,12 +1327,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. @@ -3113,6 +3119,76 @@ void tst_QQuickTableView::leftRightTopBottomProperties() QCOMPARE(bottomSpy.count(), expectedSignalCount.bottom()); } +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" |