Implement undo/redo for origins in legacy

PICKED_ITEMS_LIST knows the architecture of the undo commands so
that it can delete those ITEMs which it owns.  This represents poor
encapsulation so instead of adding yet another case, I added a
UR_TRANSIENT item flag which is set by callers whenever they create
new objects to “give to” the undo/redo stack.  This allowed some
cleanup of other code, but cleaning up UR_DELETE and UR_WIRE_IMAGE
will be a bigger task and have to wait for another day.

Fixes: lp:1542018
* https://bugs.launchpad.net/kicad/+bug/1542018

common/class_undoredo_container.cpp

@@ -108,16 +108,16 @@ void PICKED_ITEMS_LIST::ClearListAndDeleteItems()
         ITEM_PICKER wrapper = PopItem();
         if( wrapper.GetItem() == NULL ) // No more item in list.
             break;
-        switch( wrapper.GetStatus() )
-        {
-        case UR_UNSPECIFIED:
-            if( show_error_message )
-                wxMessageBox( wxT( "ClearListAndDeleteItems() error: UR_UNSPECIFIED command type" ) );
 
-            show_error_message = false;
-            break;
+        // The Link is an undo construct; it is always owned by the undo/redo container
+        if( wrapper.GetLink() )
+            delete wrapper.GetLink();
 
-        case UR_WIRE_IMAGE:
+        if( wrapper.GetFlags() & UR_TRANSIENT )
+        {
+            delete wrapper.GetItem();
+        }
+        else if( wrapper.GetStatus() == UR_WIRE_IMAGE )
         {
             // Specific to eeschema: a linked list of wires is stored.  The wrapper picks only
             // the first item (head of list), and is owner of all picked items.
@@ -131,34 +131,11 @@ void PICKED_ITEMS_LIST::ClearListAndDeleteItems()
                 item = nextitem;
             }
         }
-        break;
-
-        case UR_MOVED:
-        case UR_FLIPPED:
-        case UR_MIRRORED_X:
-        case UR_MIRRORED_Y:
-        case UR_ROTATED:
-        case UR_ROTATED_CLOCKWISE:
-        case UR_NEW:        // Do nothing, items are in use, the picker is not owner of items
-            break;
-
-        case UR_CHANGED:
-        case UR_EXCHANGE_T:
-        case UR_DRILLORIGIN:
-        case UR_GRIDORIGIN:
-            delete wrapper.GetLink();   //  the picker is owner of this item
-            break;
-
-        case UR_DELETED:            // the picker is owner of this item
-        case UR_LIBEDIT:            // LIBEDIT and LIB_RENAME save a copy of the current item
-        case UR_LIB_RENAME:         //    so the picker is the owner of the picked item
+        else if( wrapper.GetStatus() == UR_DELETED )
+        {
+            // This should really be replaced with UR_TRANSIENT, but currently many clients
+            // (eeschema in particular) abuse this to achieve non-undo-related deletions.
             delete wrapper.GetItem();
-            break;
-
-        default:
-            wxFAIL_MSG( wxString::Format( wxT( "Cannot clear unknown undo/redo command %d" ),
-                                          wrapper.GetStatus() ) );
-            break;
         }
     }
 }

common/origin_viewitem.cpp

@@ -36,6 +36,15 @@ ORIGIN_VIEWITEM::ORIGIN_VIEWITEM( const COLOR4D& aColor, MARKER_STYLE aStyle, in
 }
 
 
+ORIGIN_VIEWITEM::ORIGIN_VIEWITEM( const VECTOR2D& aPosition, STATUS_FLAGS flags ) :
+    BOARD_ITEM( nullptr, NOT_USED ),   // this item is never added to a BOARD so it needs no type
+    m_size( NOT_USED ), m_color( UNSPECIFIED_COLOR ), m_style( NONE ), m_drawAtZero( false ),
+    m_position( aPosition )
+{
+    SetFlags( flags );
+}
+
+
 ORIGIN_VIEWITEM* ORIGIN_VIEWITEM::Clone() const
 {
     return new ORIGIN_VIEWITEM( m_color, m_style, m_size, m_position );

eeschema/block.cpp

@@ -454,11 +454,8 @@ void SCH_EDIT_FRAME::copyBlockItems( PICKED_ITEMS_LIST& aItemsList )
         /* Make a copy of the original picked item. */
         SCH_ITEM* copy = DuplicateStruct( (SCH_ITEM*) aItemsList.GetPickedItem( ii ) );
         copy->SetParent( NULL );
-
-        // In list the wrapper is owner of the schematic item, we can use the UR_DELETED
-        // status for the picker because pickers with this status are owner of the picked item
-        // (or TODO ?: create a new status like UR_DUPLICATE)
-        ITEM_PICKER item( copy, UR_DELETED );
+        copy->SetFlags( copy->GetFlags() | UR_TRANSIENT );
+        ITEM_PICKER item( copy, UR_NEW );
 
         m_blockItems.PushItem( item );
     }

