diff --git a/docs/cadcodec-reader-gaps.md b/docs/cadcodec-reader-gaps.md new file mode 100644 index 000000000..e8f5cc277 --- /dev/null +++ b/docs/cadcodec-reader-gaps.md @@ -0,0 +1,196 @@ +# cadcodec DXF reader/writer gaps found by the OCS Python host audit + +Draft issue reports for `HakanSeven12/cadcodec` (the `acadrust` crate). Nothing +here has been filed. Every finding was made against **cadcodec revision +`5b682ed`** (OCS pins `acadrust` to it) by saving a document with +`DxfWriter` or `DwgWriter`, reloading it, and comparing entity fields. + +Each report names an executable canary in OCS (`src/app/plugin_host.rs`). The +canaries assert today's wrong result, so they fail loudly once cadcodec is +fixed; flip the marked expectation when that happens. To reproduce, from the +OCS repository: + +```sh +STAGE="$PWD/work/bundled-plugins/opencad.python" +bash plugins/opencad-python/tools/stage-bundled.sh "$STAGE" --debug +cargo build --bin OpenCADStudio +OCS_TEST_PYTHON_PLUGIN="$STAGE/libopencad_python.dylib" \ +OCS_PLUGIN_RUNNER_EXE="$PWD/target/debug/OpenCADStudio" \ +cargo test --lib app::plugin_host::tests:: -- --test-threads=1 +``` + +| # | Area | Severity | Effect | +|---|---|---|---| +| 1 | `ATTDEF` reader | high | Most attribute-definition properties silently reset to defaults | +| 2 | Boolean group codes (290-299) | high | Booleans read through `as_i16()` are never applied | +| 3 | `UNDERLAY` rotation | high | Rotation is not converted back to radians; repeated saves compound it | +| 4 | `LEADER` | medium | Annotation link and three vectors are lost | +| 5 | `DRAWINGVIEW` / `SECTIONLINE` | medium | Typed entity not restored from the ENTITIES section | +| 6 | `TABLE` merged ranges | low | `merged_ranges` is empty after DXF reopen | + +--- + +## 1. ATTDEF reader handles eight group codes; the writer emits all of them + +**Where:** `src/io/dxf/reader/section_reader.rs`, `read_attdef` (around line +15647). The writer, `write_attdef` in `writer/section_writer.rs` (around line +4837), emits `7, 11, 41, 51, 70, 71, 72, 73, 74, 210` and `280` in addition to +the groups the reader consumes. + +**What happens:** the reader has arms for groups `1, 2, 3, 10/20/30, 40, 50, +280` and `101` only; everything else falls through to `_ =>`. After a DXF round +trip these fields reset to defaults: + +- text style name (7) +- alignment point (11/21/31) and both alignments (72, 74) +- width factor (41) and oblique angle (51) +- attribute flags (70): invisible, constant, verify, preset +- text generation flags (71) and field length (73) +- normal (210/220/230) and the position-lock state + +Tag, prompt, default value, insertion point, height and rotation survive. +Losing the attribute flags is the worst part: an invisible or constant +attribute becomes visible and editable after a DXF save. + +**Evidence:** `audit_python_attribute_definition_lifecycle_over_real_ipc` +compares every writable field through both formats. DWG round-trips the full +state; the DXF digest is pinned to the defaults (see the `BLOCKER` comment on +`expect_edited_dxf`). + +**Suggested fix:** add reader arms for the missing groups, using the same +conversions the writer applies (rotation and oblique angle are written in +degrees). `TEXT` and `ATTRIB` already read most of these groups and can share +the implementation. + +## 2. Boolean group codes are read with `as_i16()` and never applied + +**Where:** `src/io/dxf/reader/stream_reader.rs` defines +`DxfCodePair::as_i16()` as `Some` only for `CodePairValue::Int`. Groups 290-299 +are typed `Bool`, so `as_i16()` returns `None` for them and +`unwrap_or(0) != 0` yields `false`. The pattern appears in many arms of +`section_reader.rs`. + +**Confirmed impact (round-trip tested):** + +- `HELIX` group 290 (`handedness`): a clockwise helix reopens counter-clockwise. + The reader arm is `290 => { if let Some(v) = pair.as_i16() { ... } }` + (around line 12796). +- `LIGHT` groups 291, 292, 293 (`plot_glyph`, `use_attenuation_limits`, + `cast_shadows`; around lines 18154-18182), and 290 for the photometric web + file flag: `cast_shadows` reopens `false`. + +**Same pattern, not individually tested** (line numbers in `section_reader.rs`): +13833, 13955-13969 (point-cloud style flags); 18194 (`has_web_file`); +18431-18438 and 18493-18494 (sweep, loft and revolve option flags); and several +`if let Some(v) = pair.as_i16()` arms on 290/292 near lines 9815, 10096, 10478 +and 12861. Some other arms already use `as_bool()` correctly (for example lines +18376 and 18550), so the fix is mechanical. + +**Evidence:** `audit_python_helix_lifecycle_over_real_ipc` (canary +`expect_edited_dxf`) and `audit_python_light_lifecycle_over_real_ipc`. + +**Suggested fix:** read these groups with a helper that accepts either +representation, for example +`pair.as_bool().or_else(|| pair.as_i16().map(|v| v != 0))`, and audit every arm +whose code is in 290-299. Alternatively make `as_i16()` accept `Bool` values. + +## 3. `UNDERLAY` rotation is written in degrees and read back unconverted + +**Where:** `write_underlay` writes group 50 with `underlay.rotation.to_degrees()` +(around line 11038 of `writer/section_writer.rs`). `read_underlay` stores the +raw value: `50 => { if let Some(v) = pair.as_double() { underlay.rotation = v; } }` +(`reader/section_reader.rs`, around line 19470), with no `to_radians()`. + +**What happens:** a rotation of `0.5` rad is written as 28.6479 degrees and +read back as 28.6479 rad. Because the writer converts again on every save, the +error compounds: a second save-and-reopen yields 1641.4. This silently corrupts +existing drawings each time they pass through cadcodec. + +**Evidence:** `audit_python_underlay_lifecycle_over_real_ipc`; the canary +`expect_edited_dxf` / `expect_reedited_dxf` records 28.6 then 1641.4. + +**Suggested fix:** `underlay.rotation = v.to_radians()` in the reader. Check +that `DGN`/`DWF`/`PDF` underlays share the code path. + +## 4. `LEADER`: annotation link and vectors are lost in DXF + +**a. The writer never emits group 340.** `write_leader` writes 3, 71-76, 10, 210, +211, 212 and 213 but no associated-annotation handle, while the reader has a +`340 =>` arm that fills `annotation_handle` (around line 16619). A DXF save +therefore unlinks every leader from its text, tolerance or block annotation. + +**b. The coordinate mapping does not reassemble 211, 212 and 213.** +`GroupCodeValueType::coordinate_axis` in `src/io/dxf/group_code_value.rs` maps +X to `10..=18 | 110..=112 | 210 | 1010..=1013` (and the Y/Z equivalents), so +`add_coordinate` ignores the 211/221/231, 212/222/232 and 213/223/233 series. +`read_leader` uses a `PointReader` for `horizontal_direction`, `block_offset` +and `annotation_offset`, so all three reopen as their defaults even though the +writer emits them. (`normal`, group 210, works.) + +**c. `override_color` is not persisted** in DXF or DWG: a leader coloured by +`override_color` reopens `ByLayer`. Observed, cause not traced. + +**Evidence:** the canaries in `staged_python_leader_lifecycle_over_real_ipc` +(search for `BLOCKER`). DWG keeps the annotation link, offset and arrow flag. + +**Suggested fix:** write group 340 for a non-null `annotation_handle`; extend +`coordinate_axis`/`coordinate_group` to cover the 211-213 series (or read those +groups explicitly); decide where the override colour belongs (group 62/420 on +the leader). + +## 5. `DRAWINGVIEW` and `SECTIONLINE` are dispatched only inside blocks + +**Where:** `section_reader.rs` has two entity dispatch tables: one for block +contents (around lines 3241-3390) and one for the ENTITIES section +(`read_entities`, from line 3482). `"DRAWINGVIEW"` (line 3386) and +`"SECTIONLINE"` (line 3382) appear only in the first; the ENTITIES table has no +arm for them. `write_view_border_dxf` writes `DRAWINGVIEW` correctly. + +**What happens:** a `ViewBorder` saved to DXF in the drawing's entity list +reopens as a different, opaque entity kind instead of `ViewBorder`. DWG restores +the typed record. The same applies to `SectionSymbol`. + +**Evidence:** `audit_python_view_border_lifecycle_over_real_ipc` (canary +`expect_edited_dxf: "wrong kind"`). + +**Suggested fix:** add the two arms to the ENTITIES dispatch, calling +`read_view_border_dxf` and `read_section_symbol_dxf`. + +## 6. `TABLE` `merged_ranges` is empty after a DXF reopen + +**What happens:** the writer emits the merged-range list (`AcDbFormattedTableData`, +group 90 then 91-94 per range; near line 10216 of `writer/section_writer.rs`), +and `reader/section_reader/table_content.rs` (around lines 627-690) appears to +parse it, yet `Table::merged_ranges` is empty after a DXF reopen. The merge +itself survives through the origin cell's `merge_width` and `merge_height` +(groups 175 and 176), and DWG restores `merged_ranges` but not the per-cell +dimensions. + +**Impact:** low, because the merge is preserved in the cell fields; consumers +that read `merged_ranges` directly see nothing. Cause not traced; it may be that +the entity path does not call the table-content parser. + +**Evidence:** `staged_python_table_lifecycle_over_real_ipc` (the DXF branch +asserts an empty `merged_ranges` and marks it as a blocker). + +**Suggested fix:** rebuild `merged_ranges` from the cell merge dimensions after +reading, or make sure the `AcDbFormattedTableData` block is parsed for entities. + +--- + +## Observed but not filed + +These are format constraints or OCS-side issues, not reader bugs: + +- **DWG R2010+ does not store** a leader's `text_height`, `text_width` or + `hookline_enabled`, or a 3-D polyline's `elevation`; the writer stores them + only for the older versions that define them. +- **Entity-level `MultiLeader.text_height`** is not stored by either writer + (`context.text_height` is), so OCS exposes it read-only. +- **Legacy `Polyline` reopens as `Polyline3D`** from both formats and a 2-D + polyline reopens as `LwPolyline` from DXF. These look like deliberate + normalization; OCS made the legacy kind update-only and records the DXF + conversion. +- **Spline weights in DXF** were briefly suspected but are correct: the writer + emits group 41 only when `flags.rational` is set, so the host must set that + flag when weights are present (fixed in OCS). diff --git a/docs/plugin-host-model-completion-plan.md b/docs/plugin-host-model-completion-plan.md index c9a104d9d..64d11d655 100644 --- a/docs/plugin-host-model-completion-plan.md +++ b/docs/plugin-host-model-completion-plan.md @@ -9,7 +9,7 @@ As of 20 September 2026, the working branch is `plugin/host-model-api` at [#1391](https://github.com/HakanSeven12/OpenCADStudio/pull/1391). Host API is v7. The Python feature has explicit writes for **37 of 43** canvas kinds: 36 are creatable (the legacy `Polyline` is update-only). This is **not** 37 completed -kinds: Tolerance, Shape, AttributeEntity, Hatch, MLine, Dimension, MultiLeader, Table, PolygonMesh, PolyfaceMesh, Mesh, Wipeout, Point, Line, Circle, Arc, Ellipse, LwPolyline, Text, MText, Ray, XLine, Solid and Face3D pass every completion gate; Insert now passes every gate too; the audited Polyline (update-only by decision), Polyline2D, Polyline3D and Spline pass every gate that their format allows. +kinds: Tolerance, Shape, AttributeEntity, Hatch, MLine, Dimension, MultiLeader, Table, PolygonMesh, PolyfaceMesh, Mesh, Wipeout, Point, Line, Circle, Arc, Ellipse, LwPolyline, Spline, Text, MText, Ray, XLine, Solid and Face3D pass every completion gate; Insert now passes every gate too; the audited Polyline (update-only by decision), Polyline2D and Polyline3D pass every gate that their format allows. AttributeDefinition passes its host and real IPC lifecycle but remains short of `Complete` because the previously pinned CAD codec dropped optional ATTDEF fields during DXF reads; revalidate that blocker against the current @@ -174,3 +174,4 @@ exact queue above and update statuses with evidence after each increment. - Audited the 17 pre-existing mapped kinds through the shared real-runner driver (`audit_python_*`). The audit found and fixed genuine validation gaps: Ellipse had none, Text and MText accepted a zero height, Arc accepted non-finite angles, Point and Line accepted infinite thickness, and most basic kinds had no creation-time validation. A shared `validate_basic_entity` now covers Point, Line, Circle, Arc, Ellipse, Text, MText, LwPolyline, Polyline, Polyline2D, Polyline3D and Spline, refusing only problems an edit introduces so already-invalid legacy values stay editable. Twelve of the 17 kinds now pass every gate (Point, Line, Circle, Arc, Ellipse, LwPolyline, Text, MText, Ray, XLine, Solid, Face3D). Four expose persistence findings pinned by canaries (Polyline reopens as Polyline3D; Polyline2D reopens as LwPolyline in DXF; Polyline3D loses elevation in DWG; Spline loses weights in DXF), and Insert is update-only with all applicable gates passing. The driver gained per-format DWG expectations and a host-made block fixture for Insert. - Audited `AttributeDefinition` against acadrust `5b682ed` with a full-field comparison through both formats. Everything passes in DWG. DXF keeps only tag, prompt, value, insertion point, height and rotation because the reader handles eight group codes; alignment, width factor, oblique angle, attribute flags, field length, generation flags, lock and text style reopen as defaults. The kind stays integration-tested and the gap is pinned by canary expectations; the fix belongs in cadcodec's `read_attdef`. The shared driver's block fixture now also serves `MAKEBLOCK` scripts. - Decided the two open audit questions. **Insert is creatable** by naming an existing block (existence, ordinary-block and no-cycle checks; attribute records stay with the AttributeEntity flow), which completes the kind. **Legacy Polyline is update-only** with a hint pointing scripts at Polyline2D and Polyline3D, because both formats convert it on save. The generator gained an `update_only_message` for such kinds, integer coordinates are accepted in Insert scales, and the driver gained `MAKEPOLYLINE` and `MAKEBLOCK` fixtures. +- Wrote up the cadcodec DXF reader gaps as draft issues in [cadcodec-reader-gaps.md](cadcodec-reader-gaps.md) (nothing filed): ATTDEF, boolean groups read through `as_i16()`, UNDERLAY rotation units, LEADER, DRAWINGVIEW/SECTIONLINE dispatch and TABLE merged ranges. Verifying them corrected an earlier attribution: the Spline "DXF drops weights" finding was an OCS bug (the writer emits weights only for a rational spline), now fixed by setting `flags.rational` from the weights, so Spline is complete. diff --git a/docs/plugin-host-model-coverage-ledger.md b/docs/plugin-host-model-coverage-ledger.md index 931abf8b9..13a1f79e5 100644 --- a/docs/plugin-host-model-coverage-ledger.md +++ b/docs/plugin-host-model-coverage-ledger.md @@ -1,6 +1,6 @@ # OCS canvas kind coverage ledger -Use with [the completion plan](plugin-host-model-completion-plan.md). This ledger is current as of 20 September 2026: **37 of 43 canvas kinds have explicit geometry writes** (36 creatable, plus update-only legacy `Polyline`), **25 kinds are complete**, and 6 remain layer-only (one of them, SectionSymbol, blocked). `Mapped` means at least one non-layer field is exposed for Python writes; it does **not** mean the kind passes the completion gate. `Update-only` means Python creation is unavailable. `Layer-only` means raw inspection and canvas layer edits exist, but geometry is not mapped. Record actual evidence or a blocking issue before changing any status to `Complete`. +Use with [the completion plan](plugin-host-model-completion-plan.md). This ledger is current as of 20 September 2026: **37 of 43 canvas kinds have explicit geometry writes** (36 creatable, plus update-only legacy `Polyline`), **26 kinds are complete**, and 6 remain layer-only (one of them, SectionSymbol, blocked). `Mapped` means at least one non-layer field is exposed for Python writes; it does **not** mean the kind passes the completion gate. `Update-only` means Python creation is unavailable. `Layer-only` means raw inspection and canvas layer edits exist, but geometry is not mapped. Record actual evidence or a blocking issue before changing any status to `Complete`. Completion gate codes: **C** = create through Python document model; **R** = read; **E** = edit defining geometry and another property; **D** = delete; **U** = undo/redo and atomic failure; **I** = real IPC plus GUI/canvas pick or observable behavior; **W** = DWG and DXF save/reopen; **V** = invalid-input checks and untouched-field/reference preservation; **P** = build and load the in-tree bundled plugin with a manifest matching the host toolchain and acadrust source. A kind is `Complete` only when **C/R/E/D/U/I/W/V/P** all pass and its property catalog matches implementation. `Blocked` requires a specific dependency or failing fixture. @@ -15,7 +15,7 @@ Completion gate codes: **C** = create through Python document model; **R** = rea | Polyline2D | **Integration-tested / kind changes on DXF save** | Exercised: `vertices`, `closed`, `thickness`. Passes every gate in DWG. DXF reopens a 2-D polyline as an `LwPolyline` (vertices, closure and width survive, but the kind changes and `thickness` is not part of that record's digest here); pinned by a canary expectation. `audit_python_polyline2d_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`, real runner) passes C/R/E/D/U/I/W/V/P: creation, a two-or-more-property transaction edit with the entity's common data, identity and selection preserved, canvas wires, invalid-input rejection with an atomic multi-entity rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. The audit added the shared `validate_basic_entity` (creation must be valid; an edit is refused only for problems it introduces, so already-invalid legacy values stay editable). | | Polyline3D | **Integration-tested / blocked at W by the DWG format (elevation)** | Exercised: `vertices`, `elevation`. Passes every gate in DXF; DWG does not store a 3-D polyline's `elevation`, so it reopens as 0 (pinned by a canary). Vertices persist. `audit_python_polyline3d_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`, real runner) passes C/R/E/D/U/I/W/V/P: creation, a two-or-more-property transaction edit with the entity's common data, identity and selection preserved, canvas wires, invalid-input rejection with an atomic multi-entity rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. The audit added the shared `validate_basic_entity` (creation must be valid; an edit is refused only for problems it introduces, so already-invalid legacy values stay editable). | | LwPolyline | **Complete** | Exercised: `vertices` (with a bulge), `is_closed`, `constant_width`. Rejects fewer than 2 vertices, non-finite locations, bulges and widths, a negative constant width and a zero normal. `audit_python_lwpolyline_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`, real runner) passes C/R/E/D/U/I/W/V/P: creation, a two-or-more-property transaction edit with the entity's common data, identity and selection preserved, canvas wires, invalid-input rejection with an atomic multi-entity rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. The audit added the shared `validate_basic_entity` (creation must be valid; an edit is refused only for problems it introduces, so already-invalid legacy values stay editable). | -| Spline | **Integration-tested / blocked at W by acadrust (DXF weights)** | Exercised: `control_points`, `weights`. Validation covers degree, control-point count, knot count and order, weights and finite points. Passes every gate in DWG; the DXF reader does not restore `weights` (a rational spline reopens non-rational; pinned by a canary). `audit_python_spline_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`, real runner) passes C/R/E/D/U/I/W/V/P: creation, a two-or-more-property transaction edit with the entity's common data, identity and selection preserved, canvas wires, invalid-input rejection with an atomic multi-entity rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. The audit added the shared `validate_basic_entity` (creation must be valid; an edit is refused only for problems it introduces, so already-invalid legacy values stay editable). | +| Spline | **Complete** | Exercised: `control_points`, `weights`. Validation covers degree, control-point count, knot count and order, weights and finite points. The host sets `flags.rational` whenever weights are present, because the DXF writer emits group 41 only for a rational spline; the audit first showed weights vanishing in DXF and traced it to that missing flag (an OCS bug, not a cadcodec one), now fixed. `audit_python_spline_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`, real runner) passes C/R/E/D/U/I/W/V/P with DWG and DXF reopen, re-edit, atomic rollback and three-step undo/redo. | | Text | **Complete** | Exercised: `text`, `rotation`, `height`. Rejects a non-positive height (previously accepted), a non-positive width factor, non-finite placement and angles, and a zero normal. Style and alignment fields are mapped but not individually exercised. `audit_python_text_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`, real runner) passes C/R/E/D/U/I/W/V/P: creation, a two-or-more-property transaction edit with the entity's common data, identity and selection preserved, canvas wires, invalid-input rejection with an atomic multi-entity rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. The audit added the shared `validate_basic_entity` (creation must be valid; an edit is refused only for problems it introduces, so already-invalid legacy values stay editable). | | MText | **Complete** | Exercised: `text`, `rectangle_width`, `rotation`. Rejects a non-positive height (previously accepted), a negative width, non-finite placement, a non-positive line-spacing factor and a zero normal. Attachment, columns and background fields are mapped but not individually exercised. `audit_python_mtext_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`, real runner) passes C/R/E/D/U/I/W/V/P: creation, a two-or-more-property transaction edit with the entity's common data, identity and selection preserved, canvas wires, invalid-input rejection with an atomic multi-entity rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. The audit added the shared `validate_basic_entity` (creation must be valid; an edit is refused only for problems it introduces, so already-invalid legacy values stay editable). | | Ray | **Complete** | Exercised: `base_point`, `direction`. Unit-direction and finite-point validation already existed; the audit added the full DWG and DXF reopen and re-edit gate. `audit_python_ray_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`, real runner) passes C/R/E/D/U/I/W/V/P: creation, a two-or-more-property transaction edit with the entity's common data, identity and selection preserved, canvas wires, invalid-input rejection with an atomic multi-entity rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. The audit added the shared `validate_basic_entity` (creation must be valid; an edit is refused only for problems it introduces, so already-invalid legacy values stay editable). | diff --git a/src/app/plugin_host.rs b/src/app/plugin_host.rs index 4bb0116ef..e9fc9d03c 100644 --- a/src/app/plugin_host.rs +++ b/src/app/plugin_host.rs @@ -158,6 +158,11 @@ impl<'a> HostSession<'a> { if let EntityType::Dimension(dimension) = entity { crate::entities::dimension::normalize_scripted_dimension(dimension); } + if let EntityType::Spline(spline) = entity { + // The DXF writer emits weights only for a rational spline, so the + // flag must follow the weights a script sets. + spline.flags.rational = !spline.weights.is_empty(); + } if let EntityType::Helix(new) = entity { let old = match old { Some(EntityType::Helix(old)) => Some(old), @@ -4208,8 +4213,8 @@ mod tests { expect_reedited: "deg2 cp3 y9.0 w[1.0, 2.0, 1.0]", expect_edited_dwg: "", expect_reedited_dwg: "", - expect_edited_dxf: "deg2 cp3 y8.0 w[]", - expect_reedited_dxf: "deg2 cp3 y9.0 w[]", + expect_edited_dxf: "", + expect_reedited_dxf: "", }); }