Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential crashes when layer tree insertion target group is deleted #60446

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Fix potential crashes when layer tree insertion target group is deleted #60446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96b8809
Always use switch for layer tree insertion method enum checks
nyalldawson Feb 5, 2025
7c5fd10
Fix potential crashes when layer tree insertion target group is deleted
nyalldawson Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions python/PyQt6/core/auto_generated/layertree/qgslayertreeregistrybridge.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ Tell others we have just added layers to the tree (used in QGIS to auto-select f

protected:


};

/************************************************************************
Expand Down
1 change: 1 addition & 0 deletions python/core/auto_generated/layertree/qgslayertreeregistrybridge.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ Tell others we have just added layers to the tree (used in QGIS to auto-select f

protected:


};

/************************************************************************
Expand Down
54 changes: 30 additions & 24 deletions src/app/layers/qgsapplayerhandling.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,42 +719,48 @@ QList<QgsMapLayer *> QgsAppLayerHandling::addSublayers( const QList<QgsProviderS
if ( !groupName.isEmpty() )
{
int index { 0 };
if ( QgsProject::instance()->layerTreeRegistryBridge()->layerInsertionMethod() == Qgis::LayerTreeInsertionMethod::TopOfTree )
switch ( QgsProject::instance()->layerTreeRegistryBridge()->layerInsertionMethod() )
{
group = QgsProject::instance()->layerTreeRoot()->insertGroup( 0, groupName );
}
else
{
QgsLayerTreeNode *currentNode { QgisApp::instance()->layerTreeView()->currentNode() };
if ( currentNode && currentNode->parent() )
case Qgis::LayerTreeInsertionMethod::TopOfTree:
{
if ( QgsLayerTree::isGroup( currentNode ) )
{
group = qobject_cast<QgsLayerTreeGroup *>( currentNode )->insertGroup( 0, groupName );
}
else if ( QgsLayerTree::isLayer( currentNode ) )
group = QgsProject::instance()->layerTreeRoot()->insertGroup( 0, groupName );
break;
}
case Qgis::LayerTreeInsertionMethod::AboveInsertionPoint:
case Qgis::LayerTreeInsertionMethod::OptimalInInsertionGroup:
{
QgsLayerTreeNode *currentNode { QgisApp::instance()->layerTreeView()->currentNode() };
if ( currentNode && currentNode->parent() )
{
const QList<QgsLayerTreeNode *> currentNodeSiblings { currentNode->parent()->children() };
int nodeIdx { 0 };
for ( const QgsLayerTreeNode *child : std::as_const( currentNodeSiblings ) )
if ( QgsLayerTree::isGroup( currentNode ) )
{
group = qobject_cast<QgsLayerTreeGroup *>( currentNode )->insertGroup( 0, groupName );
}
else if ( QgsLayerTree::isLayer( currentNode ) )
{
nodeIdx++;
if ( child == currentNode )
const QList<QgsLayerTreeNode *> currentNodeSiblings { currentNode->parent()->children() };
int nodeIdx { 0 };
for ( const QgsLayerTreeNode *child : std::as_const( currentNodeSiblings ) )
{
index = nodeIdx;
break;
nodeIdx++;
if ( child == currentNode )
{
index = nodeIdx;
break;
}
}
group = qobject_cast<QgsLayerTreeGroup *>( currentNode->parent() )->insertGroup( index, groupName );
}
else
{
group = QgsProject::instance()->layerTreeRoot()->insertGroup( 0, groupName );
}
group = qobject_cast<QgsLayerTreeGroup *>( currentNode->parent() )->insertGroup( index, groupName );
}
else
{
group = QgsProject::instance()->layerTreeRoot()->insertGroup( 0, groupName );
}
}
else
{
group = QgsProject::instance()->layerTreeRoot()->insertGroup( 0, groupName );
break;
}
}
}
Expand Down
41 changes: 29 additions & 12 deletions src/core/layertree/qgslayertreeregistrybridge.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ QgsLayerTreeRegistryBridge::QgsLayerTreeRegistryBridge( QgsLayerTreeGroup *root,
, mRegistryRemovingLayers( false )
, mEnabled( true )
, mNewLayersVisible( true )
, mInsertionPoint( root, 0 )
, mInsertionPointGroup( root )
{
connect( mProject, &QgsProject::legendLayersAdded, this, &QgsLayerTreeRegistryBridge::layersAdded );
connect( mProject, qOverload<const QStringList &>( &QgsProject::layersWillBeRemoved ), this, &QgsLayerTreeRegistryBridge::layersWillBeRemoved );
Expand All @@ -39,13 +39,14 @@ QgsLayerTreeRegistryBridge::QgsLayerTreeRegistryBridge( QgsLayerTreeGroup *root,

void QgsLayerTreeRegistryBridge::setLayerInsertionPoint( QgsLayerTreeGroup *parentGroup, int index )
{
mInsertionPoint.group = parentGroup;
mInsertionPoint.position = index;
mInsertionPointGroup = parentGroup;
mInsertionPointPosition = index;
}

void QgsLayerTreeRegistryBridge::setLayerInsertionPoint( const InsertionPoint &insertionPoint )
{
mInsertionPoint = insertionPoint;
mInsertionPointGroup = insertionPoint.group;
mInsertionPointPosition = insertionPoint.position;
}

void QgsLayerTreeRegistryBridge::layersAdded( const QList<QgsMapLayer *> &layers )
Expand All @@ -57,13 +58,24 @@ void QgsLayerTreeRegistryBridge::layersAdded( const QList<QgsMapLayer *> &layers
for ( QgsMapLayer *layer : layers )
{
QgsLayerTreeLayer *nodeLayer;
if ( mInsertionMethod == Qgis::LayerTreeInsertionMethod::OptimalInInsertionGroup )
switch ( mInsertionMethod )
{
nodeLayer = QgsLayerTreeUtils::insertLayerAtOptimalPlacement( mInsertionPoint.group, layer );
}
else
{
nodeLayer = new QgsLayerTreeLayer( layer );
case Qgis::LayerTreeInsertionMethod::OptimalInInsertionGroup:
{
QgsLayerTreeGroup *targetGroup = mInsertionPointGroup;
if ( !targetGroup )
targetGroup = mRoot;

nodeLayer = QgsLayerTreeUtils::insertLayerAtOptimalPlacement( targetGroup, layer );
break;
}

case Qgis::LayerTreeInsertionMethod::AboveInsertionPoint:
case Qgis::LayerTreeInsertionMethod::TopOfTree:
{
nodeLayer = new QgsLayerTreeLayer( layer );
break;
}
}

nodeLayer->setItemVisibilityChecked( mNewLayersVisible );
Expand All @@ -82,8 +94,13 @@ void QgsLayerTreeRegistryBridge::layersAdded( const QList<QgsMapLayer *> &layers
switch ( mInsertionMethod )
{
case Qgis::LayerTreeInsertionMethod::AboveInsertionPoint:
mInsertionPoint.group->insertChildNodes( mInsertionPoint.position, nodes );
break;
if ( QgsLayerTreeGroup *group = mInsertionPointGroup )
{
group->insertChildNodes( mInsertionPointPosition, nodes );
break;
}
// if no group for the insertion point, then insert into root instead
[[fallthrough]];
case Qgis::LayerTreeInsertionMethod::TopOfTree:
mRoot->insertChildNodes( 0, nodes );
break;
Expand Down
9 changes: 6 additions & 3 deletions src/core/layertree/qgslayertreeregistrybridge.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@

#include <QObject>
#include <QStringList>
#include <QPointer>

#include "qgis_core.h"
#include "qgis_sip.h"
#include "qgis.h"
#include "qgslayertreegroup.h"

class QgsLayerTreeGroup;
class QgsLayerTreeNode;
class QgsMapLayer;
class QgsProject;
Expand Down Expand Up @@ -57,7 +58,7 @@ class CORE_EXPORT QgsLayerTreeRegistryBridge : public QObject
InsertionPoint( QgsLayerTreeGroup *group, int position )
: group( group ), position( position ) {}

QgsLayerTreeGroup *group = nullptr;
QgsLayerTreeGroup *group;
int position = 0;
};

Expand Down Expand Up @@ -120,7 +121,9 @@ class CORE_EXPORT QgsLayerTreeRegistryBridge : public QObject
bool mEnabled;
bool mNewLayersVisible;

InsertionPoint mInsertionPoint;
QPointer< QgsLayerTreeGroup > mInsertionPointGroup;
int mInsertionPointPosition = 0;

Qgis::LayerTreeInsertionMethod mInsertionMethod = Qgis::LayerTreeInsertionMethod::AboveInsertionPoint;
};

Expand Down
Loading