No more warning in the model, but still doesn't work

qgis · Nov 9, 2020 · bc18ec9 · bc18ec9
1 parent 11b281e
commit bc18ec9
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 61 deletions.
diff --git a/src/core/layertree/qgslayertreemodel.cpp b/src/core/layertree/qgslayertreemodel.cpp
@@ -549,20 +549,9 @@ void QgsLayerTreeModel::refreshLayerLegend( QgsLayerTreeLayer *nodeLayer )
   int oldNodeCount = rowCount( idx );
   if ( oldNodeCount > 0 )
   {
-    if ( mRemoveRowsNestedLevel > 0 )
-    {
-      endRemoveRows();
-      mRemoveRowsNestedLevel--;
-      Q_ASSERT( mRemoveRowsNestedLevel == 0 );
-    }
-
     beginRemoveRows( idx, 0, oldNodeCount - 1 );
-    mRemoveRowsNestedLevel++;
-
     removeLegendFromLayer( nodeLayer );
     endRemoveRows();
-    mRemoveRowsNestedLevel--;
-    Q_ASSERT( mRemoveRowsNestedLevel == 0 );
   }
 
   addLegendToLayer( nodeLayer );
@@ -734,15 +723,7 @@ int QgsLayerTreeModel::scaleIconSize( int standardSize )
 
 void QgsLayerTreeModel::nodeWillAddChildren( QgsLayerTreeNode *node, int indexFrom, int indexTo )
 {
-  Q_ASSERT( node );
-  if ( mInsertRowsNestedLevel > 0 )
-  {
-    endInsertRows();
-    mInsertRowsNestedLevel--;
-    Q_ASSERT( mInsertRowsNestedLevel == 0 );
-  }
   beginInsertRows( node2index( node ), indexFrom, indexTo );
-  mInsertRowsNestedLevel++;
 }
 
 static QList<QgsLayerTreeLayer *> _layerNodesInSubtree( QgsLayerTreeNode *node, int indexFrom, int indexTo )
