fix(task-17): a layout plot never writes CenterPlot, and a refused COM write unwinds every landed one

The ActiveX Reference (CenterPlot Property, acadauto.chm, AutoCAD 2026): "This property cannot be set to True on a layout object whose PlotType property is set to acLayout." The shipped sequence wrote PlotType=5 and then CenterPlot=True for the *default* call, so page_setup_apply(layout, resolve_page_setup("ISO_A3")) could not succeed live — either COM raised with nine writes landed and nothing put back, or the result claimed applied.center=True over a read-back of False. - resolve_page_setup resolves `center` to None for plot_area="layout" (validated as a bool first; kept as the bool for "extents"), and `center_applies(plot_type)` carries the rule for both read-backs. - Both engines: page_setup_list reports `center: None` under acLayout instead of a stored flag nothing honours; apply never sets the centre bit / CenterPlot for a layout plot (ezdxf clears bit 4, the state AutoCAD stores for acLayout; COM clears a stale CenterPlot=True *before* PlotType moves to acLayout, and writes CenterPlot for an extents plot only *after* PlotType=acExtents). - COM: every write is journaled and unwound in reverse when AutoCAD refuses one; the ValueError names the refused property and says the layout was restored (or what could not be). The old device-only rollback is the same mechanism now. - tests: the fake enforces the documented CenterPlot rule (the shipped sequence fails under it, which is the reviewer's repro), and can refuse a named property; new cases pin the no-write, the ordering both ways, the reverse unwind including SetCustomScale, and the None read-backs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
2026-09-16 21:55:14 +03:00
parent 70f18efb4b
commit 20ca39d28a
5 changed files with 292 additions and 49 deletions
@@ -13,6 +13,7 @@ import math
 import sys
 import time
 import uuid
+from collections.abc import Callable
 from concurrent.futures import ThreadPoolExecutor
 from pathlib import Path
 from typing import Any
@@ -1961,7 +1962,14 @@ class ComBackend(AutoCADBackend):
    # typelib: AcPlotPaperUnits acMillimeters=1; AcPlotRotation ac0degrees=0;
    # AcPlotType acExtents=1 / acLayout=5; AcPlotScale acScaleToFit=0, ac1_1=16
    # (the full table is engineering.standards.papers.ACTIVEX_PLOT_SCALE).
-    # Fake-tested in tests/test_page_setup.py; executed live by
+    # CenterPlot (ActiveX Reference, acadauto.chm): "This property cannot be
+    # set to True on a layout object whose PlotType property is set to
+    # acLayout" — so a layout plot never writes it (resolve_page_setup hands
+    # over None), an extents plot writes it *after* PlotType, and a stale True
+    # is cleared *before* PlotType moves to acLayout. Every write is journaled
+    # and unwound in reverse when AutoCAD refuses one, so a failed apply leaves
+    # the layout as it was. Fake-tested in tests/test_page_setup.py (the fake
+    # enforces the CenterPlot rule); executed live by
    # scripts/smoke_settings_com.py (Task 24).

    _AC_MILLIMETERS = 1
@@ -1972,6 +1980,7 @@ class ComBackend(AutoCADBackend):
        from engineering.standards.papers import (
            PLOT_TYPE_NAMES,
            activex_scale_label,
+            center_applies,
            paper_from_size,
            scale_label,
        )
@@ -2024,7 +2033,7 @@ class ComBackend(AutoCADBackend):
            "plot_area": PLOT_TYPE_NAMES.get(plot_type, plot_type),
            "device": _get("ConfigName"),
            "margins_mm": margins,
-            "center": bool(_get("CenterPlot")),
+            "center": bool(_get("CenterPlot")) if center_applies(plot_type) else None,
        }

    def _com_paper_layout(self, doc, raw: str, operation: str):
@@ -2071,37 +2080,81 @@ class ComBackend(AutoCADBackend):
            doc = _acad_doc()
            target = self._com_paper_layout(doc, layout, "page_setup_apply")
            before = self._com_page_setup_row(target)
