Fix Bug #1426675 (Undo and cancel commands don't have effect for Duplicate operation in legacy mode)

common/draw_frame.cpp

@@ -749,6 +749,7 @@ bool EDA_DRAW_FRAME::HandleBlockBegin( wxDC* aDC, int aKey, const wxPoint& aPosi
     case BLOCK_DRAG:                // Drag (block defined)
     case BLOCK_DRAG_ITEM:           // Drag from a drag item command
     case BLOCK_COPY:                // Copy
+    case BLOCK_COPY_AND_INCREMENT:  // Copy and increment relevant references
     case BLOCK_DELETE:              // Delete
     case BLOCK_SAVE:                // Save
     case BLOCK_ROTATE:              // Rotate 90 deg

eeschema/block_libedit.cpp

@@ -198,7 +198,8 @@ bool LIB_EDIT_FRAME::HandleBlockEnd( wxDC* DC )
     case BLOCK_SELECT_ITEMS_ONLY:
         break;
 
-    case BLOCK_MOVE_EXACT:
+    case BLOCK_COPY_AND_INCREMENT:      // not used in Eeschema
+    case BLOCK_MOVE_EXACT:              // not used in Eeschema
         break;
     }
 

include/block_commande.h

@@ -51,6 +51,7 @@ typedef enum {
     BLOCK_IDLE,
     BLOCK_MOVE,
     BLOCK_COPY,
+    BLOCK_COPY_AND_INCREMENT,
     BLOCK_SAVE,
     BLOCK_DELETE,
     BLOCK_PASTE,

include/class_board_item.h

@@ -314,6 +314,14 @@ public:
 
     /// @copydoc VIEW_ITEM::ViewGetLayers()
     virtual void ViewGetLayers( int aLayers[], int& aCount ) const;
+
+    /*!
+     * Function IncrementItemReference
+     * Implement if the concept of "incrementing" makes sense for an
+     * item (e.g. modules and pads)
+     * @return if item reference was incremented
+     */
+    virtual bool IncrementItemReference() { return false; }
 };
 
 #endif /* BOARD_ITEM_STRUCT_H */

include/wxPcbStruct.h

@@ -206,7 +206,7 @@ protected:
      * Duplicate selected item if possible and start a move
      * @param aIncrement increment the item number if appropriate
      */
-    void duplicateItem( bool aIncrement );
+    void duplicateItems( bool aIncrement ); //override
 
     // protected so that PCB::IFACE::CreateWindow() is the only factory.
     PCB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent );
@@ -794,7 +794,7 @@ public:
      * New location is determined by the current offset from the selected
      * block's original location.
      */
-    void Block_Duplicate();
+    void Block_Duplicate( bool aIncrement );
 
     void Process_Settings( wxCommandEvent& event );
     void OnConfigurePcbOptions( wxCommandEvent& aEvent );

pcbnew/block.cpp

@@ -228,7 +228,9 @@ void PCB_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
 
     GetScreen()->m_BlockLocate.SetState( STATE_BLOCK_STOP );
 
-    switch( GetScreen()->m_BlockLocate.GetCommand() )
+    const BLOCK_COMMAND_T command = GetScreen()->m_BlockLocate.GetCommand();
+
+    switch( command )
     {
     case BLOCK_IDLE:
         break;
@@ -244,10 +246,11 @@ void PCB_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
         break;
 
     case BLOCK_COPY:     // Copy
+    case BLOCK_COPY_AND_INCREMENT:
         if( m_canvas->IsMouseCaptured() )
             m_canvas->CallMouseCapture( DC, wxDefaultPosition, false );
 
-        Block_Duplicate();
+        Block_Duplicate( command == BLOCK_COPY_AND_INCREMENT );
         GetScreen()->m_BlockLocate.ClearItemsList();
         break;
 
@@ -315,10 +318,11 @@ bool PCB_EDIT_FRAME::HandleBlockEnd( wxDC* DC )
             DisplayError( this, wxT( "Error in HandleBlockPLace" ) );
             break;
 
-        case BLOCK_DRAG:            // Drag (not used, for future enhancements)
+        case BLOCK_DRAG:                // Drag (not used, for future enhancements)
-        case BLOCK_MOVE:            // Move
+        case BLOCK_MOVE:                // Move
-        case BLOCK_COPY:            // Copy
+        case BLOCK_COPY:                // Copy
-        case BLOCK_PRESELECT_MOVE:  // Move with preselection list
+        case BLOCK_COPY_AND_INCREMENT:  // Copy and increment relevant references
+        case BLOCK_PRESELECT_MOVE:      // Move with preselection list
             GetScreen()->m_BlockLocate.SetState( STATE_BLOCK_MOVE );
             nextcmd = true;
             m_canvas->SetMouseCaptureCallback( drawMovingBlock );
@@ -845,7 +849,7 @@ void PCB_EDIT_FRAME::Block_Move()
 }
 
 
-void PCB_EDIT_FRAME::Block_Duplicate()
+void PCB_EDIT_FRAME::Block_Duplicate( bool aIncrement )
 {
     wxPoint MoveVector = GetScreen()->m_BlockLocate.GetMoveVector();
 
@@ -865,6 +869,9 @@ void PCB_EDIT_FRAME::Block_Duplicate()
 
         newitem = (BOARD_ITEM*)item->Clone();
 
+        if( aIncrement )
+            newitem->IncrementItemReference();
+
         if( item->Type() == PCB_MODULE_T )
             m_Pcb->m_Status_Pcb = 0;
 

pcbnew/block_module_editor.cpp

@@ -73,7 +73,7 @@ static void DrawMovingBlockOutlines( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wx
 static int  MarkItemsInBloc( MODULE* module, EDA_RECT& Rect );
 
 static void ClearMarkItems( MODULE* module );
-static void CopyMarkedItems( MODULE* module, wxPoint offset );
+static void CopyMarkedItems( MODULE* module, wxPoint offset, bool aIncrement );
 static void MoveMarkedItems( MODULE* module, wxPoint offset );
 static void DeleteMarkedItems( MODULE* module );
 
@@ -129,17 +129,18 @@ bool FOOTPRINT_EDIT_FRAME::HandleBlockEnd( wxDC* DC )
 
     if( GetScreen()->m_BlockLocate.GetCount() )
     {
-        BLOCK_STATE_T   state   = GetScreen()->m_BlockLocate.GetState();
+        // Set the SELECTED flag of all preselected items, and clear preselect list
-        BLOCK_COMMAND_T command = GetScreen()->m_BlockLocate.GetCommand();
+        ClearMarkItems( currentModule );
+        PICKED_ITEMS_LIST* list = &GetScreen()->m_BlockLocate.GetItems();
 
-        m_canvas->CallEndMouseCapture( DC );
+        for( unsigned ii = 0, e = list->GetCount(); ii < e; ++ii )
-        GetScreen()->m_BlockLocate.SetState( state );
+        {
-        GetScreen()->m_BlockLocate.SetCommand( command );
+            BOARD_ITEM* item = (BOARD_ITEM*) list->GetPickedItem( ii );
-        m_canvas->SetMouseCapture( DrawAndSizingBlockOutlines, AbortBlockCurrentCommand );
+            item->SetFlags( SELECTED );
+            ++itemsCount;
+        }
 
-        SetCrossHairPosition( wxPoint(  GetScreen()->m_BlockLocate.GetRight(),
+        GetScreen()->m_BlockLocate.ClearItemsList();
-                                        GetScreen()->m_BlockLocate.GetBottom() ) );
-        m_canvas->MoveCursorToCrossHair();
     }
 
     switch( GetScreen()->m_BlockLocate.GetCommand() )
@@ -148,11 +149,15 @@ bool FOOTPRINT_EDIT_FRAME::HandleBlockEnd( wxDC* DC )
         DisplayError( this, wxT( "Error in HandleBlockPLace" ) );
         break;
 
-    case BLOCK_DRAG:        // Drag
+    case BLOCK_DRAG:                // Drag
-    case BLOCK_DRAG_ITEM:   // Drag a given item (not used here)
+    case BLOCK_DRAG_ITEM:           // Drag a given item (not used here)
-    case BLOCK_MOVE:        // Move
+    case BLOCK_MOVE:                // Move
-    case BLOCK_COPY:        // Copy
+    case BLOCK_COPY:                // Copy
-        itemsCount = MarkItemsInBloc( currentModule, GetScreen()->m_BlockLocate );
+    case BLOCK_COPY_AND_INCREMENT:  // Specific to duplicate with increment command
+
+        // Find selected items if we didn't already set them manually
+        if( itemsCount == 0 )
+            itemsCount = MarkItemsInBloc( currentModule, GetScreen()->m_BlockLocate );
 
         if( itemsCount )
         {
@@ -270,7 +275,9 @@ void FOOTPRINT_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
 
     GetScreen()->m_BlockLocate.SetState( STATE_BLOCK_STOP );
 
-    switch( GetScreen()->m_BlockLocate.GetCommand() )
+    const BLOCK_COMMAND_T command = GetScreen()->m_BlockLocate.GetCommand();
+
+    switch( command )
     {
     case  BLOCK_IDLE:
         break;
@@ -284,10 +291,12 @@ void FOOTPRINT_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
         m_canvas->Refresh( true );
         break;
 
-    case BLOCK_COPY:     // Copy
+    case BLOCK_COPY:                // Copy
+    case BLOCK_COPY_AND_INCREMENT:  // Copy and increment references
         GetScreen()->m_BlockLocate.ClearItemsList();
         SaveCopyInUndoList( currentModule, UR_MODEDIT );
-        CopyMarkedItems( currentModule, GetScreen()->m_BlockLocate.GetMoveVector() );
+        CopyMarkedItems( currentModule, GetScreen()->m_BlockLocate.GetMoveVector(),
+                         command == BLOCK_COPY_AND_INCREMENT );
         break;
 
     case BLOCK_PASTE:     // Paste
@@ -419,7 +428,7 @@ static void DrawMovingBlockOutlines( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wx
 
 /* Copy marked items, at new position = old position + offset
  */
-void CopyMarkedItems( MODULE* module, wxPoint offset )
+void CopyMarkedItems( MODULE* module, wxPoint offset, bool aIncrement )
 {
     if( module == NULL )
         return;
@@ -439,6 +448,9 @@ void CopyMarkedItems( MODULE* module, wxPoint offset )
         NewPad->SetParent( module );
         NewPad->SetFlags( SELECTED );
         module->Pads().PushFront( NewPad );
+
+        if( aIncrement )
+            NewPad->IncrementItemReference();
     }
 
     BOARD_ITEM* newItem;
@@ -454,6 +466,9 @@ void CopyMarkedItems( MODULE* module, wxPoint offset )
         newItem->SetParent( module );
         newItem->SetFlags( SELECTED );
         module->GraphicalItems().PushFront( newItem );
+
+        if( aIncrement )
+            newItem->IncrementItemReference();
     }
 
     MoveMarkedItems( module, offset );

pcbnew/class_board.cpp

@@ -2625,13 +2625,6 @@ BOARD_ITEM* BOARD::DuplicateAndAddItem( const BOARD_ITEM* aItem,
     case PCB_MODULE_T:
     {
         MODULE* new_module = new MODULE( *static_cast<const MODULE*>( aItem ) );
-
-        if( aIncrementReferences )
-        {
-            // Take the next available module number
-            new_module->IncrementReference( true );
-        }
-
         new_item = new_module;
         break;
     }
@@ -2653,7 +2646,12 @@ BOARD_ITEM* BOARD::DuplicateAndAddItem( const BOARD_ITEM* aItem,
     }
 
     if( new_item )
+    {
+        if( aIncrementReferences )
+            new_item->IncrementItemReference();
+
         Add( new_item );
+    }
 
     return new_item;
 }

pcbnew/class_module.cpp

@@ -1132,12 +1132,6 @@ BOARD_ITEM* MODULE::DuplicateAndAddItem( const BOARD_ITEM* aItem,
     {
         D_PAD* new_pad = new D_PAD( *static_cast<const D_PAD*>( aItem ) );
 
-        if( aIncrementPadNumbers )
-        {
-            // Take the next available pad number
-            new_pad->IncrementPadName( true, true );
-        }
-
         Pads().PushBack( new_pad );
         new_item = new_pad;
         break;
@@ -1177,6 +1171,11 @@ BOARD_ITEM* MODULE::DuplicateAndAddItem( const BOARD_ITEM* aItem,
         break;
     }
 
+    if( aIncrementPadNumbers && new_item )
+    {
+        new_item->IncrementItemReference();
+    }
+
     return new_item;
 }
 
@@ -1220,6 +1219,13 @@ wxString MODULE::GetReferencePrefix() const
 }
 
 
+bool MODULE::IncrementItemReference()
+{
+    // Take the next available module number
+    return IncrementReference( true );
+}
+
+
 bool MODULE::IncrementReference( bool aFillSequenceGaps )
 {
     BOARD* board = GetBoard();

pcbnew/class_module.h

@@ -436,8 +436,15 @@ public:
     TEXTE_MODULE& Value() const       { return *m_Value; }
     TEXTE_MODULE& Reference() const   { return *m_Reference; }
 
+    /*!
+     * Function IncrementItemReference
+     * Implementation of the generic "reference" incrementing interface
+     * Increments the numeric suffix, filling any sequence gaps
+     */
+    bool IncrementItemReference(); //override
+
     /**
-     * Function INcrementReference
+     * Function IncrementReference
      * Increments the module's reference, if possible. A reference with
      * a numerical suffix and an optional alphabetical prefix can be
      * incremented: "A1" and "1" can be, "B" can't.

pcbnew/class_pad.cpp

@@ -407,12 +407,21 @@ void D_PAD::SetPadName( const wxString& name )
 }
 
 
-void D_PAD::IncrementPadName( bool aSkipUnconnectable, bool aFillSequenceGaps )
+bool D_PAD::IncrementItemReference()
+{
+    // Take the next available pad number
+    return IncrementPadName( true, true );
+}
+
+
+bool D_PAD::IncrementPadName( bool aSkipUnconnectable, bool aFillSequenceGaps )
 {
     bool skip = aSkipUnconnectable && ( GetAttribute() == PAD_HOLE_NOT_PLATED );
 
     if( !skip )
         SetPadName( GetParent()->GetNextPadName( aFillSequenceGaps ) );
+
+    return !skip;
 }
 
 

pcbnew/class_pad.h

@@ -108,14 +108,23 @@ public:
     void SetPadName( const wxString& name );    // Change pad name
     const wxString GetPadName() const;
 
+    /*!
+     * Function IncrementItemReference
+     * Implementation of the generic "reference" incrementing interface
+     * Increments the numeric suffix, filling any sequence gaps and skipping
+     * pads that aren't connectable
+     */
+    bool IncrementItemReference(); // override
+
     /**
      * Function IncrementPadName
      *
      * Increments the pad name to the next available name in the module.
      *
      * @param aSkipUnconnectable skips any pads that are not connectable (for example NPTH)
+     * @return pad name incremented
      */
-    void IncrementPadName( bool aSkipUnconnectable, bool aFillSequenceGaps );
+    bool IncrementPadName( bool aSkipUnconnectable, bool aFillSequenceGaps );
 
     bool PadNameEqual( const D_PAD* other ) const
     {

pcbnew/edit.cpp

@@ -1202,7 +1202,7 @@ void PCB_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
 
     case ID_POPUP_PCB_DUPLICATE_ITEM:
     case ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT:
-        duplicateItem( id == ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT );
+        duplicateItems( id == ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT );
         break;
 
     case ID_POPUP_PCB_CREATE_ARRAY:
@@ -1519,84 +1519,75 @@ void PCB_EDIT_FRAME::moveExact()
 
     if( ret == wxID_OK )
     {
-        BOARD_ITEM* item = GetScreen()->GetCurItem();
+        if( BOARD_ITEM* item = GetScreen()->GetCurItem() )
+        {
+            // When a pad is modified, the full footprint is saved
+            BOARD_ITEM* itemToSave = item;
 
-        // When a pad is modified, the full footprint is saved
+            if( item->Type() == PCB_PAD_T )
-        BOARD_ITEM* itemToSave = item;
+                itemToSave = item->GetParent();
 
-        if( item->Type() == PCB_PAD_T )
+            // Could be moved or rotated
-            itemToSave = item->GetParent();
+            SaveCopyInUndoList( itemToSave, UR_CHANGED );
 
-        // Could be moved or rotated
+            item->Move( translation );
-        SaveCopyInUndoList( itemToSave, UR_CHANGED );
+            item->Rotate( item->GetPosition(), rotation );
-
+            m_canvas->Refresh();
-        item->Move( translation );
+        }
-        item->Rotate( item->GetPosition(), rotation );
-        m_canvas->Refresh();
     }
 
     m_canvas->MoveCursorToCrossHair();
 }
 
-
+void PCB_EDIT_FRAME::duplicateItems( bool aIncrement )
-void PCB_EDIT_FRAME::duplicateItem( bool aIncrement )
 {
     BOARD_ITEM* item = GetScreen()->GetCurItem();
-    bool editingModule = NULL != dynamic_cast<FOOTPRINT_EDIT_FRAME*>( this );
-    int move_cmd = 0;
 
-    if( item->Type() == PCB_PAD_T && !editingModule )
+    if( !item )
-    {
+        return;
-        // If it is not the module editor, then duplicate the parent module instead
+
+    // In the board editor, the pads or fp texts can be edited
+    // but cannot be duplicated (only the fp editor can do that).
+    // only the footprint can be duplicated
+    if( item->Type() == PCB_PAD_T || item->Type() == PCB_MODULE_TEXT_T )
         item = static_cast<MODULE*>( item )->GetParent();
-    }
 
-    BOARD_ITEM* new_item = GetBoard()->DuplicateAndAddItem( item, aIncrement );
+    PCB_BASE_EDIT_FRAME::duplicateItem( item, aIncrement );
+}
 
-    SaveCopyInUndoList( new_item, UR_NEW );
+void PCB_BASE_EDIT_FRAME::duplicateItem( BOARD_ITEM* aItem, bool aIncrement )
+{
+    if( !aItem )
+        return;
 
-    if( new_item )
+    // The easiest way to handle a duplicate item command
+    // is to simulate a block copy command, which gives us the undo management
+    // for free
+    if( GetScreen()->m_BlockLocate.GetState() == STATE_NO_BLOCK )
     {
-        switch( new_item->Type() )
+        m_canvas->MoveCursorToCrossHair();
-        {
-        case PCB_MODULE_T:
-            move_cmd = ID_POPUP_PCB_MOVE_MODULE_REQUEST;
-            break;
-        case PCB_TEXT_T:
-            move_cmd = ID_POPUP_PCB_MOVE_TEXTEPCB_REQUEST;
-            break;
-        case PCB_LINE_T:
-            move_cmd = ID_POPUP_PCB_MOVE_DRAWING_REQUEST;
-            break;
-        case PCB_ZONE_AREA_T:
-            move_cmd = ID_POPUP_PCB_MOVE_ZONE_OUTLINES;
-            break;
-        case PCB_TRACE_T:
-            move_cmd = ID_POPUP_PCB_MOVE_TRACK_SEGMENT;
-            break;
-        case PCB_TARGET_T:
-            move_cmd = ID_POPUP_PCB_MOVE_MIRE_REQUEST;
-            break;
-        case PCB_DIMENSION_T:
-            move_cmd = ID_POPUP_PCB_MOVE_TEXT_DIMENSION_REQUEST;
-            break;
-        case PCB_PAD_T:
-            move_cmd = ID_POPUP_PCB_MOVE_PAD_REQUEST;
-            break;
-        default:
-            break;
-        }
 
-        if( move_cmd )
+        INSTALL_UNBUFFERED_DC( dc, m_canvas );
-        {
-            SetMsgPanel( new_item );
-            SetCurItem( new_item );
 
-            m_canvas->MoveCursorToCrossHair();
+        wxPoint crossHairPos = GetCrossHairPosition();
 
-            // pick up the item and start moving
+        const BLOCK_COMMAND_T blockType = aIncrement ? BLOCK_COPY_AND_INCREMENT : BLOCK_COPY;
-            PostCommandMenuEvent( move_cmd );
+
-        }
+        if( !HandleBlockBegin( &dc, blockType, crossHairPos ) )
+            return;
+
+        // Add the item to the block copy pick list:
+        PICKED_ITEMS_LIST& itemsList = GetScreen()->m_BlockLocate.GetItems();
+        ITEM_PICKER        picker( NULL, UR_UNSPECIFIED );
+
+        picker.SetItem ( aItem );
+        itemsList.PushItem( picker );
+
+        // Set 2 coordinates updated by the mouse, because our simulation
+        // does not use the mouse to call HandleBlockEnd()
+        GetScreen()->m_BlockLocate.SetLastCursorPosition( crossHairPos );
+        GetScreen()->m_BlockLocate.SetEnd( crossHairPos );
+        HandleBlockEnd( &dc );
     }
 }
 

pcbnew/modedit.cpp

@@ -869,44 +869,9 @@ void FOOTPRINT_EDIT_FRAME::moveExact()
 
 void FOOTPRINT_EDIT_FRAME::duplicateItems( bool aIncrement )
 {
-    SaveCopyInUndoList( GetBoard()->m_Modules, UR_MODEDIT );
-
     BOARD_ITEM* item = GetScreen()->GetCurItem();
-    MODULE* module = static_cast<MODULE*>( item->GetParent() );
 
-    int move_cmd = 0;
+    PCB_BASE_EDIT_FRAME::duplicateItem( item, aIncrement );
-
-    BOARD_ITEM* new_item = module->DuplicateAndAddItem(
-            item, aIncrement );
-
-    if( new_item )
-    {
-        switch( new_item->Type() )
-        {
-        case PCB_PAD_T:
-            move_cmd = ID_POPUP_PCB_MOVE_PAD_REQUEST;
-            break;
-        case PCB_MODULE_TEXT_T:
-            move_cmd = ID_POPUP_PCB_MOVE_TEXTMODULE_REQUEST;
-            break;
-        case PCB_MODULE_EDGE_T:
-            move_cmd = ID_POPUP_PCB_MOVE_EDGE;
-            break;
-        default:
-            break;
-        }
-
-        if( move_cmd )
-        {
-            SetMsgPanel( new_item );
-            SetCurItem( new_item );
-
-            m_canvas->MoveCursorToCrossHair();
-
-            // pick up the item and start moving
-            PostCommandMenuEvent( move_cmd );
-        }
-    }
 }
 
 

pcbnew/module_editor_frame.h

@@ -65,7 +65,7 @@ public:
 
     /**
      * Function GetConfigurationSettings
-     * returns the footprçint editor settings list.
+     * returns the footpr<EFBFBD>int editor settings list.
      *
      * Currently, only the settings that are needed at start
      * up by the main window are defined here.  There are other locally used
@@ -549,7 +549,7 @@ private:
      * Duplicate the item under the cursor
      * @param aIncrement increment the number of pad (if that is what is selected)
      */
-    void duplicateItems( bool aIncrement );
+    void duplicateItems( bool aIncrement ); //override
 };
 
 #endif      // MODULE_EDITOR_FRAME_H_

pcbnew/pcb_base_edit_frame.h

@@ -82,6 +82,29 @@ protected:
      * the same
      */
     void createArray();
+
+    /**
+     * Function duplicateItem
+     * Duplicate the specified item
+     * This function is shared between pcbnew and modedit, as it is virtually
+     * the same
+     * @param aItem the item to duplicate
+     * @aIncrement increment item reference (module ref, pad number, etc,
+     * if appropriate)
+     */
+    void duplicateItem( BOARD_ITEM* aItem, bool aIncrement );
+
+    /**
+     * Function duplicateItems
+     * Find and duplicate the currently selected items
+     * @param aIncrement increment item reference (module ref, pad number, etc,
+     * if appropriate)
+     *
+     * @note The implementer should find the selected item (and do processing
+     * like finding parents when relevant, and then call
+     * duplicateItem(BOARD_ITEM*, bool) above
+     */
+    virtual void duplicateItems( bool aIncrement ) = 0;
 };
 
 #endif