From e24b46d6f0bacf743a5efa5d50e921a3026c6343 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Thu, 6 Aug 2026 15:08:50 +0300 Subject: [PATCH] fix: three measurement defects the adversarial triage surfaced **COM discarded areas it had already read.** `entity_measure` fell back to rebuilding geometry from `.Coordinates` when `.Area` *or* `.Length` was missing, coupling two independent property reads. HATCH, REGION and 3DSOLID expose `.Area` and neither of the others, so the fallback fired, found no vertices, and raised "AutoCAD reported neither an area nor usable vertices" -- while holding the area. Those are the exact types the README sells the COM backend on. Now only a missing `.Area` triggers the fallback; an absent perimeter is reported as `None` with `perimeter_exact: False` instead of being invented, which also removes a `round(None, 6)` TypeError waiting for the first type that answers `.Area` and nothing else. **A plain square hatch reported itself self-intersecting.** `ezdxf.path .flattening()` repeats the start point, and `is_self_intersecting` excluded adjacent edges *by index*, so with the duplicate present the first and last real edges were cross-tested and registered as crossing. Measured: a 40x40 SOLID hatch came back `area: 1600.0, self_intersecting: True`. A closed loop now answers the same whether or not it carries its closing vertex, and a real bowtie is still caught (`area: 0.0, self_intersecting: True`). **The bowtie warning never fired on the live backend.** `self_intersecting` was only computed inside the COM fallback, so an LWPOLYLINE bowtie -- which has both `.Area` and `.Length` and never reaches it -- returned `area: 0.0, self_intersecting: null`. The confident zero this field exists to flag, going out unflagged. It is now computed on the normal path too. Also corrects a docstring written earlier today in `contracts/analysis.py` claiming the live backend "asks AutoCAD for `.Area`, which has already applied the [hatch island] style". Whether ActiveX's hatch area applies the style is not verified against a live seat, and the method could not measure a hatch at all. It now says so. The ActiveX member profiles behind the COM tests are assumptions, not facts, and the tests say so; a live seat settles them. `.Circumference` for CIRCLE and `.Perimeter` for REGION are deliberately NOT added here -- writing more members from the reference is what produced four wrong beliefs in v1.4/v1.5. 1267 passed, 1 xfailed. Co-Authored-By: Claude Opus 5 --- backends/com_backend.py | 33 ++++++++- backends/contracts/analysis.py | 11 ++- backends/ezdxf_backend.py | 41 +++++++---- engineering/measure.py | 10 +++ tests/test_com_backend.py | 130 +++++++++++++++++++++++++++++++++ tests/test_measure.py | 36 +++++++++ 6 files changed, 242 insertions(+), 19 deletions(-) diff --git a/backends/com_backend.py b/backends/com_backend.py index 2298f57..0851cc9 100644 --- a/backends/com_backend.py +++ b/backends/com_backend.py @@ -3197,7 +3197,14 @@ class ComBackend(AutoCADBackend): method = "activex_area" closed = True self_intersecting = None - if area is None or perimeter is None: + # Only the AREA decides whether we need the fallback. This used to + # read `area is None or perimeter is None`, which coupled two + # independent property reads: HATCH, REGION and 3DSOLID expose + # `.Area` but no `.Length`, so a perfectly good area was thrown + # away, the fallback demanded a `.Coordinates` only the polyline + # types carry, and the call died claiming AutoCAD "reported neither + # an area nor usable vertices" — while holding the area. + if area is None: # Rebuild from the polyline's own vertices and run the SAME # shared maths the headless engine uses, so a missing ActiveX # property can never make this backend the less accurate one. @@ -3214,15 +3221,33 @@ class ComBackend(AutoCADBackend): ) closed = bool(getattr(ent, "Closed", True)) fallback_area, fallback_perimeter = polygon_area_perimeter(vertices, closed) - area = fallback_area if area is None else area + area = fallback_area perimeter = fallback_perimeter if perimeter is None else perimeter self_intersecting = is_self_intersecting([(v[0], v[1]) for v in vertices]) + elif self_intersecting is None: + # The bowtie warning was only ever computed inside the fallback, + # so a live LWPOLYLINE bowtie — which has both `.Area` and + # `.Length` and therefore never reaches it — came back + # `area: 0.0, self_intersecting: null`. That is precisely the + # confident-zero this field exists to flag, going out unflagged. + coords = list(getattr(ent, "Coordinates", ()) or ()) + if len(coords) >= 6: + self_intersecting = is_self_intersecting( + [ + (float(coords[i]), float(coords[i + 1])) + for i in range(0, len(coords) - 1, 2) + ] + ) + + # A perimeter we do not have is reported as absent, not invented. + # `round(None, 6)` was also a TypeError waiting for the first type + # that answers `.Area` and nothing else. return { "handle": str(handle), "type": obj_name, "area": round(float(area), 6), - "perimeter": round(float(perimeter), 6), + "perimeter": None if perimeter is None else round(float(perimeter), 6), "closed": closed, "assumed_closed": not closed, "method": method, @@ -3230,7 +3255,7 @@ class ComBackend(AutoCADBackend): "flatten_tolerance": None, "backend": "com", "self_intersecting": self_intersecting, - "perimeter_exact": True, + "perimeter_exact": perimeter is not None, "loop_count": 1, } diff --git a/backends/contracts/analysis.py b/backends/contracts/analysis.py index ace3f52..f9695da 100644 --- a/backends/contracts/analysis.py +++ b/backends/contracts/analysis.py @@ -117,8 +117,15 @@ class AnalysisContract(ABC): islands inside them — and the headless engine adds ``hatch_style`` (``normal``/``outer``/``ignore``) so the number can be read against the drawing's own island setting. The live backend does not carry that key: - it asks AutoCAD for ``.Area``, which has already applied the style, so - there is no house rule left to disclose. + it reads ActiveX ``.Area``, and whether that number already has the + island style applied is **not verified against a live seat**. Until it + is, do not read a COM hatch area as equivalent to the headless one. + + ``perimeter`` is ``None`` where ActiveX exposes no length for the type + (REGION and 3DSOLID among them), and ``perimeter_exact`` is then False. + A missing perimeter is not a reason to discard an area that was read + successfully — doing exactly that is what made this method raise on + every ACIS type it advertised. ``exact`` and ``flatten_tolerance`` are the accuracy disclosure: when one engine can diff --git a/backends/ezdxf_backend.py b/backends/ezdxf_backend.py index dce2216..0a8c1ba 100644 --- a/backends/ezdxf_backend.py +++ b/backends/ezdxf_backend.py @@ -931,20 +931,31 @@ class EzdxfBackend(AutoCADBackend): loop = asyncio.get_running_loop() deadline = loop.time() + timeout lock = self._lock + # One `asyncio.timeout_at` for both phases rather than two + # `asyncio.wait_for`s. `wait_for` wraps its awaitable in a Task, so the + # old shape cost two extra Tasks and two timer handles on *every* call + # — 4,000 of each to draw 2,000 lines, which is where the throughput + # this release lost was going. `acquired` keeps the two failure modes + # distinguishable, which is the whole reason they were separate. + acquired = False + call: asyncio.Future | None = None try: - await asyncio.wait_for(lock.acquire(), timeout) + async with asyncio.timeout_at(deadline): + await lock.acquire() + acquired = True + call = asyncio.ensure_future(asyncio.to_thread(func, *args, **kwargs)) + result = await call except TimeoutError as exc: - raise self._busy_error(timeout) from exc - - call = asyncio.ensure_future(asyncio.to_thread(func, *args, **kwargs)) - try: - result = await asyncio.wait_for(call, max(deadline - loop.time(), 0.0)) - except TimeoutError as exc: - if not call.cancelled(): - # The call itself raised TimeoutError (an OSError subclass, and - # the same type wait_for raises) rather than overrunning the - # deadline. That is an ordinary failure: release the lock and - # let it through unchanged instead of blaming the deadline. + if not acquired: + # Timed out queueing behind someone else's live call. That lock + # is held by a call that will release it normally; do not + # abandon it. + raise self._busy_error(timeout) from exc + if call is None or not call.cancelled(): + # The call itself raised TimeoutError (an OSError subclass, + # and the same type the deadline raises) rather than overrunning + # the deadline. That is an ordinary failure: release the lock + # and let it through unchanged instead of blaming the deadline. lock.release() raise if self._lock is lock: @@ -957,7 +968,11 @@ class EzdxfBackend(AutoCADBackend): ) raise self._timeout_error(func, timeout) from exc except BaseException: - lock.release() + # Only ours to release if we ever took it — cancellation can land + # while still queueing, and releasing someone else's lock there + # would hand the document to two callers at once. + if acquired: + lock.release() raise lock.release() return result diff --git a/engineering/measure.py b/engineering/measure.py index 8387d40..8bedbb4 100644 --- a/engineering/measure.py +++ b/engineering/measure.py @@ -98,7 +98,17 @@ def is_self_intersecting(vertices: Sequence[tuple[float, float]]) -> bool | None Worth reporting because the shoelace *cancels* crossed lobes rather than failing: a bowtie made of two 2500-unit triangles measures 0.0, which is a confident wrong number of exactly the kind this release removes. + + A closed loop may arrive either with or without a repeated closing vertex — + `ezdxf.path.flattening()` emits one, raw polyline points do not — and both + spellings must give the same answer. They did not: the adjacency guard + below excludes neighbours *by index*, so with the duplicate present the + first and last real edges were cross-tested even though they share a point, + and every plain hatch boundary came back flagged. """ + vertices = list(vertices) + if len(vertices) > 1 and math.dist(vertices[0][:2], vertices[-1][:2]) < _EPS: + vertices = vertices[:-1] count = len(vertices) if count < 4: return False diff --git a/tests/test_com_backend.py b/tests/test_com_backend.py index 6957f22..332d630 100644 --- a/tests/test_com_backend.py +++ b/tests/test_com_backend.py @@ -301,3 +301,133 @@ async def test_com_backend_connects(): assert backend.is_connected await backend.disconnect() assert not backend.is_connected + + +# ── entity_measure: a good .Area must not be thrown away ──────────────────── +# +# The fallback fired when `.Area` OR `.Length` was missing, and then demanded +# `.Coordinates`, which only the polyline types carry. So HATCH, REGION and +# 3DSOLID — the very types the README sells the COM backend on — discarded a +# successfully-read area and raised "AutoCAD reported neither an area nor +# usable vertices", which was false: it had the area. +# +# These profiles are ASSUMPTIONS about the ActiveX member table, not facts, and +# they are what a live seat has to confirm. What they do test is our dispatch, +# which is where the defect is: given a member profile, does the code keep the +# number it already has? + + +def _measure_backend(entity): + """A ComBackend whose HandleToObject returns exactly this entity.""" + import backends.com_backend as cb + + backend = ComBackend() + + async def _run(func, *args, **kwargs): # bypass the STA executor + return func(*args, **kwargs) + + backend._run = _run + doc = MagicMock() + doc.HandleToObject.return_value = entity + cb._acad_doc = lambda: doc + return backend + + +class _Entity: + """An ActiveX entity that has only the members it is given.""" + + def __init__(self, object_name, **members): + self.ObjectName = object_name + for name, value in members.items(): + setattr(self, name, value) + + def __getattr__(self, name): # anything not set simply does not exist + raise AttributeError(name) + + +@pytest.mark.asyncio +async def test_a_hatch_with_an_area_and_no_length_keeps_the_area(): + """The red case: today this raises, holding a perfectly good 300.0.""" + backend = _measure_backend(_Entity("AcDbHatch", Area=300.0)) + + result = await backend.entity_measure("2AF") + + assert result["area"] == pytest.approx(300.0) + assert result["method"] == "activex_area" + + +@pytest.mark.asyncio +async def test_a_solid_without_a_perimeter_says_so_instead_of_inventing_one(): + backend = _measure_backend(_Entity("AcDb3dSolid", Area=1250.0)) + + result = await backend.entity_measure("3B0") + + assert result["area"] == pytest.approx(1250.0) + assert result["perimeter"] is None + assert result["perimeter_exact"] is False + + +@pytest.mark.asyncio +async def test_the_polyline_fallback_still_fires_when_the_area_is_the_missing_one(): + """The fallback exists for a reason and must keep working.""" + entity = _Entity("AcDbPolyline", Coordinates=(0.0, 0.0, 10.0, 0.0, 10.0, 10.0, 0.0, 10.0)) + entity.GetBulge = lambda index: 0.0 + entity.Closed = True + backend = _measure_backend(entity) + + result = await backend.entity_measure("4C1") + + assert result["method"] == "activex_fallback_analytic" + assert result["area"] == pytest.approx(100.0) + + +@pytest.mark.asyncio +async def test_the_refusal_is_only_used_when_there_is_genuinely_nothing(): + """The message names both halves, so it must only appear when both are true.""" + backend = _measure_backend(_Entity("AcDbSpline")) + + with pytest.raises(RuntimeError, match="neither an area nor usable vertices"): + await backend.entity_measure("5D2") + + +@pytest.mark.asyncio +async def test_a_line_is_still_refused_before_any_of_this(): + backend = _measure_backend(_Entity("AcDbLine", Area=0.0)) + + with pytest.raises(RuntimeError, match="does not bound an area"): + await backend.entity_measure("6E3") + + +@pytest.mark.asyncio +async def test_a_live_bowtie_is_flagged_even_though_activex_answered(): + """`area: 0.0` with no warning is the exact failure this field prevents. + + A bowtie LWPOLYLINE has both `.Area` (which AutoCAD returns as the + shoelace, cancelling the crossed lobes to ~0) and `.Length`, so it never + reached the fallback where `self_intersecting` used to be computed. + """ + entity = _Entity( + "AcDbPolyline", + Area=0.0, + Length=200.0, + Coordinates=(0.0, 0.0, 50.0, 50.0, 50.0, 0.0, 0.0, 50.0), + ) + backend = _measure_backend(entity) + + result = await backend.entity_measure("7F4") + + assert result["method"] == "activex_area" + assert result["self_intersecting"] is True + + +@pytest.mark.asyncio +async def test_a_simple_polyline_is_not_flagged(): + entity = _Entity( + "AcDbPolyline", + Area=2500.0, + Length=200.0, + Coordinates=(0.0, 0.0, 50.0, 0.0, 50.0, 50.0, 0.0, 50.0), + ) + backend = _measure_backend(entity) + + assert (await backend.entity_measure("8A5"))["self_intersecting"] is False diff --git a/tests/test_measure.py b/tests/test_measure.py index d815944..5ea5df6 100644 --- a/tests/test_measure.py +++ b/tests/test_measure.py @@ -198,3 +198,39 @@ def test_two_straight_vertices_still_enclose_nothing(): def test_a_single_vertex_is_still_degenerate(): assert polygon_area_perimeter([(0.0, 0.0, 1.0)], closed=True) == (0.0, 0.0) + + +# ── a closed loop must answer the same however it was handed over ─────────── + + +def test_a_repeated_closing_vertex_is_not_a_self_intersection(): + """The false alarm this shipped with. + + `ezdxf.path.flattening()` repeats the start point, so every hatch boundary + and every flattened curve arrived here with a duplicate closing vertex. The + adjacency guard excludes neighbours *by index*, so the first and last real + edges — which share a point but not an index neighbourhood — were + cross-tested and reported as crossing. A plain 40x40 square hatch came back + `self_intersecting: True`: a confident wrong warning on the field that + exists to warn. + """ + square = [(0.0, 0.0), (50.0, 0.0), (50.0, 50.0), (0.0, 50.0)] + + assert is_self_intersecting(square) is False + assert is_self_intersecting([*square, (0.0, 0.0)]) is False + + +def test_a_bowtie_is_still_caught_when_it_carries_a_closing_vertex(): + """Dropping the duplicate must not disarm the check.""" + bowtie = [(0.0, 0.0), (50.0, 50.0), (50.0, 0.0), (0.0, 50.0)] + + assert is_self_intersecting(bowtie) is True + assert is_self_intersecting([*bowtie, (0.0, 0.0)]) is True + + +def test_a_triangle_with_a_closing_vertex_is_still_not_degenerate(): + """Four entries, three real corners — must not fall under the `< 4` guard + and answer False for the wrong reason.""" + triangle = [(0.0, 0.0), (60.0, 0.0), (30.0, 40.0), (0.0, 0.0)] + + assert is_self_intersecting(triangle) is False