← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Eeschema automatic manage junctions

 

Thank you Nick for testing!  Looks like a missed a `break` when
re-formatting from the first commit.

I've corrected the issue in the attached patch set.

-Seth

On Sat, Oct 28, 2017 at 5:49 AM, Nick Østergaard <oe.nick@xxxxxxxxx> wrote:

> Hello Seth
>
> I am not entirely sure what these patches actually do, but I tired to test
> them.
>
> But I have major problems with dragging stuff around in kicad. On the
> attached I dragged a capacitor to the T junction on the wire above it.
>
> If I make the asserts continue I get a blank page eventually.
>
> Nick
>
>
>
> 2017-10-28 2:09 GMT+02:00 Seth Hillbrand <seth.hillbrand@xxxxxxxxx>:
>
>> Hi All-
>>
>> Quick ping on this.  Master has moved on, so I've re-based the patch set
>> to the current master, in case anyone is up for looking at it.  I also
>> split out the largest patch into a couple of logical blocks, in hopes of
>> making it easier to review.
>>
>> Lastly, I found and fixed a corner case where removing two segments on
>> the same junction broke the undo/redo stack.
>>
>> Thanks!
>> Seth
>>
>> On Mon, Oct 16, 2017 at 5:27 PM, Seth Hillbrand <seth.hillbrand@xxxxxxxxx
>> > wrote:
>>
>>> Attached is a proposed patchset for automatically managing junctions​,
>>> adding and deleting them as they are needed by the schematic.
>>>
>>> The patches touch a number of sections, so any reports of issues would
>>> be greatly appreciated.  Notably, this introduces an "append" feature to
>>> eeschema's undo save, which allows us to stack changes from different calls
>>> into the undo list.  This removes the issue where "Delete Connection" would
>>> break the undo stack.
>>>
>>> Schematic cleanup is moved from the SCH_SCREEN class into the
>>> SCH_EDIT_FRAME class to provide access to the undo stack.
>>>
>>> Cleanup now checks for overlapping segments as well as duplicate
>>> junctions and merge-able lines.
>>>
>>> Bugs reports addressed by this patchset:
>>> https://bugs.launchpad.net/kicad/+bug/1563153
>>> https://bugs.launchpad.net/kicad/+bug/593888
>>> https://bugs.launchpad.net/kicad/+bug/1482111
>>>
>>> Any feedback would be greatly appreciated.
>>>
>>> -S​
>>>
>>>
>>
>> _______________________________________________
>> 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 849ac7e93f708018141ed88326f7dada2eb4df40 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 6 Oct 2017 10:42:55 -0700
Subject: [PATCH 01/13] Eeschema: Improve performance and merge overlapping
 segments

