From 3ab93b241f60378ba86055a9fb2f44781c7a827f Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Sun, 15 Mar 2026 13:29:37 +0000 Subject: [PATCH] fix: find_overlapping_elements uses bounding-box intersection instead of center distance The overlap detection was comparing center-to-center Euclidean distance with a 0.5mm tolerance, missing components whose bodies physically overlap but have different centers (e.g. a resistor placed inside an opamp triangle). Now uses AABB intersection on pin-derived bounding boxes, matching the approach already used by check_wire_collisions. Extracted shared bbox logic into _compute_symbol_bbox_direct and _aabb_overlap helpers. Co-Authored-By: Claude Opus 4.6 --- python/commands/schematic_analysis.py | 133 +++++++++++++++++------- python/tests/test_schematic_analysis.py | 82 ++++++++++++--- 2 files changed, 158 insertions(+), 57 deletions(-) diff --git a/python/commands/schematic_analysis.py b/python/commands/schematic_analysis.py index 5c7fe7f..1bb357c 100644 --- a/python/commands/schematic_analysis.py +++ b/python/commands/schematic_analysis.py @@ -220,6 +220,67 @@ def _distance(p1: Tuple[float, float], p2: Tuple[float, float]) -> float: return math.sqrt((p1[0] - p2[0]) ** 2 + (p1[1] - p2[1]) ** 2) +def _aabb_overlap( + a: Tuple[float, float, float, float], + b: Tuple[float, float, float, float], +) -> bool: + """Check if two axis-aligned bounding boxes overlap. + + Each bbox is (min_x, min_y, max_x, max_y). + """ + return a[0] < b[2] and b[0] < a[2] and a[1] < b[3] and b[1] < a[3] + + +def _compute_symbol_bbox_direct( + sym: Dict[str, Any], + pin_defs: Dict[str, Dict], + margin: float = 0.0, +) -> Optional[Tuple[float, float, float, float]]: + """ + Compute bounding box of a symbol from its pin definitions and placement. + + Uses _compute_pin_positions_direct to get absolute pin positions, then + expands degenerate dimensions (pins in a line) to approximate body size. + + 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). + + Returns (min_x, min_y, max_x, max_y) in mm, or None if no pins. + """ + pin_positions = _compute_pin_positions_direct(sym, pin_defs) + 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) + + # 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 + + # Shrink bbox by margin + min_x += margin + min_y += margin + max_x -= margin + max_y -= margin + + # Skip degenerate bboxes + if max_x <= min_x or max_y <= min_y: + return None + + return (min_x, min_y, max_x, max_y) + + # --------------------------------------------------------------------------- # Tool 2: find_unconnected_pins # --------------------------------------------------------------------------- @@ -346,14 +407,35 @@ def find_overlapping_elements( overlapping_labels = [] overlapping_wires = [] - # --- Symbol-symbol overlap (O(n²)) --- + locator = PinLocator() + + # --- 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"]] - for i in range(len(non_template_symbols)): - for j in range(i + 1, len(non_template_symbols)): - s1 = non_template_symbols[i] - s2 = non_template_symbols[j] + + # Pre-compute bounding boxes for all non-template symbols + symbol_bboxes = [] + for sym in non_template_symbols: + pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) + bbox = None + if pin_defs: + bbox = _compute_symbol_bbox_direct(sym, pin_defs) + symbol_bboxes.append((sym, bbox)) + + for i in range(len(symbol_bboxes)): + s1, bbox1 = symbol_bboxes[i] + for j in range(i + 1, len(symbol_bboxes)): + s2, bbox2 = symbol_bboxes[j] dist = _distance((s1["x"], s1["y"]), (s2["x"], s2["y"])) - if dist < tolerance: + + overlap_detected = False + if bbox1 is not None and bbox2 is not None: + # Use bounding box intersection + overlap_detected = _aabb_overlap(bbox1, bbox2) + else: + # Fallback to center distance when pin data is unavailable + overlap_detected = dist < tolerance + + if overlap_detected: entry = { "element1": {"reference": s1["reference"], "libId": s1["lib_id"], "position": {"x": s1["x"], "y": s1["y"]}}, @@ -595,51 +677,22 @@ def check_wire_collisions(schematic_path: Path) -> List[Dict[str, Any]]: if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref: continue - # Get pin definitions by lib_id (works regardless of reference designator, - # so unannotated components with duplicate "Q?" references are handled correctly). pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"]) if not pin_defs: continue - # Compute absolute pin positions directly from this symbol's own position/rotation, - # bypassing the reference-name lookup in PinLocator (which always finds the first - # symbol with a given reference, breaking for unannotated duplicates like "Q?"). + bbox = _compute_symbol_bbox_direct(sym, pin_defs, margin=margin) + if bbox is None: + continue + pin_positions = _compute_pin_positions_direct(sym, pin_defs) - if not pin_positions: - continue - - 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) - - # 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 - - # Shrink bbox by margin - min_x += margin - min_y += margin - max_x -= margin - max_y -= margin - - # Skip degenerate bboxes (single-pin or very small after shrink) - if max_x <= min_x or max_y <= min_y: - continue - pin_set = set() for pos in pin_positions.values(): pin_set.add((pos[0], pos[1])) symbol_data.append({ "sym": sym, - "bbox": (min_x, min_y, max_x, max_y), + "bbox": bbox, "pin_set": pin_set, }) diff --git a/python/tests/test_schematic_analysis.py b/python/tests/test_schematic_analysis.py index d9cdc30..3b68d1c 100644 --- a/python/tests/test_schematic_analysis.py +++ b/python/tests/test_schematic_analysis.py @@ -28,6 +28,8 @@ from commands.schematic_analysis import ( _line_segment_intersects_aabb, _point_in_rect, _distance, + _aabb_overlap, + _compute_symbol_bbox_direct, compute_symbol_bbox, find_unconnected_pins, find_overlapping_elements, @@ -229,6 +231,27 @@ class TestSexpParsers: # Unit tests — analysis functions with mocked PinLocator # =================================================================== +class TestAABBOverlap: + """Test AABB overlap helper.""" + + def test_overlapping_boxes(self): + assert _aabb_overlap((0, 0, 10, 10), (5, 5, 15, 15)) is True + + def test_non_overlapping_boxes(self): + assert _aabb_overlap((0, 0, 10, 10), (20, 20, 30, 30)) is False + + def test_touching_boxes_no_overlap(self): + # Touching edges are not overlapping (strict inequality) + assert _aabb_overlap((0, 0, 10, 10), (10, 0, 20, 10)) is False + + def test_contained_box(self): + assert _aabb_overlap((0, 0, 20, 20), (5, 5, 15, 15)) is True + + def test_overlap_one_axis_only(self): + # Overlap in X but not Y + assert _aabb_overlap((0, 0, 10, 10), (5, 15, 15, 25)) is False + + class TestFindOverlappingElements: """Test overlapping detection logic.""" @@ -238,29 +261,15 @@ class TestFindOverlappingElements: assert result["totalOverlaps"] == 0 def test_overlapping_symbols_detected(self): - # Two symbols at nearly the same position - extra = """ - (symbol (lib_id "Device:R") (at 100 100 0) - (property "Reference" "R1" (at 0 0 0)) - (property "Value" "10k" (at 0 0 0))) - (symbol (lib_id "Device:R") (at 100.1 100 0) - (property "Reference" "R2" (at 0 0 0)) - (property "Value" "10k" (at 0 0 0))) - """ + # Two resistors at nearly the same position — bboxes fully overlap + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 100.1, 100) tmp = _make_temp_schematic(extra) result = find_overlapping_elements(tmp, tolerance=0.5) assert result["totalOverlaps"] >= 1 assert len(result["overlappingSymbols"]) >= 1 def test_well_separated_symbols_not_flagged(self): - extra = """ - (symbol (lib_id "Device:R") (at 100 100 0) - (property "Reference" "R1" (at 0 0 0)) - (property "Value" "10k" (at 0 0 0))) - (symbol (lib_id "Device:R") (at 200 200 0) - (property "Reference" "R2" (at 0 0 0)) - (property "Value" "10k" (at 0 0 0))) - """ + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 200, 200) tmp = _make_temp_schematic(extra) result = find_overlapping_elements(tmp, tolerance=0.5) assert result["totalOverlaps"] == 0 @@ -278,6 +287,45 @@ class TestFindOverlappingElements: result = find_overlapping_elements(tmp, tolerance=0.5) assert len(result["overlappingWires"]) >= 1 + def test_overlapping_bodies_different_centers(self): + """Two resistors whose bodies overlap even though centers are ~5mm apart. + + Device:R pins are at y ±3.81 relative to center, so the body spans + ~7.62mm vertically. Two resistors at the same X but 5mm apart in Y + have overlapping bodies — this is the bug the center-distance approach missed. + """ + # R1 at y=100, R2 at y=105 — pin spans [96.19, 103.81] and [101.19, 108.81] + # These overlap in Y from 101.19 to 103.81 + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 100, 105) + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] >= 1, ( + "Should detect overlap when component bodies intersect, " + "even if centers are far apart" + ) + assert len(result["overlappingSymbols"]) >= 1 + + def test_adjacent_resistors_no_overlap(self): + """Two vertical resistors side by side should not overlap. + + R pins at y ±3.81, but different X positions far enough apart. + """ + extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 110, 100) + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] == 0 + + def test_resistor_and_led_overlapping_bodies(self): + """A resistor and an LED placed close enough that bodies overlap. + + LED pins at x ±3.81, R pins at y ±3.81. Place LED at same position + as R — bodies clearly overlap. + """ + extra = _make_resistor_sexp("R1", 100, 100) + _make_led_sexp("D1", 100, 100) + tmp = _make_temp_schematic(extra) + result = find_overlapping_elements(tmp, tolerance=0.5) + assert result["totalOverlaps"] >= 1 + class TestGetElementsInRegion: """Test region query logic."""