kicad-developers team mailing list archive

[Chris Pavlina <pavlina.chris@xxxxxxxxx>]

Wow. I made enough stupid yesterday for all of us, didn't I?

Here you go. I'll try to read over my code more carefully, today...


On Tue, Jun 23, 2015 at 04:04:25PM +0200, jp charras wrote:
> Le 23/06/2015 15:19, Chris Pavlina a écrit :
> > Sure, I can. I usually prefer to use dynamic_cast because it will fail 
> > if the subclass isn't correct (even if you don't test, since a NULL 
> > access will result in a segfault) - just more or less as a sanity check. 
> > There's really no circumstance under which the pointer should not be an 
> > instance of SCH_LINE, and the behavior when it fails with dynamic_cast 
> > is a bit more obvious.
> > 
> > I can throw an assert in there, though, that'd definitely be more 
> > correct. Modified patch attached.
> 
> Thanks.
> 
> But there is yet something strange for me:
> I am thinking data is used after deleting:
> delete s_wires.Remove( line ) delete the data referenced by line.
> then after p = line and last_line = line.
> but p->Next() and last_line->GetStartPoint() use this deleted data.
> 
> I can be wrong.
> 
> -- 
> Jean-Pierre CHARRAS

commit 8b36ca6c62fb2c1a125dc0ba25b82f42d3f7d1c9
Author: Chris Pavlina <cpavlin1@xxxxxxxxxxxxxx>
Date:   Tue Jun 23 10:22:00 2015 -0400

    Remove backtracking segments when drawing wires

diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index 62141bd..d0c6994 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -43,6 +43,7 @@
 #include <sch_text.h>
 #include <sch_component.h>
 #include <sch_sheet.h>
+#include <trigo.h>
 
 
 static void AbortCreateNewLine( EDA_DRAW_PANEL* aPanel, wxDC* aDC );
@@ -53,6 +54,53 @@ static DLIST< SCH_ITEM > s_wires;       // when creating a new set of wires,
 
 
 /**
+ * In a contiguous list of wires, remove wires that backtrack over the previous
+ * wire. Example:
+ *
+ * Wire is added:
+ * ---------------------------------------->
+ *
+ * A second wire backtracks over it:
+ * -------------------<====================>
+ *
+ * RemoveBacktracks is called:
+ * ------------------->
+ */
+static void RemoveBacktracks( DLIST<SCH_ITEM>& aWires )
+{
+    SCH_LINE* last_line = NULL;
+
+    EDA_ITEM* first = aWires.GetFirst();
+    for( EDA_ITEM* p = first; p; )
+    {
+        SCH_LINE *line = dynamic_cast<SCH_LINE*>( p );
+        if( !line )
+        {
+            wxFAIL_MSG( "RemoveBacktracks() requires SCH_LINE items" );
+            break;
+        }
+        p = line->Next();
+
+        if( last_line )
+        {
+            wxASSERT_MSG( last_line->GetEndPoint() == line->GetStartPoint(),
+                    "RemoveBacktracks() requires contiguous lines" );
+            if( IsPointOnSegment( last_line->GetStartPoint(), line->GetStartPoint(),
+                        line->GetEndPoint() ) )
+            {
+                last_line->SetEndPoint( line->GetEndPoint() );
+                delete s_wires.Remove( line );
+            }
+            else
+                last_line = line;
+        }
+        else
+            last_line = line;
+    }
+}
+
+
+/**
  * Mouse capture callback for drawing line segments.
  */
 static void DrawSegment( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wxPoint& aPosition,
@@ -261,6 +309,9 @@ void SCH_EDIT_FRAME::EndSegment( wxDC* DC )
 
     SaveCopyInUndoList( oldItems, UR_WIRE_IMAGE );
 
+    // Remove segments backtracking over others
+    RemoveBacktracks( s_wires );
+
     // Add the new wires
     screen->Append( s_wires );