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 <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
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
|
# Tool 2: find_unconnected_pins
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -346,14 +407,35 @@ def find_overlapping_elements(
|
|||||||
overlapping_labels = []
|
overlapping_labels = []
|
||||||
overlapping_wires = []
|
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"]]
|
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)):
|
# Pre-compute bounding boxes for all non-template symbols
|
||||||
s1 = non_template_symbols[i]
|
symbol_bboxes = []
|
||||||
s2 = non_template_symbols[j]
|
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"]))
|
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 = {
|
entry = {
|
||||||
"element1": {"reference": s1["reference"], "libId": s1["lib_id"],
|
"element1": {"reference": s1["reference"], "libId": s1["lib_id"],
|
||||||
"position": {"x": s1["x"], "y": s1["y"]}},
|
"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:
|
if sym["is_power"] or ref.startswith("_TEMPLATE") or not ref:
|
||||||
continue
|
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"])
|
pin_defs = locator.get_symbol_pins(schematic_path, sym["lib_id"])
|
||||||
if not pin_defs:
|
if not pin_defs:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Compute absolute pin positions directly from this symbol's own position/rotation,
|
bbox = _compute_symbol_bbox_direct(sym, pin_defs, margin=margin)
|
||||||
# bypassing the reference-name lookup in PinLocator (which always finds the first
|
if bbox is None:
|
||||||
# symbol with a given reference, breaking for unannotated duplicates like "Q?").
|
continue
|
||||||
|
|
||||||
pin_positions = _compute_pin_positions_direct(sym, pin_defs)
|
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()
|
pin_set = set()
|
||||||
for pos in pin_positions.values():
|
for pos in pin_positions.values():
|
||||||
pin_set.add((pos[0], pos[1]))
|
pin_set.add((pos[0], pos[1]))
|
||||||
|
|
||||||
symbol_data.append({
|
symbol_data.append({
|
||||||
"sym": sym,
|
"sym": sym,
|
||||||
"bbox": (min_x, min_y, max_x, max_y),
|
"bbox": bbox,
|
||||||
"pin_set": pin_set,
|
"pin_set": pin_set,
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -28,6 +28,8 @@ from commands.schematic_analysis import (
|
|||||||
_line_segment_intersects_aabb,
|
_line_segment_intersects_aabb,
|
||||||
_point_in_rect,
|
_point_in_rect,
|
||||||
_distance,
|
_distance,
|
||||||
|
_aabb_overlap,
|
||||||
|
_compute_symbol_bbox_direct,
|
||||||
compute_symbol_bbox,
|
compute_symbol_bbox,
|
||||||
find_unconnected_pins,
|
find_unconnected_pins,
|
||||||
find_overlapping_elements,
|
find_overlapping_elements,
|
||||||
@@ -229,6 +231,27 @@ class TestSexpParsers:
|
|||||||
# Unit tests — analysis functions with mocked PinLocator
|
# 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:
|
class TestFindOverlappingElements:
|
||||||
"""Test overlapping detection logic."""
|
"""Test overlapping detection logic."""
|
||||||
|
|
||||||
@@ -238,29 +261,15 @@ class TestFindOverlappingElements:
|
|||||||
assert result["totalOverlaps"] == 0
|
assert result["totalOverlaps"] == 0
|
||||||
|
|
||||||
def test_overlapping_symbols_detected(self):
|
def test_overlapping_symbols_detected(self):
|
||||||
# Two symbols at nearly the same position
|
# Two resistors at nearly the same position — bboxes fully overlap
|
||||||
extra = """
|
extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 100.1, 100)
|
||||||
(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)))
|
|
||||||
"""
|
|
||||||
tmp = _make_temp_schematic(extra)
|
tmp = _make_temp_schematic(extra)
|
||||||
result = find_overlapping_elements(tmp, tolerance=0.5)
|
result = find_overlapping_elements(tmp, tolerance=0.5)
|
||||||
assert result["totalOverlaps"] >= 1
|
assert result["totalOverlaps"] >= 1
|
||||||
assert len(result["overlappingSymbols"]) >= 1
|
assert len(result["overlappingSymbols"]) >= 1
|
||||||
|
|
||||||
def test_well_separated_symbols_not_flagged(self):
|
def test_well_separated_symbols_not_flagged(self):
|
||||||
extra = """
|
extra = _make_resistor_sexp("R1", 100, 100) + _make_resistor_sexp("R2", 200, 200)
|
||||||
(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)))
|
|
||||||
"""
|
|
||||||
tmp = _make_temp_schematic(extra)
|
tmp = _make_temp_schematic(extra)
|
||||||
result = find_overlapping_elements(tmp, tolerance=0.5)
|
result = find_overlapping_elements(tmp, tolerance=0.5)
|
||||||
assert result["totalOverlaps"] == 0
|
assert result["totalOverlaps"] == 0
|
||||||
@@ -278,6 +287,45 @@ class TestFindOverlappingElements:
|
|||||||
result = find_overlapping_elements(tmp, tolerance=0.5)
|
result = find_overlapping_elements(tmp, tolerance=0.5)
|
||||||
assert len(result["overlappingWires"]) >= 1
|
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:
|
class TestGetElementsInRegion:
|
||||||
"""Test region query logic."""
|
"""Test region query logic."""
|
||||||
|
|||||||
Reference in New Issue
Block a user