From 3bd0fd8f97e7a33a874929a383a42e6c710bfff3 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 17 Dec 2016 06:20:06 +0000 Subject: QSFPM: Fix handling of source model layout change In sourceLayoutAboutToBeChanged the source model update is ignored if the affected parents are filtered out anyway. The same logic is attempted in the sourceLayoutChanged slot, but there the early-return logic is applied too late - the mapping is cleared before performing the early-return. Because pointers into the mapping are used in the internalPointer of QModelIndexes in this class, persistent indexes used later will segfault when attempting to dereference it. Additionally, if a parent becomes invalid as a result of the layoutChange, it would be filtered out by the condition in the loop, resulting in a different result in the comparison of emptiness of the parents container. Fix that by persisting the parent's container, and performing the test for early-return before clearing the mapping. Task-number: QTBUG-47711 Task-number: QTBUG-32981 Change-Id: If45e8a1c97d39454160f52041bc9ae7e337dce97 Reviewed-by: David Faure --- src/corelib/itemmodels/qsortfilterproxymodel.cpp | 31 +++++++++--------------- 1 file changed, 11 insertions(+), 20 deletions(-) (limited to 'src/corelib/itemmodels') diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp index b0ddfa879d..333152138e 100644 --- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp +++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp @@ -171,6 +171,7 @@ public: QRowsRemoval itemsBeingRemoved; QModelIndexPairList saved_persistent_indexes; + QList saved_layoutChange_parents; QHash::const_iterator create_mapping( const QModelIndex &source_parent) const; @@ -1331,23 +1332,23 @@ void QSortFilterProxyModelPrivate::_q_sourceLayoutAboutToBeChanged(const QList parents; + saved_layoutChange_parents.clear(); for (const QPersistentModelIndex &parent : sourceParents) { if (!parent.isValid()) { - parents << QPersistentModelIndex(); + saved_layoutChange_parents << QPersistentModelIndex(); continue; } const QModelIndex mappedParent = q->mapFromSource(parent); // Might be filtered out. if (mappedParent.isValid()) - parents << mappedParent; + saved_layoutChange_parents << mappedParent; } // All parents filtered out. - if (!sourceParents.isEmpty() && parents.isEmpty()) + if (!sourceParents.isEmpty() && saved_layoutChange_parents.isEmpty()) return; - emit q->layoutAboutToBeChanged(parents); + emit q->layoutAboutToBeChanged(saved_layoutChange_parents); if (persistent.indexes.isEmpty()) return; @@ -1359,6 +1360,9 @@ void QSortFilterProxyModelPrivate::_q_sourceLayoutChanged(const QList parents; - for (const QPersistentModelIndex &parent : sourceParents) { - if (!parent.isValid()) { - parents << QPersistentModelIndex(); - continue; - } - const QModelIndex mappedParent = q->mapFromSource(parent); - if (mappedParent.isValid()) - parents << mappedParent; - } - - if (!sourceParents.isEmpty() && parents.isEmpty()) - return; - - emit q->layoutChanged(parents); + emit q->layoutChanged(saved_layoutChange_parents); + saved_layoutChange_parents.clear(); } void QSortFilterProxyModelPrivate::_q_sourceRowsAboutToBeInserted( -- cgit v1.2.1 From 0874861bcc70313c343aba5e5566ed30b69eed1c Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Mon, 19 Dec 2016 21:13:57 +0000 Subject: QSFPM: Remove data manipulation from move handlers Similar to the fix in the parent commit, incorrect updating of the internal data structures during layout changes can lead to dangling pointers being dereferenced later. Moves are treated as layoutChanges by this proxy by forwarding to the appropriate method. However, data is incorrectly cleared prior to that forwarding. Remove that, and let the layoutChange handling take appropriate action. Change-Id: Iee951e37152328a4e6a5fb8e5385c32a2fe4c0bd Reviewed-by: David Faure --- src/corelib/itemmodels/qsortfilterproxymodel.cpp | 67 +++++------------------- 1 file changed, 12 insertions(+), 55 deletions(-) (limited to 'src/corelib/itemmodels') diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp index 333152138e..226a2401e1 100644 --- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp +++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp @@ -1418,49 +1418,27 @@ void QSortFilterProxyModelPrivate::_q_sourceRowsRemoved( void QSortFilterProxyModelPrivate::_q_sourceRowsAboutToBeMoved( const QModelIndex &sourceParent, int /* sourceStart */, int /* sourceEnd */, const QModelIndex &destParent, int /* dest */) { - Q_Q(QSortFilterProxyModel); // Because rows which are contiguous in the source model might not be contiguous // in the proxy due to sorting, the best thing we can do here is be specific about what // parents are having their children changed. // Optimize: Emit move signals if the proxy is not sorted. Will need to account for rows // being filtered out though. - saved_persistent_indexes.clear(); - QList parents; - parents << q->mapFromSource(sourceParent); + parents << sourceParent; if (sourceParent != destParent) - parents << q->mapFromSource(destParent); - emit q->layoutAboutToBeChanged(parents); - if (persistent.indexes.isEmpty()) - return; - saved_persistent_indexes = store_persistent_indexes(); + parents << destParent; + _q_sourceLayoutAboutToBeChanged(parents, QAbstractItemModel::NoLayoutChangeHint); } void QSortFilterProxyModelPrivate::_q_sourceRowsMoved( const QModelIndex &sourceParent, int /* sourceStart */, int /* sourceEnd */, const QModelIndex &destParent, int /* dest */) { - Q_Q(QSortFilterProxyModel); - - // Optimize: We only need to clear and update the persistent indexes which are children of - // sourceParent or destParent - qDeleteAll(source_index_mapping); - source_index_mapping.clear(); - - update_persistent_indexes(saved_persistent_indexes); - saved_persistent_indexes.clear(); - - if (dynamic_sortfilter && update_source_sort_column()) { - //update_source_sort_column might have created wrong mapping so we have to clear it again - qDeleteAll(source_index_mapping); - source_index_mapping.clear(); - } - QList parents; - parents << q->mapFromSource(sourceParent); + parents << sourceParent; if (sourceParent != destParent) - parents << q->mapFromSource(destParent); - emit q->layoutChanged(parents); + parents << destParent; + _q_sourceLayoutChanged(parents, QAbstractItemModel::NoLayoutChangeHint); } void QSortFilterProxyModelPrivate::_q_sourceColumnsAboutToBeInserted( @@ -1522,42 +1500,21 @@ void QSortFilterProxyModelPrivate::_q_sourceColumnsRemoved( void QSortFilterProxyModelPrivate::_q_sourceColumnsAboutToBeMoved( const QModelIndex &sourceParent, int /* sourceStart */, int /* sourceEnd */, const QModelIndex &destParent, int /* dest */) { - Q_Q(QSortFilterProxyModel); - - saved_persistent_indexes.clear(); - QList parents; - parents << q->mapFromSource(sourceParent); + parents << sourceParent; if (sourceParent != destParent) - parents << q->mapFromSource(destParent); - emit q->layoutAboutToBeChanged(parents); - - if (persistent.indexes.isEmpty()) - return; - saved_persistent_indexes = store_persistent_indexes(); + parents << destParent; + _q_sourceLayoutAboutToBeChanged(parents, QAbstractItemModel::NoLayoutChangeHint); } void QSortFilterProxyModelPrivate::_q_sourceColumnsMoved( const QModelIndex &sourceParent, int /* sourceStart */, int /* sourceEnd */, const QModelIndex &destParent, int /* dest */) { - Q_Q(QSortFilterProxyModel); - - qDeleteAll(source_index_mapping); - source_index_mapping.clear(); - - update_persistent_indexes(saved_persistent_indexes); - saved_persistent_indexes.clear(); - - if (dynamic_sortfilter && update_source_sort_column()) { - qDeleteAll(source_index_mapping); - source_index_mapping.clear(); - } - QList parents; - parents << q->mapFromSource(sourceParent); + parents << sourceParent; if (sourceParent != destParent) - parents << q->mapFromSource(destParent); - emit q->layoutChanged(parents); + parents << destParent; + _q_sourceLayoutChanged(parents, QAbstractItemModel::NoLayoutChangeHint); } /*! -- cgit v1.2.1 From baad82d242a4d8c1af6c87faaa7f25584183fd53 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Tue, 20 Dec 2016 00:44:12 +0000 Subject: QIPM: Persist model indexes after emitting layoutChange, not before Callers can persist a QModelIndex which was not persisted before in a slot connected to the signal, and such a persisted index must be updated in the course of the layoutChange. Store the indexes to persist after emitting the signal. Task-number: QTBUG-32981 Change-Id: Ibee4c0d84817d72603a03fe5b22fdeefeac0695e Reviewed-by: David Faure --- src/corelib/itemmodels/qidentityproxymodel.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'src/corelib/itemmodels') diff --git a/src/corelib/itemmodels/qidentityproxymodel.cpp b/src/corelib/itemmodels/qidentityproxymodel.cpp index e537793146..7c306799d0 100644 --- a/src/corelib/itemmodels/qidentityproxymodel.cpp +++ b/src/corelib/itemmodels/qidentityproxymodel.cpp @@ -496,15 +496,6 @@ void QIdentityProxyModelPrivate::_q_sourceLayoutAboutToBeChanged(const QListpersistentIndexList(); - for (const QPersistentModelIndex &proxyPersistentIndex : proxyPersistentIndexes) { - proxyIndexes << proxyPersistentIndex; - Q_ASSERT(proxyPersistentIndex.isValid()); - const QPersistentModelIndex srcPersistentIndex = q->mapToSource(proxyPersistentIndex); - Q_ASSERT(srcPersistentIndex.isValid()); - layoutChangePersistentIndexes << srcPersistentIndex; - } - QList parents; parents.reserve(sourceParents.size()); for (const QPersistentModelIndex &parent : sourceParents) { @@ -518,6 +509,15 @@ void QIdentityProxyModelPrivate::_q_sourceLayoutAboutToBeChanged(const QListlayoutAboutToBeChanged(parents, hint); + + const auto proxyPersistentIndexes = q->persistentIndexList(); + for (const QPersistentModelIndex &proxyPersistentIndex : proxyPersistentIndexes) { + proxyIndexes << proxyPersistentIndex; + Q_ASSERT(proxyPersistentIndex.isValid()); + const QPersistentModelIndex srcPersistentIndex = q->mapToSource(proxyPersistentIndex); + Q_ASSERT(srcPersistentIndex.isValid()); + layoutChangePersistentIndexes << srcPersistentIndex; + } } void QIdentityProxyModelPrivate::_q_sourceLayoutChanged(const QList &sourceParents, QAbstractItemModel::LayoutChangeHint hint) -- cgit v1.2.1 From 39e80062d0cf0c25b456bd89be827e50a6077efa Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 16 Dec 2016 12:48:18 -0800 Subject: Work around MSVC ABI stupidity in exporting inline members of base class In this case, the issue was ICC, when compiling QtQml: qv4sequenceobject.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: void __cdecl QList::replace(int,class QItemSelectionRange const &)" (__imp_?replace@?$QList@VQItemSelectionRange@@@@QEAAXHAEBVQItemSelectionRange@@@Z) referenced in function "public: static bool __cdecl QV4::QQmlSequence::deleteIndexedProperty(struct QV4::Managed *,unsigned int)" (?deleteIndexedProperty@?$QQmlSequence@VQItemSelection@@@QV4@@SA_NPEAUManaged@2@I@Z) This applies the same fix as qvector.h has had for ages due to QPolygon. Change-Id: I15b62e0f9cec482fbb40fffd1490d791db5056bc Reviewed-by: Marc Mutz Reviewed-by: Lars Knoll --- src/corelib/itemmodels/qitemselectionmodel.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'src/corelib/itemmodels') diff --git a/src/corelib/itemmodels/qitemselectionmodel.h b/src/corelib/itemmodels/qitemselectionmodel.h index 3d3cb00750..c22ac6dbe5 100644 --- a/src/corelib/itemmodels/qitemselectionmodel.h +++ b/src/corelib/itemmodels/qitemselectionmodel.h @@ -251,6 +251,24 @@ Q_DECLARE_OPERATORS_FOR_FLAGS(QItemSelectionModel::SelectionFlags) // dummy implentation of qHash() necessary for instantiating QList::toSet() with MSVC inline uint qHash(const QItemSelectionRange &) { return 0; } +#ifdef Q_CC_MSVC + +/* + ### Qt 6: + ### This needs to be removed for next releases of Qt. It is a workaround for vc++ because + ### Qt exports QItemSelection that inherits QList. +*/ + +# ifndef Q_TEMPLATE_EXTERN +# if defined(QT_BUILD_CORE_LIB) +# define Q_TEMPLATE_EXTERN +# else +# define Q_TEMPLATE_EXTERN extern +# endif +# endif +Q_TEMPLATE_EXTERN template class Q_CORE_EXPORT QList; +#endif // Q_CC_MSVC + class Q_CORE_EXPORT QItemSelection : public QList { public: -- cgit v1.2.1