-            previous_device = before["device"]
-            target.RefreshPlotDeviceInfo()
-            target.ConfigName = resolved["device"]
-            target.RefreshPlotDeviceInfo()
-            media = resolved["canonical_media_name"]
-            names = [str(n) for n in (target.GetCanonicalMediaNames() or ())]
-            if media not in names:
-                # Roll the one write back before refusing, so a refusal leaves
-                # the layout as it was.
-                target.ConfigName = previous_device
-                nearest = [n for n in names if n.split("_(")[0] == media.split("_(")[0]]
-                raise ValueError(
-                    f"page_setup_apply: device {resolved['device']!r} has no media "
-                    f"{media!r} (same paper on this device: {', '.join(nearest) or 'none'}; "
-                    f"{len(names)} media in total)"
-                )
-            target.CanonicalMediaName = media
-            target.PaperUnits = self._AC_MILLIMETERS
-            target.PlotRotation = self._AC_NO_ROTATION
-            target.StyleSheet = resolved["plot_style"]
-            target.PlotWithPlotStyles = True
-            target.PlotType = int(resolved["plot_type"])
-            code = resolved["activex_standard_scale"]
-            if code is not None:
-                target.StandardScale = int(code)
-                target.UseStandardScale = True
-            else:
-                numerator, denominator = (float(v) for v in resolved["scale_ratio"])
+            # Journal of (undo callable, label) for every write that landed,
+            # unwound in reverse when a later one is refused.
+            journal: list[tuple[Callable[[], None], str]] = []
+
+            def _set(attr: str, value):
+                previous = getattr(target, attr)
+                setattr(target, attr, value)
+                journal.append((lambda: setattr(target, attr, previous), attr))
+
+            def _set_custom_scale(numerator: float, denominator: float):
+                previous = tuple(float(v) for v in target.GetCustomScale())
                target.SetCustomScale(numerator, denominator)
-                target.UseStandardScale = False
-            target.CenterPlot = bool(resolved["center"])
+                journal.append((lambda: target.SetCustomScale(*previous), "SetCustomScale"))
+
+            step = "ConfigName"
+            try:
+                target.RefreshPlotDeviceInfo()
+                _set("ConfigName", resolved["device"])
+                target.RefreshPlotDeviceInfo()
+                media = resolved["canonical_media_name"]
+                names = [str(n) for n in (target.GetCanonicalMediaNames() or ())]
+                if media not in names:
+                    nearest = [n for n in names if n.split("_(")[0] == media.split("_(")[0]]
+                    raise ValueError(
+                        f"page_setup_apply: device {resolved['device']!r} has no media "
+                        f"{media!r} (same paper on this device: "
+                        f"{', '.join(nearest) or 'none'}; {len(names)} media in total)"
+                    )
+                step = "CanonicalMediaName"
+                _set("CanonicalMediaName", media)
+                step = "PaperUnits"
+                _set("PaperUnits", self._AC_MILLIMETERS)
+                step = "PlotRotation"
+                _set("PlotRotation", self._AC_NO_ROTATION)
+                step = "StyleSheet"
+                _set("StyleSheet", resolved["plot_style"])
+                step = "PlotWithPlotStyles"
+                _set("PlotWithPlotStyles", True)
+                center = resolved["center"]
+                if center is None and self._com_center_plot_is_set(target):
+                    # Clearing is legal under any PlotType; the layout plot
+                    # then carries the flag AutoCAD stores for acLayout.
+                    step = "CenterPlot"
+                    _set("CenterPlot", False)
+                step = "PlotType"
+                _set("PlotType", int(resolved["plot_type"]))
+                code = resolved["activex_standard_scale"]
+                if code is not None:
+                    step = "StandardScale"
+                    _set("StandardScale", int(code))
+                    step = "UseStandardScale"
+                    _set("UseStandardScale", True)
+                else:
+                    numerator, denominator = (float(v) for v in resolved["scale_ratio"])
+                    step = "SetCustomScale"
+                    _set_custom_scale(numerator, denominator)
+                    step = "UseStandardScale"
+                    _set("UseStandardScale", False)
+                if center is not None:
+                    # Extents plot: PlotType is already acExtents, so True is legal.
+                    step = "CenterPlot"
+                    _set("CenterPlot", bool(center))
+            except Exception as exc:
+                failed = self._com_unwind_page_setup(journal)
+                if isinstance(exc, ValueError) and not failed:
+                    raise
+                restored = (
+                    f"layout {target.Name!r} restored to its previous page setup"
+                    if not failed
+                    else f"layout {target.Name!r} could NOT be fully restored "
+                    f"(still changed: {', '.join(failed)})"
+                )
+                raise ValueError(
+                    f"page_setup_apply: AutoCAD refused Layout.{step} ({exc}); {restored}"
+                ) from exc
            after = self._com_page_setup_row(target)
            changed = {
                key: [before[key], after[key]]
@@ -2120,6 +2173,27 @@ class ComBackend(AutoCADBackend):

        return await self._run(_sync)

+    @staticmethod
+    def _com_center_plot_is_set(layout) -> bool:
+        """Whether the layout carries CenterPlot=True (a failed read counts as no)."""
+        try:
+            return bool(layout.CenterPlot)
+        except Exception as exc:
+            log.debug("Layout.CenterPlot read failed: %s", exc)
+            return False
+
+    @staticmethod
+    def _com_unwind_page_setup(journal: list[tuple[Callable[[], None], str]]) -> list[str]:
+        """Undo every journaled page-setup write in reverse; return what would not go back."""
+        failed: list[str] = []
+        for undo, label in reversed(journal):
+            try:
+                undo()
+            except Exception as exc:
+                log.warning("page_setup_apply: could not restore Layout.%s: %s", label, exc)
+                failed.append(label)
+        return failed
+
    async def plot_style_list(self) -> list[dict]:
        # Completed in Task 18 (installed ctb scan); the catalogue rows already answer.
        from engineering.standards.papers import CTB_CATALOG
@@ -18,7 +18,10 @@ class PageSetupContract(ABC):
        orientation, plot_style, scale, plot_area, device,
        margins_mm: [top, bottom, left, right], center}``. ``paper`` is the
        catalogue name when the size matches one within 0.5 mm, otherwise the
