[Kicad-developers] [PATCH] pcbnew alignment bugfixes
Attached are 4 patches for various alignment issues. The first fixes https://bugs.launchpad.net/kicad/+bug/1751352 where pads are moved despite being locked. The second is a bugfix where aligning in the X direction moved things in the Y direction. The third is a bugfix for when you select both a pad and the pad's parent module before aligning. The fourth is not a bugfix and, as such should be considered highly optional at this point. But it fixes an annoyance when aligning a group of things to their centers. I can never tell which item will be chosen for the center as currently it chooses the "centermost" item. If you have 10-20 mostly-aligned items, it's very hard to tell which will be the center. You need to move about half to one side and about half to the other before aligning. Or align, check the value and do a second move. Instead, this chooses the left-most for X-center and top-most for Y-center. You can then move one item into place and align the others to it. Delaying this has no affect of the other patches, so I'm happy to hold it for post-5. From c861f742e42a1cdb3dccf3a6668a375d3b2c4c25 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 26 Feb 2018 15:13:21 -0800 Subject: [PATCH 1/4] pcbnew: Check locks in alignment When aligning in pcbnew, check for pad/module locks before performing a move and query the user. When aligning on pads, don't move the pad without moving the footprint, so we don't break footprints. Fixes: lp:1751352 * https://bugs.launchpad.net/kicad/+bug/1751352 --- pcbnew/tools/placement_tool.cpp | 184 pcbnew/tools/placement_tool.h | 10 +++ 2 files changed, 160 insertions(+), 34 deletions(-) diff --git a/pcbnew/tools/placement_tool.cpp b/pcbnew/tools/placement_tool.cpp index f0ae369ca..d5589fba0 100644 --- a/pcbnew/tools/placement_tool.cpp +++ b/pcbnew/tools/placement_tool.cpp @@ -182,19 +182,69 @@ ALIGNMENT_RECTS GetBoundingBoxes( const SELECTION &sel ) } +int ALIGN_DISTRIBUTE_TOOL::checkLockedStatus( const SELECTION &selection ) const +{ +SELECTION moving_items( selection ); + +// Remove the anchor from the list +moving_items.Remove( moving_items.Front() ); + +bool containsLocked = false; + +// Check if the selection contains locked items +for( const auto& item : moving_items ) +{ +switch ( item->Type() ) +{ +case PCB_MODULE_T: +if( static_cast< MODULE* >( item )->IsLocked() ) +containsLocked = true; +break; + +case PCB_PAD_T: +case PCB_MODULE_EDGE_T: +case PCB_MODULE_TEXT_T: +if( static_cast< MODULE* >( item->GetParent() )->IsLocked() ) +containsLocked = true; +break; + +default:// suppress warnings +break; +} +} + +if( containsLocked ) +{ +if( IsOK( getEditFrame< PCB_EDIT_FRAME >(), +_( "Selection contains locked items. Do you want to continue?" ) ) ) +{ +return SELECTION_LOCK_OVERRIDE; +} +else +return SELECTION_LOCKED; +} + +return SELECTION_UNLOCKED; +} + + int ALIGN_DISTRIBUTE_TOOL::AlignTop( const TOOL_EVENT& aEvent ) { -const SELECTION& selection = m_selectionTool->GetSelection(); +auto frame = getEditFrame(); +const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE ); if( selection.Size() <= 1 ) return 0; -BOARD_COMMIT commit( getEditFrame() ); -commit.StageItems( selection, CHT_MODIFY ); - auto itemsToAlign = GetBoundingBoxes( selection ); std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortTopmostY ); +if( checkLockedStatus( selection ) == SELECTION_LOCKED ) +return 0; + +BOARD_COMMIT commit( frame ); +commit.StageItems( selection, CHT_MODIFY ); + // after sorting, the fist item acts as the target for all others const int targetTop = itemsToAlign.begin()->second.GetY(); @@ -202,7 +252,13 @@ int ALIGN_DISTRIBUTE_TOOL::AlignTop( const TOOL_EVENT& aEvent ) for( auto& i : itemsToAlign ) { int difference = targetTop - i.second.GetY(); -i.first->Move( wxPoint( 0, difference ) ); +BOARD_ITEM* item = i.first; + +// Don't move a pad by itself unless editing the footprint +if( item->Type() == PCB_PAD_T && frame->IsType( FRAME_PCB ) ) +item = item->GetParent(); + +item->Move( wxPoint( 0, difference ) ); } commit.Push( _( "Align to top" ) ); @@ -213,17 +269,21 @@ int ALIGN_DISTRIBUTE_TOOL::AlignTop( const TOOL_EVENT& aEvent ) int ALIGN_DISTRIBUTE_TOOL::AlignBottom( const TOOL_EVENT& aEvent ) { -const SELECTION& selection = m_selectionTool->GetSelection(); +auto frame = getEditFrame(); +const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE ); if( selection.Size() <= 1 ) return
Re: [Kicad-developers] [PATCH] pcbnew alignment bugfixes
Oops! Noticed that 1+2 should have been in the same patch (fixing my own bug there) I've squashed them down to three patches. -S 2018-02-28 16:52 GMT-08:00 Seth Hillbrand : > Attached are 4 patches for various alignment issues. > > The first fixes https://bugs.launchpad.net/kicad/+bug/1751352 where pads > are moved despite being locked. > > The second is a bugfix where aligning in the X direction moved things in > the Y direction. > > The third is a bugfix for when you select both a pad and the pad's parent > module before aligning. > > The fourth is not a bugfix and, as such should be considered highly > optional at this point. But it fixes an annoyance when aligning a group of > things to their centers. I can never tell which item will be chosen for > the center as currently it chooses the "centermost" item. If you have > 10-20 mostly-aligned items, it's very hard to tell which will be the > center. You need to move about half to one side and about half to the > other before aligning. Or align, check the value and do a second move. > Instead, this chooses the left-most for X-center and top-most for > Y-center. You can then move one item into place and align the others to > it. Delaying this has no affect of the other patches, so I'm happy to hold > it for post-5. > > From 31d7b7c92013a5c94f9adc7d97f5d15dd9e29397 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Tues, 27 Feb 2018 16:01:47 -0800 Subject: [PATCH 2/3] pcbnew: Prevent alignment on pads + parents Filter a selection that contains pads and the pads' parent modules before performing alignment operations. --- pcbnew/tools/placement_tool.cpp | 55 ++--- pcbnew/tools/placement_tool.h | 5 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/pcbnew/tools/placement_tool.cpp b/pcbnew/tools/placement_tool.cpp index 0c67e865b..54fd089a3 100644 --- a/pcbnew/tools/placement_tool.cpp +++ b/pcbnew/tools/placement_tool.cpp @@ -182,6 +182,27 @@ ALIGNMENT_RECTS GetBoundingBoxes( const SELECTION &sel ) } +void ALIGN_DISTRIBUTE_TOOL::filterPadsWithModules( SELECTION &selection ) +{ +std::set rejected; +for( auto i : selection ) +{ +auto item = static_cast( i ); +if( item->Type() == PCB_PAD_T ) +{ +MODULE* mod = static_cast( item->GetParent() ); + +// selection contains both the module and its pads - remove the pads +if( mod && selection.Contains( mod ) ) +rejected.insert( item ); +} +} + +for( BOARD_ITEM* item : rejected ) +selection.Remove( item ); +} + + int ALIGN_DISTRIBUTE_TOOL::checkLockedStatus( const SELECTION &selection ) const { SELECTION moving_items( selection ); @@ -231,14 +252,15 @@ int ALIGN_DISTRIBUTE_TOOL::checkLockedStatus( const SELECTION &selection ) const int ALIGN_DISTRIBUTE_TOOL::AlignTop( const TOOL_EVENT& aEvent ) { auto frame = getEditFrame(); -const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE ); +SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE ); if( selection.Size() <= 1 ) return 0; +filterPadsWithModules( selection ); + auto itemsToAlign = GetBoundingBoxes( selection ); std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortTopmostY ); - if( checkLockedStatus( selection ) == SELECTION_LOCKED ) return 0; @@ -270,14 +292,15 @@ int ALIGN_DISTRIBUTE_TOOL::AlignTop( const TOOL_EVENT& aEvent ) int ALIGN_DISTRIBUTE_TOOL::AlignBottom( const TOOL_EVENT& aEvent ) { auto frame = getEditFrame(); -const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE ); +SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE ); if( selection.Size() <= 1 ) return 0; +filterPadsWithModules( selection ); + auto itemsToAlign = GetBoundingBoxes( selection ); std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortBottommostY ); - if( checkLockedStatus( selection ) == SELECTION_LOCKED ) return 0; @@ -324,14 +347,15 @@ int ALIGN_DISTRIBUTE_TOOL::AlignLeft( const TOOL_EVENT& aEvent ) int ALIGN_DISTRIBUTE_TOOL::doAlignLeft() { auto frame = getEditFrame(); -const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE ); +SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE ); if( selection.Size() <= 1 ) return 0; +filterPadsWithModules( selection ); + auto itemsToAlign = GetBoundingBoxes( selection ); std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortLeftmostX ); - if( checkLockedStatus( selection ) == SELECTION_LOCKED ) return 0; @@ -378,14 +402,15 @@ int ALIGN_DISTRIBUTE_TOOL::AlignRight( const TOOL_EVENT& aEvent ) int ALIGN_DISTRIBUTE_TOOL::doAlignRight() { auto frame = getEditFrame(); -const SELECTION& selection =
Re: [Kicad-developers] [PATCH] pcbnew alignment bugfixes
It looks like this has been merged. 2018-03-01 5:00 GMT+01:00 Seth Hillbrand : > Oops! Noticed that 1+2 should have been in the same patch (fixing my own > bug there) > > I've squashed them down to three patches. > > -S > > 2018-02-28 16:52 GMT-08:00 Seth Hillbrand : > >> Attached are 4 patches for various alignment issues. >> >> The first fixes https://bugs.launchpad.net/kicad/+bug/1751352 where pads >> are moved despite being locked. >> >> The second is a bugfix where aligning in the X direction moved things in >> the Y direction. >> >> The third is a bugfix for when you select both a pad and the pad's parent >> module before aligning. >> >> The fourth is not a bugfix and, as such should be considered highly >> optional at this point. But it fixes an annoyance when aligning a group of >> things to their centers. I can never tell which item will be chosen for >> the center as currently it chooses the "centermost" item. If you have >> 10-20 mostly-aligned items, it's very hard to tell which will be the >> center. You need to move about half to one side and about half to the >> other before aligning. Or align, check the value and do a second move. >> Instead, this chooses the left-most for X-center and top-most for >> Y-center. You can then move one item into place and align the others to >> it. Delaying this has no affect of the other patches, so I'm happy to hold >> it for post-5. >> >> > > ___ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > > ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp