From 247da631f9b84489b23f4c386600b88cce834d64 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Tue, 1 Jun 2021 11:13:43 -0400 Subject: [PATCH] Revert "Fix typo and clean up leftover tracks" This reverts commit ed66c0f14dcd17947ddd339094593de104615f12. Revert "Clean up co-linear tracks after finishing routing" This reverts commit 1a102f03c0a59cdac37cdaa740d845eb3c29730e. More complex solution needed Fixes https://gitlab.com/kicad/code/kicad/-/issues/8526 --- common/commit.cpp | 28 ------- include/commit.h | 11 --- .../dialog_cleanup_tracks_and_vias.cpp | 20 ++--- pcbnew/router/pns_kicad_iface.cpp | 10 --- pcbnew/router/pns_kicad_iface.h | 2 - pcbnew/tracks_cleaner.cpp | 74 +++++-------------- pcbnew/tracks_cleaner.h | 26 +++---- 7 files changed, 34 insertions(+), 137 deletions(-) diff --git a/common/commit.cpp b/common/commit.cpp index 2768bdc2ee..e49e147e36 100644 --- a/common/commit.cpp +++ b/common/commit.cpp @@ -144,21 +144,6 @@ 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 ); @@ -230,16 +215,3 @@ 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 e2b6859307..01b8e89a6c 100644 --- a/include/commit.h +++ b/include/commit.h @@ -130,12 +130,6 @@ 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; @@ -151,11 +145,6 @@ 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 b6cf5988cd..d551288eab 100644 --- a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp +++ b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp @@ -131,20 +131,12 @@ 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 ); - 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 ); + cleaner.CleanupBoard( aDryRun, &m_items, m_cleanShortCircuitOpt->GetValue(), + m_cleanViasOpt->GetValue(), + m_mergeSegmOpt->GetValue(), + m_deleteUnconnectedOpt->GetValue(), + m_deleteTracksInPadsOpt->GetValue(), + m_deleteDanglingViasOpt->GetValue() ); if( aDryRun ) { diff --git a/pcbnew/router/pns_kicad_iface.cpp b/pcbnew/router/pns_kicad_iface.cpp index 555256a418..85ba0708c3 100644 --- a/pcbnew/router/pns_kicad_iface.cpp +++ b/pcbnew/router/pns_kicad_iface.cpp @@ -31,7 +31,6 @@ #include #include #include -#include #include @@ -1642,15 +1641,6 @@ 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 d9dcb2357b..4a6c987752 100644 --- a/pcbnew/router/pns_kicad_iface.h +++ b/pcbnew/router/pns_kicad_iface.h @@ -127,8 +127,6 @@ 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 ca3dac030a..be2d6e879f 100644 --- a/pcbnew/tracks_cleaner.cpp +++ b/pcbnew/tracks_cleaner.cpp @@ -38,39 +38,37 @@ TRACKS_CLEANER::TRACKS_CLEANER( BOARD* aPcb, BOARD_COMMIT& aCommit ) : m_brd( aPcb ), m_commit( aCommit ), m_dryRun( true ), - m_includeNewTracks( false ), m_itemsList( nullptr ) { } -void TRACKS_CLEANER::CleanupBoard( bool aDryRun, - std::vector >* aItemsList, - int aFlags, bool aIncludeNewTracksInCommit ) +/* 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 ) { bool has_deleted = false; - m_dryRun = aDryRun; - m_itemsList = aItemsList; - m_includeNewTracks = aIncludeNewTracksInCommit; + m_dryRun = aDryRun; + m_itemsList = aItemsList; - 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 ); + cleanup( aCleanVias, aMergeSegments || aRemoveMisConnected, aMergeSegments, aMergeSegments ); - cleanup( stackedVias, mergeSegments || shortingSegments, mergeSegments, mergeSegments ); - - if( shortingSegments ) + if( aRemoveMisConnected ) removeShortingTrackSegments(); - if( aFlags & CF_SEGMENTS_INSIDE_PADS ) + if( aDeleteTracksinPad ) deleteTracksInPads(); - has_deleted = deleteDanglingTracks( danglingSegments, danglingVias ); + has_deleted = deleteDanglingTracks( aDeleteUnconnected, aDeleteDanglingVias ); - if( has_deleted && mergeSegments ) + if( has_deleted && aMergeSegments ) cleanup( false, false, false, true ); } @@ -397,21 +395,6 @@ void TRACKS_CLEANER::cleanup( bool aDeleteDuplicateVias, bool aDeleteNullSegment if( aMergeSegments ) { bool merged; - std::vector addedTracks; - - // 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 ); - addedTracks.emplace_back( static_cast( item ) ); - } - } - } do { @@ -459,14 +442,6 @@ void TRACKS_CLEANER::cleanup( bool aDeleteDuplicateVias, bool aDeleteNullSegment } } } while( merged ); - - // Remove any temporary tracks that still exist that were added above; they will be - // re-added by the commit - for( TRACK* track : addedTracks ) - { - if( m_commit.GetStatus( track ) != 0 ) - m_brd->Remove( track, REMOVE_MODE::BULK ); - } } for( TRACK* track : m_brd->Tracks() ) @@ -528,9 +503,7 @@ bool TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 ) if( !m_dryRun ) { - if( m_commit.GetStatus( aSeg1 ) == 0 ) - m_commit.Modify( aSeg1 ); - + m_commit.Modify( aSeg1 ); *aSeg1 = dummy_seg; connectivity->Update( aSeg1 ); @@ -544,18 +517,7 @@ bool TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 ) // Merge succesful, seg2 has to go away m_brd->Remove( aSeg2 ); - - if( m_commit.GetStatus( aSeg2 ) == 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; - } + m_commit.Removed( aSeg2 ); } return true; diff --git a/pcbnew/tracks_cleaner.h b/pcbnew/tracks_cleaner.h index 4f086abf6c..71c18af4e8 100644 --- a/pcbnew/tracks_cleaner.h +++ b/pcbnew/tracks_cleaner.h @@ -35,26 +35,21 @@ 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 aFlags controls which cleaning operations to run: @see CLEANUP_FLAGS - * @param aIncludeNewTracksInCommit will include to-be-committed tracks when merging segments + * @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 */ - void CleanupBoard( bool aDryRun, std::vector >* aItemsList, - int aFlags, bool aIncludeNewTracksInCommit = false ); + void CleanupBoard( bool aDryRun, std::vector >* aItemsList, bool aCleanVias, + bool aRemoveMisConnected, bool aMergeSegments, bool aDeleteUnconnected, + bool aDeleteTracksinPad, bool aDeleteDanglingVias ); private: /* @@ -102,7 +97,6 @@ private: BOARD* m_brd; BOARD_COMMIT& m_commit; bool m_dryRun; - bool m_includeNewTracks; std::vector >* m_itemsList; };