diff options
-rw-r--r-- | src/quick/items/qquicktableview.cpp | 45 | ||||
-rw-r--r-- | tests/auto/quick/qquicktableview/tst_qquicktableview.cpp | 89 |
2 files changed, 36 insertions, 98 deletions
diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp index 2b49200eaa..9583ef4231 100644 --- a/src/quick/items/qquicktableview.cpp +++ b/src/quick/items/qquicktableview.cpp @@ -305,9 +305,7 @@ \l view, which means that the table's width could be larger or smaller than the viewport width. As a TableView cannot always know the exact width of the table without loading all columns in the model, the \c contentWidth is - usually an estimate based on the columns it has seen so far. This estimate - is recalculated whenever new columns are flicked into view, which means - that the content width can change dynamically. + usually an estimate based on the initially loaded table. If you know what the width of the table will be, assign a value to \c contentWidth, to avoid unnecessary calculations and updates to the @@ -324,9 +322,7 @@ \c view, which means that the table's height could be larger or smaller than the viewport height. As a TableView cannot always know the exact height of the table without loading all rows in the model, the \c contentHeight is - usually an estimate based on the rows it has seen so far. This estimate is - recalculated whenever new rows are flicked into view, which means that - the content height can change dynamically. + usually an estimate based on the initially loaded table. If you know what the height of the table will be, assign a value to \c contentHeight, to avoid unnecessary calculations and updates to @@ -1539,19 +1535,6 @@ void QQuickTableViewPrivate::processLoadRequest() if (rebuildState == RebuildState::Done) { // Loading of this edge was not done as a part of a rebuild, but // instead as an incremental build after e.g a flick. - switch (loadRequest.edge()) { - case Qt::LeftEdge: - case Qt::TopEdge: - break; - case Qt::RightEdge: - updateAverageEdgeSize(); - updateContentWidth(); - break; - case Qt::BottomEdge: - updateAverageEdgeSize(); - updateContentHeight(); - break; - } updateExtents(); drainReusePoolAfterLoadRequest(); } @@ -1838,9 +1821,23 @@ void QQuickTableViewPrivate::layoutAfterLoadingInitialTable() syncLoadedTableRectFromLoadedTable(); } - updateAverageEdgeSize(); - updateContentWidth(); - updateContentHeight(); + 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(); + updateContentWidth(); + updateContentHeight(); + } + updateExtents(); } @@ -1856,8 +1853,6 @@ void QQuickTableViewPrivate::unloadEdge(Qt::Edge edge) unloadItem(QPoint(column, r.key())); loadedColumns.remove(column); syncLoadedTableRectFromLoadedTable(); - updateAverageEdgeSize(); - updateContentWidth(); break; } case Qt::TopEdge: case Qt::BottomEdge: { @@ -1866,8 +1861,6 @@ void QQuickTableViewPrivate::unloadEdge(Qt::Edge edge) unloadItem(QPoint(c.key(), row)); loadedRows.remove(row); syncLoadedTableRectFromLoadedTable(); - updateAverageEdgeSize(); - updateContentHeight(); break; } } diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp index d03f08ec6a..fbe56abda5 100644 --- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp +++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp @@ -563,7 +563,7 @@ void tst_QQuickTableView::checkForceLayoutFunction() void tst_QQuickTableView::checkContentWidthAndHeight() { - // Check that contentWidth/Height reports the correct size of the the + // Check that contentWidth/Height reports the correct size of the // table, based on knowledge of the rows and columns that has been loaded. LOAD_TABLEVIEW("contentwidthheight.qml"); @@ -574,11 +574,7 @@ void tst_QQuickTableView::checkContentWidthAndHeight() const int tableSize = 100; const int cellSizeSmall = 100; - const int cellSizeLarge = 200; const int spacing = 1; - const int smallCellCount = 20; - const int largeCellCount = tableSize - smallCellCount; - const qreal accumulatedSpacing = ((tableSize - 1) * spacing); auto model = TestModelAsVariant(tableSize, tableSize); tableView->setModel(model); @@ -591,71 +587,12 @@ void tst_QQuickTableView::checkContentWidthAndHeight() QCOMPARE(tableViewPrivate->averageEdgeSize.width(), cellSizeSmall); QCOMPARE(tableViewPrivate->averageEdgeSize.height(), cellSizeSmall); - // Flick in 5 more rows and columns, but not so far that we start loading in - // the ones that are bigger. Loading in more rows and columns of the same - // size as the initial ones should not change the first prediction. - qreal flickTo = ((cellSizeSmall + spacing) * 5); - tableView->setContentX(flickTo); - tableView->setContentY(flickTo); + // Flick to the end, and check that content width/height stays unchanged + tableView->setContentX(tableView->contentWidth() - tableView->width()); + tableView->setContentY(tableView->contentHeight() - tableView->height()); QCOMPARE(tableView->contentWidth(), expectedSizeInit); QCOMPARE(tableView->contentHeight(), expectedSizeInit); - QCOMPARE(tableViewPrivate->averageEdgeSize.width(), cellSizeSmall); - QCOMPARE(tableViewPrivate->averageEdgeSize.height(), cellSizeSmall); - - // Flick to row and column 20 (smallCellCount), since there the row and - // column sizes increases with 100. Check that TableView then adjusts - // contentWidth and contentHeight accordingly. - flickTo = ((cellSizeSmall + spacing) * smallCellCount) - spacing; - tableView->setContentX(flickTo); - tableView->setContentY(flickTo); - - // Since we move the viewport more than a page, tableview - // will jump to the new position and do a rebuild. - QVERIFY(tableViewPrivate->scheduledRebuildOptions); - WAIT_UNTIL_POLISHED; - - // Check that the average cell size is now matching the - // large cells since they fill up the whole view. - QCOMPARE(tableViewPrivate->averageEdgeSize.width(), cellSizeLarge); - QCOMPARE(tableViewPrivate->averageEdgeSize.height(), cellSizeLarge); - - const int largeSizeCellCountInView = qCeil(tableView->width() / cellSizeLarge); - const int columnCount = smallCellCount + largeSizeCellCountInView; - QCOMPARE(tableViewPrivate->leftColumn(), smallCellCount); - QCOMPARE(tableViewPrivate->rightColumn(), columnCount - 1); - - const qreal firstHalfLength = smallCellCount * cellSizeSmall; - const qreal secondHalfOneScreenLength = largeSizeCellCountInView * cellSizeLarge; - const qreal lengthAfterFlick = firstHalfLength + secondHalfOneScreenLength; - - // Check that loadedTableOuterRect has been calculated correct thus far - const qreal spacingAfterFlick = (smallCellCount + largeSizeCellCountInView - 1) * spacing; - QCOMPARE(tableViewPrivate->loadedTableOuterRect.left(), flickTo + spacing); - QCOMPARE(tableViewPrivate->loadedTableOuterRect.right(), lengthAfterFlick + spacingAfterFlick); - QCOMPARE(tableViewPrivate->loadedTableOuterRect.top(), flickTo + spacing); - QCOMPARE(tableViewPrivate->loadedTableOuterRect.bottom(), lengthAfterFlick + spacingAfterFlick); - - // At this point, we should have the exact content width/height set, because - // TableView knows where the large cells start in the viewport, and how many - // columns that remain in the model. It will assume that the rest of the the - // columns have the same average size as the ones currently inside the viewport. - const qreal expectedContentSize = (smallCellCount * cellSizeSmall) + (largeCellCount * cellSizeLarge) + accumulatedSpacing; - QCOMPARE(tableView->contentWidth(), expectedContentSize); - QCOMPARE(tableView->contentHeight(), expectedContentSize); - - // Flick to the end (row/column 100, and overshoot a bit), and - // check that we then end up with the exact content width/height. - const qreal secondHalfLength = largeCellCount * cellSizeLarge; - const qreal expectedFullSize = (firstHalfLength + secondHalfLength) + accumulatedSpacing; - const qreal overshoot = 100; - const qreal endPosX = expectedFullSize - tableView->width() + overshoot; - const qreal endPosY = expectedFullSize - tableView->height() + overshoot; - tableView->setContentX(endPosX); - tableView->setContentY(endPosY); - - QCOMPARE(tableView->contentWidth(), expectedFullSize); - QCOMPARE(tableView->contentHeight(), expectedFullSize); // Flick back to start tableView->setContentX(0); @@ -667,7 +604,7 @@ void tst_QQuickTableView::checkContentWidthAndHeight() QVERIFY(tableViewPrivate->scheduledRebuildOptions); WAIT_UNTIL_POLISHED; - // We should now have the same content width/height as when we started + // We should still have the same content width/height as when we started QCOMPARE(tableView->contentWidth(), expectedSizeInit); QCOMPARE(tableView->contentHeight(), expectedSizeInit); } @@ -1303,8 +1240,11 @@ void tst_QQuickTableView::checkSpacingValues() tableView->polish(); WAIT_UNTIL_POLISHED; - QCOMPARE(tableView->contentWidth(), columnCount * (delegateWidth + tableView->columnSpacing()) - tableView->columnSpacing()); - QCOMPARE(tableView->contentHeight(), rowCount * (delegateHeight + tableView->rowSpacing()) - tableView->rowSpacing()); + + 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); // Valid spacing assignment tableView->setRowSpacing(42); @@ -1314,8 +1254,13 @@ void tst_QQuickTableView::checkSpacingValues() tableView->polish(); WAIT_UNTIL_POLISHED; - QCOMPARE(tableView->contentWidth(), columnCount * (delegateWidth + tableView->columnSpacing()) - tableView->columnSpacing()); - QCOMPARE(tableView->contentHeight(), rowCount * (delegateHeight + tableView->rowSpacing()) - tableView->rowSpacing()); + + // 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); // Invalid assignments (should ignore) tableView->setRowSpacing(-1); |