From c9ccd8a642243c93789a7ecca24913f54381c51a Mon Sep 17 00:00:00 2001 From: jean-pierre charras Date: Fri, 28 Jun 2019 20:23:09 +0200 Subject: [PATCH] Fix broken TRACKS_CLEANER algo to merge collinear track segments. --- pcbnew/connectivity/connectivity_items.cpp | 24 ++++ pcbnew/connectivity/connectivity_items.h | 6 + pcbnew/tracks_cleaner.cpp | 121 ++++++++++++++++----- pcbnew/tracks_cleaner.h | 16 +++ 4 files changed, 142 insertions(+), 25 deletions(-) diff --git a/pcbnew/connectivity/connectivity_items.cpp b/pcbnew/connectivity/connectivity_items.cpp index 0cf1512e6c..df3d2759a2 100644 --- a/pcbnew/connectivity/connectivity_items.cpp +++ b/pcbnew/connectivity/connectivity_items.cpp @@ -285,6 +285,30 @@ bool CN_ANCHOR::IsDangling() const } +int CN_ANCHOR::ConnectedItemsCount() const +{ + if( !m_cluster ) + return 0; + + int connected_count = 0; + + for( auto item : m_item->ConnectedItems() ) + { + if( item->Parent()->Type() == PCB_ZONE_AREA_T ) + { + ZONE_CONTAINER* zone = static_cast( item->Parent() ); + + if( zone->HitTestFilledArea( wxPoint( Pos().x, Pos().y ) ) ) + connected_count++; + } + else if( item->Parent()->HitTest( wxPoint( Pos().x, Pos().y ) ) ) + connected_count++; + } + + return connected_count; +} + + CN_CLUSTER::CN_CLUSTER() { m_items.reserve( 64 ); diff --git a/pcbnew/connectivity/connectivity_items.h b/pcbnew/connectivity/connectivity_items.h index 0c24071983..e4a05f8db0 100644 --- a/pcbnew/connectivity/connectivity_items.h +++ b/pcbnew/connectivity/connectivity_items.h @@ -124,6 +124,12 @@ public: */ bool IsDangling() const; + /** + * has meaning only for tracks and vias. + * @return the count of items connected to this anchor + */ + int ConnectedItemsCount() const; + // Tag used for unconnected items. static const int TAG_UNCONNECTED = -1; diff --git a/pcbnew/tracks_cleaner.cpp b/pcbnew/tracks_cleaner.cpp index 220015a899..aa6b678f9b 100644 --- a/pcbnew/tracks_cleaner.cpp +++ b/pcbnew/tracks_cleaner.cpp @@ -252,6 +252,39 @@ bool TRACKS_CLEANER::testTrackEndpointDangling( TRACK* aTrack ) } +bool TRACKS_CLEANER::testTrackEndpointIsNode( TRACK* aTrack, bool aTstStart ) +{ + // A node is a point where more than 2 items are connected. + + auto connectivity = m_brd->GetConnectivity(); + auto items = connectivity->GetConnectivityAlgo()->ItemEntry( aTrack ).GetItems(); + + if( items.empty() ) + return false; + + auto citem = items.front(); + + if( !citem->Valid() ) + return false; + + auto anchors = citem->Anchors(); + + VECTOR2I refpoint = aTstStart ? aTrack->GetStart() : aTrack->GetEnd(); + + for( const auto& anchor : anchors ) + { + if( anchor->Pos() != refpoint ) + continue; + + // The right anchor point is found: if more than one other item + // (pad, via, track...) is connected, it is a node: + return anchor->ConnectedItemsCount() > 1; + } + + return false; +} + + bool TRACKS_CLEANER::deleteDanglingTracks() { bool item_erased = false; @@ -362,12 +395,13 @@ bool TRACKS_CLEANER::cleanupSegments() std::set toRemove; + // Remove duplicate segments (2 superimposed identical segments): for( auto it = m_brd->Tracks().begin(); it != m_brd->Tracks().end(); it++ ) { auto track1 = *it; if( track1->Type() != PCB_TRACE_T || ( track1->GetFlags() & IS_DELETED ) - || track1->IsLocked() ) + || track1->IsLocked() ) continue; for( auto it2 = it + 1; it2 != m_brd->Tracks().end(); it2++ ) @@ -400,6 +434,13 @@ bool TRACKS_CLEANER::cleanupSegments() for( auto track_it = m_brd->Tracks().begin(); track_it != m_brd->Tracks().end(); track_it++ ) { auto segment = *track_it; + + if( segment->Type() != PCB_TRACE_T ) // one can merge only track collinear segments, not vias. + continue; + + if( segment->GetFlags() & IS_DELETED ) // already taken in account + continue; + auto connectivity = m_brd->GetConnectivity(); auto& entry = connectivity->GetConnectivityAlgo()->ItemEntry( segment ); @@ -408,10 +449,21 @@ bool TRACKS_CLEANER::cleanupSegments() { for( auto connected : citem->ConnectedItems() ) { - if( connected->Valid() && connected->Parent()->Type() == PCB_TRACE_T - && !( connected->Parent()->GetFlags() & IS_DELETED ) ) + if( !connected->Valid() || connected->Parent()->Type() != PCB_TRACE_T || + ( connected->Parent()->GetFlags() & IS_DELETED ) ) + continue; + + if( !( connected->Parent()->GetFlags() & IS_DELETED ) ) { - if( segment->ApproxCollinear( *static_cast( connected->Parent() ) ) ) + TRACK* candidate = static_cast( connected->Parent() ); + + if( !candidate->IsOnLayer( segment->GetLayer() ) ) + continue; + + if( candidate->GetWidth() != segment->GetWidth() ) + continue; + + if( segment->ApproxCollinear( *candidate ) ) { modified |= mergeCollinearSegments( segment, static_cast( connected->Parent() ) ); @@ -432,6 +484,45 @@ bool TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 ) auto connectivity = m_brd->GetConnectivity(); + // Verify the removed point after merging is not a node. + // If it is a node (i.e. if more than one other item is connected, the segments cannot be merged + TRACK dummy_seg( *aSeg1 ); + + // Calculate the new ends of the segment to merge, and store them to dummy_seg: + int min_x = std::min( aSeg1->GetStart().x, + std::min( aSeg1->GetEnd().x, std::min( aSeg2->GetStart().x, aSeg2->GetEnd().x ) ) ); + int min_y = std::min( aSeg1->GetStart().y, + std::min( aSeg1->GetEnd().y, std::min( aSeg2->GetStart().y, aSeg2->GetEnd().y ) ) ); + int max_x = std::max( aSeg1->GetStart().x, + std::max( aSeg1->GetEnd().x, std::max( aSeg2->GetStart().x, aSeg2->GetEnd().x ) ) ); + int max_y = std::max( aSeg1->GetStart().y, + std::max( aSeg1->GetEnd().y, std::max( aSeg2->GetStart().y, aSeg2->GetEnd().y ) ) ); + + if( ( aSeg1->GetStart().x > aSeg1->GetEnd().x ) + == ( aSeg1->GetStart().y > aSeg1->GetEnd().y ) ) + { + dummy_seg.SetStart( wxPoint( min_x, min_y ) ); + dummy_seg.SetEnd( wxPoint( max_x, max_y ) ); + } + else + { + dummy_seg.SetStart( wxPoint( min_x, max_y ) ); + dummy_seg.SetEnd( wxPoint( max_x, min_y ) ); + } + + // Now find the removed end(s) and stop merging if it is a node: + if( aSeg1->GetStart() != dummy_seg.GetStart() && aSeg1->GetStart() != dummy_seg.GetEnd() ) + { + if( testTrackEndpointIsNode( aSeg1, true ) ) + return false; + } + + if( aSeg1->GetEnd() != dummy_seg.GetStart() && aSeg1->GetEnd() != dummy_seg.GetEnd() ) + { + if( testTrackEndpointIsNode( aSeg1, false ) ) + return false; + } + if( m_itemsList ) { m_itemsList->emplace_back( new DRC_ITEM( m_units, DRCE_MERGE_TRACKS, @@ -442,27 +533,7 @@ bool TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 ) if( !m_dryRun ) { m_commit.Modify( aSeg1 ); - - int min_x = std::min( aSeg1->GetStart().x, - std::min( aSeg1->GetEnd().x, std::min( aSeg2->GetStart().x, aSeg2->GetEnd().x ) ) ); - int min_y = std::min( aSeg1->GetStart().y, - std::min( aSeg1->GetEnd().y, std::min( aSeg2->GetStart().y, aSeg2->GetEnd().y ) ) ); - int max_x = std::max( aSeg1->GetStart().x, - std::max( aSeg1->GetEnd().x, std::max( aSeg2->GetStart().x, aSeg2->GetEnd().x ) ) ); - int max_y = std::max( aSeg1->GetStart().y, - std::max( aSeg1->GetEnd().y, std::max( aSeg2->GetStart().y, aSeg2->GetEnd().y ) ) ); - - if( ( aSeg1->GetStart().x > aSeg1->GetEnd().x ) - == ( aSeg1->GetStart().y > aSeg1->GetEnd().y ) ) - { - aSeg1->SetStart( wxPoint( min_x, min_y ) ); - aSeg1->SetEnd( wxPoint( max_x, max_y ) ); - } - else - { - aSeg1->SetStart( wxPoint( min_x, max_y ) ); - aSeg1->SetEnd( wxPoint( max_x, min_y ) ); - } + *aSeg1 = dummy_seg; connectivity->Update( aSeg1 ); diff --git a/pcbnew/tracks_cleaner.h b/pcbnew/tracks_cleaner.h index 19885cf982..f4a3dfe5b1 100644 --- a/pcbnew/tracks_cleaner.h +++ b/pcbnew/tracks_cleaner.h @@ -94,8 +94,24 @@ private: */ bool mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 ); + /** + * @return true if aTrack has at least one end dangling, i.e. connected + * to nothing. + * if aTrack is a via, it is dangling if the via is connected to nothing + * or only one item. + * @param aTrack is the track (or the via) to test. + */ bool testTrackEndpointDangling( TRACK* aTrack ); + /** + * @return true if a track end position is a node, i.e. a end connected + * to more than one item. + * @param aTrack is the track to test. + * @param aTstStart = true ot test the start point of the track, and false to + * test the end point + */ + bool testTrackEndpointIsNode( TRACK* aTrack, bool aTstStart ); + EDA_UNITS_T m_units; BOARD* m_brd; BOARD_COMMIT& m_commit;