kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

The first of these patches has a mistake in the selection code, which
used to clear the selection and replace it, and should now augment,
not replace.

Please use the attached patches instead.

Thanks,

John

On Mon, Feb 6, 2017 at 12:37 AM, John Beard <john.j.beard@xxxxxxxxx> wrote:
> Apparently those patches don't apply because the diff context lines
> have a line of a different non-mainline patch in them (from the rotate
> CCW patch).
>
> Hopefully, these will apply to the current master.
>
> On Sun, Feb 5, 2017 at 9:23 PM, John Beard <john.j.beard@xxxxxxxxx> wrote:
>> Hi,
>>
>> These patches implement deletion of whole tracks (actually whole
>> connections, it won't span an unrouted gap).
>>
>> Fixes: https://bugs.launchpad.net/kicad/+bug/1517213
>>
>> This in in two parts:
>>
>> * Expand the current connection/track/net selection to act on multiple
>> selections, not just the first item in the selection list and refactor
>> the core of the traversal code in each case.
>> * Register the pre-existing "remove alternate" tool action, add a
>> concept of "delete flags" to COMMON_TOOLS, and when triggered, use the
>> flag to conditionally select whole tracks to delete.
>>
>> The idea of the flags is to allow flexible mappings of modified delete
>> actions, in much the same way as modifier keys, but without having to
>> commit to a hardcoded key for each flag, so the mappings can be more
>> eaisly user defined.
>>
>> This patch also reverses the mappings of the delete and backspace
>> keys, so that backspace is "normal remove" and "delete" is "remove
>> alternate". This keeps the mapping the same between legacy and GAL, as
>> per the LP issue. Other than tracks, this won't change anything else,
>> as there is as yet no other effect to alt remove, which wasn't used
>> until now.
>>
>> Cheers,
>>
>> John

From 3beb35ba9a3c391acb8de2e57a03002ab5686d53 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Sat, 4 Feb 2017 14:45:52 +0800
Subject: [PATCH 2/2] Allow delete whole track in GAL

This commit wires up the as-yet-unused "remove alternate" tool action
and uses it to select copper connections (normally 'U') before deleting
segments.

THis also reverses the sense of Delete and Backspace (Delete used to be
'remove' and Backpace was 'remove alt', now it is reversed). This means
that backspace is the key that removes a segment and Delete removes the
track. This is the same as legacy behaviour. Other delete actions are,
for now, the same between Delete and Backspace.

Fixes: lp:1517213
* https://bugs.launchpad.net/kicad/+bug/1517213
---
 pcbnew/tools/common_actions.cpp | 10 ++++++----
 pcbnew/tools/common_actions.h   |  3 +++
 pcbnew/tools/edit_tool.cpp      | 13 +++++++++++++
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/pcbnew/tools/common_actions.cpp b/pcbnew/tools/common_actions.cpp
index 498d86355..fe485cd1b 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -130,12 +130,14 @@ TOOL_ACTION COMMON_ACTIONS::mirror( "pcbnew.InteractiveEdit.mirror",
         _( "Mirror" ), _( "Mirrors selected item" ), mirror_h_xpm );
 
 TOOL_ACTION COMMON_ACTIONS::remove( "pcbnew.InteractiveEdit.remove",
-        AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
-        _( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm );
+        AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_BACK_SPACE ),
+        _( "Remove" ), _( "Deletes selected item(s)" ), delete_xpm,
+        AF_NONE, (void*) REMOVE_FLAGS::NORMAL );
 
 TOOL_ACTION COMMON_ACTIONS::removeAlt( "pcbnew.InteractiveEdit.removeAlt",
-        AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_BACK_SPACE ),
-        _( "Remove (alterative)" ), _( "Deletes selected item(s)" ), delete_xpm );
+        AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_DELETE ),
+        _( "Remove (alternative)" ), _( "Deletes selected item(s)" ), delete_xpm,
+        AF_NONE, (void*) REMOVE_FLAGS::ALT );
 
 TOOL_ACTION COMMON_ACTIONS::exchangeFootprints( "pcbnew.InteractiveEdit.ExchangeFootprints",
         AS_GLOBAL, 0,
diff --git a/pcbnew/tools/common_actions.h b/pcbnew/tools/common_actions.h
index b8167cf3c..fe8d0f67b 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -353,6 +353,9 @@ public:
     ///> Cursor control event types
     enum CURSOR_EVENT_TYPE { CURSOR_UP, CURSOR_DOWN, CURSOR_LEFT, CURSOR_RIGHT,
                              CURSOR_CLICK, CURSOR_DBL_CLICK, CURSOR_FAST_MOVE = 0x8000 };
+
+    ///> Remove event modifier flags
+    enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01 };
 };
 
 void registerAllTools( TOOL_MANAGER* aToolManager );
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index a344b3110..60adc3326 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -565,6 +565,18 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
     if( !hoverSelection() || m_selectionTool->CheckLock() == SELECTION_LOCKED )
         return 0;
 
