From a76ce7b191933edcba181e888fa62d1a8fa83a49 Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Tue, 10 Sep 2024 21:04:44 +0200 Subject: [PATCH] QItemSelectionModel: cache calls to QPersistentModelIndex functions Try to avoid the expensive calls to QPMI functions (they are not inlined) by caching its results. This is done through a helper class which does a lazy initialization of the members. Fixes: QTBUG-113137 Change-Id: I8e4d79e393fbc219f1ff8bb2207ed479521c0847 Reviewed-by: David Faure --- .../itemmodels/qitemselectionmodel.cpp | 106 +++++++++++++----- 1 file changed, 78 insertions(+), 28 deletions(-) diff --git a/src/corelib/itemmodels/qitemselectionmodel.cpp b/src/corelib/itemmodels/qitemselectionmodel.cpp index 387daa655f1..57ad5019a44 100644 --- a/src/corelib/itemmodels/qitemselectionmodel.cpp +++ b/src/corelib/itemmodels/qitemselectionmodel.cpp @@ -18,6 +18,48 @@ QT_BEGIN_NAMESPACE QT_IMPL_METATYPE_EXTERN(QItemSelectionRange) QT_IMPL_METATYPE_EXTERN(QItemSelection) +// a small helper to cache calls to QPMI functions as they are expensive +struct QItemSelectionRangeRefCache +{ + QItemSelectionRangeRefCache(const QItemSelectionRange &range) + : m_range(range) + {} + inline void invalidate() { m_top = m_left = m_bottom = m_right = -1; } + inline bool contains(int row, int column, const QModelIndex &parentIndex) + { + populate(); + return (m_bottom >= row && m_right >= column && + m_top <= row && m_left <= column && + parent() == parentIndex); + } + inline void populate() + { + if (m_top > -2) + return; + m_top = m_range.top(); + m_left = m_range.left(); + m_bottom = m_range.bottom(); + m_right = m_range.right(); + } + inline const QModelIndex &parent() + { + if (!m_parent.has_value()) + m_parent = m_range.parent(); + return m_parent.value(); + } + // we assume we're initialized for the next functions + inline int bottom() const { return m_bottom; } + inline int right() const { return m_right; } + +private: + int m_top = -2; + int m_left = 0; + int m_bottom = 0; + int m_right = 0; + std::optional m_parent; + const QItemSelectionRange &m_range; +}; + /*! \class QItemSelectionRange \inmodule QtCore @@ -1504,28 +1546,32 @@ bool QItemSelectionModel::isRowSelected(int row, const QModelIndex &parent) cons const int colCount = d->model->columnCount(parent); int unselectable = 0; + std::vector cache; + cache.reserve(d->currentSelection.size() + d->ranges.size()); + std::copy(d->currentSelection.begin(), d->currentSelection.end(), std::back_inserter(cache)); + std::copy(d->ranges.begin(), d->ranges.end(), std::back_inserter(cache)); + // check ranges and currentSelection for (int column = 0; column < colCount; ++column) { if (!isSelectable(row, column)) { ++unselectable; continue; } - const auto doCheck = [&](const auto &listToCheck) - { - for (const auto &curSel : listToCheck) { - if (curSel.contains(row, column, parent)) { - const auto right = curSel.right(); - for (int i = column + 1; i <= right; ++i) { - if (!isSelectable(row, i)) - ++unselectable; - } - column = qMax(column, right); - return true; + bool foundSelection = false; + for (auto &curSel : cache) { + if (curSel.contains(row, column, parent)) { + const auto right = curSel.right(); + for (int i = column + 1; i <= right; ++i) { + if (!isSelectable(row, i)) + ++unselectable; } + column = right; + foundSelection = true; + curSel.invalidate(); + break; } - return false; - }; - if (!doCheck(d->ranges) && !doCheck(d->currentSelection)) + } + if (!foundSelection) return false; } return unselectable < colCount; @@ -1584,28 +1630,32 @@ bool QItemSelectionModel::isColumnSelected(int column, const QModelIndex &parent const int rowCount = d->model->rowCount(parent); int unselectable = 0; + std::vector cache; + cache.reserve(d->currentSelection.size() + d->ranges.size()); + std::copy(d->currentSelection.begin(), d->currentSelection.end(), std::back_inserter(cache)); + std::copy(d->ranges.begin(), d->ranges.end(), std::back_inserter(cache)); + // check ranges and currentSelection for (int row = 0; row < rowCount; ++row) { if (!isSelectable(row, column)) { ++unselectable; continue; } - const auto doCheck = [&](const auto &listToCheck) - { - for (const auto &curSel : listToCheck) { - if (curSel.contains(row, column, parent)) { - const auto bottom = curSel.bottom(); - for (int i = row + 1; i <= bottom; ++i) { - if (!isSelectable(i, column)) - ++unselectable; - } - row = qMax(row, bottom); - return true; + bool foundSelection = false; + for (auto &curSel : cache) { + if (curSel.contains(row, column, parent)) { + const auto bottom = curSel.bottom(); + for (int i = row + 1; i <= bottom; ++i) { + if (!isSelectable(i, column)) + ++unselectable; } + row = bottom; + foundSelection = true; + curSel.invalidate(); + break; } - return false; - }; - if (!doCheck(d->ranges) && !doCheck(d->currentSelection)) + } + if (!foundSelection) return false; } return unselectable < rowCount;