← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Eeschema automatic manage junctions

 

Hi All-

Attached is an updated patchset for this feature, rebased to current master.

This should resolve the issues that Nick and Michael reported as well as
update to handle imported Eagle schematics.

Please let me know if there are any additional issues or suggestions for
improvement.

Thanks-
Seth

On Sat, Oct 28, 2017 at 7:32 PM, Seth Hillbrand <seth.hillbrand@xxxxxxxxx>
wrote:

> Hi Nick-
>
> I've tried this action ('g' to drag for me, 'd' is not mapped unless I
> missed a step) on all of schematics in my own library as well as the demos
> but can't get it to crash with the updated patch.
>
> Would you be able to send me a copy of your schematic?
>
> Thanks-
> Seth
>
> On Sat, Oct 28, 2017 at 4:29 PM, Nick Østergaard <oe.nick@xxxxxxxxx>
> wrote:
>
>> Thank you. But I still get the same assert.
>>
>> All I do it hit d on my cap and move the pin anchor to the middle of the
>> T junction where the one pin is on the same net.
>>
>> 2017-10-28 21:30 GMT+02:00 Seth Hillbrand <seth.hillbrand@xxxxxxxxx>:
>>
>>> 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 <20%2017%2010%2028> 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
>>>>>
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> 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 0359b88f2739f522a2292ecf3403df8008d01eb0 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 14:08:14 -0700
Subject: [PATCH 01/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 | 172 +++++++++++++------------------------------------------
 1 file changed, 39 insertions(+), 133 deletions(-)

diff --git a/common/trigo.cpp b/common/trigo.cpp
index 96dafa00e..5cf46d121 100644
--- a/common/trigo.cpp
+++ b/common/trigo.cpp
@@ -125,146 +125,52 @@ 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 87c2c2494a45782bdce0eac4306a9a8d5a2acb15 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 26 Oct 2017 09:53:17 -0700
Subject: [PATCH 02/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 1a52994f83fa4e743cc839f69d1d3c400983c1f1 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 15:07:55 -0700
Subject: [PATCH 03/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     | 82 +++++++++++++++++++++++++++------------------
 2 files changed, 56 insertions(+), 40 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..9dd297324 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -54,6 +54,7 @@
 #include <sch_text.h>
 #include <lib_pin.h>
 
+#include <boost/foreach.hpp>
 
 #define EESCHEMA_FILE_STAMP   "EESchema"
 
@@ -332,20 +333,59 @@ 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;
+    bool has_nonparallel = false;
+    int end_count = 0;
+
+    std::vector< SCH_LINE* > lines;
 
-    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( item->GetFlags() & STRUCT_DELETED )
+            continue;
+        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 ) )
+            lines.push_back( (SCH_LINE*) item );
+    }
+
+    BOOST_FOREACH( SCH_LINE* line, lines)
+    {
+        if( !line->IsEndPoint( aPosition ) )
+            has_line = true;
+        else
+            end_count++;
+        BOOST_REVERSE_FOREACH( SCH_LINE* second_line, lines )
+        {
+            if( line == second_line )
+                break;
+            if( line->IsEndPoint( second_line->GetStartPoint() )
+                    && line->IsEndPoint( second_line->GetEndPoint() ) )
+                end_count--;
+            if( !line->IsParallel( second_line ) )
+                has_nonparallel = true;
+        }
     }
 
+    int has_pin = !!( GetPin( aPosition, NULL, true ) );
+
+    // If there is line intersecting a pin or non-parallel end
+    if( has_pin && ( has_line || end_count > 1 ) )
+        return true;
+    // If there is at least one segment that ends on a non-parallel line or
+    // junction of two other lines
+    if( has_nonparallel && (has_line || end_count > 2 ) )
+        return true;
+
     return false;
 }
 
@@ -1243,30 +1283,6 @@ int SCH_SCREEN::GetConnection( const wxPoint& aPosition, PICKED_ITEMS_LIST& aLis
             }
         }
 
