fix: apply Y-axis flip in WireDragger.pin_world_xy

Library symbol pins are stored Y-up (positive Y is upward in the symbol
editor's coordinate system) but `.kicad_sch` is Y-down (positive Y is
downward in the schematic). `pin_world_xy` was returning `sym_y + ry`
without negating the rotated lib Y, so for any non-symmetric symbol
pin 1 and pin 2 ended up at swapped world positions.

For symmetric two-pin passives (R, C non-polarized) this was invisible
because pin 1 and pin 2 are electrically equivalent. For polarized
parts — electrolytic and polymer caps, diodes, MOSFETs, BJTs — it
silently swapped polarity. A label snapped to a polarized cap's pin 1
ended up on pin 2, which is catastrophic at first power-up.

The order matches eeschema's actual transformation:
  mirror in lib space → Y-flip to screen → rotate → translate.

The existing regression test in test_pin_locator_y_flip.py was already
written with the correct expected coordinates but the matching code fix
was never landed; that test now passes.

Three tests in test_move_with_wire_preservation.py had baked the buggy
expected coordinates into their assertions; updated those to the correct
y-flipped values. The touching-pin fixture had to flip R2's Y from
-7.62 to +7.62 so the two pins still meet under the corrected formula.

Verified end-to-end on a 46-component aerospace PDB schematic: all
8 polarized-part pins (4 polymer caps + 4 TVS diodes) now produce
world coordinates that match the labels actually placed in the file.
This commit is contained in:
Matthew Runo
2026-05-01 10:02:13 -07:00
parent d3c01e20bd
commit bf74b85caf
2 changed files with 28 additions and 31 deletions

View File

@@ -152,15 +152,16 @@ class WireDragger:
""" """
Compute the world coordinate of a pin given the symbol transform. Compute the world coordinate of a pin given the symbol transform.
KiCAD applies mirror first (in local space), then rotation, then translation. Library pins are stored Y-up; the schematic is Y-down. Order matches
mirror_x negates the local X axis; mirror_y negates the local Y axis. eeschema: mirror in lib space → Y-flip to screen → rotate → translate.
Without the Y-flip, polarized parts get pin 1/pin 2 silently swapped.
""" """
lx, ly = px, py lx, ly = px, py
if mirror_x: if mirror_x:
lx = -lx lx = -lx
if mirror_y: if mirror_y:
ly = -ly ly = -ly
rx, ry = _rotate(lx, ly, rotation) rx, ry = _rotate(lx, -ly, rotation)
return sym_x + rx, sym_y + ry return sym_x + rx, sym_y + ry
@staticmethod @staticmethod
@@ -260,8 +261,7 @@ class WireDragger:
# Remove existing (mirror ...) token(s) # Remove existing (mirror ...) token(s)
to_remove = [ to_remove = [
i for i, sub in enumerate(item) i for i, sub in enumerate(item) if isinstance(sub, list) and sub and sub[0] == mirror_k
if isinstance(sub, list) and sub and sub[0] == mirror_k
] ]
for i in reversed(to_remove): for i in reversed(to_remove):
del item[i] del item[i]

View File

