← Back to team overview

kicad-developers team mailing list archive

[PATCH] Eeschema automatic manage junctions

 

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​
From b52d51ea78fc30ccd4d89ca1002174d4468d5701 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 6 Oct 2017 10:42:55 -0700
Subject: [PATCH 1/8] 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 48c3cb397..fe13eb957 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 769fb1e5d60c789043f9c8689da1db8acabf3d26 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 14:08:14 -0700
Subject: [PATCH 2/8] 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 b6be2033f..7ccef5609 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 28ac54ef975ca74d4abddf3c832dea7941bb7ff8 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 15:07:55 -0700
Subject: [PATCH 3/8] 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 02f2030d1..63bea7eb6 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -334,20 +334,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;
 }
 
@@ -1260,7 +1278,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 245146cca5c2c2f3b9b60e2154b16d6954b4ef83 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 16:03:12 -0700
Subject: [PATCH 4/8] 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 c990fc2b0..a332615eb 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -924,8 +924,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
@@ -1083,6 +1100,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 );
 
@@ -1176,11 +1194,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 ) );
 
     /**
@@ -1189,11 +1209,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 485b52ea5cb7b8e1b4a56a0c0e823b44d54849e1 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 16:41:25 -0700
Subject: [PATCH 5/8] 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 23d4bdafa..d6fdf0613 100644
--- a/eeschema/schedit.cpp
+++ b/eeschema/schedit.cpp
@@ -656,7 +656,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 a332615eb..91066fc97 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -1102,7 +1102,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 bc26bd0e980dfef8311e673e0c6a2099561cdbc5 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 13 Oct 2017 15:08:33 -0700
Subject: [PATCH 6/8] 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 fe13eb957..d85b65d87 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 30435c6c1..97a42ba48 100644
--- a/eeschema/sch_line.h
+++ b/eeschema/sch_line.h
@@ -145,6 +145,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 05ea6fbd2f152bf46bf9eebf176f2234a45a95ae Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Mon, 16 Oct 2017 14:38:57 -0700
Subject: [PATCH 7/8] 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 d85b65d87..ab8b32d44 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 97a42ba48..7b8e7c5d4 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 2da7c1cbd3669bbd874a02cefae1ea8c1f25415f Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Mon, 16 Oct 2017 17:05:19 -0700
Subject: [PATCH 8/8] Eeschema:  Automatically manage junctions

Allows eeschema to automatically add/delete junctions when required
by the schematic.

Fixes: lp:593888
* https://bugs.launchpad.net/kicad/+bug/593888

Fixes: lp:1482111
* https://bugs.launchpad.net/kicad/+bug/1482111
---
 eeschema/block.cpp                     |  18 +-
 eeschema/bus-wire-junction.cpp         | 310 ++++++++++++++++++++++++++-------
 eeschema/class_sch_screen.h            |  32 ----
 eeschema/dialogs/dialog_erc.cpp        |   9 -
 eeschema/hierarch.cpp                  |   2 +-
 eeschema/netlist.cpp                   |   3 -
 eeschema/onleftclick.cpp               |  14 +-
 eeschema/operations_on_items_lists.cpp |  26 ++-
 eeschema/project_rescue.cpp            |   2 +-
 eeschema/sch_line.cpp                  | 111 ++++++------
 eeschema/sch_line.h                    |   6 +-
 eeschema/sch_screen.cpp                | 159 +++--------------
 eeschema/schedit.cpp                   |  28 +--
 eeschema/schematic_undo_redo.cpp       |   5 +-
 eeschema/schframe.cpp                  |   2 +-
 eeschema/schframe.h                    |  44 ++++-
 16 files changed, 431 insertions(+), 340 deletions(-)

diff --git a/eeschema/block.cpp b/eeschema/block.cpp
index 4c100e49e..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();
             }
 
@@ -238,9 +246,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 &&
@@ -314,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();
             }
 
@@ -342,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();
             }
 
@@ -361,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();
             }
 
@@ -547,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 528d09d68..ceed0bf93 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,81 +261,76 @@ 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();
+    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.
-    screen->SchematicCleanUp();
-
-    // A junction could be needed to connect the end point of the last created segment.
-    if( screen->IsJunctionNeeded( endpoint ) )
-        screen->Append( AddJunction( DC, 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 ) );
+    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();
 }
 
@@ -410,38 +405,219 @@ 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 );
+    }
+}
+
 
-SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( wxDC* aDC, const wxPoint& aPosition,
-                                           bool aPutInUndoList )
+bool SCH_EDIT_FRAME::SchematicCleanUp( bool aAppend )
 {
-    SCH_JUNCTION* junction = new SCH_JUNCTION( aPosition );
+    bool                modified = false;
+    SCH_ITEM*           nextItem = NULL;
+    PICKED_ITEMS_LIST   itemList;
 
-    SetRepeatItem( junction );
+    for( SCH_ITEM* firstItem = GetScreen()->GetDrawItems() ; firstItem; firstItem = nextItem )
+    {
+        aAppend |= modified;
 
-    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
+        // Set the next_item here because we may need to delete firstItem
+        nextItem = firstItem->Next();
+        if( ( firstItem->Type() != SCH_LINE_T ) &&
+            ( firstItem->Type() != SCH_JUNCTION_T ) &&
+            ( firstItem->Type() != SCH_NO_CONNECT_T ))
+            continue;
 
-    if( aPutInUndoList )
+        if( firstItem->GetFlags() & STRUCT_DELETED )
+            continue;
+
+        // Remove unneeded junctions
+        if( ( firstItem->Type() == SCH_JUNCTION_T )
+                && ( !GetScreen()->IsJunctionNeeded( firstItem->GetPosition() ) ) )
+        {
+            DeleteItem( firstItem, aAppend );
+            modified = true;
+            nextItem = GetScreen()->GetDrawItems();
+            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;
+                nextItem = GetScreen()->GetDrawItems();
+                continue;
+            }
+        }
+
+        for( SCH_ITEM* secondItem = nextItem; secondItem; secondItem = secondItem->Next() )
+        {
+            if( firstItem->Type() != secondItem->Type() ||
+                    ( secondItem->GetFlags() & STRUCT_DELETED ) )
+                continue;
+
+            // Merge overlapping lines
+            if( firstItem->Type() == SCH_LINE_T )
+            {
+                SCH_LINE* firstLine = (SCH_LINE*) firstItem;
+                SCH_LINE* secondLine = (SCH_LINE*) secondItem;
+                EDA_ITEM* line = secondLine->MergeOverlap( firstLine );
+                if( line )
+                {
+                    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 the split is needed, we delete the proposed merged line
+                    if( isNeeded )
+                        delete line;
+                    else
+                    {
+                        line->SetFlags( firstItem->GetFlags() | secondItem->GetFlags() );
+                        itemList.PushItem( ITEM_PICKER( line, UR_NEW ) );
+
+                        if( secondItem == nextItem )
+                            nextItem = secondItem->Next();
+
+                        GetScreen()->Append( (SCH_ITEM*) line );
+                        DeleteItem(firstItem, aAppend);
+                        DeleteItem(secondItem, aAppend);
+
+                        modified = true;
+                        nextItem = GetScreen()->GetDrawItems();
+                        break;
+                    }
+                }
+            }
+            // Remove duplicate junctions
+            else if( secondItem->GetPosition() == firstItem->GetPosition() )
+            {
+                itemList.PushItem( ITEM_PICKER( secondItem, UR_CHANGED ) );
+
+                // Keep the current flags, because the deleted segment can be flagged.
+                secondItem->SetFlags( firstItem->GetFlags() );
+
+                DeleteItem( firstItem, aAppend );
+                modified = true;
+                nextItem = GetScreen()->GetDrawItems();
+                break;
+            }
+        }
+    }
+
+    if( modified )
+        SaveCopyInUndoList( itemList, UR_CHANGED, aAppend );
+
+    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() )
     {
-        GetScreen()->Append( junction );
-        SaveCopyInUndoList( junction, UR_NEW );
-        OnModify();
+        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 )
+{
+    SCH_JUNCTION* junction = new SCH_JUNCTION( aPosition );
+    SCH_SCREEN* screen = GetScreen();
+    bool broken_segments = false;
+
+    screen->Append( junction );
+    broken_segments = BreakSegments( aPosition, aAppend );
+    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 );
 
-    SetRepeatItem( no_connect );
     GetScreen()->Append( no_connect );
-    GetScreen()->SchematicCleanUp();
+    GetScreen()->TestDanglingEnds();
     OnModify();
-    m_canvas->Refresh();
     SaveCopyInUndoList( no_connect, UR_NEW );
     return no_connect;
 }
diff --git a/eeschema/class_sch_screen.h b/eeschema/class_sch_screen.h
index a0e25cf73..af5d48291 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.
@@ -310,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 )
@@ -552,12 +526,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/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/hierarch.cpp b/eeschema/hierarch.cpp
index d66aa4ea9..8f1323b4f 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();
+        screen->TestDanglingEnds();
     }
     else
     {
diff --git a/eeschema/netlist.cpp b/eeschema/netlist.cpp
index 638978a88..ab10f9ef6 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;
 }
 
diff --git a/eeschema/onleftclick.cpp b/eeschema/onleftclick.cpp
index f93e0e80e..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.
@@ -120,7 +128,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 +145,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 +450,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/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/project_rescue.cpp b/eeschema/project_rescue.cpp
index c757cde90..d01e5454c 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( false );
     m_canvas->Refresh( true );
     OnModify();
 
diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index ab8b32d44..9aa6267c0 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 7b8e7c5d4..59cde72d9 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 63bea7eb6..0493c5ec6 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -333,15 +333,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 )
@@ -350,22 +354,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;
 }
 
@@ -440,56 +456,6 @@ bool SCH_SCREEN::IsTerminalPoint( const wxPoint& aPosition, int aLayer )
     return false;
 }
 
-
-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
@@ -951,64 +917,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() )
@@ -1153,7 +1061,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;
@@ -1427,18 +1334,6 @@ void SCH_SCREENS::ClearAnnotation()
 }
 
 
-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/schedit.cpp b/eeschema/schedit.cpp
index d6fdf0613..c5152ba3c 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:
@@ -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();
         }
@@ -233,6 +214,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
             break;
 
         DeleteItem( item );
+        SchematicCleanUp( true );
         screen->SetCurItem( NULL );
         SetRepeatItem( NULL );
         screen->TestDanglingEnds();
@@ -380,7 +362,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/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.cpp b/eeschema/schframe.cpp
index 7ec2bd11a..080d50c67 100644
--- a/eeschema/schframe.cpp
+++ b/eeschema/schframe.cpp
@@ -1226,7 +1226,7 @@ void SCH_EDIT_FRAME::OnOpenLibraryEditor( wxCommandEvent& event )
         }
     }
 
-    GetScreen()->SchematicCleanUp();
+    SchematicCleanUp( false );
     m_canvas->Refresh();
 }
 
diff --git a/eeschema/schframe.h b/eeschema/schframe.h
index 91066fc97..a16d86e4f 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
@@ -915,18 +943,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
@@ -980,7 +1007,7 @@ private:
      * Function EndSegment
      * called to terminate a bus, wire, or line creation
      */
-    void EndSegment( wxDC* DC );
+    void EndSegment();
 
     /**
      * Function DeleteCurrentSegment
@@ -1113,6 +1140,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