eeschema/block_libedit.cpp

@@ -394,11 +394,8 @@ void LIB_EDIT_FRAME::copySelectedItems()
 
         // Do not clear the 'selected' flag. It is required to have items drawn when they are pasted.
         LIB_ITEM* copy = (LIB_ITEM*) item.Clone();
-
-        // In list the wrapper is owner of the schematic item, we can use the UR_DELETED
-        // status for the picker because pickers with this status are owner of the picked item
-        // (or TODO ?: create a new status like UR_DUPLICATE)
-        ITEM_PICKER picker( copy, UR_DELETED );
+        copy->SetFlags( copy->GetFlags() | UR_TRANSIENT );
+        ITEM_PICKER picker( copy, UR_NEW );
         m_clipboard.PushItem( picker );
     }
 }

eeschema/libedit_undo_redo.cpp

@@ -41,6 +41,7 @@ void LIB_EDIT_FRAME::SaveCopyInUndoList( EDA_ITEM* ItemToCopy, UNDO_REDO_T undoT
 
     // Clear current flags (which can be temporary set by a current edit command).
     CopyItem->ClearStatus();
+    CopyItem->SetFlags( CopyItem->GetFlags() | UR_TRANSIENT );
 
     ITEM_PICKER wrapper( CopyItem, undoType );
     lastcmd->PushItem( wrapper );
@@ -61,11 +62,13 @@ void LIB_EDIT_FRAME::GetComponentFromRedoList( wxCommandEvent& event )
     ITEM_PICKER redoWrapper = redoCommand->PopItem();
     delete redoCommand;
     LIB_PART* part = (LIB_PART*) redoWrapper.GetItem();
+    part->SetFlags( part->GetFlags() & ~UR_TRANSIENT );
     UNDO_REDO_T undoRedoType = redoWrapper.GetStatus();
 
     // Store the current part in the undo buffer
     PICKED_ITEMS_LIST* undoCommand = new PICKED_ITEMS_LIST();
     LIB_PART* oldPart = GetCurPart();
+    oldPart->SetFlags( oldPart->GetFlags() | UR_TRANSIENT );
     ITEM_PICKER undoWrapper( oldPart, undoRedoType );
     undoCommand->PushItem( undoWrapper );
     GetScreen()->PushCommandToUndoList( undoCommand );
@@ -112,11 +115,13 @@ void LIB_EDIT_FRAME::GetComponentFromUndoList( wxCommandEvent& event )
     ITEM_PICKER undoWrapper = undoCommand->PopItem();
     delete undoCommand;
     LIB_PART* part = (LIB_PART*) undoWrapper.GetItem();
+    part->SetFlags( part->GetFlags() & ~UR_TRANSIENT );
     UNDO_REDO_T undoRedoType = undoWrapper.GetStatus();
 
     // Store the current part in the redo buffer
     PICKED_ITEMS_LIST* redoCommand = new PICKED_ITEMS_LIST();
     LIB_PART* oldPart = GetCurPart();
+    oldPart->SetFlags( oldPart->GetFlags() | UR_TRANSIENT );
     ITEM_PICKER redoWrapper( oldPart, undoRedoType );
     redoCommand->PushItem( redoWrapper );
     GetScreen()->PushCommandToRedoList( redoCommand );

eeschema/operations_on_items_lists.cpp

@@ -173,12 +173,9 @@ void SCH_EDIT_FRAME::CheckListConnections( PICKED_ITEMS_LIST& aItemsList, bool a
 
 void SCH_EDIT_FRAME::DeleteItemsInList( PICKED_ITEMS_LIST& aItemsList, bool aAppend )
 {
-    PICKED_ITEMS_LIST  itemsList;
-
     for( unsigned ii = 0; ii < aItemsList.GetCount(); ii++ )
     {
         SCH_ITEM* item = (SCH_ITEM*) aItemsList.GetPickedItem( ii );
-        ITEM_PICKER itemWrapper( item, UR_DELETED );
 
         if( item->GetFlags() & STRUCT_DELETED )
             continue;

include/base_struct.h

@@ -151,7 +151,9 @@ typedef const INSPECTOR_FUNC& INSPECTOR;    /// std::function passed to nested u
 #define BRIGHTENED     (1 << 26)   ///< item is drawn with a bright contour
 
 #define DP_COUPLED     (1 << 27)   ///< item is coupled with another item making a differential pair
-                                  ///< (applies to segments only)
+                                   ///< (applies to segments only)
+#define UR_TRANSIENT   (1 << 28)   ///< indicates the item is owned by the undo/redo stack
+
 
 #define EDA_ITEM_ALL_FLAGS -1
 

include/origin_viewitem.h

@@ -48,6 +48,8 @@ public:
                      MARKER_STYLE aStyle = CIRCLE_X, int aSize = 16,
                      const VECTOR2D& aPosition = VECTOR2D( 0, 0 ) );
 
+    ORIGIN_VIEWITEM( const VECTOR2D& aPosition, STATUS_FLAGS flags );
+
     ORIGIN_VIEWITEM* Clone() const override;
 
     const BOX2I ViewBBox() const override;

pcbnew/hotkeys_board_editor.cpp

@@ -37,12 +37,14 @@
 #include <class_pcb_text.h>
 #include <class_pcb_target.h>
 #include <class_drawsegment.h>
+#include <origin_viewitem.h>
 
 #include <pcbnew.h>
 #include <pcbnew_id.h>
 #include <hotkeys.h>
 #include <class_zone.h>
 #include <tool/tool_manager.h>
+#include <tools/pcbnew_control.h>
 
 /* How to add a new hotkey:
  * see hotkeys.cpp
@@ -262,14 +264,16 @@ bool PCB_EDIT_FRAME::OnHotKey( wxDC* aDC, int aHotkeyCode, const wxPoint& aPosit
         break;
 
     case HK_SET_GRID_ORIGIN:
-        SetGridOrigin( GetCrossHairPosition() );
-        OnModify();     // because grid origin is saved in board, show as modified
+        PCBNEW_CONTROL::SetGridOrigin( GetGalCanvas()->GetView(), this,
+                                       new KIGFX::ORIGIN_VIEWITEM( GetGridOrigin(), UR_TRANSIENT ),
+                                       GetCrossHairPosition() );
         m_canvas->Refresh();
         break;
 
     case HK_RESET_GRID_ORIGIN:
-        SetGridOrigin( wxPoint( 0,0 ) );
-        OnModify();     // because grid origin is saved in board, show as modified
+        PCBNEW_CONTROL::SetGridOrigin( GetGalCanvas()->GetView(), this,
+                                       new KIGFX::ORIGIN_VIEWITEM( GetGridOrigin(), UR_TRANSIENT ),
+                                       wxPoint( 0, 0 ) );
         m_canvas->Refresh();
         break;
 

pcbnew/hotkeys_module_editor.cpp

@@ -34,6 +34,8 @@
 #include <class_drawpanel.h>
 #include <confirm.h>
 #include <class_board_design_settings.h>
+#include <origin_viewitem.h>
+#include <tools/pcbnew_control.h>
 
 #include <hotkeys.h>
 
@@ -101,12 +103,16 @@ bool FOOTPRINT_EDIT_FRAME::OnHotKey( wxDC* aDC, int aHotKey, const wxPoint& aPos
         break;
 
     case HK_SET_GRID_ORIGIN:
-        SetGridOrigin( GetCrossHairPosition() );
+        PCBNEW_CONTROL::SetGridOrigin( GetGalCanvas()->GetView(), this,
+                                       new KIGFX::ORIGIN_VIEWITEM( GetGridOrigin(), UR_TRANSIENT ),
+                                       GetCrossHairPosition() );
         m_canvas->Refresh();
         break;
 
     case HK_RESET_GRID_ORIGIN:
-        SetGridOrigin( wxPoint(0,0) );
+        PCBNEW_CONTROL::SetGridOrigin( GetGalCanvas()->GetView(), this,
+                                       new KIGFX::ORIGIN_VIEWITEM( GetGridOrigin(), UR_TRANSIENT ),
+                                       wxPoint( 0, 0 ) );
         m_canvas->Refresh();
         break;
 

pcbnew/modedit_onclick.cpp

@@ -35,9 +35,11 @@
 #include <class_board.h>
 #include <class_module.h>
 #include <class_edge_mod.h>
+#include <origin_viewitem.h>
 
 #include <pcbnew.h>
 #include <pcbnew_id.h>
+#include <tools/pcbnew_control.h>
 #include <hotkeys.h>
 #include <module_editor_frame.h>
 #include <dialog_edit_module_for_Modedit.h>
@@ -177,10 +179,10 @@ void FOOTPRINT_EDIT_FRAME::OnLeftClick( wxDC* DC, const wxPoint& MousePos )
         break;
 
     case ID_MODEDIT_PLACE_GRID_COORD:
-        m_canvas->DrawGridAxis( DC, GR_XOR, GetBoard()->GetGridOrigin() );
-        SetGridOrigin( GetCrossHairPosition() );
-        m_canvas->DrawGridAxis( DC, GR_COPY, GetBoard()->GetGridOrigin() );
-        GetScreen()->SetModify();
+        PCBNEW_CONTROL::SetGridOrigin( GetGalCanvas()->GetView(), this,
+                                       new KIGFX::ORIGIN_VIEWITEM( GetBoard()->GetGridOrigin(), UR_TRANSIENT ),
+                                       GetCrossHairPosition() );
+        m_canvas->Refresh();
         break;
 
     case ID_MODEDIT_TEXT_TOOL:

pcbnew/onleftclick.cpp

@@ -42,11 +42,14 @@
 #include <class_text_mod.h>
 #include <class_module.h>
 #include <class_pcb_target.h>
+#include <origin_viewitem.h>
 #include <project.h>
 
 #include <pcbnew.h>
 #include <pcbnew_id.h>
 #include <menus_helpers.h>
+#include <tools/pcb_editor_control.h>
+#include <tools/pcbnew_control.h>
 
 
 /* Handle the left button mouse click, when a tool is active
@@ -427,17 +430,17 @@ void PCB_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
         break;
 
     case ID_PCB_PLACE_OFFSET_COORD_BUTT:
-        m_canvas->DrawAuxiliaryAxis( aDC, GR_XOR );
-        SetAuxOrigin( GetCrossHairPosition() );
-        m_canvas->DrawAuxiliaryAxis( aDC, GR_COPY );
-        OnModify();
+        PCB_EDITOR_CONTROL::SetDrillOrigin( GetGalCanvas()->GetView(), this,
+                                            new KIGFX::ORIGIN_VIEWITEM( GetAuxOrigin(), UR_TRANSIENT ),
+                                            GetCrossHairPosition() );
+        m_canvas->Refresh();
         break;
 
     case ID_PCB_PLACE_GRID_COORD_BUTT:
-        m_canvas->DrawGridAxis( aDC, GR_XOR, GetBoard()->GetGridOrigin() );
-        SetGridOrigin( GetCrossHairPosition() );
-        m_canvas->DrawGridAxis( aDC, GR_COPY, GetBoard()->GetGridOrigin() );
-        OnModify();
+        PCBNEW_CONTROL::SetGridOrigin( GetGalCanvas()->GetView(), this,
+                                       new KIGFX::ORIGIN_VIEWITEM( GetBoard()->GetGridOrigin(), UR_TRANSIENT ),
+                                       GetCrossHairPosition() );
+        m_canvas->Refresh();
         break;
 
     case ID_PCB_DRAW_VIA_BUTT:

pcbnew/tools/pcb_editor_control.cpp

@@ -837,7 +837,7 @@ bool PCB_EDITOR_CONTROL::DoSetDrillOrigin( KIGFX::VIEW* aView, PCB_BASE_FRAME* a
     aFrame->SetAuxOrigin( wxPoint( aPosition.x, aPosition.y ) );
     originViewItem->SetPosition( wxPoint( aPosition.x, aPosition.y ) );
     aView->MarkDirty();
-
+    aFrame->OnModify();
     return true;
 }
 
@@ -846,7 +846,7 @@ bool PCB_EDITOR_CONTROL::SetDrillOrigin( KIGFX::VIEW* aView, PCB_BASE_FRAME* aFr
                                          BOARD_ITEM* originViewItem, const VECTOR2D& aPosition )
 {
     aFrame->SaveCopyInUndoList( originViewItem, UR_DRILLORIGIN );
-    DoSetDrillOrigin( aView, aFrame, originViewItem, aPosition );
+    return DoSetDrillOrigin( aView, aFrame, originViewItem, aPosition );
 }
 
 

pcbnew/tools/pcbnew_control.cpp

@@ -604,7 +604,7 @@ bool PCBNEW_CONTROL::DoSetGridOrigin( KIGFX::VIEW* aView, PCB_BASE_FRAME* aFrame
     aView->GetGAL()->SetGridOrigin( aPoint );
     originViewItem->SetPosition( wxPoint( aPoint.x, aPoint.y ) );
     aView->MarkDirty();
-
+    aFrame->OnModify();
     return true;
 }