diff --git a/pcbnew/board.cpp b/pcbnew/board.cpp index c08c9c5db2..186a7f0cd7 100644 --- a/pcbnew/board.cpp +++ b/pcbnew/board.cpp @@ -1086,6 +1086,90 @@ void BOARD::Remove( BOARD_ITEM* aBoardItem, REMOVE_MODE aRemoveMode ) } +void BOARD::RemoveAll( std::initializer_list aTypes ) +{ + std::vector removed; + + if( aTypes.size() == 1 && *aTypes.begin() == TYPE_NOT_INIT ) + { + aTypes = { + PCB_NETINFO_T, PCB_MARKER_T, PCB_GROUP_T, PCB_ZONE_T, PCB_GENERATOR_T, PCB_FOOTPRINT_T, + PCB_TRACE_T, PCB_SHAPE_T + }; + } + + for( const KICAD_T& type : aTypes ) + { + switch( type ) + { + case PCB_NETINFO_T: + std::copy( m_NetInfo.begin(), m_NetInfo.end(), std::back_inserter( removed ) ); + m_NetInfo.clear(); + break; + + case PCB_MARKER_T: + std::copy( m_markers.begin(), m_markers.end(), std::back_inserter( removed ) ); + m_markers.clear(); + break; + + case PCB_GROUP_T: + std::copy( m_groups.begin(), m_groups.end(), std::back_inserter( removed ) ); + m_groups.clear(); + break; + + case PCB_ZONE_T: + std::copy( m_zones.begin(), m_zones.end(), std::back_inserter( removed ) ); + m_zones.clear(); + break; + + case PCB_GENERATOR_T: + std::copy( m_generators.begin(), m_generators.end(), std::back_inserter( removed ) ); + m_generators.clear(); + break; + + case PCB_FOOTPRINT_T: + std::copy( m_footprints.begin(), m_footprints.end(), std::back_inserter( removed ) ); + m_footprints.clear(); + break; + + case PCB_TRACE_T: + std::copy( m_tracks.begin(), m_tracks.end(), std::back_inserter( removed ) ); + m_tracks.clear(); + break; + + case PCB_ARC_T: + case PCB_VIA_T: + wxFAIL_MSG( wxT( "Use PCB_TRACE_T to remove all tracks, arcs, and vias" ) ); + break; + + case PCB_SHAPE_T: + std::copy( m_drawings.begin(), m_drawings.end(), std::back_inserter( removed ) ); + m_drawings.clear(); + break; + + case PCB_DIM_ALIGNED_T: + case PCB_DIM_CENTER_T: + case PCB_DIM_RADIAL_T: + case PCB_DIM_ORTHOGONAL_T: + case PCB_DIM_LEADER_T: + case PCB_REFERENCE_IMAGE_T: + case PCB_FIELD_T: + case PCB_TEXT_T: + case PCB_TEXTBOX_T: + case PCB_TABLE_T: + case PCB_TARGET_T: + wxFAIL_MSG( wxT( "Use PCB_SHAPE_T to remove all graphics and text" ) ); + break; + + default: + wxFAIL_MSG( wxT( "BOARD::RemoveAll() needs more ::Type() support" ) ); + } + } + + FinalizeBulkRemove( removed ); +} + + wxString BOARD::GetItemDescription( UNITS_PROVIDER* aUnitsProvider ) const { return wxString::Format( _( "PCB" ) ); diff --git a/pcbnew/board.h b/pcbnew/board.h index 8df94e112c..169dd69d27 100644 --- a/pcbnew/board.h +++ b/pcbnew/board.h @@ -312,22 +312,16 @@ public: const wxString &GetFileName() const { return m_fileName; } - TRACKS& Tracks() { return m_tracks; } const TRACKS& Tracks() const { return m_tracks; } - FOOTPRINTS& Footprints() { return m_footprints; } const FOOTPRINTS& Footprints() const { return m_footprints; } - DRAWINGS& Drawings() { return m_drawings; } const DRAWINGS& Drawings() const { return m_drawings; } - ZONES& Zones() { return m_zones; } const ZONES& Zones() const { return m_zones; } - GENERATORS& Generators() { return m_generators; } const GENERATORS& Generators() const { return m_generators; } - MARKERS& Markers() { return m_markers; } const MARKERS& Markers() const { return m_markers; } const BOARD_ITEM_SET GetItemSet(); @@ -340,7 +334,6 @@ public: * - If a group specifies a name, it must be unique * - The graph of groups containing subgroups must be cyclic. */ - GROUPS& Groups() { return m_groups; } const GROUPS& Groups() const { return m_groups; } const std::vector AllConnectedItems(); @@ -389,6 +382,16 @@ public: ///< @copydoc BOARD_ITEM_CONTAINER::Remove() void Remove( BOARD_ITEM* aBoardItem, REMOVE_MODE aMode = REMOVE_MODE::NORMAL ) override; + /** + * An efficient way to remove all items of a certain type from the board. + * Because of how items are stored, this method has some limitations in order to preserve + * performance: tracks, vias, and arcs are all removed together by PCB_TRACE_T, and all graphics + * and text object types are removed together by PCB_SHAPE_T. If you need something more + * granular than that, use BOARD::Remove. + * @param aTypes is a list of one or more types to remove, or leave default to remove all + */ + void RemoveAll( std::initializer_list aTypes = { TYPE_NOT_INIT } ); + /** * Must be used if Add() is used using a BULK_x ADD_MODE to generate a change event for * listeners. @@ -1277,6 +1280,8 @@ private: int m_timeStamp; // actually a modification counter wxString m_fileName; + + // These containers only have const accessors and must only be modified by Add()/Remove() MARKERS m_markers; DRAWINGS m_drawings; FOOTPRINTS m_footprints; diff --git a/pcbnew/dialogs/dialog_drc.cpp b/pcbnew/dialogs/dialog_drc.cpp index 2293e2d933..423e0b399f 100644 --- a/pcbnew/dialogs/dialog_drc.cpp +++ b/pcbnew/dialogs/dialog_drc.cpp @@ -848,21 +848,25 @@ void DIALOG_DRC::OnDRCItemRClick( wxDataViewEvent& aEvent ) m_ignoredList->InsertItem( m_ignoredList->GetItemCount(), wxT( " • " ) + rcItem->GetErrorText() ); - std::vector& markers = m_frame->GetBoard()->Markers(); + BOARD* board = m_frame->GetBoard(); + const std::vector& markers = board->Markers(); - for( unsigned i = 0; i < markers.size(); ) + std::vector toRemove; + + for( PCB_MARKER* marker : board->Markers() ) { - if( markers[i]->GetRCItem()->GetErrorCode() == rcItem->GetErrorCode() ) + if( marker->GetRCItem()->GetErrorCode() == rcItem->GetErrorCode() ) { - m_frame->GetCanvas()->GetView()->Remove( markers.at( i ) ); - markers.erase( markers.begin() + i ); - } - else - { - ++i; + m_frame->GetCanvas()->GetView()->Remove( marker ); + toRemove.emplace_back( marker ); } } + for( BOARD_ITEM* marker : toRemove ) + board->Remove( marker, REMOVE_MODE::BULK ); + + board->FinalizeBulkRemove( toRemove ); + if( rcItem->GetErrorCode() == DRCE_UNCONNECTED_ITEMS ) m_frame->GetCanvas()->RedrawRatsnest(); diff --git a/pcbnew/graphics_cleaner.cpp b/pcbnew/graphics_cleaner.cpp index 05e4c47dc3..5b13067c5e 100644 --- a/pcbnew/graphics_cleaner.cpp +++ b/pcbnew/graphics_cleaner.cpp @@ -36,7 +36,7 @@ #include #include -GRAPHICS_CLEANER::GRAPHICS_CLEANER( DRAWINGS& aDrawings, FOOTPRINT* aParentFootprint, +GRAPHICS_CLEANER::GRAPHICS_CLEANER( const DRAWINGS& aDrawings, FOOTPRINT* aParentFootprint, BOARD_COMMIT& aCommit, TOOL_MANAGER* aToolMgr ) : m_drawings( aDrawings ), m_parentFootprint( aParentFootprint ), @@ -410,4 +410,4 @@ void GRAPHICS_CLEANER::mergePads() m_itemsList->push_back( item ); } } -} \ No newline at end of file +} diff --git a/pcbnew/graphics_cleaner.h b/pcbnew/graphics_cleaner.h index c2f05e0e94..6df8a6b0ce 100644 --- a/pcbnew/graphics_cleaner.h +++ b/pcbnew/graphics_cleaner.h @@ -36,7 +36,7 @@ class TOOL_MANAGER; class GRAPHICS_CLEANER { public: - GRAPHICS_CLEANER( DRAWINGS& aDrawings, FOOTPRINT* aParentFootprint, BOARD_COMMIT& aCommit, + GRAPHICS_CLEANER( const DRAWINGS& aDrawings, FOOTPRINT* aParentFootprint, BOARD_COMMIT& aCommit, TOOL_MANAGER* aToolManager ); /** @@ -60,7 +60,7 @@ private: void mergePads(); private: - DRAWINGS& m_drawings; + const DRAWINGS& m_drawings; FOOTPRINT* m_parentFootprint; // nullptr if not in Footprint Editor BOARD_COMMIT& m_commit; TOOL_MANAGER* m_toolMgr; diff --git a/pcbnew/pcb_io/fabmaster/import_fabmaster.cpp b/pcbnew/pcb_io/fabmaster/import_fabmaster.cpp index c560202e21..c88296379c 100644 --- a/pcbnew/pcb_io/fabmaster/import_fabmaster.cpp +++ b/pcbnew/pcb_io/fabmaster/import_fabmaster.cpp @@ -3075,7 +3075,9 @@ bool FABMASTER::loadGraphics( BOARD* aBoard ) bool FABMASTER::orderZones( BOARD* aBoard ) { - std::sort( aBoard->Zones().begin(), aBoard->Zones().end(), + std::vector sortedZones; + std::copy( aBoard->Zones().begin(), aBoard->Zones().end(), std::back_inserter( sortedZones ) ); + std::sort( sortedZones.begin(), sortedZones.end(), [&]( const ZONE* a, const ZONE* b ) { if( a->GetLayer() == b->GetLayer() ) @@ -3087,7 +3089,7 @@ bool FABMASTER::orderZones( BOARD* aBoard ) PCB_LAYER_ID layer = UNDEFINED_LAYER; unsigned int priority = 0; - for( ZONE* zone : aBoard->Zones() ) + for( ZONE* zone : sortedZones ) { /// Rule areas do not have priorities if( zone->GetIsRuleArea() ) diff --git a/pcbnew/specctra_import_export/specctra_import.cpp b/pcbnew/specctra_import_export/specctra_import.cpp index d454dda816..de9246feac 100644 --- a/pcbnew/specctra_import_export/specctra_import.cpp +++ b/pcbnew/specctra_import_export/specctra_import.cpp @@ -341,12 +341,11 @@ void SPECCTRA_DB::FromSESSION( BOARD* aBoard ) // delete the old tracks and vias but save locked tracks/vias; they will be re-added later std::vector locked; + TRACKS tracks = aBoard->Tracks(); + aBoard->RemoveAll( { PCB_TRACE_T } ); - while( !aBoard->Tracks().empty() ) + for( PCB_TRACK* track : tracks ) { - PCB_TRACK* track = aBoard->Tracks().back(); - aBoard->Tracks().pop_back(); - if( track->IsLocked() ) { locked.push_back( track ); @@ -366,7 +365,7 @@ void SPECCTRA_DB::FromSESSION( BOARD* aBoard ) // Add locked tracks: because they are exported as Fix tracks, they are not // in .ses file. - for( PCB_TRACK* track: locked ) + for( PCB_TRACK* track : locked ) aBoard->Add( track ); if( m_session->placement ) diff --git a/pcbnew/tools/pcb_control.cpp b/pcbnew/tools/pcb_control.cpp index b459614622..37cb7c0deb 100644 --- a/pcbnew/tools/pcb_control.cpp +++ b/pcbnew/tools/pcb_control.cpp @@ -973,11 +973,12 @@ int PCB_CONTROL::Paste( const TOOL_EVENT& aEvent ) pastedItems.push_back( group ); } - clipBoard->Groups().clear(); + clipBoard->RemoveAll( { PCB_GROUP_T } ); for( FOOTPRINT* clipFootprint : clipBoard->Footprints() ) pasteFootprintItemsToFootprintEditor( clipFootprint, board(), pastedItems ); + for( BOARD_ITEM* clipDrawItem : clipBoard->Drawings() ) { switch( clipDrawItem->Type() ) @@ -1002,7 +1003,7 @@ int PCB_CONTROL::Paste( const TOOL_EVENT& aEvent ) } } - clipBoard->Drawings().clear(); + clipBoard->RemoveAll( { PCB_SHAPE_T } ); clipBoard->Visit( [&]( EDA_ITEM* item, void* testData ) @@ -1106,7 +1107,7 @@ int PCB_CONTROL::AppendBoardFromFile( const TOOL_EVENT& aEvent ) template -static void moveUnflaggedItems( std::deque& aList, std::vector& aTarget, +static void moveUnflaggedItems( const std::deque& aList, std::vector& aTarget, bool aIsNew ) { std::copy_if( aList.begin(), aList.end(), std::back_inserter( aTarget ), @@ -1119,54 +1120,26 @@ static void moveUnflaggedItems( std::deque& aList, std::vector& return doCopy; } ); - - if( aIsNew ) - aList.clear(); } -static void moveUnflaggedItems( ZONES& aList, std::vector& aTarget, bool aIsNew ) +template +static void moveUnflaggedItems( const std::vector& aList, std::vector& aTarget, + bool aIsNew ) { - if( aList.size() == 0 ) - return; - - auto obj = aList.front(); - int idx = 0; - - if( aIsNew ) - { - obj = aList.back(); - aList.pop_back(); - } - - for( ; obj ; ) - { - if( obj->HasFlag( SKIP_STRUCT ) ) - obj->ClearFlags( SKIP_STRUCT ); - else - aTarget.push_back( obj ); - - if( aIsNew ) - { - if( aList.size() ) + std::copy_if( aList.begin(), aList.end(), std::back_inserter( aTarget ), + [aIsNew]( T aItem ) { - obj = aList.back(); - aList.pop_back(); - } - else - { - obj = nullptr; - } - } - else - { - obj = idx < int(aList.size()-1) ? aList[++idx] : nullptr; - } - } + bool doCopy = ( aItem->GetFlags() & SKIP_STRUCT ) == 0; + + aItem->ClearFlags( SKIP_STRUCT ); + aItem->SetFlags( aIsNew ? IS_NEW : 0 ); + + return doCopy; + } ); } - bool PCB_CONTROL::placeBoardItems( BOARD_COMMIT* aCommit, BOARD* aBoard, bool aAnchorAtOrigin, bool aReannotateDuplicates ) { @@ -1188,6 +1161,17 @@ bool PCB_CONTROL::placeBoardItems( BOARD_COMMIT* aCommit, BOARD* aBoard, bool aA moveUnflaggedItems( aBoard->Generators(), items, isNew ); + if( isNew ) + aBoard->RemoveAll(); + + // Reparent before calling pruneItemLayers, as SetLayer can have a dependence on the + // item's parent board being set correctly. + if( isNew ) + { + for( BOARD_ITEM* item : items ) + item->SetParent( board() ); + } + pruneItemLayers( items ); return placeBoardItems( aCommit, items, isNew, aAnchorAtOrigin, aReannotateDuplicates ); diff --git a/pcbnew/zone_filler.cpp b/pcbnew/zone_filler.cpp index 706bdcfda8..3dff5699e8 100644 --- a/pcbnew/zone_filler.cpp +++ b/pcbnew/zone_filler.cpp @@ -87,7 +87,7 @@ void ZONE_FILLER::SetProgressReporter( PROGRESS_REPORTER* aReporter ) * * Caller is also responsible for re-building connectivity afterwards. */ -bool ZONE_FILLER::Fill( std::vector& aZones, bool aCheck, wxWindow* aParent ) +bool ZONE_FILLER::Fill( const std::vector& aZones, bool aCheck, wxWindow* aParent ) { std::lock_guard lock( m_board->GetConnectivity()->GetLock() ); diff --git a/pcbnew/zone_filler.h b/pcbnew/zone_filler.h index 466557b99e..5bfe809d54 100644 --- a/pcbnew/zone_filler.h +++ b/pcbnew/zone_filler.h @@ -55,7 +55,7 @@ public: * * Caller is also responsible for re-building connectivity afterwards. */ - bool Fill( std::vector& aZones, bool aCheck = false, wxWindow* aParent = nullptr ); + bool Fill( const std::vector& aZones, bool aCheck = false, wxWindow* aParent = nullptr ); bool IsDebug() const { return m_debugZoneFiller; } diff --git a/pcbnew/zone_manager/dialog_zone_manager.cpp b/pcbnew/zone_manager/dialog_zone_manager.cpp index 197207ab8f..56c2519a8e 100644 --- a/pcbnew/zone_manager/dialog_zone_manager.cpp +++ b/pcbnew/zone_manager/dialog_zone_manager.cpp @@ -402,12 +402,18 @@ void DIALOG_ZONE_MANAGER::OnButtonApplyClick( wxCommandEvent& aEvent ) m_filler = std::make_unique( board, commit.get() ); auto reporter = std::make_unique( this, _( "Fill All Zones" ), 5 ); m_filler->SetProgressReporter( reporter.get() ); - board->Zones() = m_zonesContainer->GetClonedZoneList(); + + // TODO: replace these const_cast calls with a different solution that avoids mutating the + // container of the board. This is relatively safe as-is because the original zones list is + // swapped back in below, but still should be changed to avoid invalidating the board state + // in case this code is refactored to be a non-modal dialog in the future. + const_cast( board->Zones() ) = m_zonesContainer->GetClonedZoneList(); + //NOTE - Nether revert nor commit is needed here , cause the cloned zones are not owned by // the pcb frame. m_zoneFillComplete = m_filler->Fill( board->Zones() ); board->BuildConnectivity(); - board->Zones() = m_zonesContainer->GetOriginalZoneList(); + const_cast( board->Zones() ) = m_zonesContainer->GetOriginalZoneList(); if( auto gal = m_zoneViewer->GetZoneGAL() ) {