-        // Get redundant junctions (junctions which connect < 3 end wires
-        // and no pin)
-        for( item = m_drawList.begin(); item; item = item->Next() )
-        {
-            if( item->GetFlags() & STRUCT_DELETED )
-                continue;
-
-            if( !(item->GetFlags() & CANDIDATE) )
-                continue;
-
-            if( item->Type() != SCH_JUNCTION_T )
-                continue;
-
-            SCH_JUNCTION* junction = (SCH_JUNCTION*) item;
-
-            if( CountConnectedItems( junction->GetPosition(), false ) <= 2 )
-            {
-                item->SetFlags( STRUCT_DELETED );
-
-                ITEM_PICKER picker( item, UR_DELETED );
-                aList.PushItem( picker );
-            }
-        }
-
         for( item = m_drawList.begin(); item;  item = item->Next() )
         {
             if( item->GetFlags() & STRUCT_DELETED )
@@ -1277,7 +1293,7 @@ int SCH_SCREEN::GetConnection( const wxPoint& aPosition, PICKED_ITEMS_LIST& aLis
 
             tmp = GetWireOrBus( ( (SCH_TEXT*) item )->GetPosition() );
 
-            if( tmp && tmp->GetFlags() & STRUCT_DELETED )
+            if( tmp && ( tmp->GetFlags() & STRUCT_DELETED ) )
             {
                 item->SetFlags( STRUCT_DELETED );
 
-- 
2.11.0

From 655b29c9dca1274cb5dd987f305a0b1f3f549bd1 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 16:03:12 -0700
Subject: [PATCH 04/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 | 47 +++++++++++++++++++++++++++++++++++-----
 eeschema/schframe.h              | 23 +++++++++++++++++++-
 4 files changed, 68 insertions(+), 12 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..6279127ea 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,14 @@ 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 +169,41 @@ 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;
+
+    if( !aItemsList.GetCount() )
+        return;
+
+    // Can't append a WIRE IMAGE, so fail to a new undo point
+    if( aAppend && ( aTypeCommand != UR_WIRE_IMAGE ) )
+    {
+        commandToUndo = GetScreen()->PopCommandFromUndoList();
+        if( commandToUndo && commandToUndo->m_Status == UR_WIRE_IMAGE )
+        {
+            GetScreen()->PushCommandToUndoList( commandToUndo );
+            commandToUndo = NULL;
+        }
+    }
 
-    commandToUndo->m_TransformPoint = aTransformPoint;
-    commandToUndo->m_Status = aTypeCommand;
+    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..317b9c297 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -942,7 +942,23 @@ 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 +1116,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 +1210,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 +1225,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 c20574d135d8c7dfa704cfccdd7a56897cad6a02 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 12 Oct 2017 16:41:25 -0700
Subject: [PATCH 05/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 | 56 ++++++++++++++++++++++------------
 eeschema/protos.h                      |  3 --
 eeschema/schedit.cpp                   |  2 +-
 eeschema/schframe.h                    | 11 ++++++-
 5 files changed, 49 insertions(+), 27 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..b00f71185 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,46 +135,64 @@ 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 );
+        aAppend = true;
     }
 
-    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." ) );
+    wxCHECK_RET( !( aItem->GetFlags() & STRUCT_DELETED ),
+                 wxT( "Cannot delete item that is already deleted." ) );
 
     // Here, aItem is not null.
-
     SCH_SCREEN* screen = GetScreen();
 
     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() && aItem->Type() != SCH_JUNCTION_T )
+        {
+            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 317b9c297..f0840c930 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -1118,7 +1118,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 41e1555f6810c026e21e98aa42b5fa5377f90522 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 13 Oct 2017 15:08:33 -0700
Subject: [PATCH 06/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 21b6fe8fd..d591f0d37 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -567,6 +567,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 f9e75c6c164cea1ae52d3fbf843dc4dd07db8dcc Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Mon, 16 Oct 2017 14:38:57 -0700
Subject: [PATCH 07/13] Eeschema: Add two utility functions to sch_line

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

diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index d591f0d37..c5f815dda 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -266,6 +266,40 @@ 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 if( m_end == aPosition )
+        first = m_start - aPosition;
+    else
+        return false;
+
+    if( aLine->m_start == aPosition )
+        second = aLine->m_end - aPosition;
+    else if( aLine->m_end == aPosition )
+        second = aLine->m_start - aPosition;
+    else
+        return false;
+
+    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 9aab127ebaaa199ea0d17e6995e54eaa1cb204e3 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 20 Oct 2017 09:31:02 -0700
Subject: [PATCH 08/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 a5f30389137c96ade04469ecf753f88390bf3c7d Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Tue, 31 Oct 2017 12:33:00 -0700
Subject: [PATCH 09/13] Eeschema: Remove zero-length wires in RemoveBacktracks

---
 eeschema/bus-wire-junction.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index 528d09d68..fa0175463 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -74,6 +74,12 @@ static void RemoveBacktracks( DLIST<SCH_ITEM>& aWires )
         SCH_LINE *line = static_cast<SCH_LINE*>( p );
         p = line->Next();
 
+        if( line->IsNull() )
+        {
+            delete s_wires.Remove( line );
+            continue;
+        }
+
         if( !last_lines.empty() )
         {
             SCH_LINE* last_line = last_lines[last_lines.size() - 1];
-- 
2.11.0

From 614f4cc0115fbaabf9fa57dd6ed94280dce24d1a 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 fa0175463..410f830f6 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -330,7 +330,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 ) )
@@ -417,6 +417,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 )
 {
@@ -445,7 +494,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 9dd297324..68cd0167d 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -461,55 +461,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
@@ -1422,19 +1373,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 f0840c930..d5ca03f92 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -954,11 +954,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 9bc56e1e38b3f9033d117fd68d8f1f604ee4c235 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 410f830f6..1341eca02 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -248,7 +248,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;
         }
 
@@ -267,7 +267,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();
@@ -334,11 +334,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();
 
@@ -466,29 +466,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 ed349001b..2cfc9e045 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 d5ca03f92..35408a78e 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -932,17 +932,16 @@ 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
@@ -995,7 +994,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 845d1fcab0c941341eba182f61dfd718e31abe6c 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 1341eca02..aa6f029ad 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -416,6 +416,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()
 {
@@ -465,6 +488,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 )
@@ -474,7 +558,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 68cd0167d..d5d28aef6 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -922,64 +922,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() )
@@ -1124,7 +1066,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 35408a78e..9227713ac 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 5699311375d164052254375162bd75e7ae533552 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Thu, 2 Nov 2017 14:12:31 -0700
Subject: [PATCH 13/13] Eeschema: Automatically manage junctions

Allows eeschema to automatically add and remove
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                     |  13 ++-
 eeschema/bus-wire-junction.cpp         | 202 +++++++++++++++++++--------------
 eeschema/files-io.cpp                  |  10 ++
 eeschema/hierarch.cpp                  |   8 +-
 eeschema/operations_on_items_lists.cpp |  35 ++++++
 eeschema/project_rescue.cpp            |   1 +
 eeschema/sch_line.cpp                  | 115 ++++++++++---------
 eeschema/sch_line.h                    |   6 +-
 eeschema/schedit.cpp                   |   3 +
 eeschema/schframe.cpp                  |   9 ++
 eeschema/schframe.h                    |  18 ++-
 11 files changed, 275 insertions(+), 145 deletions(-)

diff --git a/eeschema/block.cpp b/eeschema/block.cpp
index efffdef5c..2ac4e4285 100644
--- a/eeschema/block.cpp
+++ b/eeschema/block.cpp
@@ -139,7 +139,6 @@ void SCH_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
 
         SaveCopyInUndoList( block->GetItems(), UR_MOVED, false, block->GetMoveVector() );
         MoveItemsInList( block->GetItems(), block->GetMoveVector() );
-        block->ClearItemsList();
         break;
 
     case BLOCK_DUPLICATE:           /* Duplicate */
@@ -151,8 +150,6 @@ void SCH_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
 
         SaveCopyInUndoList( block->GetItems(),
                             ( block->GetCommand() == BLOCK_PRESELECT_MOVE ) ? UR_CHANGED : UR_NEW );
-
-        block->ClearItemsList();
         break;
 
     case BLOCK_PASTE:
@@ -160,13 +157,15 @@ void SCH_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
             m_canvas->CallMouseCapture( DC, wxDefaultPosition, false );
 
         PasteListOfItems( DC );
-        block->ClearItemsList();
         break;
 
     default:        // others are handled by HandleBlockEnd()
        break;
     }
 
+    CheckJunctionsInList( block->GetItems(), true );
+    block->ClearItemsList();
+    SchematicCleanUp( true );
     OnModify();
 
     // clear dome flags and pointers
@@ -228,6 +227,8 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
                 SetCrossHairPosition( rotationPoint );
                 SaveCopyInUndoList( block->GetItems(), UR_ROTATED, false, rotationPoint );
                 RotateListOfItems( block->GetItems(), rotationPoint );
+                CheckJunctionsInList( block->GetItems(), true );
+                SchematicCleanUp( true );
                 OnModify();
             }
 
