Add bulk add/remove mode flag to the BOARD_LISTENER

This attempts to fix performance when importing large changesets from schematics.
The appearance control is a BOARD_LISTENER that would otherwise redraw per item imported which may cause lockups

pcbnew/board.cpp

@@ -590,7 +590,7 @@ void BOARD::Add( BOARD_ITEM* aBoardItem, ADD_MODE aMode )
             return;
         }
 
-        if( aMode == ADD_MODE::APPEND )
+        if( aMode == ADD_MODE::APPEND || aMode == ADD_MODE::BULK_APPEND )
             m_tracks.push_back( static_cast<TRACK*>( aBoardItem ) );
         else
             m_tracks.push_front( static_cast<TRACK*>( aBoardItem ) );
@@ -598,7 +598,7 @@ void BOARD::Add( BOARD_ITEM* aBoardItem, ADD_MODE aMode )
         break;
 
     case PCB_FOOTPRINT_T:
-        if( aMode == ADD_MODE::APPEND )
+        if( aMode == ADD_MODE::APPEND || aMode == ADD_MODE::BULK_APPEND )
             m_footprints.push_back( static_cast<FOOTPRINT*>( aBoardItem ) );
         else
             m_footprints.push_front( static_cast<FOOTPRINT*>( aBoardItem ) );
@@ -612,7 +612,7 @@ void BOARD::Add( BOARD_ITEM* aBoardItem, ADD_MODE aMode )
     case PCB_SHAPE_T:
     case PCB_TEXT_T:
     case PCB_TARGET_T:
-        if( aMode == ADD_MODE::APPEND )
+        if( aMode == ADD_MODE::APPEND || aMode == ADD_MODE::BULK_APPEND )
             m_drawings.push_back( aBoardItem );
         else
             m_drawings.push_front( aBoardItem );
@@ -635,11 +635,24 @@ void BOARD::Add( BOARD_ITEM* aBoardItem, ADD_MODE aMode )
     aBoardItem->ClearEditFlags();
     m_connectivity->Add( aBoardItem );
 
+    if( aMode != ADD_MODE::BULK_INSERT && aMode != ADD_MODE::BULK_APPEND )
         InvokeListeners( &BOARD_LISTENER::OnBoardItemAdded, *this, aBoardItem );
 }
 
 
-void BOARD::Remove( BOARD_ITEM* aBoardItem )
+void BOARD::FinalizeBulkAdd( std::vector<BOARD_ITEM*>& aNewItems )
+{
+    InvokeListeners( &BOARD_LISTENER::OnBoardItemsAdded, *this, aNewItems );
+}
+
+
+void BOARD::FinalizeBulkRemove( std::vector<BOARD_ITEM*>& aRemovedItems )
+{
+    InvokeListeners( &BOARD_LISTENER::OnBoardItemsRemoved, *this, aRemovedItems );
+}
+
+
+void BOARD::Remove( BOARD_ITEM* aBoardItem, REMOVE_MODE aRemoveMode )
 {
     // find these calls and fix them!  Don't send me no stinking' NULL.
     wxASSERT( aBoardItem );
@@ -716,6 +729,7 @@ void BOARD::Remove( BOARD_ITEM* aBoardItem )
 
     m_connectivity->Remove( aBoardItem );
 
+    if( aRemoveMode != REMOVE_MODE::BULK )
         InvokeListeners( &BOARD_LISTENER::OnBoardItemRemoved, *this, aBoardItem );
 }
 
@@ -1956,6 +1970,12 @@ void BOARD::OnItemChanged( BOARD_ITEM* aItem )
 }
 
 