@@ -201,35 +201,34 @@ class TestFindSymbol:
@pytest.mark.unit @pytest.mark.unit
class TestComputePinPositions: class TestComputePinPositions:
def test_resistor_at_origin_no_rotation(self) -> None: def test_resistor_at_origin_no_rotation(self) -> None:
"""Device:R at (0, 0) rot=0 pins at (0, 3.81) and (0, -3.81).""" """Device:R at (0, 0) rot=0. Lib pins are Y-up; schematic is Y-down,
so pin 1 (lib y=+3.81) lands at world y=-3.81 and pin 2 (lib y=-3.81)
at world y=+3.81. See test_pin_locator_y_flip for the canonical case."""
sch = _make_sch_data([_make_symbol("R1", 0, 0)]) sch = _make_sch_data([_make_symbol("R1", 0, 0)])
positions = WireDragger.compute_pin_positions(sch, "R1", 10, 20) positions = WireDragger.compute_pin_positions(sch, "R1", 10, 20)
assert "1" in positions and "2" in positions assert "1" in positions and "2" in positions
old1, new1 = positions["1"] old1, new1 = positions["1"]
old2, new2 = positions["2"] old2, new2 = positions["2"]
# Pin 1 old: (0 + 0, 0 + 3.81) # Pin 1 old: (0, 0 - 3.81)
assert abs(old1[0] - 0) < 1e-4 assert abs(old1[0] - 0) < 1e-4
assert abs(old1[1] - 3.81) < 1e-4 assert abs(old1[1] - (-3.81)) < 1e-4
# Pin 2 old: (0 + 0, 0 - 3.81) # Pin 2 old: (0, 0 + 3.81)
assert abs(old2[0] - 0) < 1e-4 assert abs(old2[0] - 0) < 1e-4
assert abs(old2[1] - (-3.81)) < 1e-4 assert abs(old2[1] - 3.81) < 1e-4
# New positions shifted by (10, 20) # New positions shifted by (10, 20)
assert abs(new1[0] - 10) < 1e-4 assert abs(new1[0] - 10) < 1e-4
assert abs(new1[1] - 23.81) < 1e-4 assert abs(new1[1] - 16.19) < 1e-4
assert abs(new2[0] - 10) < 1e-4 assert abs(new2[0] - 10) < 1e-4
assert abs(new2[1] - 16.19) < 1e-4 assert abs(new2[1] - 23.81) < 1e-4
def test_resistor_rotated_90(self) -> None: def test_resistor_rotated_90(self) -> None:
"""Device:R at (100, 100) rot=90 — pins should be at (100+3.81, 100) and (100-3.81, 100).""" """Device:R at (100, 100) rot=90. Pin 1 lib (0, +3.81) → y-flipped to
(0, -3.81) → rotated 90° → (3.81, 0) → world (103.81, 100)."""
sch = _make_sch_data([_make_symbol("R1", 100, 100, rotation=90)]) sch = _make_sch_data([_make_symbol("R1", 100, 100, rotation=90)])
positions = WireDragger.compute_pin_positions(sch, "R1", 100, 100) positions = WireDragger.compute_pin_positions(sch, "R1", 100, 100)
old1, _ = positions["1"] old1, _ = positions["1"]
old2, _ = positions["2"] old2, _ = positions["2"]
# rotate(0, 3.81, 90) = (0*cos90 - 3.81*sin90, 0*sin90 + 3.81*cos90) = (-3.81, 0) assert abs(old1[0] - 103.81) < 1e-3
# Wait — pin 1 is at local (0, 3.81), rotated 90° CCW:
# x' = 0*cos90 - 3.81*sin90 = -3.81, y' = 0*sin90 + 3.81*cos90 ≈ 0
# world: (100 - 3.81, 100 + 0) = (96.19, 100)
assert abs(old1[0] - 96.19) < 1e-3
assert abs(old1[1] - 100) < 1e-3 assert abs(old1[1] - 100) < 1e-3
def test_returns_empty_for_missing_component(self) -> None: def test_returns_empty_for_missing_component(self) -> None:
@@ -691,19 +690,18 @@ class TestSynthesizeTouchingPinWires:
def test_touching_pin_gap_generates_wire(self) -> None: def test_touching_pin_gap_generates_wire(self) -> None:
""" """
R1 at (0, 0) pin2 at (0, -3.81). With Y-flip applied (lib Y-up → schematic Y-down):
R2 at (0, -7.62) pin1 at (0, -3.81). ← pins touch R1 at (0, 0) pin2 (lib y=-3.81) lands at world (0, +3.81).
Moving R1 to (10, 0) causes pin2 to move to (10, -3.81). R2 at (0, +7.62) pin1 (lib y=+3.81) lands at world (0, +3.81). ← pins touch
A wire from (0, -3.81) to (10, -3.81) should be synthesized. Moving R1 to (10, 0) drags pin2 to (10, +3.81).
A wire from (0, +3.81) to (10, +3.81) should be synthesized.
""" """
# R2 pin1 is at (0, -7.62 + 3.81) = (0, -3.81) sch = self._make_two_resistors(0, 0, 0, 7.62)
sch = self._make_two_resistors(0, 0, 0, -7.62)
# Verify the touching: R1 pin2 old = (0, -3.81), R2 pin1 = (0, -3.81)
pin_positions = WireDragger.compute_pin_positions(sch, "R1", 10, 0) pin_positions = WireDragger.compute_pin_positions(sch, "R1", 10, 0)
old2, new2 = pin_positions["2"] old2, new2 = pin_positions["2"]
assert abs(old2[0] - 0) < 1e-3 and abs(old2[1] - (-3.81)) < 1e-3 assert abs(old2[0] - 0) < 1e-3 and abs(old2[1] - 3.81) < 1e-3
assert abs(new2[0] - 10) < 1e-3 and abs(new2[1] - (-3.81)) < 1e-3 assert abs(new2[0] - 10) < 1e-3 and abs(new2[1] - 3.81) < 1e-3
wire_count_before = sum( wire_count_before = sum(
1 for item in sch if isinstance(item, list) and item and item[0] == _sym("wire") 1 for item in sch if isinstance(item, list) and item and item[0] == _sym("wire")
@@ -716,7 +714,6 @@ class TestSynthesizeTouchingPinWires:
] ]
assert len(wires) == wire_count_before + 1 assert len(wires) == wire_count_before + 1
# The new wire should span (0, -3.81) → (10, -3.81)
new_wire = wires[-1] new_wire = wires[-1]
pts = next(s for s in new_wire[1:] if isinstance(s, list) and s and s[0] == _sym("pts")) pts = next(s for s in new_wire[1:] if isinstance(s, list) and s and s[0] == _sym("pts"))
xys = [p for p in pts[1:] if isinstance(p, list) and len(p) >= 3 and p[0] == _sym("xy")] xys = [p for p in pts[1:] if isinstance(p, list) and len(p) >= 3 and p[0] == _sym("xy")]
@@ -725,11 +722,11 @@ class TestSynthesizeTouchingPinWires:
(round(float(xys[0][1]), 3), round(float(xys[0][2]), 3)), (round(float(xys[0][1]), 3), round(float(xys[0][2]), 3)),
(round(float(xys[1][1]), 3), round(float(xys[1][2]), 3)), (round(float(xys[1][1]), 3), round(float(xys[1][2]), 3)),
} }
assert (0.0, -3.81) in endpoints, f"Expected (0, -3.81) in wire endpoints, got {endpoints}" assert (0.0, 3.81) in endpoints, f"Expected (0, 3.81) in wire endpoints, got {endpoints}"
assert ( assert (
10.0, 10.0,
-3.81, 3.81,
) in endpoints, f"Expected (10, -3.81) in wire endpoints, got {endpoints}" ) in endpoints, f"Expected (10, 3.81) in wire endpoints, got {endpoints}"
def test_no_wire_when_pin_didnt_move(self) -> None: def test_no_wire_when_pin_didnt_move(self) -> None:
""" """