@@ -280,6 +281,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
             if( block->GetCount() )
             {
                 DeleteItemsInList( block->GetItems() );
+                SchematicCleanUp( true );
                 OnModify();
             }
             block->ClearItemsList();
@@ -311,6 +313,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 +342,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 +362,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();
             }
 
diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index aa6f029ad..9fb9f8831 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -266,82 +266,76 @@ void SCH_EDIT_FRAME::BeginSegment( wxDC* DC, int type )
     }
 }
 
+void SCH_EDIT_FRAME::GetSchematicConnections( std::vector< wxPoint >& aConnections )
+{
+    for( SCH_ITEM* item = GetScreen()->GetDrawItems(); item; item = item->Next() )
+        item->GetConnectionPoints( aConnections );
+
+    // We always have some overlapping connection points.  Drop duplicates here
+    std::sort( aConnections.begin(), aConnections.end(),
+            []( wxPoint& a, wxPoint& b ) -> bool
+            { return a.x < b.x || (a.x == b.x && a.y < b.y); } );
+    aConnections.erase( unique( aConnections.begin(), aConnections.end() ), aConnections.end() );
+}
 
 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() );
+    // Collect the possible connection points for the new lines
+    std::vector< wxPoint > connections;
+    std::vector< wxPoint > new_ends;
 
