From 992c9568cda4085eb751a9b1b7b94100884940fe Mon Sep 17 00:00:00 2001 From: Jacky Volpes Date: Tue, 26 Nov 2024 17:08:06 +0100 Subject: [PATCH] fix(layertreeview): add checks on nullptr to avoid segfaults check calls to layerTreeModel(), selectedModel(), and use of mProxyModel --- src/gui/layertree/qgslayertreeview.cpp | 167 +++++++++++++++------- tests/src/python/test_qgslayertreeview.py | 11 ++ 2 files changed, 130 insertions(+), 48 deletions(-) diff --git a/src/gui/layertree/qgslayertreeview.cpp b/src/gui/layertree/qgslayertreeview.cpp index 68d77f6928a3..5db4bac1619b 100644 --- a/src/gui/layertree/qgslayertreeview.cpp +++ b/src/gui/layertree/qgslayertreeview.cpp @@ -152,13 +152,14 @@ QgsMapLayer *QgsLayerTreeView::currentLayer() const void QgsLayerTreeView::setCurrentLayer( QgsMapLayer *layer ) { - if ( !layer ) + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layer || !layerModel ) { setCurrentIndex( QModelIndex() ); return; } - QgsLayerTreeLayer *nodeLayer = layerTreeModel()->rootGroup()->findLayer( layer->id() ); + QgsLayerTreeLayer *nodeLayer = layerModel->rootGroup()->findLayer( layer->id() ); if ( !nodeLayer ) return; @@ -167,9 +168,10 @@ void QgsLayerTreeView::setCurrentLayer( QgsMapLayer *layer ) void QgsLayerTreeView::setLayerVisible( QgsMapLayer *layer, bool visible ) { - if ( !layer ) + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layer || !layerModel ) return; - QgsLayerTreeLayer *nodeLayer = layerTreeModel()->rootGroup()->findLayer( layer->id() ); + QgsLayerTreeLayer *nodeLayer = layerModel->rootGroup()->findLayer( layer->id() ); if ( !nodeLayer ) return; nodeLayer->setItemVisibilityChecked( visible ); @@ -199,17 +201,18 @@ void QgsLayerTreeView::contextMenuEvent( QContextMenuEvent *event ) void QgsLayerTreeView::modelRowsInserted( const QModelIndex &index, int start, int end ) { QgsLayerTreeNode *parentNode = index2node( index ); - if ( !parentNode ) + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !parentNode || !layerModel ) return; // Embedded widgets - replace placeholders in the model by actual widgets - if ( layerTreeModel()->testFlag( QgsLayerTreeModel::UseEmbeddedWidgets ) && QgsLayerTree::isLayer( parentNode ) ) + if ( layerModel->testFlag( QgsLayerTreeModel::UseEmbeddedWidgets ) && QgsLayerTree::isLayer( parentNode ) ) { QgsLayerTreeLayer *nodeLayer = QgsLayerTree::toLayer( parentNode ); if ( QgsMapLayer *layer = nodeLayer->layer() ) { const int widgetsCount = layer->customProperty( QStringLiteral( "embeddedWidgets/count" ), 0 ).toInt(); - QList legendNodes = layerTreeModel()->layerLegendNodes( nodeLayer, true ); + QList legendNodes = layerModel->layerLegendNodes( nodeLayer, true ); for ( int i = 0; i < widgetsCount; ++i ) { const QString providerId = layer->customProperty( QStringLiteral( "embeddedWidgets/%1/id" ).arg( i ) ).toString(); @@ -247,7 +250,7 @@ void QgsLayerTreeView::modelRowsInserted( const QModelIndex &index, int start, i if ( expandedNodeKeys.isEmpty() ) return; - const auto constLayerLegendNodes = layerTreeModel()->layerLegendNodes( QgsLayerTree::toLayer( parentNode ), true ); + const auto constLayerLegendNodes = layerModel->layerLegendNodes( QgsLayerTree::toLayer( parentNode ), true ); for ( QgsLayerTreeModelLegendNode *legendNode : constLayerLegendNodes ) { const QString ruleKey = legendNode->data( QgsLayerTreeModelLegendNode::RuleKeyRole ).toString(); @@ -300,6 +303,10 @@ void QgsLayerTreeView::updateExpandedStateToNode( const QModelIndex &index ) void QgsLayerTreeView::onCurrentChanged() { + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return; + QgsMapLayer *layerCurrent = layerForIndex( currentIndex() ); const QString layerCurrentID = layerCurrent ? layerCurrent->id() : QString(); if ( mCurrentLayerID == layerCurrentID ) @@ -309,7 +316,7 @@ void QgsLayerTreeView::onCurrentChanged() QModelIndex proxyModelNodeLayerIndex; if ( layerCurrent ) { - QgsLayerTreeLayer *nodeLayer = layerTreeModel()->rootGroup()->findLayer( layerCurrentID ); + QgsLayerTreeLayer *nodeLayer = layerModel->rootGroup()->findLayer( layerCurrentID ); if ( nodeLayer ) proxyModelNodeLayerIndex = node2index( nodeLayer ); } @@ -317,12 +324,12 @@ void QgsLayerTreeView::onCurrentChanged() if ( ! proxyModelNodeLayerIndex.isValid() ) { mCurrentLayerID = QString(); - layerTreeModel()->setCurrentIndex( QModelIndex() ); + layerModel->setCurrentIndex( QModelIndex() ); } else { mCurrentLayerID = layerCurrentID; - layerTreeModel()->setCurrentIndex( mProxyModel->mapToSource( proxyModelNodeLayerIndex ) ); + layerModel->setCurrentIndex( mProxyModel->mapToSource( proxyModelNodeLayerIndex ) ); } //checkModel(); @@ -339,12 +346,13 @@ void QgsLayerTreeView::onExpandedChanged( QgsLayerTreeNode *node, bool expanded void QgsLayerTreeView::onCustomPropertyChanged( QgsLayerTreeNode *node, const QString &key ) { - if ( key != QLatin1String( "expandedLegendNodes" ) || !QgsLayerTree::isLayer( node ) ) + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( key != QLatin1String( "expandedLegendNodes" ) || !QgsLayerTree::isLayer( node ) || !layerModel ) return; const QSet expandedLegendNodes = qgis::listToSet( node->customProperty( QStringLiteral( "expandedLegendNodes" ) ).toStringList() ); - const QList legendNodes = layerTreeModel()->layerLegendNodes( QgsLayerTree::toLayer( node ), true ); + const QList legendNodes = layerModel->layerLegendNodes( QgsLayerTree::toLayer( node ), true ); for ( QgsLayerTreeModelLegendNode *legendNode : legendNodes ) { const QString key = legendNode->data( QgsLayerTreeModelLegendNode::RuleKeyRole ).toString(); @@ -355,7 +363,10 @@ void QgsLayerTreeView::onCustomPropertyChanged( QgsLayerTreeNode *node, const QS void QgsLayerTreeView::onModelReset() { - updateExpandedStateFromNode( layerTreeModel()->rootGroup() ); + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return; + updateExpandedStateFromNode( layerModel->rootGroup() ); //checkModel(); } @@ -393,7 +404,10 @@ QgsMapLayer *QgsLayerTreeView::layerForIndex( const QModelIndex &index ) const QgsLayerTreeNode *QgsLayerTreeView::currentNode() const { - return index2node( selectionModel()->currentIndex() ); + if ( QItemSelectionModel *selectModel = selectionModel() ) + return index2node( selectModel->currentIndex() ); + else + return nullptr; } QgsLayerTreeGroup *QgsLayerTreeView::currentGroupNode() const @@ -408,11 +422,14 @@ QgsLayerTreeGroup *QgsLayerTreeView::currentGroupNode() const return QgsLayerTree::toGroup( parent ); } - if ( QgsLayerTreeModelLegendNode *legendNode = index2legendNode( selectionModel()->currentIndex() ) ) + if ( QItemSelectionModel *selectModel = selectionModel() ) { - QgsLayerTreeLayer *parent = legendNode->layerNode(); - if ( QgsLayerTree::isGroup( parent->parent() ) ) - return QgsLayerTree::toGroup( parent->parent() ); + if ( QgsLayerTreeModelLegendNode *legendNode = index2legendNode( selectModel->currentIndex() ) ) + { + QgsLayerTreeLayer *parent = legendNode->layerNode(); + if ( QgsLayerTree::isGroup( parent->parent() ) ) + return QgsLayerTree::toGroup( parent->parent() ); + } } return nullptr; @@ -420,18 +437,28 @@ QgsLayerTreeGroup *QgsLayerTreeView::currentGroupNode() const QgsLayerTreeModelLegendNode *QgsLayerTreeView::currentLegendNode() const { - return index2legendNode( selectionModel()->currentIndex() ); + if ( QItemSelectionModel *selectModel = selectionModel() ) + return index2legendNode( selectModel->currentIndex() ); + else + return nullptr; } QList QgsLayerTreeView::selectedNodes( bool skipInternal ) const { + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return QList(); + QModelIndexList mapped; - const QModelIndexList selected = selectionModel()->selectedIndexes(); - mapped.reserve( selected.size() ); - for ( const QModelIndex &index : selected ) - mapped << mProxyModel->mapToSource( index ); + if ( QItemSelectionModel *selectModel = selectionModel() ) + { + const QModelIndexList selected = selectModel->selectedIndexes(); + mapped.reserve( selected.size() ); + for ( const QModelIndex &index : selected ) + mapped << mProxyModel->mapToSource( index ); + } - return layerTreeModel()->indexes2nodes( mapped, skipInternal ); + return layerModel->indexes2nodes( mapped, skipInternal ); } QList QgsLayerTreeView::selectedLayerNodes() const @@ -463,12 +490,17 @@ QList QgsLayerTreeView::selectedLayers() const QList QgsLayerTreeView::selectedLegendNodes() const { QList res; - const QModelIndexList selected = selectionModel()->selectedIndexes(); + QgsLayerTreeModel *layerModel = layerTreeModel(); + QItemSelectionModel *selectModel = selectionModel(); + if ( !layerModel || !selectModel ) + return res; + + const QModelIndexList selected = selectModel->selectedIndexes(); res.reserve( selected.size() ); for ( const QModelIndex &index : selected ) { const QModelIndex &modelIndex = mProxyModel->mapToSource( index ); - if ( QgsLayerTreeModelLegendNode *node = layerTreeModel()->index2legendNode( modelIndex ) ) + if ( QgsLayerTreeModelLegendNode *node = layerModel->index2legendNode( modelIndex ) ) { res.push_back( node ); } @@ -480,12 +512,17 @@ QList QgsLayerTreeView::selectedLegendNodes() con QList QgsLayerTreeView::selectedLayersRecursive() const { QModelIndexList mapped; - const QModelIndexList selected = selectionModel()->selectedIndexes(); + QgsLayerTreeModel *layerModel = layerTreeModel(); + QItemSelectionModel *selectModel = selectionModel(); + if ( !layerModel || !selectModel ) + return QList(); + + const QModelIndexList selected = selectModel->selectedIndexes(); mapped.reserve( selected.size() ); for ( const QModelIndex &index : selected ) mapped << mProxyModel->mapToSource( index ); - const QList nodes = layerTreeModel()->indexes2nodes( mapped, false ); + const QList nodes = layerModel->indexes2nodes( mapped, false ); const QSet layersSet = QgsLayerTreeUtils::collectMapLayersRecursive( nodes ); return qgis::setToList( layersSet ); } @@ -525,9 +562,12 @@ QStringList QgsLayerTreeView::viewOnlyCustomProperties() void QgsLayerTreeView::refreshLayerSymbology( const QString &layerId ) { - QgsLayerTreeLayer *nodeLayer = layerTreeModel()->rootGroup()->findLayer( layerId ); + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return; + QgsLayerTreeLayer *nodeLayer = layerModel->rootGroup()->findLayer( layerId ); if ( nodeLayer ) - layerTreeModel()->refreshLayerLegend( nodeLayer ); + layerModel->refreshLayerLegend( nodeLayer ); } @@ -567,14 +607,20 @@ static void _expandAllNodes( QgsLayerTreeGroup *parent, bool expanded, QgsLayerT void QgsLayerTreeView::expandAllNodes() { // unfortunately expandAll() does not emit expanded() signals - _expandAllNodes( layerTreeModel()->rootGroup(), true, layerTreeModel() ); + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return; + _expandAllNodes( layerModel->rootGroup(), true, layerModel ); expandAll(); } void QgsLayerTreeView::collapseAllNodes() { // unfortunately collapseAll() does not emit collapsed() signals - _expandAllNodes( layerTreeModel()->rootGroup(), false, layerTreeModel() ); + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return; + _expandAllNodes( layerModel->rootGroup(), false, layerModel ); collapseAll(); } @@ -584,9 +630,12 @@ void QgsLayerTreeView::setMessageBar( QgsMessageBar *messageBar ) return; mMessageBar = messageBar; + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return; if ( mMessageBar ) - connect( layerTreeModel(), &QgsLayerTreeModel::messageEmitted, this, + connect( layerModel, &QgsLayerTreeModel::messageEmitted, this, [ = ]( const QString & message, Qgis::MessageLevel level = Qgis::MessageLevel::Info, int duration = 5 ) { Q_UNUSED( duration ) @@ -597,6 +646,9 @@ void QgsLayerTreeView::setMessageBar( QgsMessageBar *messageBar ) void QgsLayerTreeView::setShowPrivateLayers( bool showPrivate ) { + if ( !mProxyModel ) + return; + mShowPrivateLayers = showPrivate; mProxyModel->setShowPrivateLayers( showPrivate ); } @@ -616,17 +668,21 @@ void QgsLayerTreeView::mouseDoubleClickEvent( QMouseEvent *event ) void QgsLayerTreeView::mouseReleaseEvent( QMouseEvent *event ) { + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return; + // we need to keep last mouse position in order to know whether to emit an indicator's clicked() signal // (the item delegate needs to know which indicator has been clicked) mLastReleaseMousePos = event->pos(); - const QgsLayerTreeModel::Flags oldFlags = layerTreeModel()->flags(); + const QgsLayerTreeModel::Flags oldFlags = layerModel->flags(); if ( event->modifiers() & Qt::ControlModifier ) - layerTreeModel()->setFlags( oldFlags | QgsLayerTreeModel::ActionHierarchical ); + layerModel->setFlags( oldFlags | QgsLayerTreeModel::ActionHierarchical ); else - layerTreeModel()->setFlags( oldFlags & ~QgsLayerTreeModel::ActionHierarchical ); + layerModel->setFlags( oldFlags & ~QgsLayerTreeModel::ActionHierarchical ); QTreeView::mouseReleaseEvent( event ); - layerTreeModel()->setFlags( oldFlags ); + layerModel->setFlags( oldFlags ); } void QgsLayerTreeView::keyPressEvent( QKeyEvent *event ) @@ -648,13 +704,16 @@ void QgsLayerTreeView::keyPressEvent( QKeyEvent *event ) } } - const QgsLayerTreeModel::Flags oldFlags = layerTreeModel()->flags(); + QgsLayerTreeModel *layerModel = layerTreeModel(); + if ( !layerModel ) + return; + const QgsLayerTreeModel::Flags oldFlags = layerModel->flags(); if ( event->modifiers() & Qt::ControlModifier ) - layerTreeModel()->setFlags( oldFlags | QgsLayerTreeModel::ActionHierarchical ); + layerModel->setFlags( oldFlags | QgsLayerTreeModel::ActionHierarchical ); else - layerTreeModel()->setFlags( oldFlags & ~QgsLayerTreeModel::ActionHierarchical ); + layerModel->setFlags( oldFlags & ~QgsLayerTreeModel::ActionHierarchical ); QTreeView::keyPressEvent( event ); - layerTreeModel()->setFlags( oldFlags ); + layerModel->setFlags( oldFlags ); } void QgsLayerTreeView::dragEnterEvent( QDragEnterEvent *event ) @@ -782,32 +841,44 @@ QgsLayerTreeProxyModel *QgsLayerTreeView::proxyModel() const QgsLayerTreeNode *QgsLayerTreeView::index2node( const QModelIndex &index ) const { - return layerTreeModel()->index2node( mProxyModel->mapToSource( index ) ); + if ( QgsLayerTreeModel *layerModel = layerTreeModel() ) + return layerModel->index2node( mProxyModel->mapToSource( index ) ); + return nullptr; } QModelIndex QgsLayerTreeView::node2index( QgsLayerTreeNode *node ) const { - return mProxyModel->mapFromSource( node2sourceIndex( node ) ); + if ( mProxyModel ) + return mProxyModel->mapFromSource( node2sourceIndex( node ) ); + return QModelIndex(); } QModelIndex QgsLayerTreeView::node2sourceIndex( QgsLayerTreeNode *node ) const { - return layerTreeModel()->node2index( node ); + if ( QgsLayerTreeModel *layerModel = layerTreeModel() ) + return layerModel->node2index( node ); + return QModelIndex(); } QgsLayerTreeModelLegendNode *QgsLayerTreeView::index2legendNode( const QModelIndex &index ) const { - return QgsLayerTreeModel::index2legendNode( mProxyModel->mapToSource( index ) ); + if ( mProxyModel ) + return QgsLayerTreeModel::index2legendNode( mProxyModel->mapToSource( index ) ); + return nullptr; } QModelIndex QgsLayerTreeView::legendNode2index( QgsLayerTreeModelLegendNode *legendNode ) { - return mProxyModel->mapFromSource( legendNode2sourceIndex( legendNode ) ); + if ( mProxyModel ) + return mProxyModel->mapFromSource( legendNode2sourceIndex( legendNode ) ); + return QModelIndex(); } QModelIndex QgsLayerTreeView::legendNode2sourceIndex( QgsLayerTreeModelLegendNode *legendNode ) { - return layerTreeModel()->legendNode2index( legendNode ); + if ( QgsLayerTreeModel *layerModel = layerTreeModel() ) + return layerModel->legendNode2index( legendNode ); + return QModelIndex(); } QgsLayerTreeProxyModel::QgsLayerTreeProxyModel( QgsLayerTreeModel *treeModel, QObject *parent ) diff --git a/tests/src/python/test_qgslayertreeview.py b/tests/src/python/test_qgslayertreeview.py index 9a243e55d600..fe175b9133b3 100644 --- a/tests/src/python/test_qgslayertreeview.py +++ b/tests/src/python/test_qgslayertreeview.py @@ -678,6 +678,17 @@ def test_selected_legend_nodes(self): model = QgsLayerTreeModel(root) layer_tree_layer = root.addLayer(layer) view = QgsLayerTreeView() + + self.assertEqual(view.selectedNodes(), []) + self.assertEqual(view.selectedLegendNodes(), []) + self.assertEqual(view.selectedLayersRecursive(), []) + self.assertEqual(view.selectedLayerNodes(), []) + + self.assertEqual(view.selectedLayers(), []) + self.assertIsNone(view.currentNode()) + self.assertIsNone(view.currentLegendNode()) + self.assertIsNone(view.currentGroupNode()) + view.setModel(model) legend_nodes = model.layerLegendNodes(layer_tree_layer)