@@ -765,8 +746,6 @@ void QgsLayerTreeModel::nodeAddedChildren( QgsLayerTreeNode *node, int indexFrom
   Q_ASSERT( node );
 
   endInsertRows();
-  mInsertRowsNestedLevel--;
-  Q_ASSERT( mInsertRowsNestedLevel == 0 );
 
   const auto subNodes = _layerNodesInSubtree( node, indexFrom, indexTo );
   for ( QgsLayerTreeLayer *newLayerNode : subNodes )
@@ -777,14 +756,7 @@ void QgsLayerTreeModel::nodeWillRemoveChildren( QgsLayerTreeNode *node, int inde
 {
   Q_ASSERT( node );
 
-  if ( mRemoveRowsNestedLevel > 0 )
-  {
-    endRemoveRows();
-    mRemoveRowsNestedLevel--;
-    Q_ASSERT( mRemoveRowsNestedLevel == 0 );
-  }
   beginRemoveRows( node2index( node ), indexFrom, indexTo );
-  mRemoveRowsNestedLevel++;
 
   // disconnect from layers and remove their legend
   const auto subNodes = _layerNodesInSubtree( node, indexFrom, indexTo );
@@ -795,8 +767,6 @@ void QgsLayerTreeModel::nodeWillRemoveChildren( QgsLayerTreeNode *node, int inde
 void QgsLayerTreeModel::nodeRemovedChildren()
 {
   endRemoveRows();
-  mRemoveRowsNestedLevel--;
-  Q_ASSERT( mRemoveRowsNestedLevel == 0 );
 }
 
 void QgsLayerTreeModel::nodeVisibilityChanged( QgsLayerTreeNode *node )
@@ -1379,14 +1349,7 @@ void QgsLayerTreeModel::addLegendToLayer( QgsLayerTreeLayer *nodeL )
 
   if ( !filteredLstNew.isEmpty() )
   {
-    if ( mInsertRowsNestedLevel > 0 )
-    {
-      endInsertRows();
-      mInsertRowsNestedLevel--;
-      Q_ASSERT( mInsertRowsNestedLevel == 0 );
-    }
     beginInsertRows( node2index( nodeL ), 0, count - 1 );
-    mInsertRowsNestedLevel++;
   }
 
   LayerLegendData data;
@@ -1400,8 +1363,6 @@ void QgsLayerTreeModel::addLegendToLayer( QgsLayerTreeLayer *nodeL )
   if ( !filteredLstNew.isEmpty() )
   {
     endInsertRows();
-    mInsertRowsNestedLevel--;
-    Q_ASSERT( mInsertRowsNestedLevel == 0 );
   }
 
   // invalidate map based data even if the data is not map-based to make sure

diff --git a/src/core/layertree/qgslayertreemodel.h b/src/core/layertree/qgslayertreemodel.h
@@ -472,10 +472,6 @@ class CORE_EXPORT QgsLayerTreeModel : public QAbstractItemModel
     //! Returns a temporary render context
     QgsRenderContext *createTemporaryRenderContext() const;
 
-    //! Keep track of the row insertion nested status
-    int mInsertRowsNestedLevel = 0;
-    //! Keep track of the row remove nested status
-    int mRemoveRowsNestedLevel = 0;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsLayerTreeModel::Flags )

diff --git a/src/core/layertree/qgslayertreenode.cpp b/src/core/layertree/qgslayertreenode.cpp
@@ -48,6 +48,22 @@ QgsLayerTreeNode::~QgsLayerTreeNode()
   qDeleteAll( mChildren );
 }
 
+QList<QgsLayerTreeNode *> QgsLayerTreeNode::abandonChildren()
+{
+  auto orphans { mChildren };
+  mChildren.clear();
+  for ( auto orphan : orphans )
+  {
+    orphan->makeOrphan( );
+  }
+  return orphans;
+}
+
+void QgsLayerTreeNode::makeOrphan()
+{
+  mParent = nullptr;
+}
+
 QgsLayerTreeNode *QgsLayerTreeNode::readXml( QDomElement &element, const QgsReadWriteContext &context )
 {
   QgsLayerTreeNode *node = nullptr;
@@ -231,12 +247,15 @@ void QgsLayerTreeNode::insertChildrenPrivate( int index, QList<QgsLayerTreeNode
   if ( index < 0 || index >= mChildren.count() )
     index = mChildren.count();
 
-  int indexTo = index + nodes.count() - 1;
-  emit willAddChildren( this, index, indexTo );
   for ( int i = 0; i < nodes.count(); ++i )
   {
     QgsLayerTreeNode *node = nodes.at( i );
+
+    const auto orphans { node->abandonChildren() };
+
+    emit willAddChildren( this, index + i, index + i );
     mChildren.insert( index + i, node );
+    emit addedChildren( this, index + i, index + i );
 
     // forward the signal towards the root
     connect( node, &QgsLayerTreeNode::willAddChildren, this, &QgsLayerTreeNode::willAddChildren );
@@ -247,27 +266,42 @@ void QgsLayerTreeNode::insertChildrenPrivate( int index, QList<QgsLayerTreeNode
     connect( node, &QgsLayerTreeNode::visibilityChanged, this, &QgsLayerTreeNode::visibilityChanged );
     connect( node, &QgsLayerTreeNode::expandedChanged, this, &QgsLayerTreeNode::expandedChanged );
     connect( node, &QgsLayerTreeNode::nameChanged, this, &QgsLayerTreeNode::nameChanged );
+
+    // Now add children
+    if ( ! orphans.isEmpty() )
+    {
+      node->insertChildrenPrivate( -1, orphans );
+    }
+
   }
-  emit addedChildren( this, index, indexTo );
 }
 
 void QgsLayerTreeNode::removeChildrenPrivate( int from, int count, bool destroy )
 {
   if ( from < 0 || count <= 0 )
     return;
 
-  int to = from + count - 1;
+  const int to = from + count - 1;
   if ( to >= mChildren.count() )
     return;
-  emit willRemoveChildren( this, from, to );
+
   while ( --count >= 0 )
   {
-    QgsLayerTreeNode *node = mChildren.takeAt( from );
-    node->mParent = nullptr;
+    QgsLayerTreeNode *node = mChildren.at( from );
+
+    // Remove children first
+    if ( ! node->children().isEmpty() )
+    {
+      node->removeChildrenPrivate( 0, node->children().count(), destroy );
+    }
+
+    emit willRemoveChildren( this, from, from );
+    node = mChildren.takeAt( from );
     if ( destroy )
       delete node;
+    node->makeOrphan();
+    emit removedChildren( this, from, from );
   }
-  emit removedChildren( this, from, to );
 }
 
 bool QgsLayerTreeNode::takeChild( QgsLayerTreeNode *node )

diff --git a/src/core/layertree/qgslayertreenode.h b/src/core/layertree/qgslayertreenode.h
@@ -112,6 +112,10 @@ 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
+    QList<QgsLayerTreeNode *> abandonChildren() SIP_SKIP;
+    //! Set parent to null
+    void makeOrphan() SIP_SKIP;
 
     /**
      * Returns name of the node

diff --git a/src/core/layertree/qgslayertreeregistrybridge.cpp b/src/core/layertree/qgslayertreeregistrybridge.cpp
@@ -30,7 +30,7 @@ QgsLayerTreeRegistryBridge::QgsLayerTreeRegistryBridge( QgsLayerTreeGroup *root,
   , mInsertionPoint( root, 0 )
 {
   connect( mProject, &QgsProject::legendLayersAdded, this, &QgsLayerTreeRegistryBridge::layersAdded );
-  connect( mProject, static_cast < void ( QgsProject::* )( const QStringList & ) >( &QgsProject::layersWillBeRemoved ), this, &QgsLayerTreeRegistryBridge::layersWillBeRemoved );
+  connect( mProject, qgis::overload<const QStringList &>::of<QgsProject>( &QgsProject::layersWillBeRemoved ), this, &QgsLayerTreeRegistryBridge::layersWillBeRemoved );
 
   connect( mRoot, &QgsLayerTreeNode::willRemoveChildren, this, &QgsLayerTreeRegistryBridge::groupWillRemoveChildren );
   connect( mRoot, &QgsLayerTreeNode::removedChildren, this, &QgsLayerTreeRegistryBridge::groupRemovedChildren );
@@ -118,7 +118,7 @@ static void _collectLayerIdsInGroup( QgsLayerTreeGroup *group, int indexFrom, in
 
 void QgsLayerTreeRegistryBridge::groupWillRemoveChildren( QgsLayerTreeNode *node, int indexFrom, int indexTo )
 {
-  if ( mRegistryRemovingLayers )
+  if ( mRegistryRemovingLayers || QgsLayerTree::isGroup( node ) )
     return; // do not try to remove those layers again
 
   Q_ASSERT( mLayerIdsForRemoval.isEmpty() );

diff --git a/src/gui/layertree/qgslayertreeview.cpp b/src/gui/layertree/qgslayertreeview.cpp
@@ -32,6 +32,10 @@
 #include <QHeaderView>
 #include <QScrollBar>
 
+#ifdef ENABLE_MODELTEST
+#include "modeltest.h"
+#endif
+
 #include "qgslayertreeviewindicator.h"
 #include "qgslayertreeviewitemdelegate.h"
 
@@ -95,6 +99,12 @@ void QgsLayerTreeView::setModel( QAbstractItemModel *model )
          );
 
   mProxyModel = new QgsLayerTreeProxyModel( treeModel, this );
+
+#ifdef ENABLE_MODELTEST
+  new ModelTest( treeModel, this );
+  new ModelTest( mProxyModel, this );
+#endif
+
   mProxyModel->setShowPrivateLayers( mShowPrivateLayers );
   QTreeView::setModel( mProxyModel );
 

diff --git a/tests/qt_modeltest/modeltest.cpp b/tests/qt_modeltest/modeltest.cpp
@@ -28,6 +28,7 @@
 #include <QStringList>
 #include <QSize>
 #include <QAbstractItemModel>
+#include <QDebug>
 
 /*!
     Connect to all of the models signals.  Whenever anything happens
@@ -493,17 +494,18 @@ void ModelTest::rowsAboutToBeInserted( const QModelIndex &parent, int start, int
 void ModelTest::rowsInserted( const QModelIndex &parent, int start, int end )
 {
   Changing c = insert.pop();
+  //*
+  if ( c.next != model->data( model->index( end + 1, 0, c.parent ) ) )
+  {
+    qDebug() << start << end;
+    for ( int i = 0; i < model->rowCount(); ++i )
+      qDebug() << model->index( i, 0 ).data().toString();
+    qDebug() << c.next << model->data( model->index( end + 1, 0, c.parent ) );
+  }
+  //*/
   Q_ASSERT( c.parent == parent );
   Q_ASSERT( c.oldSize + ( end - start + 1 ) == model->rowCount( parent ) );
   Q_ASSERT( c.last == model->data( model->index( start - 1, 0, c.parent ) ) );
-  /*
-  if (c.next != model->data(model->index(end + 1, 0, c.parent))) {
-      qDebug() << start << end;
-      for (int i=0; i < model->rowCount(); ++i)
-          qDebug() << model->index(i, 0).data().toString();
-      qDebug() << c.next << model->data(model->index(end + 1, 0, c.parent));
-  }
-  */
   Q_ASSERT( c.next == model->data( model->index( end + 1, 0, c.parent ) ) );
 }
 

diff --git a/tests/src/python/test_qgslayertreeview.py b/tests/src/python/test_qgslayertreeview.py
@@ -509,6 +509,7 @@ def testProxyModelCurrentIndex(self):
         view.setModel(self.model)
         if USE_MODEL_TESTER:
             proxy_tester = QAbstractItemModelTester(view.model())
+            tree_tester = QAbstractItemModelTester(view.layerTreeModel())
 
         view.setCurrentLayer(self.layer3)
         self.layer3.setFlags(self.layer.Private)
@@ -520,6 +521,8 @@ def testNode2IndexMethods(self):
         view.setModel(self.model)
         if USE_MODEL_TESTER:
             proxy_tester = QAbstractItemModelTester(view.model())
+            tree_tester = QAbstractItemModelTester(view.layerTreeModel())
+
         tree_model = view.layerTreeModel()
         proxy_model = view.proxyModel()