fix(task-12): restore the caller's tab from the layout listing; source the projection symbol
enter_layout read the caller's space from `backend._current_space`, which only
EzdxfBackend has. On ComBackend the getattr fell through to "Model", so
`sheet_frame` / `titleblock_apply` with a `layout=` dropped a live caller onto
the Model tab instead of putting them back -- the opposite of what the
docstring and both tool docs promise, and a dual-engine parity break in new
shared infrastructure. Measured on the live seat (AutoCAD 2026, scratch
document from drawing_new(), closed with discard):
scratch document: Drawing24.dwg
layouts: ['Layout1', 'Layout2', 'Model']
B) current BEFORE: Layout2
B) the value the OLD code read (backend._current_space):
<<absent -> getattr default 'Model'>>
`layout_list()` already reported the right value two lines earlier on both
engines (ezdxf from $TILEMODE / the active layout, COM from
`doc.ActiveLayout.Name` -- read live as "Layout2" above), so `previous` now
comes from `listing["current"]`.
The projection-angle symbol was justified by an in-code derivation that cannot
decide the question, and the test restated the implementation. The derivation
was also applied inconsistently: the third-angle symbol mirrored the cone as
well as the end view, leaving the trapezoid's short side pointing AWAY from the
circles -- which is the first-angle arrangement. Transcribed instead from two
independent reproductions of the ISO 5456-2 figure, both downloaded and
measured (ISO's own figure is paywalled and was NOT read; the module says so):
* FreeCAD's ISO 5457 sheet template A3_Landscape_ISO5457_advanced.svg,
`first_angle_trapezoid` = "m 389,222 -10,2.5 v 5 l 10,2.5 z" -> 5-unit side
at x=379, 10-unit side at x=389; circles r2.5/r5 at cx=396.
* Wikimedia Commons "Convention placement vues dessin technique.svg", both
symbols side by side, labelled FR and US: FR 35.856 @ x=13.343 / 66.135 @
x=81.869, circles x=148.005; US 35.856 @ x=380.275 / 66.135 @ x=448.801,
circles x=314.140.
Both draw the trapezoid the same way -- short side left -- and move only the
end view: first angle puts it right of the cone, third angle left. Equivalently
the short side points away from the circles in first angle and towards them in
third, which is how every source states it in words. `third` now follows.
Tests: the reference figures are transcribed into the test as data, and the
symbol is asserted to land its three landmarks in the order each figure does,
so the test cannot pass by agreeing with the code. The layout restore is pinned
through a proxy that hides `_current_space` from the real backend rather than a
fake that owns members ComBackend does not have. Both new tests fail on the
previous code (measured: "assert 16.0 < 8.0" for the mirrored cone; "- SheetB
+ Model" for the tab).
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
56e6ff7206
commit
e57ae8b3ac
@@ -250,7 +250,12 @@ async def enter_layout(backend: AutoCADBackend, layout: str | None, *, caller: s
|
||||
f"{caller}: layout {layout!r} does not exist "
|
||||
f"(have: {', '.join(sorted(names))}). Create it with layout_create first."
|
||||
)
|
||||
previous = getattr(backend, "_current_space", "Model")
|
||||
# The caller's tab comes from the listing that was just read -- both engines
|
||||
# report it there (ezdxf from $TILEMODE / the active layout, COM from
|
||||
# ``doc.ActiveLayout.Name``). A private attribute would not: only
|
||||
# EzdxfBackend has ``_current_space``, so reading it left a live caller
|
||||
# standing on Model after every border.
|
||||
previous = str(listing.get("current") or "Model")
|
||||
switched = await backend.layout_set_current(layout)
|
||||
if not switched.get("ok", True):
|
||||
raise RuntimeError(f"{caller}: {switched.get('error')}")
|
||||
|
||||
@@ -15,9 +15,10 @@ Data fields ISO 7200:2004 -- the mandatory fields are the legal owner
|
||||
shop draws (`part_no`, `material`, `scale`, `units`). ISO
|
||||
7200's full list of optional fields is NOT transcribed -- only
|
||||
the fields this block has a cell for.
|
||||
Projection angle ISO 128-30 / ISO 5456-2 -- the truncated-cone symbol; see
|
||||
`projection_symbol_prims` for the arrangement and where it
|
||||
comes from.
|
||||
Projection angle The truncated-cone symbol of ISO 5456-2 / ISO 128-30. ISO's
|
||||
own figure is paywalled and was NOT read here; the arrangement
|
||||
is transcribed from reproductions of it, measured, and named
|
||||
in `projection_symbol_prims`.
|
||||
|
||||
The row heights (20 / 15 / 15 / 10 mm) and the cell splits are this
|
||||
repository's layout, not ISO 7200 values: the standard fixes the *data*, not
|
||||
@@ -160,34 +161,63 @@ def titleblock_origin(size: str, *, orientation: str = "landscape") -> Pt:
|
||||
|
||||
|
||||
def projection_symbol_prims(at: Pt, angle: str = "first") -> tuple[Prim, ...]:
|
||||
"""The ISO 128-30 projection-angle symbol, centred on `at`.
|
||||
"""The ISO 5456-2 / ISO 128-30 projection-angle symbol, centred on `at`.
|
||||
|
||||
Arrangement, and where it comes from. The symbol is a truncated cone with a
|
||||
horizontal axis and its small end facing left. The first- and third-angle
|
||||
symbols are mirror images about a vertical axis, and which is which follows
|
||||
from the projection rule itself: first-angle projection puts the view seen
|
||||
from the left ON THE RIGHT, and the view of this cone seen from the left is
|
||||
the small circle inside the large one. So FIRST angle draws the cone on the
|
||||
left and the concentric circles on the right; THIRD angle is its mirror.
|
||||
ARRANGEMENT -- transcribed, not derived. The symbol is a truncated cone
|
||||
drawn in two views: the cone in elevation (a trapezoid) and the end view
|
||||
(two concentric circles). Which side the end view sits on is what makes the
|
||||
symbol mean *first* or *third* angle, and it is a convention that has to be
|
||||
read off the standard's figure: the projection rule alone cannot pick it,
|
||||
because the same picture is a legal drawing of the mirrored object seen
|
||||
from the other side.
|
||||
|
||||
ISO 5456-2's own figure is behind ISO's paywall and was not read. Two
|
||||
independent reproductions of it were downloaded and measured instead, and
|
||||
they agree on every landmark used here:
|
||||
|
||||
* FreeCAD's ISO 5457 sheet template, first-angle symbol
|
||||
(`src/Mod/TechDraw/Templates/ISO/A3_Landscape_ISO5457_advanced.svg`,
|
||||
ids `first_angle_trapezoid` / `first_angle_*_circle`): the trapezoid is
|
||||
`m 389,222 -10,2.5 v 5 l 10,2.5 z` -- a 5-unit side at x = 379 and a
|
||||
10-unit side at x = 389 -- and the circles (r 2.5 and r 5) are at
|
||||
cx = 396. Short side LEFT; end view to the RIGHT of the cone.
|
||||
* Wikimedia Commons `Convention placement vues dessin technique.svg`, which
|
||||
draws both symbols side by side, labelled FR and US. FR (first angle):
|
||||
trapezoid 35.856 units tall at x = 13.343 and 66.135 at x = 81.869,
|
||||
circles at x = 148.005. US (third angle): trapezoid 35.856 at
|
||||
x = 380.275 and 66.135 at x = 448.801, circles at x = 314.140.
|
||||
|
||||
So, in both symbols alike, the trapezoid's SHORT side is on the left; only
|
||||
the end view changes side:
|
||||
|
||||
first angle cone, then the circles to its RIGHT
|
||||
third angle the circles, then the cone to their RIGHT
|
||||
|
||||
which is the same thing as the rule the standard is usually quoted by --
|
||||
the short side points AWAY from the circles in first angle and TOWARDS
|
||||
them in third. `tests/test_sheet_titleblock.py` pins the ordering against
|
||||
the measured figures above, so this cannot be "fixed" back by eye.
|
||||
|
||||
The proportions (large circle diameter 8, small 4, cone 8 mm long) are this
|
||||
module's; ISO 5456-2's proportion table is not transcribed. The arrangement
|
||||
is the part that can be *wrong* on a drawing, and it is derived above.
|
||||
module's; ISO 5456-2's proportion table is not transcribed. They keep the
|
||||
1:2 small-to-large ratio both reference figures' circles have.
|
||||
"""
|
||||
if angle not in PROJECTION_ANGLES:
|
||||
raise ValueError(f"projection angle must be one of {PROJECTION_ANGLES}, got {angle!r}")
|
||||
cx, cy = float(at[0]), float(at[1])
|
||||
half = (PROJECTION_CONE_LENGTH + PROJECTION_GAP + 2 * PROJECTION_R) / 2.0
|
||||
# The end view sits right of the cone in first angle, left of it in third.
|
||||
sign = 1.0 if angle == "first" else -1.0
|
||||
cone_far = -half * sign
|
||||
cone_near = cone_far + sign * PROJECTION_CONE_LENGTH
|
||||
circles_x = (half - PROJECTION_R) * sign
|
||||
# The trapezoid itself is the same picture in both symbols: short side left.
|
||||
cone_left = -half if angle == "first" else half - PROJECTION_CONE_LENGTH
|
||||
cone_right = cone_left + PROJECTION_CONE_LENGTH
|
||||
cone = Poly(
|
||||
points=(
|
||||
(cx + cone_far, cy - PROJECTION_SMALL_R),
|
||||
(cx + cone_near, cy - PROJECTION_R),
|
||||
(cx + cone_near, cy + PROJECTION_R),
|
||||
(cx + cone_far, cy + PROJECTION_SMALL_R),
|
||||
(cx + cone_left, cy - PROJECTION_SMALL_R),
|
||||
(cx + cone_right, cy - PROJECTION_R),
|
||||
(cx + cone_right, cy + PROJECTION_R),
|
||||
(cx + cone_left, cy + PROJECTION_SMALL_R),
|
||||
),
|
||||
closed=True,
|
||||
)
|
||||
|
||||
@@ -40,3 +40,31 @@ async def backend():
|
||||
await b.drawing_new()
|
||||
yield b
|
||||
await b.disconnect()
|
||||
|
||||
|
||||
class WithoutPrivateState:
|
||||
"""The real backend with its ezdxf-only private attributes taken away.
|
||||
|
||||
``EzdxfBackend`` tracks the current tab in ``self._current_space``;
|
||||
``ComBackend`` does not -- the live tab lives in AutoCAD
|
||||
(``doc.ActiveLayout``), so there is nothing of that name on it. Shared code
|
||||
that reaches for the private attribute therefore silently takes the
|
||||
``getattr`` default on every live seat, and no ezdxf-backed test can see
|
||||
it. Wrapping the real backend (rather than writing a fake that owns members
|
||||
the real object never had) keeps every other call honest.
|
||||
"""
|
||||
|
||||
def __init__(self, inner, hide=("_current_space",)):
|
||||
object.__setattr__(self, "_inner", inner)
|
||||
object.__setattr__(self, "_hidden", frozenset(hide))
|
||||
|
||||
def __getattr__(self, name):
|
||||
if name in self._hidden:
|
||||
raise AttributeError(f"{type(self._inner).__name__} has no attribute {name!r}")
|
||||
return getattr(self._inner, name)
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def backend_without_private_state(backend):
|
||||
"""`backend`, minus the private attributes only the headless engine has."""
|
||||
return WithoutPrivateState(backend)
|
||||
|
||||
@@ -266,3 +266,36 @@ async def test_a_missing_layout_is_refused_before_anything_is_drawn(backend):
|
||||
assert "NoSuchSheet" in str(excinfo.value)
|
||||
assert "layout_create" in str(excinfo.value)
|
||||
assert len(await backend.entity_list()) == before
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_frame_puts_the_caller_back_on_the_tab_it_found_them_on(backend):
|
||||
await backend.layout_create("SheetA")
|
||||
await backend.layout_create("SheetB")
|
||||
await backend.layout_set_current("SheetB")
|
||||
await draw_sheet_frame(backend, "A3", zones=False, marks=False, layout="SheetA")
|
||||
assert (await backend.layout_list())["current"] == "SheetB"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_caller_s_tab_is_read_from_the_listing_not_a_private_attribute(
|
||||
backend, backend_without_private_state
|
||||
):
|
||||
"""`_current_space` exists on EzdxfBackend only. ComBackend keeps the live
|
||||
tab in AutoCAD, so reading the private attribute there fell through to the
|
||||
``getattr`` default and dropped a live caller onto Model after every
|
||||
border -- measured on AutoCAD 2026. `layout_list()["current"]` is the value
|
||||
both engines really report, and `enter_layout` already had it in hand.
|
||||
"""
|
||||
await backend.layout_create("SheetA")
|
||||
await backend.layout_create("SheetB")
|
||||
await backend.layout_set_current("SheetB")
|
||||
with pytest.raises(AttributeError):
|
||||
_ = backend_without_private_state._current_space
|
||||
|
||||
result = await draw_sheet_frame(
|
||||
backend_without_private_state, "A3", zones=False, marks=False, layout="SheetA"
|
||||
)
|
||||
|
||||
assert result["layout"] == "SheetA"
|
||||
assert (await backend.layout_list())["current"] == "SheetB"
|
||||
|
||||
@@ -107,29 +107,87 @@ def test_projection_none_draws_no_symbol_and_first_draws_one():
|
||||
assert len(first) == len(plain) + 4
|
||||
|
||||
|
||||
def test_first_angle_puts_the_circles_to_the_right_of_the_cone():
|
||||
#: The ISO 5456-2 / ISO 128-30 projection symbol, transcribed from two
|
||||
#: independent reproductions of the standard's figure (ISO's own is paywalled
|
||||
#: and was not read). Each entry is that source's OWN coordinates, measured off
|
||||
#: the file, and the landmark order left-to-right is what the symbol means.
|
||||
#:
|
||||
#: 1. FreeCAD's ISO 5457 sheet template, first-angle symbol --
|
||||
#: src/Mod/TechDraw/Templates/ISO/A3_Landscape_ISO5457_advanced.svg,
|
||||
#: ids `first_angle_trapezoid`, `first_angle_inner circle`,
|
||||
#: `first_angle_outer circle`.
|
||||
#: 2. Wikimedia Commons `Convention placement vues dessin technique.svg`, which
|
||||
#: draws both symbols side by side, labelled FR (first) and US (third).
|
||||
#:
|
||||
#: A test that read these numbers out of `projection_symbol_prims` instead
|
||||
#: would pass whichever way round the symbol was drawn, which is exactly how
|
||||
#: the mirrored third-angle cone shipped in the first place.
|
||||
REFERENCE_FIGURES = {
|
||||
"first": (
|
||||
# source, short-side x, long-side x, end-view (circles) x
|
||||
("FreeCAD A3_Landscape_ISO5457_advanced.svg", 379.0, 389.0, 396.0),
|
||||
("Commons Convention placement vues dessin technique.svg (FR)", 13.343, 81.869, 148.005),
|
||||
),
|
||||
"third": (
|
||||
("Commons Convention placement vues dessin technique.svg (US)", 380.275, 448.801, 314.140),
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
#: The three landmarks, in the order `_landmarks` and REFERENCE_FIGURES list them.
|
||||
LANDMARKS = ("short", "long", "circles")
|
||||
|
||||
|
||||
def _landmarks(angle: str) -> tuple[float, float, float]:
|
||||
"""(short-side x, long-side x, end-view x) of the symbol this module draws."""
|
||||
prims = projection_symbol_prims((0.0, 0.0), angle)
|
||||
cone = next(p for p in prims if isinstance(p, Poly))
|
||||
circles = [p for p in prims if isinstance(p, Circle)]
|
||||
left_x = min(x for x, _ in cone.points)
|
||||
right_x = max(x for x, _ in cone.points)
|
||||
left_h = max(y for x, y in cone.points if x == left_x) * 2
|
||||
right_h = max(y for x, y in cone.points if x == right_x) * 2
|
||||
short_x, long_x = (left_x, right_x) if left_h < right_h else (right_x, left_x)
|
||||
assert len({c.center[0] for c in circles}) == 1
|
||||
return short_x, long_x, circles[0].center[0]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("angle", ["first", "third"])
|
||||
def test_the_symbol_lands_the_landmarks_in_the_order_the_reference_figures_do(angle):
|
||||
"""The meaning of the symbol is the left-to-right order of three landmarks:
|
||||
the cone's short side, its long side and the end view. Assert that order
|
||||
against the measured figures, not against what the module computes."""
|
||||
mine = _landmarks(angle)
|
||||
for source, *reference in REFERENCE_FIGURES[angle]:
|
||||
expected = [name for _x, name in sorted(zip(reference, LANDMARKS, strict=True))]
|
||||
got = [name for _x, name in sorted(zip(mine, LANDMARKS, strict=True))]
|
||||
assert got == expected, f"{angle}-angle symbol disagrees with {source}"
|
||||
|
||||
|
||||
def test_the_short_side_points_away_from_the_circles_in_first_and_towards_in_third():
|
||||
"""The mirror-invariant half of the rule, and the one every source states
|
||||
in words: Wikipedia's Multiview orthographic projection, Symbol --
|
||||
"the first-angle symbol shows the trapezoid with its shortest side away
|
||||
from the circles", "the third-angle symbol shows the trapezoid with its
|
||||
shortest side towards the circles"."""
|
||||
short_x, long_x, circles_x = _landmarks("first")
|
||||
assert abs(short_x - circles_x) > abs(long_x - circles_x)
|
||||
short_x, long_x, circles_x = _landmarks("third")
|
||||
assert abs(short_x - circles_x) < abs(long_x - circles_x)
|
||||
|
||||
|
||||
def test_the_symbol_keeps_the_reference_figures_proportions_and_its_axis():
|
||||
"""Both reference figures draw the end view as two circles of ratio 1:2
|
||||
(FreeCAD r 2.5 / r 5) on a centre line; the absolute size is this
|
||||
module's."""
|
||||
first = projection_symbol_prims((0.0, 0.0), "first")
|
||||
third = projection_symbol_prims((0.0, 0.0), "third")
|
||||
cone_first = next(p for p in first if isinstance(p, Poly))
|
||||
circles_first = [p for p in first if isinstance(p, Circle)]
|
||||
assert max(x for x, _ in cone_first.points) < min(c.center[0] for c in circles_first)
|
||||
cone_third = next(p for p in third if isinstance(p, Poly))
|
||||
circles_third = [p for p in third if isinstance(p, Circle)]
|
||||
assert min(x for x, _ in cone_third.points) > max(c.center[0] for c in circles_third)
|
||||
assert {round(c.radius, 6) for c in circles_first} == {4.0, 2.0}
|
||||
circles = [p for p in first if isinstance(p, Circle)]
|
||||
assert {round(c.radius, 6) for c in circles} == {4.0, 2.0}
|
||||
assert any(isinstance(p, Line) and p.role == "center" for p in first)
|
||||
|
||||
|
||||
def test_the_cone_s_small_end_faces_the_circles_side_in_first_angle():
|
||||
"""The derivation only holds if the view drawn as circles really is the
|
||||
view from the cone's small end."""
|
||||
cone = next(p for p in projection_symbol_prims((0.0, 0.0), "first") if isinstance(p, Poly))
|
||||
left_x = min(x for x, _ in cone.points)
|
||||
right_x = max(x for x, _ in cone.points)
|
||||
left_height = max(y for x, y in cone.points if x == left_x) * 2
|
||||
right_height = max(y for x, y in cone.points if x == right_x) * 2
|
||||
assert left_height == pytest.approx(4.0, abs=EPS)
|
||||
assert right_height == pytest.approx(8.0, abs=EPS)
|
||||
def test_the_two_symbols_are_not_the_same_picture():
|
||||
assert _landmarks("first") != _landmarks("third")
|
||||
|
||||
|
||||
def test_an_unknown_projection_angle_is_refused():
|
||||
@@ -188,3 +246,19 @@ async def test_apply_titleblock_draws_every_size_with_its_symbol(backend):
|
||||
)
|
||||
assert result["ok"] is True and result["size"] == size
|
||||
assert len(result["projection_symbol"]) == 4
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_titleblock_puts_the_caller_back_on_the_tab_it_found_them_on(
|
||||
backend, backend_without_private_state
|
||||
):
|
||||
"""`apply_titleblock` routes through the same `enter_layout`, so the live
|
||||
engine dropped the caller onto Model here too."""
|
||||
await backend.layout_create("SheetA")
|
||||
await backend.layout_create("SheetB")
|
||||
await backend.layout_set_current("SheetB")
|
||||
result = await apply_titleblock(
|
||||
backend_without_private_state, size="A3", metadata=_meta(), layout="SheetA"
|
||||
)
|
||||
assert result["layout"] == "SheetA"
|
||||
assert (await backend.layout_list())["current"] == "SheetB"
|
||||
|
||||
Reference in New Issue
Block a user