+    // is this "alternative" remove?
+    const bool isAlt = aEvent.Parameter<intptr_t>() ==
+            (int) COMMON_ACTIONS::REMOVE_FLAGS::ALT;
+
+    // in "alternative" mode, deletion is not just a simple list
+    // of selected items, it is:
+    //   - whole tracks, not just segments
+    if( isAlt )
+    {
+        m_toolMgr->RunAction( COMMON_ACTIONS::selectConnection, true );
+    }
+
     // Get a copy instead of a reference, as we are going to clear current selection
     auto selection = m_selectionTool->GetSelection().GetItems();
 
@@ -819,6 +831,7 @@ void EDIT_TOOL::SetTransitions()
     Go( &EDIT_TOOL::Rotate,     COMMON_ACTIONS::rotate.MakeEvent() );
     Go( &EDIT_TOOL::Flip,       COMMON_ACTIONS::flip.MakeEvent() );
     Go( &EDIT_TOOL::Remove,     COMMON_ACTIONS::remove.MakeEvent() );
+    Go( &EDIT_TOOL::Remove,     COMMON_ACTIONS::removeAlt.MakeEvent() );
     Go( &EDIT_TOOL::Properties, COMMON_ACTIONS::properties.MakeEvent() );
     Go( &EDIT_TOOL::MoveExact,  COMMON_ACTIONS::moveExact.MakeEvent() );
     Go( &EDIT_TOOL::Duplicate,  COMMON_ACTIONS::duplicate.MakeEvent() );
-- 
2.11.0

From 060e8eb97981b0ffb7569d11094dd149b7d402f6 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Sat, 4 Feb 2017 14:44:17 +0800
Subject: [PATCH 1/2] Allow selectConnection/Copper/Net on multiple items

The previous behaviour was to act on only the first item in the
selection. The new behaviour is to act on every eligible item (in this
case, tracks and vias).
---
 pcbnew/tools/selection_tool.cpp | 120 ++++++++++++++++++++++++++--------------
 pcbnew/tools/selection_tool.h   |  24 +++++++-
 2 files changed, 98 insertions(+), 46 deletions(-)

diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp
index 1d8563b4d..b81f718f3 100644
--- a/pcbnew/tools/selection_tool.cpp
+++ b/pcbnew/tools/selection_tool.cpp
@@ -92,8 +92,9 @@ bool SELECTION_TOOL::Init()
 {
     using S_C = SELECTION_CONDITIONS;
 
-    auto showSelectMenuFunctor = ( S_C::OnlyType( PCB_VIA_T ) || S_C::OnlyType( PCB_TRACE_T ) )
-                                    && S_C::Count( 1 );
+    // can show the select menu as long as there is something the
+    // subitems can be run on
+    auto showSelectMenuFunctor = ( S_C::HasType( PCB_VIA_T ) || S_C::HasType( PCB_TRACE_T ) );
 
     auto selectMenu = std::make_shared<SELECT_MENU>();
     selectMenu->SetTool( this );
@@ -555,86 +556,119 @@ int SELECTION_TOOL::UnselectItem( const TOOL_EVENT& aEvent )
 }
 
 
