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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
25f8737132
commit
e24b46d6f0
+29
-4
@@ -3197,7 +3197,14 @@ class ComBackend(AutoCADBackend):
|
|||||||
method = "activex_area"
|
method = "activex_area"
|
||||||
closed = True
|
closed = True
|
||||||
self_intersecting = None
|
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
|
# Rebuild from the polyline's own vertices and run the SAME
|
||||||
# shared maths the headless engine uses, so a missing ActiveX
|
# shared maths the headless engine uses, so a missing ActiveX
|
||||||
# property can never make this backend the less accurate one.
|
# property can never make this backend the less accurate one.
|
||||||
@@ -3214,15 +3221,33 @@ class ComBackend(AutoCADBackend):
|
|||||||
)
|
)
|
||||||
closed = bool(getattr(ent, "Closed", True))
|
closed = bool(getattr(ent, "Closed", True))
|
||||||
fallback_area, fallback_perimeter = polygon_area_perimeter(vertices, closed)
|
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
|
perimeter = fallback_perimeter if perimeter is None else perimeter
|
||||||
self_intersecting = is_self_intersecting([(v[0], v[1]) for v in vertices])
|
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 {
|
return {
|
||||||
"handle": str(handle),
|
"handle": str(handle),
|
||||||
"type": obj_name,
|
"type": obj_name,
|
||||||
"area": round(float(area), 6),
|
"area": round(float(area), 6),
|
||||||
"perimeter": round(float(perimeter), 6),
|
"perimeter": None if perimeter is None else round(float(perimeter), 6),
|
||||||
"closed": closed,
|
"closed": closed,
|
||||||
"assumed_closed": not closed,
|
"assumed_closed": not closed,
|
||||||
"method": method,
|
"method": method,
|
||||||
@@ -3230,7 +3255,7 @@ class ComBackend(AutoCADBackend):
|
|||||||
"flatten_tolerance": None,
|
"flatten_tolerance": None,
|
||||||
"backend": "com",
|
"backend": "com",
|
||||||
"self_intersecting": self_intersecting,
|
"self_intersecting": self_intersecting,
|
||||||
"perimeter_exact": True,
|
"perimeter_exact": perimeter is not None,
|
||||||
"loop_count": 1,
|
"loop_count": 1,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -117,8 +117,15 @@ class AnalysisContract(ABC):
|
|||||||
islands inside them — and the headless engine adds ``hatch_style``
|
islands inside them — and the headless engine adds ``hatch_style``
|
||||||
(``normal``/``outer``/``ignore``) so the number can be read against the
|
(``normal``/``outer``/``ignore``) so the number can be read against the
|
||||||
drawing's own island setting. The live backend does not carry that key:
|
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
|
it reads ActiveX ``.Area``, and whether that number already has the
|
||||||
there is no house rule left to disclose.
|
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
|
``exact`` and
|
||||||
``flatten_tolerance`` are the accuracy disclosure: when one engine can
|
``flatten_tolerance`` are the accuracy disclosure: when one engine can
|
||||||
|
|||||||
+28
-13
@@ -931,20 +931,31 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
loop = asyncio.get_running_loop()
|
loop = asyncio.get_running_loop()
|
||||||
deadline = loop.time() + timeout
|
deadline = loop.time() + timeout
|
||||||
lock = self._lock
|
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:
|
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:
|
except TimeoutError as exc:
|
||||||
raise self._busy_error(timeout) from exc
|
if not acquired:
|
||||||
|
# Timed out queueing behind someone else's live call. That lock
|
||||||
call = asyncio.ensure_future(asyncio.to_thread(func, *args, **kwargs))
|
# is held by a call that will release it normally; do not
|
||||||
try:
|
# abandon it.
|
||||||
result = await asyncio.wait_for(call, max(deadline - loop.time(), 0.0))
|
raise self._busy_error(timeout) from exc
|
||||||
except TimeoutError as exc:
|
if call is None or not call.cancelled():
|
||||||
if not call.cancelled():
|
# The call itself raised TimeoutError (an OSError subclass,
|
||||||
# The call itself raised TimeoutError (an OSError subclass, and
|
# and the same type the deadline raises) rather than overrunning
|
||||||
# the same type wait_for raises) rather than overrunning the
|
# the deadline. That is an ordinary failure: release the lock
|
||||||
# deadline. That is an ordinary failure: release the lock and
|
# and let it through unchanged instead of blaming the deadline.
|
||||||
# let it through unchanged instead of blaming the deadline.
|
|
||||||
lock.release()
|
lock.release()
|
||||||
raise
|
raise
|
||||||
if self._lock is lock:
|
if self._lock is lock:
|
||||||
@@ -957,7 +968,11 @@ class EzdxfBackend(AutoCADBackend):
|
|||||||
)
|
)
|
||||||
raise self._timeout_error(func, timeout) from exc
|
raise self._timeout_error(func, timeout) from exc
|
||||||
except BaseException:
|
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
|
raise
|
||||||
lock.release()
|
lock.release()
|
||||||
return result
|
return result
|
||||||
|
|||||||
@@ -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
|
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
|
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.
|
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)
|
count = len(vertices)
|
||||||
if count < 4:
|
if count < 4:
|
||||||
return False
|
return False
|
||||||
|
|||||||
@@ -301,3 +301,133 @@ async def test_com_backend_connects():
|
|||||||
assert backend.is_connected
|
assert backend.is_connected
|
||||||
await backend.disconnect()
|
await backend.disconnect()
|
||||||
assert not backend.is_connected
|
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
|
||||||
|
|||||||
@@ -198,3 +198,39 @@ def test_two_straight_vertices_still_enclose_nothing():
|
|||||||
|
|
||||||
def test_a_single_vertex_is_still_degenerate():
|
def test_a_single_vertex_is_still_degenerate():
|
||||||
assert polygon_area_perimeter([(0.0, 0.0, 1.0)], closed=True) == (0.0, 0.0)
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user