diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index 6593252d..fc9b9e29 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -88,8 +88,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> }, /// Complete current boundaries of topology-aware analytic offset associations. /// The local association identity survives retirement and resurrection of arc roles. diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index 7a9e205c..a54e83a1 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -795,7 +795,7 @@ pub fn select_profiles(ss: &SolvedSketch, sel: &ProfileSel) -> Result Result 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 916d9475..554215c6 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -38,9 +38,9 @@ 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, RevolveExtent, SolvedSketch, ThreadInfo, edge_names_for, edge_refs, object_height, - parse_metric_thread, pattern_matrices, revolve_extent, revolve_extent_with_convention, select_profiles, state_in_frame, state_in_frame_at, - through_all_distance, world_state, + EdgeRef, FeatureResult, Model, ModelBody, ModelState, RevolveExtent, SolvedSketch, ThreadInfo, curve_pick_error, edge_names_for, edge_refs, + object_height, parse_metric_thread, pattern_matrices, revolve_extent, revolve_extent_with_convention, select_profiles, state_in_frame, + state_in_frame_at, through_all_distance, world_state, }; pub use params::{ParamRow, ParamUser, cycles}; pub use solvecraft_kernel as kernel; diff --git a/crates/doc/src/profile_refs.rs b/crates/doc/src/profile_refs.rs index ff92be90..7359809d 100644 --- a/crates/doc/src/profile_refs.rs +++ b/crates/doc/src/profile_refs.rs @@ -112,12 +112,16 @@ pub fn profile_selection_indices(ss: &SolvedSketch, selection: &ProfileSel) -> R } indices.push(unique_profile(ss, &ss.sketch.offset_boundary(id)?)?); } else { - // Preserve the historical first exact ordinary-loop match. + // Repeated saved IDs identify exact graph spans. Their multiplicity + // must not silently turn into a whole-source certificate on resize. + if curves.iter().collect::>().len() != curves.len() + && !ss.profiles.iter().any(|profile| same_ids(curves, &profile.outer_curves)) + { + return Err(crate::curve_pick_error(solvecraft_sketch::CurvePick::Missing, curves, &ss.name)); + } indices.push( - ss.profiles - .iter() - .position(|profile| same_ids(curves, &profile.outer_curves)) - .ok_or_else(|| DocError::Unknown(format!("profile bounded by {curves:?} in {}", ss.name)))?, + solvecraft_sketch::profile_by_curves(&ss.profiles, curves) + .map_err(|error| crate::curve_pick_error(error, curves, &ss.name))?, ); } } diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index f18010df..cbcfff75 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -195,6 +195,29 @@ 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) +} + /// Fields of a stored feature left out of its definition: bookkeeping timeline.edit recomputes, and /// the name and suppression, which timeline.rename and timeline.suppress change. const NOT_EDITABLE: &[&str] = &["id", "type", "name", "suppressed", "param_names", "edge_refs", "edge_names", "face_names"]; @@ -247,6 +270,7 @@ fn edit_feature(s: &mut Session, p: &Value) -> Result { 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; @@ -273,6 +297,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 d9ef37b8..76f6e0a9 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") @@ -204,31 +204,54 @@ 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::profile_at_point; let st = s.model.state(); - 'sels: for (sketch, sel) in kind.profile_sels_mut() { + for (sketch, sel) in kind.profile_sels_mut() { let Some(ss) = st.sketch(sketch) else { continue }; - if !matches!( - sel, - ProfileSel::Indices { .. } | ProfileSel::Curves { .. } | ProfileSel::OffsetBoundaries { .. } | ProfileSel::WholeBoundaries { .. } - ) { - continue; - } - let indices = solvecraft_doc::profile_selection_indices(ss, sel)?; - 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 !matches!(sel, ProfileSel::WholeBoundaries { .. }) - && (pr.outer_curves.is_empty() || ss.profiles.iter().filter(|q| same_loop(&q.outer_curves, &pr.outer_curves)).count() != 1) - { - continue 'sels; + let indices = match sel { + ProfileSel::Indices { .. } => solvecraft_doc::profile_selection_indices(ss, sel)?, + ProfileSel::Points { points } => { + let Some(indices) = points.iter().map(|point| profile_at_point(&ss.profiles, *point)).collect::>>() else { continue }; + indices } - } + ProfileSel::Curves { loops } => { + // The command's ordinary curve lists are sets, including repeated + // caller IDs. Saved graph-span multisets retain their legacy meaning. + // Reserved offset roles never use this ordinary normalization. + let mut normalized = loops.clone(); + let reserved = ss + .sketch + .offsets + .iter() + .filter_map(|offset| offset.topology.as_ref()) + .flat_map(|topology| topology.roles.iter().map(|role| role.output.as_str())) + .collect::>(); + for curves in &mut normalized { + let dynamic = curves.iter().any(|curve| reserved.contains(curve.as_str())); + if !dynamic { + let mut seen = std::collections::BTreeSet::new(); + curves.retain(|curve| seen.insert(curve.clone())); + } + } + let selection = ProfileSel::Curves { loops: normalized }; + solvecraft_doc::profile_selection_indices(ss, &selection)?; + *sel = selection; + continue; + } + ProfileSel::OffsetBoundaries { .. } | ProfileSel::WholeBoundaries { .. } => { + // Explicit identities retain the caller's semantics. Validate through the + // same resolver as recomputation, including reserved-role precedence. + solvecraft_doc::profile_selection_indices(ss, sel)?; + continue; + } + ProfileSel::All => continue, + }; let captured = solvecraft_doc::capture_profile_selection(ss, &indices)?; *sel = match captured { ProfileSel::OffsetBoundaries { associations } @@ -260,13 +283,6 @@ fn stable_profiles(s: &Session, kind: &mut FeatureKind) -> Result<()> { 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 1d157477..59c72e92 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -638,6 +638,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..dc1d6c65 --- /dev/null +++ b/crates/engine/src/profile_curves_tests.rs @@ -0,0 +1,206 @@ +//! 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)); +} + +/// New explicit API lists are sets even when reordered or repeated. Their saved +/// form must not accidentally become a legacy graph-span multiset. +#[test] +fn explicit_curve_pick_normalizes_repeats_before_save_and_parameter_edit() { + let mut s = tray(); + run(&mut s, "solid.extrude", json!({"sketch":"Tray","profiles":[{"curves":["top","bottom","div","left","bottom","top"]}],"distance":10})); + let ProfileSel::Curves { loops } = extrude_kind(&s) else { panic!("{:?}", extrude_kind(&s)) }; + assert_eq!(loops, vec![vec!["top", "bottom", "div", "left"]]); + let mut reopened = Session::new(solvecraft_doc::format::read(&s.doc.to_json()).unwrap()); + run(&mut reopened, "parameters.change", json!({"name":"split","expression":"20 mm"})); + assert_eq!(last_error(&reopened), None); + assert!((volumes(&reopened)[0] - 4000.0).abs() < 1.0); +} + +#[test] +fn repeated_explicit_curve_ids_do_not_count_split_graph_spans() { + 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!({})); + assert_eq!(s.model.state().sketches[0].profiles[0].outer_curves.iter().filter(|id| *id == "a").count(), 2); + run(&mut s, "solid.extrude", json!({"sketch":"S","profiles":[{"curves":["d","a","a","a","c","b"]}],"distance":2})); + let ProfileSel::Curves { loops } = extrude_kind(&s) else { panic!("{:?}", extrude_kind(&s)) }; + assert_eq!(loops, vec![vec!["d", "a", "c", "b"]]); + let reopened = Session::new(solvecraft_doc::format::read(&s.doc.to_json()).unwrap()); + assert_eq!(last_error(&reopened), None); + assert!((volumes(&reopened)[0] - 200.0).abs() < 1e-3); +} + +/// 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/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 b6367f38..ea049d5c 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 @@ -347,6 +354,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 cb771d76..0fb2578c 100644 --- a/crates/kernel/src/tests.rs +++ b/crates/kernel/src/tests.rs @@ -290,6 +290,60 @@ 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}"); + // A boss ring standing on the sloping top around the hole, its foot just under the top + // everywhere: it adds its annulus times its height above the top at the centre. + let foot = 10.0 + t * 30.0 - 0.2; + let u = boolean(&floor, &ring(foot, 14.0), BoolOp::Union).unwrap().unwrap(); + let v = measure(&u).unwrap().volume; + let expect = v_floor + PI * 300.0 * (14.0 - (10.0 + t * 50.0)); + assert!(rel(v, expect) < 2e-5, "boss ring on the slope: {v} vs {expect}"); + assert!(u.validity().is_empty(), "{:?}", u.validity()); +} + /// A trough whose floor falls 1° (or 2° toward a drain, in four faces), and short round pads /// and bosses crossing the fall. The B-rep results were right, but the plausibility check meshed /// them without refinement: on a round side trimmed to a thin band by the shallow cut, the plain diff --git a/crates/sketch/src/lib.rs b/crates/sketch/src/lib.rs index fa909052..678840cd 100644 --- a/crates/sketch/src/lib.rs +++ b/crates/sketch/src/lib.rs @@ -34,7 +34,7 @@ pub use model::{Constraint, ConstraintKind, Curve, CurveKind, SPoint, Sketch, Sk pub use offset::OffsetChain; pub use offset_topology::{OffsetCreated, OffsetRole, OffsetRoleFlags, OffsetRoleKind, OffsetTopology}; pub use periodic::periodic_beziers; -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 f43a6f2f..98e90e33 100644 --- a/crates/sketch/src/profiles.rs +++ b/crates/sketch/src/profiles.rs @@ -922,6 +922,69 @@ fn find_profiles_bounded(sk: &Sketch, max_edges: usize) -> 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 590d34f5..e889c74e 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2932,7 +2932,7 @@ pub(crate) fn profile_indices(ss: &solvecraft_engine::doc::SolvedSketch, sel: &P solvecraft_engine::doc::profile_selection_indices(ss, &ProfileSel::Curves { loops: vec![l.clone()] }).ok()?.first().copied() }) .collect(), - ProfileSel::Points { points } => points.iter().filter_map(|q| ps.iter().position(|p| p.region.contains(*q))).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 511867d8..096a3bfb 100644 --- a/docs/book/src/guide/sketching.md +++ b/docs/book/src/guide/sketching.md @@ -105,3 +105,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.