diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 01d170f..b56f427 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -150,13 +150,90 @@ def _parse_no_connects(sexp_data: list) -> Set[Tuple[float, float]]: return positions -def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict[str, Dict]]: +def _parse_lib_symbol_graphics(symbol_def: list) -> List[Tuple[float, float]]: + """ + Parse graphical body elements from a lib_symbol definition and return + local-coordinate bounding points. + + Extracts points from rectangle, polyline, circle, arc, and bezier + elements found in sub-symbols (typically the ``_0_1`` layers that + contain body shapes). + + Returns a list of ``(x, y)`` points in local symbol coordinates. + """ + points: List[Tuple[float, float]] = [] + + def _extract_graphics_recursive(sexp: list) -> None: + if not isinstance(sexp, list) or len(sexp) == 0: + return + + tag = sexp[0] + + if tag == Symbol("rectangle"): + # (rectangle (start x y) (end x y) ...) + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) >= 3: + if sub[0] in (Symbol("start"), Symbol("end")): + points.append((float(sub[1]), float(sub[2]))) + + elif tag == Symbol("polyline"): + # (polyline (pts (xy x y) (xy x y) ...) ...) + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) > 0 and sub[0] == Symbol("pts"): + for pt in sub[1:]: + if isinstance(pt, list) and len(pt) >= 3 and pt[0] == Symbol("xy"): + points.append((float(pt[1]), float(pt[2]))) + + elif tag == Symbol("circle"): + # (circle (center x y) (radius r) ...) + cx, cy, r = 0.0, 0.0, 0.0 + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) >= 3 and sub[0] == Symbol("center"): + cx, cy = float(sub[1]), float(sub[2]) + elif isinstance(sub, list) and len(sub) >= 2 and sub[0] == Symbol("radius"): + r = float(sub[1]) + if r > 0: + points.extend([ + (cx - r, cy - r), + (cx + r, cy + r), + ]) + + elif tag == Symbol("arc"): + # (arc (start x y) (mid x y) (end x y) ...) + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) >= 3: + if sub[0] in (Symbol("start"), Symbol("mid"), Symbol("end")): + points.append((float(sub[1]), float(sub[2]))) + + elif tag == Symbol("bezier"): + # (bezier (pts (xy x y) ...) ...) + for sub in sexp[1:]: + if isinstance(sub, list) and len(sub) > 0 and sub[0] == Symbol("pts"): + for pt in sub[1:]: + if isinstance(pt, list) and len(pt) >= 3 and pt[0] == Symbol("xy"): + points.append((float(pt[1]), float(pt[2]))) + + else: + # Recurse into sub-symbols to find graphics in nested definitions + for sub in sexp[1:]: + if isinstance(sub, list): + _extract_graphics_recursive(sub) + + # Search the top-level symbol definition and its sub-symbols + for item in symbol_def[1:]: + if isinstance(item, list): + _extract_graphics_recursive(item) + + return points + + +def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict]: """ Walk the lib_symbols section of already-parsed sexp_data and return - pin definitions for every symbol definition. + pin definitions and graphics points for every symbol definition. Returns: - Dict mapping lib_id → pin definitions (pin_number → pin_data dict). + Dict mapping lib_id → {"pins": pin_defs, "graphics_points": [(x,y), ...]}. """ lib_symbols_section = None for item in sexp_data: @@ -168,12 +245,15 @@ def _extract_lib_symbols(sexp_data: list) -> Dict[str, Dict[str, Dict]]: if not lib_symbols_section: return {} - result: Dict[str, Dict[str, Dict]] = {} + result: Dict[str, Dict] = {} for item in lib_symbols_section[1:]: if (isinstance(item, list) and len(item) > 1 and item[0] == Symbol("symbol")): symbol_name = str(item[1]).strip('"') - result[symbol_name] = PinLocator.parse_symbol_definition(item) + result[symbol_name] = { + "pins": PinLocator.parse_symbol_definition(item), + "graphics_points": _parse_lib_symbol_graphics(item), + } return result @@ -258,21 +338,48 @@ def _aabb_overlap( return a[0] < b[2] and b[0] < a[2] and a[1] < b[3] and b[1] < a[3] +def _transform_local_point( + lx: float, ly: float, + sym_x: float, sym_y: float, + rotation: float, + mirror_x: bool, mirror_y: bool, +) -> Tuple[float, float]: + """ + Transform a point from local symbol coordinates to absolute schematic + coordinates using KiCad's transform order: mirror → rotate → translate. + """ + # Apply mirroring in local coords + if mirror_x: + ly = -ly + if mirror_y: + lx = -lx + + # Apply rotation + if rotation != 0: + lx, ly = PinLocator.rotate_point(lx, ly, rotation) + + return (sym_x + lx, sym_y + ly) + + def _compute_symbol_bbox_direct( sym: Dict[str, Any], pin_defs: Dict[str, Dict], margin: float = 0.0, + graphics_points: Optional[List[Tuple[float, float]]] = None, ) -> Optional[Tuple[float, float, float, float]]: """ - Compute bounding box of a symbol from its pin definitions and placement. + Compute bounding box of a symbol from its graphics and pin definitions. - Uses _compute_pin_positions_direct to get absolute pin positions, then - expands degenerate dimensions (pins in a line) to approximate body size. + When graphics_points are available (from lib_symbol body shapes), uses + those for the bbox and unions with pin positions. Falls back to + pin-only estimation with degenerate expansion when no graphics data + is available. Args: sym: Parsed symbol dict with x, y, rotation, mirror_x, mirror_y. pin_defs: Pin definitions from PinLocator.get_symbol_pins(). margin: Shrink bbox by this amount on each side (mm). + graphics_points: Local-coordinate points from symbol body graphics. Returns (min_x, min_y, max_x, max_y) in mm, or None if no pins. """ @@ -280,20 +387,39 @@ def _compute_symbol_bbox_direct( if not pin_positions: return None - xs = [p[0] for p in pin_positions.values()] - ys = [p[1] for p in pin_positions.values()] - min_x, min_y, max_x, max_y = min(xs), min(ys), max(xs), max(ys) + if graphics_points: + # Transform graphics points to absolute coordinates + sym_x, sym_y = sym["x"], sym["y"] + rotation = sym["rotation"] + mirror_x = sym.get("mirror_x", False) + mirror_y = sym.get("mirror_y", False) - # Expand degenerate dimensions (pins in a line) to approximate body size - min_body = 1.5 # mm minimum half-extent for component body - if max_x - min_x < 2 * min_body: - cx = (min_x + max_x) / 2 - min_x = cx - min_body - max_x = cx + min_body - if max_y - min_y < 2 * min_body: - cy = (min_y + max_y) / 2 - min_y = cy - min_body - max_y = cy + min_body + abs_points = [ + _transform_local_point(lx, ly, sym_x, sym_y, rotation, mirror_x, mirror_y) + for lx, ly in graphics_points + ] + + # Union with pin positions so pins extending beyond body are included + all_xs = [p[0] for p in abs_points] + [p[0] for p in pin_positions.values()] + all_ys = [p[1] for p in abs_points] + [p[1] for p in pin_positions.values()] + + min_x, min_y = min(all_xs), min(all_ys) + max_x, max_y = max(all_xs), max(all_ys) + else: + # Fallback: pin-only estimation with degenerate expansion + xs = [p[0] for p in pin_positions.values()] + ys = [p[1] for p in pin_positions.values()] + min_x, min_y, max_x, max_y = min(xs), min(ys), max(xs), max(ys) + + min_body = 1.5 # mm minimum half-extent for component body + if max_x - min_x < 2 * min_body: + cx = (min_x + max_x) / 2 + min_x = cx - min_body + max_x = cx + min_body + if max_y - min_y < 2 * min_body: + cy = (min_y + max_y) / 2 + min_y = cy - min_body + max_y = cy + min_body # Shrink bbox by margin min_x += margin @@ -372,7 +498,7 @@ def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: return True return False - lib_pin_defs = _extract_lib_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) unconnected = [] for sym in symbols: @@ -381,7 +507,8 @@ def find_unconnected_pins(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - pin_defs = lib_pin_defs.get(sym["lib_id"], {}) + lib_data = lib_defs.get(sym["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) if not pin_defs: continue @@ -434,7 +561,7 @@ def find_overlapping_elements( overlapping_labels = [] overlapping_wires = [] - lib_pin_defs = _extract_lib_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) # --- Symbol-symbol overlap using bounding-box intersection (O(n²)) --- non_template_symbols = [s for s in symbols if not s["reference"].startswith("_TEMPLATE") and s["reference"]] @@ -442,10 +569,12 @@ def find_overlapping_elements( # Pre-compute bounding boxes for all non-template symbols symbol_bboxes = [] for sym in non_template_symbols: - pin_defs = lib_pin_defs.get(sym["lib_id"], {}) + lib_data = lib_defs.get(sym["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) + graphics_points = lib_data.get("graphics_points", []) bbox = None if pin_defs: - bbox = _compute_symbol_bbox_direct(sym, pin_defs) + bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) symbol_bboxes.append((sym, bbox)) for i in range(len(symbol_bboxes)): @@ -589,7 +718,7 @@ def get_elements_in_region( wires = _parse_wires(sexp_data) labels = _parse_labels(sexp_data) - lib_pin_defs = _extract_lib_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) # Symbols: include if position is within bounds region_symbols = [] @@ -604,7 +733,8 @@ def get_elements_in_region( "isPower": sym["is_power"], } # Include pin positions (compute directly to handle unannotated duplicates) - pin_defs = lib_pin_defs.get(sym["lib_id"], {}) + lib_data = lib_defs.get(sym["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) if pin_defs: pin_positions = _compute_pin_positions_direct(sym, pin_defs) if pin_positions: @@ -710,7 +840,7 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: symbols = _parse_symbols(sexp_data) wires = _parse_wires(sexp_data) - lib_pin_defs = _extract_lib_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) margin = 0.5 # mm margin to shrink bbox (avoids false positives at pin tips) pin_tolerance = 0.05 # mm @@ -723,11 +853,13 @@ def find_wires_crossing_symbols(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - pin_defs = lib_pin_defs.get(sym["lib_id"], {}) + lib_data = lib_defs.get(sym["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) if not pin_defs: continue - bbox = _compute_symbol_bbox_direct(sym, pin_defs, margin=margin) + graphics_points = lib_data.get("graphics_points", []) + bbox = _compute_symbol_bbox_direct(sym, pin_defs, margin=margin, graphics_points=graphics_points) if bbox is None: continue diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index 9ec2740..4f31446 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -26,6 +26,8 @@ from commands.schematic_analysis import ( _parse_no_connects, _load_sexp, _extract_lib_symbols, + _parse_lib_symbol_graphics, + _transform_local_point, _line_segment_intersects_aabb, _point_in_rect, _distance, @@ -679,7 +681,7 @@ class TestExtractLibSymbols: )""") result = _extract_lib_symbols(sexp) assert "Device:R" in result - pins = result["Device:R"] + pins = result["Device:R"]["pins"] assert "1" in pins assert "2" in pins assert pins["1"]["y"] == pytest.approx(3.81) @@ -695,3 +697,236 @@ class TestExtractLibSymbols: )""") result = _extract_lib_symbols(sexp) assert result == {} + + def test_extract_includes_graphics_points(self): + """_extract_lib_symbols should return graphics_points from body shapes.""" + sexp = sexpdata.loads("""(kicad_sch + (lib_symbols + (symbol "Device:R" + (symbol "Device:R_0_1" + (rectangle (start -1.016 -2.54) (end 1.016 2.54) + (stroke (width 0.254) (type default)) + (fill (type none)))) + (symbol "Device:R_1_1" + (pin passive line (at 0 3.81 270) (length 1.27) + (name "~" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27))))) + (pin passive line (at 0 -3.81 90) (length 1.27) + (name "~" (effects (font (size 1.27 1.27)))) + (number "2" (effects (font (size 1.27 1.27))))))) + ) + )""") + result = _extract_lib_symbols(sexp) + lib_data = result["Device:R"] + assert "graphics_points" in lib_data + gfx = lib_data["graphics_points"] + assert len(gfx) >= 2 + # Rectangle corners should be present + xs = [p[0] for p in gfx] + ys = [p[1] for p in gfx] + assert pytest.approx(-1.016) in xs + assert pytest.approx(1.016) in xs + assert pytest.approx(-2.54) in ys + assert pytest.approx(2.54) in ys + + +# =================================================================== +# Unit tests — _parse_lib_symbol_graphics +# =================================================================== + +class TestParseLibSymbolGraphics: + """Test graphics extraction from lib_symbol definitions.""" + + def test_rectangle(self): + sexp = sexpdata.loads("""(symbol "Device:R" + (symbol "Device:R_0_1" + (rectangle (start -1.016 -2.54) (end 1.016 2.54) + (stroke (width 0.254) (type default)) + (fill (type none)))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert len(pts) == 2 + assert (-1.016, -2.54) in pts + assert (1.016, 2.54) in pts + + def test_polyline(self): + sexp = sexpdata.loads("""(symbol "Device:C" + (symbol "Device:C_0_1" + (polyline + (pts (xy -2.032 -0.762) (xy 2.032 -0.762)) + (stroke (width 0.508) (type default)) + (fill (type none)))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert (-2.032, -0.762) in pts + assert (2.032, -0.762) in pts + + def test_circle(self): + sexp = sexpdata.loads("""(symbol "Test:Circle" + (symbol "Test:Circle_0_1" + (circle (center 0 0) (radius 5) + (stroke (width 0.254) (type default)) + (fill (type none)))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert len(pts) == 2 + assert (-5.0, -5.0) in pts + assert (5.0, 5.0) in pts + + def test_arc(self): + sexp = sexpdata.loads("""(symbol "Test:Arc" + (symbol "Test:Arc_0_1" + (arc (start 1 0) (mid 0 1) (end -1 0) + (stroke (width 0.254) (type default)) + (fill (type none)))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert (1.0, 0.0) in pts + assert (0.0, 1.0) in pts + assert (-1.0, 0.0) in pts + + def test_no_graphics(self): + sexp = sexpdata.loads("""(symbol "Test:Empty" + (symbol "Test:Empty_1_1" + (pin passive line (at 0 0 0) (length 1.27) + (name "~" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27)))))))""") + pts = _parse_lib_symbol_graphics(sexp) + assert pts == [] + + +# =================================================================== +# Unit tests — _transform_local_point +# =================================================================== + +class TestTransformLocalPoint: + """Test local→absolute coordinate transform.""" + + def test_no_transform(self): + x, y = _transform_local_point(1.0, 2.0, 100.0, 200.0, 0, False, False) + assert x == pytest.approx(101.0) + assert y == pytest.approx(202.0) + + def test_mirror_x(self): + x, y = _transform_local_point(1.0, 2.0, 0.0, 0.0, 0, True, False) + assert x == pytest.approx(1.0) + assert y == pytest.approx(-2.0) + + def test_mirror_y(self): + x, y = _transform_local_point(1.0, 2.0, 0.0, 0.0, 0, False, True) + assert x == pytest.approx(-1.0) + assert y == pytest.approx(2.0) + + def test_rotation_90(self): + x, y = _transform_local_point(1.0, 0.0, 0.0, 0.0, 90, False, False) + assert x == pytest.approx(0.0, abs=1e-9) + assert y == pytest.approx(1.0, abs=1e-9) + + +# =================================================================== +# Unit tests — _compute_symbol_bbox_direct with graphics +# =================================================================== + +class TestComputeSymbolBboxWithGraphics: + """Test that bounding box computation uses graphics points when available.""" + + def test_resistor_bbox_from_graphics(self): + """Device:R rectangle is (-1.016, -2.54) to (1.016, 2.54) in local coords. + Pins at (0, ±3.81). Placed at (100, 100) with no rotation. + Bbox should span from pin-to-pin in Y and use rectangle width in X.""" + sym = {"x": 100.0, "y": 100.0, "rotation": 0, "mirror_x": False, "mirror_y": False} + pin_defs = { + "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, + "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + } + graphics_points = [(-1.016, -2.54), (1.016, 2.54)] + + bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) + assert bbox is not None + min_x, min_y, max_x, max_y = bbox + # X should come from rectangle: 100 ± 1.016 + assert min_x == pytest.approx(100 - 1.016) + assert max_x == pytest.approx(100 + 1.016) + # Y should come from pins (extending beyond rectangle): 100 ± 3.81 + assert min_y == pytest.approx(100 - 3.81) + assert max_y == pytest.approx(100 + 3.81) + + def test_fallback_without_graphics(self): + """Without graphics_points, should use the old degenerate expansion.""" + sym = {"x": 100.0, "y": 100.0, "rotation": 0, "mirror_x": False, "mirror_y": False} + pin_defs = { + "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, + "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + } + + bbox = _compute_symbol_bbox_direct(sym, pin_defs) + assert bbox is not None + min_x, min_y, max_x, max_y = bbox + # X should be expanded with min_body=1.5: 100 ± 1.5 + assert min_x == pytest.approx(100 - 1.5) + assert max_x == pytest.approx(100 + 1.5) + + def test_rotated_symbol_graphics(self): + """Graphics points should be rotated along with the symbol.""" + sym = {"x": 100.0, "y": 100.0, "rotation": 90, "mirror_x": False, "mirror_y": False} + pin_defs = { + "1": {"x": 0, "y": 3.81, "angle": 270, "length": 1.27, "name": "~", "type": "passive"}, + "2": {"x": 0, "y": -3.81, "angle": 90, "length": 1.27, "name": "~", "type": "passive"}, + } + # Rectangle corners in local coords + graphics_points = [(-1.016, -2.54), (1.016, 2.54)] + + bbox = _compute_symbol_bbox_direct(sym, pin_defs, graphics_points=graphics_points) + assert bbox is not None + min_x, min_y, max_x, max_y = bbox + # After 90° rotation, X and Y swap roles + # Pins now extend along X: 100 ± 3.81 + # Rectangle now extends along Y: 100 ± 1.016 + assert min_x == pytest.approx(100 - 3.81, abs=0.01) + assert max_x == pytest.approx(100 + 3.81, abs=0.01) + + +@pytest.mark.integration +class TestIntegrationGraphicsBbox: + """Integration tests verifying graphics-based bbox from real template data.""" + + def test_resistor_bbox_uses_rectangle(self): + """The template's Device:R has a rectangle body. + Verify that the bbox for a placed resistor uses the actual + rectangle width rather than the degenerate 1.5mm expansion.""" + extra = _make_resistor_sexp("R1", 100, 100) + tmp = _make_temp_schematic(extra) + sexp_data = _load_sexp(tmp) + symbols = _parse_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) + + r1 = [s for s in symbols if s["reference"] == "R1"][0] + lib_data = lib_defs.get(r1["lib_id"], {}) + pin_defs = lib_data.get("pins", {}) + graphics_points = lib_data.get("graphics_points", []) + + assert len(graphics_points) >= 2, "Should have extracted rectangle points" + + bbox = _compute_symbol_bbox_direct(r1, pin_defs, graphics_points=graphics_points) + assert bbox is not None + min_x, min_y, max_x, max_y = bbox + # Rectangle is ±1.016 in X, NOT ±1.5 from degenerate expansion + assert max_x - min_x == pytest.approx(2 * 1.016, abs=0.01) + + def test_led_bbox_uses_polyline(self): + """The template's Device:LED uses polylines for its body. + Verify that the bbox uses polyline extents.""" + extra = _make_led_sexp("D1", 100, 100) + tmp = _make_temp_schematic(extra) + sexp_data = _load_sexp(tmp) + symbols = _parse_symbols(sexp_data) + lib_defs = _extract_lib_symbols(sexp_data) + + d1 = [s for s in symbols if s["reference"] == "D1"][0] + lib_data = lib_defs.get(d1["lib_id"], {}) + graphics_points = lib_data.get("graphics_points", []) + + assert len(graphics_points) >= 4, "Should have extracted polyline points" + # LED body polylines span from -1.27 to 1.27 in both X and Y + xs = [p[0] for p in graphics_points] + ys = [p[1] for p in graphics_points] + assert min(xs) == pytest.approx(-1.27) + assert max(xs) == pytest.approx(1.27) + assert min(ys) == pytest.approx(-1.27) + assert max(ys) == pytest.approx(1.27)