From e88d41f7647525a9544ec1613103f53e3f4df12f Mon Sep 17 00:00:00 2001 From: Marek Roszko Date: Mon, 7 Dec 2020 18:29:30 -0500 Subject: [PATCH] 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 | 32 +++++++++++--- pcbnew/board.h | 21 ++++++++- pcbnew/board_commit.cpp | 59 +++++++++++++++++++++---- pcbnew/board_item_container.h | 12 ++++- pcbnew/dialogs/dialog_net_inspector.cpp | 27 +++++++++++ pcbnew/dialogs/dialog_net_inspector.h | 3 ++ pcbnew/footprint.cpp | 2 +- pcbnew/footprint.h | 2 +- pcbnew/plugins/kicad/pcb_parser.cpp | 42 ++++++++++++++---- pcbnew/widgets/appearance_controls.cpp | 19 ++++++++ pcbnew/widgets/appearance_controls.h | 6 +++ 11 files changed, 197 insertions(+), 28 deletions(-) diff --git a/pcbnew/board.cpp b/pcbnew/board.cpp index 330c861f91..7d80d789e1 100644 --- a/pcbnew/board.cpp +++ b/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( aBoardItem ) ); else m_tracks.push_front( static_cast( 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( aBoardItem ) ); else m_footprints.push_front( static_cast( 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 ); - InvokeListeners( &BOARD_LISTENER::OnBoardItemAdded, *this, 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& aNewItems ) +{ + InvokeListeners( &BOARD_LISTENER::OnBoardItemsAdded, *this, aNewItems ); +} + + +void BOARD::FinalizeBulkRemove( std::vector& 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,7 +729,8 @@ void BOARD::Remove( BOARD_ITEM* aBoardItem ) m_connectivity->Remove( aBoardItem ); - InvokeListeners( &BOARD_LISTENER::OnBoardItemRemoved, *this, 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& aItems ) +{ + InvokeListeners( &BOARD_LISTENER::OnBoardItemsChanged, *this, aItems ); +} + + void BOARD::ResetNetHighLight() { m_highLight.Clear(); diff --git a/pcbnew/board.h b/pcbnew/board.h index dd7ba219e0..784d4ca7e4 100644 --- a/pcbnew/board.h +++ b/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& aBoardItem ) { } virtual void OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardItem ) { } + virtual void OnBoardItemsRemoved( BOARD& aBoard, std::vector& aBoardItem ) { } virtual void OnBoardNetSettingsChanged( BOARD& aBoard ) { } virtual void OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem ) { } + virtual void OnBoardItemsChanged( BOARD& aBoard, std::vector& 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& 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& 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& aItems ); + /* * Consistency check of internal m_groups structure. * @param repair if true, modify groups structure until it passes the sanity check. diff --git a/pcbnew/board_commit.cpp b/pcbnew/board_commit.cpp index 2a964eec9a..a1fb040ab7 100644 --- a/pcbnew/board_commit.cpp +++ b/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(); bool itemsDeselected = false; + std::vector bulkAddedItems; + std::vector bulkRemovedItems; + std::vector 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 bulkAddedItems; + std::vector bulkRemovedItems; + std::vector 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(); diff --git a/pcbnew/board_item_container.h b/pcbnew/board_item_container.h index 968ad37f43..78b103c9c1 100644 --- a/pcbnew/board_item_container.h +++ b/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. diff --git a/pcbnew/dialogs/dialog_net_inspector.cpp b/pcbnew/dialogs/dialog_net_inspector.cpp index 9c5602c82a..8bd2926665 100644 --- a/pcbnew/dialogs/dialog_net_inspector.cpp +++ b/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& 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( aBoardItem ) ) @@ -1242,6 +1251,16 @@ void DIALOG_NET_INSPECTOR::OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoard } +void DIALOG_NET_INSPECTOR::OnBoardItemsRemoved( + BOARD& aBoard, std::vector& aBoardItems ) +{ + for( BOARD_ITEM* item : aBoardItems ) + { + OnBoardItemRemoved( aBoard, item ); + } +} + + void DIALOG_NET_INSPECTOR::OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem ) { if( dynamic_cast( 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& aBoardItems ) +{ + buildNetsList(); + m_netsList->Refresh(); +} + + void DIALOG_NET_INSPECTOR::OnBoardHighlightNetChanged( BOARD& aBoard ) { if( !m_brd->IsHighLightNetON() ) diff --git a/pcbnew/dialogs/dialog_net_inspector.h b/pcbnew/dialogs/dialog_net_inspector.h index 987065bf0b..39a3666c15 100644 --- a/pcbnew/dialogs/dialog_net_inspector.h +++ b/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& aBoardItems ) override; virtual void OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override; + virtual void OnBoardItemsRemoved( BOARD& aBoard, std::vector& aBoardItems ) override; virtual void OnBoardNetSettingsChanged( BOARD& aBoard ) override; virtual void OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override; + virtual void OnBoardItemsChanged( BOARD& aBoard, std::vector& aBoardItems ) override; virtual void OnBoardHighlightNetChanged( BOARD& aBoard ) override; private: diff --git a/pcbnew/footprint.cpp b/pcbnew/footprint.cpp index 5074dce6f1..5b03966fe8 100644 --- a/pcbnew/footprint.cpp +++ b/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() ) { diff --git a/pcbnew/footprint.h b/pcbnew/footprint.h index 6b66cf2a95..5b8f330f8a 100644 --- a/pcbnew/footprint.h +++ b/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 diff --git a/pcbnew/plugins/kicad/pcb_parser.cpp b/pcbnew/plugins/kicad/pcb_parser.cpp index 5938d6b4c1..b9ee9fd575 100644 --- a/pcbnew/plugins/kicad/pcb_parser.cpp +++ b/pcbnew/plugins/kicad/pcb_parser.cpp @@ -537,6 +537,9 @@ BOARD* PCB_PARSER::parseBOARD_unchecked() parseHeader(); + std::vector 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 ) diff --git a/pcbnew/widgets/appearance_controls.cpp b/pcbnew/widgets/appearance_controls.cpp index f391dcd4df..3ae8bfaaae 100644 --- a/pcbnew/widgets/appearance_controls.cpp +++ b/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& 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& 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& aBoardItems ) +{ + handleBoardItemsChanged(); +} + void APPEARANCE_CONTROLS::handleBoardItemsChanged() { diff --git a/pcbnew/widgets/appearance_controls.h b/pcbnew/widgets/appearance_controls.h index bb8561cd6e..b06a34c850 100644 --- a/pcbnew/widgets/appearance_controls.h +++ b/pcbnew/widgets/appearance_controls.h @@ -206,10 +206,16 @@ public: void OnBoardItemAdded( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override; + void OnBoardItemsAdded( BOARD& aBoard, std::vector& aBoardItems ) override; + void OnBoardItemRemoved( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override; + void OnBoardItemsRemoved( BOARD& aBoard, std::vector& aBoardItems ) override; + void OnBoardItemChanged( BOARD& aBoard, BOARD_ITEM* aBoardItem ) override; + void OnBoardItemsChanged( BOARD& aBoard, std::vector& aBoardItems ) override; + ///> Updates the colors on all the widgets from the new chosen color theme void OnColorThemeChanged();