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>
This commit is contained in:
Christian Ehrlicher 2019-12-23 15:44:48 +01:00
parent b8be5b4002
commit 8de62d3432
11 changed files with 436 additions and 4 deletions

View File

@ -3335,8 +3335,19 @@ void QAbstractItemView::dataChanged(const QModelIndex &topLeft, const QModelInde
}
} else {
d->updateEditorData(topLeft, bottomRight);
if (isVisible() && !d->delayedPendingLayout)
d->viewport->update();
if (isVisible() && !d->delayedPendingLayout) {
if (!topLeft.isValid() ||
topLeft.parent() != bottomRight.parent() ||
topLeft.row() > bottomRight.row() ||
topLeft.column() > bottomRight.column()) {
// invalid parameter - call update() to redraw all
d->viewport->update();
} else {
const QRect updateRect = d->intersectedRect(d->viewport->rect(), topLeft, bottomRight);
if (!updateRect.isEmpty())
d->viewport->update(updateRect);
}
}
}
#ifndef QT_NO_ACCESSIBILITY
@ -3620,6 +3631,19 @@ void QAbstractItemViewPrivate::_q_columnsMoved(const QModelIndex &, int, int, co
_q_layoutChanged();
}
QRect QAbstractItemViewPrivate::intersectedRect(const QRect rect, const QModelIndex &topLeft, const QModelIndex &bottomRight) const
{
Q_Q(const QAbstractItemView);
const auto parentIdx = topLeft.parent();
QRect updateRect;
for (int r = topLeft.row(); r <= bottomRight.row(); ++r) {
for (int c = topLeft.column(); c <= bottomRight.column(); ++c)
updateRect |= q->visualRect(model->index(r, c, parentIdx));
}
return rect.intersected(updateRect);
}
/*!
This slot is called when the selection is changed. The previous
selection (which may be empty), is specified by \a deselected, and the

View File

@ -108,6 +108,7 @@ public:
virtual void _q_layoutChanged();
virtual void _q_rowsMoved(const QModelIndex &source, int sourceStart, int sourceEnd, const QModelIndex &destination, int destinationStart);
virtual void _q_columnsMoved(const QModelIndex &source, int sourceStart, int sourceEnd, const QModelIndex &destination, int destinationStart);
virtual QRect intersectedRect(const QRect rect, const QModelIndex &topLeft, const QModelIndex &bottomRight) const;
void _q_headerDataChanged() { doDelayedItemsLayout(); }
void _q_scrollerStateChanged();

View File

@ -669,6 +669,52 @@ void QTableViewPrivate::trimHiddenSelections(QItemSelectionRange *range) const
*range = QItemSelectionRange(topLeft, bottomRight);
}
QRect QTableViewPrivate::intersectedRect(const QRect rect, const QModelIndex &topLeft, const QModelIndex &bottomRight) const
{
Q_Q(const QTableView);
using minMaxPair = std::pair<int, int>;
const auto calcMinMax = [q, rect](QHeaderView *hdr, int startIdx, int endIdx, minMaxPair bounds) -> minMaxPair
{
minMaxPair ret(std::numeric_limits<int>::max(), std::numeric_limits<int>::min());
if (hdr->sectionsMoved()) {
for (int i = startIdx; i <= endIdx; ++i) {
const int start = hdr->sectionViewportPosition(i);
const int end = start + hdr->sectionSize(i);
ret.first = std::min(start, ret.first);
ret.second = std::max(end, ret.second);
if (ret.first <= bounds.first && ret.second >= bounds.second)
break;
}
} else {
if (q->isRightToLeft() && q->horizontalHeader() == hdr)
std::swap(startIdx, endIdx);
ret.first = hdr->sectionViewportPosition(startIdx);
ret.second = hdr->sectionViewportPosition(endIdx) +
hdr->sectionSize(endIdx);
}
return ret;
};
const auto yVals = calcMinMax(verticalHeader, topLeft.row(), bottomRight.row(),
minMaxPair(rect.top(), rect.bottom()));
if (yVals.first == yVals.second) // all affected rows are hidden
return QRect();
// short circuit: check if no row is inside rect
const QRect colRect(QPoint(rect.left(), yVals.first),
QPoint(rect.right(), yVals.second));
const QRect intersected = rect.intersected(colRect);
if (intersected.isNull())
return QRect();
const auto xVals = calcMinMax(horizontalHeader, topLeft.column(), bottomRight.column(),
minMaxPair(rect.left(), rect.right()));
const QRect updateRect(QPoint(xVals.first, yVals.first),
QPoint(xVals.second, yVals.second));
return rect.intersected(updateRect);
}
/*!
\internal
Sets the span for the cell at (\a row, \a column).

View File

@ -149,6 +149,7 @@ public:
}
void init();
void trimHiddenSelections(QItemSelectionRange *range) const;
QRect intersectedRect(const QRect rect, const QModelIndex &topLeft, const QModelIndex &bottomRight) const override;
inline bool isHidden(int row, int col) const {
return verticalHeader->isSectionHidden(row)

View File

@ -1396,6 +1396,24 @@ void QTreeViewPrivate::_q_modelDestroyed()
QAbstractItemViewPrivate::_q_modelDestroyed();
}
QRect QTreeViewPrivate::intersectedRect(const QRect rect, const QModelIndex &topLeft, const QModelIndex &bottomRight) const
{
Q_Q(const QTreeView);
const auto parentIdx = topLeft.parent();
executePostedLayout();
QRect updateRect;
for (int r = topLeft.row(); r <= bottomRight.row(); ++r) {
if (isRowHidden(model->index(r, 0, parentIdx)))
continue;
for (int c = topLeft.column(); c <= bottomRight.column(); ++c) {
const QModelIndex idx(model->index(r, c, parentIdx));
updateRect |= q->visualRect(idx);
}
}
return rect.intersected(updateRect);
}
/*!
\reimp

View File

@ -135,6 +135,7 @@ public:
void _q_modelAboutToBeReset();
void _q_sortIndicatorChanged(int column, Qt::SortOrder order);
void _q_modelDestroyed() override;
QRect intersectedRect(const QRect rect, const QModelIndex &topLeft, const QModelIndex &bottomRight) const override;
void layout(int item, bool recusiveExpanding = false, bool afterIsUninitialized = false);

View File

@ -12,4 +12,5 @@ add_qt_test(tst_qabstractitemview
Qt::GuiPrivate
Qt::TestPrivate
Qt::Widgets
Qt::WidgetsPrivate
)

View File

@ -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

View File

@ -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))

View File

@ -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;

View File

@ -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);