diff options
author | Christian Ehrlicher <ch.ehrlicher@gmx.de> | 2019-12-23 15:44:48 +0100 |
---|---|---|
committer | Christian Ehrlicher <ch.ehrlicher@gmx.de> | 2020-03-28 09:03:18 +0100 |
commit | 8de62d34321cd827e60b0a7b6e7434070de301ae (patch) | |
tree | 048f18f000b6d9235057d800e5f3d79be35a5570 /tests/auto/widgets/itemviews | |
parent | b8be5b4002bd6163851bbae397171ebbf632f02f (diff) |
QAbstractItemView::dataChanged(): optimize call to QWidget::update()
When topLeft and bottomRight are different in QAIV::dataChanged(), the
current implementation simply calls QWidget::update() without checking
if the affected cells are visible. This results in a big performance hit
when cells are updated frequently.
Now try to compute the exact update rect by iterating through the
modified indexes.
Fixes: QTBUG-58580
Change-Id: I97de567d494e40ed8cdb1ea1f5b3cf3a2f60455e
Reviewed-by: Samuel Gaist <samuel.gaist@idiap.ch>
Diffstat (limited to 'tests/auto/widgets/itemviews')
5 files changed, 343 insertions, 2 deletions
diff --git a/tests/auto/widgets/itemviews/qabstractitemview/CMakeLists.txt b/tests/auto/widgets/itemviews/qabstractitemview/CMakeLists.txt index 520a0b4b98..2fc8c14eb6 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/CMakeLists.txt +++ b/tests/auto/widgets/itemviews/qabstractitemview/CMakeLists.txt @@ -12,4 +12,5 @@ add_qt_test(tst_qabstractitemview Qt::GuiPrivate Qt::TestPrivate Qt::Widgets + Qt::WidgetsPrivate ) diff --git a/tests/auto/widgets/itemviews/qabstractitemview/qabstractitemview.pro b/tests/auto/widgets/itemviews/qabstractitemview/qabstractitemview.pro index 809a996324..b9091a134e 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/qabstractitemview.pro +++ b/tests/auto/widgets/itemviews/qabstractitemview/qabstractitemview.pro @@ -1,4 +1,4 @@ CONFIG += testcase TARGET = tst_qabstractitemview -QT += widgets testlib testlib-private gui-private +QT += widgets testlib testlib-private gui-private widgets-private SOURCES += tst_qabstractitemview.cpp diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp index 0834d989e0..44b728a042 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp +++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp @@ -51,6 +51,7 @@ #include <QTest> #include <QVBoxLayout> #include <QtTest/private/qtesthelpers_p.h> +#include <private/qabstractitemview_p.h> Q_DECLARE_METATYPE(Qt::ItemFlags); @@ -115,6 +116,9 @@ private slots: void setCurrentIndex_data(); void setCurrentIndex(); + void checkIntersectedRect_data(); + void checkIntersectedRect(); + void task221955_selectedEditor(); void task250754_fontChange(); void task200665_itemEntered(); @@ -468,7 +472,8 @@ void tst_QAbstractItemView::basic_tests(QAbstractItemView *view) view->setCurrentIndex(QModelIndex()); // protected methods - view->dataChanged(QModelIndex(), QModelIndex()); + // will assert because an invalid index is passed + //view->dataChanged(QModelIndex(), QModelIndex()); view->rowsInserted(QModelIndex(), -1, -1); view->rowsAboutToBeRemoved(QModelIndex(), -1, -1); view->selectionChanged(QItemSelection(), QItemSelection()); @@ -1098,6 +1103,101 @@ void tst_QAbstractItemView::setCurrentIndex() QVERIFY(view->currentIndex() == model->index(result ? 1 : 0, 0)); } +void tst_QAbstractItemView::checkIntersectedRect_data() +{ + auto createModel = [](int rowCount) -> QStandardItemModel* + { + QStandardItemModel *model = new QStandardItemModel; + for (int i = 0; i < rowCount; ++i) { + const QList<QStandardItem *> sil({new QStandardItem(QLatin1String("Row %1 Item").arg(i)), + new QStandardItem(QLatin1String("2nd column"))}); + model->appendRow(sil); + } + return model; + }; + QTest::addColumn<QStandardItemModel *>("model"); + QTest::addColumn<QVector<QModelIndex>>("changedIndexes"); + QTest::addColumn<bool>("isEmpty"); + { + auto model = createModel(5); + QTest::newRow("multiple columns") << model + << QVector<QModelIndex>({model->index(0, 0), + model->index(0, 1)}) + << false; + } + { + auto model = createModel(5); + QTest::newRow("multiple rows") << model + << QVector<QModelIndex>({model->index(0, 0), + model->index(1, 0), + model->index(2, 0)}) + << false; + } + { + auto model = createModel(5); + QTest::newRow("hidden rows") << model + << QVector<QModelIndex>({model->index(3, 0), + model->index(4, 0)}) + << true; + } + { + auto model = createModel(500); + QTest::newRow("rows outside viewport") << model + << QVector<QModelIndex>({model->index(498, 0), + model->index(499, 0)}) + << true; + } +} + +void tst_QAbstractItemView::checkIntersectedRect() +{ + QFETCH(QStandardItemModel *, model); + QFETCH(const QVector<QModelIndex>, changedIndexes); + QFETCH(bool, isEmpty); + + class TableView : public QTableView + { + public: + void dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles = QVector<int>()) override + { + QTableView::dataChanged(topLeft, bottomRight, roles); + // we want to check the base class implementation here! + QAbstractItemViewPrivate *av = static_cast<QAbstractItemViewPrivate*>(qt_widget_private(this)); + m_intersectecRect = av->intersectedRect(av->viewport->rect(), topLeft, bottomRight); + } + mutable QRect m_intersectecRect; + }; + + TableView view; + model->setParent(&view); + view.setModel(model); + view.resize(400, 400); + view.show(); + view.setRowHidden(3, true); + view.setRowHidden(4, true); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + + view.m_intersectecRect = QRect(); + emit view.model()->dataChanged(changedIndexes.first(), changedIndexes.last()); + if (isEmpty) { + QVERIFY(view.m_intersectecRect.isEmpty()); + } else { + const auto parent = changedIndexes.first().parent(); + const int rCount = view.model()->rowCount(parent); + const int cCount = view.model()->columnCount(parent); + for (int r = 0; r < rCount; ++r) { + for (int c = 0; c < cCount; ++c) { + const QModelIndex &idx = view.model()->index(r, c, parent); + const auto rect = view.visualRect(idx); + if (changedIndexes.contains(idx)) + QVERIFY(view.m_intersectecRect.contains(rect)); + else + QVERIFY(!view.m_intersectecRect.contains(rect)); + } + } + } +} + void tst_QAbstractItemView::task221955_selectedEditor() { if (!QGuiApplicationPrivate::platformIntegration()->hasCapability(QPlatformIntegration::WindowActivation)) diff --git a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp index 544069243f..ccc6997f26 100644 --- a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp +++ b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp @@ -243,6 +243,14 @@ public: verticalHeader()->setMinimumSectionSize(0); } + void dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles = QVector<int>()) override + { + QTableView::dataChanged(topLeft, bottomRight, roles); + QTableViewPrivate *av = static_cast<QTableViewPrivate*>(qt_widget_private(this)); + m_intersectecRect = av->intersectedRect(av->viewport->rect(), topLeft, bottomRight); + } + mutable QRect m_intersectecRect; + using QTableView::moveCursor; using QTableView::isIndexHidden; using QTableView::setSelection; @@ -402,6 +410,9 @@ private slots: void selectionSignal(); void setCurrentIndex(); + void checkIntersectedRect_data(); + void checkIntersectedRect(); + // task-specific tests: void task173773_updateVerticalHeader(); void task227953_setRootIndex(); @@ -3842,6 +3853,121 @@ void tst_QTableView::setCurrentIndex() QCOMPARE(model.submit_count, 4); } +void tst_QTableView::checkIntersectedRect_data() +{ + QTest::addColumn<QtTestTableModel *>("model"); + QTest::addColumn<QVector<QModelIndex>>("changedIndexes"); + QTest::addColumn<bool>("isEmpty"); + QTest::addColumn<bool>("swapFirstAndLastIndexRow"); // for QHeaderView::sectionsMoved() + QTest::addColumn<bool>("swapFirstAndLastIndexColumn"); // for QHeaderView::sectionsMoved() + QTest::addColumn<Qt::LayoutDirection>("layoutDirection"); + QTest::addColumn<int>("hiddenRow"); + QTest::addColumn<int>("hiddenCol"); + const auto testName = [](const QByteArray &prefix, Qt::LayoutDirection dir, bool r, bool c) + { + const char *strDir = dir == Qt::LeftToRight ? ", LeftToRight" : ", RightToLeft"; + const char *strRow = r ? ", rowsSwapped" : ""; + const char *strCol = c ? ", colsSwapped" : ""; + return prefix + strDir + strRow + strCol; + }; + for (int i = 0; i < 2; ++i) { + const Qt::LayoutDirection dir(i == 0 ? Qt::LeftToRight : Qt::RightToLeft); + for (int j = 0; j < 4; ++j) { + const bool swapRow = ((j & 1) == 1); + const bool swapColumn = ((j & 2) == 2); + { + QtTestTableModel *model = new QtTestTableModel(10, 3); + QTest::newRow(testName("multiple columns", dir, swapRow, swapColumn).data()) + << model + << QVector<QModelIndex>({model->index(0, 0), + model->index(0, 1)}) + << false << swapRow << swapColumn << dir << -1 << -1; + } + { + QtTestTableModel *model = new QtTestTableModel(10, 3); + QTest::newRow(testName("multiple rows", dir, swapRow, swapColumn).data()) + << model + << QVector<QModelIndex>({model->index(0, 0), + model->index(1, 0), + model->index(2, 0)}) + << false << swapRow << swapColumn << dir << -1 << -1; + } + { + QtTestTableModel *model = new QtTestTableModel(10, 3); + QTest::newRow(testName("hidden row", dir, swapRow, swapColumn).data()) + << model + << QVector<QModelIndex>({model->index(3, 0), + model->index(3, 1)}) + << true << swapRow << swapColumn << dir << 3 << -1; + } + { + QtTestTableModel *model = new QtTestTableModel(50, 2); + QTest::newRow(testName("row outside viewport", dir, swapRow, swapColumn).data()) + << model + << QVector<QModelIndex>({model->index(49, 0), + model->index(49, 1)}) + << true << swapRow << swapColumn << dir << -1 << -1; + } + } + } +} + +void tst_QTableView::checkIntersectedRect() +{ + QFETCH(QtTestTableModel *, model); + QFETCH(const QVector<QModelIndex>, changedIndexes); + QFETCH(bool, isEmpty); + QFETCH(bool, swapFirstAndLastIndexRow); + QFETCH(bool, swapFirstAndLastIndexColumn); + QFETCH(Qt::LayoutDirection, layoutDirection); + QFETCH(int, hiddenRow); + QFETCH(int, hiddenCol); + + QtTestTableView view; + model->setParent(&view); + view.setLayoutDirection(layoutDirection); + view.setModel(model); + view.resize(400, 400); + view.show(); + if (hiddenRow >= 0) + view.hideRow(hiddenRow); + if (hiddenCol >= 0) + view.hideRow(hiddenCol); + if (swapFirstAndLastIndexRow) + view.verticalHeader()->swapSections(changedIndexes.first().row(), changedIndexes.last().row()); + if (swapFirstAndLastIndexColumn) + view.horizontalHeader()->swapSections(changedIndexes.first().column(), changedIndexes.last().column()); + + QVERIFY(QTest::qWaitForWindowExposed(&view)); + + const auto toString = [](const QModelIndex &idx) + { + return QStringLiteral("idx: %1/%2").arg(idx.row()).arg(idx.column()); + }; + + view.m_intersectecRect = QRect(); + emit view.model()->dataChanged(changedIndexes.first(), changedIndexes.last()); + if (isEmpty) { + QVERIFY(view.m_intersectecRect.isEmpty()); + } else if (!changedIndexes.first().isValid()) { + QCOMPARE(view.m_intersectecRect, view.viewport()->rect()); + } else { + const auto parent = changedIndexes.first().parent(); + const int rCount = view.model()->rowCount(parent); + const int cCount = view.model()->columnCount(parent); + for (int r = 0; r < rCount; ++r) { + for (int c = 0; c < cCount; ++c) { + const QModelIndex &idx = view.model()->index(r, c, parent); + const auto rect = view.visualRect(idx); + if (changedIndexes.contains(idx)) + QVERIFY2(view.m_intersectecRect.contains(rect), qPrintable(toString(idx))); + else + QVERIFY2(!view.m_intersectecRect.contains(rect), qPrintable(toString(idx))); + } + } + } +} + class task173773_EventFilter : public QObject { int paintEventCount_ = 0; diff --git a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp index 7259fa1ab0..95501136cc 100644 --- a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp +++ b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp @@ -89,6 +89,13 @@ public: QTreeView::paintEvent(event); wasPainted = true; } + void dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles = QVector<int>()) override + { + QTreeView::dataChanged(topLeft, bottomRight, roles); + QTreeViewPrivate *av = static_cast<QTreeViewPrivate*>(qt_widget_private(this)); + m_intersectecRect = av->intersectedRect(av->viewport->rect(), topLeft, bottomRight); + } + mutable QRect m_intersectecRect; bool wasPainted = false; public slots: void handleSelectionChanged() @@ -208,6 +215,8 @@ private slots: void statusTip_data(); void statusTip(); void fetchMoreOnScroll(); + void checkIntersectedRect_data(); + void checkIntersectedRect(); // task-specific tests: void task174627_moveLeftToRoot(); @@ -4832,6 +4841,111 @@ void tst_QTreeView::fetchMoreOnScroll() QCOMPARE(im.item(19)->rowCount(), 20); } +void tst_QTreeView::checkIntersectedRect_data() +{ + auto createModel = [](int rowCount) + { + QStandardItemModel *model = new QStandardItemModel; + for (int i = 0; i < rowCount; ++i) { + const QList<QStandardItem *> sil({new QStandardItem(QLatin1String("Row %1 Item").arg(i)), + new QStandardItem(QLatin1String("2nd column"))}); + model->appendRow(sil); + } + for (int i = 2; i < 4; ++i) { + const QList<QStandardItem *> sil({new QStandardItem(QLatin1String("Row %1 Item").arg(i)), + new QStandardItem(QLatin1String("2nd column"))}); + model->item(i)->appendRow(sil); + } + return model; + }; + QTest::addColumn<QStandardItemModel *>("model"); + QTest::addColumn<QVector<QModelIndex>>("changedIndexes"); + QTest::addColumn<bool>("isEmpty"); + { + auto model = createModel(5); + QTest::newRow("multiple columns") << model + << QVector<QModelIndex>({model->index(0, 0), + model->index(0, 1)}) + << false; + } + { + auto model = createModel(5); + QTest::newRow("multiple rows") << model + << QVector<QModelIndex>({model->index(0, 0), + model->index(1, 0), + model->index(2, 0)}) + << false; + } + { + auto model = createModel(5); + const QModelIndex idxRow2(model->indexFromItem(model->item(2))); + QTest::newRow("child row") << model + << QVector<QModelIndex>({model->index(0, 0, idxRow2), + model->index(0, 1, idxRow2)}) + << false; + } + { + auto model = createModel(5); + QTest::newRow("hidden row") << model + << QVector<QModelIndex>({model->index(3, 0), + model->index(3, 1)}) + << true; + } + { + auto model = createModel(5); + const QModelIndex idxRow3(model->indexFromItem(model->item(3))); + QTest::newRow("hidden child row") << model + << QVector<QModelIndex>({model->index(0, 0, idxRow3), + model->index(0, 1, idxRow3)}) + << true; + } + { + auto model = createModel(50); + QTest::newRow("row outside viewport") << model + << QVector<QModelIndex>({model->index(49, 0), + model->index(49, 1)}) + << true; + } +} + +void tst_QTreeView::checkIntersectedRect() +{ + QFETCH(QStandardItemModel *, model); + QFETCH(const QVector<QModelIndex>, changedIndexes); + QFETCH(bool, isEmpty); + + TreeView view; + model->setParent(&view); + view.setModel(model); + view.resize(400, 400); + view.show(); + view.expandAll(); + view.setRowHidden(3, QModelIndex(), true); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + + view.m_intersectecRect = QRect(); + emit view.model()->dataChanged(changedIndexes.first(), changedIndexes.last()); + if (isEmpty) { + QVERIFY(view.m_intersectecRect.isEmpty()); + } else if (!changedIndexes.first().isValid()) { + QCOMPARE(view.m_intersectecRect, view.viewport()->rect()); + } else { + const auto parent = changedIndexes.first().parent(); + const int rCount = view.model()->rowCount(parent); + const int cCount = view.model()->columnCount(parent); + for (int r = 0; r < rCount; ++r) { + for (int c = 0; c < cCount; ++c) { + const QModelIndex &idx = view.model()->index(r, c, parent); + const auto rect = view.visualRect(idx); + if (changedIndexes.contains(idx)) + QVERIFY(view.m_intersectecRect.contains(rect)); + else + QVERIFY(!view.m_intersectecRect.contains(rect)); + } + } + } +} + static void fillModeltaskQTBUG_8376(QAbstractItemModel &model) { model.insertRow(0); |