Fix Move command grabbing from grid point instead of anchor
The Move command grabbed footprints from the nearest grid point rather than the footprint anchor. This was caused by the reference point being grid-aligned at drag start (commit89be3fd390), which was a workaround for fractional-nanometer positioning errors with non-page display origin. Fix the root cause instead: AlignGrid(VECTOR2I, VECTOR2D, VECTOR2D) relied on implicit VECTOR2D->VECTOR2I truncation to pass grid parameters to computeNearest. For grid sizes that aren't exact in IEEE 754 double (e.g., 0.254mm = 10 mil becomes 253999.999... instead of 254000), the truncation produces incorrect grid sizes. Use KiROUND for the conversion so all grid operations produce exact grid multiples. With AlignGrid returning correct positions, the move tool no longer needs to grid-align its reference point. Restore the anchor-based grab and the GetMoveWarpsCursor() preference that89be3fd390removed. Fixes https://gitlab.com/kicad/code/kicad/-/issues/23308
This commit is contained in:
@@ -434,7 +434,11 @@ VECTOR2I GRID_HELPER::AlignGrid( const VECTOR2I& aPoint ) const
|
||||
VECTOR2I GRID_HELPER::AlignGrid( const VECTOR2I& aPoint, const VECTOR2D& aGrid,
|
||||
const VECTOR2D& aOffset ) const
|
||||
{
|
||||
return computeNearest( aPoint, aGrid, aOffset );
|
||||
// Round the grid size and offset rather than relying on the implicit VECTOR2D->VECTOR2I
|
||||
// truncation in computeNearest. Grid sizes that aren't exact in IEEE 754 (e.g., 0.254mm =
|
||||
// 10 mil) would otherwise truncate to the wrong integer (253999 instead of 254000),
|
||||
// producing positions that aren't true grid multiples.
|
||||
return computeNearest( aPoint, KiROUND( aGrid ), KiROUND( aOffset ) );
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -1291,30 +1291,19 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit
|
||||
}
|
||||
else
|
||||
{
|
||||
// Get the best drag origin (nearest item anchor to where the user clicked)
|
||||
VECTOR2I dragOrigin = m_cursor;
|
||||
|
||||
// Grid-align the reference point so that movement deltas between
|
||||
// grid-snapped cursor positions remain on-grid. Without this, dragging
|
||||
// from a non-grid-aligned anchor (e.g. a pad center that doesn't fall on
|
||||
// the current grid) produces fractional-nanometer position errors that
|
||||
// become visible when Display Origin is set to Grid Origin or Aux Origin.
|
||||
VECTOR2I snappedRef = grid.AlignGrid( dragOrigin,
|
||||
grid.GetSelectionGrid( selection ) );
|
||||
selection.SetReferencePoint( snappedRef );
|
||||
selection.SetReferencePoint( dragOrigin );
|
||||
|
||||
// Set up construction/snap lines at the actual item position for visual
|
||||
// alignment, not the snapped position
|
||||
if( angleSnapMode != LEADER_MODE::DIRECT )
|
||||
grid.SetSnapLineOrigin( dragOrigin );
|
||||
|
||||
grid.SetAuxAxes( true, dragOrigin );
|
||||
|
||||
// Initialize m_cursor to the grid-aligned reference so that the first
|
||||
// movement delta (m_cursor - prevPos) is grid-aligned. Without this,
|
||||
// prevPos would be an unsnapped position and the first move would put
|
||||
// items off-grid.
|
||||
m_cursor = snappedRef;
|
||||
if( !editFrame->GetMoveWarpsCursor() )
|
||||
m_cursor = originalCursorPos;
|
||||
else
|
||||
m_cursor = dragOrigin;
|
||||
}
|
||||
|
||||
originalPos = selection.GetReferencePoint();
|
||||
|
||||
@@ -419,4 +419,70 @@ BOOST_AUTO_TEST_CASE( AlignGridWithNonPageOrigin )
|
||||
}
|
||||
|
||||
|
||||
BOOST_AUTO_TEST_CASE( MovementFromOffGridAnchor )
|
||||
{
|
||||
// Issue #23308 / #21800: When moving a footprint by its anchor, the reference point
|
||||
// should be the actual anchor (not grid-snapped). The final position must still land
|
||||
// exactly on-grid because AlignGrid now rounds VECTOR2D grid parameters correctly.
|
||||
|
||||
GRID_HELPER helper;
|
||||
helper.SetGridSnapping( true );
|
||||
|
||||
constexpr int IU_PER_MM = 1000000;
|
||||
|
||||
struct TestCase
|
||||
{
|
||||
const char* name;
|
||||
double gridSizeMM;
|
||||
int anchorX;
|
||||
int anchorY;
|
||||
int mouseTargetX;
|
||||
int mouseTargetY;
|
||||
};
|
||||
|
||||
std::vector<TestCase> cases = {
|
||||
// Anchor at 0.05mm on a 0.1mm grid
|
||||
{ "0.1mm grid, anchor at half-grid",
|
||||
0.1, 50000, 50000, 300000, 300000 },
|
||||
|
||||
// Anchor at 0.127mm on a 0.254mm (10 mil) grid
|
||||
{ "10mil grid, anchor at half-grid",
|
||||
0.254, 127000, 127000, 762000, 508000 },
|
||||
|
||||
// Anchor at 0.3175mm on a 0.635mm (25 mil) grid
|
||||
{ "25mil grid, anchor at half-grid",
|
||||
0.635, 317500, 317500, 1905000, 1270000 },
|
||||
};
|
||||
|
||||
for( const auto& tc : cases )
|
||||
{
|
||||
BOOST_TEST_CONTEXT( tc.name )
|
||||
{
|
||||
int gridSizeIU = KiROUND( tc.gridSizeMM * IU_PER_MM );
|
||||
|
||||
VECTOR2D gridD( tc.gridSizeMM * IU_PER_MM, tc.gridSizeMM * IU_PER_MM );
|
||||
VECTOR2D offsetD( 0, 0 );
|
||||
|
||||
// Simulate the cursor snapping to grid at the target mouse position. This is
|
||||
// what BestSnapAnchor does during the move loop.
|
||||
VECTOR2I snappedTarget = helper.AlignGrid( VECTOR2I( tc.mouseTargetX, tc.mouseTargetY ),
|
||||
gridD, offsetD );
|
||||
|
||||
// The movement delta starts from the actual anchor (off-grid), not a grid-snapped ref
|
||||
VECTOR2I anchor( tc.anchorX, tc.anchorY );
|
||||
VECTOR2I movement = snappedTarget - anchor;
|
||||
VECTOR2I finalPos = anchor + movement;
|
||||
|
||||
// Final position must be exactly on-grid
|
||||
BOOST_CHECK_EQUAL( finalPos.x % gridSizeIU, 0 );
|
||||
BOOST_CHECK_EQUAL( finalPos.y % gridSizeIU, 0 );
|
||||
|
||||
// And it must equal the snapped target exactly
|
||||
BOOST_CHECK_EQUAL( finalPos.x, snappedTarget.x );
|
||||
BOOST_CHECK_EQUAL( finalPos.y, snappedTarget.y );
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
||||
Reference in New Issue
Block a user