+void BOARD::OnItemsChanged( std::vector<BOARD_ITEM*>& aItems )
+{
+    InvokeListeners( &BOARD_LISTENER::OnBoardItemsChanged, *this, aItems );
+}
+
+
 void BOARD::ResetNetHighLight()
 {
     m_highLight.Clear();

pcbnew/board.h

@@ -157,9 +157,12 @@ class BOARD_LISTENER
 public:
     virtual ~BOARD_LISTENER() { }
     virtual void OnBoardItemAdded( BOARD& aBoard, BOARD_ITEM* aBoardItem ) { }
+    virtual void OnBoardItemsAdded( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItem ) { }
     virtual void OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardItem ) { }
+    virtual void OnBoardItemsRemoved( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItem ) { }
     virtual void OnBoardNetSettingsChanged( BOARD& aBoard ) { }
     virtual void OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem ) { }
+    virtual void OnBoardItemsChanged( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItem ) { }
     virtual void OnBoardHighlightNetChanged( BOARD& aBoard ) { }
 };
 
@@ -338,7 +341,17 @@ public:
 
     void Add( BOARD_ITEM* aItem, ADD_MODE aMode = ADD_MODE::INSERT ) override;
 
-    void Remove( BOARD_ITEM* aBoardItem ) override;
+    void Remove( BOARD_ITEM* aBoardItem, REMOVE_MODE aMode = REMOVE_MODE::NORMAL ) override;
+
+    /**
+     * Must be used if Add() is used using a BULK_x ADD_MODE to generate a change event for listeners
+     */
+    void FinalizeBulkAdd( std::vector<BOARD_ITEM*>& aNewItems );
+
+    /**
+     * Must be used if Remove() is used using a BULK_x REMOVE_MODE to generate a change event for listeners
+     */
+    void FinalizeBulkRemove( std::vector<BOARD_ITEM*>& aRemovedItems );
 
     /**
      * Gets the first footprint on the board or nullptr.
@@ -1116,6 +1129,12 @@ public:
       */
     void OnItemChanged( BOARD_ITEM* aItem );
 
