Pcbnew: fix graphic item undo/redo bug. (--fixes lp:1252264)

* Use std::swap for PCB_LINE_T items in SwapData().
* Use std::swap for PCB_MODULE_EDGE_T items in SwapData().
* Add assignment operator to EDGE_MODULE object.
* Move code from copy method to assignment operator.
* Minor coding policy fixes.

pcbnew/board_undo_redo.cpp

@@ -38,6 +38,7 @@
 #include <class_module.h>
 #include <class_dimension.h>
 #include <class_zone.h>
+#include <class_edge_mod.h>
 
 
 /* Functions to undo and redo edit commands.
@@ -195,81 +196,70 @@ void SwapData( BOARD_ITEM* aItem, BOARD_ITEM* aImage )
     switch( aItem->Type() )
     {
     case PCB_MODULE_T:
-        {
-            MODULE* tmp = (MODULE*) aImage->Clone();
-            ( (MODULE*) aImage )->Copy( (MODULE*) aItem );
-            ( (MODULE*) aItem )->Copy( tmp );
-            delete tmp;
-        }
+    {
+        MODULE* tmp = (MODULE*) aImage->Clone();
+        ( (MODULE*) aImage )->Copy( (MODULE*) aItem );
+        ( (MODULE*) aItem )->Copy( tmp );
+        delete tmp;
+    }
         break;
 
     case PCB_ZONE_AREA_T:
-        {
-            ZONE_CONTAINER* tmp = (ZONE_CONTAINER*) aImage->Clone();
-            ( (ZONE_CONTAINER*) aImage )->Copy( (ZONE_CONTAINER*) aItem );
-            ( (ZONE_CONTAINER*) aItem )->Copy( tmp );
-            delete tmp;
-        }
+    {
+        ZONE_CONTAINER* tmp = (ZONE_CONTAINER*) aImage->Clone();
+        ( (ZONE_CONTAINER*) aImage )->Copy( (ZONE_CONTAINER*) aItem );
+        ( (ZONE_CONTAINER*) aItem )->Copy( tmp );
+        delete tmp;
+    }
         break;
 
     case PCB_LINE_T:
-#if 0
-        EXCHG( ( (DRAWSEGMENT*) aItem )->m_Start, ( (DRAWSEGMENT*) aImage )->m_Start );
-        EXCHG( ( (DRAWSEGMENT*) aItem )->m_End, ( (DRAWSEGMENT*) aImage )->m_End );
-        EXCHG( ( (DRAWSEGMENT*) aItem )->m_Width, ( (DRAWSEGMENT*) aImage )->m_Width );
-        EXCHG( ( (DRAWSEGMENT*) aItem )->m_Shape, ( (DRAWSEGMENT*) aImage )->m_Shape );
-#else
-        {
-            DRAWSEGMENT tmp = *(DRAWSEGMENT*) aImage;
-            *aImage = *aItem;
-            *aItem  = tmp;
-        }
-#endif
+        std::swap( *((DRAWSEGMENT*)aItem), *((DRAWSEGMENT*)aImage) );
         break;
 
     case PCB_TRACE_T:
     case PCB_VIA_T:
-        {
-            TRACK* track = (TRACK*) aItem;
-            TRACK* image = (TRACK*) aImage;
+    {
+        TRACK* track = (TRACK*) aItem;
+        TRACK* image = (TRACK*) aImage;
 
-            // swap start, end, width and shape for track and image.
-            wxPoint exchp = track->GetStart();
-            track->SetStart( image->GetStart() );
-            image->SetStart( exchp );
-            exchp = track->GetEnd();
-            track->SetEnd( image->GetEnd() );
-            image->SetEnd( exchp );
+        // swap start, end, width and shape for track and image.
+        wxPoint exchp = track->GetStart();
+        track->SetStart( image->GetStart() );
+        image->SetStart( exchp );
+        exchp = track->GetEnd();
+        track->SetEnd( image->GetEnd() );
+        image->SetEnd( exchp );
 
-            int atmp = track->GetWidth();
-            track->SetWidth( image->GetWidth() );
-            image->SetWidth( atmp );
-            atmp = track->GetShape();
-            track->SetShape( image->GetShape() );
-            image->SetShape( atmp );
+        int atmp = track->GetWidth();
+        track->SetWidth( image->GetWidth() );
+        image->SetWidth( atmp );
+        atmp = track->GetShape();
+        track->SetShape( image->GetShape() );
+        image->SetShape( atmp );
 
-            atmp = track->GetDrillValue();
+        atmp = track->GetDrillValue();
 
-            if( track->IsDrillDefault() )
-                atmp = -1;
+        if( track->IsDrillDefault() )
+            atmp = -1;
 
-            int itmp = image->GetDrillValue();
+        int itmp = image->GetDrillValue();
 
-            if( image->IsDrillDefault() )
-                itmp = -1;
+        if( image->IsDrillDefault() )
+            itmp = -1;
 
-            EXCHG(itmp, atmp );
+        EXCHG(itmp, atmp );
 
-            if( atmp > 0 )
-                track->SetDrill( atmp );
-            else
-                track->SetDrillDefault();
+        if( atmp > 0 )
+            track->SetDrill( atmp );
+        else
+            track->SetDrillDefault();
 
-            if( itmp > 0 )
-                image->SetDrill( itmp );
-            else
-                image->SetDrillDefault();
-        }
+        if( itmp > 0 )
+            image->SetDrill( itmp );
+        else
+            image->SetDrillDefault();
+    }
         break;
 
     case PCB_TEXT_T:

pcbnew/class_edge_mod.cpp

@@ -66,17 +66,27 @@ EDGE_MODULE::~EDGE_MODULE()
 }
 
 
+const EDGE_MODULE& EDGE_MODULE::operator = ( const EDGE_MODULE& rhs )
+{
+    if( &rhs == this )
+        return *this;
+
+    DRAWSEGMENT::operator=( rhs );
+
+    m_Start0 = rhs.m_Start0;
+    m_End0   = rhs.m_End0;
+
+    m_PolyPoints = rhs.m_PolyPoints;    // std::vector copy
+    return *this;
+}
+
+
 void EDGE_MODULE::Copy( EDGE_MODULE* source )
 {
     if( source == NULL )
         return;
 
-    DRAWSEGMENT::Copy( source );
-
-    m_Start0 = source->m_Start0;
-    m_End0   = source->m_End0;
-
-    m_PolyPoints = source->m_PolyPoints;    // std::vector copy
+    *this = *source;
 }
 
 

pcbnew/class_edge_mod.h

@@ -32,6 +32,8 @@
 #define CLASS_EDGE_MOD_H_
 
 
+#include <wx/gdicmn.h>
+
 #include <class_drawsegment.h>
 
 
@@ -52,6 +54,9 @@ public:
     EDGE_MODULE* Next() const { return (EDGE_MODULE*) Pnext; }
     EDGE_MODULE* Back() const { return (EDGE_MODULE*) Pback; }
 
+    /// skip the linked list stuff, and parent
+    const EDGE_MODULE& operator = ( const EDGE_MODULE& rhs );
+
     void Copy( EDGE_MODULE* source );           // copy structure
 
     void SetStart0( const wxPoint& aPoint )     { m_Start0 = aPoint; }

pcbnew/dialogs/dialog_graphic_item_properties.cpp

-/**
- * @file dialog_graphic_item_properties.cpp
- */
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 1992-2013 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2010 Jean-Pierre Charras <jp.charras@wanadoo.fr>
+ * Copyright (C) 1992-2011 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -24,6 +22,10 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
  */
 
