pcbnew: serious bugs fixed (see changelog)

CHANGELOG.txt

@@ -5,6 +5,20 @@ Started 2007-June-11
 Please add newer entries at the top, list the date and your name with
 email address.
 
+2009-mar-05 UPDATE Jean-Pierre Charras <jean-pierre.charras@inpg.fr>
+================================================================================
+++pcbnew:
+    bug fix in move_or_drag_track.cpp:
+        function SortTrackEndPoints() broken: does not handle pointers to pads for start and end and flags relative to these pointers
+        MergeCollinearTracks( ) broken, because it merge segments having different width or without any connectivity test.
+            2 collinear segments can be merged only in no other segment or vais is connected to the common point
+            and if they have the same width. See cleanup.cpp for merge functions
+        These functions break the connectivity calculations.
+        So they are temporary disabled (see my comments in these functions)
+    bug fix in Marque_Une_Piste(): the last segments created can be see as part of the flagged track:
+        so delete track (or edit track width) deletes the track and some others segments (last created)
+
+    
 2009-Feb-25 UPDATE Wayne Stambaugh <stambaughw@verizon.net>
 ================================================================================
 ++EESchema

pcbnew/move_or_drag_track.cpp

@@ -60,7 +60,9 @@ static void Abort_MoveTrack( WinEDA_DrawPanel* Panel, wxDC* DC )
 
     Panel->GetScreen()->m_Curseur = oldpos;
     g_HightLigt_Status = FALSE;
-    ( (WinEDA_PcbFrame*) Panel->m_Parent )->GetBoard()->DrawHighLight( Panel, DC, g_HightLigth_NetCode );
+    ( (WinEDA_PcbFrame*) Panel->m_Parent )->GetBoard()->DrawHighLight( Panel,
+                                                                       DC,
+                                                                       g_HightLigth_NetCode );
 
     if( NewTrack )
     {
@@ -102,7 +104,7 @@ static void Abort_MoveTrack( WinEDA_DrawPanel* Panel, wxDC* DC )
 
     Panel->ManageCurseur = NULL;
     Panel->ForceCloseManageCurseur = NULL;
-    ((WinEDA_PcbFrame*)Panel->m_Parent)->SetCurItem(NULL);
+    ( (WinEDA_PcbFrame*) Panel->m_Parent )->SetCurItem( NULL );
 
     /* Annulation deplacement et Redessin des segments dragges */
     DRAG_SEGM* pt_drag = g_DragSegmentList;
@@ -118,7 +120,9 @@ static void Abort_MoveTrack( WinEDA_DrawPanel* Panel, wxDC* DC )
     g_HightLigth_NetCode = Old_HightLigth_NetCode;
     g_HightLigt_Status   = Old_HightLigt_Status;
     if( g_HightLigt_Status )
-        ( (WinEDA_PcbFrame*) Panel->m_Parent )->GetBoard()->DrawHighLight( Panel, DC, g_HightLigth_NetCode );
+        ( (WinEDA_PcbFrame*) Panel->m_Parent )->GetBoard()->DrawHighLight( Panel,
+                                                                           DC,
+                                                                           g_HightLigth_NetCode );
 
     EraseDragListe();
 }
@@ -292,10 +296,10 @@ static void Show_Drag_Track_Segment_With_Cte_Slope( WinEDA_DrawPanel* panel,
     dy = Pos.y - s_LastPos.y;
 
     //move the line by dx and dy
-    tx1 = (double) (Track->m_Start.x + dx);
-    ty1 = (double) (Track->m_Start.y + dy);
-    tx2 = (double) (Track->m_End.x + dx);
-    ty2 = (double) (Track->m_End.y + dy);
+    tx1 = (double) ( Track->m_Start.x + dx );
+    ty1 = (double) ( Track->m_Start.y + dy );
+    tx2 = (double) ( Track->m_End.x + dx );
+    ty2 = (double) ( Track->m_End.y + dy );
 
     // recalculate the segments new parameters and intersection points
     // only the intercept will change, segment slopes does not change
@@ -482,6 +486,7 @@ bool InitialiseDragParameters()
                 tSegmentToStart = TrackSegWrapper->m_Segm; // Get the segment connected to the start point
         }
     }
+
     //would be nice to eliminate collinear segments here, so we don't
     //have to deal with that annoying "Unable to drag this segment: two collinear segments"
 