-int SELECTION_TOOL::selectConnection( const TOOL_EVENT& aEvent )
+void SELECTION_TOOL::selectAllItemsConnectedToTrack( TRACK& sourceTrack )
 {
-    if( !selectCursor( true ) )
-        return 0;
-
-    auto item = m_selection.Front();
-    clearSelection();
-
-    if( item->Type() != PCB_TRACE_T && item->Type() != PCB_VIA_T )
-        return 0;
-
     int segmentCount;
-    TRACK* trackList = board()->MarkTrace( static_cast<TRACK*>( item ), &segmentCount,
-                                                     NULL, NULL, true );
-
-    if( segmentCount == 0 )
-        return 0;
+    TRACK* trackList = board()->MarkTrace( &sourceTrack, &segmentCount,
+                                           nullptr, nullptr, true );
 
     for( int i = 0; i < segmentCount; ++i )
     {
         select( trackList );
         trackList = trackList->Next();
     }
+}
+
+
+int SELECTION_TOOL::selectConnection( const TOOL_EVENT& aEvent )
+{
+    if( !selectCursor() )
+        return 0;
+
+    // copy the selection, since we're going to iterate and modify
+    auto selection = m_selection.GetItems();
+
+    for ( auto item : selection )
+    {
+        // only TRACK items can be checked for trivial connections
+        if( item->Type() == PCB_TRACE_T || item->Type() == PCB_VIA_T )
+        {
+            TRACK& trackItem = static_cast<TRACK&>( *item );
+            selectAllItemsConnectedToTrack( trackItem );
+        }
+    }
 
     // Inform other potentially interested tools
-    m_toolMgr->ProcessEvent( SelectedEvent );
+    if( m_selection.Size() > 0 )
+        m_toolMgr->ProcessEvent( SelectedEvent );
 
     return 0;
 }
 
 
-int SELECTION_TOOL::selectCopper( const TOOL_EVENT& aEvent )
+void SELECTION_TOOL::selectAllItemsConnectedToItem( BOARD_CONNECTED_ITEM& sourceItem )
 {
-    if( !selectCursor( true ) )
-        return 0;
+    RN_DATA* ratsnest = getModel<BOARD>()->GetRatsnest();
+    std::list<BOARD_CONNECTED_ITEM*> itemsList;
+    ratsnest->GetConnectedItems( &sourceItem, itemsList,
+                                 (RN_ITEM_TYPE)( RN_TRACKS | RN_VIAS ) );
+
+    for( BOARD_CONNECTED_ITEM* i : itemsList )
+        select( i );
+}
 
-    auto item = static_cast<BOARD_CONNECTED_ITEM*> ( m_selection.Front() );
-    clearSelection();
 
-    if( item->Type() != PCB_TRACE_T && item->Type() != PCB_VIA_T )
+int SELECTION_TOOL::selectCopper( const TOOL_EVENT& aEvent )
+{
+    if( !selectCursor( ) )
         return 0;
 
-    std::list<BOARD_CONNECTED_ITEM*> itemsList;
-    RN_DATA* ratsnest = getModel<BOARD>()->GetRatsnest();
+    // copy the selection, since we're going to iterate and modify
+    auto selection = m_selection.GetItems();
 
-    ratsnest->GetConnectedItems( item, itemsList, (RN_ITEM_TYPE)( RN_TRACKS | RN_VIAS ) );
+    for( auto item : selection )
+    {
+        // only connected items can be traversed in the ratsnest
+        if ( item->IsConnected() )
+        {
+            auto& connItem = static_cast<BOARD_CONNECTED_ITEM&> ( *item );
 
-    for( BOARD_CONNECTED_ITEM* i : itemsList )
-        select( i );
+            selectAllItemsConnectedToItem( connItem );
+        }
+    }
 
     // Inform other potentially interested tools
-    if( itemsList.size() > 0 )
+    if( m_selection.Size() > 0 )
         m_toolMgr->ProcessEvent( SelectedEvent );
 
     return 0;
 }
 
 
