diff --git a/common/commit.cpp b/common/commit.cpp index e49e147e36..2768bdc2ee 100644 --- a/common/commit.cpp +++ b/common/commit.cpp @@ -144,6 +144,21 @@ void eraseIf( Container& c, F&& f ) } +COMMIT& COMMIT::Unstage( EDA_ITEM* aItem ) +{ + if( m_changedItems.find( aItem ) != m_changedItems.end() ) + { + eraseIf( m_changes, + [aItem] ( const COMMIT_LINE& aEnt ) + { + return aEnt.m_item == aItem; + } ); + } + + return *this; +} + + COMMIT& COMMIT::createModified( EDA_ITEM* aItem, EDA_ITEM* aCopy, int aExtraFlags ) { EDA_ITEM* parent = parentObject( aItem ); @@ -215,3 +230,16 @@ CHANGE_TYPE COMMIT::convert( UNDO_REDO aType ) const } } + +std::vector COMMIT::GetAddedItems() const +{ + std::vector ret; + + for( const COMMIT_LINE& change : m_changes ) + { + if( change.m_type == CHT_ADD ) + ret.emplace_back( change.m_item ); + } + + return ret; +} diff --git a/include/commit.h b/include/commit.h index 01b8e89a6c..e2b6859307 100644 --- a/include/commit.h +++ b/include/commit.h @@ -130,6 +130,12 @@ public: virtual COMMIT& Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED ); + /** + * Removes an item that was previously staged to this commit from the commit. + * Use this with care: + */ + virtual COMMIT& Unstage( EDA_ITEM* aItem ); + ///< Execute the changes. virtual void Push( const wxString& aMessage = wxT( "A commit" ), bool aCreateUndoEntry = true, bool aSetDirtyBit = true ) = 0; @@ -145,6 +151,11 @@ public: ///< Returns status of an item. int GetStatus( EDA_ITEM* aItem ); + /** + * @return a list of items added by this commit + */ + std::vector GetAddedItems() const; + protected: struct COMMIT_LINE { diff --git a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp index d551288eab..b6cf5988cd 100644 --- a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp +++ b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp @@ -131,12 +131,20 @@ void DIALOG_CLEANUP_TRACKS_AND_VIAS::doCleanup( bool aDryRun ) // Old model has to be refreshed, GAL normally does not keep updating it m_parentFrame->Compile_Ratsnest( false ); - cleaner.CleanupBoard( aDryRun, &m_items, m_cleanShortCircuitOpt->GetValue(), - m_cleanViasOpt->GetValue(), - m_mergeSegmOpt->GetValue(), - m_deleteUnconnectedOpt->GetValue(), - m_deleteTracksInPadsOpt->GetValue(), - m_deleteDanglingViasOpt->GetValue() ); + int flags = ( m_cleanShortCircuitOpt->GetValue() ? + TRACKS_CLEANER::CF_SHORTING_SEGMENTS : 0 ) | + ( m_cleanViasOpt->GetValue() ? + TRACKS_CLEANER::CF_STACKED_VIAS : 0 ) | + ( m_mergeSegmOpt->GetValue() ? + TRACKS_CLEANER::CF_COLLINEAR_SEGMENTS : 0 ) | + ( m_deleteUnconnectedOpt->GetValue() ? + TRACKS_CLEANER::CF_DANGLING_SEGMENTS : 0 ) | + ( m_deleteTracksInPadsOpt->GetValue() ? + TRACKS_CLEANER::CF_SEGMENTS_INSIDE_PADS : 0 ) | + ( m_deleteDanglingViasOpt->GetValue() ? + TRACKS_CLEANER::CF_DANGLING_VIAS : 0 ); + + cleaner.CleanupBoard( aDryRun, &m_items, flags ); if( aDryRun ) { diff --git a/pcbnew/router/pns_kicad_iface.cpp b/pcbnew/router/pns_kicad_iface.cpp index 85ba0708c3..555256a418 100644 --- a/pcbnew/router/pns_kicad_iface.cpp +++ b/pcbnew/router/pns_kicad_iface.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include @@ -1641,6 +1642,15 @@ void PNS_KICAD_IFACE::Commit() m_fpOffsets.clear(); + // If we have added routes, those may result in co-linear segments if we completed a route on + // top of an existing stub. These won't be cleaned by the optimizer as the stub won't be pulled + // in to the newly-routed line, so we have to post-process them. + TRACKS_CLEANER cleaner( m_board, *m_commit.get() ); + std::vector> items; + + // Cleanup: only merge segments + cleaner.CleanupBoard( false, &items, TRACKS_CLEANER::CF_COLLINEAR_SEGMENTS, true ); + m_commit->Push( _( "Interactive Router" ) ); m_commit = std::make_unique( m_tool ); } diff --git a/pcbnew/router/pns_kicad_iface.h b/pcbnew/router/pns_kicad_iface.h index 4a6c987752..d9dcb2357b 100644 --- a/pcbnew/router/pns_kicad_iface.h +++ b/pcbnew/router/pns_kicad_iface.h @@ -127,6 +127,8 @@ public: void UpdateNet( int aNetCode ) override; + BOARD_COMMIT* GetCommit() const { return m_commit.get(); } + private: struct OFFSET { diff --git a/pcbnew/tracks_cleaner.cpp b/pcbnew/tracks_cleaner.cpp index be2d6e879f..d48ed9d7f4 100644 --- a/pcbnew/tracks_cleaner.cpp +++ b/pcbnew/tracks_cleaner.cpp @@ -38,37 +38,39 @@ TRACKS_CLEANER::TRACKS_CLEANER( BOARD* aPcb, BOARD_COMMIT& aCommit ) : m_brd( aPcb ), m_commit( aCommit ), m_dryRun( true ), + m_includeNewTracks( false ), m_itemsList( nullptr ) { } -/* Main cleaning function. - * Delete - * - Redundant points on tracks (merge aligned segments) - * - vias on pad - * - null length segments - */ -void TRACKS_CLEANER::CleanupBoard( bool aDryRun, std::vector >* aItemsList, - bool aRemoveMisConnected, bool aCleanVias, bool aMergeSegments, - bool aDeleteUnconnected, bool aDeleteTracksinPad, bool aDeleteDanglingVias ) +void TRACKS_CLEANER::CleanupBoard( bool aDryRun, + std::vector >* aItemsList, + int aFlags, bool aIncludeNewTracksInCommit ) { bool has_deleted = false; - m_dryRun = aDryRun; - m_itemsList = aItemsList; + m_dryRun = aDryRun; + m_itemsList = aItemsList; + m_includeNewTracks = aIncludeNewTracksInCommit; - cleanup( aCleanVias, aMergeSegments || aRemoveMisConnected, aMergeSegments, aMergeSegments ); + bool stackedVias = ( aFlags & CF_STACKED_VIAS ); + bool mergeSegments = ( aFlags & CF_COLLINEAR_SEGMENTS ); + bool shortingSegments = ( aFlags & CF_SHORTING_SEGMENTS ); + bool danglingSegments = ( aFlags & CF_DANGLING_SEGMENTS ); + bool danglingVias = ( aFlags & CF_DANGLING_VIAS ); - if( aRemoveMisConnected ) + cleanup( stackedVias, mergeSegments || shortingSegments, mergeSegments, mergeSegments ); + + if( shortingSegments ) removeShortingTrackSegments(); - if( aDeleteTracksinPad ) + if( aFlags & CF_SEGMENTS_INSIDE_PADS ) deleteTracksInPads(); - has_deleted = deleteDanglingTracks( aDeleteUnconnected, aDeleteDanglingVias ); + has_deleted = deleteDanglingTracks( danglingSegments, danglingVias ); - if( has_deleted && aMergeSegments ) + if( has_deleted && mergeSegments ) cleanup( false, false, false, true ); } @@ -396,6 +398,17 @@ void TRACKS_CLEANER::cleanup( bool aDeleteDuplicateVias, bool aDeleteNullSegment { bool merged; + // We need to add any tracks that were added in the commit, to handle the case where this is + // called from the router and we are cleaning up right before the router commit is pushed. + if( m_includeNewTracks ) + { + for( EDA_ITEM* item : m_commit.GetAddedItems() ) + { + if( item->Type() == PCB_TRACE_T ) + m_brd->Add( static_cast( item ), ADD_MODE::BULK_APPEND ); + } + } + do { merged = false; @@ -503,7 +516,9 @@ bool TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 ) if( !m_dryRun ) { - m_commit.Modify( aSeg1 ); + if( m_commit.GetStatus( aSeg1 ) == 0 ) + m_commit.Modify( aSeg1 ); + *aSeg1 = dummy_seg; connectivity->Update( aSeg1 ); @@ -517,7 +532,18 @@ bool TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 ) // Merge succesful, seg2 has to go away m_brd->Remove( aSeg2 ); - m_commit.Removed( aSeg2 ); + + if( m_commit.GetStatus( aSeg1 ) == 0 ) + { + m_commit.Removed( aSeg2 ); + } + else + { + // This track was a temporary item about to be added to the board by the router. It's + // no longer needed, so let's actually get rid of it. + m_commit.Unstage( aSeg2 ); + delete aSeg2; + } } return true; diff --git a/pcbnew/tracks_cleaner.h b/pcbnew/tracks_cleaner.h index 71c18af4e8..4f086abf6c 100644 --- a/pcbnew/tracks_cleaner.h +++ b/pcbnew/tracks_cleaner.h @@ -35,21 +35,26 @@ class CLEANUP_ITEM; class TRACKS_CLEANER { public: + + enum CLEANUP_FLAGS + { + CF_STACKED_VIAS = ( 1 << 0 ), ///< Remove superimposed vias + CF_DANGLING_VIAS = ( 1 << 1 ), ///< Remove vias that only connect on one layer + CF_SHORTING_SEGMENTS = ( 1 << 2 ), ///< Remove segments that short two nets + CF_COLLINEAR_SEGMENTS = ( 1 << 3 ), ///< Merge co-linear segments + CF_DANGLING_SEGMENTS = ( 1 << 4 ), ///< Remove unconnected segments + CF_SEGMENTS_INSIDE_PADS = ( 1 << 5 ) ///< Remove segments fully inside a pad + }; + TRACKS_CLEANER( BOARD* aPcb, BOARD_COMMIT& aCommit ); /** * the cleanup function. - * @param aCleanVias = true to remove superimposed vias - * @param aRemoveMisConnected = true to remove segments connecting 2 different nets - * (short circuits) - * @param aMergeSegments = true to merge collinear segmenst and remove 0 len segm - * @param aDeleteUnconnected = true to remove dangling tracks - * @param aDeleteTracksinPad = true to remove tracks fully inside pads - * @param aDeleteDanglingVias = true to remove a via that is only connected to a single layer + * @param aFlags controls which cleaning operations to run: @see CLEANUP_FLAGS + * @param aIncludeNewTracksInCommit will include to-be-committed tracks when merging segments */ - void CleanupBoard( bool aDryRun, std::vector >* aItemsList, bool aCleanVias, - bool aRemoveMisConnected, bool aMergeSegments, bool aDeleteUnconnected, - bool aDeleteTracksinPad, bool aDeleteDanglingVias ); + void CleanupBoard( bool aDryRun, std::vector >* aItemsList, + int aFlags, bool aIncludeNewTracksInCommit = false ); private: /* @@ -97,6 +102,7 @@ private: BOARD* m_brd; BOARD_COMMIT& m_commit; bool m_dryRun; + bool m_includeNewTracks; std::vector >* m_itemsList; };