From 32cea2e656fa73b808b90733e4dd3dbba42cbdc9 Mon Sep 17 00:00:00 2001 From: jean-pierre charras Date: Sat, 4 Feb 2012 21:30:00 +0100 Subject: [PATCH] Pcbnew: Fix incorrect behavior of undo/redo command (this issue happens only when a new track is created, and an old redundant track is deleted) Minor code cleaning --- include/class_undoredo_container.h | 3 ++ pcbnew/board_undo_redo.cpp | 16 ++++--- pcbnew/class_board.cpp | 70 +++++++++++------------------- pcbnew/class_board.h | 4 +- pcbnew/deltrack.cpp | 3 -- pcbnew/editrack.cpp | 9 ++-- pcbnew/tr_modif.cpp | 12 ++--- pcbnew/xchgmod.cpp | 8 ++-- 8 files changed, 55 insertions(+), 70 deletions(-) diff --git a/include/class_undoredo_container.h b/include/class_undoredo_container.h index 30c5cda79f..0efe9c00dd 100644 --- a/include/class_undoredo_container.h +++ b/include/class_undoredo_container.h @@ -98,6 +98,7 @@ public: * copy of an active item) and m_Link points the active * item in schematic */ +public: ITEM_PICKER( EDA_ITEM* aItem = NULL, UNDO_REDO_T aUndoRedoStatus = UR_UNSPECIFIED ); EDA_ITEM* GetItem() const { return m_PickedItem; } @@ -106,6 +107,8 @@ public: KICAD_T GetItemType() const { return m_PickedItemType; } + void SetStatus( UNDO_REDO_T aStatus ) { m_UndoRedoStatus = aStatus; } + void SetItemType( KICAD_T aType ) { m_PickedItemType = aType; } void SetLink( EDA_ITEM* aItem ) { m_Link = aItem; } diff --git a/pcbnew/board_undo_redo.cpp b/pcbnew/board_undo_redo.cpp index 5c923504cb..8a12dc109a 100644 --- a/pcbnew/board_undo_redo.cpp +++ b/pcbnew/board_undo_redo.cpp @@ -399,10 +399,6 @@ void PCB_EDIT_FRAME::SaveCopyInUndoList( PICKED_ITEMS_LIST& aItemsList, */ if( commandToUndo->GetPickedItemLink( ii ) == NULL ) commandToUndo->SetPickedItemLink( item->Clone(), ii ); - - if( commandToUndo->GetPickedItemLink( ii ) == NULL ) - wxMessageBox( wxT( "SaveCopyInUndoList() in UR_CHANGED mode: NULL link" ) ); - break; case UR_MOVED: @@ -459,7 +455,7 @@ void PCB_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool aRed // Undo in the reverse order of list creation: (this can allow stacked changes // like the same item can be changes and deleted in the same complex command - TestForExistingItem( GetBoard(), NULL ); // Build list of existing items, for integrity test + bool build_item_list = true; // if true the list of esiting items must be rebuilt for( int ii = aList->GetCount()-1; ii >= 0 ; ii-- ) { item = (BOARD_ITEM*) aList->GetPickedItem( ii ); @@ -470,9 +466,16 @@ void PCB_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool aRed * - if a call to SaveCopyInUndoList was forgotten in Pcbnew * - in zones outlines, when a change in one zone merges this zone with an other * This test avoids a Pcbnew crash + * the test is made only to avoid crashes, so it is not needed for deleted or new items */ - if( aList->GetPickedItemStatus( ii ) != UR_DELETED ) + UNDO_REDO_T status = aList->GetPickedItemStatus( ii ); + if( status != UR_DELETED && status != UR_NEW ) { + if( build_item_list ) + // Build list of existing items, for integrity test + TestForExistingItem( GetBoard(), NULL ); + build_item_list = false; + if( !TestForExistingItem( GetBoard(), item ) ) { // Remove this non existant item @@ -517,6 +520,7 @@ void PCB_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool aRed case UR_DELETED: /* deleted items are put in List, as new items */ aList->SetPickedItemStatus( UR_NEW, ii ); GetBoard()->Add( item ); + build_item_list = true; break; case UR_MOVED: diff --git a/pcbnew/class_board.cpp b/pcbnew/class_board.cpp index 96afab7ad0..de5986576c 100644 --- a/pcbnew/class_board.cpp +++ b/pcbnew/class_board.cpp @@ -2074,6 +2074,11 @@ BOARD_CONNECTED_ITEM* BOARD::GetLockPoint( const wxPoint& aPosition, int aLayerM TRACK* BOARD::CreateLockPoint( wxPoint& aPosition, TRACK* aSegment, PICKED_ITEMS_LIST* aList ) { + /* creates an intermediate point on aSegment and break it into two segments + * at aPosition. + * The new segment starts from aPosition and ends at the end point of + * aSegment. The original segment now ends at aPosition. + */ if( aSegment->m_Start == aPosition || aSegment->m_End == aPosition ) return NULL; @@ -2087,43 +2092,26 @@ TRACK* BOARD::CreateLockPoint( wxPoint& aPosition, TRACK* aSegment, PICKED_ITEMS // Calculation coordinate of intermediate point relative to the start point of aSegment wxPoint delta = aSegment->m_End - aSegment->m_Start; - // Not yet in use: -#if 0 - int ox, oy, fx, fy; - - if( aRefSegm ) - { - ox = aRefSegm->m_Start.x - aSegment->m_Start.x; - oy = aRefSegm->m_Start.y - aSegment->m_Start.y; - fx = aRefSegm->m_End.x - aSegment->m_Start.x; - fy = aRefSegm->m_End.y - aSegment->m_Start.y; - } -#endif - // calculate coordinates of aPosition relative to aSegment->m_Start - wxPoint newPoint = aPosition - aSegment->m_Start; + wxPoint lockPoint = aPosition - aSegment->m_Start; - // newPoint must be on aSegment: - // Ensure newPoint.y/newPoint.y = delta.y/delta.x + // lockPoint must be on aSegment: + // Ensure lockPoint.y/lockPoint.y = delta.y/delta.x if( delta.x == 0 ) - newPoint.x = 0; /* horizontal segment*/ + lockPoint.x = 0; /* horizontal segment*/ else - newPoint.y = wxRound( ( (double)newPoint.x * delta.y ) / delta.x ); + lockPoint.y = wxRound( ( (double)lockPoint.x * delta.y ) / delta.x ); /* Create the intermediate point (that is to say creation of a new * segment, beginning at the intermediate point. */ - newPoint.x += aSegment->m_Start.x; - newPoint.y += aSegment->m_Start.y; + lockPoint += aSegment->m_Start; TRACK* newTrack = (TRACK*)aSegment->Clone(); - - if( aList ) - { - ITEM_PICKER picker( newTrack, UR_NEW ); - aList->PushItem( picker ); - } - + // The new segment begins at the new point, + newTrack->m_Start = lockPoint; + newTrack->start = aSegment; + newTrack->SetState( BEGIN_ONPAD, OFF ); DLIST* list = (DLIST*)aSegment->GetList(); wxASSERT( list ); @@ -2131,28 +2119,22 @@ TRACK* BOARD::CreateLockPoint( wxPoint& aPosition, TRACK* aSegment, PICKED_ITEMS if( aList ) { - ITEM_PICKER picker( aSegment, UR_CHANGED ); - picker.m_Link = aSegment->Clone(); + // Prepare the undo command for the now track segment + ITEM_PICKER picker( newTrack, UR_NEW ); + aList->PushItem( picker ); + // Prepare the undo command for the old track segment + // before modifications + picker.SetItem( aSegment ); + picker.SetStatus( UR_CHANGED ); + picker.SetLink( aSegment->Clone() ); aList->PushItem( picker ); } - /* Correct pointer at the end of the new segment. */ - newTrack->end = aSegment->end; - newTrack->SetState( END_ONPAD, aSegment->GetState( END_ONPAD ) ); - - /* Set connections info relative to the new point - */ - - /* Old segment now ends at new point. */ - aSegment->m_End = newPoint; + // Old track segment now ends at new point. + aSegment->m_End = lockPoint; aSegment->end = newTrack; aSegment->SetState( END_ONPAD, OFF ); - /* The new segment begins at the new point. */ - newTrack->m_Start = newPoint; - newTrack->start = aSegment; - newTrack->SetState( BEGIN_ONPAD, OFF ); - D_PAD * pad = GetPad( newTrack, START ); if ( pad ) @@ -2163,7 +2145,7 @@ TRACK* BOARD::CreateLockPoint( wxPoint& aPosition, TRACK* aSegment, PICKED_ITEMS aSegment->SetState( END_ONPAD, ON ); } - aPosition = newPoint; + aPosition = lockPoint; return newTrack; } diff --git a/pcbnew/class_board.h b/pcbnew/class_board.h index 04d634e056..10960c3bc0 100644 --- a/pcbnew/class_board.h +++ b/pcbnew/class_board.h @@ -1308,10 +1308,10 @@ public: /** * Function CreateLockPoint * creates an intermediate point on \a aSegment and break it into two segments - * at \a aPoition. + * at \a aPosition. *

* The new segment starts from \a aPosition and ends at the end point of \a - * aSegement. The original segment now ends at \a aPosition. + * aSegment. The original segment now ends at \a aPosition. *

* * @param aPosition A wxPoint object containing the position to test and the new diff --git a/pcbnew/deltrack.cpp b/pcbnew/deltrack.cpp index c177a9787c..c97627cbca 100644 --- a/pcbnew/deltrack.cpp +++ b/pcbnew/deltrack.cpp @@ -215,9 +215,6 @@ void PCB_EDIT_FRAME::Remove_One_Track( wxDC* DC, TRACK* pt_segm ) next_track = tracksegment->Next(); tracksegment->SetState( BUSY, OFF ); - //D( printf( "%s: track %p status=\"%s\"\n", __func__, tracksegment, - // TO_UTF8( TRACK::ShowState( tracksegment->GetState( -1 ) ) ) - // ); ) D( std::cout << __func__ << ": track " << tracksegment << " status=" \ << TO_UTF8( TRACK::ShowState( tracksegment->GetState( -1 ) ) ) \ << std::endl; ) diff --git a/pcbnew/editrack.cpp b/pcbnew/editrack.cpp index 3deadbb941..84cde40a3d 100644 --- a/pcbnew/editrack.cpp +++ b/pcbnew/editrack.cpp @@ -50,6 +50,8 @@ static void ComputeBreakPoint( TRACK* track, int n, wxPoint end ); static void DeleteNullTrackSegments( BOARD* pcb, DLIST& aTrackList ); static void EnsureEndTrackOnPad( D_PAD* Pad ); +// A PICKED_ITEMS_LIST to store tracks which are modified/added/deleted +// during a track edition: static PICKED_ITEMS_LIST s_ItemsListPicker; @@ -456,12 +458,9 @@ bool PCB_EDIT_FRAME::End_Route( TRACK* aTrack, wxDC* aDC ) else // If end point of is on a different track, // creates a lock point if not exists { - TRACK* adr_buf = (TRACK*) LockPoint; - GetBoard()->SetHighLightNet( adr_buf->GetNet() ); - - // Creates a lock point (if not already exists): + // Creates a lock point, if not already exists: LockPoint = GetBoard()->CreateLockPoint( g_CurrentTrackSegment->m_End, - adr_buf, + (TRACK*) LockPoint, &s_ItemsListPicker ); } } diff --git a/pcbnew/tr_modif.cpp b/pcbnew/tr_modif.cpp index e231200702..270067565b 100644 --- a/pcbnew/tr_modif.cpp +++ b/pcbnew/tr_modif.cpp @@ -25,7 +25,8 @@ /** * @file tr_modif.cpp - * @brief Trace editing. + * @brief Trace editing: detects an removes a track which is become redunding, + * after a new track is craeted. */ #include @@ -42,6 +43,10 @@ static void ListSetState( EDA_ITEM* Start, int NbItem, int State, int onoff ); +/* + * This function try to remove an old track, when a new track is created, + * and the old track is no more needed + */ int PCB_EDIT_FRAME::EraseRedundantTrack( wxDC* aDC, TRACK* aNewTrack, int aNewTrackSegmentsCount, @@ -292,14 +297,11 @@ int PCB_EDIT_FRAME::EraseRedundantTrack( wxDC* aDC, } -/* Set the bits of .m_State member to on off value, using bit mask State +/* Set the bits of .m_State member to on/off value, using bit mask State * of a list of EDA_ITEM */ static void ListSetState( EDA_ITEM* Start, int NbItem, int State, int onoff ) { - if( Start == NULL ) - return; - for( ; (Start != NULL ) && ( NbItem > 0 ); NbItem--, Start = Start->Next() ) { Start->SetState( State, onoff ); diff --git a/pcbnew/xchgmod.cpp b/pcbnew/xchgmod.cpp index 5a686f30b9..c791e733f7 100644 --- a/pcbnew/xchgmod.cpp +++ b/pcbnew/xchgmod.cpp @@ -152,7 +152,6 @@ int DIALOG_EXCHANGE_MODULE::Maj_ListeCmp( const wxString& reference, FILE* FichCmp, * NewFile; char line[1024]; wxString msg; - char* ignore; if( old_name == new_name ) return 0; @@ -190,9 +189,9 @@ int DIALOG_EXCHANGE_MODULE::Maj_ListeCmp( const wxString& reference, return 1; } - ignore = fgets( line, sizeof(line), FichCmp ); + fgets( line, sizeof(line), FichCmp ); - fprintf( NewFile, "Cmp-Mod V01 Genere par PcbNew le %s\n", TO_UTF8( DateAndTime() ) ); + fprintf( NewFile, "Cmp-Mod V01 Created by PcbNew date = %s\n", TO_UTF8( DateAndTime() ) ); bool start_descr = false; @@ -588,7 +587,6 @@ void PCB_EDIT_FRAME::RecreateCmpFileFromBoard( wxCommandEvent& aEvent ) MODULE* Module = GetBoard()->m_Modules; wxString msg; wxString wildcard; - char* ignore; if( Module == NULL ) { @@ -619,7 +617,7 @@ void PCB_EDIT_FRAME::RecreateCmpFileFromBoard( wxCommandEvent& aEvent ) return; } - ignore = fgets( line, sizeof(line), FichCmp ); + fgets( line, sizeof(line), FichCmp ); fprintf( FichCmp, "Cmp-Mod V01 Genere par PcbNew le %s\n", TO_UTF8( DateAndTime() ) ); for( ; Module != NULL; Module = Module->Next() )