More performant (and more correct) deletion of DRC markers.

Fixes https://gitlab.com/kicad/code/kicad/issues/5057

common/rc_item.cpp

@@ -387,17 +387,16 @@ void RC_TREE_MODEL::ValueChanged( RC_TREE_NODE* aNode )
 
 void RC_TREE_MODEL::DeleteCurrentItem( bool aDeep )
 {
-    DeleteItems( true, false, false, aDeep );
+    DeleteItems( true, false, aDeep );
 }
 
 
-void RC_TREE_MODEL::DeleteItems( bool aCurrent, bool aWarningsAndErrors, bool aExclusions,
-                                 bool aDeep )
+void RC_TREE_MODEL::DeleteItems( bool aCurrentOnly, bool aIncludeExclusions, bool aDeep )
 {
     RC_TREE_NODE*  current_node = ToNode( m_view->GetCurrentItem() );
     const RC_ITEM* current_item = current_node ? current_node->m_RcItem : nullptr;
 
-    if( !current_item && !aWarningsAndErrors && !aExclusions )
+    if( aCurrentOnly && !current_item )
     {
         wxBell();
         return;
@@ -407,11 +406,14 @@ void RC_TREE_MODEL::DeleteItems( bool aCurrent, bool aWarningsAndErrors, bool aE
     {
         RC_ITEM*     rcItem = m_rcItemsProvider->GetItem( i );
         MARKER_BASE* marker = rcItem->GetParent();
+        bool         excluded = marker ? marker->IsExcluded() : false;
+
+        if( aCurrentOnly && rcItem != current_item )
+            continue;
+
+        if( excluded && !aIncludeExclusions )
+            continue;
 
-        if( ( aCurrent && rcItem == current_item )
-            || ( aWarningsAndErrors && marker && !marker->IsExcluded() )
-            || ( aExclusions && marker && marker->IsExcluded() ) )
-        {
         wxDataViewItem      markerItem = ToItem( m_tree[i] );
         wxDataViewItemArray childItems;
         wxDataViewItem      parentItem = ToItem( m_tree[i]->m_Parent );
@@ -429,11 +431,17 @@ void RC_TREE_MODEL::DeleteItems( bool aCurrent, bool aWarningsAndErrors, bool aE
         m_tree.erase( m_tree.begin() + i );
         ItemDeleted( parentItem, markerItem );
 
-            m_rcItemsProvider->DeleteItem( i, aDeep );
+        // Only deep delete the current item here; others will be done through the
+        // DeleteAllItems() call below, which is more efficient.
+        m_rcItemsProvider->DeleteItem( i, aDeep && aCurrentOnly );
 
-            if( !aWarningsAndErrors && !aExclusions )
+        if( aCurrentOnly )
             break;
     }
+
+    if( !aCurrentOnly )
+    {
+        m_rcItemsProvider->DeleteAllItems( aIncludeExclusions, aDeep );
     }
 }
 

common/rc_item.h

@@ -59,6 +59,8 @@ public:
      */
     virtual void DeleteItem( int aIndex, bool aDeep ) = 0;
 
+    virtual void DeleteAllItems( bool aIncludeExclusions, bool aDeep ) = 0;
+
     virtual ~RC_ITEMS_PROVIDER() { }
 };
 
@@ -272,7 +274,12 @@ public:
     void ValueChanged( RC_TREE_NODE* aNode );
 
     void DeleteCurrentItem( bool aDeep );
-    void DeleteItems( bool aCurrent, bool aWarningsAndErrors, bool aExclusions, bool aDeep );
+
+    /**
+     * Deletes the current item or all items.  If all, \a aIncludeExclusions determines
+     * whether or not exclusions are also deleted.
+     */
+    void DeleteItems( bool aCurrentOnly, bool aIncludeExclusions, bool aDeep );
 
 private:
     void rebuildModel( RC_ITEMS_PROVIDER* aProvider, int aSeverities );

eeschema/dialogs/dialog_erc.cpp

@@ -582,7 +582,7 @@ void DIALOG_ERC::deleteAllMarkers( bool aIncludeExclusions )
     // Clear current selection list to avoid selection of deleted items
     m_parent->GetToolManager()->RunAction( EE_ACTIONS::clearSelection, true );
 
-    m_markerTreeModel->DeleteItems( false, true, aIncludeExclusions, true );
+    m_markerTreeModel->DeleteItems( false, true, aIncludeExclusions );
 }
 
 

eeschema/erc_settings.cpp

@@ -315,3 +315,15 @@ void SHEETLIST_ERC_ITEMS_PROVIDER::DeleteItem( int aIndex, bool aDeep )
         screens.DeleteMarker( marker );
     }
 }
