From d4c25209072e8eaa27a613a1037787a09311047f Mon Sep 17 00:00:00 2001 From: Felix Riestra Date: Sun, 20 Sep 2026 14:48:44 +0200 Subject: [PATCH] Map Helix in the Python host model; record DXF handedness blocker The host rebuilds the derived spline from the generating parameters on scripted create/edit. staged_python_helix_lifecycle_over_real_ipc passes every gate except DXF handedness, which acadrust's reader never applies; the gap is pinned by a canary via per-format expectations in the shared mesh lifecycle driver. Co-Authored-By: Claude Sonnet 5 --- crates/ocs_plugin_api/build.rs | 1 + .../entity_coverage_policy.json | 16 +++++ crates/ocs_plugin_api/src/entity_coverage.rs | 49 +++++++++++++ docs/plugin-host-model-completion-plan.md | 7 +- docs/plugin-host-model-coverage-ledger.md | 4 +- docs/plugin-host-model.md | 4 +- plugins/opencad-python/README.md | 2 +- plugins/opencad-python/entity_manifest.json | 11 ++- plugins/opencad-python/src/ocs_module.rs | 2 +- src/app/plugin_host.rs | 72 ++++++++++++++++++- src/entities/helix.rs | 25 +++++++ 11 files changed, 180 insertions(+), 13 deletions(-) diff --git a/crates/ocs_plugin_api/build.rs b/crates/ocs_plugin_api/build.rs index 4d0e3ae57..958375596 100644 --- a/crates/ocs_plugin_api/build.rs +++ b/crates/ocs_plugin_api/build.rs @@ -790,6 +790,7 @@ fn generate_entity_coverage(out_dir: &Path, registry: &TypeRegistry) { | ("PolygonMesh", _) | ("PolyfaceMesh", _) | ("Mesh", _) + | ("Helix", _) | ( "MLine", "flags" diff --git a/crates/ocs_plugin_api/entity_coverage_policy.json b/crates/ocs_plugin_api/entity_coverage_policy.json index fc277f86b..5c5153824 100644 --- a/crates/ocs_plugin_api/entity_coverage_policy.json +++ b/crates/ocs_plugin_api/entity_coverage_policy.json @@ -377,6 +377,17 @@ "faces", "edges", "override_option" + ], + "Helix": [ + "layer", + "axis_base_point", + "start_point", + "axis_vector", + "radius", + "turns", + "turn_height", + "handedness", + "constraint" ] }, "readable": { @@ -432,6 +443,11 @@ ], "Mesh": [ "version" + ], + "Helix": [ + "spline", + "major_version", + "maintenance_version" ] }, "aliases": { diff --git a/crates/ocs_plugin_api/src/entity_coverage.rs b/crates/ocs_plugin_api/src/entity_coverage.rs index 1e56b8b75..4ea721692 100644 --- a/crates/ocs_plugin_api/src/entity_coverage.rs +++ b/crates/ocs_plugin_api/src/entity_coverage.rs @@ -743,6 +743,44 @@ fn validate_mesh( Ok(()) } +/// Helix generating parameters. The spline is derived by the host, so only +/// the parameters are validated. +#[cfg(feature = "host")] +fn validate_helix( + old: Option<&crate::host::acadrust::entities::Helix>, + new: &crate::host::acadrust::entities::Helix, +) -> Result<(), String> { + if old.is_some_and(|old| { + old.axis_base_point == new.axis_base_point + && old.start_point == new.start_point + && old.axis_vector == new.axis_vector + && old.radius.to_bits() == new.radius.to_bits() + && old.turns.to_bits() == new.turns.to_bits() + && old.turn_height.to_bits() == new.turn_height.to_bits() + && old.handedness == new.handedness + && old.constraint == new.constraint + }) { + return Ok(()); + } + finite_vector("Helix.axis_base_point", &new.axis_base_point)?; + finite_vector("Helix.start_point", &new.start_point)?; + solid_normal("Helix.axis_vector", &new.axis_vector)?; + for (name, value) in [("radius", new.radius), ("turns", new.turns), ("turn_height", new.turn_height)] { + if !value.is_finite() || value <= 0.0 { + return Err(format!("Helix.{name} must be finite and greater than zero")); + } + } + let axis = new.axis_vector; + let length = axis.x.hypot(axis.y).hypot(axis.z); + let delta = new.start_point - new.axis_base_point; + let along = (delta.x * axis.x + delta.y * axis.y + delta.z * axis.z) / length; + let radial = ((delta.x * delta.x + delta.y * delta.y + delta.z * delta.z) - along * along).max(0.0).sqrt(); + if radial <= 1.0e-9 { + return Err("Helix.start_point must not lie on the axis".into()); + } + Ok(()) +} + /// Validate newly created geometry for the kinds whose mapped fields have /// host checks. Called by the Python add path before an entity enters the document. #[cfg(feature = "host")] @@ -842,6 +880,7 @@ pub fn validate_new_canvas_entity(entity: &crate::host::EntityType) -> Result<() EntityType::PolygonMesh(value) => validate_polygon_mesh(None, value)?, EntityType::PolyfaceMesh(value) => validate_polyface_mesh(None, value)?, EntityType::Mesh(value) => validate_mesh(None, value)?, + EntityType::Helix(value) => validate_helix(None, value)?, EntityType::AttributeDefinition(value) => { finite_vector( "AttributeDefinition.insertion_point", @@ -1202,6 +1241,7 @@ pub fn validate_entity_mutation( (EntityType::PolygonMesh(old), EntityType::PolygonMesh(new)) => validate_polygon_mesh(Some(old), new)?, (EntityType::PolyfaceMesh(old), EntityType::PolyfaceMesh(new)) => validate_polyface_mesh(Some(old), new)?, (EntityType::Mesh(old), EntityType::Mesh(new)) => validate_mesh(Some(old), new)?, + (EntityType::Helix(old), EntityType::Helix(new)) => validate_helix(Some(old), new)?, (EntityType::MLine(old), EntityType::MLine(new)) => validate_mline(Some(old), new)?, (EntityType::Leader(old), EntityType::Leader(new)) => validate_leader(Some(old), new)?, (EntityType::AttributeDefinition(old), EntityType::AttributeDefinition(new)) => { @@ -1879,6 +1919,7 @@ mod tests { | "PolygonMesh" | "PolyfaceMesh" | "Mesh" + | "Helix" ) { assert!( @@ -2134,6 +2175,14 @@ mod tests { "Mesh.faces".to_owned(), "Mesh.edges".to_owned(), "Mesh.override_option".to_owned(), + "Helix.axis_base_point".to_owned(), + "Helix.start_point".to_owned(), + "Helix.axis_vector".to_owned(), + "Helix.radius".to_owned(), + "Helix.turns".to_owned(), + "Helix.turn_height".to_owned(), + "Helix.handedness".to_owned(), + "Helix.constraint".to_owned(), ]) ); } diff --git a/docs/plugin-host-model-completion-plan.md b/docs/plugin-host-model-completion-plan.md index 8fd05c77a..128f70c8f 100644 --- a/docs/plugin-host-model-completion-plan.md +++ b/docs/plugin-host-model-completion-plan.md @@ -7,13 +7,13 @@ Build a general Python document model over the OCS host API for all **43 canvas As of 20 September 2026, the working branch is `plugin/host-model-api` at `a8dd3640` plus the Leader increment, published as draft PR [#1391](https://github.com/HakanSeven12/OpenCADStudio/pull/1391). Host API is -v7. The Python feature has explicit writes for **30 of 43** canvas kinds: 29 +v7. The Python feature has explicit writes for **31 of 43** canvas kinds: 30 are creatable and `Insert` remains update-only. This is **not** 30 completed kinds: Tolerance, Shape, AttributeEntity, Hatch, MLine, Dimension, MultiLeader, Table, PolygonMesh, PolyfaceMesh and Mesh pass every completion gate. 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 -`acadrust` revision `7ea4247`. Leader is mapped and integration-tested but blocked at W by three acadrust persistence gaps (see the ledger). The other 13 canvas kinds have only `layer` +`acadrust` revision `7ea4247`. Leader is mapped and integration-tested but blocked at W by three acadrust persistence gaps (see the ledger). Leader and Helix are mapped and integration-tested but blocked at W by acadrust persistence gaps (see the ledger). The other 12 canvas kinds have only `layer` writes through the document model. All 43 have raw typed snapshots. There are three internal records and two opaque fallbacks outside the 43 canvas kinds. @@ -123,7 +123,7 @@ The last five may require new engine capabilities rather than only Python conver ## Handoff start point -Continue with **Helix** (queue item 9). Leader is done except for its W +Continue with **RasterImage** (queue item 10). Leader is done except for its W gate; when the engine writes DXF group 340, reads group 213 and the DWG R2010+ text-size question is decided, flip the canary assertions in `staged_python_leader_lifecycle_over_real_ipc` and mark it `Complete`. @@ -163,3 +163,4 @@ exact queue above and update statuses with evidence after each increment. - Completed `MultiLeader`. The adapter generator now converts `Color`, `LineWeight`, mixed unit/payload enums, fixed float arrays and bitflags-2 newtypes, and a `keep_types` override lets a hand-written setter reuse generated converters (used for the merge-style `context` patch). `Leader.override_color` became writable through the same `Color` form; neither file format stores it. `staged_python_multileader_lifecycle_over_real_ipc` passes the full gate with DWG and DXF reopen. The queue now continues with `Table`. - Completed `Table`. The generator now handles tuples and `usize`; the host keeps DXF cell merge dimensions in step with `merged_ranges`. `staged_python_table_lifecycle_over_real_ipc` passes the full gate with DWG and DXF reopen. The queue now continues with `PolygonMesh`. - Completed `PolygonMesh`, `PolyfaceMesh` and `Mesh` (queue items 6-8) with one shared `run_mesh_lifecycle` test driver and per-kind host validation. The generator now omits a sub-record's own `EntityCommon` from the scripted form. All three pass the full gate including DWG and DXF reopen with re-edit. The queue now continues with `Helix`. +- Mapped `Helix` with a host-rebuilt spline. `run_mesh_lifecycle` gained per-format DXF expectations so an engine gap is pinned by a canary instead of hidden. All gates pass except W for `handedness` in DXF (reader ignores boolean group 290). The queue now continues with `RasterImage`. diff --git a/docs/plugin-host-model-coverage-ledger.md b/docs/plugin-host-model-coverage-ledger.md index 1f1d2cb1c..326839968 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: **30 of 43 canvas kinds have explicit geometry writes** (29 creatable, plus update-only `Insert`), **11 kinds are complete**, and 13 remain layer-only. `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: **31 of 43 canvas kinds have explicit geometry writes** (30 creatable, plus update-only `Insert`), **11 kinds are complete**, and 12 remain layer-only. `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. @@ -36,7 +36,7 @@ Completion gate codes: **C** = create through Python document model; **R** = rea | PolygonMesh | **Complete** | Writable: `flags`, `m_vertex_count`, `n_vertex_count`, `m_smooth_density`, `n_smooth_density`, `smooth_type`, `elevation`, `normal`, `vertices` (each `{location, flags}`; a vertex's own common data is not scripted and defaults). Host validation requires M and N of at least 2, exactly M×N vertices, non-negative densities, finite elevation and locations, and a nonzero normal, so inconsistent counts are refused before any mutation. `staged_python_polygon_mesh_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`) passes C/R/E/D/U/I/W/V/P: a 3×3 grid, a vertex move plus density and cubic smoothing in one transaction, canvas wires, seven rejection cases plus an atomic rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. | | PolyfaceMesh | **Complete** | Writable: `elevation`, `flags`, `normal`, `start_width`, `end_width`, `smooth_surface`, `thickness`, `vertices` (location, flags, bulge, widths, tangent, id) and `faces` (four indices, flags, optional colour). `seqend_handle` is read-only and preserved. Host validation requires at least 3 finite vertices and 1 face, every face index within the vertex count (a negative index hides that edge, 0 marks an unused corner), at least 3 indices per face, and finite widths, thickness and elevation. `staged_python_polyface_mesh_lifecycle_over_real_ipc` passes the full gate: two triangles, a vertex move plus a new vertex and a hidden-edge face in one transaction, canvas wires, six rejection cases plus an atomic rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. | | Mesh | **Complete** | Writable: `blend_crease`, `subdivision_level`, `vertices`, `faces`, `edges` (start, end, optional crease), `override_option`; `version` is read-only. Host validation requires at least 3 finite vertices, faces of at least 3 in-range indices, edges with distinct in-range endpoints and a finite non-negative crease, and a non-negative subdivision level, so out-of-range topology is rejected before mutation. `staged_python_mesh_lifecycle_over_real_ipc` passes the full gate: a quad with a creased edge, an added vertex and triangle plus subdivision level 1 in one transaction, canvas wires, seven rejection cases plus an atomic rollback, DWG and DXF reopen with re-edit, and three-step undo/redo. | -| Helix | Layer-only | Requires valid derived-spline fixture. | +| Helix | **Integration-tested / blocked at W by acadrust (DXF handedness)** | Writable generating parameters: `axis_base_point`, `start_point`, `axis_vector`, `radius` (the top radius; the base radius is the start point's distance from the axis), `turns`, `turn_height`, `handedness`, `constraint`. Read-only: `spline`, `major_version`, `maintenance_version`. The host rebuilds the derived NURBS spline with OCS's own helix builder on every scripted create or parameter edit (`normalize_scripted_helix`), so the spline always matches the parameters. Host validation requires finite positive radius, turns and turn height, a nonzero axis and a start point off the axis. `staged_python_helix_lifecycle_over_real_ipc` (shared `run_mesh_lifecycle`) passes C/R/E/D/U/I/V/P: creation with a derived spline ending at z = turns × turn height, a turns and handedness edit that regenerates it, canvas wires, seven rejection cases plus an atomic rollback, DWG reopen and re-edit, DXF reopen and re-edit, and three-step undo/redo. **W blocker (engine, pinned by a canary assertion):** the DXF reader parses boolean group 290 as an `i16` and never applies it, so a clockwise helix reopens counter-clockwise from DXF while DWG keeps it. Not `Complete` until the reader is fixed. | | RasterImage | Layer-only | Requires image definition and file fixture. | | Wipeout | Layer-only | Requires clip and image-definition fixture. | | Underlay | Layer-only | Requires external definition/file fixture. | diff --git a/docs/plugin-host-model.md b/docs/plugin-host-model.md index e6c25d849..d1f6a1a02 100644 --- a/docs/plugin-host-model.md +++ b/docs/plugin-host-model.md @@ -35,7 +35,7 @@ attached attribute and sequence records. Those fields report `transaction_geomet catalog. All 43 canvas kinds also support nonempty `layer` changes through the same undoable transaction; internal and opaque variants remain read-only through the document model. The adapter accepts only `layer` patches for -canvas kinds outside its 30 geometry converters. Other writable properties +canvas kinds outside its 31 geometry converters. Other writable properties still report `type_conversion_only`; broader CAD range and cross-property checks remain outstanding. The host's `entity_snapshot` helper serializes any typed entity as a detached @@ -91,7 +91,7 @@ Input tokens are bound to the drawing tab that requested the pick. Polling a token from another tab returns no result and does not consume it. Python `doc.entities[handle]` returns a descriptor for every entity. For kinds -outside the 30-kind generated schema it contains only handle, kind, and layer; +outside the 31-kind generated schema it contains only handle, kind, and layer; `layer` is editable on canvas kinds. `doc.coverage()` enumerates the catalog; `doc.coverage("Line")` and `line.coverage` return one entry. For covered kinds, the coverage object lists actual readable and editable diff --git a/plugins/opencad-python/README.md b/plugins/opencad-python/README.md index 214d16b05..b46227440 100644 --- a/plugins/opencad-python/README.md +++ b/plugins/opencad-python/README.md @@ -94,7 +94,7 @@ The host-owned catalog classifies all 48 entity variants and tracks every traced field, including fields marked `unmapped`. Query it with `doc.coverage()` or `doc.coverage("Line")`; each row includes source field, type, access, and current validation status. Geometry edits remain limited to -the 30 mapped kinds. +the 31 mapped kinds. For any entity, `entity.snapshot` returns a detached dictionary of its raw CAD fields, including `snapshot["common"]` and kind-specific nested values. It can inspect an unmapped kind; editing the dictionary does diff --git a/plugins/opencad-python/entity_manifest.json b/plugins/opencad-python/entity_manifest.json index 011b31133..a6f2e60dd 100644 --- a/plugins/opencad-python/entity_manifest.json +++ b/plugins/opencad-python/entity_manifest.json @@ -29,7 +29,8 @@ "Table", "PolygonMesh", "PolyfaceMesh", - "Mesh" + "Mesh", + "Helix" ], "manual_kinds": ["Dimension"], "base_fields": ["handle", "layer", "owner_handle"], @@ -331,6 +332,14 @@ "faces": { "required": true }, "version": { "read_only": true } } + }, + "Helix": { + "fields": { + "start_point": { "required": true }, + "spline": { "read_only": true, "_comment": "Derived by the host from the generating parameters." }, + "major_version": { "read_only": true }, + "maintenance_version": { "read_only": true } + } } } } diff --git a/plugins/opencad-python/src/ocs_module.rs b/plugins/opencad-python/src/ocs_module.rs index 26bd318bf..bb6537a92 100644 --- a/plugins/opencad-python/src/ocs_module.rs +++ b/plugins/opencad-python/src/ocs_module.rs @@ -4756,7 +4756,7 @@ mod tests { ); } } - assert_eq!(emitted.len(), 30); + assert_eq!(emitted.len(), 31); } #[cfg(feature = "experimental-host-model")] diff --git a/src/app/plugin_host.rs b/src/app/plugin_host.rs index 9a13acd6b..949b57a1b 100644 --- a/src/app/plugin_host.rs +++ b/src/app/plugin_host.rs @@ -158,6 +158,13 @@ impl<'a> HostSession<'a> { if let EntityType::Dimension(dimension) = entity { crate::entities::dimension::normalize_scripted_dimension(dimension); } + if let EntityType::Helix(new) = entity { + let old = match old { + Some(EntityType::Helix(old)) => Some(old), + _ => None, + }; + crate::entities::helix::normalize_scripted_helix(old, new)?; + } if let EntityType::Table(new) = entity { let old = match old { Some(EntityType::Table(old)) => Some(old), @@ -3112,6 +3119,10 @@ mod tests { expect_created: &'static str, expect_edited: &'static str, expect_reedited: &'static str, + /// Digest a DXF reopen yields when the engine loses a field (empty: + /// same as DWG). A canary that fails once the engine round-trips it. + expect_edited_dxf: &'static str, + expect_reedited_dxf: &'static str, } fn run_mesh_lifecycle(case: &MeshCase) { @@ -3172,13 +3183,18 @@ mod tests { let dxf = crate::io::load_bytes("mesh.dxf", acadrust::DxfWriter::new(host.document()).write_to_vec().unwrap()).unwrap(); let mut gaps = Vec::new(); for (format, document) in [("DWG", &dwg), ("DXF", &dxf)] { + let expected_digest = if format == "DXF" && !case.expect_edited_dxf.is_empty() { + case.expect_edited_dxf + } else { + case.expect_edited + }; match document.get_entity(handle) { - Some(entity) if (case.digest)(entity) == case.expect_edited => {} + Some(entity) if (case.digest)(entity) == expected_digest => {} Some(entity) => gaps.push(format!("{format}: {}", (case.digest)(entity))), None => gaps.push(format!("{format}: entity missing")), } } - assert!(gaps.is_empty(), "persistence gaps (expected {}): {gaps:#?}", case.expect_edited); + assert!(gaps.is_empty(), "persistence gaps (expected {} / DXF {:?}): {gaps:#?}", case.expect_edited, case.expect_edited_dxf); for (name, mut document) in [("mesh-reedit.dwg", dwg), ("mesh-reedit.dxf", dxf)] { let before = document.get_entity(handle).unwrap().clone(); let mut after = before.clone(); @@ -3192,7 +3208,12 @@ mod tests { acadrust::DxfWriter::new(&document).write_to_vec().unwrap() }; let reopened = crate::io::load_bytes(name, bytes).unwrap(); - assert_eq!((case.digest)(reopened.get_entity(handle).unwrap()), case.expect_reedited, "{name}"); + let expected_digest = if name.ends_with("dxf") && !case.expect_reedited_dxf.is_empty() { + case.expect_reedited_dxf + } else { + case.expect_reedited + }; + assert_eq!((case.digest)(reopened.get_entity(handle).unwrap()), expected_digest, "{name}"); } dispatch(&mut host, &format!("PY_EVAL ocs.active_document.delete_entity({})", handle.value())); @@ -3252,6 +3273,8 @@ mod tests { expect_created: "3x3 v9 z0 d0/0 NoSmooth", expect_edited: "3x3 v9 z2.5 d6/6 Cubic", expect_reedited: "3x3 v9 z2.5 d6/6 Cubic", + expect_edited_dxf: "", + expect_reedited_dxf: "", }); } @@ -3293,6 +3316,8 @@ mod tests { expect_created: "v4 f2 z0 last Some((1, 3, 4))", expect_edited: "v5 f3 z3 last Some((3, -4, 5))", expect_reedited: "v5 f3 z3 last Some((3, -4, 5))", + expect_edited_dxf: "", + expect_reedited_dxf: "", }); } @@ -3333,6 +3358,47 @@ mod tests { expect_created: "v4 f1 e1 sub0 crease Some(1.5)", expect_edited: "v5 f2 e1 sub1 crease Some(1.5)", expect_reedited: "v5 f2 e1 sub1 crease Some(1.5)", + expect_edited_dxf: "", + expect_reedited_dxf: "", + }); + } + + #[test] + fn staged_python_helix_lifecycle_over_real_ipc() { + run_mesh_lifecycle(&MeshCase { + create: concat!( + "doc.create_entity('Helix', axis_base_point=P(0.0, 0.0, 0.0), axis_vector=P(0.0, 0.0, 1.0),\n", + " start_point=P(5.0, 0.0, 0.0), radius=5.0, turns=3.0, turn_height=2.0)\n"), + edit: concat!( + "helix = doc.entities[HANDLE]\n", + "with doc.transaction('Edit helix'):\n", + " helix.turns = 5.0\n", + " helix.handedness = False\n", + "doc.selection = [helix]\n"), + rejects: &[ + ("'radius':0.0", "greater than zero"), + ("'turns':-1.0", "greater than zero"), + ("'turn_height':float('nan')", "greater than zero"), + ("'axis_vector':{'x':0.0,'y':0.0,'z':0.0}", "nonzero"), + ("'start_point':{'x':0.0,'y':0.0,'z':3.0}", "on the axis"), + ("'spline':{}", "read-only"), + ("'major_version':1", "read-only"), + ], + is_kind: |entity| matches!(entity, EntityType::Helix(_)), + digest: |entity| match entity { + EntityType::Helix(h) => format!("r{} t{} h{} endz{:.1} ccw{} cp{}", h.radius, h.turns, h.turn_height, + h.spline.control_points.last().map_or(f64::NAN, |p| p.z), h.handedness, + !h.spline.control_points.is_empty()), + _ => "wrong kind".into(), + }, + reedit: |entity| if let EntityType::Helix(h) = entity { h.turns = 6.0; }, + expect_created: "r5 t3 h2 endz6.0 ccwtrue cptrue", + expect_edited: "r5 t5 h2 endz10.0 ccwfalse cptrue", + expect_reedited: "r5 t6 h2 endz10.0 ccwfalse cptrue", + // BLOCKER: the DXF reader parses boolean group 290 as an i16 and + // never applies it, so handedness always reopens counter-clockwise. + expect_edited_dxf: "r5 t5 h2 endz10.0 ccwtrue cptrue", + expect_reedited_dxf: "r5 t6 h2 endz10.0 ccwtrue cptrue", }); } diff --git a/src/entities/helix.rs b/src/entities/helix.rs index e78643321..2d479f17f 100644 --- a/src/entities/helix.rs +++ b/src/entities/helix.rs @@ -602,3 +602,28 @@ mod tests { } } } + +/// Rebuild the derived spline after a scripted create or edit of the +/// generating parameters. `radius` is the top radius; the base radius is the +/// start point's distance from the axis. +pub(crate) fn normalize_scripted_helix(old: Option<&Helix>, new: &mut Helix) -> Result<(), String> { + let unchanged = old.is_some_and(|old| { + old.axis_base_point == new.axis_base_point + && old.start_point == new.start_point + && old.axis_vector == new.axis_vector + && old.radius.to_bits() == new.radius.to_bits() + && old.turns.to_bits() == new.turns.to_bits() + && old.turn_height.to_bits() == new.turn_height.to_bits() + && old.handedness == new.handedness + && old.constraint == new.constraint + }); + if unchanged { + return Ok(()); + } + let top = new.radius; + if rebuild(new, top) { + Ok(()) + } else { + Err("Helix parameters do not define a valid curve".into()) + } +}