From efcc8bbba14a2bdf6cc3bc6c5f6f2ef3c5ce7e69 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Tue, 22 Sep 2026 11:06:04 +0300 Subject: [PATCH] fix(task-7): crossings test the exact arc, so a line through a jump's apex counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit flatten_bulges chops a semicircle into an even number of chords (36 for r=50, 12 for r=5), so a flattened vertex lands exactly on the apex — the axis of symmetry, the one place a P&ID line most plausibly meets a crossing jump. segments_cross treats a vertex hit as a junction, so both neighbouring chords dropped it and the arc contributed 0: the (45,0)->(55,0) bulge -1 jump reported crossings 0 at x=50 and 1 at x=50.3 or x=49, the plan's own (0,0)->(100,0) semicircle 0 at x=50, and the mirrored frame (extrusion -Z) 0 at x=-50. count_crossings now takes bulges= (per-chain DXF bulge lists, zero-padded) and tests every bulged edge as its exact arc through the new segment_arc_crossings — circle/segment intersection restricted to the sweep — with the same junction rule as the straight case: a hit within _EPS of the segment's ends, of the arc's own two ends, or a tangent, is not a crossing. Nothing along the sweep is a vertex any more. The arc parametrisation is factored into _arc_of_bulge and shared with flatten_bulges, which keeps its promised signature and its ezdxf pin but no longer feeds the crossing count. draw_line passes the vertices and bulges through unflattened. Pinned in tests: the apex case on the semicircle (x=50, y 20->60), the r=5 jump at x=50 / 50.3 / 49 on +Z and -Z frames, the junction cases (T-touch on the arc, through the arc's endpoint, chord side, tangent along the apex, in-and-out at y=4.9 counts 2), and 4x400 seeded random segment/arc pairs against ezdxf's ConstructionArc.intersect_line (a 20 000-case run in the worktree found 0 mismatches). The eight new assertions fail on the flattened rule and pass on the exact one. Headless finding, headless evidence; no new ActiveX call was written, so the shared live AutoCAD was not touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- engineering/pid/drawlines.py | 5 +- engineering/pid/lines.py | 125 +++++++++++++++++++++++++++-------- tests/test_pid_line_draw.py | 35 +++++++++- tests/test_pid_lines.py | 66 ++++++++++++++++++ 4 files changed, 201 insertions(+), 30 deletions(-) diff --git a/engineering/pid/drawlines.py b/engineering/pid/drawlines.py index cbbecb8..b635606 100644 --- a/engineering/pid/drawlines.py +++ b/engineering/pid/drawlines.py @@ -13,7 +13,6 @@ from .lines import ( PID_LINE_LAYERS, aim_points, count_crossings, - flatten_bulges, format_line_number, label_placement, marker_positions, @@ -238,7 +237,9 @@ async def draw_line( existing = await existing_pid_lines(backend) crossings = count_crossings( - path, [flatten_bulges(ln["vertices"], ln["bulges"]) for ln in existing] + path, + [ln["vertices"] for ln in existing], + bulges=[ln["bulges"] for ln in existing], ) used: set[tuple[str, str]] = set() max_seq = 0 diff --git a/engineering/pid/lines.py b/engineering/pid/lines.py index 4f9afda..f3d5729 100644 --- a/engineering/pid/lines.py +++ b/engineering/pid/lines.py @@ -238,23 +238,50 @@ def marker_positions(vertices, min_len: float = 15.0) -> list[tuple[float, float return out -#: Max chord deviation (mm) when a bulged polyline edge is flattened for the -#: crossing test — a tenth of the thinnest ISO 128 lineweight. +#: Max chord deviation (mm) when a bulged polyline edge is flattened into +#: chords (``flatten_bulges``) — a tenth of the thinnest ISO 128 lineweight. +#: The crossing count does **not** flatten: it tests the exact arc. FLATTEN_SAGITTA = 0.05 +def _arc_of_bulge(a: Point, b: Point, bulge: float) -> tuple[float, float, float, float, float]: + """``(cx, cy, radius, start, theta)`` of the arc a→b with the DXF bulge + ``tan(sweep/4)``: ``theta`` is the signed sweep in radians (positive + counter-clockwise) and ``start`` the angle of ``a`` about the centre. + + Same arithmetic as ``engineering/measure.py`` (sweep ``4·atan(bulge)``, + radius ``chord / 2·sin(|sweep|/2)``) and ``graph._point_segment_distance`` + (centre on the chord's left normal, ``radius - sagitta`` carrying it across + the chord past a semicircle). The caller guarantees a non-zero bulge and a + non-degenerate chord. + """ + chord = _dist(a, b) + theta = 4.0 * math.atan(bulge) # signed sweep, radians + radius = chord / (2.0 * math.sin(abs(theta) / 2.0)) + sag = abs(bulge) * chord / 2.0 + nx, ny = -(b[1] - a[1]) / chord, (b[0] - a[0]) / chord # left of a→b + offset = (radius - sag) * (1.0 if bulge > 0 else -1.0) + cx = (a[0] + b[0]) / 2.0 + offset * nx + cy = (a[1] + b[1]) / 2.0 + offset * ny + start = math.atan2(a[1] - cy, a[0] - cx) + return cx, cy, radius, start, theta + + def flatten_bulges(points, bulges, sagitta: float = FLATTEN_SAGITTA) -> list[Point]: """The polyline as straight segments, every bulged edge replaced by chords that deviate from the arc by at most ``sagitta``. - Same arc arithmetic as ``engineering/measure.py`` (sweep ``4·atan(bulge)``, - radius ``chord / 2·sin(|sweep|/2)``) and ``graph._point_segment_distance`` - (centre on the chord's left normal, ``radius - sagitta`` carrying it across - the chord past a semicircle). Checked against ``ezdxf.math.bulge_to_arc`` - + ``ConstructionArc.flattening`` in ``tests/test_pid_lines.py``: identical - vertex count and positions to 1e-12 for bulges 0.2 … 2.0 of either sign. - A straight edge (bulge 0) is passed through; ``bulges`` shorter than the - edge count is padded with zeros. + Arc arithmetic from ``_arc_of_bulge``. Checked against + ``ezdxf.math.bulge_to_arc`` + ``ConstructionArc.flattening`` in + ``tests/test_pid_lines.py``: identical vertex count and positions to 1e-12 + for bulges 0.2 … 2.0 of either sign. A straight edge (bulge 0) is passed + through; ``bulges`` shorter than the edge count is padded with zeros. + + Not used by the crossing count: a flattened chain has artificial vertices, + and ``segments_cross`` treats a hit on any vertex as a junction — a + semicircle flattens to an even chord count, so a line through its apex + (the axis of symmetry, exactly where a P&ID line meets a crossing jump) + would be dropped by both chords. ``count_crossings`` tests the exact arc. """ pts = [(float(x), float(y)) for x, y in points] if len(pts) < 2: @@ -263,17 +290,9 @@ def flatten_bulges(points, bulges, sagitta: float = FLATTEN_SAGITTA) -> list[Poi for i, (a, b) in enumerate(zip(pts, pts[1:], strict=False)): out.append(a) bulge = float(bulges[i]) if i < len(bulges) and bulges[i] else 0.0 - chord = _dist(a, b) - if abs(bulge) < 1e-12 or chord < 1e-12: + if abs(bulge) < 1e-12 or _dist(a, b) < 1e-12: continue - theta = 4.0 * math.atan(bulge) # signed sweep, radians - radius = chord / (2.0 * math.sin(abs(theta) / 2.0)) - sag = abs(bulge) * chord / 2.0 - nx, ny = -(b[1] - a[1]) / chord, (b[0] - a[0]) / chord # left of a→b - offset = (radius - sag) * (1.0 if bulge > 0 else -1.0) - cx = (a[0] + b[0]) / 2.0 + offset * nx - cy = (a[1] + b[1]) / 2.0 + offset * ny - start = math.atan2(a[1] - cy, a[0] - cx) + cx, cy, radius, start, theta = _arc_of_bulge(a, b, bulge) if sagitta >= radius: segments = 1 else: @@ -296,15 +315,67 @@ def segments_cross(p1, p2, q1, q2) -> bool: return d1 * d2 < -_EPS and d3 * d4 < -_EPS -def count_crossings(vertices, others) -> int: +def segment_arc_crossings(p1, p2, a, b, bulge: float) -> int: + """Proper crossings (0, 1 or 2) of the straight segment p1→p2 with the + exact arc a→b of DXF ``bulge``; a zero bulge is the chord test. + + The same junction rule as ``segments_cross``: a hit within ``_EPS`` (mm) + of either end of the segment or of the arc is a T-touch or a shared + vertex, not a crossing, and a tangent (the line reaches no deeper than + ``_EPS`` into the circle) touches without crossing. Only the arc's own + two ends are junction points — nothing along the sweep is a vertex. + """ + if abs(bulge) < 1e-12 or _dist(a, b) < 1e-12: + return 1 if segments_cross(p1, p2, a, b) else 0 + cx, cy, radius, start, theta = _arc_of_bulge(a, b, bulge) + dx, dy = p2[0] - p1[0], p2[1] - p1[1] + length_sq = dx * dx + dy * dy + if length_sq < 1e-24: + return 0 + length = math.sqrt(length_sq) + fx, fy = p1[0] - cx, p1[1] - cy + # |p1 + t·d − c|² = r² → A t² + B t + C = 0 with A = |d|² + coeff_b = 2.0 * (fx * dx + fy * dy) + coeff_c = fx * fx + fy * fy - radius * radius + disc = coeff_b * coeff_b - 4.0 * length_sq * coeff_c + if disc <= 0.0: + return 0 + root = math.sqrt(disc) + if root / (2.0 * length) <= _EPS: + return 0 # tangent: the half-chord inside the circle is nothing + sweep = abs(theta) + sign = 1.0 if theta > 0 else -1.0 + count = 0 + for numerator in (-coeff_b - root, -coeff_b + root): + t = numerator / (2.0 * length_sq) + if t * length <= _EPS or (1.0 - t) * length <= _EPS: + continue # the segment ends on the arc: a T-touch + x, y = p1[0] + t * dx, p1[1] + t * dy + along = (sign * (math.atan2(y - cy, x - cx) - start)) % (2.0 * math.pi) + if along * radius <= _EPS or abs(sweep - along) * radius <= _EPS: + continue # the arc's own end: the polyline's vertex, a junction + if along > sweep: + continue # on the circle but outside the arc + count += 1 + return count + + +def count_crossings(vertices, others, bulges=None) -> int: + """Proper crossings of the straight chain ``vertices`` with every chain in + ``others``. ``bulges[i]`` is the per-vertex DXF bulge list of ``others[i]`` + (the arc leaving each vertex; short lists are zero-padded, ``None`` means + every chain is straight): a bulged edge is tested as its exact arc, never + as its chord or as flattened chords. + """ pts = [(float(x), float(y)) for x, y in vertices] count = 0 - for a, b in zip(pts, pts[1:], strict=False): - for other in others: - opts = [(float(x), float(y)) for x, y in other] - for c, d in zip(opts, opts[1:], strict=False): - if segments_cross(a, b, c, d): - count += 1 + for index, other in enumerate(others): + opts = [(float(x), float(y)) for x, y in other] + obulges = list(bulges[index]) if bulges is not None and index < len(bulges) else [] + for a, b in zip(pts, pts[1:], strict=False): + for j, (c, d) in enumerate(zip(opts, opts[1:], strict=False)): + bulge = float(obulges[j]) if j < len(obulges) and obulges[j] else 0.0 + count += segment_arc_crossings(a, b, c, d, bulge) return count diff --git a/tests/test_pid_line_draw.py b/tests/test_pid_line_draw.py index de34e1b..b8b7f84 100644 --- a/tests/test_pid_line_draw.py +++ b/tests/test_pid_line_draw.py @@ -467,7 +467,11 @@ def test_payload_helpers_accept_only_well_formed_values(): async def test_crossings_follow_a_bulged_line_not_its_chord(backend): """A foreign P&ID line with a semicircular jump (bulge -1 from (0,0) to (100,0) bows to +Y, apex (50,50)): a vertical utility line at x=30 from - y=20 to y=60 crosses the arc once at y≈45.8 and its chord never.""" + y=20 to y=60 crosses the arc once at y≈45.8 and its chord never — and so + does one at x=50, straight through the apex. The apex is the axis of + symmetry, where a flattened semicircle (an even chord count) puts a + vertex; a vertex hit is a junction to ``segments_cross``, so a chord + chain reported 0 there. The arc is tested exactly, not flattened.""" msp = backend._msp() msp.add_lwpolyline( [(0, 0, 0, 0, -1.0), (100, 0, 0, 0, 0)], @@ -480,10 +484,39 @@ async def test_crossings_follow_a_bulged_line_not_its_chord(backend): assert lines[0]["bulges"] == [-1.0, 0.0] result = await draw_line(backend, {"x": 30, "y": 20}, {"x": 30, "y": 60}, line_class="utility") assert result["crossings"] == 1 + apex = await draw_line(backend, {"x": 50, "y": 20}, {"x": 50, "y": 60}, line_class="utility") + assert apex["crossings"] == 1, "through the apex — the flattened vertex — is still one crossing" below = await draw_line(backend, {"x": 70, "y": -20}, {"x": 70, "y": -60}, line_class="utility") assert below["crossings"] == 0, "the chord side of the arc is empty" +@pytest.mark.parametrize("extrusion", [(0, 0, 1), (0, 0, -1)]) +async def test_crossings_count_a_line_through_a_jump_apex(backend, extrusion): + """The jump a P&ID draws where two lines cross: (45,0)→(55,0) bulge -1, a + 5 mm semicircle with its apex at (50,5). A line at x=50 meets the apex + exactly (36 chords of r=50, 12 of r=5: always even, always a vertex on + the axis) and is one crossing, the same as at x=50.3 or x=49 — on a + mirrored frame (extrusion -Z, WCS x negated) too.""" + msp = backend._msp() + msp.add_lwpolyline( + [(0, 0, 0, 0, 0), (45, 0, 0, 0, -1.0), (55, 0, 0, 0, 0), (100, 0, 0, 0, 0)], + format="xyseb", + dxfattribs={"layer": "PROCESS-PIPING-MAIN", "extrusion": extrusion}, + ) + backend._mark_dirty() + sign = extrusion[2] + for x in (50.0, 50.3, 49.0): + drawn = await draw_line( + backend, {"x": sign * x, "y": -20}, {"x": sign * x, "y": 20}, line_class="utility" + ) + assert drawn["crossings"] == 1, f"x={sign * x}" + await backend.entity_delete(drawn["handle"]) + beside = await draw_line( + backend, {"x": sign * 60, "y": 2}, {"x": sign * 60, "y": 20}, line_class="utility" + ) + assert beside["crossings"] == 0, "clear of the jump and above the straight run" + + async def test_com_existing_lines_carry_bulges(monkeypatch): payload = line_payload("process_major", "100-P-7", "100", "P", None, None, 7, None, None) poly = _com_pid_polyline("3E", (0.0, 0.0, 100.0, 0.0), payload) diff --git a/tests/test_pid_lines.py b/tests/test_pid_lines.py index 8cc4f75..2751709 100644 --- a/tests/test_pid_lines.py +++ b/tests/test_pid_lines.py @@ -206,6 +206,72 @@ def test_flatten_bulges_matches_ezdxf_flattening(bulge): assert mine[len(mine) // 2] == pytest.approx(reference[len(reference) // 2]) +def test_crossings_test_the_exact_arc_so_a_flattened_vertex_hides_nothing(): + """(45,0)→(55,0) bulge -1 is the r=5 jump; its apex (50,5) is a vertex of + the flattened chain (12 chords, even), so the chord chain drops a line at + x=50 from both neighbours. The exact arc counts it once — as it does at + x=50.3 — and keeps the junction rule everywhere it belongs.""" + from engineering.pid.lines import count_crossings, flatten_bulges, segment_arc_crossings + + jump, bulges = [(45.0, 0.0), (55.0, 0.0)], [-1.0, 0.0] + flat = flatten_bulges(jump, bulges) + assert (50.0, 5.0) in flat, "the apex is a flattened vertex — the case that was missed" + assert count_crossings([(50, -20), (50, 20)], [flat]) == 0, "chords: the hit is a vertex" + assert count_crossings([(50, -20), (50, 20)], [jump], bulges=[bulges]) == 1 + assert count_crossings([(50.3, -20), (50.3, 20)], [jump], bulges=[bulges]) == 1 + assert count_crossings([(50, 20), (50, 60)], [[(0, 0), (100, 0)]], bulges=[[-1.0, 0.0]]) == 1 + # Junctions stay junctions: the segment ending on the arc, a line through + # the arc's own endpoint (the polyline's vertex), the empty chord side, + # and a tangent along the apex that touches without crossing. + assert segment_arc_crossings((50, 5), (50, 20), *jump, -1.0) == 0 + assert segment_arc_crossings((45, -5), (45, 5), *jump, -1.0) == 0 + assert segment_arc_crossings((50, -20), (50, -1), *jump, -1.0) == 0 + assert segment_arc_crossings((40, 5), (60, 5), *jump, -1.0) == 0 + assert segment_arc_crossings((40, 4.9), (60, 4.9), *jump, -1.0) == 2, "in and out again" + assert segment_arc_crossings((50, -20), (50, 20), *jump, 1.0) == 1, "bulge +1 bows to -Y" + assert segment_arc_crossings((50, -20), (50, 20), *jump, 0.0) == 1, "bulge 0 is the chord" + # A short or missing bulge list is zero-padded: straight edges as before. + assert count_crossings([(5, -5), (5, 5)], [[(0, 0), (10, 0), (10, 10)]], bulges=[[]]) == 1 + assert count_crossings([(5, -5), (5, 5)], [[(0, 0), (10, 0)]], bulges=None) == 1 + + +@pytest.mark.parametrize("seed", range(4)) +def test_segment_arc_crossings_match_ezdxf_arc_line_intersection(seed): + """Pinned against ezdxf's own ``ConstructionArc.intersect_line`` (hits + strictly inside the segment) over random segments and bulges of either + sign, on both sides of a semicircle.""" + import random + + from ezdxf.math import ConstructionArc, ConstructionLine, Vec2, bulge_to_arc + + from engineering.pid.lines import segment_arc_crossings + + rng = random.Random(seed) + checked = 0 + for _ in range(400): + a = (rng.uniform(-50, 50), rng.uniform(-50, 50)) + b = (rng.uniform(-50, 50), rng.uniform(-50, 50)) + if math.dist(a, b) < 1.0: + continue + magnitude = rng.choice([0.2, 0.5, 1.0, 1.5, 2.5, rng.uniform(0.05, 3.0)]) + bulge = rng.choice([-1.0, 1.0]) * magnitude + p1 = (rng.uniform(-80, 80), rng.uniform(-80, 80)) + p2 = (rng.uniform(-80, 80), rng.uniform(-80, 80)) + centre, s_rad, e_rad, radius = bulge_to_arc(Vec2(a), Vec2(b), bulge) + arc = ConstructionArc( + center=centre, + radius=radius, + start_angle=math.degrees(s_rad), + end_angle=math.degrees(e_rad), + ) + d = Vec2(p2) - Vec2(p1) + hits = arc.intersect_line(ConstructionLine(Vec2(p1), Vec2(p2)), abs_tol=1e-9) + expected = sum(1 for h in hits if 1e-6 < (h - Vec2(p1)).dot(d) / d.dot(d) < 1 - 1e-6) + assert segment_arc_crossings(p1, p2, a, b, bulge) == expected, (p1, p2, a, b, bulge) + checked += 1 + assert checked > 350 + + def test_flatten_bulges_passes_straight_edges_through_and_pads_short_bulge_lists(): from engineering.pid.lines import flatten_bulges