+    /**
+      * Notify the board and its listeners that an item on the board has
+      * been modified in some way.
+      */
+    void OnItemsChanged( std::vector<BOARD_ITEM*>& aItems );
+
     /*
      * Consistency check of internal m_groups structure.
      * @param repair if true, modify groups structure until it passes the sanity check.

pcbnew/board_commit.cpp

@@ -96,6 +96,10 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
     SELECTION_TOOL*     selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
     bool                itemsDeselected = false;
 
+    std::vector<BOARD_ITEM*> bulkAddedItems;
+    std::vector<BOARD_ITEM*> bulkRemovedItems;
+    std::vector<BOARD_ITEM*> itemsChanged;
+
     if( Empty() )
         return;
 
@@ -164,7 +168,10 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
                         undoList.PushItem( ITEM_PICKER( nullptr, boardItem, UNDO_REDO::NEWITEM ) );
 
                     if( !( changeFlags & CHT_DONE ) )
-                        board->Add( boardItem );        // handles connectivity
+                    {
+                        board->Add( boardItem, ADD_MODE::BULK_INSERT ); // handles connectivity
+                        bulkAddedItems.push_back( boardItem );
+                    }
                 }
 
                 if( boardItem->Type() != PCB_NETINFO_T )
@@ -230,7 +237,10 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
                     view->Remove( boardItem );
 
                     if( !( changeFlags & CHT_DONE ) )
-                        board->Remove( boardItem );
+                    {
+                        board->Remove( boardItem, REMOVE_MODE::BULK );
+                        bulkRemovedItems.push_back( boardItem );
+                    }
 
                     break;
 
@@ -244,7 +254,10 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
                     footprint->ClearFlags();
 
                     if( !( changeFlags & CHT_DONE ) )
-                        board->Remove( footprint );        // handles connectivity
+                    {
+                        board->Remove( footprint, REMOVE_MODE::BULK ); // handles connectivity
+                        bulkRemovedItems.push_back( footprint );
+                    }
                 }
                 break;
 
@@ -256,13 +269,17 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
                         if( m_isFootprintEditor )
                             board->GetFirstFootprint()->Remove( boardItem );
                         else
-                            board->Remove( boardItem );
+                        {
+                            board->Remove( boardItem, REMOVE_MODE::BULK );
+                            bulkRemovedItems.push_back( boardItem );
+                        }
                     }
                     break;
 
                 // Metadata items
                 case PCB_NETINFO_T:
-                    board->Remove( boardItem );
+                    board->Remove( boardItem, REMOVE_MODE::BULK );
+                    bulkRemovedItems.push_back( boardItem );
                     break;
 
                 default:                        // other types do not need to (or should not) be handled
@@ -298,7 +315,7 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
                             });
                 }
 
-                board->OnItemChanged( boardItem );
+                itemsChanged.push_back( boardItem );
 
                 // if no undo entry is needed, the copy would create a memory leak
                 if( !aCreateUndoEntry )
@@ -313,6 +330,15 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
         }
     }
 
+    if( bulkAddedItems.size() > 0 )
+        board->FinalizeBulkAdd( bulkAddedItems );
+
+    if( bulkRemovedItems.size() > 0 )
+        board->FinalizeBulkRemove( bulkRemovedItems );
+
+    if( itemsChanged.size() > 0 )
+        board->OnItemsChanged( itemsChanged );
+
     if( !m_isFootprintEditor )
     {
         size_t num_changes = m_changes.size();
@@ -397,6 +423,10 @@ void BOARD_COMMIT::Revert()
     BOARD* board = (BOARD*) m_toolMgr->GetModel();
     auto connectivity = board->GetConnectivity();
 
+    std::vector<BOARD_ITEM*> bulkAddedItems;
+    std::vector<BOARD_ITEM*> bulkRemovedItems;
+    std::vector<BOARD_ITEM*> itemsChanged;
+
     for( auto it = m_changes.rbegin(); it != m_changes.rend(); ++it )
     {
         COMMIT_LINE& ent = *it;
@@ -413,7 +443,8 @@ void BOARD_COMMIT::Revert()
 
             view->Remove( item );
             connectivity->Remove( item );
-            board->Remove( item );
+            board->Remove( item, REMOVE_MODE::BULK );
+            bulkRemovedItems.push_back( item );
             break;
 
         case CHT_REMOVE:
@@ -422,7 +453,8 @@ void BOARD_COMMIT::Revert()
 
             view->Add( item );
             connectivity->Add( item );
-            board->Add( item );
+            board->Add( item, ADD_MODE::INSERT );
+            bulkAddedItems.push_back( item );
             break;
 
         case CHT_MODIFY:
@@ -435,6 +467,8 @@ void BOARD_COMMIT::Revert()
             view->Add( item );
             connectivity->Add( item );
             board->OnItemChanged( item );
+            itemsChanged.push_back( item );
+
             delete copy;
             break;
         }
@@ -445,6 +479,15 @@ void BOARD_COMMIT::Revert()
         }
     }
 
+    if( bulkAddedItems.size() > 0 )
+        board->FinalizeBulkAdd( bulkAddedItems );
+
+    if( bulkRemovedItems.size() > 0 )
+        board->FinalizeBulkRemove( bulkRemovedItems );
+
+    if( itemsChanged.size() > 0 )
+        board->OnItemsChanged( itemsChanged );
+
     if ( !m_isFootprintEditor )
         connectivity->RecalculateRatsnest();
 

pcbnew/board_item_container.h

@@ -33,7 +33,15 @@
 enum class ADD_MODE
 {
     INSERT,
-    APPEND
+    APPEND,
+    BULK_APPEND,
+    BULK_INSERT
+};
+
+enum class REMOVE_MODE
+{
+    NORMAL,
+    BULK
 };
 
 /**
@@ -58,7 +66,7 @@ public:
     /**
      * @brief Removes an item from the container.
      */