-int SELECTION_TOOL::selectNet( const TOOL_EVENT& aEvent )
+void SELECTION_TOOL::selectAllItemsOnNet( int aNetCode )
 {
-    if( !selectCursor( true ) )
-        return 0;
+    RN_DATA* ratsnest = getModel<BOARD>()->GetRatsnest();
+    std::list<BOARD_CONNECTED_ITEM*> itemsList;
+
+    ratsnest->GetNetItems( aNetCode, itemsList,
+                           (RN_ITEM_TYPE)( RN_TRACKS | RN_VIAS ) );
+
+    for( BOARD_CONNECTED_ITEM* i : itemsList )
+        select( i );
+}
 
-    auto item = dynamic_cast<BOARD_CONNECTED_ITEM*> ( m_selection.Front() );
 
-    if( !item )
+int SELECTION_TOOL::selectNet( const TOOL_EVENT& aEvent )
+{
+    if( !selectCursor() )
         return 0;
 
-    std::list<BOARD_CONNECTED_ITEM*> itemsList;
-    RN_DATA* ratsnest = getModel<BOARD>()->GetRatsnest();
-    int netCode = item->GetNetCode();
+    // copy the selection, since we're going to iterate and modify
+    auto selection = m_selection.GetItems();
 
-    clearSelection();
-    ratsnest->GetNetItems( netCode, itemsList, (RN_ITEM_TYPE)( RN_TRACKS | RN_VIAS ) );
+    for( auto item : selection )
+    {
+        // only connected items get a net code
+        if ( item->IsConnected() )
+        {
+            auto& connItem = static_cast<BOARD_CONNECTED_ITEM&> ( *item );
 
-    for( BOARD_CONNECTED_ITEM* i : itemsList )
-        select( i );
+            selectAllItemsOnNet( connItem.GetNetCode() );
+        }
+    }
 
     // Inform other potentially interested tools
-    if( itemsList.size() > 0 )
+    if( m_selection.Size() > 0 )
         m_toolMgr->ProcessEvent( SelectedEvent );
 
     return 0;
diff --git a/pcbnew/tools/selection_tool.h b/pcbnew/tools/selection_tool.h
index d6d194362..bbb0182ea 100644
--- a/pcbnew/tools/selection_tool.h
+++ b/pcbnew/tools/selection_tool.h
@@ -254,15 +254,33 @@ private:
      */
     bool selectMultiple();
 
-    ///> Selects a trivial connection (between two junctions).
+    ///> Selects a trivial connection (between two junctions) of items in selection
     int selectConnection( const TOOL_EVENT& aEvent );
 
-    ///> Selects a continuous copper connection.
+    ///> Selects items with a continuous copper connection to items in selection
     int selectCopper( const TOOL_EVENT& aEvent );
 
-    ///> Selects all copper connections belonging to a single net.
+    /**
+     * Selects all copper connections belonging to the same net(s) as the
+     * items in the selection
+     */
     int selectNet( const TOOL_EVENT& aEvent );
 
+    /**
+     * Selects all items connected by copper tracks to the given TRACK
+     */
+    void selectAllItemsConnectedToTrack( TRACK& sourceTrack );
+
+    /**
+     * Selects all items connected (by copper) to the given item
+     */
+    void selectAllItemsConnectedToItem( BOARD_CONNECTED_ITEM& sourceItem );
+
+    /**
+     * Selects all items with the given net code
+     */
+    void selectAllItemsOnNet( int aNetCode );
+
     ///> Find dialog callback.
     void findCallback( BOARD_ITEM* aItem );
 
-- 
2.11.0