+/**
+ * @file dialog_graphic_item_properties.cpp
+ */
+
 /* Edit parameters values of graphic items type DRAWSEGMENTS:
  * Lines
  * Circles
@@ -48,6 +50,7 @@
 #include <dialog_graphic_item_properties_base.h>
 #include <class_pcb_layer_box_selector.h>
 
+
 class DIALOG_GRAPHIC_ITEM_PROPERTIES: public DIALOG_GRAPHIC_ITEM_PROPERTIES_BASE
 {
 private:
@@ -68,7 +71,7 @@ private:
 };
 
 DIALOG_GRAPHIC_ITEM_PROPERTIES::DIALOG_GRAPHIC_ITEM_PROPERTIES( PCB_EDIT_FRAME* aParent,
-                                                          DRAWSEGMENT * aItem, wxDC * aDC):
+                                                                DRAWSEGMENT * aItem, wxDC * aDC ):
     DIALOG_GRAPHIC_ITEM_PROPERTIES_BASE( aParent )
 {
     m_parent = aParent;
@@ -82,13 +85,11 @@ DIALOG_GRAPHIC_ITEM_PROPERTIES::DIALOG_GRAPHIC_ITEM_PROPERTIES( PCB_EDIT_FRAME*
 }
 
 
-/*******************************************************************************************/
-void PCB_EDIT_FRAME::InstallGraphicItemPropertiesDialog(DRAWSEGMENT * aItem, wxDC* aDC)
-/*******************************************************************************************/
+void PCB_EDIT_FRAME::InstallGraphicItemPropertiesDialog( DRAWSEGMENT* aItem, wxDC* aDC )
 {
     if ( aItem == NULL )
     {
-        DisplayError(this, wxT("InstallGraphicItemPropertiesDialog() error: NULL item"));
+        DisplayError( this, wxT( "InstallGraphicItemPropertiesDialog() error: NULL item" ) );
         return;
     }
 
@@ -99,12 +100,8 @@ void PCB_EDIT_FRAME::InstallGraphicItemPropertiesDialog(DRAWSEGMENT * aItem, wxD
     m_canvas->SetIgnoreMouseEvents( false );
 }
 
-/**************************************************************************/
+
 void DIALOG_GRAPHIC_ITEM_PROPERTIES::initDlg( )
-/**************************************************************************/
-/* Initialize messages and values in text control,
- * according to the item parameters values
-*/
 {
     m_StandardButtonsSizerOK->SetDefault();
 
@@ -124,6 +121,7 @@ void DIALOG_GRAPHIC_ITEM_PROPERTIES::initDlg( )
     {
         if( texts_unit[ii] == NULL )
             break;
+
         texts_unit[ii]->SetLabel( GetAbbreviatedUnitsLabel() );
     }
 
@@ -133,31 +131,30 @@ void DIALOG_GRAPHIC_ITEM_PROPERTIES::initDlg( )
     switch ( m_Item->GetShape() )
     {
     case S_CIRCLE:
-        m_StartPointXLabel->SetLabel(_("Center X"));
-        m_StartPointYLabel->SetLabel(_("Center Y"));
-        m_EndPointXLabel->SetLabel(_("Point X"));
-        m_EndPointYLabel->SetLabel(_("Point Y"));
-        m_Angle_Text->Show(false);
-        m_Angle_Ctrl->Show(false);
-        m_AngleUnit->Show(false);
+        m_StartPointXLabel->SetLabel( _( "Center X" ) );
+        m_StartPointYLabel->SetLabel( _( "Center Y" ) );
+        m_EndPointXLabel->SetLabel( _( "Point X" ) );
+        m_EndPointYLabel->SetLabel( _( "Point Y" ) );
+        m_Angle_Text->Show( false );
+        m_Angle_Ctrl->Show( false );
+        m_AngleUnit->Show( false );
         break;
 
     case S_ARC:
-        m_StartPointXLabel->SetLabel(_("Center X"));
-        m_StartPointYLabel->SetLabel(_("Center Y"));
-        m_EndPointXLabel->SetLabel(_("Start Point X"));
-        m_EndPointYLabel->SetLabel(_("Start Point Y"));
+        m_StartPointXLabel->SetLabel( _( "Center X" ) );
+        m_StartPointYLabel->SetLabel( _( "Center Y" ) );
+        m_EndPointXLabel->SetLabel( _( "Start Point X" ) );
+        m_EndPointYLabel->SetLabel( _( "Start Point Y" ) );
 
-        // Here the angle is a double, but the UI is still working
-        // with integers
+        // Here the angle is a double, but the UI is still working with integers.
         msg << int( m_Item->GetAngle() );
-        m_Angle_Ctrl->SetValue(msg);
+        m_Angle_Ctrl->SetValue( msg );
         break;
 
     default:
-        m_Angle_Text->Show(false);
-        m_Angle_Ctrl->Show(false);
-        m_AngleUnit->Show(false);
+        m_Angle_Text->Show( false );
+        m_Angle_Ctrl->Show( false );
+        m_AngleUnit->Show( false );
         break;
     }
 
@@ -188,16 +185,14 @@ void DIALOG_GRAPHIC_ITEM_PROPERTIES::initDlg( )
 
     if( m_LayerSelectionCtrl->SetLayerSelection( m_Item->GetLayer() ) < 0 )
     {
-        wxMessageBox( _("This item has an illegal layer id.\n"
-                        "Now, forced on the drawings layer. Please, fix it") );
+        wxMessageBox( _( "This item has an illegal layer id.\n"
+                         "Now, forced on the drawings layer. Please, fix it" ) );
         m_LayerSelectionCtrl->SetLayerSelection( DRAW_N );
     }
 }
 
 
-/*******************************************************************/
 void DIALOG_GRAPHIC_ITEM_PROPERTIES::OnLayerChoice( wxCommandEvent& event )
-/*******************************************************************/
 {
     int thickness;
 
@@ -209,11 +204,8 @@ void DIALOG_GRAPHIC_ITEM_PROPERTIES::OnLayerChoice( wxCommandEvent& event )
     PutValueInLocalUnits( *m_DefaultThicknessCtrl, thickness );
 }
 
-/*******************************************************************/
+
 void DIALOG_GRAPHIC_ITEM_PROPERTIES::OnOkClick( wxCommandEvent& event )
-/*******************************************************************/
-/* Copy values in text control to the item parameters
-*/
 {
     m_parent->SaveCopyInUndoList( m_Item, UR_CHANGED );