-    screen->SetCurItem( NULL );
-    m_canvas->EndMouseCapture( -1, -1, wxEmptyString, false );
-
-    // 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();
-
-    // 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();
+    GetSchematicConnections( connections );
+    // 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 ) );
-
-    m_canvas->Refresh();
+    SchematicCleanUp( true );
+    for( auto i : new_ends )
+    {
+        if( screen->IsJunctionNeeded( i, true ) )
+            AddJunction( i, true );
+    }
 
+    screen->TestDanglingEnds();
+    screen->ClearDrawingState();
+    screen->SetCurItem( NULL );
+    m_canvas->EndMouseCapture( -1, -1, wxEmptyString, false );
     OnModify();
 }
 
@@ -439,53 +433,95 @@ void SCH_EDIT_FRAME::SaveWireImage()
     }
 }
 
-
-bool SCH_EDIT_FRAME::SchematicCleanUp()
+bool SCH_EDIT_FRAME::SchematicCleanUp( bool aAppend )
 {
-    bool      modified = false;
+    SCH_ITEM*           item = NULL;
+    SCH_ITEM*           secondItem = NULL;
+    PICKED_ITEMS_LIST   itemList;
+    SCH_SCREEN*         screen = GetScreen();
+
+    auto remove_item = [ &itemList, screen ]( SCH_ITEM* aItem ) -> void
+    {
+        aItem->SetFlags( STRUCT_DELETED );
+        itemList.PushItem( ITEM_PICKER( aItem, UR_DELETED ) );
+    };
 
-    for( SCH_ITEM* item = GetScreen()->GetDrawItems() ; item; item = item->Next() )
+    for( item = screen->GetDrawItems(); item; item = item->Next() )
     {
-        if( ( item->Type() != SCH_LINE_T ) && ( item->Type() != SCH_JUNCTION_T ) )
+        if( ( item->Type() != SCH_LINE_T ) &&
+            ( item->Type() != SCH_JUNCTION_T ) &&
+            ( item->Type() != SCH_NO_CONNECT_T ))
             continue;
 
-        bool restart;
+        if( item->GetFlags() & STRUCT_DELETED )
+            continue;
 
-        for( SCH_ITEM* testItem = item->Next(); testItem; testItem = restart ? GetScreen()->GetDrawItems() : testItem->Next() )
+        // Remove unneeded junctions
+        if( ( item->Type() == SCH_JUNCTION_T )
+                && ( !screen->IsJunctionNeeded( item->GetPosition() ) ) )
+        {
+            remove_item( item );
+            continue;
+        }
+        // Remove zero-length lines
+        if( item->Type() == SCH_LINE_T
+                && ( (SCH_LINE*) item )->IsNull() )
         {
-            restart = false;
+            remove_item( item );
+            continue;
+        }
 
-            if( ( item->Type() == SCH_LINE_T ) && ( testItem->Type() == SCH_LINE_T ) )
-            {
-                SCH_LINE* line = (SCH_LINE*) item;
+        for( secondItem = item->Next(); secondItem; secondItem = secondItem->Next() )
+        {
+            if( item->Type() != secondItem->Type() || ( secondItem->GetFlags() & STRUCT_DELETED ) )
+                continue;
 
-                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 ) )
+            // Merge overlapping lines
+            if( item->Type() == SCH_LINE_T )
             {
-                if ( testItem->HitTest( item->GetPosition() ) )
+                SCH_LINE* firstLine = (SCH_LINE*) item;
+                SCH_LINE* secondLine = (SCH_LINE*) secondItem;
+                SCH_LINE* line = NULL;
+                bool needed = false;
+
+                if( !secondLine->IsParallel( firstLine ) )
+                    continue;
+
+                // Check if a junction needs to be kept
+                // This can only happen if:
+                //   1) the endpoints overlap,
+                //   2) the lines are not pointing in the same direction AND
+                //   3) IsJunction Needed is false
+                if( secondLine->IsEndPoint( firstLine->GetStartPoint() )
+                        && !secondLine->IsSameQuadrant( firstLine, firstLine->GetStartPoint() ) )
+                    needed = screen->IsJunctionNeeded( firstLine->GetStartPoint() );
+                else if( secondLine->IsEndPoint( firstLine->GetEndPoint() )
+                        && !secondLine->IsSameQuadrant( firstLine, firstLine->GetEndPoint() ) )
+                    needed = screen->IsJunctionNeeded( firstLine->GetEndPoint() );
+
+                if( !needed && ( 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;
-                    modified = true;
+                    remove_item( item );
+                    remove_item( secondItem );
+                    itemList.PushItem( ITEM_PICKER( line, UR_NEW ) );
+                    screen->Append( (SCH_ITEM*) line );
+                    break;
                 }
             }
+            // Remove duplicate junctions and no-connects
+            else if( secondItem->GetPosition() == item->GetPosition() )
+                remove_item( secondItem );
         }
     }
+    for( item = screen->GetDrawItems(); item; item = secondItem )
+    {
+        secondItem = item->Next();
+        if( item->GetFlags() & STRUCT_DELETED )
+            screen->Remove( item );
+    }
+    SaveCopyInUndoList( itemList, UR_CHANGED, aAppend );
 
-    GetScreen()->TestDanglingEnds();
-
-    return modified;
+    return !!( itemList.GetCount() );
 }
 
 bool SCH_EDIT_FRAME::BreakSegment( SCH_LINE *aSegment, const wxPoint& aPoint, bool aAppend )
diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp
index f38dee5cb..0a7da72e3 100644
--- a/eeschema/files-io.cpp
+++ b/eeschema/files-io.cpp
@@ -344,6 +344,11 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector<wxString>& aFileSet, in
         SCH_SCREENS schematic;
 
         schematic.UpdateSymbolLinks();      // Update all symbol library links for all sheets.
+
+        // Ensure the schematic is fully segmented on first display
+        BreakSegmentsOnJunctions();
+        SchematicCleanUp( true );
+        GetScreen()->ClearUndoORRedoList( GetScreen()->m_UndoList, 1 );
         GetScreen()->TestDanglingEnds();    // Only perform the dangling end test on root sheet.
     }
 
@@ -643,6 +648,11 @@ bool SCH_EDIT_FRAME::ImportFile( const wxString& aFileName, int aFileType )
 
                 UpdateFileHistory( fullFileName );
                 schematic.UpdateSymbolLinks();      // Update all symbol library links for all sheets.
+
+                // Ensure the schematic is fully segmented on first display
+                BreakSegmentsOnJunctions();
+                SchematicCleanUp( true );
+                GetScreen()->ClearUndoORRedoList( GetScreen()->m_UndoList, 1 );
                 GetScreen()->TestDanglingEnds();    // Only perform the dangling end test on root sheet.
 
                 GetScreen()->SetGrid( ID_POPUP_GRID_LEVEL_1000 + m_LastGridSizeId );
