fix(task-14): match ACAD_SCALELIST entries by their group-300 name, not the dictionary key
`_ensure_scale_entry` decided "already present" with `scales.get(name)`, a lookup by DICTIONARY key. AutoCAD saves ACAD_SCALELIST keyed A0, A1, ... with the scale name only in the SCALE object's AcDbScale group 300, so on any AutoCAD-authored drawing (whose default metric list already carries 1:1 ... 1:100, 2:1) the check never matched and every `annotation_scale` write appended a second SCALE object with the same name. Presence is now decided by iterating the list and reading each entry's group-300 name (`_scale_entry_name`, which reads the DXFTagStorage subclass ezdxf loads SCALE into). A genuinely new entry is still keyed by its name (the plan's test contract); if that key is somehow taken it falls back to the next free `A<n>` slot like AutoCAD. Regression test seeds an A0/A1/A2-keyed list the way AutoCAD writes it and asserts: an existing 1:50 is reused (no new key, exactly one object named 1:50), a missing 1:25 is appended exactly once and is idempotent, and the file round-trips with a clean audit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
Umut CAN
co-authored by
Claude Opus 5
Claude Opus 5
parent
df39b21f2e
commit
e8e7e3c524
@@ -5937,9 +5937,34 @@ class EzdxfBackend(AutoCADBackend):
|
||||
return default if entry is None else str(entry.dxf.value)
|
||||
|
||||
@staticmethod
|
||||
def _ensure_scale_entry(doc, name: str, paper: float, drawing: float) -> bool:
|
||||
def _scale_entry_name(entry) -> str | None:
|
||||
"""The name a SCALE object carries in its AcDbScale group 300.
|
||||
|
||||
The dictionary *key* is not the name: AutoCAD saves ACAD_SCALELIST
|
||||
keyed `A0`, `A1`, … and only group 300 says `1:50`. ezdxf loads SCALE
|
||||
as `DXFTagStorage`, so the tag is read from the stored subclass.
|
||||
"""
|
||||
xtags = getattr(entry, "xtags", None)
|
||||
if xtags is None:
|
||||
return None
|
||||
for subclass in xtags.subclasses:
|
||||
if subclass and subclass[0].code == 100 and subclass[0].value == "AcDbScale":
|
||||
for tag in subclass:
|
||||
if tag.code == 300:
|
||||
return str(tag.value)
|
||||
return None
|
||||
|
||||
@classmethod
|
||||
def _ensure_scale_entry(cls, doc, name: str, paper: float, drawing: float) -> bool:
|
||||
"""Add `name` to ACAD_SCALELIST unless present; True when added.
|
||||
|
||||
Presence is decided by the SCALE objects' group-300 names, never by
|
||||
the dictionary key — an AutoCAD-authored drawing keys the list `A0`,
|
||||
`A1`, … (its default metric list already carries 1:1 … 1:100, 2:1),
|
||||
and matching on the key appended a duplicate `1:50` to every one of
|
||||
them. A new entry is keyed by its name unless that key is taken, in
|
||||
which case it takes the next free `A<n>` slot like AutoCAD does.
|
||||
|
||||
ezdxf has no SCALE entity class, so the object is authored as raw tags
|
||||
and loaded through the factory (a `DXFTagStorage` that exports
|
||||
verbatim). Measured: it round-trips, ezdxf registers the CLASS, and
|
||||
@@ -5952,8 +5977,16 @@ class EzdxfBackend(AutoCADBackend):
|
||||
scales = doc.rootdict.get("ACAD_SCALELIST")
|
||||
if scales is None:
|
||||
scales = doc.rootdict.add_new_dict("ACAD_SCALELIST")
|
||||
if scales.get(name) is not None:
|
||||
return False
|
||||
existing_keys = set(scales.keys())
|
||||
for _key, entry in scales.items():
|
||||
if cls._scale_entry_name(entry) == name:
|
||||
return False
|
||||
key = name
|
||||
if key in existing_keys:
|
||||
index = 0
|
||||
while f"A{index}" in existing_keys:
|
||||
index += 1
|
||||
key = f"A{index}"
|
||||
handle = doc.entitydb.next_handle()
|
||||
unit = 1 if paper == drawing else 0
|
||||
text = (
|
||||
@@ -5963,7 +5996,7 @@ class EzdxfBackend(AutoCADBackend):
|
||||
entry = factory.load(ExtendedTags.from_text(text), doc)
|
||||
doc.entitydb.add(entry)
|
||||
doc.objects.add_object(entry)
|
||||
scales.add(name, entry)
|
||||
scales.add(key, entry)
|
||||
return True
|
||||
|
||||
def _set_annotation_scale(self, doc, value: Any) -> str:
|
||||
|
||||
@@ -173,6 +173,71 @@ async def test_annotation_scale_writes_the_variable_dictionary_and_scale_list(ba
|
||||
assert backend._doc.audit().has_errors is False
|
||||
|
||||
|
||||
def _seed_autocad_style_scale_list(doc, names):
|
||||
"""ACAD_SCALELIST the way AutoCAD saves it: keyed A0, A1, ... with the scale
|
||||
name only in the SCALE object's group 300 (the default metric list already
|
||||
carries 1:1, 1:2, 1:5, 1:10, 1:20, 1:50, 1:100, 2:1 ...)."""
|
||||
from ezdxf.entities import factory
|
||||
from ezdxf.lldxf.extendedtags import ExtendedTags
|
||||
|
||||
scales = doc.rootdict.get("ACAD_SCALELIST")
|
||||
if scales is None:
|
||||
scales = doc.rootdict.add_new_dict("ACAD_SCALELIST")
|
||||
for index, name in enumerate(names):
|
||||
paper, drawing = (float(part) for part in name.split(":"))
|
||||
handle = doc.entitydb.next_handle()
|
||||
text = (
|
||||
f" 0\nSCALE\n 5\n{handle}\n330\n{scales.dxf.handle}\n100\nAcDbScale\n"
|
||||
f" 70\n0\n300\n{name}\n140\n{paper}\n141\n{drawing}\n290\n{int(paper == drawing)}\n"
|
||||
)
|
||||
entry = factory.load(ExtendedTags.from_text(text), doc)
|
||||
doc.entitydb.add(entry)
|
||||
doc.objects.add_object(entry)
|
||||
scales.add(f"A{index}", entry)
|
||||
return scales
|
||||
|
||||
|
||||
def _scale_names(scales):
|
||||
return [
|
||||
tag.value
|
||||
for _key, entry in scales.items()
|
||||
for subclass in entry.xtags.subclasses
|
||||
for tag in subclass
|
||||
if tag.code == 300
|
||||
]
|
||||
|
||||
|
||||
async def test_annotation_scale_matches_an_autocad_keyed_scale_list_by_name(backend, tmp_path):
|
||||
"""Regression: the presence check used to look the scale up by dictionary
|
||||
*key*, so on any AutoCAD-authored drawing (keys A0, A1, ...) every write
|
||||
appended a second SCALE object with the same name."""
|
||||
scales = _seed_autocad_style_scale_list(backend._doc, ["1:1", "1:2", "1:50"])
|
||||
assert list(scales.keys()) == ["A0", "A1", "A2"]
|
||||
|
||||
res = await backend.drawing_settings({"annotation_scale": "1:50"})
|
||||
assert res["ok"] is True, res.get("errors")
|
||||
assert res["changed"]["annotation_scale"][1] == {"name": "1:50", "value": 0.02}
|
||||
assert list(scales.keys()) == ["A0", "A1", "A2"], "the existing entry is reused, not duplicated"
|
||||
assert _scale_names(scales).count("1:50") == 1
|
||||
assert backend._doc.rootdict.get("AcDbVariableDictionary")["CANNOSCALE"].dxf.value == "1:50"
|
||||
|
||||
# a scale the list does not carry is still appended, exactly once, under a
|
||||
# key that does not collide with AutoCAD's A<n> keys
|
||||
res = await backend.drawing_settings({"annotation_scale": "1:25"})
|
||||
assert res["ok"] is True, res.get("errors")
|
||||
assert _scale_names(scales).count("1:25") == 1
|
||||
assert len(list(scales.keys())) == 4
|
||||
await backend.drawing_settings({"annotation_scale": "1:25"})
|
||||
assert len(list(scales.keys())) == 4
|
||||
|
||||
path = str(tmp_path / "autocad_keyed.dxf")
|
||||
await backend.drawing_save_as(path)
|
||||
await backend.drawing_open(path)
|
||||
reopened = backend._doc.rootdict.get("ACAD_SCALELIST")
|
||||
assert sorted(_scale_names(reopened)) == ["1:1", "1:2", "1:25", "1:50"]
|
||||
assert backend._doc.audit().has_errors is False
|
||||
|
||||
|
||||
async def test_unit_formats_by_name_and_by_code(backend):
|
||||
res = await backend.drawing_settings({"linear_units": "architectural", "angular_units": "dms"})
|
||||
assert res["ok"] is True, res.get("errors")
|
||||
|
||||
Reference in New Issue
Block a user