kicad-developers team mailing list archive

[Simon Richter <Simon.Richter@xxxxxxxxxx>]

---
 eeschema/sch_line.cpp   | 58 ++++++++++++++++++++++++++-----------------------
 eeschema/sch_line.h     | 16 ++++++++++----
 eeschema/sch_screen.cpp |  3 ++-
 3 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index 72efb5c..372885b 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -284,7 +284,7 @@ bool sort_by_ends_position(const wxPoint * ref, const wxPoint * tst )
  * MergeOverlap try to merge 2 lines that are colinear.
  * this function expects these 2 lines have at least a common end
  */
-bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
+bool SCH_LINE::CanMerge( const SCH_LINE* aLine ) const
 {
     wxCHECK_MSG( aLine != NULL && aLine->Type() == SCH_LINE_T, false,
                  wxT( "Cannot test line segment for overlap." ) );
@@ -292,22 +292,25 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
     if( this == aLine || GetLayer() != aLine->GetLayer() )
         return false;
 
+    auto rhs_start = aLine->m_start;
+    auto rhs_end = aLine->m_end;
+
     // Search for a common end:
-    if( m_start == aLine->m_start )
+    if( m_start == rhs_start )
     {
-        if( m_end == aLine->m_end )     // Trivial case
+        if( m_end == rhs_end )     // Trivial case
             return true;
     }
-    else if( m_start == aLine->m_end )
+    else if( m_start == rhs_end )
     {
-        if( m_end == aLine->m_start )     // Trivial case
+        if( m_end == rhs_start )     // Trivial case
             return true;
     }
-    else if( m_end == aLine->m_end )
+    else if( m_end == rhs_end )
     {
-        std::swap( aLine->m_start, aLine->m_end );
+        std::swap( rhs_start, rhs_end );
     }
-    else if( m_end != aLine->m_start )
+    else if( m_end != rhs_start )
     {
         // No common end point, segments cannot be merged.
         return false;
@@ -318,14 +321,14 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
     /* Test alignment: */
     if( m_start.y == m_end.y )       // Horizontal segment
     {
-        if( aLine->m_start.y == aLine->m_end.y )
+        if( rhs_start.y == rhs_end.y )
         {
             colinear = true;
         }
     }
     else if( m_start.x == m_end.x )  // Vertical segment
     {
-        if( aLine->m_start.x == aLine->m_end.x )
+        if( rhs_start.x == rhs_end.x )
         {
             colinear = true;
         }
@@ -333,32 +336,33 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
     else
     {
         if( atan2( (double) ( m_start.x - m_end.x ), (double) ( m_start.y - m_end.y ) )
-            == atan2( (double) ( aLine->m_start.x - aLine->m_end.x ),
-                      (double) ( aLine->m_start.y - aLine->m_end.y ) ) )
+            == atan2( (double) ( rhs_start.x - rhs_end.x ),
+                      (double) ( rhs_start.y - rhs_end.y ) ) )
         {
             colinear = true;
         }
     }
 
+    return colinear;
+}
+
+
+void SCH_LINE::MergeOverlap( SCH_LINE* aLine )
+{
     // Make a segment which merge the 2 segments
     // we must find the extremums
     // i.e. the more to the left and to the right points, or
     // for horizontal segments the uppermost and the lowest point
-    if( colinear )
-    {
-        static std::vector <wxPoint*> candidates;
-        candidates.clear();
-        candidates.push_back( &m_start );
-        candidates.push_back( &m_end );
-        candidates.push_back( &aLine->m_start );
-        candidates.push_back( &aLine->m_end );
-        sort( candidates.begin(), candidates.end(), sort_by_ends_position );
-        wxPoint tmp = *candidates[3];
-        m_start = *candidates[0];
-        m_end = tmp;
-        return true;
-    }
-    return false;
+    std::vector <wxPoint*> candidates;
+    candidates.reserve(4);
+    candidates.push_back( &m_start );
+    candidates.push_back( &m_end );
+    candidates.push_back( &aLine->m_start );
+    candidates.push_back( &aLine->m_end );
+    sort( candidates.begin(), candidates.end(), sort_by_ends_position );
+    wxPoint tmp = *candidates[3];
+    m_start = *candidates[0];
+    m_end = tmp;
 }
 
 
diff --git a/eeschema/sch_line.h b/eeschema/sch_line.h
index e52280c..a35d899 100644
--- a/eeschema/sch_line.h
+++ b/eeschema/sch_line.h
@@ -102,16 +102,24 @@ public:
     virtual void Rotate( wxPoint aPosition ) override;
 
     /**
-     * Check line against \a aLine to see if it overlaps and merge if it does.
+     * Check line against \a aLine to see if it overlaps.
+     * @param[in]   aLine   Line to be compared
+     * @return  whether lines can be merged
+     */
+    bool CanMerge( const SCH_LINE* aLine ) const;
+
+    /**
+     * Merge line with \a aLine
      *
      * This method will change the line to be equivalent of the line and \a aLine if the
      * two lines overlap.  This method is used to merge multiple line segments into a single
      * line.
      *
-     * @param aLine - Line to compare.
-     * @return True if lines overlap and the line was merged with \a aLine.
+     * This function will only work if CanMerge returned true.
+     *
+     * @param[in]   aLine   Line to merge
      */
-    bool MergeOverlap( SCH_LINE* aLine );
+    void MergeOverlap( SCH_LINE* aLine );
 
     virtual void GetEndPoints( std::vector<DANGLING_END_ITEM>& aItemList ) override;
 
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index a2655a7..93cd9d6 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -461,8 +461,9 @@ bool SCH_SCREEN::SchematicCleanUp()
             {
                 SCH_LINE* line = (SCH_LINE*) item;
 
-                if( line->MergeOverlap( (SCH_LINE*) testItem ) )
+                if( line->CanMerge( (SCH_LINE*) testItem ) )
                 {
+                    line->MergeOverlap( (SCH_LINE*) testItem );
                     // Keep the current flags, because the deleted segment can be flagged.
                     item->SetFlags( testItem->GetFlags() );
                     DeleteItem( testItem );