+
+
+void SHEETLIST_ERC_ITEMS_PROVIDER::DeleteAllItems( bool aIncludeExclusions, bool aDeep )
+{
+    // Filtered list was already handled through DeleteItem() by the tree control
+
+    if( aDeep )
+    {
+        SCH_SCREENS screens( m_schematic->Root() );
+        screens.DeleteAllMarkers( MARKER_BASE::MARKER_ERC, aIncludeExclusions );
+    }
+}

eeschema/erc_settings.h

@@ -190,6 +190,8 @@ public:
     ERC_ITEM* GetItem( int aIndex ) override;
 
     void DeleteItem( int aIndex, bool aDeep ) override;
+
+    void DeleteAllItems( bool aIncludeExclusions, bool aDeep ) override;
 };
 
 

pcbnew/class_board.cpp

@@ -715,6 +715,28 @@ void BOARD::DeleteMARKERs()
 }
 
 
+void BOARD::DeleteMARKERs( bool aWarningsAndErrors, bool aExclusions )
+{
+    // Deleting lots of items from a vector can be very slow.  Copy remaining items instead.
+    MARKERS remaining;
+
+    for( MARKER_PCB* marker : m_markers )
+    {
+        if( ( marker->IsExcluded() && aExclusions )
+                || ( !marker->IsExcluded() && aWarningsAndErrors ) )
+        {
+            delete marker;
+        }
+        else
+        {
+            remaining.push_back( marker );
+        }
+    }
+
+    m_markers = remaining;
+}
+
+
 void BOARD::DeleteZONEOutlines()
 {
     // the vector does not know how to delete the ZONE Outlines, it holds pointers

pcbnew/class_board.h

@@ -367,6 +367,8 @@ public:
      */
     void DeleteMARKERs();
 
+    void DeleteMARKERs( bool aWarningsAndErrors, bool aExclusions );
+
     /**
      * Function DeleteZONEOutlines
      * deletes ALL zone outlines from the board.

pcbnew/cleanup_item.h

@@ -106,6 +106,17 @@ public:
             m_sourceVector->erase( m_sourceVector->begin() + aIndex );
         }
     }
+
+    void DeleteAllItems( bool aIncludeExclusions, bool aDeep ) override
+    {
+        if( aDeep )
+        {
+            for( CLEANUP_ITEM* item : *m_sourceVector )
+                delete item;
+
+            m_sourceVector->clear();
+        }
+    }
 };
 
 

pcbnew/dialogs/dialog_drc.cpp

@@ -513,7 +513,7 @@ void DIALOG_DRC::deleteAllMarkers( bool aIncludeExclusions )
     // Clear current selection list to avoid selection of deleted items
     m_brdEditor->GetToolManager()->RunAction( PCB_ACTIONS::selectionClear, true );
 
-    m_markerTreeModel->DeleteItems( false, true, aIncludeExclusions, true );
+    m_markerTreeModel->DeleteItems( false, true, aIncludeExclusions );
 }
 
 

pcbnew/drc/drc_provider.h

@@ -159,6 +159,14 @@ public:
         if( aDeep )
             m_board->Delete( marker );
     }
+
+    void DeleteAllItems( bool aIncludeExclusions, bool aDeep ) override
+    {
+        // Filtered list was already handled through DeleteItem() by the tree control
+
+        if( aDeep )
+            m_board->DeleteMARKERs( true, aIncludeExclusions );
+    }
 };
 
 
@@ -246,6 +254,19 @@ public:
             }
         }
     }
+
+    void DeleteAllItems( bool aIncludeExclusions, bool aDeep ) override
+    {
+        if( aDeep )
+        {
+            for( DRC_ITEM* item : *m_sourceVector )
+                delete item;
+
+            m_sourceVector->clear();
+        }
+
+        m_filteredVector.clear();   // no ownership of DRC_ITEM pointers
+    }
 };