← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Clean up unused junctions

 

Hi Jon-

Thanks for catching that.  The delete node/delete connection follow a
different path to deletion, so I missed them initially.  As I was doing
this, I noticed a couple of corner cases where junctions were not properly
added/deleted, so this fixes that issue as well.

There is one remaining issue, which is that junctions along the wire are
not deleted if they are no longer required.  But I'm holding off on adding
this because I don't think that we want to keep the wire crossing multiple
junctions and I'll try to lay out that proposal at a later date.

Best-
Seth

On Tue, Sep 26, 2017 at 5:58 PM, Jon Evans <jon@xxxxxxxxxxxxx> wrote:

> Hi Seth,
>
> Neat feature!  I just checked this out and it does seem to work in most
> cases.
> There seem to be some situations where it doesn't remove junctions in my
> existing schematics -- when I use "delete connection" it works, but when I
> use "delete wire" it only works some of the time.  I haven't looked closely
> at the code to see why this might be happening.
>
> -Jon
>
> On Tue, Sep 26, 2017 at 7:39 PM, Seth Hillbrand <seth.hillbrand@xxxxxxxxx>
> wrote:
>
>> ​Hi All-
>>
>> Currently, when laying wire in eeschema, junctions are automatically
>> added​ where needed.  This patch provides the reverse.  When deleting
>> segments in eeschema, it automatically removes the junction if it is no
>> longer needed.
>>
>> This should save extra clicks when re-wiring and hopefully help new Kicad
>> users keep their schematics cleaner.
>>
>> Best-
>> Seth
>>
>> _______________________________________________
>> 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 c864fae9824269d64e34c6fea548921d6b09de21 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Tue, 26 Sep 2017 16:17:14 -0700
Subject: [PATCH 1/1] Eeschema: Automatically remove dangling junctions

---
 eeschema/bus-wire-junction.cpp         |  6 ++--
 eeschema/operations_on_items_lists.cpp | 57 ++++++++++++++++++++++++++++++++--
 eeschema/sch_screen.cpp                | 30 +++++++++++++-----
 3 files changed, 81 insertions(+), 12 deletions(-)

diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index 528d09d68..41812bdab 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -327,11 +327,13 @@ void SCH_EDIT_FRAME::EndSegment( wxDC* DC )
     screen->SchematicCleanUp();
 
     // A junction could be needed to connect the end point of the last created segment.
-    if( screen->IsJunctionNeeded( endpoint ) )
+    if( screen->IsJunctionNeeded( endpoint ) &&
+            !( screen->GetItem( endpoint, 0, SCH_JUNCTION_T ) ) )
         screen->Append( AddJunction( DC, endpoint ) );
 
     // A junction could be needed to connect the start point of the set of new created wires
-    if( screen->IsJunctionNeeded( startPoint ) )
+    if( screen->IsJunctionNeeded( startPoint ) &&
+            !( screen->GetItem( startPoint, 0, SCH_JUNCTION_T ) ) )
         screen->Append( AddJunction( DC, startPoint ) );
 
     m_canvas->Refresh();
diff --git a/eeschema/operations_on_items_lists.cpp b/eeschema/operations_on_items_lists.cpp
index 6f0b790c2..596e16e29 100644
--- a/eeschema/operations_on_items_lists.cpp
+++ b/eeschema/operations_on_items_lists.cpp
@@ -137,6 +137,9 @@ 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->GetFlags() & STRUCT_DELETED )
+            continue;
+
         if( item->Type() == SCH_SHEET_PIN_T )
         {
             /* this item is depending on a sheet, and is not in global list */
@@ -145,12 +148,37 @@ void DeleteItemsInList( EDA_DRAW_PANEL* panel, PICKED_ITEMS_LIST& aItemsList )
         else
         {
             screen->Remove( item );
-
             /* Unlink the structure */
             itemsList.PushItem( itemWrapper );
+            item->SetFlags( STRUCT_DELETED );
+
+            if( item->Type() == SCH_LINE_T )
+            {
+                wxPoint endpoint = ( (SCH_LINE*) item )->GetEndPoint();
+                wxPoint startpoint = ( (SCH_LINE*) item )->GetStartPoint();
+
+                // Clean out junctions that are no longer needed
+                for( auto&& point : { startpoint, endpoint } )
+                {
+                    SCH_ITEM* junction;
+
+                    if( !screen->IsJunctionNeeded( point ) &&
+                        ( junction = screen->GetItem( point, 0, SCH_JUNCTION_T ) ) &&
+                        !( junction->GetFlags() & STRUCT_DELETED ) )
+                    {
+                        junction->SetFlags( STRUCT_DELETED );
+
+                        ITEM_PICKER picker( junction, UR_DELETED );
+
+                        itemsList.PushItem( picker );
+                        screen->Remove( junction );
+                    }
+                }
+            }
         }
     }
 
+    screen->ClearDrawingState();
     frame->SaveCopyInUndoList( itemsList, UR_DELETED );
 }
 
@@ -175,8 +203,33 @@ void SCH_EDIT_FRAME::DeleteItem( SCH_ITEM* aItem )
     }
     else
     {
+        PICKED_ITEMS_LIST itemsList;
+
         screen->Remove( aItem );
-        SaveCopyInUndoList( aItem, UR_DELETED );
+        itemsList.PushItem(aItem);
+
+        if ( aItem->Type() == SCH_LINE_T )
+        {
+            wxPoint endpoint = ( (SCH_LINE*) aItem )->GetEndPoint();
+            wxPoint startpoint = ( (SCH_LINE*) aItem )->GetStartPoint();
+
+            // Clean out junctions that are no longer needed
+            for (auto&& point : { startpoint, endpoint } )
+            {
+                SCH_ITEM* junction;
+
+                if( !screen->IsJunctionNeeded( point ) &&
+                    ( junction = screen->GetItem( point, 0, SCH_JUNCTION_T ) ) )
+                {
+
+                    ITEM_PICKER picker( junction, UR_DELETED );
+
+                    itemsList.PushItem( picker );
+                    screen->Remove( junction );
+                }
+            }
+        }
+        SaveCopyInUndoList( itemsList, UR_DELETED );
         m_canvas->RefreshDrawingRect( aItem->GetBoundingBox() );
     }
 }
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index 02f2030d1..0941808b1 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -336,18 +336,32 @@ void SCH_SCREEN::MarkConnections( SCH_LINE* aSegment )
 
 bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition )
 {
-    if( GetItem( aPosition, 0, SCH_JUNCTION_T ) )
-        return false;
+    int line_count = 0;
+    int end_count = 0;
 
-    if( GetWire( aPosition, 0, EXCLUDE_END_POINTS_T ) )
+    for( SCH_ITEM* item = m_drawList.begin(); item; item = item->Next() )
     {
-        if( GetWire( aPosition, 0, END_POINTS_ONLY_T ) )
-            return true;
+        if( item->Type() != SCH_LINE_T )
+            continue;
 
-        if( GetPin( aPosition, NULL, true ) )
-            return true;
+        if( item->GetLayer() != LAYER_WIRE )
+            continue;
+
+        if( !item->HitTest( aPosition, 0 ) )
+            continue;
+
+        if( ( (SCH_LINE*) item )->IsEndPoint( aPosition ) )
+            end_count++;
+        else
+            line_count++;
     }
 
+    if( end_count > 2 )
+        return true;
+
+    if( line_count && ( end_count || GetPin( aPosition, NULL, true ) ) )
+        return true;
+
     return false;
 }
 
@@ -1260,7 +1274,7 @@ int SCH_SCREEN::GetConnection( const wxPoint& aPosition, PICKED_ITEMS_LIST& aLis
 
             SCH_JUNCTION* junction = (SCH_JUNCTION*) item;
 
-            if( CountConnectedItems( junction->GetPosition(), false ) <= 2 )
+            if( !IsJunctionNeeded( junction->GetPosition() ) )
             {
                 item->SetFlags( STRUCT_DELETED );
 
-- 
2.11.0


Follow ups

References