diff --git a/python/core/qgsmaplayerstore.sip b/python/core/qgsmaplayerstore.sip index a3c7df1753b..ea3b27e2623 100644 --- a/python/core/qgsmaplayerstore.sip +++ b/python/core/qgsmaplayerstore.sip @@ -77,7 +77,8 @@ class QgsMapLayerStore : QObject %End - QList addMapLayers( const QList &layers /Transfer/ ); + QList addMapLayers( const QList &layers /Transfer/); + %Docstring \brief Add a list of ``layers`` to the store. Ownership of the layers is transferred @@ -86,6 +87,9 @@ class QgsMapLayerStore : QObject The layersAdded() and layerWasAdded() signals will always be emitted. \param layers A list of layer which should be added to the store. + \param takeOwnership Ownership will be transferred to the layer store. + If you specify false here you have take care of deleting + the layers yourself. Not available in Python. :return: a list of the map layers that were added successfully. If a layer is invalid, or already exists in the store, @@ -95,7 +99,8 @@ class QgsMapLayerStore : QObject :rtype: list of QgsMapLayer %End - QgsMapLayer *addMapLayer( QgsMapLayer *layer /Transfer/ ); + QgsMapLayer *addMapLayer( QgsMapLayer *layer /Transfer/); + %Docstring \brief Add a ``layer`` to the store. Ownership of the layer is transferred to the @@ -106,6 +111,9 @@ class QgsMapLayerStore : QObject addMapLayers() instead. \param layer A layer to add to the store + \param takeOwnership Ownership will be transferred to the layer store. + If you specify false here you have take care of deleting + the layers yourself. Not available in Python. :return: None if unable to add layer, otherwise pointer to newly added layer @@ -137,6 +145,19 @@ class QgsMapLayerStore : QObject %End void removeMapLayers( const QList &layers ); +%Docstring + \brief + Remove a set of ``layers`` from the store. + + The specified layers will be removed from the store. + These layers will also be deleted. + + \param layers A list of layers to remove. Null pointers are ignored. + +.. seealso:: takeMapLayer() +.. seealso:: removeMapLayer() +.. seealso:: removeAllMapLayers() +%End void removeMapLayer( const QString &id ); %Docstring diff --git a/src/core/qgsmaplayerstore.cpp b/src/core/qgsmaplayerstore.cpp index 671e88ff3f5..01e489639cf 100644 --- a/src/core/qgsmaplayerstore.cpp +++ b/src/core/qgsmaplayerstore.cpp @@ -50,8 +50,7 @@ QList QgsMapLayerStore::mapLayersByName( const QString &layerName return myResultList; } -QList QgsMapLayerStore::addMapLayers( - const QList &layers ) +QList QgsMapLayerStore::addMapLayers( const QList &layers, bool takeOwnership ) { QList myResultList; Q_FOREACH ( QgsMapLayer *myLayer, layers ) @@ -66,7 +65,10 @@ QList QgsMapLayerStore::addMapLayers( { mMapLayers[myLayer->id()] = myLayer; myResultList << mMapLayers[myLayer->id()]; - myLayer->setParent( this ); + if ( takeOwnership ) + { + myLayer->setParent( this ); + } connect( myLayer, &QObject::destroyed, this, &QgsMapLayerStore::onMapLayerDeleted ); emit layerWasAdded( myLayer ); } @@ -79,10 +81,10 @@ QList QgsMapLayerStore::addMapLayers( } QgsMapLayer * -QgsMapLayerStore::addMapLayer( QgsMapLayer *layer ) +QgsMapLayerStore::addMapLayer( QgsMapLayer *layer, bool takeOwnership ) { QList addedLayers; - addedLayers = addMapLayers( QList() << layer ); + addedLayers = addMapLayers( QList() << layer, takeOwnership ); return addedLayers.isEmpty() ? nullptr : addedLayers[0]; } diff --git a/src/core/qgsmaplayerstore.h b/src/core/qgsmaplayerstore.h index d2ffb4788c3..d8d72656028 100644 --- a/src/core/qgsmaplayerstore.h +++ b/src/core/qgsmaplayerstore.h @@ -124,6 +124,9 @@ class CORE_EXPORT QgsMapLayerStore : public QObject * The layersAdded() and layerWasAdded() signals will always be emitted. * * \param layers A list of layer which should be added to the store. + * \param takeOwnership Ownership will be transferred to the layer store. + * If you specify false here you have take care of deleting + * the layers yourself. Not available in Python. * * \returns a list of the map layers that were added * successfully. If a layer is invalid, or already exists in the store, @@ -131,7 +134,8 @@ class CORE_EXPORT QgsMapLayerStore : public QObject * * \see addMapLayer() */ - QList addMapLayers( const QList &layers SIP_TRANSFER ); + QList addMapLayers( const QList &layers SIP_TRANSFER, + bool takeOwnership SIP_PYARGREMOVE = true ); /** * \brief @@ -143,6 +147,9 @@ class CORE_EXPORT QgsMapLayerStore : public QObject * addMapLayers() instead. * * \param layer A layer to add to the store + * \param takeOwnership Ownership will be transferred to the layer store. + * If you specify false here you have take care of deleting + * the layers yourself. Not available in Python. * * \returns nullptr if unable to add layer, otherwise pointer to newly added layer * @@ -151,7 +158,8 @@ class CORE_EXPORT QgsMapLayerStore : public QObject * \note Use addMapLayers() if adding more than one layer at a time. * \see addMapLayers() */ - QgsMapLayer *addMapLayer( QgsMapLayer *layer SIP_TRANSFER ); + QgsMapLayer *addMapLayer( QgsMapLayer *layer SIP_TRANSFER, + bool takeOwnership SIP_PYARGREMOVE = true ); /** * \brief @@ -182,7 +190,6 @@ class CORE_EXPORT QgsMapLayerStore : public QObject * \see removeMapLayer() * \see removeAllMapLayers() */ - //TODO QGIS 3.0 - add PyName alias to avoid list type conversion error void removeMapLayers( const QList &layers ); /** diff --git a/src/core/qgsproject.cpp b/src/core/qgsproject.cpp index e24712650ee..35ce7aca154 100644 --- a/src/core/qgsproject.cpp +++ b/src/core/qgsproject.cpp @@ -46,6 +46,7 @@ #include "qgssettings.h" #include "qgsmaplayerlistutils.h" #include "qgslayoutmanager.h" +#include "qgsmaplayerstore.h" #include #include @@ -322,6 +323,7 @@ void removeKey_( const QString &scope, QgsProject::QgsProject( QObject *parent ) : QObject( parent ) + , mLayerStore( new QgsMapLayerStore( this ) ) , mBadLayerHandler( new QgsProjectBadLayerHandler() ) , mSnappingConfig( this ) , mRelationManager( new QgsRelationManager( this ) ) @@ -343,6 +345,21 @@ QgsProject::QgsProject( QObject *parent ) connect( this, &QgsProject::layersAdded, this, &QgsProject::onMapLayersAdded ); connect( this, &QgsProject::layersRemoved, this, [ = ] { cleanTransactionGroups(); } ); connect( this, static_cast < void ( QgsProject::* )( const QList & ) >( &QgsProject::layersWillBeRemoved ), this, &QgsProject::onMapLayersRemoved ); + + // proxy map layer store signals to this + connect( mLayerStore.get(), static_cast( &QgsMapLayerStore::layersWillBeRemoved ), + this, static_cast( &QgsProject::layersWillBeRemoved ) ); + connect( mLayerStore.get(), static_cast & )>( &QgsMapLayerStore::layersWillBeRemoved ), + this, static_cast & )>( &QgsProject::layersWillBeRemoved ) ); + connect( mLayerStore.get(), static_cast( &QgsMapLayerStore::layerWillBeRemoved ), + this, static_cast( &QgsProject::layerWillBeRemoved ) ); + connect( mLayerStore.get(), static_cast( &QgsMapLayerStore::layerWillBeRemoved ), + this, static_cast( &QgsProject::layerWillBeRemoved ) ); + connect( mLayerStore.get(), static_cast( &QgsMapLayerStore::layersRemoved ), this, &QgsProject::layersRemoved ); + connect( mLayerStore.get(), &QgsMapLayerStore::layerRemoved, this, &QgsProject::layerRemoved ); + connect( mLayerStore.get(), &QgsMapLayerStore::allLayersRemoved, this, &QgsProject::removeAll ); + connect( mLayerStore.get(), &QgsMapLayerStore::layersAdded, this, &QgsProject::layersAdded ); + connect( mLayerStore.get(), &QgsMapLayerStore::layerWasAdded, this, &QgsProject::layerWasAdded ); } @@ -884,7 +901,8 @@ bool QgsProject::read() // Resolve references to other vector layers // Needs to be done here once all dependent layers are loaded - for ( QMap::iterator it = mMapLayers.begin(); it != mMapLayers.end(); it++ ) + QMap layers = mLayerStore->mapLayers(); + for ( QMap::iterator it = layers.begin(); it != layers.end(); it++ ) { if ( QgsVectorLayer *vl = qobject_cast( it.value() ) ) vl->resolveReferences( this ); @@ -2066,31 +2084,23 @@ QMap, QgsTransactionGroup *> QgsProject::transactionGrou // -// QgsMapLayerRegistry methods +// QgsMapLayerStore methods // int QgsProject::count() const { - return mMapLayers.size(); + return mLayerStore->count(); } QgsMapLayer *QgsProject::mapLayer( const QString &layerId ) const { - return mMapLayers.value( layerId ); + return mLayerStore->mapLayer( layerId ); } QList QgsProject::mapLayersByName( const QString &layerName ) const { - QList myResultList; - Q_FOREACH ( QgsMapLayer *layer, mMapLayers ) - { - if ( layer->name() == layerName ) - { - myResultList << layer; - } - } - return myResultList; + return mLayerStore->mapLayersByName( layerName ); } QList QgsProject::addMapLayers( @@ -2098,31 +2108,9 @@ QList QgsProject::addMapLayers( bool addToLegend, bool takeOwnership ) { - QList myResultList; - Q_FOREACH ( QgsMapLayer *myLayer, layers ) - { - if ( !myLayer || !myLayer->isValid() ) - { - QgsDebugMsg( "Cannot add invalid layers" ); - continue; - } - //check the layer is not already registered! - if ( !mMapLayers.contains( myLayer->id() ) ) - { - mMapLayers[myLayer->id()] = myLayer; - myResultList << mMapLayers[myLayer->id()]; - if ( takeOwnership ) - { - myLayer->setParent( this ); - } - connect( myLayer, &QObject::destroyed, this, &QgsProject::onMapLayerDeleted ); - emit layerWasAdded( myLayer ); - } - } + QList myResultList = mLayerStore->addMapLayers( layers, takeOwnership ); if ( !myResultList.isEmpty() ) { - emit layersAdded( myResultList ); - if ( addToLegend ) emit legendLayersAdded( myResultList ); } @@ -2141,116 +2129,47 @@ QgsProject::addMapLayer( QgsMapLayer *layer, void QgsProject::removeMapLayers( const QStringList &layerIds ) { - QList layers; - Q_FOREACH ( const QString &myId, layerIds ) - { - layers << mMapLayers.value( myId ); - } - - removeMapLayers( layers ); + mLayerStore->removeMapLayers( layerIds ); } void QgsProject::removeMapLayers( const QList &layers ) { - if ( layers.isEmpty() ) - return; - - QStringList layerIds; - QList layerList; - - Q_FOREACH ( QgsMapLayer *layer, layers ) - { - // check layer and the registry contains it - if ( layer && mMapLayers.contains( layer->id() ) ) - { - layerIds << layer->id(); - layerList << layer; - } - } - - if ( layerIds.isEmpty() ) - return; - - emit layersWillBeRemoved( layerIds ); - emit layersWillBeRemoved( layerList ); - - Q_FOREACH ( QgsMapLayer *lyr, layerList ) - { - QString myId( lyr->id() ); - emit layerWillBeRemoved( myId ); - emit layerWillBeRemoved( lyr ); - mMapLayers.remove( myId ); - if ( lyr->parent() == this ) - { - delete lyr; - } - emit layerRemoved( myId ); - } - - emit layersRemoved( layerIds ); + mLayerStore->removeMapLayers( layers ); } void QgsProject::removeMapLayer( const QString &layerId ) { - removeMapLayers( QList() << mMapLayers.value( layerId ) ); + mLayerStore->removeMapLayer( layerId ); } void QgsProject::removeMapLayer( QgsMapLayer *layer ) { - if ( layer ) - removeMapLayers( QList() << layer ); + mLayerStore->removeMapLayer( layer ); } QgsMapLayer *QgsProject::takeMapLayer( QgsMapLayer *layer ) { - if ( !layer ) - return nullptr; - - if ( mMapLayers.contains( layer->id() ) ) - { - emit layersWillBeRemoved( QStringList() << layer->id() ); - emit layersWillBeRemoved( QList() << layer ); - emit layerWillBeRemoved( layer->id() ); - emit layerWillBeRemoved( layer ); - - mMapLayers.remove( layer->id() ); - layer->setParent( nullptr ); - emit layerRemoved( layer->id() ); - emit layersRemoved( QStringList() << layer->id() ); - return layer; - } - return nullptr; //don't return layer - it wasn't owned and accordingly we aren't transferring ownership + return mLayerStore->takeMapLayer( layer ); } void QgsProject::removeAllMapLayers() { - emit removeAll(); - // now let all observers know to clear themselves, - // and then consequently any of their map legends - removeMapLayers( mMapLayers.keys() ); - mMapLayers.clear(); + mLayerStore->removeAllMapLayers(); } void QgsProject::reloadAllLayers() { - Q_FOREACH ( QgsMapLayer *layer, mMapLayers ) + QMap layers = mLayerStore->mapLayers(); + QMap::const_iterator it = layers.constBegin(); + for ( ; it != layers.constEnd(); ++it ) { - layer->reload(); - } -} - -void QgsProject::onMapLayerDeleted( QObject *obj ) -{ - QString id = mMapLayers.key( static_cast( obj ) ); - - if ( !id.isNull() ) - { - QgsDebugMsg( QString( "Map layer deleted without unregistering! %1" ).arg( id ) ); - mMapLayers.remove( id ); + it.value()->reload(); } } QMap QgsProject::mapLayers() const { - return mMapLayers; + return mLayerStore->mapLayers(); } + + diff --git a/src/core/qgsproject.h b/src/core/qgsproject.h index 2a41ff401eb..bba53063782 100644 --- a/src/core/qgsproject.h +++ b/src/core/qgsproject.h @@ -38,6 +38,7 @@ #include "qgscoordinatereferencesystem.h" #include "qgsprojectproperty.h" #include "qgsmaplayer.h" +#include "qgsmaplayerstore.h" class QFileInfo; class QDomDocument; @@ -567,17 +568,7 @@ class CORE_EXPORT QgsProject : public QObject, public QgsExpressionContextGenera template QVector layers() const { - QVector layers; - QMap::const_iterator layerIt = mMapLayers.constBegin(); - for ( ; layerIt != mMapLayers.constEnd(); ++layerIt ) - { - T tLayer = qobject_cast( layerIt.value() ); - if ( tLayer ) - { - layers << tLayer; - } - } - return layers; + return mLayerStore->layers(); } /** @@ -968,8 +959,6 @@ class CORE_EXPORT QgsProject : public QObject, public QgsExpressionContextGenera void onMapLayersRemoved( const QList &layers ); void cleanTransactionGroups( bool force = false ); - void onMapLayerDeleted( QObject *obj ); - private: static QgsProject *sProject; @@ -1002,7 +991,7 @@ class CORE_EXPORT QgsProject : public QObject, public QgsExpressionContextGenera //! \note not available in Python bindings void loadEmbeddedNodes( QgsLayerTreeGroup *group ); - QMap mMapLayers; + std::unique_ptr< QgsMapLayerStore > mLayerStore; QString mErrorMessage; diff --git a/tests/src/python/test_qgsmaplayerregistry.py b/tests/src/python/test_qgsmaplayerregistry.py index 26eb14730ae..1ea2aa1d121 100644 --- a/tests/src/python/test_qgsmaplayerregistry.py +++ b/tests/src/python/test_qgsmaplayerregistry.py @@ -511,7 +511,7 @@ class TestQgsProjectMapLayers(unittest.TestCase): # add one layer to project p.addMapLayer(l1) self.assertEqual(p.mapLayers(), {l1.id(): l1}) - self.assertEqual(l1.parent(), p) + self.assertEqual(l1.parent().parent(), p) # try taking some layers which don't exist in project self.assertFalse(p.takeMapLayer(None))