-        media name as stored. Raises ``ValueError`` for an unknown layout.
+        media name as stored. ``center`` is a bool for an extents plot and
+        ``None`` for a layout plot, where centring is not applicable (AutoCAD
+        greys it out; ActiveX refuses ``CenterPlot = True`` under
+        ``acLayout``). Raises ``ValueError`` for an unknown layout.
        ``Model`` is never listed: model space is plotted through
        ``drawing_export_pdf(layout=None)`` and carries no sheet.
        """
@@ -33,7 +36,9 @@ class PageSetupContract(ABC):
        plot_style_known, viewports_kept}``. ``changed`` is computed from the
        read-back before and after, so re-applying the same setup reports
        ``{}``. Refuses ``Model``, an unknown layout, and a ``setup`` missing a
-        resolved key, before anything is written. Never deletes viewports.
+        resolved key, before anything is written. A write the engine refuses
+        part-way through is unwound (every landed value put back) before the
+        ``ValueError`` names the refused property. Never deletes viewports.
        """
        ...

@@ -2336,6 +2336,7 @@ class EzdxfBackend(AutoCADBackend):
    def _page_setup_row(layout) -> dict:
        from engineering.standards.papers import (
            PLOT_TYPE_NAMES,
+            center_applies,
            dxf_scale_label,
            paper_from_size,
            scale_label,
@@ -2355,6 +2356,7 @@ class EzdxfBackend(AutoCADBackend):
            scale = dxf_scale_label(int(dxf.standard_scale_type), numerator, denominator)
        else:
            scale = scale_label(numerator, denominator)
+        plot_type = int(dxf.plot_type)
        return {
            "layout": layout.name,
            "paper": paper_from_size(width, height) or media,
@@ -2363,7 +2365,7 @@ class EzdxfBackend(AutoCADBackend):
            "orientation": "landscape" if width >= height else "portrait",
            "plot_style": str(dxf.current_style_sheet),
            "scale": scale,
-            "plot_area": PLOT_TYPE_NAMES.get(int(dxf.plot_type), str(dxf.plot_type)),
+            "plot_area": PLOT_TYPE_NAMES.get(plot_type, str(dxf.plot_type)),
            "device": str(dxf.plot_configuration_file),
            "margins_mm": [
                round(float(dxf.top_margin), 3),
@@ -2371,7 +2373,9 @@ class EzdxfBackend(AutoCADBackend):
                round(float(dxf.left_margin), 3),
                round(float(dxf.right_margin), 3),
            ],
-            "center": bool(flags & layout.PLOT_CENTERED),
+            # A layout plot has no centring (AutoCAD greys it out; ActiveX
+            # refuses CenterPlot=True under acLayout): None, not a bit nobody honours.
+            "center": bool(flags & layout.PLOT_CENTERED) if center_applies(plot_type) else None,
        }

    @staticmethod
@@ -2474,6 +2478,9 @@ class EzdxfBackend(AutoCADBackend):
                dxf.standard_scale_type = 16
                dxf.scale_numerator, dxf.scale_denominator = numerator, denominator
                target.use_standard_scale(False)
+            # resolve_page_setup hands over None for a layout plot: the bit is
+            # cleared, never set — the state AutoCAD itself stores for acLayout,
+            # and the mirror of the live engine never writing CenterPlot there.
            target.plot_centered(bool(resolved["center"]))
            # Limits follow the paper; viewports are deliberately left alone
            # (Paperspace.page_setup() would delete them).