Fix sheet pin alignment calculation in "Align Items to Grid"
When aligning a sheet to grid, the sheet pins were not being properly aligned because the delta calculation passed a delta value to AlignGrid() instead of an absolute position. AlignGrid() snaps positions to grid, so passing a small delta like (-6350, 6350) IU would snap to (0, 0). The fix recognizes that pins have already moved with the sheet when SCH_SHEET::Move() is called, so the additional alignment calculation simply needs to snap the current pin position to grid. Refactored the alignment logic into a separate testable function AlignSchematicItemsToGrid() with callbacks, enabling proper QA testing with the actual reproduction case from the issue. Fixes https://gitlab.com/kicad/code/kicad/-/issues/22864
This commit is contained in:
@@ -392,6 +392,7 @@ set( EESCHEMA_SRCS
|
||||
sch_field.cpp
|
||||
sch_group.cpp
|
||||
sch_item.cpp
|
||||
sch_item_alignment.cpp
|
||||
sch_junction.cpp
|
||||
sch_label.cpp
|
||||
sch_line.cpp
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
/*
|
||||
* This program source code file is part of KiCad, a free EDA CAD application.
|
||||
*
|
||||
* Copyright The KiCad Developers, see AUTHORS.txt for contributors.
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of the GNU General Public License
|
||||
* as published by the Free Software Foundation; either version 2
|
||||
* of the License, or (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with this program; if not, you may find one here:
|
||||
* http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
|
||||
* or you may search the http://www.gnu.org website for the version 2 license,
|
||||
* or you may write to the Free Software Foundation, Inc.,
|
||||
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
|
||||
*/
|
||||
|
||||
#include "sch_item_alignment.h"
|
||||
|
||||
#include <sch_item.h>
|
||||
#include <sch_line.h>
|
||||
#include <sch_sheet.h>
|
||||
#include <sch_sheet_pin.h>
|
||||
#include <sch_screen.h>
|
||||
#include <tools/ee_grid_helper.h>
|
||||
|
||||
#include <map>
|
||||
#include <set>
|
||||
|
||||
|
||||
void AlignSchematicItemsToGrid( SCH_SCREEN* aScreen,
|
||||
const std::vector<EDA_ITEM*>& aItems,
|
||||
EE_GRID_HELPER& aGrid,
|
||||
GRID_HELPER_GRIDS aSelectionGrid,
|
||||
const SCH_ALIGNMENT_CALLBACKS& aCallbacks )
|
||||
{
|
||||
for( EDA_ITEM* item : aItems )
|
||||
{
|
||||
if( item->Type() == SCH_LINE_T )
|
||||
{
|
||||
SCH_LINE* line = static_cast<SCH_LINE*>( item );
|
||||
std::vector<int> flags{ STARTPOINT, ENDPOINT };
|
||||
std::vector<VECTOR2I> pts{ line->GetStartPoint(), line->GetEndPoint() };
|
||||
|
||||
for( int ii = 0; ii < 2; ++ii )
|
||||
{
|
||||
EDA_ITEMS drag_items{ item };
|
||||
line->ClearFlags();
|
||||
line->SetFlags( SELECTED );
|
||||
line->SetFlags( flags[ii] );
|
||||
|
||||
if( aCallbacks.m_getConnectedDragItems )
|
||||
aCallbacks.m_getConnectedDragItems( line, pts[ii], drag_items );
|
||||
|
||||
std::set<EDA_ITEM*> unique_items( drag_items.begin(), drag_items.end() );
|
||||
|
||||
VECTOR2I delta = aGrid.AlignGrid( pts[ii], aSelectionGrid ) - pts[ii];
|
||||
|
||||
if( delta != VECTOR2I( 0, 0 ) )
|
||||
{
|
||||
for( EDA_ITEM* dragItem : unique_items )
|
||||
{
|
||||
if( dragItem->GetParent() && dragItem->GetParent()->IsSelected() )
|
||||
continue;
|
||||
|
||||
aCallbacks.m_doMoveItem( dragItem, delta );
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
else if( item->Type() == SCH_FIELD_T || item->Type() == SCH_TEXT_T )
|
||||
{
|
||||
VECTOR2I delta = aGrid.AlignGrid( item->GetPosition(), aSelectionGrid )
|
||||
- item->GetPosition();
|
||||
|
||||
if( delta != VECTOR2I( 0, 0 ) )
|
||||
aCallbacks.m_doMoveItem( item, delta );
|
||||
}
|
||||
else if( item->Type() == SCH_SHEET_T )
|
||||
{
|
||||
SCH_SHEET* sheet = static_cast<SCH_SHEET*>( item );
|
||||
VECTOR2I topLeft = sheet->GetPosition();
|
||||
VECTOR2I bottomRight = topLeft + sheet->GetSize();
|
||||
VECTOR2I tl_delta = aGrid.AlignGrid( topLeft, aSelectionGrid ) - topLeft;
|
||||
VECTOR2I br_delta = aGrid.AlignGrid( bottomRight, aSelectionGrid ) - bottomRight;
|
||||
|
||||
if( tl_delta != VECTOR2I( 0, 0 ) || br_delta != VECTOR2I( 0, 0 ) )
|
||||
{
|
||||
aCallbacks.m_doMoveItem( sheet, tl_delta );
|
||||
|
||||
VECTOR2I newSize = (VECTOR2I) sheet->GetSize() - tl_delta + br_delta;
|
||||
sheet->SetSize( VECTOR2I( newSize.x, newSize.y ) );
|
||||
|
||||
if( aCallbacks.m_updateItem )
|
||||
aCallbacks.m_updateItem( sheet );
|
||||
}
|
||||
|
||||
for( SCH_SHEET_PIN* pin : sheet->GetPins() )
|
||||
{
|
||||
// Pin already moved with the sheet (via SCH_SHEET::Move), so pin->GetPosition()
|
||||
// is the new position. We just need to calculate the additional delta to align
|
||||
// the pin to grid.
|
||||
VECTOR2I pinPos = pin->GetPosition();
|
||||
VECTOR2I delta = aGrid.AlignGrid( pinPos, aSelectionGrid ) - pinPos;
|
||||
|
||||
if( delta != VECTOR2I( 0, 0 ) )
|
||||
{
|
||||
EDA_ITEMS drag_items;
|
||||
|
||||
if( aCallbacks.m_getConnectedDragItems )
|
||||
{
|
||||
aCallbacks.m_getConnectedDragItems( pin, pin->GetConnectionPoints()[0],
|
||||
drag_items );
|
||||
}
|
||||
|
||||
aCallbacks.m_doMoveItem( pin, delta );
|
||||
|
||||
for( EDA_ITEM* dragItem : drag_items )
|
||||
{
|
||||
if( dragItem->GetParent() && dragItem->GetParent()->IsSelected() )
|
||||
continue;
|
||||
|
||||
aCallbacks.m_doMoveItem( dragItem, delta );
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
SCH_ITEM* schItem = static_cast<SCH_ITEM*>( item );
|
||||
std::vector<VECTOR2I> connections = schItem->GetConnectionPoints();
|
||||
EDA_ITEMS drag_items;
|
||||
|
||||
if( aCallbacks.m_getConnectedDragItems )
|
||||
{
|
||||
for( const VECTOR2I& point : connections )
|
||||
aCallbacks.m_getConnectedDragItems( schItem, point, drag_items );
|
||||
}
|
||||
|
||||
std::map<VECTOR2I, int> shifts;
|
||||
VECTOR2I most_common( 0, 0 );
|
||||
int max_count = 0;
|
||||
|
||||
for( const VECTOR2I& conn : connections )
|
||||
{
|
||||
VECTOR2I gridpt = aGrid.AlignGrid( conn, aSelectionGrid ) - conn;
|
||||
|
||||
shifts[gridpt]++;
|
||||
|
||||
if( shifts[gridpt] > max_count )
|
||||
{
|
||||
most_common = gridpt;
|
||||
max_count = shifts[most_common];
|
||||
}
|
||||
}
|
||||
|
||||
if( most_common != VECTOR2I( 0, 0 ) )
|
||||
{
|
||||
aCallbacks.m_doMoveItem( item, most_common );
|
||||
|
||||
for( EDA_ITEM* dragItem : drag_items )
|
||||
{
|
||||
if( dragItem->GetParent() && dragItem->GetParent()->IsSelected() )
|
||||
continue;
|
||||
|
||||
aCallbacks.m_doMoveItem( dragItem, most_common );
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,89 @@
|
||||
/*
|
||||
* This program source code file is part of KiCad, a free EDA CAD application.
|
||||
*
|
||||
* Copyright The KiCad Developers, see AUTHORS.txt for contributors.
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of the GNU General Public License
|
||||
* as published by the Free Software Foundation; either version 2
|
||||
* of the License, or (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with this program; if not, you may find one here:
|
||||
* http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
|
||||
* or you may search the http://www.gnu.org website for the version 2 license,
|
||||
* or you may write to the Free Software Foundation, Inc.,
|
||||
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
|
||||
*/
|
||||
|
||||
#ifndef SCH_ITEM_ALIGNMENT_H
|
||||
#define SCH_ITEM_ALIGNMENT_H
|
||||
|
||||
#include <functional>
|
||||
#include <vector>
|
||||
#include <math/vector2d.h>
|
||||
#include <tool/grid_helper.h>
|
||||
|
||||
class EDA_ITEM;
|
||||
class SCH_ITEM;
|
||||
class SCH_SCREEN;
|
||||
class EE_GRID_HELPER;
|
||||
|
||||
using EDA_ITEMS = std::vector<EDA_ITEM*>;
|
||||
|
||||
|
||||
/**
|
||||
* Callbacks for alignment operations.
|
||||
*
|
||||
* These allow the alignment logic to be separated from the tool infrastructure,
|
||||
* making it testable in isolation.
|
||||
*/
|
||||
struct SCH_ALIGNMENT_CALLBACKS
|
||||
{
|
||||
/**
|
||||
* Callback to move an item by a delta.
|
||||
* Responsible for committing the change and updating the display.
|
||||
*/
|
||||
std::function<void( EDA_ITEM* aItem, const VECTOR2I& aDelta )> m_doMoveItem;
|
||||
|
||||
/**
|
||||
* Callback to get items connected to a given item at a specific point.
|
||||
* These are the "drag items" that should move along with the primary item.
|
||||
*/
|
||||
std::function<void( SCH_ITEM* aItem, const VECTOR2I& aPoint, EDA_ITEMS& aList )> m_getConnectedDragItems;
|
||||
|
||||
/**
|
||||
* Optional callback to update an item's display after modification.
|
||||
* Used for operations like SetSize that don't go through m_doMoveItem.
|
||||
*/
|
||||
std::function<void( EDA_ITEM* aItem )> m_updateItem;
|
||||
};
|
||||
|
||||
|
||||
/**
|
||||
* Align a set of schematic items to the grid.
|
||||
*
|
||||
* This function implements the core alignment logic used by the "Align Items to Grid"
|
||||
* action. It handles different item types appropriately: lines align their endpoints,
|
||||
* sheets align their corners and resize, sheet pins align to grid while maintaining
|
||||
* connectivity with their connected wires, etc.
|
||||
*
|
||||
* @param aScreen The schematic screen containing the items
|
||||
* @param aItems The items to align (typically the current selection)
|
||||
* @param aGrid The grid helper used for alignment calculations
|
||||
* @param aSelectionGrid The grid type to use for alignment
|
||||
* @param aCallbacks Callbacks for moving items and getting connected drag items
|
||||
*/
|
||||
void AlignSchematicItemsToGrid( SCH_SCREEN* aScreen,
|
||||
const std::vector<EDA_ITEM*>& aItems,
|
||||
EE_GRID_HELPER& aGrid,
|
||||
GRID_HELPER_GRIDS aSelectionGrid,
|
||||
const SCH_ALIGNMENT_CALLBACKS& aCallbacks );
|
||||
|
||||
|
||||
#endif // SCH_ITEM_ALIGNMENT_H
|
||||
@@ -55,6 +55,7 @@
|
||||
#include <math/box2.h>
|
||||
#include <base_units.h>
|
||||
#include <sch_screen.h>
|
||||
#include <sch_item_alignment.h>
|
||||
#include <trace_helpers.h>
|
||||
|
||||
|
||||
@@ -2506,130 +2507,24 @@ int SCH_MOVE_TOOL::AlignToGrid( const TOOL_EVENT& aEvent )
|
||||
}
|
||||
}
|
||||
|
||||
for( EDA_ITEM* item : selection )
|
||||
{
|
||||
if( item->Type() == SCH_LINE_T )
|
||||
{
|
||||
SCH_LINE* line = static_cast<SCH_LINE*>( item );
|
||||
std::vector<int> flags{ STARTPOINT, ENDPOINT };
|
||||
std::vector<VECTOR2I> pts{ line->GetStartPoint(), line->GetEndPoint() };
|
||||
SCH_ALIGNMENT_CALLBACKS callbacks;
|
||||
|
||||
for( int ii = 0; ii < 2; ++ii )
|
||||
callbacks.m_doMoveItem = doMoveItem;
|
||||
|
||||
callbacks.m_getConnectedDragItems =
|
||||
[&]( SCH_ITEM* aItem, const VECTOR2I& aPoint, EDA_ITEMS& aList )
|
||||
{
|
||||
EDA_ITEMS drag_items{ item };
|
||||
line->ClearFlags();
|
||||
line->SetFlags( SELECTED );
|
||||
line->SetFlags( flags[ii] );
|
||||
getConnectedDragItems( &commit, line, pts[ii], drag_items );
|
||||
std::set<EDA_ITEM*> unique_items( drag_items.begin(), drag_items.end() );
|
||||
getConnectedDragItems( &commit, aItem, aPoint, aList );
|
||||
};
|
||||
|
||||
VECTOR2I delta = grid.AlignGrid( pts[ii], selectionGrid ) - pts[ii];
|
||||
|
||||
if( delta != VECTOR2I( 0, 0 ) )
|
||||
{
|
||||
for( EDA_ITEM* dragItem : unique_items )
|
||||
{
|
||||
if( dragItem->GetParent() && dragItem->GetParent()->IsSelected() )
|
||||
continue;
|
||||
|
||||
doMoveItem( dragItem, delta );
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
else if( item->Type() == SCH_FIELD_T || item->Type() == SCH_TEXT_T )
|
||||
{
|
||||
VECTOR2I delta = grid.AlignGrid( item->GetPosition(), selectionGrid ) - item->GetPosition();
|
||||
|
||||
if( delta != VECTOR2I( 0, 0 ) )
|
||||
doMoveItem( item, delta );
|
||||
}
|
||||
else if( item->Type() == SCH_SHEET_T )
|
||||
{
|
||||
SCH_SHEET* sheet = static_cast<SCH_SHEET*>( item );
|
||||
VECTOR2I topLeft = sheet->GetPosition();
|
||||
VECTOR2I bottomRight = topLeft + sheet->GetSize();
|
||||
VECTOR2I tl_delta = grid.AlignGrid( topLeft, selectionGrid ) - topLeft;
|
||||
VECTOR2I br_delta = grid.AlignGrid( bottomRight, selectionGrid ) - bottomRight;
|
||||
|
||||
if( tl_delta != VECTOR2I( 0, 0 ) || br_delta != VECTOR2I( 0, 0 ) )
|
||||
callbacks.m_updateItem =
|
||||
[&]( EDA_ITEM* aItem )
|
||||
{
|
||||
doMoveItem( sheet, tl_delta );
|
||||
updateItem( aItem, true );
|
||||
};
|
||||
|
||||
VECTOR2I newSize = (VECTOR2I) sheet->GetSize() - tl_delta + br_delta;
|
||||
sheet->SetSize( VECTOR2I( newSize.x, newSize.y ) );
|
||||
updateItem( sheet, true );
|
||||
}
|
||||
|
||||
for( SCH_SHEET_PIN* pin : sheet->GetPins() )
|
||||
{
|
||||
VECTOR2I newPos;
|
||||
|
||||
if( pin->GetSide() == SHEET_SIDE::TOP || pin->GetSide() == SHEET_SIDE::LEFT )
|
||||
newPos = pin->GetPosition() + tl_delta;
|
||||
else
|
||||
newPos = pin->GetPosition() + br_delta;
|
||||
|
||||
VECTOR2I delta = grid.AlignGrid( newPos - pin->GetPosition(), selectionGrid );
|
||||
|
||||
if( delta != VECTOR2I( 0, 0 ) )
|
||||
{
|
||||
EDA_ITEMS drag_items;
|
||||
getConnectedDragItems( &commit, pin, pin->GetConnectionPoints()[0],
|
||||
drag_items );
|
||||
|
||||
doMoveItem( pin, delta );
|
||||
|
||||
for( EDA_ITEM* dragItem : drag_items )
|
||||
{
|
||||
if( dragItem->GetParent() && dragItem->GetParent()->IsSelected() )
|
||||
continue;
|
||||
|
||||
doMoveItem( dragItem, delta );
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
SCH_ITEM* schItem = static_cast<SCH_ITEM*>( item );
|
||||
std::vector<VECTOR2I> connections = schItem->GetConnectionPoints();
|
||||
EDA_ITEMS drag_items;
|
||||
|
||||
for( const VECTOR2I& point : connections )
|
||||
getConnectedDragItems( &commit, schItem, point, drag_items );
|
||||
|
||||
std::map<VECTOR2I, int> shifts;
|
||||
VECTOR2I most_common( 0, 0 );
|
||||
int max_count = 0;
|
||||
|
||||
for( const VECTOR2I& conn : connections )
|
||||
{
|
||||
VECTOR2I gridpt = grid.AlignGrid( conn, selectionGrid ) - conn;
|
||||
|
||||
shifts[gridpt]++;
|
||||
|
||||
if( shifts[gridpt] > max_count )
|
||||
{
|
||||
most_common = gridpt;
|
||||
max_count = shifts[most_common];
|
||||
}
|
||||
}
|
||||
|
||||
if( most_common != VECTOR2I( 0, 0 ) )
|
||||
{
|
||||
doMoveItem( item, most_common );
|
||||
|
||||
for( EDA_ITEM* dragItem : drag_items )
|
||||
{
|
||||
if( dragItem->GetParent() && dragItem->GetParent()->IsSelected() )
|
||||
continue;
|
||||
|
||||
doMoveItem( dragItem, most_common );
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
std::vector<EDA_ITEM*> items( selection.begin(), selection.end() );
|
||||
AlignSchematicItemsToGrid( m_frame->GetScreen(), items, grid, selectionGrid, callbacks );
|
||||
|
||||
SCH_LINE_WIRE_BUS_TOOL* lwbTool = m_toolMgr->GetTool<SCH_LINE_WIRE_BUS_TOOL>();
|
||||
lwbTool->TrimOverLappingWires( &commit, &selection );
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
(kicad_sch
|
||||
(version 20260101)
|
||||
(generator "eeschema")
|
||||
(generator_version "9.99")
|
||||
(uuid "0c75c306-927c-401a-9754-31b7a0ff1a88")
|
||||
(paper "A4")
|
||||
(lib_symbols)
|
||||
(wire
|
||||
(pts
|
||||
(xy 115.57 106.68) (xy 127.635 106.68)
|
||||
)
|
||||
(stroke
|
||||
(width 0)
|
||||
(type default)
|
||||
)
|
||||
(uuid "15674a81-6897-4411-83ee-04c3fc99c7cd")
|
||||
)
|
||||
(wire
|
||||
(pts
|
||||
(xy 115.57 104.14) (xy 127.635 104.14)
|
||||
)
|
||||
(stroke
|
||||
(width 0)
|
||||
(type default)
|
||||
)
|
||||
(uuid "3d864797-f502-4c0c-808e-aff8299e0a14")
|
||||
)
|
||||
(sheet
|
||||
(at 80.01 96.52)
|
||||
(size 35.56 19.05)
|
||||
(exclude_from_sim no)
|
||||
(in_bom yes)
|
||||
(on_board yes)
|
||||
(dnp no)
|
||||
(fields_autoplaced yes)
|
||||
(stroke
|
||||
(width 0.1524)
|
||||
(type solid)
|
||||
)
|
||||
(fill
|
||||
(color 0 0 0 0)
|
||||
)
|
||||
(uuid "55d5f2c9-5bd3-4e5f-83c9-32f32b2c9868")
|
||||
(property "Sheetname" "sheet1"
|
||||
(at 80.01 95.8084 0)
|
||||
(show_name no)
|
||||
(do_not_autoplace no)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left bottom)
|
||||
)
|
||||
)
|
||||
(property "Sheetfile" "sheet1.kicad_sch"
|
||||
(at 80.01 116.1546 0)
|
||||
(show_name no)
|
||||
(do_not_autoplace no)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left top)
|
||||
)
|
||||
)
|
||||
(pin "Pin1" output
|
||||
(at 115.57 104.14 0)
|
||||
(uuid "e062f655-b6e4-4db7-96a1-3cfedf861493")
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify right)
|
||||
)
|
||||
)
|
||||
(pin "Pin2" output
|
||||
(at 115.57 106.68 0)
|
||||
(uuid "f3b189f5-5fdc-4781-841f-d565333cd3b6")
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify right)
|
||||
)
|
||||
)
|
||||
(instances
|
||||
(project ""
|
||||
(path "/0c75c306-927c-401a-9754-31b7a0ff1a88"
|
||||
(page "2")
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(sheet
|
||||
(at 127.635 95.885)
|
||||
(size 35.56 19.05)
|
||||
(exclude_from_sim no)
|
||||
(in_bom yes)
|
||||
(on_board yes)
|
||||
(dnp no)
|
||||
(fields_autoplaced yes)
|
||||
(stroke
|
||||
(width 0.1524)
|
||||
(type solid)
|
||||
)
|
||||
(fill
|
||||
(color 0 0 0 0)
|
||||
)
|
||||
(uuid "c63030d0-0d12-481f-8161-de2cf4c2b48d")
|
||||
(property "Sheetname" "sheet2"
|
||||
(at 127.635 95.1734 0)
|
||||
(show_name no)
|
||||
(do_not_autoplace no)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left bottom)
|
||||
)
|
||||
)
|
||||
(property "Sheetfile" "sheet2.kicad_sch"
|
||||
(at 127.635 115.5196 0)
|
||||
(show_name no)
|
||||
(do_not_autoplace no)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left top)
|
||||
)
|
||||
)
|
||||
(pin "Pin1" input
|
||||
(at 127.635 104.14 180)
|
||||
(uuid "5bed9a7f-58fe-4682-80d7-a4489dec6b0f")
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
)
|
||||
(pin "Pin2" input
|
||||
(at 127.635 106.68 180)
|
||||
(uuid "25c59901-73dc-4e3e-a504-821e3ba9826a")
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
)
|
||||
(instances
|
||||
(project "Test_Move_Grid"
|
||||
(path "/0c75c306-927c-401a-9754-31b7a0ff1a88"
|
||||
(page "3")
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(sheet_instances
|
||||
(path "/"
|
||||
(page "1")
|
||||
)
|
||||
)
|
||||
(embedded_fonts no)
|
||||
)
|
||||
@@ -0,0 +1,30 @@
|
||||
(kicad_sch
|
||||
(version 20260101)
|
||||
(generator "eeschema")
|
||||
(generator_version "9.99")
|
||||
(uuid "a0430ec0-779e-485a-815a-9819eac4b79a")
|
||||
(paper "A4")
|
||||
(lib_symbols)
|
||||
(hierarchical_label "Pin1"
|
||||
(shape output)
|
||||
(at 140.97 119.38 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
(uuid "249cc818-53d2-4783-8c71-37e8bb19c5ab")
|
||||
)
|
||||
(hierarchical_label "Pin2"
|
||||
(shape output)
|
||||
(at 140.97 121.92 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
(uuid "7797a5f3-f86a-4ff0-8da8-0603742288a3")
|
||||
)
|
||||
)
|
||||
@@ -0,0 +1,30 @@
|
||||
(kicad_sch
|
||||
(version 20260101)
|
||||
(generator "eeschema")
|
||||
(generator_version "9.99")
|
||||
(uuid "a0430ec0-779e-485a-815a-9819eac4b79a")
|
||||
(paper "A4")
|
||||
(lib_symbols)
|
||||
(hierarchical_label "Pin1"
|
||||
(shape input)
|
||||
(at 139.7 106.68 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
(uuid "11ef11a3-f753-4581-8ee8-4bf6eda9df04")
|
||||
)
|
||||
(hierarchical_label "Pin2"
|
||||
(shape input)
|
||||
(at 139.7 109.22 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
(uuid "4502b5af-3cef-4777-b471-e303174db36e")
|
||||
)
|
||||
)
|
||||
@@ -76,6 +76,7 @@ set( QA_EESCHEMA_SRCS
|
||||
test_issue22286_pin_alternate.cpp
|
||||
test_issue22620_group_annotation.cpp
|
||||
test_issue22651_sheet_annotation.cpp
|
||||
test_issue22864_align_sheet_pins.cpp
|
||||
test_issue22576_subsheet_standalone.cpp
|
||||
test_issue16915_bus_netclass.cpp
|
||||
test_nc_pin_connectivity.cpp
|
||||
|
||||
@@ -0,0 +1,267 @@
|
||||
/*
|
||||
* This program source code file is part of KiCad, a free EDA CAD application.
|
||||
*
|
||||
* Copyright The KiCad Developers, see AUTHORS.TXT for contributors.
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or
|
||||
* modify it under the terms of the GNU General Public License
|
||||
* as published by the Free Software Foundation; either version 2
|
||||
* of the License, or (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with this program; if not, you may find one here:
|
||||
* http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
|
||||
* or you may search the http://www.gnu.org website for the version 2 license,
|
||||
* or you may write to the Free Software Foundation, Inc.,
|
||||
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
|
||||
*/
|
||||
|
||||
/**
|
||||
* @file test_issue22864_align_sheet_pins.cpp
|
||||
*
|
||||
* Test for issue #22864: "Align Items to Grid" disconnects wires from hierarchical sheet pins.
|
||||
*
|
||||
* Bug scenario:
|
||||
* - Sheet2 is positioned off-grid at (127.635mm, 95.885mm)
|
||||
* - Sheet2 has pins on the left side connected to wires from Sheet1
|
||||
* - When aligning Sheet2 to grid, the sheet and pins move but the wire endpoints
|
||||
* should also move to maintain connectivity
|
||||
*
|
||||
* Root cause: The alignment code incorrectly passed a delta vector to AlignGrid()
|
||||
* instead of a position. AlignGrid() snaps positions to grid, so passing a small
|
||||
* delta like (0.635mm, -0.635mm) results in snapping to (0, 0), causing no movement.
|
||||
*/
|
||||
|
||||
#include <qa_utils/wx_utils/unit_test_utils.h>
|
||||
#include <schematic_utils/schematic_file_util.h>
|
||||
|
||||
#include <schematic.h>
|
||||
#include <sch_sheet.h>
|
||||
#include <sch_sheet_path.h>
|
||||
#include <sch_sheet_pin.h>
|
||||
#include <sch_screen.h>
|
||||
#include <sch_line.h>
|
||||
#include <settings/settings_manager.h>
|
||||
#include <locale_io.h>
|
||||
#include <base_units.h>
|
||||
|
||||
#include <sch_item_alignment.h>
|
||||
#include <tools/ee_grid_helper.h>
|
||||
|
||||
|
||||
struct ISSUE22864_FIXTURE
|
||||
{
|
||||
ISSUE22864_FIXTURE() { }
|
||||
|
||||
SETTINGS_MANAGER m_settingsManager;
|
||||
std::unique_ptr<SCHEMATIC> m_schematic;
|
||||
};
|
||||
|
||||
|
||||
BOOST_AUTO_TEST_SUITE( Issue22864AlignSheetPins )
|
||||
|
||||
|
||||
/**
|
||||
* Helper class to test grid alignment.
|
||||
*
|
||||
* Creates an EE_GRID_HELPER that returns a fixed grid size for all grid types,
|
||||
* allowing testing without a full tool manager.
|
||||
*/
|
||||
class TEST_GRID_HELPER : public EE_GRID_HELPER
|
||||
{
|
||||
public:
|
||||
TEST_GRID_HELPER( const VECTOR2D& aGridSize ) :
|
||||
EE_GRID_HELPER(),
|
||||
m_gridSize( aGridSize )
|
||||
{
|
||||
}
|
||||
|
||||
VECTOR2D GetGridSize( GRID_HELPER_GRIDS aGrid ) const override
|
||||
{
|
||||
return m_gridSize;
|
||||
}
|
||||
|
||||
private:
|
||||
VECTOR2D m_gridSize;
|
||||
};
|
||||
|
||||
|
||||
/**
|
||||
* Test that AlignSchematicItemsToGrid properly aligns sheet pins and connected wires.
|
||||
*
|
||||
* This test verifies that when a sheet is aligned to grid:
|
||||
* 1. The sheet position moves to grid
|
||||
* 2. The sheet pins move to grid
|
||||
* 3. Wires connected to the pins also move to maintain connectivity
|
||||
*/
|
||||
BOOST_FIXTURE_TEST_CASE( Issue22864SheetPinAlignment, ISSUE22864_FIXTURE )
|
||||
{
|
||||
LOCALE_IO dummy;
|
||||
|
||||
KI_TEST::LoadSchematic( m_settingsManager, "issue22864/Test_Move_Grid", m_schematic );
|
||||
|
||||
// Use RootScreen() to get the actual content screen (not the virtual root)
|
||||
SCH_SCREEN* screen = m_schematic->RootScreen();
|
||||
BOOST_REQUIRE( screen != nullptr );
|
||||
|
||||
// Find sheet2 (the one at off-grid position 127.635mm, 95.885mm)
|
||||
SCH_SHEET* sheet2 = nullptr;
|
||||
int sheetCount = 0;
|
||||
|
||||
for( SCH_ITEM* item : screen->Items().OfType( SCH_SHEET_T ) )
|
||||
{
|
||||
SCH_SHEET* sheet = static_cast<SCH_SHEET*>( item );
|
||||
sheetCount++;
|
||||
BOOST_TEST_MESSAGE( "Found sheet: \"" << sheet->GetName() << "\" at ("
|
||||
<< sheet->GetPosition().x << ", " << sheet->GetPosition().y << ")" );
|
||||
|
||||
if( sheet->GetName() == "sheet2" )
|
||||
{
|
||||
sheet2 = sheet;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
BOOST_TEST_MESSAGE( "Total sheets found: " << sheetCount );
|
||||
BOOST_REQUIRE_MESSAGE( sheet2 != nullptr, "Could not find sheet named 'sheet2'" );
|
||||
|
||||
// Verify initial off-grid position
|
||||
// 127.635mm = 1276350 IU, 95.885mm = 958850 IU
|
||||
VECTOR2I initialPos = sheet2->GetPosition();
|
||||
BOOST_TEST_MESSAGE( "Sheet2 initial position: (" << initialPos.x << ", " << initialPos.y << ")" );
|
||||
|
||||
// Verify sheet has pins
|
||||
std::vector<SCH_SHEET_PIN*> pins = sheet2->GetPins();
|
||||
BOOST_REQUIRE( pins.size() >= 2 );
|
||||
|
||||
// Record initial pin positions
|
||||
std::map<SCH_SHEET_PIN*, VECTOR2I> initialPinPositions;
|
||||
|
||||
for( SCH_SHEET_PIN* pin : pins )
|
||||
initialPinPositions[pin] = pin->GetPosition();
|
||||
|
||||
// Find wires connected to the pins
|
||||
std::map<SCH_SHEET_PIN*, SCH_LINE*> connectedWires;
|
||||
|
||||
for( SCH_ITEM* item : screen->Items().OfType( SCH_LINE_T ) )
|
||||
{
|
||||
SCH_LINE* wire = static_cast<SCH_LINE*>( item );
|
||||
|
||||
if( wire->GetLayer() != LAYER_WIRE )
|
||||
continue;
|
||||
|
||||
for( SCH_SHEET_PIN* pin : pins )
|
||||
{
|
||||
VECTOR2I pinPos = pin->GetPosition();
|
||||
|
||||
if( wire->GetStartPoint() == pinPos || wire->GetEndPoint() == pinPos )
|
||||
connectedWires[pin] = wire;
|
||||
}
|
||||
}
|
||||
|
||||
BOOST_REQUIRE( !connectedWires.empty() );
|
||||
|
||||
// Record initial wire endpoints that connect to pins
|
||||
std::map<SCH_SHEET_PIN*, VECTOR2I> initialWireEndpoints;
|
||||
|
||||
for( const auto& [pin, wire] : connectedWires )
|
||||
{
|
||||
VECTOR2I pinPos = pin->GetPosition();
|
||||
|
||||
if( wire->GetStartPoint() == pinPos )
|
||||
initialWireEndpoints[pin] = wire->GetStartPoint();
|
||||
else
|
||||
initialWireEndpoints[pin] = wire->GetEndPoint();
|
||||
}
|
||||
|
||||
// Standard eeschema grid is 2.54mm = 25400 IU (100 mils)
|
||||
const VECTOR2D gridSize( schIUScale.mmToIU( 2.54 ), schIUScale.mmToIU( 2.54 ) );
|
||||
TEST_GRID_HELPER grid( gridSize );
|
||||
|
||||
// Track all move operations
|
||||
std::map<EDA_ITEM*, std::vector<VECTOR2I>> moveDeltas;
|
||||
|
||||
SCH_ALIGNMENT_CALLBACKS callbacks;
|
||||
|
||||
callbacks.m_doMoveItem = [&]( EDA_ITEM* aItem, const VECTOR2I& aDelta )
|
||||
{
|
||||
moveDeltas[aItem].push_back( aDelta );
|
||||
|
||||
if( aItem->Type() == SCH_SHEET_T )
|
||||
static_cast<SCH_SHEET*>( aItem )->Move( aDelta );
|
||||
else if( aItem->Type() == SCH_SHEET_PIN_T )
|
||||
static_cast<SCH_SHEET_PIN*>( aItem )->Move( aDelta );
|
||||
else if( aItem->Type() == SCH_LINE_T )
|
||||
static_cast<SCH_LINE*>( aItem )->Move( aDelta );
|
||||
else
|
||||
static_cast<SCH_ITEM*>( aItem )->Move( aDelta );
|
||||
};
|
||||
|
||||
// Note: We don't provide m_getConnectedDragItems for this test
|
||||
// The alignment function should still work, but won't drag connected items
|
||||
// The bug we're testing is in the delta calculation, not the drag behavior
|
||||
|
||||
// Create selection with just sheet2
|
||||
std::vector<EDA_ITEM*> selection{ sheet2 };
|
||||
|
||||
// Call the alignment function
|
||||
AlignSchematicItemsToGrid( screen, selection, grid, GRID_CONNECTABLE, callbacks );
|
||||
|
||||
// Verify sheet2 moved
|
||||
VECTOR2I finalPos = sheet2->GetPosition();
|
||||
BOOST_TEST_MESSAGE( "Sheet2 final position: (" << finalPos.x << ", " << finalPos.y << ")" );
|
||||
|
||||
// Sheet should have moved (initial position was off-grid)
|
||||
bool sheetMoved = ( finalPos != initialPos );
|
||||
BOOST_CHECK_MESSAGE( sheetMoved, "Sheet should have moved to align to grid" );
|
||||
|
||||
// Verify sheet position is now on grid
|
||||
VECTOR2I expectedSheetPos(
|
||||
KiROUND( initialPos.x / gridSize.x ) * static_cast<int>( gridSize.x ),
|
||||
KiROUND( initialPos.y / gridSize.y ) * static_cast<int>( gridSize.y ) );
|
||||
|
||||
BOOST_CHECK_EQUAL( finalPos.x, expectedSheetPos.x );
|
||||
BOOST_CHECK_EQUAL( finalPos.y, expectedSheetPos.y );
|
||||
|
||||
// The critical bug test: pins should end up ON GRID after alignment
|
||||
// With the buggy code, the pin delta calculation uses AlignGrid(delta) instead of
|
||||
// AlignGrid(position) - delta, which causes small deltas to snap to (0,0)
|
||||
// This means pins don't get properly aligned to grid
|
||||
for( SCH_SHEET_PIN* pin : pins )
|
||||
{
|
||||
VECTOR2I pinPos = pin->GetPosition();
|
||||
BOOST_TEST_MESSAGE( "Pin final position: (" << pinPos.x << ", " << pinPos.y << ")" );
|
||||
|
||||
// Calculate what the Y position should be if properly aligned to grid
|
||||
int gridInt = static_cast<int>( gridSize.y );
|
||||
int expectedPinY = KiROUND( static_cast<double>( pinPos.y ) / gridInt ) * gridInt;
|
||||
|
||||
// The pin Y position should be exactly on grid
|
||||
// With the buggy code, this check FAILS because the pin doesn't get
|
||||
// the additional alignment delta applied
|
||||
bool pinYOnGrid = ( pinPos.y == expectedPinY );
|
||||
|
||||
BOOST_CHECK_MESSAGE( pinYOnGrid,
|
||||
"Pin Y position should be on grid after alignment. "
|
||||
"Actual Y: " << pinPos.y << ", "
|
||||
"Nearest grid Y: " << expectedPinY << ", "
|
||||
"Difference: " << ( pinPos.y - expectedPinY ) );
|
||||
|
||||
// Also check X position is on grid (sheet edge which should be on grid)
|
||||
int expectedPinX = KiROUND( static_cast<double>( pinPos.x ) / gridSize.x ) * static_cast<int>( gridSize.x );
|
||||
bool pinXOnGrid = ( pinPos.x == expectedPinX );
|
||||
|
||||
BOOST_CHECK_MESSAGE( pinXOnGrid,
|
||||
"Pin X position should be on grid after alignment. "
|
||||
"Actual X: " << pinPos.x << ", "
|
||||
"Nearest grid X: " << expectedPinX );
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
||||
Reference in New Issue
Block a user