diff --git a/eeschema/hierarch.cpp b/eeschema/hierarch.cpp
index c41c82164..c65f388c4 100644
--- a/eeschema/hierarch.cpp
+++ b/eeschema/hierarch.cpp
@@ -299,7 +299,13 @@ void SCH_EDIT_FRAME::DisplayCurrentSheet()
         screen->m_FirstRedraw = false;
         SetCrossHairPosition( GetScrollCenterPosition() );
         m_canvas->MoveCursorToCrossHair();
-        SchematicCleanUp();
+
+        // Ensure the schematic is fully segmented on first display
+        BreakSegmentsOnJunctions();
+        SchematicCleanUp( true );
+        screen->ClearUndoORRedoList( screen->m_UndoList, 1 );
+
+        screen->TestDanglingEnds();
     }
     else
     {
diff --git a/eeschema/operations_on_items_lists.cpp b/eeschema/operations_on_items_lists.cpp
index b00f71185..f6eb03a42 100644
--- a/eeschema/operations_on_items_lists.cpp
+++ b/eeschema/operations_on_items_lists.cpp
@@ -121,6 +121,41 @@ void MoveItemsInList( PICKED_ITEMS_LIST& aItemsList, const wxPoint aMoveVector )
 }
 
 
+void SCH_EDIT_FRAME::CheckJunctionsInList( PICKED_ITEMS_LIST& aItemsList, bool aAppend )
+{
+    std::vector< wxPoint > pts;
+    std::vector< wxPoint > connections;
+
+    GetSchematicConnections( connections );
+    for( unsigned ii = 0; ii < aItemsList.GetCount(); ii++ )
+    {
+        SCH_ITEM* item = (SCH_ITEM*) aItemsList.GetPickedItem( ii );
+        item->GetConnectionPoints( pts );
+        if( item->Type() == SCH_LINE_T )
+        {
+            SCH_LINE* line = (SCH_LINE*) item;
+            for( auto i : connections )
+                if( IsPointOnSegment( line->GetStartPoint(), line->GetEndPoint(), i ) )
+                    pts.push_back( i );
+        }
+
+    }
+    // We always have some overlapping connection points.  Drop duplicates here
+    std::sort( pts.begin(), pts.end(),
+            []( wxPoint& a, wxPoint& b ) -> bool
+            { return a.x < b.x || (a.x == b.x && a.y < b.y); } );
+    pts.erase( unique( pts.begin(), pts.end() ), pts.end() );
+
+    for( auto point : pts )
+    {
+        if( GetScreen()->IsJunctionNeeded( point, true ) )
+        {
+            AddJunction( point, aAppend );
+            aAppend = true;
+        }
+    }
+}
+
 /**
  * Function DeleteItemsInList
  * delete schematic items in aItemsList
diff --git a/eeschema/project_rescue.cpp b/eeschema/project_rescue.cpp
index 9ad8a2dbb..ec9b5b4a1 100644
--- a/eeschema/project_rescue.cpp
+++ b/eeschema/project_rescue.cpp
@@ -615,6 +615,7 @@ bool SCH_EDIT_FRAME::RescueProject( bool aRunningOnDemand )
 
     // Clean up wire ends
     SchematicCleanUp();
+    GetScreen()->ClearUndoORRedoList( GetScreen()->m_UndoList, 1 );
     m_canvas->Refresh( true );
     OnModify();
 
diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index c5f815dda..ae7362e0b 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -301,87 +301,96 @@ bool SCH_LINE::IsParallel( SCH_LINE* aLine )
     return !( (long long) firstSeg.x * secondSeg.y - (long long) firstSeg.y * secondSeg.x );
 }
 
+static inline bool operator<( const wxPoint& lhs, const wxPoint& rhs )
+{
+    if( lhs.x == rhs.x )
+        return lhs.y < rhs.y;
+    return lhs.x < rhs.x;
+};
+
 /*
  * 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 )
 {
-    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;
 
-    // Search for a common end:
-    if( m_start == aLine->m_start )
-    {
-        if( m_end == aLine->m_end )     // Trivial case
-            return true;
-    }
-    else if( m_start == aLine->m_end )
-    {
-        if( m_end == aLine->m_start )     // Trivial case
-            return true;
-    }
-    else if( m_end == aLine->m_end )
+    SCH_LINE leftmost = SCH_LINE( *aLine );
+    SCH_LINE rightmost = SCH_LINE( *this );
+
+    // We place the start to the left and below the end of both lines
+    if( leftmost.m_start != std::min( { leftmost.m_start, leftmost.m_end } ) )
+        std::swap( leftmost.m_start, leftmost.m_end );
+    if( rightmost.m_start != std::min( { rightmost.m_start, rightmost.m_end } ) )
+        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( rightmost.m_start < leftmost.m_start )
+        std::swap( leftmost, rightmost );
+
+    SCH_LINE other = SCH_LINE( rightmost );
+
+    if( rightmost.m_end < leftmost.m_end )
+        rightmost = leftmost;
+
+    // If we end one before the beginning of the other, no overlap is possible
+    if( leftmost.m_end < other.m_start )
     {
-        std::swap( aLine->m_start, aLine->m_end );
+        return NULL;
     }
-    else if( m_end != aLine->m_start )
+
+    // Search for a common end:
+    if( ( leftmost.m_start == other.m_start )
+            && ( leftmost.m_end == other.m_end ) )     // Trivial case
     {
-        // No common end point, segments cannot be merged.
-        return false;
+        return new SCH_LINE( leftmost );
     }
 
     bool colinear = false;
 
     /* Test alignment: */
