← Back to team overview

kicad-developers team mailing list archive

[PATCH] Delete whole tracks in GAL

 

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 9fc80d8c5fc971cab9f756bc87a7383b732dfeba 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 604201fd5..fec8c683e 100644
--- a/pcbnew/tools/common_actions.cpp
+++ b/pcbnew/tools/common_actions.cpp
@@ -136,12 +136,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 b9800dcd4..33d0d7ab9 100644
--- a/pcbnew/tools/common_actions.h
+++ b/pcbnew/tools/common_actions.h
@@ -356,6 +356,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 031f2db4f..35cace213 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -569,6 +569,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();
 
@@ -824,6 +836,7 @@ void EDIT_TOOL::SetTransitions()
     Go( &EDIT_TOOL::Rotate,     COMMON_ACTIONS::rotateCcw.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 10f49283f08355dbe82938755f8c68bed259758c 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 | 123 ++++++++++++++++++++++++++--------------
 pcbnew/tools/selection_tool.h   |  24 +++++++-
 2 files changed, 101 insertions(+), 46 deletions(-)

diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp
index 1d8563b4d..ccb9d7252 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,122 @@ 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();
+    clearSelection();
+
+    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();
+    clearSelection();
 
-    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;
 
-    auto item = dynamic_cast<BOARD_CONNECTED_ITEM*> ( m_selection.Front() );
+    ratsnest->GetNetItems( aNetCode, itemsList,
+                           (RN_ITEM_TYPE)( RN_TRACKS | RN_VIAS ) );
 
-    if( !item )
-        return 0;
+    for( BOARD_CONNECTED_ITEM* i : itemsList )
+        select( i );
+}
 
-    std::list<BOARD_CONNECTED_ITEM*> itemsList;
-    RN_DATA* ratsnest = getModel<BOARD>()->GetRatsnest();
-    int netCode = item->GetNetCode();
 
+int SELECTION_TOOL::selectNet( const TOOL_EVENT& aEvent )
+{
+    if( !selectCursor() )
+        return 0;
+
+    // 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( BOARD_CONNECTED_ITEM* i : itemsList )
-        select( i );
+    for( auto item : selection )
+    {
+        // only connected items get a net code
+        if ( item->IsConnected() )
+        {
+            auto& connItem = static_cast<BOARD_CONNECTED_ITEM&> ( *item );
+
+            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


Follow ups