← Back to team overview

kicad-developers team mailing list archive

Re: [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 <seth.hillbrand@xxxxxxxxx>:

> ​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 <hillbrand@xxxxxxxxxxx>
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<BOARD_ITEM*> rejected;
+    for( auto i : selection )
+    {
+        auto item = static_cast<BOARD_ITEM*>( i );
+        if( item->Type() == PCB_PAD_T )
+        {
+            MODULE* mod = static_cast<MODULE*>( 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<PCB_BASE_FRAME>();
-    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<PCB_BASE_FRAME>();
-    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<PCB_BASE_FRAME>();
-    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<PCB_BASE_FRAME>();
-    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(), SortRightmostX );
-
     if( checkLockedStatus( selection ) == SELECTION_LOCKED )
         return 0;
 
@@ -417,14 +442,15 @@ int ALIGN_DISTRIBUTE_TOOL::doAlignRight()
 int ALIGN_DISTRIBUTE_TOOL::AlignCenterX( const TOOL_EVENT& aEvent )
 {
     auto frame = getEditFrame<PCB_BASE_FRAME>();
-    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(), SortCenterX );
-
     if( checkLockedStatus( selection ) == SELECTION_LOCKED )
         return 0;
 
@@ -456,14 +482,15 @@ int ALIGN_DISTRIBUTE_TOOL::AlignCenterX( const TOOL_EVENT& aEvent )
 int ALIGN_DISTRIBUTE_TOOL::AlignCenterY( const TOOL_EVENT& aEvent )
 {
     auto frame = getEditFrame<PCB_BASE_FRAME>();
-    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(), SortCenterY );
-
     if( checkLockedStatus( selection ) == SELECTION_LOCKED )
         return 0;
 
@@ -495,7 +522,7 @@ int ALIGN_DISTRIBUTE_TOOL::AlignCenterY( const TOOL_EVENT& aEvent )
 int ALIGN_DISTRIBUTE_TOOL::DistributeHorizontally( const TOOL_EVENT& aEvent )
 {
     auto frame = getEditFrame<PCB_BASE_FRAME>();
-    const SELECTION& selection = m_selectionTool->RequestSelection(
+    SELECTION& selection = m_selectionTool->RequestSelection(
             SELECTION_EDITABLE | SELECTION_SANITIZE_PADS );
 
     if( selection.Size() <= 1 )
@@ -582,7 +609,7 @@ void ALIGN_DISTRIBUTE_TOOL::doDistributeCentersHorizontally( ALIGNMENT_RECTS &it
 int ALIGN_DISTRIBUTE_TOOL::DistributeVertically( const TOOL_EVENT& aEvent )
 {
     auto frame = getEditFrame<PCB_BASE_FRAME>();
-    const SELECTION& selection = m_selectionTool->RequestSelection(
+    SELECTION& selection = m_selectionTool->RequestSelection(
             SELECTION_EDITABLE | SELECTION_SANITIZE_PADS );
 
     if( selection.Size() <= 1 )
diff --git a/pcbnew/tools/placement_tool.h b/pcbnew/tools/placement_tool.h
index 3d4752029..40fc28b9e 100644
--- a/pcbnew/tools/placement_tool.h
+++ b/pcbnew/tools/placement_tool.h
@@ -120,6 +120,11 @@ private:
     CONTEXT_MENU* m_placementMenu;
 
     /**
+     * Remove pads from a multi-unit select that also includes the pads' parents
+     */
+    void filterPadsWithModules( SELECTION &selection );
+
+    /**
      * Check a selection to ensure locks are valid for alignment.
      *
      * This is slightly different from the standard lock checking in that we ignore the lock
-- 
2.11.0

From e24c6dc2aaf5ac6f7a01a38e0652febce74cc1ac Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Mon, 26 Feb 2018 15:13:21 -0800
Subject: [PATCH 1/3] 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..0c67e865b 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<PCB_BASE_FRAME>();
+    const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE );
 
     if( selection.Size() <= 1 )
         return 0;
 
-    BOARD_COMMIT commit( getEditFrame<PCB_BASE_FRAME>() );
-    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<PCB_BASE_FRAME>();
+    const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE );
 
     if( selection.Size() <= 1 )
         return 0;
 
-    BOARD_COMMIT commit( getEditFrame<PCB_BASE_FRAME>() );
-    commit.StageItems( selection, CHT_MODIFY );
-
     auto itemsToAlign = GetBoundingBoxes( selection );
     std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortBottommostY );
 
+    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 targetBottom = itemsToAlign.begin()->second.GetBottom();
 
@@ -231,7 +291,13 @@ int ALIGN_DISTRIBUTE_TOOL::AlignBottom( const TOOL_EVENT& aEvent )
     for( auto& i : itemsToAlign )
     {
         int difference = targetBottom - i.second.GetBottom();
-        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 bottom" ) );
@@ -257,17 +323,21 @@ int ALIGN_DISTRIBUTE_TOOL::AlignLeft( const TOOL_EVENT& aEvent )
 
 int ALIGN_DISTRIBUTE_TOOL::doAlignLeft()
 {
-    const SELECTION& selection = m_selectionTool->GetSelection();
+    auto frame = getEditFrame<PCB_BASE_FRAME>();
+    const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE );
 
     if( selection.Size() <= 1 )
         return 0;
 
-    BOARD_COMMIT commit( getEditFrame<PCB_BASE_FRAME>() );
-    commit.StageItems( selection, CHT_MODIFY );
-
     auto itemsToAlign = GetBoundingBoxes( selection );
     std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortLeftmostX );
 
+    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 targetLeft = itemsToAlign.begin()->second.GetX();
 
@@ -275,7 +345,13 @@ int ALIGN_DISTRIBUTE_TOOL::doAlignLeft()
     for( auto& i : itemsToAlign )
     {
         int difference = targetLeft - i.second.GetX();
-        i.first->Move( wxPoint( difference, 0 ) );
+        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( difference, 0 ) );
     }
 
     commit.Push( _( "Align to left" ) );
@@ -301,17 +377,21 @@ int ALIGN_DISTRIBUTE_TOOL::AlignRight( const TOOL_EVENT& aEvent )
 
 int ALIGN_DISTRIBUTE_TOOL::doAlignRight()
 {
-    const SELECTION& selection = m_selectionTool->GetSelection();
+    auto frame = getEditFrame<PCB_BASE_FRAME>();
+    const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE );
 
     if( selection.Size() <= 1 )
         return 0;
 
-    BOARD_COMMIT commit( getEditFrame<PCB_BASE_FRAME>() );
-    commit.StageItems( selection, CHT_MODIFY );
-
     auto itemsToAlign = GetBoundingBoxes( selection );
     std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortRightmostX );
 
+    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 targetRight = itemsToAlign.begin()->second.GetRight();
 
@@ -319,7 +399,13 @@ int ALIGN_DISTRIBUTE_TOOL::doAlignRight()
     for( auto& i : itemsToAlign )
     {
         int difference = targetRight - i.second.GetRight();
-        i.first->Move( wxPoint( difference, 0 ) );
+        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( difference, 0 ) );
     }
 
     commit.Push( _( "Align to right" ) );
@@ -330,17 +416,21 @@ int ALIGN_DISTRIBUTE_TOOL::doAlignRight()
 
 int ALIGN_DISTRIBUTE_TOOL::AlignCenterX( const TOOL_EVENT& aEvent )
 {
-    const SELECTION& selection = m_selectionTool->GetSelection();
+    auto frame = getEditFrame<PCB_BASE_FRAME>();
+    const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE );
 
     if( selection.Size() <= 1 )
         return 0;
 
-    BOARD_COMMIT commit( getEditFrame<PCB_BASE_FRAME>() );
-    commit.StageItems( selection, CHT_MODIFY );
-
     auto itemsToAlign = GetBoundingBoxes( selection );
     std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortCenterX );
 
+    if( checkLockedStatus( selection ) == SELECTION_LOCKED )
+        return 0;
+
+    BOARD_COMMIT commit( frame );
+    commit.StageItems( selection, CHT_MODIFY );
+
     // after sorting use the x coordinate of the middle item as a target for all other items
     const int targetX = itemsToAlign.at( itemsToAlign.size() / 2 ).second.GetCenter().x;
 
@@ -348,7 +438,13 @@ int ALIGN_DISTRIBUTE_TOOL::AlignCenterX( const TOOL_EVENT& aEvent )
     for( auto& i : itemsToAlign )
     {
         int difference = targetX - i.second.GetCenter().x;
-        i.first->Move( wxPoint( difference, 0 ) );
+        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( difference, 0 ) );
     }
 
     commit.Push( _( "Align to middle" ) );
@@ -359,17 +455,21 @@ int ALIGN_DISTRIBUTE_TOOL::AlignCenterX( const TOOL_EVENT& aEvent )
 
 int ALIGN_DISTRIBUTE_TOOL::AlignCenterY( const TOOL_EVENT& aEvent )
 {
-    const SELECTION& selection = m_selectionTool->GetSelection();
+    auto frame = getEditFrame<PCB_BASE_FRAME>();
+    const SELECTION& selection = m_selectionTool->RequestSelection( SELECTION_EDITABLE );
 
     if( selection.Size() <= 1 )
         return 0;
 
-    BOARD_COMMIT commit( getEditFrame<PCB_BASE_FRAME>() );
-    commit.StageItems( selection, CHT_MODIFY );
-
     auto itemsToAlign = GetBoundingBoxes( selection );
     std::sort( itemsToAlign.begin(), itemsToAlign.end(), SortCenterY );
 
+    if( checkLockedStatus( selection ) == SELECTION_LOCKED )
+        return 0;
+
+    BOARD_COMMIT commit( frame );
+    commit.StageItems( selection, CHT_MODIFY );
+
     // after sorting use the y coordinate of the middle item as a target for all other items
     const int targetY = itemsToAlign.at( itemsToAlign.size() / 2 ).second.GetCenter().y;
 
@@ -377,7 +477,13 @@ int ALIGN_DISTRIBUTE_TOOL::AlignCenterY( const TOOL_EVENT& aEvent )
     for( auto& i : itemsToAlign )
     {
         int difference = targetY - i.second.GetCenter().y;
-        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 center" ) );
@@ -388,12 +494,17 @@ int ALIGN_DISTRIBUTE_TOOL::AlignCenterY( const TOOL_EVENT& aEvent )
 
 int ALIGN_DISTRIBUTE_TOOL::DistributeHorizontally( const TOOL_EVENT& aEvent )
 {
-    const SELECTION& selection = m_selectionTool->GetSelection();
+    auto frame = getEditFrame<PCB_BASE_FRAME>();
+    const SELECTION& selection = m_selectionTool->RequestSelection(
+            SELECTION_EDITABLE | SELECTION_SANITIZE_PADS );
 
     if( selection.Size() <= 1 )
         return 0;
 
-    BOARD_COMMIT commit( getEditFrame<PCB_BASE_FRAME>() );
+    if( m_selectionTool->CheckLock() == SELECTION_LOCKED )
+        return 0;
+
+    BOARD_COMMIT commit( frame );
     commit.StageItems( selection, CHT_MODIFY );
 
     auto itemsToDistribute = GetBoundingBoxes( selection );
@@ -470,12 +581,17 @@ void ALIGN_DISTRIBUTE_TOOL::doDistributeCentersHorizontally( ALIGNMENT_RECTS &it
 
 int ALIGN_DISTRIBUTE_TOOL::DistributeVertically( const TOOL_EVENT& aEvent )
 {
-    const SELECTION& selection = m_selectionTool->GetSelection();
+    auto frame = getEditFrame<PCB_BASE_FRAME>();
+    const SELECTION& selection = m_selectionTool->RequestSelection(
+            SELECTION_EDITABLE | SELECTION_SANITIZE_PADS );
 
     if( selection.Size() <= 1 )
         return 0;
 
-    BOARD_COMMIT commit( getEditFrame<PCB_BASE_FRAME>() );
+    if( m_selectionTool->CheckLock() == SELECTION_LOCKED )
+        return 0;
+
+    BOARD_COMMIT commit( frame );
     commit.StageItems( selection, CHT_MODIFY );
 
     auto itemsToDistribute = GetBoundingBoxes( selection );
diff --git a/pcbnew/tools/placement_tool.h b/pcbnew/tools/placement_tool.h
index a5206ed54..3d4752029 100644
--- a/pcbnew/tools/placement_tool.h
+++ b/pcbnew/tools/placement_tool.h
@@ -120,6 +120,16 @@ private:
     CONTEXT_MENU* m_placementMenu;
 
     /**
+     * Check a selection to ensure locks are valid for alignment.
+     *
+     * This is slightly different from the standard lock checking in that we ignore the lock
+     * of the first element in the selection as this is meant to be our anchor.
+     * We also check the lock of a pad's parent as we will not move pads independently of
+     * the parent module
+     */
+    int checkLockedStatus( const SELECTION &selection ) const;
+
+    /**
      * Distributes selected items using an even spacing between the centers of their bounding boxes
      *
      * NOTE: Using the centers of bounding box of items can give unsatisfactory visual results since
-- 
2.11.0

From 9a1074739be373db7c11dc223535523654ab8db7 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Wed, 28 Feb 2018 16:34:48 -0800
Subject: [PATCH 3/3] pcbnew: align centers to the top and left

When aligning module centers, it can be hard to determine which item
will be chosen for the alignment target when it chooses based on the
median of the center values.  Instead, this patch chooses the alignment
target as the top and left most items of the Y and X centering,
respectively
---
 pcbnew/tools/placement_tool.cpp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/pcbnew/tools/placement_tool.cpp b/pcbnew/tools/placement_tool.cpp
index 54fd089a3..c4c742e12 100644
--- a/pcbnew/tools/placement_tool.cpp
+++ b/pcbnew/tools/placement_tool.cpp
@@ -457,8 +457,9 @@ int ALIGN_DISTRIBUTE_TOOL::AlignCenterX( const TOOL_EVENT& aEvent )
     BOARD_COMMIT commit( frame );
     commit.StageItems( selection, CHT_MODIFY );
 
-    // after sorting use the x coordinate of the middle item as a target for all other items
-    const int targetX = itemsToAlign.at( itemsToAlign.size() / 2 ).second.GetCenter().x;
+    // after sorting use the center x coordinate of the leftmost item as a target
+    // for all other items
+    const int targetX = itemsToAlign.begin()->second.GetCenter().x;
 
     // Move the selected items
     for( auto& i : itemsToAlign )
@@ -497,8 +498,9 @@ int ALIGN_DISTRIBUTE_TOOL::AlignCenterY( const TOOL_EVENT& aEvent )
     BOARD_COMMIT commit( frame );
     commit.StageItems( selection, CHT_MODIFY );
 
-    // after sorting use the y coordinate of the middle item as a target for all other items
-    const int targetY = itemsToAlign.at( itemsToAlign.size() / 2 ).second.GetCenter().y;
+    // after sorting use the center y coordinate of the top-most item as a target
+    // for all other items
+    const int targetY = itemsToAlign.begin()->second.GetCenter().y;
 
     // Move the selected items
     for( auto& i : itemsToAlign )
-- 
2.11.0


Follow ups

References