-    if( m_start.y == m_end.y )       // Horizontal segment
+    if( ( leftmost.m_start.y == leftmost.m_end.y )
+            && ( other.m_start.y == other.m_end.y ) )       // Horizontal segment
     {
-        if( aLine->m_start.y == aLine->m_end.y )
-        {
-            colinear = true;
-        }
+        colinear = ( leftmost.m_start.y == other.m_start.y );
     }
-    else if( m_start.x == m_end.x )  // Vertical segment
+    else if( ( leftmost.m_start.x == leftmost.m_end.x )
+            && ( other.m_start.x == other.m_end.x ) )  // Vertical segment
     {
-        if( aLine->m_start.x == aLine->m_end.x )
-        {
-            colinear = true;
-        }
+        colinear = ( leftmost.m_start.x == other.m_start.x );
     }
     else
     {
-        if( atan2( (double) ( m_start.x - m_end.x ), (double) ( m_start.y - m_end.y ) )
-            == atan2( (double) ( aLine->m_start.x - aLine->m_end.x ),
-                      (double) ( aLine->m_start.y - aLine->m_end.y ) ) )
-        {
-            colinear = true;
-        }
+        // We use long long here to avoid overflow -- it enforces promotion
+        // Don't use double as we need to make a direct comparison
+        // The slope of the left-most line is dy/dx.  Then we check that the slope
+        // from the left most start to the right most start is the same as well as
+        // the slope from the left most start to right most end.
+        long long dx = leftmost.m_end.x - leftmost.m_start.x;
+        long long dy = leftmost.m_end.y - leftmost.m_start.y;
+        colinear = ( ( ( other.m_start.y - leftmost.m_start.y ) * dx ==
+                ( other.m_start.x - leftmost.m_start.x ) * dy ) &&
+            ( ( other.m_end.y - leftmost.m_start.y ) * dx ==
+                ( other.m_end.x - leftmost.m_start.x ) * dy ) );
     }
 
