kicad-developers team mailing list archive

[Seth Hillbrand <seth.hillbrand@xxxxxxxxx>]

No, JP is right, that behavior should not occur in merging (it's a
secondary step when drawing).

JP, I assumed that this patch was not of interest, but I still needed the
functionality for junction management, so I merged it with a later patchset.

I've broken out the corrected patch for this functionality here.

Best-
Seth


On Fri, Nov 17, 2017 at 4:42 AM, Nick Østergaard <oe.nick@xxxxxxxxx> wrote:

> Isn't this the expected behavior?
>
> Usually you can sort of trim a wire by drawing from the end of it and into
> itself to remove that section.
>
> 2017-11-17 11:03 GMT+01:00 jp charras <jp.charras@xxxxxxxxxx>:
>
>> Le 10/10/2017 à 07:19, Seth Hillbrand a écrit :
>> > Sorry for the repeat on this one.  I missed a corner case in testing
>> the first time (overlapping,
>> > horizontal segments offset in Y)
>> >
>> > I've attached a collapsed patch for this.
>> >
>> > -Seth
>> >
>>
>> Hi Seth,
>>
>> I tested you patch and unfortunately it did not very well.
>>
>> For instance a basic case:
>>
>> draw a horizontal wire from point A to Point B.
>>
>> Then draw a second wire from the middle of AB to B.
>> the expected result is only on wire from A to B
>> but the result is one shorter wire from A to the middle of AB.
>>
>>
>> --
>> Jean-Pierre CHARRAS
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>

From 2f6ae50a879bd30f6e23a1b3cfa293a6a07c9b62 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 17 Nov 2017 08:35:20 -0800
Subject: [PATCH] Eeschema: Improve wire merging

CHANGED: wires now merge along the full length rather than just
endpoints.  Wires also merge at oblique angles when aligned.
---
 eeschema/sch_line.cpp | 108 ++++++++++++++++++++++++++++----------------------
 1 file changed, 60 insertions(+), 48 deletions(-)

diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index 4391ceb9c..ff49d6743 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -424,80 +424,92 @@ void SCH_LINE::Rotate( wxPoint aPosition )
  */
 bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
 {
+    auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
+    {
+        if( lhs.x == rhs.x )
+            return lhs.y < rhs.y;
+        return lhs.x < rhs.x;
+    };
+
     wxCHECK_MSG( aLine != NULL && aLine->Type() == SCH_LINE_T, false,
                  wxT( "Cannot test line segment for overlap." ) );
 
     if( this == aLine || GetLayer() != aLine->GetLayer() )
         return false;
 
-    // Search for a common end:
-    if( m_start == aLine->m_start )
-    {
-        if( m_end == aLine->m_end )     // Trivial case
-            return true;
-    }
-    else if( m_start == aLine->m_end )
-    {
-        if( m_end == aLine->m_start )     // Trivial case
-            return true;
-    }
-    else if( m_end == aLine->m_end )
+    SCH_LINE leftmost = SCH_LINE( *aLine );
+    SCH_LINE rightmost = SCH_LINE( *this );
+
+    // We place the start to the left and below the end of both lines
+    if( leftmost.m_start != std::min( { leftmost.m_start, leftmost.m_end }, less ) )
+        std::swap( leftmost.m_start, leftmost.m_end );
+    if( rightmost.m_start != std::min( { rightmost.m_start, rightmost.m_end }, less ) )
+        std::swap( rightmost.m_start, rightmost.m_end );
+
+    // -leftmost is the line that starts farthest to the left
+    // -other is the line that is _not_ leftmost
+    // -rightmost is the line that ends farthest to the right.  This may or
+    //   may not be 'other' as the second line may be completely covered by
+    //   the first.
+    if( less( rightmost.m_start, leftmost.m_start ) )
+        std::swap( leftmost, rightmost );
+
+    SCH_LINE other = SCH_LINE( rightmost );
+
+    if( less( rightmost.m_end, leftmost.m_end ) )
+        rightmost = leftmost;
+
+    // If we end one before the beginning of the other, no overlap is possible
+    if( less( leftmost.m_end, other.m_start ) )
     {
-        std::swap( aLine->m_start, aLine->m_end );
+        return false;
     }
-    else if( m_end != aLine->m_start )
+
+    // Search for a common end:
+    if( ( leftmost.m_start == other.m_start )
+            && ( leftmost.m_end == other.m_end ) )     // Trivial case
     {
-        // No common end point, segments cannot be merged.
-        return false;
+        m_start = leftmost.m_start;
+        m_end = leftmost.m_end;
+        return true;
     }
 
     bool colinear = false;
 
     /* Test alignment: */
-    if( m_start.y == m_end.y )       // Horizontal segment
+    if( ( leftmost.m_start.y == leftmost.m_end.y )
+            && ( other.m_start.y == other.m_end.y ) )       // Horizontal segment
     {
-        if( aLine->m_start.y == aLine->m_end.y )
-        {
-            colinear = true;
-        }
+        colinear = ( leftmost.m_start.y == other.m_start.y );
     }
-    else if( m_start.x == m_end.x )  // Vertical segment
+    else if( ( leftmost.m_start.x == leftmost.m_end.x )
+            && ( other.m_start.x == other.m_end.x ) )  // Vertical segment
     {
-        if( aLine->m_start.x == aLine->m_end.x )
-        {
-            colinear = true;
-        }
+        colinear = ( leftmost.m_start.x == other.m_start.x );
     }
     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 ) ) )
-        {
-            colinear = true;
-        }
+        // We use long long here to avoid overflow -- it enforces promotion
+        // Don't use double as we need to make a direct comparison
+        // The slope of the left-most line is dy/dx.  Then we check that the slope
+        // from the left most start to the right most start is the same as well as
+        // the slope from the left most start to right most end.
+        long long dx = leftmost.m_end.x - leftmost.m_start.x;
+        long long dy = leftmost.m_end.y - leftmost.m_start.y;
+        colinear = ( ( ( other.m_start.y - leftmost.m_start.y ) * dx ==
+                ( other.m_start.x - leftmost.m_start.x ) * dy ) &&
+            ( ( other.m_end.y - leftmost.m_start.y ) * dx ==
+                ( other.m_end.x - leftmost.m_start.x ) * dy ) );
     }
 
-    // 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
+    // Make a new segment that merges the 2 segments
     if( colinear )
     {
-        auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
-        {
-            if( lhs.x == rhs.x )
-                return lhs.y < rhs.y;
-            return lhs.x < rhs.x;
-        };
-
-        wxPoint top_left = std::min( { m_start, m_end, aLine->m_start, aLine->m_end }, less );
-        wxPoint bottom_right = std::max( { m_start, m_end, aLine->m_start, aLine->m_end }, less );
-
-        m_start = top_left;
-        m_end = bottom_right;
+        m_start = leftmost.m_start;
+        m_end = rightmost.m_end;
         return true;
     }
+
     return false;
 }
 
-- 
2.11.0