From 848e7aec0e783de29182ca1893c66608ea8e7df5 Mon Sep 17 00:00:00 2001 From: CypherNaugh_0x <9931495+CypherNaught-0x@users.noreply.github.com> Date: Sun, 11 Oct 2026 10:17:25 +0200 Subject: [PATCH 1/2] feat: pick sketch profiles by their bounding curve ids so features keep the same region when parameters move the geometry, matching curves cut into pieces, refusing ambiguous or missing picks instead of taking another region, and storing point picks by curves --- crates/doc/src/document.rs | 6 +- crates/doc/src/eval.rs | 24 +-- crates/doc/src/lib.rs | 4 +- crates/engine/src/cmd/edit.rs | 28 ++++ crates/engine/src/cmd/features.rs | 73 +++++---- crates/engine/src/lib.rs | 2 + crates/engine/src/profile_curves_tests.rs | 176 ++++++++++++++++++++++ crates/sketch/src/lib.rs | 2 +- crates/sketch/src/profiles.rs | 63 ++++++++ crates/ui-egui/src/dialogs.rs | 15 +- docs/book/src/guide/sketching.md | 9 ++ 11 files changed, 342 insertions(+), 60 deletions(-) create mode 100644 crates/engine/src/profile_curves_tests.rs diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index 0fce5008..2748a6bb 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -76,8 +76,10 @@ pub enum ProfileSel { All, /// Profiles by index in the solved sketch's profile list. Indices { indices: Vec }, - /// Profiles whose outer loop is made of exactly these curves (order-insensitive); stable - /// across parameter changes. + /// Profiles whose outer loop is made of exactly these curves, or pieces of them where other + /// curves cut them (order and repeats don't matter; see `sketch::profile_by_curves`). Found + /// again on every evaluation, so stable across parameter changes; no match or several is + /// an error, never another profile. Curves { loops: Vec> }, /// The profile containing each sketch point. Points { points: Vec }, diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index 5dba61bb..5ff46d3a 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -724,14 +724,8 @@ fn select_profiles(ss: &SolvedSketch, sel: &ProfileSel) -> Result> ProfileSel::Curves { loops } => { let mut v = Vec::new(); for l in loops { - let mut want: Vec<&String> = l.iter().collect(); - want.sort(); - let found = ps.iter().find(|p| { - let mut have: Vec<&String> = p.outer_curves.iter().collect(); - have.sort(); - have == want - }); - v.push(found.ok_or_else(|| DocError::Unknown(format!("profile bounded by {l:?} in {}", ss.name)))?.region.clone()); + let i = solvecraft_sketch::profile_by_curves(ps, l).map_err(|e| curve_pick_error(e, l, &ss.name))?; + v.push(ps.get(i).ok_or_else(|| DocError::Unknown(format!("profile {i} of {}", ss.name)))?.region.clone()); } v } @@ -739,7 +733,7 @@ fn select_profiles(ss: &SolvedSketch, sel: &ProfileSel) -> Result> let mut v = Vec::new(); for p in points { // Innermost profile containing the point. - let found = ps.iter().filter(|q| q.region.contains(*p)).min_by(|a, b| a.area.total_cmp(&b.area)); + let found = solvecraft_sketch::profile_at_point(ps, *p).and_then(|i| ps.get(i)); v.push(found.ok_or_else(|| DocError::Unknown(format!("profile at {:?} in {}", [p.x, p.y], ss.name)))?.region.clone()); } v @@ -751,6 +745,18 @@ fn select_profiles(ss: &SolvedSketch, sel: &ProfileSel) -> Result> Ok(out) } +/// The error for a profile picked by curves that no longer names exactly one profile. +pub fn curve_pick_error(e: solvecraft_sketch::CurvePick, ids: &[String], sketch: &str) -> DocError { + match e { + solvecraft_sketch::CurvePick::Missing => DocError::Invalid(format!( + "no profile of {sketch} is bounded by exactly the curves {ids:?} (a curve was deleted, or another curve now cuts the region)" + )), + solvecraft_sketch::CurvePick::Ambiguous(n) => DocError::Invalid(format!( + "{n} profiles of {sketch} are bounded by exactly the curves {ids:?}; add a curve that tells them apart, or pick by index or point" + )), + } +} + fn new_name(state: &mut ModelState, f: &Feature, k: usize) -> String { if let Some(n) = f.body_names.get(k) && !n.is_empty() diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index f4b6cfc9..528be383 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -28,8 +28,8 @@ pub mod sheet; pub use assembly::{IDENTITY, Mat, Occurrence, apply_point, apply_vector, is_identity, map_point_expr, mat_inverse, mat_mul, rigid}; pub use document::*; pub use eval::{ - EdgeRef, FeatureResult, Model, ModelBody, ModelState, SolvedSketch, ThreadInfo, edge_names_for, edge_refs, parse_metric_thread, pattern_matrices, - state_in_frame, world_state, + EdgeRef, FeatureResult, Model, ModelBody, ModelState, SolvedSketch, ThreadInfo, curve_pick_error, edge_names_for, edge_refs, parse_metric_thread, + pattern_matrices, state_in_frame, world_state, }; pub use params::{ParamRow, ParamUser, cycles}; pub use solvecraft_kernel as kernel; diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index cdfcd4cc..f80d49e8 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -188,12 +188,36 @@ fn merge(dst: &mut Value, src: &Value, depth: usize) { } } +/// `profiles` (also a loft section's) given as the feature commands take it (`"all"`, +/// `[index]`, `[{curves}]`, `[{point}]`...) becomes the stored form; the stored form passes. +fn command_profiles(set: &Value, cmd: &str) -> Result { + let conv = |v: &mut Value| -> Result<()> { + if let Some(pv) = v.get("profiles").filter(|x| x.get("type").is_none()) { + let sel = super::features::profiles(&json!({"profiles": pv}), cmd)?; + let sv = serde_json::to_value(sel).map_err(|e| EngineError::Other(e.to_string()))?; + if let Some(o) = v.as_object_mut() { + o.insert("profiles".into(), sv); + } + } + Ok(()) + }; + let mut set = set.clone(); + conv(&mut set)?; + if let Some(secs) = set.get_mut("sections").and_then(Value::as_array_mut) { + for sec in secs.iter_mut().take(10_000) { + conv(sec)?; + } + } + Ok(set) +} + fn edit_feature(s: &mut Session, p: &Value) -> Result { let cmd = "timeline.edit"; let id = feature_id(s, p.get("feature"), cmd)?; let set = p.get("set").filter(|v| v.is_object()).ok_or_else(|| bad(cmd, "`set` must be an object of fields"))?; let f = s.doc.feature(id).cloned().ok_or_else(|| bad(cmd, "feature"))?; let mut v = serde_json::to_value(&f).map_err(|e| EngineError::Other(e.to_string()))?; + let set = &command_profiles(set, cmd)?; merge(&mut v, set, 0); let mut nf: Feature = serde_json::from_value(v).map_err(|e| bad(cmd, format!("invalid feature: {e}")))?; nf.id = f.id; @@ -216,6 +240,10 @@ fn edit_feature(s: &mut Session, p: &Value) -> Result { nf.edge_refs = solvecraft_doc::edge_refs(&before, body, a); nf.edge_names = solvecraft_doc::edge_names_for(&before, body, a); } + // Profiles picked by index or point are kept by their curves, as when the feature was made. + if nf.kind.clone().profile_sels_mut() != f.kind.clone().profile_sels_mut() { + super::features::stable_profiles(s, &mut nf.kind)?; + } // New face points: their faces' names now. if nf.kind.face_points() != f.kind.face_points() { nf.face_names = solvecraft_doc::naming::face_names_at(&s.model.state_before(id), &nf.kind.face_points()); diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index 13641d0f..40910f00 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -13,7 +13,7 @@ pub static COMMANDS: &[CommandSpec] = &[ .at("SOLID", "CREATE") .icon("extrude") .key("E") - .params("distance: expr (or through_all: true); taper?: angle expr; sketch?: id|name (default: active or last sketch); profiles?: all | [index] | [[curve ids]] | [{point:[x,y]}]; face?: [x,y,z] (extrude a planar body face instead of a sketch profile); direction?: positive|negative|symmetric; distance2?; start_offset?: expr (along the sketch normal, also with direction: negative); to?: [x,y,z] (To Object: a face, vertex or point; to_offset?: expr past it); from?: [x,y,z] (From Object: start at that face or point); operation?: new|join|cut|intersect|auto (cut into a body, otherwise join existing solids; new in an empty component); targets?: [body]; name?; body_name?"), + .params("distance: expr (or through_all: true); taper?: angle expr; sketch?: id|name (default: active or last sketch); profiles?: all | [index] | [{curves: [ids]}] (the profile whose outer loop is made of exactly these curves, or pieces of them; found again on every recompute, so it follows parameter changes; islands inside are its holes; an error when no profile or several match) | [{point:[x,y]}] (index and point picks are stored by their curves when that finds the same profile); face?: [x,y,z] (extrude a planar body face instead of a sketch profile); direction?: positive|negative|symmetric; distance2?; start_offset?: expr (along the sketch normal, also with direction: negative); to?: [x,y,z] (To Object: a face, vertex or point; to_offset?: expr past it); from?: [x,y,z] (From Object: start at that face or point); operation?: new|join|cut|intersect|auto (cut into a body, otherwise join existing solids; new in an empty component); targets?: [body]; name?; body_name?"), CommandSpec::new("solid.revolve", "Revolve", revolve) .at("SOLID", "CREATE") .icon("revolve") @@ -124,18 +124,7 @@ fn extrude_base(s: &Session, p: &Value) -> Option<(Vec3, Vec3)> { ProfileSel::All => ps.first()?.region.interior_point(), ProfileSel::Indices { indices } => ps.get(*indices.first()?)?.region.interior_point(), ProfileSel::Points { points } => *points.first()?, - ProfileSel::Curves { loops } => { - let mut want: Vec<&String> = loops.first()?.iter().collect(); - want.sort(); - ps.iter() - .find(|x| { - let mut have: Vec<&String> = x.outer_curves.iter().collect(); - have.sort(); - have == want - })? - .region - .interior_point() - } + ProfileSel::Curves { loops } => ps.get(solvecraft_doc::sketch::profile_by_curves(ps, loops.first()?).ok()?)?.region.interior_point(), }; Some((ss.plane.to_world(q), ss.plane.normal())) } @@ -212,37 +201,55 @@ 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<()> { +/// Profiles picked by index or point are stored by the curve ids of each profile's outer loop +/// as the sketch is now solved, so they follow the geometry when parameters move it. A pick +/// whose curves would not find that same profile again (two profiles bounded by the same +/// curves) stays as given. Curve picks are checked: one that names no profile, or several, is +/// refused now rather than failing later. +pub(super) fn stable_profiles(s: &Session, kind: &mut FeatureKind) -> Result<()> { + use solvecraft_doc::sketch::{distinct_curves, profile_at_point, profile_by_curves}; 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 { + let picked: Vec = match sel { + ProfileSel::Indices { indices } => indices + .iter() + .map(|i| { + if *i < ss.profiles.len() { + Ok(*i) + } else { + Err(EngineError::Other(format!("no profile {i} in sketch `{}` (it has {})", ss.name, ss.profiles.len()))) + } + }) + .collect::>()?, + ProfileSel::Points { points } => { + let Some(v) = points.iter().map(|q| profile_at_point(&ss.profiles, *q)).collect::>>() else { continue }; + v + } + ProfileSel::Curves { loops } => { + for l in loops.iter() { + profile_by_curves(&ss.profiles, l) + .map_err(|e| EngineError::Other(solvecraft_doc::curve_pick_error(e, l, &ss.name).to_string()))?; + } + continue; + } + ProfileSel::All => continue, + }; + let mut loops = Vec::with_capacity(picked.len()); + for i in picked { + let Some(pr) = ss.profiles.get(i) else { continue 'sels }; + let ids = distinct_curves(&pr.outer_curves); + // Keep the pick as given when its curves don't lead back to this profile. + if ids.is_empty() || profile_by_curves(&ss.profiles, &ids) != Ok(i) { continue 'sels; } - loops.push(pr.outer_curves.clone()); + loops.push(ids); } *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 { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index a6907769..757edd2a 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -550,6 +550,8 @@ mod extent_tests; #[cfg(test)] mod pattern_suppress_tests; #[cfg(test)] +mod profile_curves_tests; +#[cfg(test)] mod profile_refs_tests; #[cfg(test)] mod split_face_tests; diff --git a/crates/engine/src/profile_curves_tests.rs b/crates/engine/src/profile_curves_tests.rs new file mode 100644 index 00000000..23453ee0 --- /dev/null +++ b/crates/engine/src/profile_curves_tests.rs @@ -0,0 +1,176 @@ +//! Profiles picked by the curves around them stay the same region when a parameter moves the +//! sketch, where a pick by point can land in another region. + +use serde_json::{Value, json}; + +use crate::Session; +use solvecraft_doc::{FeatureKind, ProfileSel}; + +fn run(s: &mut Session, id: &str, p: Value) -> Value { + match s.execute(id, &p) { + Ok(v) => v, + Err(e) => panic!("{id} {p}: {e}"), + } +} + +fn volumes(s: &Session) -> Vec { + s.model.state().bodies.iter().map(|b| b.body.tessellate(1e-2).unwrap().measure().volume).collect() +} + +fn last_error(s: &Session) -> Option { + let id = s.doc.features.last().unwrap().id; + s.model.result(id).and_then(|r| r.error.clone()) +} + +/// A 60 × 20 tray split in two by a divider `split` mm from its left side: the left bay is +/// bounded by the left side, the divider and the pieces of the bottom and top the divider cuts. +fn tray() -> Session { + let mut s = Session::default(); + run(&mut s, "parameters.change", json!({"name": "split", "expression": "30 mm"})); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Tray"})); + run(&mut s, "sketch.line", json!({"points": [[0, 0], [60, 0], [60, 20], [0, 20]], "closed": true, "ids": ["bottom", "right", "top", "left"]})); + run(&mut s, "sketch.constraint.fix", json!({"entities": ["bottom", "right", "top", "left"]})); + run(&mut s, "sketch.line", json!({"points": [[30, 0], [30, 20]], "ids": ["div"]})); + run(&mut s, "sketch.constraint.coincident", json!({"a": "div.start", "b": "bottom"})); + run(&mut s, "sketch.constraint.coincident", json!({"a": "div.end", "b": "top"})); + run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": "div"})); + run(&mut s, "sketch.dimension", json!({"entities": ["left", "div"], "value": "split"})); + run(&mut s, "sketch.finish", json!({})); + s +} + +fn extrude_kind(s: &Session) -> ProfileSel { + match &s.doc.features.last().unwrap().kind { + FeatureKind::Extrude { profiles, .. } => profiles.clone(), + k => panic!("{k:?}"), + } +} + +/// The user's case: the left bay picked by its curves keeps extruding the left bay when +/// `split` moves the divider past the old pick point; a pick by that point (as a design saved +/// before picks were stored by curves holds it) silently extrudes the right bay instead. +#[test] +fn a_profile_picked_by_curves_follows_a_parameter_change() { + let mut s = tray(); + run(&mut s, "solid.extrude", json!({"sketch": "Tray", "profiles": [{"curves": ["left", "bottom", "div", "top"]}], "distance": 10})); + let v = volumes(&s); + assert_eq!(v.len(), 1); + assert!((v[0] - 30.0 * 20.0 * 10.0).abs() < 1.0, "{v:?}"); + run(&mut s, "parameters.change", json!({"name": "split", "expression": "20 mm"})); + assert_eq!(last_error(&s), None); + let v = volumes(&s); + assert!((v[0] - 20.0 * 20.0 * 10.0).abs() < 1.0, "still the left bay, now 20 wide: {v:?}"); + + // The same bay picked by a point (25, 10), stored as a point. + let mut s = tray(); + run(&mut s, "solid.extrude", json!({"sketch": "Tray", "profiles": [{"point": [25, 10]}], "distance": 10})); + let id = s.doc.features.last().unwrap().id; + if let Some(f) = s.doc_mut().feature_mut(id) + && let FeatureKind::Extrude { profiles, .. } = &mut f.kind + { + *profiles = ProfileSel::Points { points: vec![solvecraft_geom::Vec2::new(25.0, 10.0)] }; + } + s.refresh(); + assert!((volumes(&s)[0] - 6000.0).abs() < 1.0); + run(&mut s, "parameters.change", json!({"name": "split", "expression": "20 mm"})); + let v = volumes(&s); + assert!((v[0] - 40.0 * 20.0 * 10.0).abs() < 1.0, "the point now sits in the right bay: {v:?}"); +} + +/// A pick by point is stored by the curves around the picked profile, so it follows too. +#[test] +fn a_point_pick_is_stored_by_curves() { + let mut s = tray(); + run(&mut s, "solid.extrude", json!({"sketch": "Tray", "profiles": [{"point": [25, 10]}], "distance": 10})); + let ProfileSel::Curves { loops } = extrude_kind(&s) else { panic!("{:?}", extrude_kind(&s)) }; + let mut ids = loops[0].clone(); + ids.sort(); + assert_eq!(ids, ["bottom", "div", "left", "top"]); + run(&mut s, "parameters.change", json!({"name": "split", "expression": "20 mm"})); + assert_eq!(last_error(&s), None); + assert!((volumes(&s)[0] - 4000.0).abs() < 1.0, "{:?}", volumes(&s)); +} + +/// Curves cut into several pieces of one loop still name it: a line ending on the square's +/// bottom from outside cuts the bottom in two, both pieces on the square's loop. +#[test] +fn a_curve_cut_into_pieces_still_names_its_profile() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "S"})); + run(&mut s, "sketch.line", json!({"points": [[0, 0], [10, 0], [10, 10], [0, 10]], "closed": true, "ids": ["a", "b", "c", "d"]})); + run(&mut s, "sketch.line", json!({"points": [[5, -5], [5, 0]], "ids": ["stub"]})); + run(&mut s, "sketch.finish", json!({})); + let ps = &s.model.state().sketches[0].profiles; + assert_eq!(ps.len(), 1); + assert_eq!(ps[0].outer_curves.iter().filter(|c| *c == "a").count(), 2, "the bottom is in two pieces: {:?}", ps[0].outer_curves); + run(&mut s, "solid.extrude", json!({"sketch": "S", "profiles": [{"curves": ["a", "b", "c", "d"]}], "distance": 2})); + assert_eq!(last_error(&s), None); + assert!((volumes(&s)[0] - 200.0).abs() < 1e-3, "{:?}", volumes(&s)); +} + +/// Both halves of a circle cut by a line are bounded by the same two curves: an error, not one +/// of them. A list no profile matches is an error too. +#[test] +fn ambiguous_and_missing_curve_picks_are_errors() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "S"})); + run(&mut s, "sketch.circle.center", json!({"center": [0, 0], "radius": 10, "id": "ring"})); + run(&mut s, "sketch.line", json!({"points": [[-20, 0], [20, 0]], "ids": ["cut"]})); + run(&mut s, "sketch.circle.center", json!({"center": [40, 0], "radius": 3, "id": "dot"})); + run(&mut s, "sketch.finish", json!({})); + let e = s.execute("solid.extrude", &json!({"sketch": "S", "profiles": [{"curves": ["ring", "cut"]}], "distance": 2})).unwrap_err().to_string(); + assert!(e.contains("2 profiles"), "{e}"); + assert!(s.model.state().bodies.is_empty()); + let e = s.execute("solid.extrude", &json!({"sketch": "S", "profiles": [{"curves": ["ring"]}], "distance": 2})).unwrap_err().to_string(); + assert!(e.contains("no profile"), "{e}"); + let e = s.execute("solid.extrude", &json!({"sketch": "S", "profiles": [{"curves": ["dot", "cut"]}], "distance": 2})).unwrap_err().to_string(); + assert!(e.contains("no profile"), "{e}"); + // A closed curve alone. + run(&mut s, "solid.extrude", json!({"sketch": "S", "profiles": [{"curves": ["dot"]}], "distance": 2})); + assert!((volumes(&s)[0] - std::f64::consts::PI * 9.0 * 2.0).abs() < 0.5, "{:?}", volumes(&s)); +} + +/// An island inside the picked outline is its hole, as with a pick by point; the island is a +/// profile of its own, picked by its curve. +#[test] +fn a_curve_pick_keeps_islands_as_holes() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "S"})); + run(&mut s, "sketch.line", json!({"points": [[0, 0], [20, 0], [20, 20], [0, 20]], "closed": true, "ids": ["a", "b", "c", "d"]})); + run(&mut s, "sketch.circle.center", json!({"center": [10, 10], "radius": 5, "id": "hole"})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"sketch": "S", "profiles": [{"curves": ["a", "b", "c", "d"]}], "distance": 1})); + let disc = std::f64::consts::PI * 25.0; + assert!((volumes(&s)[0] - (400.0 - disc)).abs() < 0.5, "{:?}", volumes(&s)); + run(&mut s, "solid.extrude", json!({"sketch": "S", "profiles": [{"curves": ["hole"]}], "distance": 1, "operation": "new"})); + assert!((volumes(&s)[1] - disc).abs() < 0.5, "{:?}", volumes(&s)); +} + +/// Curve picks are saved in the design file and read back; timeline.edit takes the command's +/// `profiles` form. +#[test] +fn curve_picks_save_load_and_edit() { + let mut s = tray(); + run( + &mut s, + "solid.extrude", + json!({"sketch": "Tray", "profiles": [{"curves": ["left", "bottom", "div", "top"]}], "distance": 10, "name": "Bay"}), + ); + let text = serde_json::to_string(&*s.doc).unwrap(); + let doc = solvecraft_doc::format::read(&text).unwrap(); + let mut t = Session::new(doc); + assert_eq!(extrude_kind(&t), extrude_kind(&s)); + run(&mut t, "parameters.change", json!({"name": "split", "expression": "20 mm"})); + assert!((volumes(&t)[0] - 4000.0).abs() < 1.0, "{:?}", volumes(&t)); + // Switch to the right bay through timeline.edit. + run(&mut t, "timeline.edit", json!({"feature": "Bay", "set": {"profiles": [{"curves": ["div", "bottom", "right", "top"]}]}})); + assert!((volumes(&t)[0] - 8000.0).abs() < 1.0, "{:?}", volumes(&t)); + run(&mut t, "timeline.edit", json!({"feature": "Bay", "set": {"profiles": [{"point": [5, 5]}]}})); + assert!(matches!(extrude_kind(&t), ProfileSel::Curves { .. }), "{:?}", extrude_kind(&t)); + assert!((volumes(&t)[0] - 4000.0).abs() < 1.0, "{:?}", volumes(&t)); + // An old design's point pick still loads and evaluates. + let old = text.replace(r#"{"type":"curves","loops":[["left","bottom","div","top"]]}"#, r#"{"type":"points","points":[[25.0,10.0]]}"#); + assert_ne!(old, text); + let t = Session::new(solvecraft_doc::format::read(&old).unwrap()); + assert!((volumes(&t)[0] - 6000.0).abs() < 1.0, "{:?}", volumes(&t)); +} diff --git a/crates/sketch/src/lib.rs b/crates/sketch/src/lib.rs index 3079d6db..13fb34b7 100644 --- a/crates/sketch/src/lib.rs +++ b/crates/sketch/src/lib.rs @@ -27,7 +27,7 @@ pub use dims::{DimFrame, DimLayout, chain_centres, default_text, dim_frame, dim_ pub use link::{Link, LinkGeom, LinkKind, LinkSource, MAX_LINK_ENTITIES}; pub use model::{Constraint, ConstraintKind, Curve, CurveKind, SPoint, Sketch, SketchError, SketchView}; pub use offset::OffsetChain; -pub use profiles::{Profile, find_drawn_profiles, find_profiles, merge_regions}; +pub use profiles::{CurvePick, Profile, distinct_curves, find_drawn_profiles, find_profiles, merge_regions, profile_at_point, profile_by_curves}; pub use shape::{Shape, intersections}; pub use solver::{Hold, SolveReport, SolveStatus, measure_dimension, solve, solve_holding}; pub use text::{MAX_TEXT_CHARS, text_geometry}; diff --git a/crates/sketch/src/profiles.rs b/crates/sketch/src/profiles.rs index a2ce3596..67d97888 100644 --- a/crates/sketch/src/profiles.rs +++ b/crates/sketch/src/profiles.rs @@ -829,6 +829,69 @@ pub fn find_profiles(sk: &Sketch) -> Vec { out } +/// Why no single profile is bounded by a list of curves. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CurvePick { + /// No profile's outer loop is made of exactly those curves. + Missing, + /// This many profiles are (e.g. both halves of a circle cut by a line). + Ambiguous(usize), +} + +/// The profile picked by the curves around it, by index into `profiles`. A profile matches when +/// its outer loop is made of the listed curves and no others: every listed curve, or a piece of +/// it where other curves cut it, lies on the loop, and every piece of the loop belongs to a +/// listed curve. Order and repeats don't matter. Holes don't take part (islands inside a profile +/// are its holes, as for a profile picked by a point). When several profiles match, one whose +/// loop has exactly the listed pieces (a curve listed twice for two pieces) wins; otherwise the +/// pick is ambiguous, never a guess. +pub fn profile_by_curves(profiles: &[Profile], ids: &[String]) -> Result { + use std::collections::BTreeSet; + let want: BTreeSet<&str> = ids.iter().map(String::as_str).collect(); + if want.is_empty() { + return Err(CurvePick::Missing); + } + let hits: Vec = profiles + .iter() + .enumerate() + .filter(|(_, p)| p.outer_curves.iter().map(String::as_str).collect::>() == want) + .map(|(i, _)| i) + .collect(); + match hits.as_slice() { + [] => Err(CurvePick::Missing), + [i] => Ok(*i), + _ => { + fn sorted(v: &[String]) -> Vec<&str> { + let mut v: Vec<&str> = v.iter().map(String::as_str).collect(); + v.sort_unstable(); + v + } + let exact = sorted(ids); + let same: Vec = hits.iter().copied().filter(|i| profiles.get(*i).is_some_and(|p| sorted(&p.outer_curves) == exact)).collect(); + match same.as_slice() { + [i] => Ok(*i), + _ => Err(CurvePick::Ambiguous(hits.len())), + } + } + } +} + +/// The innermost profile containing a sketch point, by index into `profiles`. +pub fn profile_at_point(profiles: &[Profile], p: Vec2) -> Option { + profiles.iter().enumerate().filter(|(_, q)| q.region.contains(p)).min_by(|a, b| a.1.area.total_cmp(&b.1.area)).map(|(i, _)| i) +} + +/// The distinct curve ids of a loop, in loop order (a curve cut into pieces is listed once). +pub fn distinct_curves(ids: &[String]) -> Vec { + let mut out: Vec = Vec::with_capacity(ids.len()); + for id in ids { + if !out.contains(id) { + out.push(id.clone()); + } + } + out +} + fn same_seg(a: &Seg2, b: &Seg2, tol: f64) -> bool { match (*a, *b) { (Seg2::Line { a: p, b: q }, Seg2::Line { a: r, b: s }) => p.dist(r) <= tol && q.dist(s) <= tol, diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 5038b79e..6a6b6ce1 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2682,19 +2682,8 @@ pub(crate) fn profile_indices(ss: &solvecraft_engine::doc::SolvedSketch, sel: &P match sel { ProfileSel::All => (0..ps.len()).collect(), ProfileSel::Indices { indices } => indices.iter().copied().filter(|i| *i < ps.len()).collect(), - ProfileSel::Curves { loops } => loops - .iter() - .filter_map(|l| { - let mut want: Vec<&String> = l.iter().collect(); - want.sort(); - ps.iter().position(|p| { - let mut have: Vec<&String> = p.outer_curves.iter().collect(); - have.sort(); - have == want - }) - }) - .collect(), - ProfileSel::Points { points } => points.iter().filter_map(|q| ps.iter().position(|p| p.region.contains(*q))).collect(), + ProfileSel::Curves { loops } => loops.iter().filter_map(|l| solvecraft_engine::doc::sketch::profile_by_curves(ps, l).ok()).collect(), + ProfileSel::Points { points } => points.iter().filter_map(|q| solvecraft_engine::doc::sketch::profile_at_point(ps, *q)).collect(), } } diff --git a/docs/book/src/guide/sketching.md b/docs/book/src/guide/sketching.md index d10bf732..3a019c52 100644 --- a/docs/book/src/guide/sketching.md +++ b/docs/book/src/guide/sketching.md @@ -32,3 +32,12 @@ constraint while you drag. Closed loops become profiles automatically, including regions split by crossing lines and islands inside outlines. Extrude, revolve, sweep and loft pick profiles. + +A feature remembers a picked profile by the curves around it, not by where it was clicked: +`"profiles": [{"curves": ["l1", "l2", "a1", "l3"]}]` (or `[{"curves": ["c1"]}]` for a circle) +picks the region whose outer loop is made of exactly those curves, or the pieces of them that +other curves cut off. It is found again on every recompute, so the feature keeps the same region +when a parameter moves the geometry. Islands inside the region are its holes. When no region, or +more than one (both halves of a circle cut by a line), is bounded by exactly those curves, the +feature reports an error rather than taking another region. Profiles picked by index or by point +(`[{"point": [x, y]}]`) are stored the same way when the feature is made. From 897025467225de664a0b1bd1b209d70308eead12 Mon Sep 17 00:00:00 2001 From: CypherNaugh_0x <9931495+CypherNaught-0x@users.noreply.github.com> Date: Sun, 11 Oct 2026 13:01:54 +0200 Subject: [PATCH 2/2] =?UTF-8?q?A=20drain=20pad=20whose=20bore=20is=20the?= =?UTF-8?q?=20drain=20hole's=20wall=20(a=20ring=20joined=20flush=20under?= =?UTF-8?q?=20a=20floor,=20or=20overlapping=20it)=20now=20joins=20instead?= =?UTF-8?q?=20of=20failing=20with=20"coincident=20faces=20on=20curved=20bo?= =?UTF-8?q?dies":=20the=20mesh=20intersection=20can't=20take=20two=20walls?= =?UTF-8?q?=20on=20one=20cylinder,=20and=20the=20coincident-face=20push=20?= =?UTF-8?q?only=20knew=20planes.=20Such=20unions=20now=20narrow=20one=20bo?= =?UTF-8?q?dy's=20bore=20(its=20walls=20pushed=20toward=20the=20axis,=20cl?= =?UTF-8?q?ear=20of=20the=20other's),=20join,=20and=20cut=20the=20original?= =?UTF-8?q?=20bore's=20cylinder=20out=20again=20over=20that=20body's=20len?= =?UTF-8?q?gth,=20which=20is=20exact=20when=20the=20cylinder=20holds=20no?= =?UTF-8?q?=20material=20of=20either=20body=20(checked=20on=20samples).=20?= =?UTF-8?q?Inside=20that=20step=20a=20face=20is=20pushed=20into=20the=20ot?= =?UTF-8?q?her=20body=20only=20as=20deep=20as=20there=20is=20material=20be?= =?UTF-8?q?hind=20it=20all=20over,=20so=20a=20narrowed=20rim=20over=20the?= =?UTF-8?q?=20other's=20bore=20can't=20fill=20it;=20then=20the=20other=20b?= =?UTF-8?q?ody's=20bore=20is=20narrowed=20instead.=20Test:=20drain=5Fpads?= =?UTF-8?q?=5Fflush=5Funder=5Fa=5Ffloor=5Fwith=5Fa=5Fhole=20(ring=20flush?= =?UTF-8?q?=20and=202=20mm=20into=20a=201=C2=B0=20floor,=20and=20a=20disc?= =?UTF-8?q?=20across=20the=20hole,=20against=20hand-calculated=20volumes)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/kernel/src/coplanar.rs | 169 +++++++++++++++++++++++++++++++++- crates/kernel/src/ops.rs | 16 ++++ crates/kernel/src/tests.rs | 46 +++++++++ 3 files changed, 230 insertions(+), 1 deletion(-) diff --git a/crates/kernel/src/coplanar.rs b/crates/kernel/src/coplanar.rs index f986de45..a0eaa96c 100644 --- a/crates/kernel/src/coplanar.rs +++ b/crates/kernel/src/coplanar.rs @@ -268,7 +268,22 @@ pub(crate) fn boolean_apart(a: &Body, b: &Body, op: BoolOp) -> Option Option bool { + let size = a.size().max(b.size()); + let tol = (size * 1e-7).max(1e-9) * 10.0; + let faces: Vec = planes(b).into_iter().filter(|(_, p)| p.n.dot(m) > 1.0 - 1e-9 && (p.d - d).abs() < tol).map(|(i, _)| i).collect(); + let (Ok(ma), Ok(mb)) = (a.tessellate_with(size * 1e-3, true), b.tessellate_with(size * 1e-3, true)) else { return false }; + let inside = ma.inside_index(); + let mut points = Vec::new(); + for (t, fi) in mb.triangles.iter().zip(&mb.tri_face) { + if !faces.contains(&(*fi as usize)) { + continue; + } + let Some([p, q, r]) = mb.tri(t) else { continue }; + let c = (p + q + r) / 3.0; + points.extend([c, c.lerp(p, 0.8), c.lerp(q, 0.8), c.lerp(r, 0.8)]); + if points.len() > 4000 { + break; + } + } + !points.is_empty() && points.iter().all(|p| [0.05, 0.5, 1.0].iter().all(|k| inside.contains(*p + m * (depth * k)))) +} + /// Union of bodies that touch along one identical planar face, back to back (a body joined /// with its mirror image): the two faces go and the shells are stitched along their edges. /// `None` when no such pair of faces exists. @@ -387,3 +425,132 @@ pub(crate) fn glue(a: &Body, b: &Body) -> Option> { } None } + +/// A cylindrical face: its index, a point on the axis, the unit axis and the radius. +#[derive(Clone, Copy, Debug)] +struct CylFace { + index: usize, + origin: Vec3, + axis: Vec3, + radius: f64, +} + +fn cylinders(b: &Body) -> Vec { + (0..b.face_count()) + .filter_map(|index| match crate::face_surface(b, index)? { + crate::FaceSurface::Cylinder { origin, axis, radius } => Some(CylFace { index, origin, axis, radius }), + _ => None, + }) + .collect() +} + +/// Do the two cylinders lie on the same surface? +fn same_cylinder(x: &CylFace, y: &CylFace, tol: f64) -> bool { + let off = y.origin - x.origin; + x.axis.dot(y.axis).abs() > 1.0 - 1e-9 && (x.radius - y.radius).abs() < tol && (off - x.axis * off.dot(x.axis)).len() < tol +} + +/// Do the bodies have cylindrical faces on a common cylinder? +pub(crate) fn has_coincident_cylinders(a: &Body, b: &Body) -> bool { + let tol = (a.size().max(b.size()) * 1e-7).max(1e-9) * 10.0; + let cb = cylinders(b); + cylinders(a).iter().any(|x| cb.iter().any(|y| same_cylinder(x, y, tol))) +} + +/// Union of bodies with bores on a common cylinder (a drain pad's bore around the drain hole +/// it is joined over). The mesh intersection can't take the coincident walls, so the tool's +/// bore is narrowed (its walls pushed toward the axis, now clear of the target's), the bodies +/// are joined, and the cylinder of the original bore is cut out again over the tool's length. +/// That is exact when the cylinder holds no material of either body, which is checked on +/// sample points. Each way (the tool's bore narrowed, else the target's) gives a result only +/// when `accept` takes it. `None` when there are no coincident bores or no way works. +pub(crate) fn bore_union(a: &Body, b: &Body, accept: &dyn Fn(&Body) -> bool) -> Option { + let size = a.size().max(b.size()); + let tol = (size * 1e-7).max(1e-9) * 10.0; + let mtol = size * 2e-3; + let (ma, mb) = (a.tessellate_with(mtol, true).ok()?, b.tessellate_with(mtol, true).ok()?); + let (ia, ib) = (ma.inside_index(), mb.inside_index()); + let empty = |p: Vec3| !ia.contains(p) && !ib.contains(p); + let (ca, cb) = (cylinders(a), cylinders(b)); + // (body to narrow, its mesh and inside test, the other body, their faces on the cylinder). + let ways = [(b, &mb, &ib, a, &cb, &ca), (a, &ma, &ia, b, &ca, &cb)]; + for (tool, mesh, inside, target, ct, co) in ways { + for c in ct { + if !co.iter().any(|o| same_cylinder(c, o, tol)) { + continue; + } + let bore: Vec = ct.iter().filter(|x| same_cylinder(c, x, tol)).map(|x| x.index).collect(); + // The tool's extent along the axis on that cylinder, and whether it is a bore (the + // material outside it) at a point of the face. + let (mut t0, mut t1) = (f64::INFINITY, f64::NEG_INFINITY); + let mut probe = None; + for (t, fi) in mesh.triangles.iter().zip(&mesh.tri_face) { + if !bore.contains(&(*fi as usize)) { + continue; + } + let Some(ps) = mesh.tri(t) else { continue }; + for p in ps { + let s = (p - c.origin).dot(c.axis); + t0 = t0.min(s); + t1 = t1.max(s); + } + if probe.is_none() { + probe = Some((ps[0] + ps[1] + ps[2]) / 3.0); + } + } + let Some(q) = probe else { continue }; + if !(t1 - t0 > tol && c.radius > tol) { + continue; + } + let along = c.origin + c.axis * (q - c.origin).dot(c.axis); + let Some(radial) = (q - along).normalized() else { continue }; + let on = along + radial * c.radius; + let e = (c.radius * 0.05).max(size * 1e-4); + if !(inside.contains(on + radial * e) && !inside.contains(on - radial * e)) { + continue; + } + // The cylinder over the tool's length holds no material of either body. + let (u, w) = (c.axis.any_perp(), c.axis.cross(c.axis.any_perp())); + let at = |s: f64, r: f64, ang: f64| c.origin + c.axis * s + (u * ang.cos() + w * ang.sin()) * r; + let clear = (1..=9).all(|i| { + let s = t0 + (t1 - t0) * f64::from(i) / 10.0; + empty(at(s, 0.0, 0.0)) + && (0..8).all(|k| { + let ang = f64::from(k) * std::f64::consts::FRAC_PI_4 + 0.3; + empty(at(s, c.radius * 0.5, ang)) && empty(at(s, c.radius * 0.9, ang)) + }) + }); + if !clear { + continue; + } + // Past an end where the space is empty well beyond the radius too, the cut runs on a + // little (so its cap doesn't lie in a face's plane). Not where the other body's bore + // goes on: the cut's side would lie on its wall. + let ext = size * 0.01; + let past = |s: f64| { + empty(at(s, 0.0, 0.0)) + && (0..8).all(|k| { + let ang = f64::from(k) * std::f64::consts::FRAC_PI_4 + 0.3; + empty(at(s, c.radius * 0.9, ang)) && empty(at(s, c.radius * 1.1, ang)) && empty(at(s, c.radius * 1.5, ang)) + }) + }; + let lo = if past(t0 - ext * 0.5) && past(t0 - ext) { t0 - ext } else { t0 }; + let hi = if past(t1 + ext * 0.5) && past(t1 + ext) { t1 + ext } else { t1 }; + let d = (c.radius * 0.2).min(size * 0.01); + let run = || -> Result> { + let narrowed = crate::offset::offset_body(tool, |i, _| if bore.contains(&i) { d } else { 0.0 })?; + if !narrowed.validity().is_empty() { + return Err(KernelError::Failed("narrowing a bore: an unsound result".into())); + } + let Some(joined) = crate::ops::boolean(target, &narrowed, BoolOp::Union)? else { return Ok(None) }; + let cut = crate::build::cylinder(at(lo, 0.0, 0.0), c.axis, c.radius, hi - lo)?; + crate::ops::boolean(&joined, &cut, BoolOp::Cut) + }; + match run() { + Ok(Some(r)) if accept(&r) => return Some(r), + _ => continue, + } + } + } + None +} diff --git a/crates/kernel/src/ops.rs b/crates/kernel/src/ops.rs index 5cc5904d..1a61fc0b 100644 --- a/crates/kernel/src/ops.rs +++ b/crates/kernel/src/ops.rs @@ -175,6 +175,13 @@ thread_local! { /// Set while a boolean runs on bodies whose coincident faces were pushed apart (no /// second round of pushing). static APART: std::cell::Cell = const { std::cell::Cell::new(false) }; + /// Set while a union runs with a bore narrowed off a common cylinder (no second round). + static BORES: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +/// Is a union with a narrowed bore running (see [`crate::coplanar::bore_union`])? +pub(crate) fn narrowing_bores() -> bool { + BORES.with(|c| c.get()) } /// Boolean of two bodies. The result may be empty (`Ok(None)`) for a cut that removes @@ -344,6 +351,15 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { } } } + // Bores on a common cylinder (a pad's bore around the hole it is joined over). + if op == BoolOp::Union && !BORES.with(|c| c.get()) && crate::coplanar::has_coincident_cylinders(a, b) { + BORES.with(|c| c.set(true)); + let r = crate::coplanar::bore_union(a, b, &|body| check(body).0); + BORES.with(|c| c.set(false)); + if let Some(body) = r { + return Ok(Some(body)); + } + } let mut empty_votes = 0; let keep: Vec> = a.split_keep().iter().chain(b.split_keep()).cloned().collect(); for (attempt, j) in JITTER.iter().enumerate() { diff --git a/crates/kernel/src/tests.rs b/crates/kernel/src/tests.rs index b65c41a5..f8dcc04c 100644 --- a/crates/kernel/src/tests.rs +++ b/crates/kernel/src/tests.rs @@ -290,6 +290,52 @@ fn moving_a_body_with_an_intersection_curve_edge_ends() { assert!(rel(v0, v1) < 1e-6, "{v0} vs {v1}"); } +/// Drain pads joined flush under a floor with a drain hole. The floor (100 × 80, top sloping +/// 1°, so not a prism) has a Ø20 hole through it at (50, 40). +/// - A ring whose bore is the hole's wall (coincident cylinders): used to fail. +/// - The same ring 2 mm into the floor: used to fail too. +/// - A disc across the hole: worked before, and must stay exact. +#[test] +fn drain_pads_flush_under_a_floor_with_a_hole() { + let t = 1f64.to_radians().tan(); + let hs = |n: Vec3, p: Vec3| { + let n = n.normalized().unwrap(); + HalfSpace { n, d: n.dot(p) } + }; + let slab = convex_polyhedron(&[ + hs(Vec3::X, Vec3::new(100.0, 0.0, 0.0)), + hs(-Vec3::X, Vec3::ZERO), + hs(Vec3::Y, Vec3::new(0.0, 80.0, 0.0)), + hs(-Vec3::Y, Vec3::ZERO), + hs(-Vec3::Z, Vec3::ZERO), + hs(Vec3::new(-t, 0.0, 1.0), Vec3::new(0.0, 0.0, 10.0)), + ]) + .unwrap(); + let hole = cylinder(Vec3::new(50.0, 40.0, -5.0), Vec3::Z, 10.0, 30.0).unwrap(); + let floor = boolean(&slab, &hole, BoolOp::Cut).unwrap().unwrap(); + // Top z = 10 + t x: the slab's volume is its mean height (at x = 50) times its area, the + // hole's likewise (its top is planar, centred at x = 50). + let v_floor = (10.0 + t * 50.0) * (8000.0 - PI * 100.0); + assert!(rel(measure(&floor).unwrap().volume, v_floor) < 2e-5, "{} vs {v_floor}", measure(&floor).unwrap().volume); + let ring = |z0: f64, z1: f64| { + let r = Region2 { outer: Loop2::circle(Vec2::new(50.0, 40.0), 20.0), holes: vec![Loop2::circle(Vec2::new(50.0, 40.0), 10.0).reversed()] }; + extrude(&Plane::XY, &[r], z0, z1).unwrap().pop().unwrap() + }; + // The ring's part inside the floor adds nothing: 5 mm of ring below it either way. + let v_ring = PI * (400.0 - 100.0) * 5.0; + for (z0, z1) in [(-5.0, 0.0), (-5.0, 2.0)] { + let u = boolean(&floor, &ring(z0, z1), BoolOp::Union).unwrap().unwrap(); + let v = measure(&u).unwrap().volume; + assert!(rel(v, v_floor + v_ring) < 2e-5, "ring {z0}..{z1}: {v} vs {}", v_floor + v_ring); + assert!(u.validity().is_empty(), "{:?}", u.validity()); + } + let disc = cylinder(Vec3::new(50.0, 40.0, -5.0), Vec3::Z, 20.0, 5.0).unwrap(); + let u = boolean(&floor, &disc, BoolOp::Union).unwrap().unwrap(); + let v = measure(&u).unwrap().volume; + let expect = v_floor + PI * 400.0 * 5.0; + assert!(rel(v, expect) < 2e-5, "disc across the hole: {v} vs {expect}"); +} + #[test] fn planar_booleans_with_coincident_faces() { // L-bracket: a wall flush with three sides of its base.