-    // Make a segment which merge the 2 segments
-    // we must find the extremums
-    // i.e. the more to the left and to the right points, or
-    // for horizontal segments the uppermost and the lowest point
+    // Make a new segment that merges the 2 segments
     if( colinear )
     {
-        auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
-        {
-            if( lhs.x == rhs.x )
-                return lhs.y < rhs.y;
-            return lhs.x < rhs.x;
-        };
-
-        wxPoint top_left = std::min( { m_start, m_end, aLine->m_start, aLine->m_end }, less );
-        wxPoint bottom_right = std::max( { m_start, m_end, aLine->m_start, aLine->m_end }, less );
-
-        m_start = top_left;
-        m_end = bottom_right;
-        return true;
+        leftmost.m_end = rightmost.m_end;
+        return new SCH_LINE( leftmost );
     }
-    return false;
+
+    return NULL;
 }
 
 
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/schedit.cpp b/eeschema/schedit.cpp
index 232ddf6a0..de3f301fa 100644
--- a/eeschema/schedit.cpp
+++ b/eeschema/schedit.cpp
@@ -190,6 +190,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
     case ID_POPUP_SCH_DELETE_CONNECTION:
         m_canvas->MoveCursorToCrossHair();
         DeleteConnection( id == ID_POPUP_SCH_DELETE_CONNECTION );
+        SchematicCleanUp( true );
         screen->SetCurItem( NULL );
         SetRepeatItem( NULL );
 
@@ -214,6 +215,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
             break;
 
         DeleteItem( item );
+        SchematicCleanUp( true );
         screen->SetCurItem( NULL );
         SetRepeatItem( NULL );
         screen->TestDanglingEnds();
@@ -630,6 +632,7 @@ void SCH_EDIT_FRAME::DeleteConnection( bool aFullConnection )
     if( screen->GetConnection( pos, pickList, aFullConnection ) != 0 )
     {
         DeleteItemsInList( pickList );
+        SchematicCleanUp( true );
         OnModify();
     }
 }
diff --git a/eeschema/schframe.cpp b/eeschema/schframe.cpp
index 12bce0e0e..b3d24e764 100644
--- a/eeschema/schframe.cpp
+++ b/eeschema/schframe.cpp
@@ -1394,7 +1394,16 @@ void SCH_EDIT_FRAME::addCurrentItemToList( bool aRedraw )
     m_canvas->EndMouseCapture();
 
     if( item->IsConnectable() )
+    {
+        std::vector< wxPoint > pts;
+        item->GetConnectionPoints( pts );
+        for( auto i : pts )
+        {
+            if( screen->IsJunctionNeeded( i, true ) )
+                AddJunction( i, true );
+        }
         screen->TestDanglingEnds();
+    }
 
     if( aRedraw )
         GetCanvas()->Refresh();
diff --git a/eeschema/schframe.h b/eeschema/schframe.h
index 9227713ac..ca13054df 100644
--- a/eeschema/schframe.h
+++ b/eeschema/schframe.h
@@ -978,13 +978,20 @@ private:
     void SaveWireImage();
 
     /**
+     * Function GetSchematicConnections
+     * Collects a unique list of all possible connection points in the schematic
+     */
+    void GetSchematicConnections( std::vector< wxPoint >& aConnections );
+
+    /**
      * 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
@@ -1155,6 +1162,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