@@ -691,57 +696,73 @@ void WinEDA_PcbFrame::Start_MoveOneNodeOrSegment( TRACK* track, wxDC* DC, int co
     GetBoard()->DrawHighLight( DrawPanel, DC, g_HightLigth_NetCode );
     DrawPanel->ManageCurseur( DrawPanel, DC, TRUE );
 }
-void SortTrackEndPoints(TRACK* track)
+
+
+#if 0
+// @todo: This function is broken: does not handle pointers to pads for start and end and flags relative to these pointers
+void SortTrackEndPoints( TRACK* track )
 {
     //sort the track endpoints -- should not matter in terms of drawing
     //or producing the pcb -- but makes doing comparisons easier.
-    wxPoint tmp;
     int     dx = track->m_End.x - track->m_Start.x;
-    if(dx){
-        if( track->m_Start.x > track->m_End.x ){
-            tmp = track->m_Start;
-            track->m_Start = track->m_End;
-            track->m_End = tmp;
+
+    if( dx )
+    {
+        if( track->m_Start.x > track->m_End.x )
+        {
+            EXCHG(track->m_Start, track->m_End);
         }
-    }else{
-        if( track->m_Start.y > track->m_End.y ){
-            tmp = track->m_Start;
-            track->m_Start = track->m_End;
-            track->m_End = tmp;
+    }
+    else
+    {
+        if( track->m_Start.y > track->m_End.y )
+        {
+            EXCHG(track->m_Start, track->m_End);
         }
     }
 }
+
+
 /***********************************************************************************/
 bool WinEDA_PcbFrame::MergeCollinearTracks( TRACK* track, wxDC* DC, int end )
 /***********************************************************************************/
-{
-    TRACK* testtrack = NULL;
+/**
+ * @todo: this function is broken, because it merge segments having different width or without any connectivity test.
+ * 2 collinear segments can be merged only in no other segment or vais is connected to the common point
+ * and if they have the same width. See cleanup.cpp for merge functions,
+ * and consider Marque_Une_Piste() to locate segments that can be merged
+ */
 
     testtrack = (TRACK*) Locate_Piste_Connectee( track, GetBoard()->m_Track, NULL, end );
     if( testtrack )
     {
-        SortTrackEndPoints(track);
-        SortTrackEndPoints(testtrack);
+        SortTrackEndPoints( track );
+        SortTrackEndPoints( testtrack );
         int dx  = track->m_End.x - track->m_Start.x;
         int dy  = track->m_End.y - track->m_Start.y;
         int tdx = testtrack->m_End.x - testtrack->m_Start.x;
         int tdy = testtrack->m_End.y - testtrack->m_Start.y;
 
         if( (dy * tdx == dx * tdy && dy != 0 && dx != 0 && tdy != 0 && tdx != 0) /*angle, same slope*/
-               || (dy == 0 && tdy == 0 && dx*tdx )/*horizontal*/
-               || (dx == 0 && tdx == 0 && dy*tdy )/*vertical*/ ) {
-            if(track->m_Start == testtrack->m_Start || track->m_End == testtrack->m_Start){
-                if( ( dx*tdx && testtrack->m_End.x > track->m_End.x )
-                  ||( dy*tdy && testtrack->m_End.y > track->m_End.y )){
+           || (dy == 0 && tdy == 0 && dx * tdx )  /*horizontal*/
+           || (dx == 0 && tdx == 0 && dy * tdy ) /*vertical*/ )
+        {
+            if( track->m_Start == testtrack->m_Start || track->m_End == testtrack->m_Start )
+            {
+                if( ( dx * tdx && testtrack->m_End.x > track->m_End.x )
+                   ||( dy * tdy && testtrack->m_End.y > track->m_End.y ) )
+                {
                     track->m_End = testtrack->m_End;
 
                     Delete_Segment( DC, testtrack );
                     return true;
                 }
             }
-            if(track->m_Start == testtrack->m_End || track->m_End == testtrack->m_End){
-                if( ( dx*tdx && testtrack->m_Start.x < track->m_Start.x )
-                  ||( dy*tdy && testtrack->m_Start.y < track->m_Start.y )){
+            if( track->m_Start == testtrack->m_End || track->m_End == testtrack->m_End )
+            {
+                if( ( dx * tdx && testtrack->m_Start.x < track->m_Start.x )
+                   ||( dy * tdy && testtrack->m_Start.y < track->m_Start.y ) )
+                {
                     track->m_Start = testtrack->m_Start;
 
                     Delete_Segment( DC, testtrack );
@@ -752,6 +773,8 @@ bool WinEDA_PcbFrame::MergeCollinearTracks( TRACK* track, wxDC* DC, int end )
     }
     return false;
 }
+#endif
+
 /***********************************************************************************/
 void WinEDA_PcbFrame::Start_DragTrackSegmentAndKeepSlope( TRACK* track, wxDC* DC )
 /***********************************************************************************/
@@ -763,13 +786,22 @@ void WinEDA_PcbFrame::Start_DragTrackSegmentAndKeepSlope( TRACK* track, wxDC* DC
     if( !track )
         return;
 
-    while(MergeCollinearTracks(track, DC, START)){};
-    while(MergeCollinearTracks(track, DC, END)){};
+
+#if 0
+    // Broken functions: see comments
+    while( MergeCollinearTracks( track, DC, START ) )
+    {
+    };
+    while( MergeCollinearTracks( track, DC, END ) )
+    {
+    };
+#endif
 
     s_StartSegmentPresent = s_EndSegmentPresent = TRUE;
 
     if( (track->start == NULL) || (track->start->Type() == TYPE_TRACK) )
-        TrackToStartPoint = (TRACK*) Locate_Piste_Connectee( track, GetBoard()->m_Track, NULL, START );
+        TrackToStartPoint = (TRACK*) Locate_Piste_Connectee( track,
+                                                             GetBoard()->m_Track, NULL, START );
 
     //  Test if more than one segment is connected to this point
     if( TrackToStartPoint )

pcbnew/track.cpp

@@ -55,18 +55,24 @@ TRACK* Marque_Une_Piste( WinEDA_BasePcbFrame* frame, wxDC* DC,
     if( aTrackList == NULL )
         return NULL;
 
-    /* Marquage du segment pointe */
     if( flagcolor )
         aTrackList->Draw( frame->DrawPanel, DC, flagcolor );
 
+    // Ensure the flag BUSY is cleared because we use it to mark segments of the track
+    for( TRACK* track = frame->GetBoard()->m_Track; track; track = track->Next() )
+        track->SetState( BUSY , OFF );
+
+    /* Set flags of the initial track segment */
     aTrackList->SetState( BUSY, ON );
     int masque_layer = aTrackList->ReturnMaskLayer();
 
     trackList.push_back( aTrackList );
 
-    /* Traitement du segment pointe : si c'est un segment, le cas est simple.
-     *  Si c'est une via, on doit examiner le nombre de segments connectes.
-     *  Si <=2, on doit detecter une piste, si > 2 seule la via est marquee
+    /* Examine the initial track segment : if it is really a segment, this is easy.
+     *  If it is a via, one must search for connected segments.
+     *  If <=2, this via connect 2 segments (or is connected to only one segment)
+     *      and this via and these 2 segments are a part of a track.
+     *  If > 2 only this via is flagged (the track has only this via)
      */
     if( aTrackList->Type() == TYPE_VIA )
     {
@@ -83,17 +89,17 @@ TRACK* Marque_Une_Piste( WinEDA_BasePcbFrame* frame, wxDC* DC,
             Segm3 = Fast_Locate_Piste( Segm2->Next(), NULL,
                                       aTrackList->m_Start, masque_layer );
         }
-        if( Segm3 )
+        if( Segm3 )     // More than 2 segments are connected to this via. the "track" is only this via
         {
             *nb_segm = 1;
             return aTrackList;
         }
-        if( Segm1 )
+        if( Segm1 )     // search for others segments connected to the initial segment start point
         {
             masque_layer = Segm1->ReturnMaskLayer();
             Marque_Chaine_segments( frame->GetBoard(), aTrackList->m_Start, masque_layer, &trackList );
         }
-        if( Segm2 )
+        if( Segm2 )     // search for others segments connected to the initial segment end point
         {
             masque_layer = Segm2->ReturnMaskLayer();
             Marque_Chaine_segments( frame->GetBoard(), aTrackList->m_Start, masque_layer, &trackList );
@@ -171,9 +177,7 @@ TRACK* Marque_Une_Piste( WinEDA_BasePcbFrame* frame, wxDC* DC,
             if( track->GetState( BUSY ) )
             {
                 NbSegmBusy++;
-
                 track->UnLink();
-
                 list->Insert( track, firstTrack->Next() );
             }
         }