[Kicad-developers] [PATCH] Remove backtracking wires in eeschema
If you place a wire segment and then backtrack over it, you end up with a broken wire in the end: http://misc.c4757p.com/backtrack.mp4 This is because the two segments are merged together, rather than the second *subtracting* from the first, even though the latter case is the way it's shown as you're drawing. This patch fixes that by adding a function to erase backtracks. -- Chris commit d18fec08e34f7b509553fc72ee0616eeda6579a6 Author: Chris Pavlina Date: Mon Jun 22 16:16:50 2015 -0400 Remove backtracking segments when drawing wires diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp index a33f7b5..eb86ba5 100644 --- a/eeschema/bus-wire-junction.cpp +++ b/eeschema/bus-wire-junction.cpp @@ -43,6 +43,7 @@ #include #include #include +#include static void AbortCreateNewLine( EDA_DRAW_PANEL* aPanel, wxDC* aDC ); @@ -53,6 +54,45 @@ 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& aWires ) +{ +SCH_LINE* last_line = NULL; + +EDA_ITEM* first = aWires.GetFirst(); +for( EDA_ITEM* p = first; p; p = p->Next() ) +{ +SCH_LINE *line = dynamic_cast( p ); + +if( p != first ) +{ +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 ); +p = line; +} +} +last_line = line; +} +} + + +/** * Mouse capture callback for drawing line segments. */ static void DrawSegment( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wxPoint& aPosition, @@ -261,6 +301,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 ); ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Remove backtracking wires in eeschema
I orginally pinged Chris about this bug and he friendly took up the task. Thumbs up. I have applied the patch and it fixes the issue. On a side note: There was once upon a time that one could draw backwards on a wire to delete it, but one can't do that anymore. (But I think that has been impossible for quite some time now.) Is this something I am dreaming or was this really the case? 2015-06-22 22:50 GMT+02:00 Chris Pavlina : > If you place a wire segment and then backtrack over it, you end up with > a broken wire in the end: > > http://misc.c4757p.com/backtrack.mp4 > > This is because the two segments are merged together, rather than the > second *subtracting* from the first, even though the latter case is the > way it's shown as you're drawing. This patch fixes that by adding a > function to erase backtracks. > > -- > Chris > > ___ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Remove backtracking wires in eeschema
Le 22/06/2015 22:50, Chris Pavlina a écrit : > If you place a wire segment and then backtrack over it, you end up with > a broken wire in the end: > > http://misc.c4757p.com/backtrack.mp4 > > This is because the two segments are merged together, rather than the > second *subtracting* from the first, even though the latter case is the > way it's shown as you're drawing. This patch fixes that by adding a > function to erase backtracks. > > -- > Chris > Thanks for this patch, it is very usefull. I noticed you are using a dynamic cast in line: SCH_LINE *line = dynamic_cast( p ); but the variable line is used without any test for NULL. Can you have a look at this (use a static cast or a test for NULL) ? Thanks. -- Jean-Pierre CHARRAS ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Remove backtracking wires in eeschema
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. On Tue, Jun 23, 2015 at 09:24:36AM +0200, jp charras wrote: > Le 22/06/2015 22:50, Chris Pavlina a écrit : > > If you place a wire segment and then backtrack over it, you end up with > > a broken wire in the end: > > > > http://misc.c4757p.com/backtrack.mp4 > > > > This is because the two segments are merged together, rather than the > > second *subtracting* from the first, even though the latter case is the > > way it's shown as you're drawing. This patch fixes that by adding a > > function to erase backtracks. > > > > -- > > Chris > > > > Thanks for this patch, it is very usefull. > > I noticed you are using a dynamic cast in line: > > SCH_LINE *line = dynamic_cast( p ); > > but the variable line is used without any test for NULL. > > Can you have a look at this (use a static cast or a test for NULL) ? > > Thanks. > > > -- > Jean-Pierre CHARRAS commit 5b7beeba531124fe9b7786df9f4e977f8ada7ff5 Author: Chris Pavlina Date: Tue Jun 23 09:18:36 2015 -0400 Remove backtracking segments when drawing wires diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp index 62141bd..c173f4e 100644 --- a/eeschema/bus-wire-junction.cpp +++ b/eeschema/bus-wire-junction.cpp @@ -43,6 +43,7 @@ #include #include #include +#include static void AbortCreateNewLine( EDA_DRAW_PANEL* aPanel, wxDC* aDC ); @@ -53,6 +54,47 @@ 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& aWires ) +{ +SCH_LINE* last_line = NULL; + +EDA_ITEM* first = aWires.GetFirst(); +for( EDA_ITEM* p = first; p; p = p->Next() ) +{ +SCH_LINE *line = dynamic_cast( p ); +wxASSERT_MSG( line, "RemoveBacktracks() requires SCH_LINE items" ); +continue; + +if( p != first ) +{ +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 ); +p = line; +} +} +last_line = line; +} +} + + +/** * Mouse capture callback for drawing line segments. */ static void DrawSegment( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wxPoint& aPosition, @@ -261,6 +303,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 ); ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Remove backtracking wires in eeschema
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 ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Remove backtracking wires in eeschema
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 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 #include #include +#include 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& aWires ) +{ +SCH_LINE* last_line = NULL; + +EDA_ITEM* first = aWires.GetFirst(); +for( EDA_ITEM* p = first; p; ) +{ +SCH_LINE *line = dynamic_cast( 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 ); ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Remove backtracking wires in eeschema
Le 23/06/2015 16:24, Chris Pavlina a écrit : > 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... > Committed. Thanks. -- Jean-Pierre CHARRAS ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp