← Back to team overview

kicad-developers team mailing list archive

Re: Add a cmake option for legacy canvas

 

Here is an example of the kind of commit that would remove a feature
from the legacy canvas: Duplicate Item.

This tool doesn't have a lot of code behind it because it mostly
piggy-backs on other block operations, so this patch is a fairly good
example of the overhead and wiring that can be removed. The rest of
the block code is also very complex but most of it is also used for
the copy/paste operation too, so can't be removed at this time.

Nevertheless, this removes over 200 lines of code and touches 18 files
including very pervasive headers, which illustrates the complexity of
legacy tools. The GAL equivalent is around 70 lines of code, contained
in a single file and a few lines for declaring the tool actions,
without touching the "core" headers.

I haven't removed any icons, as that would bloat the diff and make it
less clear for illustrative purposes.

Basic aspects:

* Removal of virtual and non-virtual functions from PCB_BASE_EDIT_FRAME
* Removal of overrides from each of FOOTPRINT_EDIT_FRAME and PCB_EDIT_FRAME
* Removal of legacy hotkey and handlers
* Removal of tool from legacy menus
* Removal of duplicate-specific code in the block operation

The primary "win" here is the removal of interfaces from the *FRAME
classes. The secondary win is the (very minor in this case)
simplification of backend code (block.cpp) which would help future
removal efforts by decreasing complexity. Tertiary win is saving on
code size, compile/link times and bug surface, but because this tool
doesn't have a lot of backend of it's own, it's not a significant
factor.

Note: I am not submitting this patch in the expectation that it be
merged, it's an example (I wouldn't stop anyone though!).

Cheers,

John

On Sun, Feb 5, 2017 at 1:25 PM, John Beard <john.j.beard@xxxxxxxxx> wrote:
> Hi,
>
> I support the general idea of being able to turn off bits of the
> legacy canvas at will, in principle. However, I'm not sure how many
> people will actually bother to check if stuff behind an #ifdef is
> getting broken, and legacy stuff is quite vulnerable because of tight
> binding to large classes like PCB_EDIT_FRAME, etc. I think if you had
> this option on, you'd most get one of these options: 1) no-one uses it
> 2) everyone who does use it has to compile and test everything twice
> (leading to case 1) or 3) lots of people use it and don't fully check
> legacy works for every patch, and effort has to be expended
> maintaining and revising patches and requesting re-tests (this is
> already bad enough with platform-specific problems, we don't want to
> segment testing any further, I think).
>
> I would suggest that the legacy canvas remains as it is, but
> individual non-essential features of it that are available and solid
> in GAL get removed piece by piece. This way you get everyone and
> compiling and testing and _using_ the same code, and no-one loses a
> feature for want of a re-compile (they might need to switch canvases
> though). Additionally, legacy code complexity is drained down slowly
> and deliberately, a logically separate piece at a time, which helps
> locate refactoring opportunities and indentify legacy switch-off
> problem points sooner rather than later. Waiting for GAL-Day and
> nuking legacy fast would be more likely (IMO) to result in breakages
> and less careful consideration of how each separate piece can be
> tidied away into a GAL-only framework.
>
> Examples of "non-essential" features might be things like net
> highlighting, arrays tools and so on, and could even extend to things
> like the drawing tool and copy/paste over time, leaving only a hard
> core of very basic moving/view change functions and any remaining
> non-GAL features, which get drawn off over time. Any major breakage
> during this time (e.g. if the legacy track tool is removed and PNS
> breaks) is breakage that probably would still have happened during a
> firesale removal of legacy, but it will happen under controlled and
> more easily-revertable circumstances. The remaining "core" of legacy
> functionality, by the end of the process, should be dramatically
> smaller and more easily conceptualised, making it easier to finally
> remove it entirely.
>
> I'm not proposing a timeline, but if legacy is to be part of a stable
> release again, I would suggest that fewer features in it would drive
> use and testing of GAL tools, reduce user (especially newly-arrived
> Eagle users!) frustration with using legacy tools, and reduce bug
> surface area though reduction of legacy code volume, which is
> substantial, complex, and ultimately doomed anyway. It would also free
> code that is currently used by both GAL and legacy (e.g. many dialogs)
> to be able to evolve more freely when only the GAL calling code needs
> to be updated to support it, enabling faster response to bugs and
> feature requests on both stable and devel branches. It would also
> promote a lot of careful checking of legacy/GAL crossover code and
> refactorings, so it would hopefully be a good vehicle for code quality
> discussion too.
>
> I hope that makes as much sense as it did in my head!
>
> Cheers,
>
> John
>
> On Tue, Jan 31, 2017 at 11:22 PM, Chris Pavlina <pavlina.chris@xxxxxxxxx> wrote:
>> I think it's worth revisiting this. I know we're not ready to remove
>> legacy yet, but we're getting close. Starting to factor it out into a
>> switchable build option is a good way to make sure the transition is
>> smooth and help find anything that is still missing. Obviously the
>> default build option should be to keep legacy in, so users won't notice
>> anything.
>>
>> On Sat, Sep 17, 2016 at 10:59:45PM +1200, Simon Wells wrote:
>>> As legacy canvas in pcbnew is legacy is it worth conditional compiling
>>> all the code related and only used by legacy canvas based on a cmake
>>> option aka something like
>>>
>>> KICAD_BUILD_LEGACY_CANVAS with a default of ON, this will allow people
>>> who have no use for the legacy canvas as they have a truly functional
>>> opengl canvas to see in their usual workflow if anything is missing.
>>>
>>> I realise that wayne is waiting on a replacement non-gl dependent GAL
>>> canvas before removing legacy, but in the interim is it worth making
>>> it an option making less work later on when its time to remove it
>>>
>>> Simon
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~kicad-developers
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
From f15ef940f9754089fda498c87a9e58a35cbd323b Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Tue, 7 Feb 2017 09:19:54 +0800
Subject: [PATCH] Remove Duplicate Item tool from legacy canvas

---
 common/draw_frame.cpp            |  1 -
 eeschema/block_libedit.cpp       |  1 -
 include/block_commande.h         |  1 -
 include/wxPcbStruct.h            | 11 ++------
 pcbnew/block.cpp                 |  6 ++--
 pcbnew/block_module_editor.cpp   | 12 ++------
 pcbnew/edit.cpp                  | 59 ----------------------------------------
 pcbnew/hotkeys.cpp               |  7 ++---
 pcbnew/hotkeys.h                 |  2 --
 pcbnew/hotkeys_board_editor.cpp  | 16 ++---------
 pcbnew/hotkeys_module_editor.cpp | 34 -----------------------
 pcbnew/modedit.cpp               | 16 -----------
 pcbnew/modedit_onclick.cpp       | 11 --------
 pcbnew/module_editor_frame.h     |  8 ------
 pcbnew/onrightclick.cpp          | 25 -----------------
 pcbnew/pcb_base_edit_frame.h     | 23 ----------------
 pcbnew/pcbnew_id.h               |  2 --
 pcbnew/tools/common_actions.cpp  |  4 +--
 18 files changed, 13 insertions(+), 226 deletions(-)

diff --git a/common/draw_frame.cpp b/common/draw_frame.cpp
index b55f47b19..394456d99 100644
--- a/common/draw_frame.cpp
+++ b/common/draw_frame.cpp
@@ -828,7 +828,6 @@ bool EDA_DRAW_FRAME::HandleBlockBegin( wxDC* aDC, EDA_KEY aKey, const wxPoint& a
     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
diff --git a/eeschema/block_libedit.cpp b/eeschema/block_libedit.cpp
index 865d4e673..6f5d8d558 100644
--- a/eeschema/block_libedit.cpp
+++ b/eeschema/block_libedit.cpp
@@ -205,7 +205,6 @@ bool LIB_EDIT_FRAME::HandleBlockEnd( wxDC* DC )
     case BLOCK_SELECT_ITEMS_ONLY:
         break;
 
-    case BLOCK_COPY_AND_INCREMENT:      // not used in Eeschema
     case BLOCK_MOVE_EXACT:              // not used in Eeschema
         break;
     }
diff --git a/include/block_commande.h b/include/block_commande.h
index 8c7d5e866..8d2b94fa0 100644
--- a/include/block_commande.h
+++ b/include/block_commande.h
@@ -51,7 +51,6 @@ typedef enum {
     BLOCK_IDLE,
     BLOCK_MOVE,
     BLOCK_COPY,
-    BLOCK_COPY_AND_INCREMENT,
     BLOCK_SAVE,
     BLOCK_DELETE,
     BLOCK_PASTE,
diff --git a/include/wxPcbStruct.h b/include/wxPcbStruct.h
index 1058f6b2d..1af5e9289 100644
--- a/include/wxPcbStruct.h
+++ b/include/wxPcbStruct.h
@@ -219,13 +219,6 @@ protected:
      */
     void moveExact();
 
-    /**
-     * Function duplicateItems
-     * Duplicate selected item if possible and start a move
-     * @param aIncrement increment the item number if appropriate
-     */
-    void duplicateItems( bool aIncrement ) override;
-
     // protected so that PCB::IFACE::CreateWindow() is the only factory.
     PCB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent );
 
@@ -498,7 +491,7 @@ public:
      * @param aIdCommand = the hotkey command id
      * @return true if item duplicated or arrayed
      */
-    bool OnHotkeyDuplicateOrArrayItem( int aIdCommand );
+    bool OnHotkeyArrayItem( int aIdCommand );
 
     /**
      * Function OnHotkeyMoveItem
@@ -754,7 +747,7 @@ public:
      * New location is determined by the current offset from the selected
      * block's original location.
      */
-    void Block_Duplicate( bool aIncrement );
+    void Block_Duplicate();
 
     void Process_Settings( wxCommandEvent& event );
     void OnConfigurePcbOptions( wxCommandEvent& aEvent );
diff --git a/pcbnew/block.cpp b/pcbnew/block.cpp
index 1ef9e819f..f82edd2e1 100644
--- a/pcbnew/block.cpp
+++ b/pcbnew/block.cpp
@@ -246,11 +246,10 @@ 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( command == BLOCK_COPY_AND_INCREMENT );
+        Block_Duplicate();
         GetScreen()->m_BlockLocate.ClearItemsList();
         break;
 
@@ -322,7 +321,6 @@ bool PCB_EDIT_FRAME::HandleBlockEnd( wxDC* DC )
         case BLOCK_DRAG:                // Drag (not used, for future enhancements)
         case BLOCK_MOVE:                // Move
         case BLOCK_COPY:                // Copy
-        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;
@@ -846,7 +844,7 @@ void PCB_EDIT_FRAME::Block_Move()
 }
 
 
-void PCB_EDIT_FRAME::Block_Duplicate( bool aIncrement )
+void PCB_EDIT_FRAME::Block_Duplicate()
 {
     wxPoint MoveVector = GetScreen()->m_BlockLocate.GetMoveVector();
 
diff --git a/pcbnew/block_module_editor.cpp b/pcbnew/block_module_editor.cpp
index a34eb5842..dcdb7efad 100644
--- a/pcbnew/block_module_editor.cpp
+++ b/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, bool aIncrement );
+static void CopyMarkedItems( MODULE* module, wxPoint offset );
 static void MoveMarkedItems( MODULE* module, wxPoint offset );
 static void DeleteMarkedItems( MODULE* module );
 
@@ -160,7 +160,6 @@ bool FOOTPRINT_EDIT_FRAME::HandleBlockEnd( wxDC* DC )
     case BLOCK_DRAG_ITEM:           // Drag a given item (not used here)
     case BLOCK_MOVE:                // Move
     case BLOCK_COPY:                // Copy
-    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 )
@@ -299,11 +298,9 @@ void FOOTPRINT_EDIT_FRAME::HandleBlockPlace( wxDC* DC )
         break;
 
     case BLOCK_COPY:                // Copy
-    case BLOCK_COPY_AND_INCREMENT:  // Copy and increment pad names
         GetScreen()->m_BlockLocate.ClearItemsList();
         SaveCopyInUndoList( currentModule, UR_CHANGED );
-        CopyMarkedItems( currentModule, GetScreen()->m_BlockLocate.GetMoveVector(),
-                         command == BLOCK_COPY_AND_INCREMENT );
+        CopyMarkedItems( currentModule, GetScreen()->m_BlockLocate.GetMoveVector() );
         break;
 
     case BLOCK_PASTE:     // Paste
@@ -435,7 +432,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, bool aIncrement )
+void CopyMarkedItems( MODULE* module, wxPoint offset )
 {
     if( module == NULL )
         return;
@@ -455,9 +452,6 @@ void CopyMarkedItems( MODULE* module, wxPoint offset, bool aIncrement )
         NewPad->SetParent( module );
         NewPad->SetFlags( SELECTED );
         module->Pads().PushFront( NewPad );
-
-        if( aIncrement )
-            NewPad->IncrementPadName( true, true );
     }
 
     BOARD_ITEM* newItem;
diff --git a/pcbnew/edit.cpp b/pcbnew/edit.cpp
index 12ebcb3bc..9e85d8fd6 100644
--- a/pcbnew/edit.cpp
+++ b/pcbnew/edit.cpp
@@ -1216,11 +1216,6 @@ void PCB_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
         moveExact();
         break;
 
-    case ID_POPUP_PCB_DUPLICATE_ITEM:
-    case ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT:
-        duplicateItems( id == ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT );
-        break;
-
     case ID_POPUP_PCB_CREATE_ARRAY:
         createArray();
         break;
@@ -1560,60 +1555,6 @@ void PCB_EDIT_FRAME::moveExact()
 }
 
 
-void PCB_EDIT_FRAME::duplicateItems( bool aIncrement )
-{
-    BOARD_ITEM* item = GetScreen()->GetCurItem();
-
-    if( !item )
-        return;
-
-    // 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();
-
-    PCB_BASE_EDIT_FRAME::duplicateItem( item, aIncrement );
-}
-
-
-void PCB_BASE_EDIT_FRAME::duplicateItem( BOARD_ITEM* aItem, bool aIncrement )
-{
-    if( !aItem )
-        return;
-
-    // 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 )
-    {
-        m_canvas->MoveCursorToCrossHair();
-
-        INSTALL_UNBUFFERED_DC( dc, m_canvas );
-
-        wxPoint crossHairPos = GetCrossHairPosition();
-
-        const BLOCK_COMMAND_T blockType = aIncrement ? BLOCK_COPY_AND_INCREMENT : BLOCK_COPY;
-
-        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 );
-    }
-}
-
-
 class LEGACY_ARRAY_CREATOR: public ARRAY_CREATOR
 {
 public:
diff --git a/pcbnew/hotkeys.cpp b/pcbnew/hotkeys.cpp
index 2fa1fa5d7..431929c63 100644
--- a/pcbnew/hotkeys.cpp
+++ b/pcbnew/hotkeys.cpp
@@ -117,9 +117,6 @@ static EDA_HOTKEY HkFlipItem( _HKI( "Flip Item" ), HK_FLIP_ITEM, 'F' );
 static EDA_HOTKEY HkRotateItem( _HKI( "Rotate Item" ), HK_ROTATE_ITEM, 'R' );
 static EDA_HOTKEY HkMoveItem( _HKI( "Move Item" ), HK_MOVE_ITEM, 'M' );
 static EDA_HOTKEY HkMoveItemExact( _HKI( "Move Item Exactly" ), HK_MOVE_ITEM_EXACT, 'M' + GR_KB_CTRL );
-static EDA_HOTKEY HkDuplicateItem( _HKI( "Duplicate Item" ), HK_DUPLICATE_ITEM, 'D' + GR_KB_CTRL );
-static EDA_HOTKEY HkDuplicateItemAndIncrement( _HKI( "Duplicate Item and Increment" ),
-                                   HK_DUPLICATE_ITEM_AND_INCREMENT, 'D' + GR_KB_SHIFTCTRL );
 static EDA_HOTKEY HkCreateArray( _HKI( "Create Array" ), HK_CREATE_ARRAY, 'N' + GR_KB_CTRL );
 static EDA_HOTKEY HkCopyItem( _HKI( "Copy Item" ), HK_COPY_ITEM, 'C' );
 static EDA_HOTKEY HkDragFootprint( _HKI( "Drag Item" ), HK_DRAG_ITEM, 'G' );
@@ -263,7 +260,7 @@ EDA_HOTKEY* board_edit_Hotkey_List[] =
     &HkMoveItem,
     &HkFlipItem,
     &HkRotateItem,             &HkMoveItemExact,
-    &HkDuplicateItem,          &HkDuplicateItemAndIncrement, &HkCreateArray,
+    &HkCreateArray,
     &HkDragFootprint,
     &HkGetAndMoveFootprint,    &HkLock_Unlock_Footprint,     &HkSavefile, &HkSavefileAs,
     &HkLoadfile,               &HkFindItem,                  &HkEditBoardItem,
@@ -283,7 +280,7 @@ EDA_HOTKEY* board_edit_Hotkey_List[] =
 // List of hotkey descriptors for the module editor
 EDA_HOTKEY* module_edit_Hotkey_List[] = {
     &HkMoveItem,               &HkRotateItem,                &HkEditBoardItem,
-    &HkMoveItemExact,          &HkDuplicateItem,             &HkDuplicateItemAndIncrement,
+    &HkMoveItemExact,
     &HkCreateArray,            &HkDelete,                    &HkSaveModule,
     &HkCanvasDefault,          &HkCanvasCairo,               &HkCanvasOpenGL,
     NULL
diff --git a/pcbnew/hotkeys.h b/pcbnew/hotkeys.h
index f4f3ff190..80b82e1bb 100644
--- a/pcbnew/hotkeys.h
+++ b/pcbnew/hotkeys.h
@@ -63,8 +63,6 @@ enum hotkey_id_commnand {
     HK_FIND_ITEM,
     HK_EDIT_ITEM,
     HK_EDIT_MODULE_WITH_MODEDIT,
-    HK_DUPLICATE_ITEM,
-    HK_DUPLICATE_ITEM_AND_INCREMENT,
     HK_CREATE_ARRAY,
     HK_PLACE_ITEM,
     HK_SWITCH_TRACK_WIDTH_TO_NEXT,
diff --git a/pcbnew/hotkeys_board_editor.cpp b/pcbnew/hotkeys_board_editor.cpp
index 516388d9d..99fc53789 100644
--- a/pcbnew/hotkeys_board_editor.cpp
+++ b/pcbnew/hotkeys_board_editor.cpp
@@ -472,10 +472,8 @@ bool PCB_EDIT_FRAME::OnHotKey( wxDC* aDC, int aHotkeyCode, const wxPoint& aPosit
         break;
 
     case HK_MOVE_ITEM_EXACT:
-    case HK_DUPLICATE_ITEM:
-    case HK_DUPLICATE_ITEM_AND_INCREMENT:
     case HK_CREATE_ARRAY:
-        OnHotkeyDuplicateOrArrayItem( HK_Descr->m_Idcommand );
+        OnHotkeyArrayItem( HK_Descr->m_Idcommand );
         break;
 
     case HK_SWITCH_HIGHCONTRAST_MODE: // switch to high contrast mode and refresh the canvas
@@ -1042,7 +1040,7 @@ bool PCB_EDIT_FRAME::OnHotkeyRotateItem( int aIdCommand )
 }
 
 
-bool PCB_EDIT_FRAME::OnHotkeyDuplicateOrArrayItem( int aIdCommand )
+bool PCB_EDIT_FRAME::OnHotkeyArrayItem( int aIdCommand )
 {
     BOARD_ITEM* item = GetCurItem();
     bool itemCurrentlyEdited = item && item->GetFlags();
@@ -1081,16 +1079,6 @@ bool PCB_EDIT_FRAME::OnHotkeyDuplicateOrArrayItem( int aIdCommand )
                 evt_type = ID_POPUP_PCB_CREATE_ARRAY;
             break;
 
-        case HK_DUPLICATE_ITEM_AND_INCREMENT:
-            if( canDuplicate )
-                evt_type = ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT;
-            break;
-
-        case HK_DUPLICATE_ITEM:
-            if( canDuplicate )
-                evt_type = ID_POPUP_PCB_DUPLICATE_ITEM;
-            break;
-
         case HK_MOVE_ITEM_EXACT:
             evt_type = ID_POPUP_PCB_MOVE_EXACT;
             break;
diff --git a/pcbnew/hotkeys_module_editor.cpp b/pcbnew/hotkeys_module_editor.cpp
index a7a73050e..a207870bd 100644
--- a/pcbnew/hotkeys_module_editor.cpp
+++ b/pcbnew/hotkeys_module_editor.cpp
@@ -183,11 +183,6 @@ bool FOOTPRINT_EDIT_FRAME::OnHotKey( wxDC* aDC, int aHotKey, const wxPoint& aPos
         OnHotkeyRotateItem( HK_ROTATE_ITEM );
         break;
 
-    case HK_DUPLICATE_ITEM:
-    case HK_DUPLICATE_ITEM_AND_INCREMENT:
-        OnHotkeyDuplicateItem( HK_Descr->m_Idcommand );
-        break;
-
     case HK_CREATE_ARRAY:
         PostCommandMenuEvent( ID_POPUP_PCB_CREATE_ARRAY );
     }
@@ -367,35 +362,6 @@ bool FOOTPRINT_EDIT_FRAME::OnHotkeyMoveItemExact()
 }
 
 
-bool FOOTPRINT_EDIT_FRAME::OnHotkeyDuplicateItem( int aIdCommand )
-{
-    BOARD_ITEM* item = PrepareItemForHotkey( true );
-
-    if( item == NULL )
-        return false;
-
-    int evt_type = 0;       // Used to post a wxCommandEvent on demand
-
-    switch( item->Type() )
-    {
-    case PCB_PAD_T:
-    case PCB_MODULE_EDGE_T:
-    case PCB_MODULE_TEXT_T:
-        if( aIdCommand == HK_DUPLICATE_ITEM )
-            evt_type = ID_POPUP_PCB_DUPLICATE_ITEM;
-        else
-            evt_type = ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT;
-
-        break;
-
-    default:
-        break;
-    }
-
-    return PostCommandMenuEvent( evt_type );
-}
-
-
 bool FOOTPRINT_EDIT_FRAME::OnHotkeyRotateItem( int aIdCommand )
 {
     BOARD_ITEM* item = PrepareItemForHotkey( false );
diff --git a/pcbnew/modedit.cpp b/pcbnew/modedit.cpp
index 5eea2fb1a..cab5bc733 100644
--- a/pcbnew/modedit.cpp
+++ b/pcbnew/modedit.cpp
@@ -633,14 +633,6 @@ void FOOTPRINT_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
         m_canvas->MoveCursorToCrossHair();
         break;
 
-    case ID_POPUP_PCB_DUPLICATE_ITEM:
-        duplicateItems( false );
-        break;
-
-    case ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT:
-        duplicateItems( true );
-        break;
-
     case ID_POPUP_PCB_MOVE_EXACT:
         moveExact();
         break;
@@ -859,14 +851,6 @@ void FOOTPRINT_EDIT_FRAME::moveExact()
 }
 
 
-void FOOTPRINT_EDIT_FRAME::duplicateItems( bool aIncrement )
-{
-    BOARD_ITEM* item = GetScreen()->GetCurItem();
-
-    PCB_BASE_EDIT_FRAME::duplicateItem( item, aIncrement );
-}
-
-
 void FOOTPRINT_EDIT_FRAME::Transform( MODULE* module, int transform )
 {
     switch( transform )
diff --git a/pcbnew/modedit_onclick.cpp b/pcbnew/modedit_onclick.cpp
index 97c1c090a..e0ed2ec9a 100644
--- a/pcbnew/modedit_onclick.cpp
+++ b/pcbnew/modedit_onclick.cpp
@@ -317,9 +317,6 @@ bool FOOTPRINT_EDIT_FRAME::OnRightClick( const wxPoint& MousePos, wxMenu* PopMen
             msg = AddHotkeyName( _("Delete Pad" ), g_Module_Editor_Hokeys_Descr, HK_DELETE );
             AddMenuItem( PopMenu, ID_POPUP_PCB_DELETE_PAD, msg, KiBitmap( delete_pad_xpm ) );
 
-            msg = AddHotkeyName( _( "Duplicate Pad" ), g_Module_Editor_Hokeys_Descr, HK_DUPLICATE_ITEM );
-            AddMenuItem( PopMenu, ID_POPUP_PCB_DUPLICATE_ITEM, msg, KiBitmap( duplicate_pad_xpm ) );
-
             msg = AddHotkeyName( _("Move Pad Exactly" ), g_Module_Editor_Hokeys_Descr, HK_MOVE_ITEM_EXACT );
             AddMenuItem( PopMenu, ID_POPUP_PCB_MOVE_EXACT, msg, KiBitmap( move_pad_xpm ) );
 
@@ -358,11 +355,6 @@ bool FOOTPRINT_EDIT_FRAME::OnRightClick( const wxPoint& MousePos, wxMenu* PopMen
 
                 if( &module->Reference() != text && &module->Value() != text )
                 {
-                    msg = AddHotkeyName( _( "Duplicate Text" ),
-                                         g_Module_Editor_Hokeys_Descr, HK_DUPLICATE_ITEM );
-                    AddMenuItem( PopMenu, ID_POPUP_PCB_DUPLICATE_ITEM,
-                                 msg, KiBitmap( duplicate_text_xpm ) );
-
                     msg = AddHotkeyName( _("Create Text Array" ),
                                          g_Module_Editor_Hokeys_Descr, HK_CREATE_ARRAY );
                     AddMenuItem( PopMenu, ID_POPUP_PCB_CREATE_ARRAY,
@@ -401,9 +393,6 @@ bool FOOTPRINT_EDIT_FRAME::OnRightClick( const wxPoint& MousePos, wxMenu* PopMen
                 AddMenuItem( PopMenu, ID_POPUP_PCB_MOVE_EDGE, msg, KiBitmap( move_line_xpm ) );
             }
 
-            msg = AddHotkeyName( _( "Duplicate Edge" ), g_Module_Editor_Hokeys_Descr, HK_DUPLICATE_ITEM );
-            AddMenuItem( PopMenu, ID_POPUP_PCB_DUPLICATE_ITEM, msg, KiBitmap( duplicate_line_xpm ) );
-
             msg = AddHotkeyName( _("Move Edge Exactly" ), g_Module_Editor_Hokeys_Descr, HK_MOVE_ITEM_EXACT );
             AddMenuItem( PopMenu, ID_POPUP_PCB_MOVE_EXACT, msg, KiBitmap( move_line_xpm ) );
 
diff --git a/pcbnew/module_editor_frame.h b/pcbnew/module_editor_frame.h
index eee715516..fa826cead 100644
--- a/pcbnew/module_editor_frame.h
+++ b/pcbnew/module_editor_frame.h
@@ -162,7 +162,6 @@ public:
     bool OnHotkeyMoveItem( int aIdCommand );
     bool OnHotkeyMoveItemExact();
     bool OnHotkeyRotateItem( int aIdCommand );
-    bool OnHotkeyDuplicateItem( int aIdCommand );
 
     /**
      * Function Show3D_Frame
@@ -498,13 +497,6 @@ private:
      * user the enter the move delta
      */
     void moveExact();
-
-    /**
-     * Function duplicateItems
-     * Duplicate the item under the cursor
-     * @param aIncrement increment the number of pad (if that is what is selected)
-     */
-    void duplicateItems( bool aIncrement ) override;
 };
 
 #endif      // MODULE_EDITOR_FRAME_H_
diff --git a/pcbnew/onrightclick.cpp b/pcbnew/onrightclick.cpp
index f5ed5d065..9bc2edb38 100644
--- a/pcbnew/onrightclick.cpp
+++ b/pcbnew/onrightclick.cpp
@@ -196,11 +196,6 @@ bool PCB_EDIT_FRAME::OnRightClick( const wxPoint& aMousePos, wxMenu* aPopMenu )
                 AddMenuItem( aPopMenu, ID_POPUP_PCB_MOVE_DRAWING_REQUEST,
                              msg, KiBitmap( move_xpm ) );
 
-                msg = AddHotkeyName( _( "Duplicate Drawing" ), g_Board_Editor_Hokeys_Descr,
-                                     HK_DUPLICATE_ITEM );
-                AddMenuItem( aPopMenu, ID_POPUP_PCB_DUPLICATE_ITEM,
-                             msg, KiBitmap( duplicate_line_xpm ) );
-
                 msg = AddHotkeyName( _("Move Drawing Exactly" ), g_Board_Editor_Hokeys_Descr,
                                      HK_MOVE_ITEM_EXACT );
                 AddMenuItem( aPopMenu, ID_POPUP_PCB_MOVE_EXACT,
@@ -272,11 +267,6 @@ bool PCB_EDIT_FRAME::OnRightClick( const wxPoint& aMousePos, wxMenu* aPopMenu )
                 AddMenuItem( aPopMenu, ID_POPUP_PCB_MOVE_TEXT_DIMENSION_REQUEST,
                              msg, KiBitmap( move_text_xpm ) );
 
-                msg = AddHotkeyName( _( "Duplicate Dimension" ), g_Board_Editor_Hokeys_Descr,
-                                     HK_DUPLICATE_ITEM );
-                AddMenuItem( aPopMenu, ID_POPUP_PCB_DUPLICATE_ITEM,
-                             msg, KiBitmap( duplicate_text_xpm ) );
-
                 msg = AddHotkeyName( _("Move Dimension Exactly" ), g_Board_Editor_Hokeys_Descr,
                                      HK_MOVE_ITEM_EXACT );
                 AddMenuItem( aPopMenu, ID_POPUP_PCB_MOVE_EXACT,
@@ -303,11 +293,6 @@ bool PCB_EDIT_FRAME::OnRightClick( const wxPoint& aMousePos, wxMenu* aPopMenu )
                 AddMenuItem( aPopMenu, ID_POPUP_PCB_MOVE_EXACT,
                              msg, KiBitmap( move_target_xpm ) );
 
-                msg = AddHotkeyName( _( "Duplicate Target" ), g_Board_Editor_Hokeys_Descr,
-                                     HK_DUPLICATE_ITEM );
-                AddMenuItem( aPopMenu, ID_POPUP_PCB_DUPLICATE_ITEM,
-                             msg, KiBitmap( duplicate_target_xpm ) );
-
                 msg = AddHotkeyName( _( "Edit Target" ), g_Board_Editor_Hokeys_Descr,
                                      HK_EDIT_ITEM );
                 AddMenuItem( aPopMenu, ID_POPUP_PCB_EDIT_MIRE, msg, KiBitmap( edit_xpm ) );
@@ -541,11 +526,6 @@ void PCB_EDIT_FRAME::createPopupMenuForTracks( TRACK* Track, wxMenu* PopMenu )
                 AddMenuItem( PopMenu, ID_POPUP_PCB_DRAG_TRACK_SEGMENT,
                              msg, KiBitmap( drag_track_segment_xpm ) );
 
-                msg = AddHotkeyName( _( "Duplicate Track" ), g_Board_Editor_Hokeys_Descr,
-                                     HK_DUPLICATE_ITEM );
-                AddMenuItem( PopMenu, ID_POPUP_PCB_DUPLICATE_ITEM,
-                             msg, KiBitmap( duplicate_line_xpm ) );
-
                 msg = AddHotkeyName( _("Move Track Exactly" ), g_Board_Editor_Hokeys_Descr,
                                      HK_MOVE_ITEM_EXACT );
                 AddMenuItem( PopMenu, ID_POPUP_PCB_MOVE_EXACT,
@@ -839,11 +819,6 @@ void PCB_EDIT_FRAME::createPopUpMenuForFootprints( MODULE* aModule, wxMenu* menu
         AddMenuItem( sub_menu_footprint, ID_POPUP_PCB_MOVE_EXACT,
                      msg, KiBitmap( move_module_xpm ) );
 
-        msg = AddHotkeyName( _( "Duplicate Footprint" ), g_Board_Editor_Hokeys_Descr,
-                             HK_DUPLICATE_ITEM );
-        AddMenuItem( sub_menu_footprint, ID_POPUP_PCB_DUPLICATE_ITEM,
-                     msg, KiBitmap( duplicate_module_xpm ) );
-
         msg = AddHotkeyName( _("Create Footprint Array" ), g_Board_Editor_Hokeys_Descr,
                              HK_CREATE_ARRAY );
         AddMenuItem( sub_menu_footprint, ID_POPUP_PCB_CREATE_ARRAY,
diff --git a/pcbnew/pcb_base_edit_frame.h b/pcbnew/pcb_base_edit_frame.h
index ef68feebd..7d1553236 100644
--- a/pcbnew/pcb_base_edit_frame.h
+++ b/pcbnew/pcb_base_edit_frame.h
@@ -176,29 +176,6 @@ 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
-     * @param aIncrement (has meaning only for pads in footprint editor):
-     * increment pad name if appropriate
-     */
-    void duplicateItem( BOARD_ITEM* aItem, bool aIncrement );
-
-    /**
-     * Function duplicateItems
-     * Find and duplicate the currently selected items
-     * @param aIncrement (has meaning only for pads in footprint editor):
-     * increment pad name 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
diff --git a/pcbnew/pcbnew_id.h b/pcbnew/pcbnew_id.h
index 8e98979e7..a83a33577 100644
--- a/pcbnew/pcbnew_id.h
+++ b/pcbnew/pcbnew_id.h
@@ -74,8 +74,6 @@ enum pcbnew_ids
     ID_POPUP_PCB_ROTATE_PAD,
     ID_POPUP_PCB_MOVE_PAD_REQUEST,
     ID_POPUP_PCB_DRAG_PAD_REQUEST,
-    ID_POPUP_PCB_DUPLICATE_ITEM,
-    ID_POPUP_PCB_DUPLICATE_ITEM_AND_INCREMENT,
     ID_POPUP_PCB_MOVE_EXACT,
     ID_POPUP_PCB_CREATE_ARRAY,
 
diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index fda4fa97e..34b4eeccb 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -107,11 +107,11 @@ TOOL_ACTION COMMON_ACTIONS::drag( "pcbnew.InteractiveEdit.drag",
         drag_module_xpm, AF_ACTIVATE, (void*) MOVE_FLAGS::DRAG_TRACKS );
 
 TOOL_ACTION COMMON_ACTIONS::duplicate( "pcbnew.InteractiveEdit.duplicate",
-        AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DUPLICATE_ITEM ),
+        AS_GLOBAL, MD_CTRL + 'D',
         _( "Duplicate" ), _( "Duplicates the selected item(s)" ), duplicate_module_xpm );
 
 TOOL_ACTION COMMON_ACTIONS::duplicateIncrement( "pcbnew.InteractiveEdit.duplicateIncrementPads",
-        AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DUPLICATE_ITEM_AND_INCREMENT ),
+        AS_GLOBAL, MD_CTRL + MD_SHIFT + 'D',
         _( "Duplicate" ), _( "Duplicates the selected item(s), incrementing pad numbers" ) );
 
 TOOL_ACTION COMMON_ACTIONS::moveExact( "pcbnew.InteractiveEdit.moveExact",
-- 
2.11.0


References