When adding segments, eeschema should check both slope and overlap
instead of only overlapping endpoints.
---
 eeschema/sch_line.cpp | 69 +++++++++++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index 21b6fe8fd..2c1379155 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -273,12 +273,36 @@ void SCH_LINE::Rotate( wxPoint aPosition )
  */
 bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
 {
+    SCH_LINE *leftmost, *rightmost;
     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;
 
+    auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
+    {
+        if( lhs.x == rhs.x )
+            return lhs.y < rhs.y;
+        return lhs.x < rhs.x;
+    };
+
+    if( m_start != std::min( { m_start, m_end }, less ) )
+        std::swap( m_start, m_end );
+    if( aLine->m_start != std::min( { aLine->m_start, aLine->m_end }, less ) )
+        std::swap( aLine->m_start, aLine->m_end );
+
+    if( less( m_start, aLine->m_start ) )
+    {
+        leftmost = this;
+        rightmost = aLine;
+    }
+    else
+    {
+        leftmost = aLine;
+        rightmost = this;
+    }
+
     // Search for a common end:
     if( m_start == aLine->m_start )
     {
@@ -296,35 +320,30 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
     }
     else if( m_end != aLine->m_start )
     {
-        // No common end point, segments cannot be merged.
-        return false;
+        // No common end point, check for maybe overlapping
+        if( less ( leftmost->m_end, rightmost->m_start ) )
+            return false;
     }
 
     bool colinear = false;
 
     /* Test alignment: */
-    if( m_start.y == m_end.y )       // Horizontal segment
+    if( ( m_start.y == m_end.y ) && ( aLine->m_start.y == aLine->m_end.y ) )       // Horizontal segment
     {
-        if( aLine->m_start.y == aLine->m_end.y )
-        {
-            colinear = true;
-        }
+        colinear = ( m_start.y == aLine->m_start.y );
     }
-    else if( m_start.x == m_end.x )  // Vertical segment
+    else if( ( m_start.x == m_end.x ) && ( aLine->m_start.x == aLine->m_end.x ) )  // Vertical segment
     {
-        if( aLine->m_start.x == aLine->m_end.x )
-        {
-            colinear = true;
-        }
+        colinear = ( m_start.x == aLine->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;
-        }
+        int dx = leftmost->m_end.x - leftmost->m_start.x;
+        int dy = leftmost->m_end.y - leftmost->m_start.y;
+        colinear = ( ( ( rightmost->m_start.y - leftmost->m_start.y ) * dx ==
+                ( rightmost->m_start.x - leftmost->m_start.x ) * dy ) &&
+            ( ( rightmost->m_end.y - leftmost->m_start.y ) * dx ==
+                ( rightmost->m_end.x - leftmost->m_start.x ) * dy ) );
     }
 
     // Make a segment which merge the 2 segments
@@ -333,18 +352,8 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
     // for horizontal segments the uppermost and the lowest point
     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

From 012ccf9ece412f81d585c5e21258a3f2a32e6bb5 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 14:08:14 -0700
Subject: [PATCH 02/13] Optimize speed of TestSegmentHit

Improves speed of function by exiting on non-hits more quickly for
the majority of tests.  Hits are also calculated slightly faster.
---
 common/trigo.cpp | 173 +++++++++++++------------------------------------------
 1 file changed, 40 insertions(+), 133 deletions(-)

diff --git a/common/trigo.cpp b/common/trigo.cpp
index 96dafa00e..30ae2ee8d 100644
--- a/common/trigo.cpp
+++ b/common/trigo.cpp
@@ -125,146 +125,53 @@ bool SegmentIntersectsSegment( const wxPoint &a_p1_l1, const wxPoint &a_p2_l1,
  * aRefPoint = reference point to test
  * aStart, aEnd are coordinates of end points segment
  * aDist = maximum distance for hit
- * Note: for calculation time reasons, the distance between the ref point
- * and the segment is not always exactly calculated
- * (we only know if the actual dist is < aDist, not exactly know this dist.
- * Because many times we have horizontal or vertical segments,
- * a special calcultaion is made for them
- * Note: sometimes we need to calculate the distande between 2 points
- * A square root should be calculated.
- * However, because we just compare 2 distnaces, to avoid calculating square root,
- * the square of distances are compared.
+ *
 */
-static inline double square( int x )    // helper function to calculate x*x
-{
-    return (double) x * x;
-}
+
 bool TestSegmentHit( const wxPoint &aRefPoint, wxPoint aStart,
                      wxPoint aEnd, int aDist )
 {
-    // test for vertical or horizontal segment
-    if( aEnd.x == aStart.x )
-    {
-        // vertical segment
-        int ll = abs( aRefPoint.x - aStart.x );
-
-        if( ll > aDist )
-            return false;
-
-        // To have only one case to examine, ensure aEnd.y > aStart.y
-        if( aEnd.y < aStart.y )
-            std::swap( aStart.y, aEnd.y );
-
-        if( aRefPoint.y <= aEnd.y && aRefPoint.y >= aStart.y )
-            return true;
-
-        // there is a special case: x,y near an end point (distance < dist )
-        // the distance should be carefully calculated
-        if( (aStart.y - aRefPoint.y) < aDist )
-        {
-            double dd = square( aRefPoint.x - aStart.x) +
-                 square( aRefPoint.y - aStart.y );
-            if( dd <= square( aDist ) )
-                return true;
-        }
-
-        if( (aRefPoint.y - aEnd.y) < aDist )
-        {
-            double dd = square( aRefPoint.x - aEnd.x ) +
-                 square( aRefPoint.y - aEnd.y );
-            if( dd <= square( aDist ) )
-                return true;
-        }
-    }
-    else if( aEnd.y == aStart.y )
-    {
-        // horizontal segment
-        int ll = abs( aRefPoint.y - aStart.y );
-
-        if( ll > aDist )
-            return false;
-
-        // To have only one case to examine, ensure xf > xi
-        if( aEnd.x < aStart.x )
-            std::swap( aStart.x, aEnd.x );
-
-        if( aRefPoint.x <= aEnd.x && aRefPoint.x >= aStart.x )
-            return true;
-
-        // there is a special case: x,y near an end point (distance < dist )
-        // the distance should be carefully calculated
-        if( (aStart.x - aRefPoint.x) <= aDist )
-        {
-            double dd = square( aRefPoint.x - aStart.x ) +
-                        square( aRefPoint.y - aStart.y );
-            if( dd <= square( aDist ) )
-                return true;
-        }
-
-        if( (aRefPoint.x - aEnd.x) <= aDist )
-        {
-            double dd = square( aRefPoint.x - aEnd.x ) +
-                        square( aRefPoint.y - aEnd.y );
-            if( dd <= square( aDist ) )
-                return true;
-        }
-    }
-    else
+    int xmin = aStart.x;
+    int xmax = aEnd.x;
+    int ymin = aStart.y;
+    int ymax = aEnd.y;
+    double dist_square = (double)aDist * aDist;
+    double length_square;
+    double delta_x, delta_y;
+
+    if( xmax < xmin )
+        std::swap( xmax, xmin );
+    if( ymax < ymin )
+        std::swap( ymax, ymin );
+
+    // First, check if we are outside of the bounding box
+    if ( ( ymin - aRefPoint.y > aDist ) || ( aRefPoint.y - ymax > aDist ) )
+        return false;
+    if ( ( xmin - aRefPoint.x > aDist ) || ( aRefPoint.x - xmax > aDist ) )
+        return false;
+
+    // Next, deal with a potential zero-length case
+    if ( aStart == aEnd )
     {
-        // oblique segment:
-        // First, we need to calculate the distance between the point
-        // and the line defined by aStart and aEnd
-        // this dist should be < dist
-        //
-        // find a,slope such that aStart and aEnd lie on y = a + slope*x
-        double  slope   = (double) (aEnd.y - aStart.y) / (aEnd.x - aStart.x);
-        double  a   = (double) aStart.y - slope * aStart.x;
-        // find c,orthoslope such that (x,y) lies on y = c + orthoslope*x,
-        // where orthoslope=(-1/slope)
-        // to calculate xp, yp = near point from aRefPoint
-        // which is on the line defined by aStart, aEnd
-        double  orthoslope   = -1.0 / slope;
-        double  c   = (double) aRefPoint.y - orthoslope * aRefPoint.x;
-        // find nearest point to (x,y) on line defined by aStart, aEnd
-        double  xp  = (a - c) / (orthoslope - slope);
-        double  yp  = a + slope * xp;
-        // find distance to line, in fact the square of dist,
-        // because we just know if it is > or < aDist
-        double dd = square( aRefPoint.x - xp ) + square( aRefPoint.y - yp );
-        double dist = square( aDist );
-
-        if( dd > dist )    // this reference point is not a good candiadte.
-            return false;
-
-        // dd is < dist, therefore we should make a fine test
-        if( fabs( slope ) > 0.7 )
-        {
-            // line segment more vertical than horizontal
-            if( (aEnd.y > aStart.y && yp <= aEnd.y && yp >= aStart.y) ||
-                (aEnd.y < aStart.y && yp >= aEnd.y && yp <= aStart.y) )
-                return true;
-        }
-        else
-        {
-            // line segment more horizontal than vertical
-            if( (aEnd.x > aStart.x && xp <= aEnd.x && xp >= aStart.x) ||
-                (aEnd.x < aStart.x && xp >= aEnd.x && xp <= aStart.x) )
-                return true;
-        }
-
-        // Here, the test point is still a good candidate,
-        // however it is not "between" the end points of the segment.
-        // It is "outside" the segment, but it could be near a segment end point
-        // Therefore, we test the dist from the test point to each segment end point
-        dd = square( aRefPoint.x - aEnd.x ) + square( aRefPoint.y - aEnd.y );
-        if( dd <= dist )
-            return true;
-        dd = square( aRefPoint.x - aStart.x ) + square( aRefPoint.y - aStart.y );
-        if( dd <= dist )
-            return true;
+        delta_x = aStart.x - aRefPoint.x;
+        delta_y = aStart.y - aRefPoint.y;
+        return ( delta_x * delta_x + delta_y * delta_y <= dist_square);
     }
 
-    return false;    // no hit
+    delta_x = aEnd.x - aStart.x;
+    delta_y = aEnd.y - aStart.y;
+    length_square = delta_x * delta_x + delta_y * delta_y;
+    // This is now the projection of the vector from start to reference onto the segment
+    double u = (aRefPoint.x - aStart.x) * delta_x + (aRefPoint.y - aStart.y) * delta_y;
+    // Sanity check that the projection is never negative or longer than the element itself
+    u = std::min( std::max( 0., u ), length_square );
+
+    // Calculate the x,y offsets by shifting from the start along the projection
+    // to the nearest point on the segment
+    double x = ( aStart.x - aRefPoint.x) + ( u * delta_x ) / length_square;
+    double y = ( aStart.y - aRefPoint.y) + ( u * delta_y ) / length_square;
+
+    return dist_square >= x * x + y * y;
 }
 
 
-- 
2.11.0

From ecc2f6b116ac045209b9d766d4798f10464858f0 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 26 Oct 2017 09:53:17 -0700
Subject: [PATCH 03/13] Eeschema: Simplify GetItem

Avoids calling HitTest on each item when it is not needed
---
 eeschema/sch_screen.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index 02f2030d1..a87a3b519 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -204,9 +204,6 @@ SCH_ITEM* SCH_SCREEN::GetItem( const wxPoint& aPosition, int aAccuracy, KICAD_T
 {
     for( SCH_ITEM* item = m_drawList.begin(); item; item = item->Next() )
     {
-        if( item->HitTest( aPosition, aAccuracy ) && (aType == NOT_USED) )
-            return item;
-
         if( (aType == SCH_FIELD_T) && (item->Type() == SCH_COMPONENT_T) )
         {
             SCH_COMPONENT* component = (SCH_COMPONENT*) item;
@@ -228,7 +225,8 @@ SCH_ITEM* SCH_SCREEN::GetItem( const wxPoint& aPosition, int aAccuracy, KICAD_T
             if( label )
                 return (SCH_ITEM*) label;
         }
-        else if( (item->Type() == aType) && item->HitTest( aPosition, aAccuracy ) )
+        else if( ( ( item->Type() == aType ) || ( aType == NOT_USED ) )
+                && item->HitTest( aPosition, aAccuracy ) )
         {
             return item;
         }
-- 
2.11.0

From a8d7ab6b554377ca6310a1e9c9c9339cef142eb4 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 15:07:55 -0700
Subject: [PATCH 04/13] Eeschema: Correctly enumerate juction requirements

Fixes: lp:1563153
* https://bugs.launchpad.net/kicad/+bug/1563153
---
 eeschema/class_sch_screen.h | 14 +++++++-------
 eeschema/sch_screen.cpp     | 36 +++++++++++++++++++++++++++---------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/eeschema/class_sch_screen.h b/eeschema/class_sch_screen.h
index 82c95948c..a0e25cf73 100644
--- a/eeschema/class_sch_screen.h
+++ b/eeschema/class_sch_screen.h
@@ -364,19 +364,19 @@ public:
      * Function IsJunctionNeeded
      * tests if a junction is required for the items at \a aPosition on the screen.
      * <p>
-     * A junction is required at \a aPosition if the following criteria are satisfied:
+     * A junction is required at \a aPosition if one of the following criteria is satisfied:
      * <ul>
-     * <li>one wire midpoint, one or more wire endpoints and no junction.</li>
-     * <li>three or more wire endpoints and no junction.</li>
-     * <li>two wire midpoints and no junction</li>
-     * <li>one wire midpoint, a component pin, and no junction.</li>
-     * <li>three wire endpoints, a component pin, and no junction.</li>
+     * <li>one wire midpoint and one or more wire endpoints;</li>
+     * <li>three or more wire endpoints;</li>
+     * <li>one wire midpoint and a component pin;</li>
+     * <li>two or more wire endpoints and a component pin.</li>
      * </ul>
      * </p>
      * @param aPosition The position to test.
+     * @param aNew Checks if a _new_ junction is needed, i.e. there isn't one already
      * @return True if a junction is required at \a aPosition.
      */
-    bool IsJunctionNeeded( const wxPoint& aPosition );
+    bool IsJunctionNeeded( const wxPoint& aPosition, bool aNew = false );
 
     /**
      * Function IsTerminalPoint
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index a87a3b519..67449ac7b 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -332,20 +332,38 @@ void SCH_SCREEN::MarkConnections( SCH_LINE* aSegment )
 }
 
 
-bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition )
+bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew )
 {
-    if( GetItem( aPosition, 0, SCH_JUNCTION_T ) )
-        return false;
+    bool has_line = false;
+    int end_count = 0;
 
-    if( GetWire( aPosition, 0, EXCLUDE_END_POINTS_T ) )
+    for( SCH_ITEM* item = m_drawList.begin(); item; item = item->Next() )
     {
-        if( GetWire( aPosition, 0, END_POINTS_ONLY_T ) )
-            return true;
+        if( aNew && ( item->Type() == SCH_JUNCTION_T ) && ( item->HitTest(aPosition) ) )
+            return false;
 
-        if( GetPin( aPosition, NULL, true ) )
-            return true;
+        if( item->Type() != SCH_LINE_T )
+            continue;
+
+        if( item->GetLayer() != LAYER_WIRE )
+            continue;
+
+        if( !item->HitTest( aPosition, 0 ) )
+            continue;
+
+        if( ( (SCH_LINE*) item )->IsEndPoint( aPosition ) )
+            end_count++;
+        else
+            has_line = true;
     }
 
+    int has_pin = !!( GetPin( aPosition, NULL, true ) );
+    if( end_count + has_pin > 2 )
+        return true;
+
+    if( has_line && ( end_count || has_pin ) )
+        return true;
+
     return false;
 }
 
@@ -1258,7 +1276,7 @@ int SCH_SCREEN::GetConnection( const wxPoint& aPosition, PICKED_ITEMS_LIST& aLis
 
             SCH_JUNCTION* junction = (SCH_JUNCTION*) item;
 
-            if( CountConnectedItems( junction->GetPosition(), false ) <= 2 )
+            if( !IsJunctionNeeded( junction->GetPosition() ) )
             {
                 item->SetFlags( STRUCT_DELETED );
 
-- 
2.11.0

From 050b5dd3ef022dd8454f7c693696af875ee5b330 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 16:03:12 -0700
Subject: [PATCH 05/13] Eeschema: Add 'append' option to undo/redo

Gives multiple-stage edits the ability to
add to previous undo commit.
---
 eeschema/block.cpp               |  8 ++++----
 eeschema/edit_bitmap.cpp         |  2 +-
 eeschema/schematic_undo_redo.cpp | 42 ++++++++++++++++++++++++++++++++++++----
 eeschema/schframe.h              | 24 ++++++++++++++++++++++-
 4 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/eeschema/block.cpp b/eeschema/block.cpp
index 272d92a75..f86ed94b8 100644
--- a/eeschema/block.cpp
+++ b/eeschema/block.cpp
@@ -137,7 +137,7 @@ void SCH_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
         if( m_canvas->IsMouseCaptured() )
             m_canvas->CallMouseCapture( DC, wxDefaultPosition, false );
 
-        SaveCopyInUndoList( block->GetItems(), UR_MOVED, block->GetMoveVector() );
+        SaveCopyInUndoList( block->GetItems(), UR_MOVED, false, block->GetMoveVector() );
         MoveItemsInList( block->GetItems(), block->GetMoveVector() );
         block->ClearItemsList();
         break;
@@ -226,7 +226,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 wxPoint rotationPoint = block->Centre();
                 rotationPoint = GetNearestGridPosition( rotationPoint );
                 SetCrossHairPosition( rotationPoint );
-                SaveCopyInUndoList( block->GetItems(), UR_ROTATED, rotationPoint );
+                SaveCopyInUndoList( block->GetItems(), UR_ROTATED, false, rotationPoint );
                 RotateListOfItems( block->GetItems(), rotationPoint );
                 OnModify();
             }
@@ -340,7 +340,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 wxPoint mirrorPoint = block->Centre();
                 mirrorPoint = GetNearestGridPosition( mirrorPoint );
                 SetCrossHairPosition( mirrorPoint );
-                SaveCopyInUndoList( block->GetItems(), UR_MIRRORED_X, mirrorPoint );
+                SaveCopyInUndoList( block->GetItems(), UR_MIRRORED_X, false, mirrorPoint );
                 MirrorX( block->GetItems(), mirrorPoint );
                 OnModify();
             }
@@ -359,7 +359,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 wxPoint mirrorPoint = block->Centre();
                 mirrorPoint = GetNearestGridPosition( mirrorPoint );
                 SetCrossHairPosition( mirrorPoint );
-                SaveCopyInUndoList( block->GetItems(), UR_MIRRORED_Y, mirrorPoint );
+                SaveCopyInUndoList( block->GetItems(), UR_MIRRORED_Y, false, mirrorPoint );
                 MirrorY( block->GetItems(), mirrorPoint );
                 OnModify();
             }
diff --git a/eeschema/edit_bitmap.cpp b/eeschema/edit_bitmap.cpp
index ad2a2b93a..d49efcda8 100644
--- a/eeschema/edit_bitmap.cpp
+++ b/eeschema/edit_bitmap.cpp
@@ -161,7 +161,7 @@ void SCH_EDIT_FRAME::MoveImage( SCH_BITMAP* aImageItem, wxDC* aDC )
 void SCH_EDIT_FRAME::RotateImage( SCH_BITMAP* aItem )
 {
     if( aItem->GetFlags( ) == 0 )
-        SaveCopyInUndoList( aItem, UR_ROTATED, aItem->GetPosition() );
+        SaveCopyInUndoList( aItem, UR_ROTATED, false, aItem->GetPosition() );
 
     aItem->Rotate( aItem->GetPosition() );
     OnModify();
diff --git a/eeschema/schematic_undo_redo.cpp b/eeschema/schematic_undo_redo.cpp
index ecc288bfb..eaa6e5e59 100644
--- a/eeschema/schematic_undo_redo.cpp
+++ b/eeschema/schematic_undo_redo.cpp
@@ -108,8 +108,11 @@
 
 void SCH_EDIT_FRAME::SaveCopyInUndoList( SCH_ITEM*      aItem,
                                          UNDO_REDO_T    aCommandType,
+                                         bool           aAppend,
                                          const wxPoint& aTransformPoint )
 {
+     PICKED_ITEMS_LIST* commandToUndo = NULL;
+
     /* Does not save a null item or a UR_WIRE_IMAGE command type.  UR_WIRE_IMAGE commands
      * are handled by the overloaded version of SaveCopyInUndoList that takes a reference
      * to a PICKED_ITEMS_LIST.
@@ -117,8 +120,15 @@ void SCH_EDIT_FRAME::SaveCopyInUndoList( SCH_ITEM*      aItem,
     if( aItem == NULL || aCommandType == UR_WIRE_IMAGE )
         return;
 
-    PICKED_ITEMS_LIST* commandToUndo = new PICKED_ITEMS_LIST();
-    commandToUndo->m_TransformPoint = aTransformPoint;
+    if( aAppend ) {
+        commandToUndo = GetScreen()->PopCommandFromUndoList();
+    }
+
+    if( !commandToUndo )
+    {
+        commandToUndo = new PICKED_ITEMS_LIST();
+        commandToUndo->m_TransformPoint = aTransformPoint;
+    }
 
     ITEM_PICKER itemWrapper( aItem, aCommandType );
     itemWrapper.SetFlags( aItem->GetFlags() );
@@ -160,15 +170,39 @@ void SCH_EDIT_FRAME::SaveCopyInUndoList( SCH_ITEM*      aItem,
 
 void SCH_EDIT_FRAME::SaveCopyInUndoList( const PICKED_ITEMS_LIST& aItemsList,
                                          UNDO_REDO_T        aTypeCommand,
+                                         bool               aAppend,
                                          const wxPoint&     aTransformPoint )
 {
-    PICKED_ITEMS_LIST* commandToUndo = new PICKED_ITEMS_LIST();
+    PICKED_ITEMS_LIST* commandToUndo = NULL;
+
+    // Can't append a WIRE IMAGE, so fail to a new undo point
+    if( aAppend && ( aTypeCommand != UR_WIRE_IMAGE ) )
+    {
+        commandToUndo = GetScreen()->PopCommandFromUndoList();
+        if( commandToUndo->m_Status == UR_WIRE_IMAGE )
+        {
+            GetScreen()->PushCommandToUndoList( commandToUndo );
+            commandToUndo = NULL;
+        }
+    }
+
+    if( !commandToUndo )
+    {
+        commandToUndo = new PICKED_ITEMS_LIST();
+    }
 
     commandToUndo->m_TransformPoint = aTransformPoint;
     commandToUndo->m_Status = aTypeCommand;
 
     // Copy picker list:
-    commandToUndo->CopyList( aItemsList );
+    if( !commandToUndo->GetCount() )
+        commandToUndo->CopyList( aItemsList );
+    else
+    {
+        // Unless we are appending, in which case, get the picker items
+        for( unsigned ii = 0; ii < aItemsList.GetCount(); ii++ )
+            commandToUndo->PushItem( aItemsList.GetItemWrapper( ii) );
+    }
 
     // Verify list, and creates data if needed
     for( unsigned ii = 0; ii < commandToUndo->GetCount(); ii++ )
diff --git a/eeschema/schframe.h b/eeschema/schframe.h
index 72402e94c..cb1d389e8 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -941,8 +941,25 @@ private:
     /**
      * Function AddJunction
      * adds a new junction at \a aPosition.
+     * @
      */
-    SCH_JUNCTION* AddJunction( wxDC* aDC, const wxPoint& aPosition, bool aPutInUndoList = false );
+    SCH_JUNCTION* AddJunction( wxDC* aDC, const wxPoint& aPosition, bool aAppend = false );
+
+    /**
+     * Function SaveWireImage
+     * saves a copy of the current wire image in the undo list
+     */
+    void SaveWireImage();
+
+    /**
+     * Function SchematicCleanUp
+     * performs routine schematic cleaning including breaking wire and buses and
+     * deleting identical objects superimposed on top of each other.
+     * @param aAppend = True if we are updating the previous undo state
+     *
+     * @return True if any schematic clean up was performed.
+     */
+    bool SchematicCleanUp( bool aAppend = false );
 
     /**
      * Function PrepareMoveItem
@@ -1100,6 +1117,7 @@ public:
      * Function DeleteItem
      * removes \a aItem from the current screen and saves it in the undo list.
      * @param aItem The item to remove from the current screen.
+     * @param aAppend True if we are updating a previous Undo state
      */
     void DeleteItem( SCH_ITEM* aItem );
 
@@ -1193,11 +1211,13 @@ public:
      * exchanged with current wire list
      * @param aItemToCopy = the schematic item modified by the command to undo
      * @param aTypeCommand = command type (see enum UNDO_REDO_T)
+     * @param aAppend = add the item to the previous undo list
      * @param aTransformPoint = the reference point of the transformation,
      *                          for commands like move
      */
     void SaveCopyInUndoList( SCH_ITEM* aItemToCopy,
                              UNDO_REDO_T aTypeCommand,
+                             bool aAppend = false,
                              const wxPoint& aTransformPoint = wxPoint( 0, 0 ) );
 
     /**
@@ -1206,11 +1226,13 @@ public:
      * add a list of pickers to handle a list of items
      * @param aItemsList = the list of items modified by the command to undo
      * @param aTypeCommand = command type (see enum UNDO_REDO_T)
+     * @param aAppend = add the item to the previous undo list
      * @param aTransformPoint = the reference point of the transformation,
      *                          for commands like move
      */
     void SaveCopyInUndoList( const PICKED_ITEMS_LIST& aItemsList,
                              UNDO_REDO_T aTypeCommand,
+                             bool aAppend = false,
                              const wxPoint& aTransformPoint = wxPoint( 0, 0 ) );
 
 private:
-- 
2.11.0

From b1b28bbb360fd615eb8c1608ae2cd9d7a2742120 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 16:41:25 -0700
Subject: [PATCH 06/13] Eeschema: Unify delete operations

DeleteItemsInList now shares the code for DeleteItem.
DeleteItem will remove junctions that were connected to the
item and are no longer needed.
---
 eeschema/block.cpp                     |  4 +--
 eeschema/operations_on_items_lists.cpp | 52 +++++++++++++++++++++-------------
 eeschema/protos.h                      |  3 --
 eeschema/schedit.cpp                   |  2 +-
 eeschema/schframe.h                    | 11 ++++++-
 5 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/eeschema/block.cpp b/eeschema/block.cpp
index f86ed94b8..4c100e49e 100644
--- a/eeschema/block.cpp
+++ b/eeschema/block.cpp
@@ -282,7 +282,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
 
             if( block->GetCount() )
             {
-                DeleteItemsInList( m_canvas, block->GetItems() );
+                DeleteItemsInList( block->GetItems() );
                 OnModify();
             }
             block->ClearItemsList();
@@ -313,7 +313,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 wxPoint move_vector = -GetScreen()->m_BlockLocate.GetLastCursorPosition();
                 copyBlockItems( block->GetItems() );
                 MoveItemsInList( m_blockItems.GetItems(), move_vector );
-                DeleteItemsInList( m_canvas, block->GetItems() );
+                DeleteItemsInList( block->GetItems() );
                 OnModify();
             }
 
diff --git a/eeschema/operations_on_items_lists.cpp b/eeschema/operations_on_items_lists.cpp
index 6f0b790c2..3c61be363 100644
--- a/eeschema/operations_on_items_lists.cpp
+++ b/eeschema/operations_on_items_lists.cpp
@@ -126,10 +126,8 @@ void MoveItemsInList( PICKED_ITEMS_LIST& aItemsList, const wxPoint aMoveVector )
  * delete schematic items in aItemsList
  * deleted items are put in undo list
  */
-void DeleteItemsInList( EDA_DRAW_PANEL* panel, PICKED_ITEMS_LIST& aItemsList )
+void SCH_EDIT_FRAME::DeleteItemsInList( PICKED_ITEMS_LIST& aItemsList, bool aAppend )
 {
-    SCH_SCREEN*        screen = (SCH_SCREEN*) panel->GetScreen();
-    SCH_EDIT_FRAME*    frame  = (SCH_EDIT_FRAME*) panel->GetParent();
     PICKED_ITEMS_LIST  itemsList;
 
     for( unsigned ii = 0; ii < aItemsList.GetCount(); ii++ )
@@ -137,25 +135,17 @@ void DeleteItemsInList( EDA_DRAW_PANEL* panel, PICKED_ITEMS_LIST& aItemsList )
         SCH_ITEM* item = (SCH_ITEM*) aItemsList.GetPickedItem( ii );
         ITEM_PICKER itemWrapper( item, UR_DELETED );
 
-        if( item->Type() == SCH_SHEET_PIN_T )
-        {
-            /* this item is depending on a sheet, and is not in global list */
-            wxMessageBox( wxT( "DeleteItemsInList() err: unexpected SCH_SHEET_PIN_T" ) );
-        }
-        else
-        {
-            screen->Remove( item );
+        if( item->GetFlags() & STRUCT_DELETED )
+            continue;
 
-            /* Unlink the structure */
-            itemsList.PushItem( itemWrapper );
-        }
+        DeleteItem( item, aAppend || !!ii );
     }
 
-    frame->SaveCopyInUndoList( itemsList, UR_DELETED );
+    GetScreen()->ClearDrawingState();
 }
 
 
-void SCH_EDIT_FRAME::DeleteItem( SCH_ITEM* aItem )
+void SCH_EDIT_FRAME::DeleteItem( SCH_ITEM* aItem, bool aAppend )
 {
     wxCHECK_RET( aItem != NULL, wxT( "Cannot delete invalid item." ) );
 
@@ -165,18 +155,42 @@ void SCH_EDIT_FRAME::DeleteItem( SCH_ITEM* aItem )
 
     if( aItem->Type() == SCH_SHEET_PIN_T )
     {
-        // This iten is attached to a node, and is not accessible by the global list directly.
+        // This item is attached to a node, and is not accessible by the global list directly.
         SCH_SHEET* sheet = (SCH_SHEET*) aItem->GetParent();
         wxCHECK_RET( (sheet != NULL) && (sheet->Type() == SCH_SHEET_T),
                      wxT( "Sheet label has invalid parent item." ) );
-        SaveCopyInUndoList( (SCH_ITEM*) sheet, UR_CHANGED );
+        SaveCopyInUndoList( (SCH_ITEM*) sheet, UR_CHANGED, aAppend );
         sheet->RemovePin( (SCH_SHEET_PIN*) aItem );
         m_canvas->RefreshDrawingRect( sheet->GetBoundingBox() );
     }
     else
     {
+        PICKED_ITEMS_LIST itemsList;
+        ITEM_PICKER picker( aItem, UR_DELETED );
+
+        aItem->SetFlags( STRUCT_DELETED );
+        itemsList.PushItem( picker );
         screen->Remove( aItem );
-        SaveCopyInUndoList( aItem, UR_DELETED );
+
+        if( aItem->IsConnectable() )
+        {
+            std::vector< wxPoint > pts;
+            aItem->GetConnectionPoints( pts );
+            for( auto point : pts )
+            {
+                SCH_ITEM* junction;
+                if( !screen->IsJunctionNeeded( point )
+                        && ( junction = screen->GetItem( point, 0, SCH_JUNCTION_T ) ) )
+                {
+                    ITEM_PICKER picker_juction( junction, UR_DELETED );
+                    junction->SetFlags( STRUCT_DELETED );
+                    itemsList.PushItem( picker_juction );
+                    screen->Remove( junction );
+                }
+            }
+        }
+
+        SaveCopyInUndoList( itemsList, UR_DELETED, aAppend );
         m_canvas->RefreshDrawingRect( aItem->GetBoundingBox() );
     }
 }
diff --git a/eeschema/protos.h b/eeschema/protos.h
index 441c36d16..a80b5e672 100644
--- a/eeschema/protos.h
+++ b/eeschema/protos.h
@@ -31,9 +31,6 @@ class PICKED_ITEMS_LIST;
 class SCH_ITEM;
 
 
-// operations_on_item_lists.cpp
-void DeleteItemsInList( EDA_DRAW_PANEL* panel, PICKED_ITEMS_LIST& aItemsList );
-
 /**
  * Function DuplicateStruct
  * creates a new copy of given struct.
diff --git a/eeschema/schedit.cpp b/eeschema/schedit.cpp
index b350c5634..75e6aac8e 100644
--- a/eeschema/schedit.cpp
+++ b/eeschema/schedit.cpp
@@ -648,7 +648,7 @@ void SCH_EDIT_FRAME::DeleteConnection( bool aFullConnection )
 
     if( screen->GetConnection( pos, pickList, aFullConnection ) != 0 )
     {
-        DeleteItemsInList( m_canvas, pickList );
+        DeleteItemsInList( pickList );
         OnModify();
     }
 }
diff --git a/eeschema/schframe.h b/eeschema/schframe.h
index cb1d389e8..6349d69f7 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -1119,7 +1119,16 @@ public:
      * @param aItem The item to remove from the current screen.
      * @param aAppend True if we are updating a previous Undo state
      */
-    void DeleteItem( SCH_ITEM* aItem );
+    void DeleteItem( SCH_ITEM* aItem, bool aAppend = false );
+
+    /**
+     * Function DeleteItemsInList
+     * removes all items (and unused junctions that connect to them) and saves
+     * each in the undo list
+     * @param aItemsList The list of items to delete
+     * @param aAppend True if we are updating a previous commit
+     */
+    void DeleteItemsInList( PICKED_ITEMS_LIST& aItemsList, bool aAppend = false );
 
     int GetLabelIncrement() const { return m_repeatLabelDelta; }
 
-- 
2.11.0

From 2b7f005b6d9014e7051a7b54e12db247f0a5ebdc Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 13 Oct 2017 15:08:33 -0700
Subject: [PATCH 07/13] Eeschema: Add SwapData to SCH_LINE class

---
 eeschema/sch_line.cpp | 11 +++++++++++
 eeschema/sch_line.h   |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index 2c1379155..865e51feb 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -576,6 +576,17 @@ bool SCH_LINE::HitTest( const EDA_RECT& aRect, bool aContained, int aAccuracy )
     return rect.Intersects( m_start, m_end );
 }
 
+void SCH_LINE::SwapData( SCH_ITEM* aItem )
+{
+    SCH_LINE* item = (SCH_LINE*) aItem;
+
+    std::swap( m_Layer, item->m_Layer );
+
+    std::swap( m_start, item->m_start );
+    std::swap( m_end, item->m_end );
+    std::swap( m_startIsDangling, item->m_startIsDangling );
+    std::swap( m_endIsDangling, item->m_endIsDangling );
+}
 
 bool SCH_LINE::doIsConnected( const wxPoint& aPosition ) const
 {
diff --git a/eeschema/sch_line.h b/eeschema/sch_line.h
index 7b8cf5d1b..c457c9aa9 100644
--- a/eeschema/sch_line.h
+++ b/eeschema/sch_line.h
@@ -147,6 +147,8 @@ public:
 
     EDA_ITEM* Clone() const override;
 
+    void SwapData( SCH_ITEM* aItem ) override;
+
 #if defined(DEBUG)
     void Show( int nestLevel, std::ostream& os ) const override;
 #endif
-- 
2.11.0

From bd543081ae8f45d039d8f3ec9142fec3bb194cb4 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Mon, 16 Oct 2017 14:38:57 -0700
Subject: [PATCH 08/13] Eeschema: Add two utility functions to sch_line

Functions are useful to compare lines for merging/
optimization
---
 eeschema/sch_line.cpp | 30 ++++++++++++++++++++++++++++++
 eeschema/sch_line.h   | 12 ++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index 865e51feb..ee0d60659 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -266,6 +266,36 @@ void SCH_LINE::Rotate( wxPoint aPosition )
     RotatePoint( &m_end, aPosition, 900 );
 }
 
+bool SCH_LINE::IsSameQuadrant( SCH_LINE* aLine, const wxPoint& aPosition )
+{
+    wxPoint first;
+    wxPoint second;
+
+    if( m_start == aPosition )
+        first = m_end - aPosition;
+    else
+        first = m_start - aPosition;
+
+    if( aLine->m_start == aPosition)
+        second = aLine->m_end - aPosition;
+    else
+        second = aLine->m_start - aPosition;
+
+    return (std::signbit( first.x ) == std::signbit( second.x ) &&
+            std::signbit( first.y ) == std::signbit( second.y ) );
+}
+
+bool SCH_LINE::IsParallel( SCH_LINE* aLine )
+{
+    wxCHECK_MSG( aLine != NULL && aLine->Type() == SCH_LINE_T, false,
+                 wxT( "Cannot test line segment for overlap." ) );
+
+    wxPoint firstSeg   = m_end - m_start;
+    wxPoint secondSeg = aLine->m_end - aLine->m_start;
+
+    // Use long long here to avoid overflow in calculations
+    return !( (long long) firstSeg.x * secondSeg.y - (long long) firstSeg.y * secondSeg.x );
+}
 
 /*
  * MergeOverlap try to merge 2 lines that are colinear.
diff --git a/eeschema/sch_line.h b/eeschema/sch_line.h
index c457c9aa9..9aece825f 100644
--- a/eeschema/sch_line.h
+++ b/eeschema/sch_line.h
@@ -113,6 +113,18 @@ public:
      */
     bool MergeOverlap( SCH_LINE* aLine );
 
+    /**
+     * Check if two lines are in the same quadrant as each other, using a reference point as
+     * the origin
+     *
+     * @param aLine - Line to compare
+     * @param aPosition - Point to reference against lines
+     * @return true if lines are mostly in different quadrants of aPosition, false otherwise
+     */
+    bool IsSameQuadrant( SCH_LINE* aLine, const wxPoint& aPosition );
+
+    bool IsParallel( SCH_LINE* aLine );
+
     void GetEndPoints( std::vector<DANGLING_END_ITEM>& aItemList ) override;
 
     bool IsDanglingStateChanged( std::vector< DANGLING_END_ITEM >& aItemList ) override;
-- 
2.11.0

From 6ae5e39905e65e40af5066e4b81b02d1c60e6cb1 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 20 Oct 2017 09:31:02 -0700
Subject: [PATCH 09/13] Eeschema: don't cleanup unseen schematics

Changes to the schematic shouldn't be made
where the user isn't looking.  Removing the
cleanup to ERC and netlisting prevents
unintended consequences
---
 eeschema/dialogs/dialog_erc.cpp | 9 ---------
 eeschema/netlist.cpp            | 4 ----
 2 files changed, 13 deletions(-)

diff --git a/eeschema/dialogs/dialog_erc.cpp b/eeschema/dialogs/dialog_erc.cpp
index bf7770f6c..1e3025e17 100644
--- a/eeschema/dialogs/dialog_erc.cpp
+++ b/eeschema/dialogs/dialog_erc.cpp
@@ -477,15 +477,6 @@ void DIALOG_ERC::TestErc( wxArrayString* aMessagesList )
     // Erase all previous DRC markers.
     screens.DeleteAllMarkers( MARKER_BASE::MARKER_ERC );
 
-    for( SCH_SCREEN* screen = screens.GetFirst(); screen != NULL; screen = screens.GetNext() )
-    {
-        /* Ff wire list has changed, delete Undo Redo list to avoid pointers on deleted
-         * data problems.
-         */
-        if( screen->SchematicCleanUp() )
-            screen->ClearUndoRedoList();
-    }
-
     /* Test duplicate sheet names inside a given sheet, one cannot have sheets with
      * duplicate names (file names can be duplicated).
      */
diff --git a/eeschema/netlist.cpp b/eeschema/netlist.cpp
index d118bf28f..2e789319f 100644
--- a/eeschema/netlist.cpp
+++ b/eeschema/netlist.cpp
@@ -83,9 +83,6 @@ bool SCH_EDIT_FRAME::prepareForNetlist()
             return false;
     }
 
-    // Cleanup the entire hierarchy
-    schematic.SchematicCleanUp();
-
     return true;
 }
 
@@ -123,7 +120,6 @@ bool SCH_EDIT_FRAME::CreateNetlist( int aFormat, const wxString& aFullFileName,
         schematic.UpdateSymbolLinks();
         SCH_SHEET_LIST sheets( g_RootSheet );
         sheets.AnnotatePowerSymbols( Prj().SchLibs() );
-        schematic.SchematicCleanUp();
     }
 
     std::unique_ptr<NETLIST_OBJECT_LIST> connectedItemsList( BuildNetListBase() );
-- 
2.11.0

From 4321c224501fa079cdc47aa0d874f34ed115da1d Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Wed, 25 Oct 2017 15:21:42 -0700
Subject: [PATCH 10/13] Eeschema: Moving SchematicCleanup to SCH_EDIT_FRAME

---
 eeschema/bus-wire-junction.cpp | 53 ++++++++++++++++++++++++++++++++++--
 eeschema/class_sch_screen.h    | 15 ----------
 eeschema/hierarch.cpp          |  2 +-
 eeschema/project_rescue.cpp    |  2 +-
 eeschema/sch_screen.cpp        | 62 ------------------------------------------
 eeschema/schframe.cpp          |  2 +-
 eeschema/schframe.h            |  3 +-
 7 files changed, 55 insertions(+), 84 deletions(-)

diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index 528d09d68..ec6c71b77 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -324,7 +324,7 @@ void SCH_EDIT_FRAME::EndSegment( wxDC* DC )
     screen->Append( s_wires );
 
     // Correct and remove segments that need to be merged.
-    screen->SchematicCleanUp();
+    SchematicCleanUp();
 
     // A junction could be needed to connect the end point of the last created segment.
     if( screen->IsJunctionNeeded( endpoint ) )
@@ -411,6 +411,55 @@ void SCH_EDIT_FRAME::DeleteCurrentSegment( wxDC* DC )
 }
 
 
+bool SCH_EDIT_FRAME::SchematicCleanUp()
+{
+    bool      modified = false;
+
+    for( SCH_ITEM* item = GetScreen()->GetDrawItems() ; item; item = item->Next() )
+    {
+        if( ( item->Type() != SCH_LINE_T ) && ( item->Type() != SCH_JUNCTION_T ) )
+            continue;
+
+        bool restart;
+
+        for( SCH_ITEM* testItem = item->Next(); testItem; testItem = restart ? GetScreen()->GetDrawItems() : testItem->Next() )
+        {
+            restart = false;
+
+            if( ( item->Type() == SCH_LINE_T ) && ( testItem->Type() == SCH_LINE_T ) )
+            {
+                SCH_LINE* line = (SCH_LINE*) item;
+
+                if( line->MergeOverlap( (SCH_LINE*) testItem ) )
+                {
+                    // Keep the current flags, because the deleted segment can be flagged.
+                    item->SetFlags( testItem->GetFlags() );
+                    DeleteItem( testItem );
+                    restart = true;
+                    modified = true;
+                }
+            }
+            else if ( ( ( item->Type() == SCH_JUNCTION_T )
+                      && ( testItem->Type() == SCH_JUNCTION_T ) ) && ( testItem != item ) )
+            {
+                if ( testItem->HitTest( item->GetPosition() ) )
+                {
+                    // Keep the current flags, because the deleted segment can be flagged.
+                    item->SetFlags( testItem->GetFlags() );
+                    DeleteItem( testItem );
+                    restart = true;
+                    modified = true;
+                }
+            }
+        }
+    }
+
+    GetScreen()->TestDanglingEnds();
+
+    return modified;
+}
+
+
 SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( wxDC* aDC, const wxPoint& aPosition,
                                            bool aPutInUndoList )
 {
@@ -439,7 +488,7 @@ SCH_NO_CONNECT* SCH_EDIT_FRAME::AddNoConnect( wxDC* aDC, const wxPoint& aPositio
 
     SetRepeatItem( no_connect );
     GetScreen()->Append( no_connect );
-    GetScreen()->SchematicCleanUp();
+    SchematicCleanUp();
     OnModify();
     m_canvas->Refresh();
     SaveCopyInUndoList( no_connect, UR_NEW );
diff --git a/eeschema/class_sch_screen.h b/eeschema/class_sch_screen.h
index a0e25cf73..76e3a3eb8 100644
--- a/eeschema/class_sch_screen.h
+++ b/eeschema/class_sch_screen.h
@@ -252,15 +252,6 @@ public:
     bool CheckIfOnDrawList( SCH_ITEM* st );
 
     /**
-     * Function SchematicCleanUp
-     * performs routine schematic cleaning including breaking wire and buses and
-     * deleting identical objects superimposed on top of each other.
-     *
-     * @return True if any schematic clean up was performed.
-     */
-    bool SchematicCleanUp();
-
-    /**
      * Function TestDanglingEnds
      * tests all of the connectible objects in the schematic for unused connection points.
      * @return True if any connection state changes were made.
@@ -552,12 +543,6 @@ public:
     void ClearAnnotation();
 
     /**
-     * Function SchematicCleanUp
-     * merges and breaks wire segments in the entire schematic hierarchy.
-     */
-    void SchematicCleanUp();
-
-    /**
      * Function ReplaceDuplicateTimeStamps
      * test all sheet and component objects in the schematic for duplicate time stamps
      * an replaces them as necessary.  Time stamps must be unique in order for complex
diff --git a/eeschema/hierarch.cpp b/eeschema/hierarch.cpp
index d66aa4ea9..c41c82164 100644
--- a/eeschema/hierarch.cpp
+++ b/eeschema/hierarch.cpp
@@ -299,7 +299,7 @@ void SCH_EDIT_FRAME::DisplayCurrentSheet()
         screen->m_FirstRedraw = false;
         SetCrossHairPosition( GetScrollCenterPosition() );
         m_canvas->MoveCursorToCrossHair();
-        screen->SchematicCleanUp();
+        SchematicCleanUp();
     }
     else
     {
diff --git a/eeschema/project_rescue.cpp b/eeschema/project_rescue.cpp
index c757cde90..9ad8a2dbb 100644
--- a/eeschema/project_rescue.cpp
+++ b/eeschema/project_rescue.cpp
@@ -614,7 +614,7 @@ bool SCH_EDIT_FRAME::RescueProject( bool aRunningOnDemand )
     Prj().SetElem( PROJECT::ELEM_SCH_PART_LIBS, NULL );
 
     // Clean up wire ends
-    GetScreen()->SchematicCleanUp();
+    SchematicCleanUp();
     m_canvas->Refresh( true );
     OnModify();
 
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index 67449ac7b..7b859e1f3 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -439,55 +439,6 @@ bool SCH_SCREEN::IsTerminalPoint( const wxPoint& aPosition, int aLayer )
 }
 
 
-bool SCH_SCREEN::SchematicCleanUp()
-{
-    bool      modified = false;
-
-    for( SCH_ITEM* item = m_drawList.begin() ; item; item = item->Next() )
-    {
-        if( ( item->Type() != SCH_LINE_T ) && ( item->Type() != SCH_JUNCTION_T ) )
-            continue;
-
-        bool restart;
-
-        for( SCH_ITEM* testItem = item->Next(); testItem; testItem = restart ? m_drawList.begin() : testItem->Next() )
-        {
-            restart = false;
-
-            if( ( item->Type() == SCH_LINE_T ) && ( testItem->Type() == SCH_LINE_T ) )
-            {
-                SCH_LINE* line = (SCH_LINE*) item;
-
-                if( line->MergeOverlap( (SCH_LINE*) testItem ) )
-                {
-                    // Keep the current flags, because the deleted segment can be flagged.
-                    item->SetFlags( testItem->GetFlags() );
-                    DeleteItem( testItem );
-                    restart = true;
-                    modified = true;
-                }
-            }
-            else if ( ( ( item->Type() == SCH_JUNCTION_T )
-                      && ( testItem->Type() == SCH_JUNCTION_T ) ) && ( testItem != item ) )
-            {
-                if ( testItem->HitTest( item->GetPosition() ) )
-                {
-                    // Keep the current flags, because the deleted segment can be flagged.
-                    item->SetFlags( testItem->GetFlags() );
-                    DeleteItem( testItem );
-                    restart = true;
-                    modified = true;
-                }
-            }
-        }
-    }
-
-    TestDanglingEnds();
-
-    return modified;
-}
-
-
 bool SCH_SCREEN::Save( FILE* aFile ) const
 {
     // Creates header
@@ -1424,19 +1375,6 @@ void SCH_SCREENS::ClearAnnotation()
         m_screens[i]->ClearAnnotation( NULL );
 }
 
-
-void SCH_SCREENS::SchematicCleanUp()
-{
-    for( size_t i = 0;  i < m_screens.size();  i++ )
-    {
-        // if wire list has changed, delete the undo/redo list to avoid
-        // pointer problems with deleted data.
-        if( m_screens[i]->SchematicCleanUp() )
-            m_screens[i]->ClearUndoRedoList();
-    }
-}
-
-
 int SCH_SCREENS::ReplaceDuplicateTimeStamps()
 {
     EDA_ITEMS items;
diff --git a/eeschema/schframe.cpp b/eeschema/schframe.cpp
index 3ec354575..12bce0e0e 100644
--- a/eeschema/schframe.cpp
+++ b/eeschema/schframe.cpp
@@ -1227,7 +1227,7 @@ void SCH_EDIT_FRAME::OnOpenLibraryEditor( wxCommandEvent& event )
         }
     }
 
-    GetScreen()->SchematicCleanUp();
+    SchematicCleanUp();
     m_canvas->Refresh();
 }
 
diff --git a/eeschema/schframe.h b/eeschema/schframe.h
index 6349d69f7..e56cd5539 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -955,11 +955,10 @@ private:
      * Function SchematicCleanUp
      * performs routine schematic cleaning including breaking wire and buses and
      * deleting identical objects superimposed on top of each other.
-     * @param aAppend = True if we are updating the previous undo state
      *
      * @return True if any schematic clean up was performed.
      */
-    bool SchematicCleanUp( bool aAppend = false );
+    bool SchematicCleanUp();
 
     /**
      * Function PrepareMoveItem
-- 
2.11.0

From eba13696a2ab950dd1d53f5a97c8df00271b3589 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Wed, 25 Oct 2017 16:24:47 -0700
Subject: [PATCH 11/13] Eeschema: Removing DC dependencies

---
 eeschema/bus-wire-junction.cpp | 34 ++++++++++++++--------------------
 eeschema/onleftclick.cpp       |  6 +++---
 eeschema/schedit.cpp           |  4 ++--
 eeschema/schframe.h            |  7 +++----
 4 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index ec6c71b77..6b6656412 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -242,7 +242,7 @@ void SCH_EDIT_FRAME::BeginSegment( wxDC* DC, int type )
         // Terminate the command if the end point is on a pin, junction, or another wire or bus.
         if( GetScreen()->IsTerminalPoint( cursorpos, segment->GetLayer() ) )
         {
-            EndSegment( DC );
+            EndSegment();
             return;
         }
 
@@ -261,7 +261,7 @@ void SCH_EDIT_FRAME::BeginSegment( wxDC* DC, int type )
 }
 
 
-void SCH_EDIT_FRAME::EndSegment( wxDC* DC )
+void SCH_EDIT_FRAME::EndSegment()
 {
     SCH_SCREEN* screen = GetScreen();
     SCH_LINE* segment = (SCH_LINE*) screen->GetCurItem();
@@ -328,11 +328,11 @@ void SCH_EDIT_FRAME::EndSegment( wxDC* DC )
 
     // A junction could be needed to connect the end point of the last created segment.
     if( screen->IsJunctionNeeded( endpoint ) )
-        screen->Append( AddJunction( DC, endpoint ) );
+        screen->Append( AddJunction( endpoint ) );
 
     // A junction could be needed to connect the start point of the set of new created wires
     if( screen->IsJunctionNeeded( startPoint ) )
-        screen->Append( AddJunction( DC, startPoint ) );
+        screen->Append( AddJunction( startPoint ) );
 
     m_canvas->Refresh();
 
@@ -460,29 +460,23 @@ bool SCH_EDIT_FRAME::SchematicCleanUp()
 }
 
 
-SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( wxDC* aDC, const wxPoint& aPosition,
-                                           bool aPutInUndoList )
+
+SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( const wxPoint& aPosition, bool aAppend )
 {
     SCH_JUNCTION* junction = new SCH_JUNCTION( aPosition );
+    SCH_SCREEN* screen = GetScreen();
+    bool broken_segments = false;
 
-    SetRepeatItem( junction );
-
-    m_canvas->CrossHairOff( aDC );     // Erase schematic cursor
-    junction->Draw( m_canvas, aDC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
-    m_canvas->CrossHairOn( aDC );      // Display schematic cursor
-
-    if( aPutInUndoList )
-    {
-        GetScreen()->Append( junction );
-        SaveCopyInUndoList( junction, UR_NEW );
-        OnModify();
-    }
-
+    screen->Append( junction );
+    broken_segments = screen->BreakSegment( aPosition );
+    screen->TestDanglingEnds();
+    OnModify();
+    SaveCopyInUndoList( junction, UR_NEW, broken_segments || aAppend );
     return junction;
 }
 
 
-SCH_NO_CONNECT* SCH_EDIT_FRAME::AddNoConnect( wxDC* aDC, const wxPoint& aPosition )
+SCH_NO_CONNECT* SCH_EDIT_FRAME::AddNoConnect( const wxPoint& aPosition )
 {
     SCH_NO_CONNECT* no_connect = new SCH_NO_CONNECT( aPosition );
 
diff --git a/eeschema/onleftclick.cpp b/eeschema/onleftclick.cpp
index f93e0e80e..7892af06d 100644
--- a/eeschema/onleftclick.cpp
+++ b/eeschema/onleftclick.cpp
@@ -120,7 +120,7 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
         {
             if( GetScreen()->GetItem( gridPosition, 0, SCH_NO_CONNECT_T ) == NULL )
             {
-                SCH_NO_CONNECT*  no_connect = AddNoConnect( aDC, gridPosition );
+                SCH_NO_CONNECT*  no_connect = AddNoConnect( gridPosition );
                 SetRepeatItem( no_connect );
                 GetScreen()->SetCurItem( no_connect );
                 m_canvas->SetAutoPanRequest( true );
@@ -137,7 +137,7 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
         {
             if( GetScreen()->GetItem( gridPosition, 0, SCH_JUNCTION_T ) == NULL )
             {
-                SCH_JUNCTION* junction = AddJunction( aDC, gridPosition, true );
+                SCH_JUNCTION* junction = AddJunction( gridPosition );
                 SetRepeatItem( junction );
                 GetScreen()->SetCurItem( junction );
                 m_canvas->SetAutoPanRequest( true );
@@ -442,7 +442,7 @@ void SCH_EDIT_FRAME::OnLeftDClick( wxDC* aDC, const wxPoint& aPosition )
     case ID_WIRE_BUTT:
     case ID_LINE_COMMENT_BUTT:
         if( item && item->IsNew() )
-            EndSegment( aDC );
+            EndSegment();
 
         break;
     }
diff --git a/eeschema/schedit.cpp b/eeschema/schedit.cpp
index 75e6aac8e..3e8f7babe 100644
--- a/eeschema/schedit.cpp
+++ b/eeschema/schedit.cpp
@@ -169,7 +169,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
 
     case ID_POPUP_END_LINE:
         m_canvas->MoveCursorToCrossHair();
-        EndSegment( &dc );
+        EndSegment();
         break;
 
     case ID_POPUP_SCH_BEGIN_WIRE:
@@ -372,7 +372,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
 
     case ID_POPUP_SCH_ADD_JUNCTION:
         m_canvas->MoveCursorToCrossHair();
-        screen->SetCurItem( AddJunction( &dc, GetCrossHairPosition(), true ) );
+        screen->SetCurItem( AddJunction( GetCrossHairPosition() ) );
 
         if( screen->TestDanglingEnds() )
             m_canvas->Refresh();
diff --git a/eeschema/schframe.h b/eeschema/schframe.h
index e56cd5539..1ee53fa63 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -932,18 +932,17 @@ private:
     /**
      * Function AddNoConnect
      * add a no connect item to the current schematic sheet at \a aPosition.
-     * @param aDC The device context to draw the no connect to.
      * @param aPosition The position in logical (drawing) units to add the no connect.
      * @return The no connect item added.
      */
-    SCH_NO_CONNECT* AddNoConnect( wxDC* aDC, const wxPoint& aPosition );
+    SCH_NO_CONNECT* AddNoConnect( const wxPoint& aPosition );
 
     /**
      * Function AddJunction
      * adds a new junction at \a aPosition.
      * @
      */
-    SCH_JUNCTION* AddJunction( wxDC* aDC, const wxPoint& aPosition, bool aAppend = false );
+    SCH_JUNCTION* AddJunction( const wxPoint& aPosition, bool aAppend = false );
 
     /**
      * Function SaveWireImage
@@ -996,7 +995,7 @@ private:
      * Function EndSegment
      * called to terminate a bus, wire, or line creation
      */
-    void EndSegment( wxDC* DC );
+    void EndSegment();
 
     /**
      * Function DeleteCurrentSegment
-- 
2.11.0

From ae2bb1957db65e6b06cff9cd642343bc2412a9bd Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 26 Oct 2017 08:25:47 -0700
Subject: [PATCH 12/13] Eeschema: Moving BreakSegment into SCH_EDIT_FRAME

BreakSegment now breaks a known segment and BreakSegments
breaks all segments. This allows functions to break a
segment without needing to iterate through the whole list.
---
 eeschema/block.cpp             |  3 --
 eeschema/bus-wire-junction.cpp | 86 +++++++++++++++++++++++++++++++++++++++++-
 eeschema/class_sch_screen.h    | 17 ---------
 eeschema/sch_screen.cpp        | 59 -----------------------------
 eeschema/schedit.cpp           | 23 +----------
 eeschema/schframe.h            | 28 ++++++++++++++
 6 files changed, 115 insertions(+), 101 deletions(-)

diff --git a/eeschema/block.cpp b/eeschema/block.cpp
index 4c100e49e..efffdef5c 100644
--- a/eeschema/block.cpp
+++ b/eeschema/block.cpp
@@ -238,9 +238,6 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
 
         case BLOCK_DRAG:
         case BLOCK_DRAG_ITEM:   // Drag from a drag command
-            GetScreen()->BreakSegmentsOnJunctions();
-            // fall through
-
         case BLOCK_MOVE:
         case BLOCK_DUPLICATE:
             if( block->GetCommand() == BLOCK_DRAG_ITEM &&
diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index 6b6656412..e90c2cbe6 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -410,6 +410,29 @@ void SCH_EDIT_FRAME::DeleteCurrentSegment( wxDC* DC )
     screen->SetCurItem( NULL );
 }
 
+void SCH_EDIT_FRAME::SaveWireImage()
+{
+    DLIST< SCH_ITEM > oldWires;
+
+    oldWires.SetOwnership( false );      // Prevent DLIST for deleting items in destructor.
+    GetScreen()->ExtractWires( oldWires, true );
+
+    if( oldWires.GetCount() != 0 )
+    {
+        PICKED_ITEMS_LIST oldItems;
+
+        oldItems.m_Status = UR_WIRE_IMAGE;
+
+        while( oldWires.GetCount() != 0 )
+        {
+            ITEM_PICKER picker = ITEM_PICKER( oldWires.PopFront(), UR_WIRE_IMAGE );
+            oldItems.PushItem( picker );
+        }
+
+        SaveCopyInUndoList( oldItems, UR_WIRE_IMAGE );
+    }
+}
+
 
 bool SCH_EDIT_FRAME::SchematicCleanUp()
 {
@@ -459,6 +482,67 @@ bool SCH_EDIT_FRAME::SchematicCleanUp()
     return modified;
 }
 
+bool SCH_EDIT_FRAME::BreakSegment( SCH_LINE *aSegment, const wxPoint& aPoint, bool aAppend )
+{
+    if( !IsPointOnSegment( aSegment->GetStartPoint(), aSegment->GetEndPoint(), aPoint )
+            || aSegment->IsEndPoint( aPoint ) )
+        return false;
+
+    SaveCopyInUndoList( aSegment, UR_CHANGED, aAppend );
+    SCH_LINE* newSegment = new SCH_LINE( *aSegment );
+    SaveCopyInUndoList( newSegment, UR_NEW, true );
+
+    newSegment->SetStartPoint( aPoint );
+    aSegment->SetEndPoint( aPoint );
+    GetScreen()->Append( newSegment );
+
+    return true;
+}
+
+
+bool SCH_EDIT_FRAME::BreakSegments( const wxPoint& aPoint, bool aAppend )
+{
+    bool brokenSegments = false;
+
+    for( SCH_ITEM* segment = GetScreen()->GetDrawItems(); segment; segment = segment->Next() )
+    {
+        if( ( segment->Type() != SCH_LINE_T ) || ( segment->GetLayer() == LAYER_NOTES ) )
+            continue;
+
+        brokenSegments |= BreakSegment( (SCH_LINE*) segment, aPoint, aAppend || brokenSegments );
+    }
+
+    return brokenSegments;
+}
+
+
+bool SCH_EDIT_FRAME::BreakSegmentsOnJunctions()
+{
+    bool brokenSegments = false;
+
+    for( SCH_ITEM* item = GetScreen()->GetDrawItems(); item; item = item->Next() )
+    {
+        if( item->Type() == SCH_JUNCTION_T )
+        {
+            SCH_JUNCTION* junction = ( SCH_JUNCTION* ) item;
+
+            if( BreakSegments( junction->GetPosition(), brokenSegments ) )
+                brokenSegments = true;
+        }
+        else
+        {
+            SCH_BUS_ENTRY_BASE* busEntry = dynamic_cast<SCH_BUS_ENTRY_BASE*>( item );
+            if( busEntry )
+            {
+                if( BreakSegments( busEntry->GetPosition(), brokenSegments )
+                 || BreakSegments( busEntry->m_End(), brokenSegments ) )
+                    brokenSegments = true;
+            }
+        }
+    }
+
+    return brokenSegments;
+}
 
 
 SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( const wxPoint& aPosition, bool aAppend )
@@ -468,7 +552,7 @@ SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( const wxPoint& aPosition, bool aAppen
     bool broken_segments = false;
 
     screen->Append( junction );
-    broken_segments = screen->BreakSegment( aPosition );
+    broken_segments = BreakSegments( aPosition, true );
     screen->TestDanglingEnds();
     OnModify();
     SaveCopyInUndoList( junction, UR_NEW, broken_segments || aAppend );
diff --git a/eeschema/class_sch_screen.h b/eeschema/class_sch_screen.h
index 76e3a3eb8..af5d48291 100644
--- a/eeschema/class_sch_screen.h
+++ b/eeschema/class_sch_screen.h
@@ -301,23 +301,6 @@ public:
      */
     int GetConnection( const wxPoint& aPosition, PICKED_ITEMS_LIST& aList, bool aFullConnection );
 
-    /**
-     * Function BreakSegment
-     * checks every wire and bus for a intersection at \a aPoint and break into two segments
-     * at \a aPoint if an intersection is found.
-     * @param aPoint Test this point for an intersection.
-     * @return True if any wires or buses were broken.
-     */
-    bool BreakSegment( const wxPoint& aPoint );
-
-    /**
-     * Function BreakSegmentsOnJunctions
-     * tests all junctions and bus entries in the schematic for intersections with wires and
-     * buses and breaks any intersections into multiple segments.
-     * @return True if any wires or buses were broken.
-     */
-    bool BreakSegmentsOnJunctions();
-
     /* full undo redo management : */
     // use BASE_SCREEN::PushCommandToUndoList( PICKED_ITEMS_LIST* aItem )
     // use BASE_SCREEN::PushCommandToRedoList( PICKED_ITEMS_LIST* aItem )
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index 7b859e1f3..400faf96f 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -900,64 +900,6 @@ bool SCH_SCREEN::TestDanglingEnds()
 }
 
 
-bool SCH_SCREEN::BreakSegment( const wxPoint& aPoint )
-{
-    SCH_LINE* segment;
-    SCH_LINE* newSegment;
-    bool brokenSegments = false;
-
-    for( SCH_ITEM* item = m_drawList.begin(); item; item = item->Next() )
-    {
-        if( (item->Type() != SCH_LINE_T) || (item->GetLayer() == LAYER_NOTES) )
-            continue;
-
-        segment = (SCH_LINE*) item;
-
-        if( !segment->HitTest( aPoint, 0 ) || segment->IsEndPoint( aPoint ) )
-            continue;
-
-        // Break the segment at aPoint and create a new segment.
-        newSegment = new SCH_LINE( *segment );
-        newSegment->SetStartPoint( aPoint );
-        segment->SetEndPoint( aPoint );
-        m_drawList.Insert( newSegment, segment->Next() );
-        item = newSegment;
-        brokenSegments = true;
-    }
-
-    return brokenSegments;
-}
-
-
-bool SCH_SCREEN::BreakSegmentsOnJunctions()
-{
-    bool brokenSegments = false;
-
-    for( SCH_ITEM* item = m_drawList.begin(); item; item = item->Next() )
-    {
-        if( item->Type() == SCH_JUNCTION_T )
-        {
-            SCH_JUNCTION* junction = ( SCH_JUNCTION* ) item;
-
-            if( BreakSegment( junction->GetPosition() ) )
-                brokenSegments = true;
-        }
-        else
-        {
-            SCH_BUS_ENTRY_BASE* busEntry = dynamic_cast<SCH_BUS_ENTRY_BASE*>( item );
-            if( busEntry )
-            {
-                if( BreakSegment( busEntry->GetPosition() )
-                 || BreakSegment( busEntry->m_End() ) )
-                    brokenSegments = true;
-            }
-        }
-    }
-
-    return brokenSegments;
-}
-
-
 int SCH_SCREEN::GetNode( const wxPoint& aPosition, EDA_ITEMS& aList )
 {
     for( SCH_ITEM* item = m_drawList.begin(); item; item = item->Next() )
@@ -1102,7 +1044,6 @@ int SCH_SCREEN::GetConnection( const wxPoint& aPosition, PICKED_ITEMS_LIST& aLis
 
     // Clear flags member for all items.
     ClearDrawingState();
-    BreakSegmentsOnJunctions();
 
     if( GetNode( aPosition, list ) == 0 )
         return 0;
diff --git a/eeschema/schedit.cpp b/eeschema/schedit.cpp
index 3e8f7babe..232ddf6a0 100644
--- a/eeschema/schedit.cpp
+++ b/eeschema/schedit.cpp
@@ -200,28 +200,9 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
 
     case ID_POPUP_SCH_BREAK_WIRE:
         {
-            DLIST< SCH_ITEM > oldWires;
-
-            oldWires.SetOwnership( false );      // Prevent DLIST for deleting items in destructor.
+            SaveWireImage();
             m_canvas->MoveCursorToCrossHair();
-            screen->ExtractWires( oldWires, true );
-            screen->BreakSegment( GetCrossHairPosition() );
-
-            if( oldWires.GetCount() != 0 )
-            {
-                PICKED_ITEMS_LIST oldItems;
-
-                oldItems.m_Status = UR_WIRE_IMAGE;
-
-                while( oldWires.GetCount() != 0 )
-                {
-                    ITEM_PICKER picker = ITEM_PICKER( oldWires.PopFront(), UR_WIRE_IMAGE );
-                    oldItems.PushItem( picker );
-                }
-
-                SaveCopyInUndoList( oldItems, UR_WIRE_IMAGE );
-            }
-
+            BreakSegments( GetCrossHairPosition() );
             if( screen->TestDanglingEnds() )
                 m_canvas->Refresh();
         }
diff --git a/eeschema/schframe.h b/eeschema/schframe.h
index 1ee53fa63..2dea042fd 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -485,6 +485,34 @@ public:
                                     bool            aWarpMouse );
 
     /**
+     * Function BreakSegment
+     * Breaks a single segment into two at the specified point
+     * @param aSegment Line segment to break
+     * @param aPoint Point at which to break the segment
+     * @param aAppend Add the changes to the previous undo state
+     * @return True if any wires or buses were broken.
+     */
+    bool BreakSegment( SCH_LINE* aSegment, const wxPoint& aPoint, bool aAppend = false );
+
+    /**
+     * Function BreakSegments
+     * checks every wire and bus for a intersection at \a aPoint and break into two segments
+     * at \a aPoint if an intersection is found.
+     * @param aPoint Test this point for an intersection.
+     * @param aAppend Add the changes to the previous undo state
+     * @return True if any wires or buses were broken.
+     */
+    bool BreakSegments( const wxPoint& aPoint, bool aAppend = false );
+
+    /**
+     * Function BreakSegmentsOnJunctions
+     * tests all junctions and bus entries in the schematic for intersections with wires and
+     * buses and breaks any intersections into multiple segments.
+     * @return True if any wires or buses were broken.
+     */
+    bool BreakSegmentsOnJunctions();
+
+    /**
      * Function SendMessageToPcbnew
      * send a remote to Pcbnew via a socket connection.
      * @param aObjectToSync = item to be located on board
-- 
2.11.0

From 8f8cc60ebe32bcd62d51277e78dcdac094dd21d0 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 26 Oct 2017 08:55:55 -0700
Subject: [PATCH 13/13] Eeschema: Automatically manage junctions

---
 eeschema/block.cpp                     |  15 ++-
 eeschema/bus-wire-junction.cpp         | 210 +++++++++++++++++++++------------
 eeschema/onleftclick.cpp               |   8 ++
 eeschema/operations_on_items_lists.cpp |  26 +++-
 eeschema/sch_line.cpp                  | 111 +++++++++--------
 eeschema/sch_line.h                    |   6 +-
 eeschema/sch_screen.cpp                |  39 ++++--
 eeschema/schedit.cpp                   |   1 +
 eeschema/schematic_undo_redo.cpp       |   5 +-
 eeschema/schframe.h                    |  12 +-
 10 files changed, 282 insertions(+), 151 deletions(-)

diff --git a/eeschema/block.cpp b/eeschema/block.cpp
index efffdef5c..0506c2b25 100644
--- a/eeschema/block.cpp
+++ b/eeschema/block.cpp
@@ -113,7 +113,6 @@ void SCH_EDIT_FRAME::InitBlockPasteInfos()
 void SCH_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
 {
     BLOCK_SELECTOR* block = &GetScreen()->m_BlockLocate;
-
     if( !m_canvas->IsMouseCaptured() )
     {
         DisplayError( this, wxT( "HandleBlockPLace() : m_mouseCaptureCallback = NULL" ) );
@@ -139,6 +138,8 @@ void SCH_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
 
         SaveCopyInUndoList( block->GetItems(), UR_MOVED, false, block->GetMoveVector() );
         MoveItemsInList( block->GetItems(), block->GetMoveVector() );
+        CheckJunctionsInList( block->GetItems(), true );
+        SchematicCleanUp( true );
         block->ClearItemsList();
         break;
 
@@ -152,6 +153,9 @@ void SCH_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
         SaveCopyInUndoList( block->GetItems(),
                             ( block->GetCommand() == BLOCK_PRESELECT_MOVE ) ? UR_CHANGED : UR_NEW );
 
+        CheckJunctionsInList( block->GetItems(), true );
+        SchematicCleanUp( true );
+
         block->ClearItemsList();
         break;
 
@@ -160,6 +164,9 @@ void SCH_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
             m_canvas->CallMouseCapture( DC, wxDefaultPosition, false );
 
         PasteListOfItems( DC );
+
+        CheckJunctionsInList( block->GetItems(), true );
+        SchematicCleanUp( true );
         block->ClearItemsList();
         break;
 
@@ -228,6 +235,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 SetCrossHairPosition( rotationPoint );
                 SaveCopyInUndoList( block->GetItems(), UR_ROTATED, false, rotationPoint );
                 RotateListOfItems( block->GetItems(), rotationPoint );
+                SchematicCleanUp( true );
                 OnModify();
             }
 
@@ -311,6 +319,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 copyBlockItems( block->GetItems() );
                 MoveItemsInList( m_blockItems.GetItems(), move_vector );
                 DeleteItemsInList( block->GetItems() );
+                SchematicCleanUp( true );
                 OnModify();
             }
 
@@ -339,6 +348,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 SetCrossHairPosition( mirrorPoint );
                 SaveCopyInUndoList( block->GetItems(), UR_MIRRORED_X, false, mirrorPoint );
                 MirrorX( block->GetItems(), mirrorPoint );
+                SchematicCleanUp( true );
                 OnModify();
             }
 
@@ -358,6 +368,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 SetCrossHairPosition( mirrorPoint );
                 SaveCopyInUndoList( block->GetItems(), UR_MIRRORED_Y, false, mirrorPoint );
                 MirrorY( block->GetItems(), mirrorPoint );
+                SchematicCleanUp( true );
                 OnModify();
             }
 
@@ -544,6 +555,8 @@ void SCH_EDIT_FRAME::PasteListOfItems( wxDC* DC )
 
     MoveItemsInList( picklist, GetScreen()->m_BlockLocate.GetMoveVector() );
 
+    SchematicCleanUp( true );
+
     // Clear flags for all items.
     GetScreen()->ClearDrawingState();
 
diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index e90c2cbe6..91fd5220a 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -265,77 +265,71 @@ void SCH_EDIT_FRAME::EndSegment()
 {
     SCH_SCREEN* screen = GetScreen();
     SCH_LINE* segment = (SCH_LINE*) screen->GetCurItem();
+    PICKED_ITEMS_LIST itemList;
 
     if( segment == NULL || segment->Type() != SCH_LINE_T || !segment->IsNew() )
         return;
 
-    // Delete zero length segments and clear item flags.
-    SCH_ITEM* item = s_wires.begin();
-
-    while( item )
-    {
-        item->ClearFlags();
-
-        wxCHECK_RET( item->Type() == SCH_LINE_T, wxT( "Unexpected object type in wire list." ) );
-
-        segment = (SCH_LINE*) item;
-        item = item->Next();
-
-        if( segment->IsNull() )
-            delete s_wires.Remove( segment );
-    }
+    // Remove segments backtracking over others
+    RemoveBacktracks( s_wires );
 
     if( s_wires.GetCount() == 0 )
         return;
 
-    // Get the last non-null wire (this is the last created segment).
-    SetRepeatItem( segment = (SCH_LINE*) s_wires.GetLast() );
-
-    screen->SetCurItem( NULL );
-    m_canvas->EndMouseCapture( -1, -1, wxEmptyString, false );
+    // Collect the possible connection points for the new lines
+    std::vector< wxPoint > connections;
+    std::vector< wxPoint > new_ends;
+    for( SCH_ITEM* item = screen->GetDrawItems(); item; item = item->Next() )
+    {
+        int layer = item->GetLayer();
+        if( layer == LAYER_WIRE || layer == LAYER_BUS || layer == LAYER_PIN )
+            item->GetConnectionPoints( connections );
+    }
 
-    // store the terminal point of this last segment: a junction could be needed
-    // (the last wire could be merged/deleted/modified, and lost)
-    wxPoint endpoint = segment->GetEndPoint();
+    // We always have some overlapping connection points.  Drop duplicates here
+    std::sort( connections.begin(), connections.end(),
+            []( wxPoint& a, wxPoint& b ) -> bool
+            { return a.x < b.x || (a.x == b.x && a.y < b.y); } );
+    connections.erase( unique( connections.begin(), connections.end() ), connections.end() );
 
-    // store the starting point of this first segment: a junction could be needed
-    SCH_LINE* firstsegment = (SCH_LINE*) s_wires.GetFirst();
-    wxPoint startPoint = firstsegment->GetStartPoint();
+    // Check each new segment for possible junctions and add/split if needed
+    for( SCH_ITEM* wire = s_wires.GetFirst(); wire; wire=wire->Next() )
+    {
+        SCH_LINE* test_line = (SCH_LINE*) wire;
+        if( wire->GetFlags() & SKIP_STRUCT )
+            continue;
 
-    // Save the old wires for the undo command
-    DLIST< SCH_ITEM > oldWires;                     // stores here the old wires
-    GetScreen()->ExtractWires( oldWires, true );    // Save them in oldWires list
-    // Put the snap shot of the previous wire, buses, and junctions in the undo/redo list.
-    PICKED_ITEMS_LIST oldItems;
-    oldItems.m_Status = UR_WIRE_IMAGE;
+        wire->GetConnectionPoints( new_ends );
 
-    while( oldWires.GetCount() != 0 )
-    {
-        ITEM_PICKER picker = ITEM_PICKER( oldWires.PopFront(), UR_WIRE_IMAGE );
-        oldItems.PushItem( picker );
+        for( auto i : connections )
+        {
+            if( IsPointOnSegment( test_line->GetStartPoint(), test_line->GetEndPoint(), i ) )
+            {
+                new_ends.push_back( i );
+            }
+        }
+        itemList.PushItem( ITEM_PICKER( wire, UR_NEW ) );
     }
 
-    SaveCopyInUndoList( oldItems, UR_WIRE_IMAGE );
-
-    // Remove segments backtracking over others
-    RemoveBacktracks( s_wires );
+    // Get the last non-null wire (this is the last created segment).
+    SetRepeatItem( segment = (SCH_LINE*) s_wires.GetLast() );
 
     // Add the new wires
     screen->Append( s_wires );
+    SaveCopyInUndoList(itemList, UR_NEW);
 
-    // Correct and remove segments that need to be merged.
-    SchematicCleanUp();
-
-    // A junction could be needed to connect the end point of the last created segment.
-    if( screen->IsJunctionNeeded( endpoint ) )
-        screen->Append( AddJunction( endpoint ) );
-
-    // A junction could be needed to connect the start point of the set of new created wires
-    if( screen->IsJunctionNeeded( startPoint ) )
-        screen->Append( AddJunction( startPoint ) );
+    // Correct and remove segments that need to be merged.SchematicCleanUp( true );
+    for( auto i : new_ends )
+    {
+        if( screen->IsJunctionNeeded( i, true ) )
+            AddJunction( i, true );
+    }
 
-    m_canvas->Refresh();
+    GetScreen()->TestDanglingEnds();
+    GetScreen()->ClearDrawingState();
 
+    screen->SetCurItem( NULL );
+    m_canvas->EndMouseCapture( -1, -1, wxEmptyString, false );
     OnModify();
 }
 
@@ -434,50 +428,112 @@ void SCH_EDIT_FRAME::SaveWireImage()
 }
 
 
-bool SCH_EDIT_FRAME::SchematicCleanUp()
+bool SCH_EDIT_FRAME::SchematicCleanUp( bool aAppend )
 {
-    bool      modified = false;
+    bool                modified = false;
+    SCH_ITEM*           firstNextItem = NULL;
+    SCH_ITEM*           secondNextItem = NULL;
+    PICKED_ITEMS_LIST   itemList;
+    SCH_SCREEN*         screen = GetScreen();
 
-    for( SCH_ITEM* item = GetScreen()->GetDrawItems() ; item; item = item->Next() )
+    for( SCH_ITEM* firstItem = screen->GetDrawItems() ; firstItem; firstItem = firstNextItem )
     {
-        if( ( item->Type() != SCH_LINE_T ) && ( item->Type() != SCH_JUNCTION_T ) )
+        aAppend |= modified;
+
+        // Set the next_item here because we may need to delete firstItem
+        firstNextItem = firstItem->Next();
+        if( ( firstItem->Type() != SCH_LINE_T ) &&
+            ( firstItem->Type() != SCH_JUNCTION_T ) &&
+            ( firstItem->Type() != SCH_NO_CONNECT_T ))
+            continue;
+
+        if( firstItem->GetFlags() & STRUCT_DELETED )
             continue;
 
-        bool restart;
+        // Remove unneeded junctions
+        if( ( firstItem->Type() == SCH_JUNCTION_T )
+                && ( !screen->IsJunctionNeeded( firstItem->GetPosition() ) ) )
+        {
+            DeleteItem( firstItem, aAppend );
+            modified = true;
+            continue;
+        }
+        // Remove zero-length lines
+        if( firstItem->Type() == SCH_LINE_T )
+        {
+            SCH_LINE* firstLine = (SCH_LINE*) firstItem;
+            if( firstLine->GetStartPoint() == firstLine->GetEndPoint() )
+            {
+                DeleteItem( firstItem, aAppend );
+                modified = true;
+                continue;
+            }
+        }
 
-        for( SCH_ITEM* testItem = item->Next(); testItem; testItem = restart ? GetScreen()->GetDrawItems() : testItem->Next() )
+        for( SCH_ITEM* secondItem = firstNextItem; secondItem; secondItem = secondNextItem )
         {
-            restart = false;
+            secondNextItem = secondItem->Next();
+            if( firstItem->Type() != secondItem->Type()
+                    || ( secondItem->GetFlags() & STRUCT_DELETED ) )
+                continue;
 
-            if( ( item->Type() == SCH_LINE_T ) && ( testItem->Type() == SCH_LINE_T ) )
+            // Merge overlapping lines
+            if( firstItem->Type() == SCH_LINE_T )
             {
-                SCH_LINE* line = (SCH_LINE*) item;
+                SCH_LINE* firstLine = (SCH_LINE*) firstItem;
+                SCH_LINE* secondLine = (SCH_LINE*) secondItem;
+                SCH_LINE* line = NULL;
+                bool isNeeded = false;
+
+                if( secondLine->IsEndPoint( firstLine->GetStartPoint() ) )
+                    isNeeded = GetScreen()->IsJunctionNeeded( firstLine->GetStartPoint() );
+                else if( secondLine->IsEndPoint( firstLine->GetEndPoint() ) )
+                    isNeeded = GetScreen()->IsJunctionNeeded( firstLine->GetEndPoint() );
 
-                if( line->MergeOverlap( (SCH_LINE*) testItem ) )
+                if( !isNeeded && ( line = (SCH_LINE*) secondLine->MergeOverlap( firstLine ) ) )
                 {
-                    // Keep the current flags, because the deleted segment can be flagged.
-                    item->SetFlags( testItem->GetFlags() );
-                    DeleteItem( testItem );
-                    restart = true;
+                    line->SetFlags( firstItem->GetFlags() | secondItem->GetFlags() );
+                    itemList.PushItem( ITEM_PICKER( line, UR_NEW ) );
+
+                    firstItem->SetFlags( STRUCT_DELETED );
+                    itemList.PushItem( ITEM_PICKER( firstItem, UR_DELETED ) );
+                    secondItem->SetFlags( STRUCT_DELETED );
+                    itemList.PushItem( ITEM_PICKER( secondItem, UR_DELETED ) );
+
+                    // Ensure that the loop pointers remain valid before deleting.
+                    // This can only happen on the first loop
+                    if( secondItem == firstNextItem )
+                    {
+                        if( ( firstNextItem = secondNextItem ) )
+                            secondNextItem = secondNextItem->Next();
+                    }
+
+                    screen->Append( (SCH_ITEM*) line );
+                    screen->Remove( firstItem );
+                    screen->Remove( secondItem );
+
                     modified = true;
+                    break;
                 }
             }
-            else if ( ( ( item->Type() == SCH_JUNCTION_T )
-                      && ( testItem->Type() == SCH_JUNCTION_T ) ) && ( testItem != item ) )
+            // Remove duplicate junctions
+            else if( secondItem->GetPosition() == firstItem->GetPosition() )
             {
-                if ( testItem->HitTest( item->GetPosition() ) )
-                {
-                    // Keep the current flags, because the deleted segment can be flagged.
-                    item->SetFlags( testItem->GetFlags() );
-                    DeleteItem( testItem );
-                    restart = true;
-                    modified = true;
-                }
+                itemList.PushItem( ITEM_PICKER( secondItem, UR_CHANGED ) );
+
+                // Keep the current flags, because the deleted junction can be flagged.
+                secondItem->SetFlags( firstItem->GetFlags() );
+
+                DeleteItem( firstItem, aAppend );
+                modified = true;
+                firstNextItem = screen->GetDrawItems();
+                break;
             }
         }
     }
 
-    GetScreen()->TestDanglingEnds();
+    if( modified )
+        SaveCopyInUndoList( itemList, UR_CHANGED, aAppend );
 
     return modified;
 }
@@ -564,11 +620,9 @@ SCH_NO_CONNECT* SCH_EDIT_FRAME::AddNoConnect( const wxPoint& aPosition )
 {
     SCH_NO_CONNECT* no_connect = new SCH_NO_CONNECT( aPosition );
 
-    SetRepeatItem( no_connect );
     GetScreen()->Append( no_connect );
     SchematicCleanUp();
     OnModify();
-    m_canvas->Refresh();
     SaveCopyInUndoList( no_connect, UR_NEW );
     return no_connect;
 }
diff --git a/eeschema/onleftclick.cpp b/eeschema/onleftclick.cpp
index 7892af06d..a49a431fa 100644
--- a/eeschema/onleftclick.cpp
+++ b/eeschema/onleftclick.cpp
@@ -85,6 +85,14 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
             case SCH_BITMAP_T:
             case SCH_NO_CONNECT_T:
                 addCurrentItemToList();
+                if( item->IsConnectable() )
+                {
+                    std::vector< wxPoint > pts;
+                    item->GetConnectionPoints( pts );
+                    for( auto i : pts )
+                        if( GetScreen()->IsJunctionNeeded( i, true ) )
+                            AddJunction( i, true );
+                }
                 return;
 
             case SCH_LINE_T:    // May already be drawing segment.
diff --git a/eeschema/operations_on_items_lists.cpp b/eeschema/operations_on_items_lists.cpp
index 3c61be363..071af1543 100644
--- a/eeschema/operations_on_items_lists.cpp
+++ b/eeschema/operations_on_items_lists.cpp
@@ -121,6 +121,27 @@ void MoveItemsInList( PICKED_ITEMS_LIST& aItemsList, const wxPoint aMoveVector )
 }
 
 
+void SCH_EDIT_FRAME::CheckJunctionsInList( PICKED_ITEMS_LIST& aItemsList, bool aAppend )
+{
+    for( unsigned ii = 0; ii < aItemsList.GetCount(); ii++ )
+    {
+        SCH_ITEM* item = (SCH_ITEM*) aItemsList.GetPickedItem( ii );
+        if( item->IsConnectable() )
+        {
+            std::vector< wxPoint > pts;
+            item->GetConnectionPoints( pts );
+            for( auto point : pts )
+            {
+                if( GetScreen()->IsJunctionNeeded( point, true ) )
+                {
+                    AddJunction( point, aAppend );
+                    aAppend = true;
+                }
+            }
+        }
+    }
+}
+
 /**
  * Function DeleteItemsInList
  * delete schematic items in aItemsList
@@ -138,7 +159,8 @@ void SCH_EDIT_FRAME::DeleteItemsInList( PICKED_ITEMS_LIST& aItemsList, bool aApp
         if( item->GetFlags() & STRUCT_DELETED )
             continue;
 
-        DeleteItem( item, aAppend || !!ii );
+        DeleteItem( item, aAppend );
+        aAppend = true;
     }
 
     GetScreen()->ClearDrawingState();
@@ -172,7 +194,7 @@ void SCH_EDIT_FRAME::DeleteItem( SCH_ITEM* aItem, bool aAppend )
         itemsList.PushItem( picker );
         screen->Remove( aItem );
 
-        if( aItem->IsConnectable() )
+        if( aItem->IsConnectable() && aItem->Type() != SCH_JUNCTION_T )
         {
             std::vector< wxPoint > pts;
             aItem->GetConnectionPoints( pts );
diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index ee0d60659..affde22b6 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -301,14 +301,13 @@ bool SCH_LINE::IsParallel( SCH_LINE* aLine )
  * 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 )
+EDA_ITEM* SCH_LINE::MergeOverlap( SCH_LINE* aLine )
 {
-    SCH_LINE *leftmost, *rightmost;
-    wxCHECK_MSG( aLine != NULL && aLine->Type() == SCH_LINE_T, false,
+    wxCHECK_MSG( aLine != NULL && aLine->Type() == SCH_LINE_T, NULL,
                  wxT( "Cannot test line segment for overlap." ) );
 
     if( this == aLine || GetLayer() != aLine->GetLayer() )
-        return false;
+        return NULL;
 
     auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
     {
@@ -317,76 +316,85 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
         return lhs.x < rhs.x;
     };
 
-    if( m_start != std::min( { m_start, m_end }, less ) )
-        std::swap( m_start, m_end );
-    if( aLine->m_start != std::min( { aLine->m_start, aLine->m_end }, less ) )
-        std::swap( aLine->m_start, aLine->m_end );
-
-    if( less( m_start, aLine->m_start ) )
+    SCH_LINE *leftmost = new SCH_LINE( *aLine );
+    SCH_LINE *rightmost = new 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 ) )
     {
-        leftmost = this;
-        rightmost = aLine;
+        std::swap( leftmost, rightmost );
     }
-    else
+    SCH_LINE *other = 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 ) )
     {
-        leftmost = aLine;
-        rightmost = this;
+        delete leftmost;
+        delete other;
+        return NULL;
     }
 
     // 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( ( leftmost->m_start == other->m_start )
+            && ( leftmost->m_end == other->m_end ) )     // Trivial case
     {
-        if( m_end == aLine->m_start )     // Trivial case
-            return true;
-    }
-    else if( m_end == aLine->m_end )
-    {
-        std::swap( aLine->m_start, aLine->m_end );
-    }
-    else if( m_end != aLine->m_start )
-    {
-        // No common end point, check for maybe overlapping
-        if( less ( leftmost->m_end, rightmost->m_start ) )
-            return false;
+        delete other;
+        return leftmost;
     }
 
     bool colinear = false;
 
     /* Test alignment: */
-    if( ( m_start.y == m_end.y ) && ( aLine->m_start.y == aLine->m_end.y ) )       // Horizontal segment
+    if( ( leftmost->m_start.y == leftmost->m_end.y )
+            && ( other->m_start.y == other->m_end.y ) )       // Horizontal segment
     {
-        colinear = ( m_start.y == aLine->m_start.y );
+        colinear = ( leftmost->m_start.y == other->m_start.y );
     }
-    else if( ( m_start.x == m_end.x ) && ( aLine->m_start.x == aLine->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
     {
-        colinear = ( m_start.x == aLine->m_start.x );
+        colinear = ( leftmost->m_start.x == other->m_start.x );
     }
     else
     {
-        int dx = leftmost->m_end.x - leftmost->m_start.x;
-        int dy = leftmost->m_end.y - leftmost->m_start.y;
-        colinear = ( ( ( rightmost->m_start.y - leftmost->m_start.y ) * dx ==
-                ( rightmost->m_start.x - leftmost->m_start.x ) * dy ) &&
-            ( ( rightmost->m_end.y - leftmost->m_start.y ) * dx ==
-                ( rightmost->m_end.x - leftmost->m_start.x ) * dy ) );
+        // 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 )
     {
-        m_start = leftmost->m_start;
-        m_end = rightmost->m_end;
-        return true;
+        leftmost->m_end = rightmost->m_end;
+        delete other;
+        return leftmost;
     }
-    return false;
+
+    delete leftmost;
+    delete other;
+
+    return NULL;
 }
 
 
@@ -611,7 +619,6 @@ void SCH_LINE::SwapData( SCH_ITEM* aItem )
     SCH_LINE* item = (SCH_LINE*) aItem;
 
     std::swap( m_Layer, item->m_Layer );
-
     std::swap( m_start, item->m_start );
     std::swap( m_end, item->m_end );
     std::swap( m_startIsDangling, item->m_startIsDangling );
diff --git a/eeschema/sch_line.h b/eeschema/sch_line.h
index 9aece825f..588fd6c02 100644
--- a/eeschema/sch_line.h
+++ b/eeschema/sch_line.h
@@ -104,14 +104,14 @@ public:
     /**
      * Check line against \a aLine to see if it overlaps and merge if it does.
      *
-     * This method will change the line to be equivalent of the line and \a aLine if the
+     * This method will return an equivalent of the union of 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.
+     * @return New line that combines the two or NULL on non-overlapping segments.
      */
-    bool MergeOverlap( SCH_LINE* aLine );
+    EDA_ITEM* MergeOverlap( SCH_LINE* aLine );
 
     /**
      * Check if two lines are in the same quadrant as each other, using a reference point as
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index 400faf96f..7575b018b 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -331,15 +331,19 @@ void SCH_SCREEN::MarkConnections( SCH_LINE* aSegment )
     }
 }
 
-
 bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew )
 {
     bool has_line = false;
-    int end_count = 0;
+    int total_count = 0;
+    int nonparallel_count = 0;
+
+    std::vector< SCH_LINE* > lines;
 
     for( SCH_ITEM* item = m_drawList.begin(); item; item = item->Next() )
     {
-        if( aNew && ( item->Type() == SCH_JUNCTION_T ) && ( item->HitTest(aPosition) ) )
+        if( item->GetFlags() & STRUCT_DELETED )
+            continue;
+        if( aNew && ( item->Type() == SCH_JUNCTION_T ) && ( item->HitTest( aPosition ) ) )
             return false;
 
         if( item->Type() != SCH_LINE_T )
@@ -348,22 +352,34 @@ bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew )
         if( item->GetLayer() != LAYER_WIRE )
             continue;
 
-        if( !item->HitTest( aPosition, 0 ) )
-            continue;
+        if( item->HitTest( aPosition, 0 ) )
+            lines.push_back( (SCH_LINE*) item );
+    }
 
-        if( ( (SCH_LINE*) item )->IsEndPoint( aPosition ) )
-            end_count++;
+    for( std::vector< SCH_LINE* >::iterator it = lines.begin(); it != lines.end(); ++it )
+    {
+        SCH_LINE* line = *it;
+        for( std::vector< SCH_LINE* >::iterator second_it = it + 1; second_it != lines.end(); ++ second_it )
+        {
+            SCH_LINE* second_line = *second_it;
+            if( !line->IsParallel( second_line ) )
+                nonparallel_count++;
+            // If the two lines are pointing in the same direction, we don't count the second
+            else if ( line->IsSameQuadrant( second_line, aPosition ) )
+                total_count--;
+        }
+        if( line->IsEndPoint( aPosition ) )
+            total_count++;
         else
             has_line = true;
     }
 
     int has_pin = !!( GetPin( aPosition, NULL, true ) );
-    if( end_count + has_pin > 2 )
-        return true;
 
-    if( has_line && ( end_count || has_pin ) )
+    if( ( has_line || total_count > 1 ) && has_pin )
+        return true;
+    if( nonparallel_count && ( has_line || total_count > 2 ) )
         return true;
-
     return false;
 }
 
@@ -1316,6 +1332,7 @@ void SCH_SCREENS::ClearAnnotation()
         m_screens[i]->ClearAnnotation( NULL );
 }
 
+
 int SCH_SCREENS::ReplaceDuplicateTimeStamps()
 {
     EDA_ITEMS items;
diff --git a/eeschema/schedit.cpp b/eeschema/schedit.cpp
index 232ddf6a0..20cd66525 100644
--- a/eeschema/schedit.cpp
+++ b/eeschema/schedit.cpp
@@ -214,6 +214,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
             break;
 
         DeleteItem( item );
+        SchematicCleanUp( true );
         screen->SetCurItem( NULL );
         SetRepeatItem( NULL );
         screen->TestDanglingEnds();
diff --git a/eeschema/schematic_undo_redo.cpp b/eeschema/schematic_undo_redo.cpp
index eaa6e5e59..1fac24360 100644
--- a/eeschema/schematic_undo_redo.cpp
+++ b/eeschema/schematic_undo_redo.cpp
@@ -189,11 +189,10 @@ void SCH_EDIT_FRAME::SaveCopyInUndoList( const PICKED_ITEMS_LIST& aItemsList,
     if( !commandToUndo )
     {
         commandToUndo = new PICKED_ITEMS_LIST();
+        commandToUndo->m_TransformPoint = aTransformPoint;
+        commandToUndo->m_Status = aTypeCommand;
     }
 
-    commandToUndo->m_TransformPoint = aTransformPoint;
-    commandToUndo->m_Status = aTypeCommand;
-
     // Copy picker list:
     if( !commandToUndo->GetCount() )
         commandToUndo->CopyList( aItemsList );
diff --git a/eeschema/schframe.h b/eeschema/schframe.h
index 2dea042fd..122f1186c 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -982,10 +982,11 @@ private:
      * Function SchematicCleanUp
      * performs routine schematic cleaning including breaking wire and buses and
      * deleting identical objects superimposed on top of each other.
+     * @param aAppend The changes to the schematic should be appended to the previous undo
      *
      * @return True if any schematic clean up was performed.
      */
-    bool SchematicCleanUp();
+    bool SchematicCleanUp( bool aAppend = false );
 
     /**
      * Function PrepareMoveItem
@@ -1156,6 +1157,15 @@ public:
      */
     void DeleteItemsInList( PICKED_ITEMS_LIST& aItemsList, bool aAppend = false );
 
+    /**
+     * Function CheckJunctionsInList
+     * Adds junctions if needed to each item in the list after they have been
+     * moved.
+     * @param aItemsList The list of items to check
+     * @param aAppend True if we are updating a previous commit
+     */
+    void CheckJunctionsInList( PICKED_ITEMS_LIST& aItemsList, bool aAppend = false );
+
     int GetLabelIncrement() const { return m_repeatLabelDelta; }
 
 private:
-- 
2.11.0


Follow ups

References