-    virtual void Remove( BOARD_ITEM* aItem ) = 0;
+    virtual void Remove( BOARD_ITEM* aItem, REMOVE_MODE aMode = REMOVE_MODE::NORMAL ) = 0;
 
     /**
      * @brief Removes an item from the container and deletes it.

pcbnew/dialogs/dialog_net_inspector.cpp

@@ -1188,6 +1188,15 @@ void DIALOG_NET_INSPECTOR::OnBoardItemAdded( BOARD& aBoard, BOARD_ITEM* aBoardIt
 }
 
 
+void DIALOG_NET_INSPECTOR::OnBoardItemsAdded( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItem )
+{
+    for( BOARD_ITEM* item : aBoardItem )
+    {
+        OnBoardItemAdded( aBoard, item );
+    }
+}
+
+
 void DIALOG_NET_INSPECTOR::OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardItem )
 {
     if( NETINFO_ITEM* net = dynamic_cast<NETINFO_ITEM*>( aBoardItem ) )
@@ -1242,6 +1251,16 @@ void DIALOG_NET_INSPECTOR::OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoard
 }
 
 
+void DIALOG_NET_INSPECTOR::OnBoardItemsRemoved(
+        BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems )
+{
+    for( BOARD_ITEM* item : aBoardItems )
+    {
+        OnBoardItemRemoved( aBoard, item );
+    }
+}
+
+
 void DIALOG_NET_INSPECTOR::OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem )
 {
     if( dynamic_cast<BOARD_CONNECTED_ITEM*>( aBoardItem ) != nullptr
@@ -1253,6 +1272,14 @@ void DIALOG_NET_INSPECTOR::OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoard
 }
 
 
+void DIALOG_NET_INSPECTOR::OnBoardItemsChanged(
+        BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems )
+{
+    buildNetsList();
+    m_netsList->Refresh();
+}
+
+
 void DIALOG_NET_INSPECTOR::OnBoardHighlightNetChanged( BOARD& aBoard )
 {
     if( !m_brd->IsHighLightNetON() )

pcbnew/dialogs/dialog_net_inspector.h

@@ -56,9 +56,12 @@ public:
     SETTINGS Settings() const;
 
     virtual void OnBoardItemAdded( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override;
+    virtual void OnBoardItemsAdded( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems ) override;
     virtual void OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override;
+    virtual void OnBoardItemsRemoved( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems ) override;
     virtual void OnBoardNetSettingsChanged( BOARD& aBoard ) override;
     virtual void OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override;
+    virtual void OnBoardItemsChanged( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems ) override;
     virtual void OnBoardHighlightNetChanged( BOARD& aBoard ) override;
 
 private:

pcbnew/footprint.cpp

@@ -492,7 +492,7 @@ void FOOTPRINT::Add( BOARD_ITEM* aBoardItem, ADD_MODE aMode )
 }
 
 
-void FOOTPRINT::Remove( BOARD_ITEM* aBoardItem )
+void FOOTPRINT::Remove( BOARD_ITEM* aBoardItem, REMOVE_MODE aMode )
 {
     switch( aBoardItem->Type() )
     {

pcbnew/footprint.h

@@ -129,7 +129,7 @@ public:
     void Add( BOARD_ITEM* aItem, ADD_MODE aMode = ADD_MODE::INSERT ) override;
 
     ///> @copydoc BOARD_ITEM_CONTAINER::Remove()
-    void Remove( BOARD_ITEM* aItem ) override;
+    void Remove( BOARD_ITEM* aItem, REMOVE_MODE aMode = REMOVE_MODE::NORMAL ) override;
 
     /**
      * Function ClearAllNets

pcbnew/plugins/kicad/pcb_parser.cpp

@@ -537,6 +537,9 @@ BOARD* PCB_PARSER::parseBOARD_unchecked()
 
     parseHeader();
 
+    std::vector<BOARD_ITEM*> bulkAddedItems;
+    BOARD_ITEM* item = nullptr;
+
     for( token = NextTok();  token != T_RIGHT;  token = NextTok() )
     {
         if( token != T_LEFT )
@@ -588,28 +591,40 @@ BOARD* PCB_PARSER::parseBOARD_unchecked()
         case T_gr_poly:
         case T_gr_circle:
         case T_gr_rect:
-            m_board->Add( parsePCB_SHAPE(), ADD_MODE::APPEND );
+            item = parsePCB_SHAPE();
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         case T_gr_text:
-            m_board->Add( parsePCB_TEXT(), ADD_MODE::APPEND );
+            item = parsePCB_TEXT();
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         case T_dimension:
-            m_board->Add( parseDIMENSION(), ADD_MODE::APPEND );
+            item = parseDIMENSION();
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         case T_module:      // legacy token
         case T_footprint:
-            m_board->Add( parseFOOTPRINT(), ADD_MODE::APPEND );
+            item = parseFOOTPRINT();
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         case T_segment:
-            m_board->Add( parseTRACK(), ADD_MODE::APPEND );
+            item = parseTRACK();
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         case T_arc:
-            m_board->Add( parseARC(), ADD_MODE::APPEND );
+            item = parseARC();
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         case T_group:
@@ -617,15 +632,21 @@ BOARD* PCB_PARSER::parseBOARD_unchecked()
             break;
 
         case T_via:
-            m_board->Add( parseVIA(), ADD_MODE::APPEND );
+            item = parseVIA();
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         case T_zone:
-            m_board->Add( parseZONE( m_board ), ADD_MODE::APPEND );
+            item = parseZONE( m_board );
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         case T_target:
-            m_board->Add( parsePCB_TARGET(), ADD_MODE::APPEND );
+            item = parsePCB_TARGET();
+            m_board->Add( item, ADD_MODE::BULK_APPEND );
+            bulkAddedItems.push_back( item );
             break;
 
         default:
@@ -635,6 +656,9 @@ BOARD* PCB_PARSER::parseBOARD_unchecked()
         }
     }
 
+    if( bulkAddedItems.size() > 0 )
+        m_board->FinalizeBulkAdd( bulkAddedItems );
+
     m_board->SetProperties( properties );
 
     if( m_undefinedLayers.size() > 0 )

pcbnew/widgets/appearance_controls.cpp

@@ -955,6 +955,12 @@ void APPEARANCE_CONTROLS::OnBoardItemAdded( BOARD& aBoard, BOARD_ITEM* aBoardIte
 }
 
 
+void APPEARANCE_CONTROLS::OnBoardItemsAdded( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems )
+{
+    handleBoardItemsChanged();
+}
+
+
 void APPEARANCE_CONTROLS::OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardItem )
 {
     if( aBoardItem->Type() == PCB_NETINFO_T )
@@ -962,6 +968,13 @@ void APPEARANCE_CONTROLS::OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardI
 }
 
 
+void APPEARANCE_CONTROLS::OnBoardItemsRemoved(
+        BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems )
+{
+    handleBoardItemsChanged();
+}
+
+
 void APPEARANCE_CONTROLS::OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem )
 {
     if( aBoardItem->Type() == PCB_NETINFO_T )
@@ -969,6 +982,12 @@ void APPEARANCE_CONTROLS::OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardI
 }
 
 
+void APPEARANCE_CONTROLS::OnBoardItemsChanged(
+        BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems )
+{
+    handleBoardItemsChanged();
+}
+
 
 void APPEARANCE_CONTROLS::handleBoardItemsChanged()
 {

pcbnew/widgets/appearance_controls.h

@@ -206,10 +206,16 @@ public:
 
     void OnBoardItemAdded( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override;
 
+    void OnBoardItemsAdded( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems ) override;
+
     void OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override;
 
+    void OnBoardItemsRemoved( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems ) override;
+
     void OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override;
 
+    void OnBoardItemsChanged( BOARD& aBoard, std::vector<BOARD_ITEM*>& aBoardItems ) override;
+
     ///> Updates the colors on all the widgets from the new chosen color theme
     void OnColorThemeChanged();