aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/quick/items/qquicktableview.cpp88
-rw-r--r--src/quick/items/qquicktableview_p_p.h9
-rw-r--r--tests/auto/quick/qquicktableview/tst_qquicktableview.cpp96
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"