From 3a416ccb7da5d13aa0746ebef3007241642dcbc8 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Fri, 9 Oct 2026 21:31:18 +0000 Subject: [PATCH] Profiles picked by index are kept by the curves around them: add_feature turns profile indices into the curve ids of each profile's outer loop (Extrude, Revolve, Sweep, Loft, Emboss, Sheet Base, Patch), so a face sketch whose projected outline is rebuilt still extrudes the picked region; a profile that is gone is an error on the feature, never another pick Fixes #30 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RjGw4gA5ZvKrU8MQP6sBgT --- crates/doc/src/document.rs | 14 ++++ crates/engine/src/cmd/features.rs | 34 ++++++++++ crates/engine/src/lib.rs | 2 + crates/engine/src/profile_refs_tests.rs | 87 +++++++++++++++++++++++++ 4 files changed, 137 insertions(+) create mode 100644 crates/engine/src/profile_refs_tests.rs diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index 14ed1687..8e626671 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -1078,6 +1078,20 @@ impl FeatureKind { /// Every expression the feature uses. /// Points on faces the feature refers to (shell, draft, offset and replace faces, a hole's /// placement), for persistent face names. + /// The feature's profile selections, each with its sketch. + pub fn profile_sels_mut(&mut self) -> Vec<(u64, &mut ProfileSel)> { + match self { + FeatureKind::Extrude { sketch, profiles, .. } + | FeatureKind::Revolve { sketch, profiles, .. } + | FeatureKind::Sweep { sketch, profiles, .. } + | FeatureKind::Emboss { sketch, profiles, .. } + | FeatureKind::SheetBase { sketch, profiles, .. } => vec![(*sketch, profiles)], + FeatureKind::Patch { sketch: Some(sk), profiles, .. } => vec![(*sk, profiles)], + FeatureKind::Loft { sections, .. } => sections.iter_mut().filter(|s| s.point.is_none()).map(|s| (s.sketch, &mut s.profiles)).collect(), + _ => Vec::new(), + } + } + pub fn face_points_mut(&mut self) -> Vec<&mut Vec3> { match self { FeatureKind::Shell { faces, .. } diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index 6c44104c..ad6f67e5 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -209,6 +209,37 @@ pub(super) fn feature_sketch(s: &Session, p: &Value, cmd: &str) -> Result { .ok_or_else(|| bad(cmd, "there is no sketch to use (create one first)")) } +/// Profile indices become the curve ids of each profile's loops as the sketch is now solved. +fn stable_profiles(s: &Session, kind: &mut FeatureKind) -> Result<()> { + let st = s.model.state(); + 'sels: for (sketch, sel) in kind.profile_sels_mut() { + let ProfileSel::Indices { indices } = sel else { continue }; + let Some(ss) = st.sketch(sketch) else { continue }; + let mut loops = Vec::with_capacity(indices.len()); + for i in indices.iter() { + let pr = ss + .profiles + .get(*i) + .ok_or_else(|| EngineError::Other(format!("no profile {i} in sketch `{}` (it has {})", ss.name, ss.profiles.len())))?; + // Two profiles with the same outer curves (one inside the other's holes cannot be): + // keep the index then. + if pr.outer_curves.is_empty() || ss.profiles.iter().filter(|q| same_loop(&q.outer_curves, &pr.outer_curves)).count() != 1 { + continue 'sels; + } + loops.push(pr.outer_curves.clone()); + } + *sel = ProfileSel::Curves { loops }; + } + Ok(()) +} + +fn same_loop(a: &[String], b: &[String]) -> bool { + let (mut a, mut b) = (a.to_vec(), b.to_vec()); + a.sort(); + b.sort(); + a == b +} + pub(super) fn profiles(p: &Value, cmd: &str) -> Result { let Some(v) = p.get("profiles") else { return Ok(ProfileSel::All) }; match v { @@ -270,6 +301,9 @@ pub(super) fn add_feature(s: &mut Session, p: &Value, kind: FeatureKind) -> Resu *e = doc.with_design_unit(e, k); } } + // Profiles picked by index are kept by the curves around them: a recompute can renumber a + // sketch's profiles (a face sketch's projected outline rebuilt), never the curves. + stable_profiles(s, &mut kind)?; // Blends remember where their edges sat, to find them again after upstream edits. let (refs, names) = match &kind { FeatureKind::Fillet { edges, body, .. } | FeatureKind::Chamfer { edges, body, .. } => { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 39bdef5b..efe4e15c 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -433,6 +433,8 @@ mod delete_face_tests; #[cfg(test)] mod extent_tests; #[cfg(test)] +mod profile_refs_tests; +#[cfg(test)] mod split_face_tests; #[cfg(test)] mod tests; diff --git a/crates/engine/src/profile_refs_tests.rs b/crates/engine/src/profile_refs_tests.rs new file mode 100644 index 00000000..00d7e751 --- /dev/null +++ b/crates/engine/src/profile_refs_tests.rs @@ -0,0 +1,87 @@ +//! Profile picks survive recomputes (#30): picked by index, they are kept by the curves around +//! them, so a face sketch whose projected outline is rebuilt still extrudes the same region. + +use serde_json::{Value, json}; + +use crate::Session; + +fn run(s: &mut Session, id: &str, p: Value) -> Value { + match s.execute(id, &p) { + Ok(v) => v, + Err(e) => panic!("{id} {p}: {e}"), + } +} + +/// The demo enclosure (80 × 60 × 30, rounded, shelled) with a USB port cut through a side, the +/// port picked by index as the Extrude dialog does. +fn enclosure() -> Session { + let mut s = Session::default(); + let script = json!([ + {"command": "parameters.change", "params": {"name": "width", "expression": "80 mm", "comment": "outside, X"}}, + {"command": "parameters.change", "params": {"name": "depth", "expression": "60 mm", "comment": "outside, Y"}}, + {"command": "parameters.change", "params": {"name": "height", "expression": "30 mm"}}, + {"command": "parameters.change", "params": {"name": "wall", "expression": "2 mm"}}, + {"command": "sketch.create", "params": {"plane": "XY", "name": "Outline"}}, + {"command": "sketch.rectangle.center", "params": {"center": [0, 0], "corner": [40, 30]}}, + {"command": "sketch.constraint.coincident", "params": {"a": "p5", "b": "origin"}}, + {"command": "sketch.dimension", "params": {"entities": ["l1"], "value": "width"}}, + {"command": "sketch.dimension", "params": {"entities": ["l2"], "value": "depth"}}, + {"command": "sketch.finish", "params": {}}, + {"command": "solid.extrude", "params": {"distance": "height", "body_name": "Enclosure"}}, + {"command": "solid.fillet", "params": {"edges": [[40, 30, 15], [-40, 30, 15], [-40, -30, 15], [40, -30, 15]], "radius": 6}}, + {"command": "solid.shell", "params": {"faces": [[0, 0, 30]], "thickness": "wall"}}, + {"command": "sketch.create", "params": {"plane": {"face": [40, 10, 20]}, "name": "USB port"}}, + {"command": "sketch.rectangle.center", "params": {"center": [0, 12], "corner": [6, 15.5]}}, + {"command": "sketch.finish", "params": {}}, + {"command": "solid.extrude", "params": {"distance": "-wall", "operation": "cut", "profiles": [1]}} + ]); + s.run_script(&script).unwrap(); + s +} + +fn bodies(s: &Session) -> Vec<(String, f64)> { + s.model.state().bodies.iter().map(|b| (b.name.clone(), b.body.tessellate(1e-2).unwrap().measure().volume)).collect() +} + +#[test] +fn a_port_picked_by_index_stays_a_cut_after_a_width_change() { + let mut s = enclosure(); + let f = s.doc.features.last().unwrap(); + let solvecraft_doc::FeatureKind::Extrude { profiles, .. } = &f.kind else { panic!("{:?}", f.kind) }; + assert!(matches!(profiles, solvecraft_doc::ProfileSel::Curves { .. }), "stored by curves: {profiles:?}"); + let before = bodies(&s); + assert_eq!(before.len(), 1, "{before:?}"); + run(&mut s, "parameters.change", json!({"name": "width", "expression": "100 mm"})); + let after = bodies(&s); + assert_eq!(after.len(), 1, "the port is still a cut, not a new body: {after:?}"); + // The longer box less the same port: more wall and floor, the port still taken out. + assert!(after[0].1 > before[0].1 + 1000.0, "{before:?} {after:?}"); + let id = s.doc.features.last().unwrap().id; + assert!(s.model.result(id).is_some_and(|r| r.error.is_none()), "{:?}", s.model.result(id)); +} + +/// A pick the sketch no longer has is an error on the feature, never a different profile. +#[test] +fn a_lost_profile_is_an_error_not_another_pick() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "S"})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [10, 10], "ids": ["a", "b", "c", "d"]})); + run(&mut s, "sketch.circle.center", json!({"center": [30, 5], "radius": 3, "id": "ring"})); + run(&mut s, "sketch.finish", json!({})); + let ring = s.model.state().sketches[0].profiles.iter().position(|p| p.outer_curves == ["ring"]).unwrap(); + run(&mut s, "solid.extrude", json!({"sketch": "S", "profiles": [ring], "distance": 5})); + let n = s.model.state().bodies.len(); + assert_eq!(n, 1); + // Delete the picked circle: the extrude fails with a message instead of taking the square. + let sk = s.doc.find_feature("S").unwrap().id; + let picked = match &s.doc.features.last().unwrap().kind { + solvecraft_doc::FeatureKind::Extrude { profiles: solvecraft_doc::ProfileSel::Curves { loops }, .. } => loops[0].clone(), + k => panic!("{k:?}"), + }; + run(&mut s, "sketch.edit", json!({"sketch": sk})); + run(&mut s, "sketch.delete", json!({"entities": picked})); + run(&mut s, "sketch.finish", json!({})); + assert!(s.model.state().bodies.is_empty(), "no silent re-pick: {:?}", bodies(&s)); + let id = s.doc.features.last().unwrap().id; + assert!(s.model.result(id).is_some_and(|r| r.error.is_some()), "{:?}", s.model.result(id)); +}