diff options
author | Axel Spoerl <axel.spoerl@qt.io> | 2023-08-16 20:09:52 +0200 |
---|---|---|
committer | Axel Spoerl <axel.spoerl@qt.io> | 2023-09-01 00:11:51 +0200 |
commit | 4f4a8e75ab34003a4a49b89392ae7712415ac788 (patch) | |
tree | 24107251d0106e66fb9394e5bd8f95286e5251ce /src/corelib/itemmodels | |
parent | 381612f7944b202c8b1428f0cc9d1af72f5f7647 (diff) |
QItemSelectionModelPrivate: use QObjectPrivate::connect
QItemSelectionModelPrivate::initModel() uses string based connections,
to connect/disconnet its QAbstractItemModel.
The QObject::destroyed signal is connected to modelDestroyed(), which
does not disconnect other signals.
QQuickTableView's selection model binds to its QAbstractItemModel.
The binding also reacts to QObject::destroyed
Eventually, QItemSelectionModel::setModel(nullptr) is called.
At this point, only a QOBject is left from the QAbstractItemModel.
That leads to warnings about disconnecting string based signals, which
belong to QAbstractItemModel.
This patch changes the connect syntax to the QObjectPrivate::connect
API. Instead of keeping a list of string based connections around, the
connections themselves are kept in a list member. Disconnecting happens
based on that list.
Connections are also disconnected in
QAbstractItemModelPrivate::modelDestroyed.
An auto test is added in tst_QItemSelectionModel.
Fixes: QTBUG-116056
Pick-to: 6.6 6.5 6.2
Change-Id: I57e5c0f0a574f154eb312a282003774dd0613dd6
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
Diffstat (limited to 'src/corelib/itemmodels')
-rw-r--r-- | src/corelib/itemmodels/qitemselectionmodel.cpp | 94 | ||||
-rw-r--r-- | src/corelib/itemmodels/qitemselectionmodel.h | 7 | ||||
-rw-r--r-- | src/corelib/itemmodels/qitemselectionmodel_p.h | 29 |
3 files changed, 70 insertions, 60 deletions
diff --git a/src/corelib/itemmodels/qitemselectionmodel.cpp b/src/corelib/itemmodels/qitemselectionmodel.cpp index 1f32519fd6..710f07c3bd 100644 --- a/src/corelib/itemmodels/qitemselectionmodel.cpp +++ b/src/corelib/itemmodels/qitemselectionmodel.cpp @@ -550,52 +550,55 @@ void QItemSelection::split(const QItemSelectionRange &range, void QItemSelectionModelPrivate::initModel(QAbstractItemModel *m) { - static constexpr auto connections = qOffsetStringArray( - QT_STRINGIFY_SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)), - QT_STRINGIFY_SLOT(_q_rowsAboutToBeRemoved(QModelIndex,int,int)), - QT_STRINGIFY_SIGNAL(columnsAboutToBeRemoved(QModelIndex,int,int)), - QT_STRINGIFY_SLOT(_q_columnsAboutToBeRemoved(QModelIndex,int,int)), - QT_STRINGIFY_SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)), - QT_STRINGIFY_SLOT(_q_rowsAboutToBeInserted(QModelIndex,int,int)), - QT_STRINGIFY_SIGNAL(columnsAboutToBeInserted(QModelIndex,int,int)), - QT_STRINGIFY_SLOT(_q_columnsAboutToBeInserted(QModelIndex,int,int)), - QT_STRINGIFY_SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - QT_STRINGIFY_SLOT(_q_layoutAboutToBeChanged()), - QT_STRINGIFY_SIGNAL(columnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - QT_STRINGIFY_SLOT(_q_layoutAboutToBeChanged()), - QT_STRINGIFY_SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)), - QT_STRINGIFY_SLOT(_q_layoutChanged()), - QT_STRINGIFY_SIGNAL(columnsMoved(QModelIndex,int,int,QModelIndex,int)), - QT_STRINGIFY_SLOT(_q_layoutChanged()), - QT_STRINGIFY_SIGNAL(layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)), - QT_STRINGIFY_SLOT(_q_layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)), - QT_STRINGIFY_SIGNAL(layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)), - QT_STRINGIFY_SLOT(_q_layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)), - QT_STRINGIFY_SIGNAL(modelReset()), - QT_STRINGIFY_SLOT(reset()), - QT_STRINGIFY_SIGNAL(destroyed(QObject*)), - QT_STRINGIFY_SLOT(_q_modelDestroyed()) - ); - + Q_Q(QItemSelectionModel); if (model == m) return; - Q_Q(QItemSelectionModel); - if (model.value()) { - for (int i = 0; i < connections.count(); i += 2) - QObject::disconnect(model.value(), connections.at(i), q, connections.at(i + 1)); - q->reset(); - } + if (model) + disconnectModel(); // Caller has to call notify(), unless calling during construction (the common case). model.setValueBypassingBindings(m); if (model.value()) { - for (int i = 0; i < connections.count(); i += 2) - QObject::connect(model.value(), connections.at(i), q, connections.at(i + 1)); + connections = std::array<QMetaObject::Connection, 12> { + QObjectPrivate::connect(model, &QAbstractItemModel::rowsAboutToBeRemoved, + this, &QItemSelectionModelPrivate::rowsAboutToBeRemoved), + QObjectPrivate::connect(model, &QAbstractItemModel::columnsAboutToBeRemoved, + this, &QItemSelectionModelPrivate::columnsAboutToBeRemoved), + QObjectPrivate::connect(model, &QAbstractItemModel::rowsAboutToBeInserted, + this, &QItemSelectionModelPrivate::rowsAboutToBeInserted), + QObjectPrivate::connect(model, &QAbstractItemModel::columnsAboutToBeInserted, + this, &QItemSelectionModelPrivate::columnsAboutToBeInserted), + QObjectPrivate::connect(model, &QAbstractItemModel::rowsAboutToBeMoved, + this, &QItemSelectionModelPrivate::triggerLayoutToBeChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::columnsAboutToBeMoved, + this, &QItemSelectionModelPrivate::triggerLayoutToBeChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::rowsMoved, + this, &QItemSelectionModelPrivate::triggerLayoutChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::columnsMoved, + this, &QItemSelectionModelPrivate::triggerLayoutChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::layoutAboutToBeChanged, + this, &QItemSelectionModelPrivate::layoutAboutToBeChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::layoutChanged, + this, &QItemSelectionModelPrivate::layoutChanged), + QObject::connect(model, &QAbstractItemModel::modelReset, + q, &QItemSelectionModel::reset), + QObjectPrivate::connect(model, &QAbstractItemModel::destroyed, + this, &QItemSelectionModelPrivate::modelDestroyed)}; } } +void QItemSelectionModelPrivate::disconnectModel() +{ + Q_Q(QItemSelectionModel); + for (auto &connection : connections) { + QObject::disconnect(connection); + connection = QMetaObject::Connection(); + } + q->reset(); +} + /*! \internal @@ -638,7 +641,7 @@ QItemSelection QItemSelectionModelPrivate::expandSelection(const QItemSelection /*! \internal */ -void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &parent, +void QItemSelectionModelPrivate::rowsAboutToBeRemoved(const QModelIndex &parent, int start, int end) { Q_Q(QItemSelectionModel); @@ -721,7 +724,7 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &pare /*! \internal */ -void QItemSelectionModelPrivate::_q_columnsAboutToBeRemoved(const QModelIndex &parent, +void QItemSelectionModelPrivate::columnsAboutToBeRemoved(const QModelIndex &parent, int start, int end) { Q_Q(QItemSelectionModel); @@ -758,7 +761,7 @@ void QItemSelectionModelPrivate::_q_columnsAboutToBeRemoved(const QModelIndex &p Split selection ranges if columns are about to be inserted in the middle. */ -void QItemSelectionModelPrivate::_q_columnsAboutToBeInserted(const QModelIndex &parent, +void QItemSelectionModelPrivate::columnsAboutToBeInserted(const QModelIndex &parent, int start, int end) { Q_UNUSED(end); @@ -788,7 +791,7 @@ void QItemSelectionModelPrivate::_q_columnsAboutToBeInserted(const QModelIndex & Split selection ranges if rows are about to be inserted in the middle. */ -void QItemSelectionModelPrivate::_q_rowsAboutToBeInserted(const QModelIndex &parent, +void QItemSelectionModelPrivate::rowsAboutToBeInserted(const QModelIndex &parent, int start, int end) { Q_Q(QItemSelectionModel); @@ -829,7 +832,8 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeInserted(const QModelIndex &par preparation for the layoutChanged() signal, where the indexes can be merged again. */ -void QItemSelectionModelPrivate::_q_layoutAboutToBeChanged(const QList<QPersistentModelIndex> &, QAbstractItemModel::LayoutChangeHint hint) +void QItemSelectionModelPrivate::layoutAboutToBeChanged(const QList<QPersistentModelIndex> &, + QAbstractItemModel::LayoutChangeHint hint) { savedPersistentIndexes.clear(); savedPersistentCurrentIndexes.clear(); @@ -976,7 +980,7 @@ static QItemSelection mergeIndexes(const QList<QPersistentModelIndex> &indexes) /*! \internal - Sort predicate function for QItemSelectionModelPrivate::_q_layoutChanged(), + Sort predicate function for QItemSelectionModelPrivate::layoutChanged(), sorting by parent first in addition to operator<(). This is to prevent fragmentation of the selection by grouping indexes with the same row, column of different parents next to each other, which may happen when a selection @@ -994,7 +998,7 @@ static bool qt_PersistentModelIndexLessThan(const QPersistentModelIndex &i1, con Merge the selected indexes into selection ranges again. */ -void QItemSelectionModelPrivate::_q_layoutChanged(const QList<QPersistentModelIndex> &, QAbstractItemModel::LayoutChangeHint hint) +void QItemSelectionModelPrivate::layoutChanged(const QList<QPersistentModelIndex> &, QAbstractItemModel::LayoutChangeHint hint) { // special case for when all indexes are selected if (tableSelected && tableColCount == model->columnCount(tableParent) @@ -1078,9 +1082,10 @@ void QItemSelectionModelPrivate::_q_layoutChanged(const QList<QPersistentModelIn We decide to break the new rule, imposed by bindable properties, and not break the old rule, because that may break existing code. */ -void QItemSelectionModelPrivate::_q_modelDestroyed() +void QItemSelectionModelPrivate::modelDestroyed() { model.setValueBypassingBindings(nullptr); + disconnectModel(); model.notify(); } @@ -1289,7 +1294,7 @@ void QItemSelectionModel::select(const QItemSelection &selection, QItemSelection // If d->ranges is non-empty when the source model is reset the persistent indexes // it contains will be invalid. We can't clear them in a modelReset slot because that might already // be too late if another model observer is connected to the same modelReset slot and is invoked first - // it might call select() on this selection model before any such QItemSelectionModelPrivate::_q_modelReset() slot + // it might call select() on this selection model before any such QItemSelectionModelPrivate::modelReset() slot // is invoked, so it would not be cleared yet. We clear it invalid ranges in it here. d->ranges.removeIf(QtFunctionObjects::IsNotValid()); @@ -1883,7 +1888,6 @@ void QItemSelectionModel::setModel(QAbstractItemModel *model) d->model.removeBindingUnlessInWrapper(); if (d->model == model) return; - d->initModel(model); d->model.notify(); } diff --git a/src/corelib/itemmodels/qitemselectionmodel.h b/src/corelib/itemmodels/qitemselectionmodel.h index 4237e7f74f..a19923a90b 100644 --- a/src/corelib/itemmodels/qitemselectionmodel.h +++ b/src/corelib/itemmodels/qitemselectionmodel.h @@ -165,13 +165,6 @@ protected: private: Q_DISABLE_COPY(QItemSelectionModel) - Q_PRIVATE_SLOT(d_func(), void _q_columnsAboutToBeRemoved(const QModelIndex&, int, int)) - Q_PRIVATE_SLOT(d_func(), void _q_rowsAboutToBeRemoved(const QModelIndex&, int, int)) - Q_PRIVATE_SLOT(d_func(), void _q_columnsAboutToBeInserted(const QModelIndex&, int, int)) - Q_PRIVATE_SLOT(d_func(), void _q_rowsAboutToBeInserted(const QModelIndex&, int, int)) - Q_PRIVATE_SLOT(d_func(), void _q_layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents = QList<QPersistentModelIndex>(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoHint)) - Q_PRIVATE_SLOT(d_func(), void _q_layoutChanged(const QList<QPersistentModelIndex> &parents = QList<QPersistentModelIndex>(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoHint)) - Q_PRIVATE_SLOT(d_func(), void _q_modelDestroyed()) }; Q_DECLARE_OPERATORS_FOR_FLAGS(QItemSelectionModel::SelectionFlags) diff --git a/src/corelib/itemmodels/qitemselectionmodel_p.h b/src/corelib/itemmodels/qitemselectionmodel_p.h index 0f5fd30b28..d3138971d6 100644 --- a/src/corelib/itemmodels/qitemselectionmodel_p.h +++ b/src/corelib/itemmodels/qitemselectionmodel_p.h @@ -18,6 +18,7 @@ #include "qitemselectionmodel.h" #include "private/qobject_p.h" #include "private/qproperty_p.h" +#include <array> QT_REQUIRE_CONFIG(itemmodel); @@ -36,13 +37,23 @@ public: void initModel(QAbstractItemModel *model); - void _q_rowsAboutToBeRemoved(const QModelIndex &parent, int start, int end); - void _q_columnsAboutToBeRemoved(const QModelIndex &parent, int start, int end); - void _q_rowsAboutToBeInserted(const QModelIndex &parent, int start, int end); - void _q_columnsAboutToBeInserted(const QModelIndex &parent, int start, int end); - void _q_layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents = QList<QPersistentModelIndex>(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoLayoutChangeHint); - void _q_layoutChanged(const QList<QPersistentModelIndex> &parents = QList<QPersistentModelIndex>(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoLayoutChangeHint); - void _q_modelDestroyed(); + void rowsAboutToBeRemoved(const QModelIndex &parent, int start, int end); + void columnsAboutToBeRemoved(const QModelIndex &parent, int start, int end); + void rowsAboutToBeInserted(const QModelIndex &parent, int start, int end); + void columnsAboutToBeInserted(const QModelIndex &parent, int start, int end); + void layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint); + void triggerLayoutToBeChanged() + { + layoutAboutToBeChanged(QList<QPersistentModelIndex>(), QAbstractItemModel::NoLayoutChangeHint); + } + + void layoutChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint); + void triggerLayoutChanged() + { + layoutChanged(QList<QPersistentModelIndex>(), QAbstractItemModel::NoLayoutChangeHint); + } + + void modelDestroyed(); inline void remove(QList<QItemSelectionRange> &r) { @@ -59,7 +70,8 @@ public: } void setModel(QAbstractItemModel *mod) { q_func()->setModel(mod); } - void modelChanged(QAbstractItemModel *mod) { q_func()->modelChanged(mod); } + void disconnectModel(); + void modelChanged(QAbstractItemModel *mod) { emit q_func()->modelChanged(mod); } Q_OBJECT_COMPAT_PROPERTY_WITH_ARGS(QItemSelectionModelPrivate, QAbstractItemModel *, model, &QItemSelectionModelPrivate::setModel, &QItemSelectionModelPrivate::modelChanged, nullptr) @@ -76,6 +88,7 @@ public: bool tableSelected; QPersistentModelIndex tableParent; int tableColCount, tableRowCount; + std::array<QMetaObject::Connection, 12> connections; }; QT_END_NAMESPACE |