Fix tree model loading from xml

qgis · Nov 10, 2020 · 8d7f540 · 8d7f540
1 parent 525aded
commit 8d7f540
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 18 deletions.
diff --git a/python/core/auto_generated/layertree/qgslayertreenode.sip.in b/python/core/auto_generated/layertree/qgslayertreenode.sip.in
@@ -98,6 +98,8 @@ Gets pointer to the parent. If parent is ``None``, the node is a root node
 Gets list of children of the node. Children are owned by the parent
 %End
 
+
+
     virtual QString name() const = 0;
 %Docstring
 Returns name of the node

diff --git a/src/app/qgsprojectlayergroupdialog.cpp b/src/app/qgsprojectlayergroupdialog.cpp
@@ -204,7 +204,10 @@ void QgsProjectLayerGroupDialog::changeProjectFile()
   QDomElement layerTreeElem = projectDom.documentElement().firstChildElement( QStringLiteral( "layer-tree-group" ) );
   if ( !layerTreeElem.isNull() )
   {
-    mRootGroup->readChildrenFromXml( layerTreeElem, QgsReadWriteContext() );
+    // Use a temporary tree to read the nodes to prevent signals being delivered to the models
+    QgsLayerTree tempTree;
+    tempTree.readChildrenFromXml( layerTreeElem,  QgsReadWriteContext() );
+    mRootGroup->insertChildNodes( -1, tempTree.abandonChildren() );
   }
   else
   {

diff --git a/src/core/layertree/qgslayertreemodel.cpp b/src/core/layertree/qgslayertreemodel.cpp
@@ -899,12 +899,12 @@ void QgsLayerTreeModel::connectToLayer( QgsLayerTreeLayer *nodeLayer )
   {
     // in order to connect to layer, we need to have it loaded.
     // keep an eye on the layer ID: once loaded, we will use it
-    connect( nodeLayer, &QgsLayerTreeLayer::layerLoaded, this, &QgsLayerTreeModel::nodeLayerLoaded );
+    connect( nodeLayer, &QgsLayerTreeLayer::layerLoaded, this, &QgsLayerTreeModel::nodeLayerLoaded, Qt::UniqueConnection );
     return;
   }
 
   // watch if the layer is getting removed
-  connect( nodeLayer, &QgsLayerTreeLayer::layerWillBeUnloaded, this, &QgsLayerTreeModel::nodeLayerWillBeUnloaded );
+  connect( nodeLayer, &QgsLayerTreeLayer::layerWillBeUnloaded, this, &QgsLayerTreeModel::nodeLayerWillBeUnloaded, Qt::UniqueConnection );
 
   if ( testFlag( ShowLegend ) )
   {
@@ -1003,14 +1003,13 @@ void QgsLayerTreeModel::connectToRootNode()
 {
   Q_ASSERT( mRootNode );
 
-  connect( mRootNode, &QgsLayerTreeNode::willAddChildren, this, &QgsLayerTreeModel::nodeWillAddChildren );
-  connect( mRootNode, &QgsLayerTreeNode::addedChildren, this, &QgsLayerTreeModel::nodeAddedChildren );
-  connect( mRootNode, &QgsLayerTreeNode::willRemoveChildren, this, &QgsLayerTreeModel::nodeWillRemoveChildren );
-  connect( mRootNode, &QgsLayerTreeNode::removedChildren, this, &QgsLayerTreeModel::nodeRemovedChildren );
-  connect( mRootNode, &QgsLayerTreeNode::visibilityChanged, this, &QgsLayerTreeModel::nodeVisibilityChanged );
-  connect( mRootNode, &QgsLayerTreeNode::nameChanged, this, &QgsLayerTreeModel::nodeNameChanged );
-
-  connect( mRootNode, &QgsLayerTreeNode::customPropertyChanged, this, &QgsLayerTreeModel::nodeCustomPropertyChanged );
+  connect( mRootNode, &QgsLayerTreeNode::willAddChildren, this, &QgsLayerTreeModel::nodeWillAddChildren, Qt::ConnectionType::UniqueConnection );
+  connect( mRootNode, &QgsLayerTreeNode::addedChildren, this, &QgsLayerTreeModel::nodeAddedChildren, Qt::ConnectionType::UniqueConnection );
+  connect( mRootNode, &QgsLayerTreeNode::willRemoveChildren, this, &QgsLayerTreeModel::nodeWillRemoveChildren, Qt::ConnectionType::UniqueConnection );
+  connect( mRootNode, &QgsLayerTreeNode::removedChildren, this, &QgsLayerTreeModel::nodeRemovedChildren, Qt::ConnectionType::UniqueConnection );
+  connect( mRootNode, &QgsLayerTreeNode::visibilityChanged, this, &QgsLayerTreeModel::nodeVisibilityChanged, Qt::ConnectionType::UniqueConnection );
+  connect( mRootNode, &QgsLayerTreeNode::nameChanged, this, &QgsLayerTreeModel::nodeNameChanged, Qt::ConnectionType::UniqueConnection );
+  connect( mRootNode, &QgsLayerTreeNode::customPropertyChanged, this, &QgsLayerTreeModel::nodeCustomPropertyChanged, Qt::ConnectionType::UniqueConnection );
 
   connectToLayers( mRootNode );
 }

diff --git a/src/core/layertree/qgslayertreenode.cpp b/src/core/layertree/qgslayertreenode.cpp
@@ -61,6 +61,7 @@ QList<QgsLayerTreeNode *> QgsLayerTreeNode::abandonChildren()
 
 void QgsLayerTreeNode::makeOrphan()
 {
+  disconnect();
   mParent = nullptr;
 }
 

diff --git a/src/core/layertree/qgslayertreenode.h b/src/core/layertree/qgslayertreenode.h
@@ -112,9 +112,18 @@ class CORE_EXPORT QgsLayerTreeNode : public QObject
     QList<QgsLayerTreeNode *> children() { return mChildren; }
     //! Gets list of children of the node. Children are owned by the parent
     QList<QgsLayerTreeNode *> children() const { return mChildren; } SIP_SKIP
-    //! Remove the childrens and sets their parent to null
+
+    /**
+     * Removes the childrens, disconnect all their signals and sets their parent to NULLPTR
+     * \return the removed children
+     * \since QGIS 3.16
+     */
     QList<QgsLayerTreeNode *> abandonChildren() SIP_SKIP;
-    //! Set parent to null
+
+    /**
+     * Sets parent to NULLPTR and disconnects all signals
+     * \since QGIS 3.16
+     */
     void makeOrphan() SIP_SKIP;
 
     /**

diff --git a/src/core/qgsproject.cpp b/src/core/qgsproject.cpp
@@ -1536,7 +1536,10 @@ bool QgsProject::readProjectFile( const QString &filename, QgsProject::ReadFlags
   QDomElement layerTreeElem = doc->documentElement().firstChildElement( QStringLiteral( "layer-tree-group" ) );
   if ( !layerTreeElem.isNull() )
   {
-    mRootGroup->readChildrenFromXml( layerTreeElem, context );
+    // Use a temporary tree to read the nodes to prevent signals being delivered to the models
+    QgsLayerTree tempTree;
+    tempTree.readChildrenFromXml( layerTreeElem, context );
+    mRootGroup->insertChildNodes( -1, tempTree.abandonChildren() );
   }
   else
   {

diff --git a/src/gui/layertree/qgslayertreeview.cpp b/src/gui/layertree/qgslayertreeview.cpp
@@ -89,9 +89,6 @@ void QgsLayerTreeView::setModel( QAbstractItemModel *model )
   if ( !treeModel )
     return;
 
-  connect( treeModel, &QAbstractItemModel::rowsInserted, this, &QgsLayerTreeView::modelRowsInserted );
-  connect( treeModel, &QAbstractItemModel::rowsRemoved, this, &QgsLayerTreeView::modelRowsRemoved );
-
   if ( mMessageBar )
     connect( treeModel, &QgsLayerTreeModel::messageEmitted,
              [ = ]( const QString & message, Qgis::MessageLevel level = Qgis::Info, int duration = 5 )
@@ -100,6 +97,9 @@ void QgsLayerTreeView::setModel( QAbstractItemModel *model )
 
   mProxyModel = new QgsLayerTreeProxyModel( treeModel, this );
 
+  connect( mProxyModel, &QAbstractItemModel::rowsInserted, this, &QgsLayerTreeView::modelRowsInserted );
+  connect( mProxyModel, &QAbstractItemModel::rowsRemoved, this, &QgsLayerTreeView::modelRowsRemoved );
+
 #ifdef ENABLE_MODELTEST
   new ModelTest( treeModel, this );
   new ModelTest( mProxyModel, this );
@@ -118,6 +118,8 @@ void QgsLayerTreeView::setModel( QAbstractItemModel *model )
   connect( treeModel, &QAbstractItemModel::dataChanged, this, &QgsLayerTreeView::onDataChanged );
 
   updateExpandedStateFromNode( treeModel->rootGroup() );
+
+  //checkModel();
 }
 
 QgsLayerTreeModel *QgsLayerTreeView::layerTreeModel() const
@@ -186,7 +188,7 @@ void QgsLayerTreeView::contextMenuEvent( QContextMenuEvent *event )
 
 void QgsLayerTreeView::modelRowsInserted( const QModelIndex &index, int start, int end )
 {
-  QgsLayerTreeNode *parentNode = layerTreeModel()->index2node( index );
+  QgsLayerTreeNode *parentNode = index2node( index );
   if ( !parentNode )
     return;
 
@@ -313,6 +315,8 @@ void QgsLayerTreeView::onCurrentChanged()
     layerTreeModel()->setCurrentIndex( mProxyModel->mapToSource( proxyModelNodeLayerIndex ) );
   }
 
+  //checkModel();
+
   emit currentLayerChanged( layerCurrent );
 }
 
@@ -342,6 +346,7 @@ void QgsLayerTreeView::onCustomPropertyChanged( QgsLayerTreeNode *node, const QS
 void QgsLayerTreeView::onModelReset()
 {
   updateExpandedStateFromNode( layerTreeModel()->rootGroup() );
+  //checkModel();
 }
 
 void QgsLayerTreeView::updateExpandedStateFromNode( QgsLayerTreeNode *node )
@@ -650,7 +655,37 @@ void QgsLayerTreeView::onDataChanged( const QModelIndex &topLeft, const QModelIn
 
   if ( roles.contains( Qt::SizeHintRole ) )
     viewport()->update();
+
+  //checkModel();
+}
+
+#if 0
+// for model debugging
+void QgsLayerTreeView::checkModel()
+{
+  std::function<void( QgsLayerTreeNode *, int )> debug;
+  debug = [ & ]( QgsLayerTreeNode * node, int depth )
+  {
+    if ( depth == 1 )
+      qDebug() << "----------------------------------------------";
+
+    qDebug() << depth << node->name() << node2index( node ) << layerTreeModel()->rowCount( node2sourceIndex( node ) ) << mProxyModel->rowCount( node2index( node ) );
+    Q_ASSERT( node == index2node( node2index( node ) ) );
+    Q_ASSERT( node == layerTreeModel()->index2node( node2sourceIndex( node ) ) );
+    Q_ASSERT( layerTreeModel()->rowCount( node2sourceIndex( node ) ) == mProxyModel->rowCount( node2index( node ) ) );
+
+    for ( int i = 0; i < mProxyModel->rowCount( node2index( node ) ); i++ )
+    {
+      QgsLayerTreeNode *childNode { index2node( mProxyModel->index( i, 0, node2index( node ) ) ) };
+      if ( childNode )
+        debug( childNode, depth + 1 );
+      else
+        qDebug() << "Warning no child node!";
+    }
+  };
+  debug( layerTreeModel()->rootGroup(), 1 );
 }
+#endif
 
 QgsLayerTreeProxyModel *QgsLayerTreeView::proxyModel() const
 {

diff --git a/src/gui/layertree/qgslayertreeview.h b/src/gui/layertree/qgslayertreeview.h
@@ -80,6 +80,7 @@ class GUI_EXPORT QgsLayerTreeProxyModel : public QSortFilterProxyModel
     QgsLayerTreeModel *mLayerTreeModel = nullptr;
     QString mFilterText;
     bool mShowPrivateLayers = false;
+
 };
 
 
@@ -403,6 +404,9 @@ class GUI_EXPORT QgsLayerTreeView : public QTreeView
 
     bool mShowPrivateLayers = false;
 
+    // For model  debugging
+    // void checkModel( );
+
     // friend so it can access viewOptions() method and mLastReleaseMousePos without making them public
     friend class QgsLayerTreeViewItemDelegate;
 };