Draft cadcodec issues for the DXF reader gaps; fix Spline rational flag

Documents six cadcodec reader/writer gaps with source references, canary
tests and suggested fixes (nothing filed). Verifying them showed the Spline
DXF weight loss was an OCS bug: the writer emits weights only for a rational
spline, so the host now sets flags.rational from the weights. Spline passes
every gate.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
Felix RiestraandClaude Sonnet 5 committed 2026-09-20 16:37:41 +02:00
1 parent 226860d3b3
commit f1813b3ebe
4 files changed
+207 -5

No files matched your search

+196
View File
@@ -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 name> -- --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).
+2 -1
View File
@@ -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.
+2 -2
View File
@@ -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). |
+7 -2
View File
@@ -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: "",
});
}