diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index c7f70e3a..7b94485c 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -939,6 +939,66 @@ fn z_axis() -> Vec3 { } impl FeatureKind { + /// Sketches the feature is built from (profiles, paths, points): deleting one of them + /// deletes the feature. Every kind is listed, so a new one has to say what it uses. + pub fn input_sketches(&self) -> Vec { + use FeatureKind as K; + match self { + K::Extrude { sketch, .. } + | K::Revolve { sketch, .. } + | K::Emboss { sketch, .. } + | K::Rib { sketch, .. } + | K::SheetBase { sketch, .. } + | K::SheetContour { sketch, .. } => vec![*sketch], + K::Sweep { sketch, path_sketch, .. } => vec![*sketch, *path_sketch], + K::Pipe { path_sketch, .. } | K::Pattern { pattern: PatternKind::Path { path_sketch, .. }, .. } => vec![*path_sketch], + K::Loft { sections, .. } => sections.iter().map(|s| s.sketch).collect(), + K::SheetFold { sketch, .. } | K::SplitFace { sketch, .. } | K::Patch { sketch, .. } => sketch.iter().copied().collect(), + K::Hole { points, .. } => points.iter().map(|p| p.sketch).collect(), + K::Sketch { .. } + | K::Pattern { .. } + | K::Fillet { .. } + | K::Chamfer { .. } + | K::Box { .. } + | K::Cylinder { .. } + | K::Sphere { .. } + | K::Torus { .. } + | K::Combine { .. } + | K::Mirror { .. } + | K::Thread { .. } + | K::Shell { .. } + | K::Draft { .. } + | K::ConstructionPlane { .. } + | K::Split { .. } + | K::ConstructionAxis { .. } + | K::ConstructionPoint { .. } + | K::Scale { .. } + | K::OffsetFace { .. } + | K::DeleteFace { .. } + | K::BoundingSolid { .. } + | K::ReplaceFace { .. } + | K::Align { .. } + | K::Coil { .. } + | K::SheetFlange { .. } + | K::SheetHem { .. } + | K::SheetUnfold { .. } + | K::SheetConvert { .. } + | K::Boss { .. } + | K::Lip { .. } + | K::SnapFit { .. } + | K::Rest { .. } + | K::BoundaryFill { .. } + | K::Remove { .. } + | K::Stitch { .. } + | K::Thicken { .. } + | K::SurfaceExtend { .. } + | K::SurfaceTrim { .. } + | K::Move { .. } + | K::Import { .. } + | K::MeshImport { .. } => Vec::new(), + } + } + /// A sheet metal feature (its result depends on the sheet metal rules). pub fn is_plastic(&self) -> bool { matches!(self, FeatureKind::Boss { .. } | FeatureKind::Lip { .. } | FeatureKind::SnapFit { .. } | FeatureKind::Rest { .. }) @@ -1620,18 +1680,7 @@ impl Document { pub fn delete_feature(&mut self, id: u64) -> Result> { let idx = self.feature_index(id).ok_or_else(|| DocError::Unknown(format!("feature {id}")))?; let mut gone = vec![id]; - for f in &self.features { - match &f.kind { - FeatureKind::Extrude { sketch, .. } | FeatureKind::Revolve { sketch, .. } if *sketch == id => gone.push(f.id), - FeatureKind::Sweep { sketch, path_sketch, .. } if *sketch == id || *path_sketch == id => gone.push(f.id), - FeatureKind::Loft { sections, .. } if sections.iter().any(|s| s.sketch == id) => gone.push(f.id), - FeatureKind::Pattern { pattern: PatternKind::Path { path_sketch, .. }, .. } if *path_sketch == id => gone.push(f.id), - FeatureKind::Emboss { sketch, .. } | FeatureKind::Rib { sketch, .. } if *sketch == id => gone.push(f.id), - FeatureKind::SheetBase { sketch, .. } | FeatureKind::SheetContour { sketch, .. } if *sketch == id => gone.push(f.id), - FeatureKind::SheetFold { sketch: Some(s), .. } | FeatureKind::SplitFace { sketch: Some(s), .. } if *s == id => gone.push(f.id), - _ => {} - } - } + gone.extend(self.features.iter().filter(|f| f.kind.input_sketches().contains(&id)).map(|f| f.id)); self.features.retain(|f| !gone.contains(&f.id)); // Configuration columns of deleted features go with them. let keep: Vec = self.configs.columns.iter().map(|c| !matches!(c, crate::config::Column::Suppress(id) if gone.contains(id))).collect(); diff --git a/crates/doc/src/project.rs b/crates/doc/src/project.rs index 6e42aa42..5378b1b8 100644 --- a/crates/doc/src/project.rs +++ b/crates/doc/src/project.rs @@ -485,7 +485,7 @@ fn tilted_circle(plane: &Plane, pts: &[Vec3]) -> Option> { let (c, n, r, closed) = circle3(pts)?; let sn = plane.normal(); let cosang = n.dot(sn).abs(); - if cosang > 1.0 - 1e-12 || cosang < 1e-9 { + if !(1e-9..=1.0 - 1e-12).contains(&cosang) { return None; } if closed { diff --git a/crates/doc/src/sheet.rs b/crates/doc/src/sheet.rs index cdbd6a91..6bee00c8 100644 --- a/crates/doc/src/sheet.rs +++ b/crates/doc/src/sheet.rs @@ -899,7 +899,7 @@ impl SheetBody { return Err(DocError::Invalid("the hem is shorter than its bend".into())); } // A hem folds back over the face it starts from (toward the bottom unless flipped). - let up = !top == flip; + let up = top != flip; let up = !up; self.flanges.push(SheetFlange { parent: panel, diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index b74f6da1..5a4b9016 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -295,3 +295,53 @@ fn parameter_cycles_are_found() { assert!(errs["a"].contains("circular reference: a → b → c → a"), "{}", errs["a"]); assert!(vals.contains_key("d") && errs.contains_key("a") && errs.contains_key("e")); } + +/// Features that use a sketch's points or regions go with it (not only profile and path users). +#[test] +fn input_sketches_cover_points_and_patches() { + let hole: FeatureKind = serde_json::from_value(serde_json::json!({ + "type": "hole", "position": [0, 0, 0], "direction": [0, 0, -1], "diameter": "3", + "points": {"sketch": 7, "ids": ["p1"]}, + })) + .unwrap(); + assert_eq!(hole.input_sketches(), vec![7]); + let patch: FeatureKind = serde_json::from_value(serde_json::json!({"type": "patch", "sketch": 9})).unwrap(); + assert_eq!(patch.input_sketches(), vec![9]); + let pipe: FeatureKind = serde_json::from_value(serde_json::json!({"type": "pipe", "path_sketch": 72, "path": ["l1"], "diameter": "4"})).unwrap(); + assert_eq!(pipe.input_sketches(), vec![72]); +} + +/// Guard for `FeatureKind::input_sketches`: a variant with a field that refers to a sketch +/// (a field named `*sketch*` holding an id, sketch points or loft sections) must not sit in the +/// arm that returns no sketches. New variants are caught by the compiler (the match has no +/// wildcard); this catches a sketch field added to an existing variant. +#[test] +fn input_sketches_lists_every_variant_with_a_sketch_field() { + let src = include_str!("document.rs"); + let body = src.split("pub enum FeatureKind {").nth(1).and_then(|s| s.split("\n}\n").next()).unwrap(); + let mut with_sketch = Vec::new(); + let mut variant = ""; + for line in body.lines() { + let t = line.trim(); + if line.starts_with(" ") && !line.starts_with(" ") && t.chars().next().is_some_and(|c| c.is_ascii_uppercase()) { + variant = t.split(|c: char| !c.is_alphanumeric()).next().unwrap_or(""); + continue; + } + let Some((name, ty)) = t.strip_prefix("pub ").unwrap_or(t).split_once(':') else { continue }; + let refers = (name.contains("sketch") && (ty.contains("u64") || ty.contains("SketchPoints"))) + || ty.contains("SketchPoints") + || ty.contains("LoftSection"); + if refers && !line.trim_start().starts_with("//") && !with_sketch.contains(&variant) { + with_sketch.push(variant); + } + } + assert!(with_sketch.len() >= 10, "the FeatureKind parse found too few sketch users: {with_sketch:?}"); + let f = src.split("pub fn input_sketches(&self)").nth(1).unwrap(); + let empty_arm = f.split("=> Vec::new()").next().and_then(|s| s.rsplit("=> ").next()).unwrap(); + for v in with_sketch { + assert!( + !empty_arm.contains(&format!("K::{v} {{")), + "FeatureKind::{v} has a sketch field but input_sketches says it uses no sketch: deleting the sketch would leave it broken" + ); + } +} diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 08eb8695..cdfcd4cc 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -10,6 +10,12 @@ use crate::{EngineError, Result, Sel, Session, Snapshot}; pub static COMMANDS: &[CommandSpec] = &[ CommandSpec::new("edit.undo", "Undo", undo).icon("undo").key("Ctrl+Z").noundo(), CommandSpec::new("edit.redo", "Redo", redo).icon("redo").key("Ctrl+Y").noundo(), + CommandSpec::new("edit.checkpoint", "Checkpoint", checkpoint) + .noundo() + .params("→ {checkpoint: id}: remember the design, active sketch and undo history (restore with edit.restore_checkpoint)"), + CommandSpec::new("edit.restore_checkpoint", "Restore Checkpoint", restore_checkpoint) + .noundo() + .params("checkpoint: id; forget?: bool (true = drop it without going back). Goes back to it, undo history included"), CommandSpec::new("timeline.compute_all", "Compute All", compute_all).at("SOLID", "MODIFY").icon("compute").noundo(), CommandSpec::new("timeline.delete", "Delete", delete) .at("SOLID", "MODIFY") @@ -60,6 +66,21 @@ fn redo(s: &mut Session, _p: &Value) -> Result { Ok(json!({"redone": label})) } +fn checkpoint(s: &mut Session, _p: &Value) -> Result { + Ok(json!({"checkpoint": s.checkpoint()})) +} + +fn restore_checkpoint(s: &mut Session, p: &Value) -> Result { + let cmd = "edit.restore_checkpoint"; + let id = p.get("checkpoint").and_then(Value::as_u64).ok_or_else(|| bad(cmd, "`checkpoint` must be an id from edit.checkpoint"))?; + if bool_(p, "forget") == Some(true) { + s.drop_checkpoint(id); + return Ok(json!({"checkpoint": id, "restored": false})); + } + s.restore_checkpoint(id)?; + Ok(json!({"checkpoint": id, "restored": true})) +} + fn compute_all(s: &mut Session, _p: &Value) -> Result { s.model = solvecraft_doc::Model::new(); s.refresh(); diff --git a/crates/engine/src/cmd/measure_sel.rs b/crates/engine/src/cmd/measure_sel.rs index 95dc2dc9..9c978dfc 100644 --- a/crates/engine/src/cmd/measure_sel.rs +++ b/crates/engine/src/cmd/measure_sel.rs @@ -55,13 +55,13 @@ fn densify(pts: &[Vec3]) -> Vec { /// The mesh edge a selection means: by index if it still passes through the point, otherwise /// the nearest. -fn edge_points(m: &Mesh, index: usize, point: Vec3) -> Option> { +fn edge_index(m: &Mesh, index: usize, point: Vec3) -> Option { let near = |e: &Vec| e.windows(2).map(|w| point.dist_to_segment(w[0], w[1])).fold(f64::INFINITY, f64::min); let tol = m.bounds().diagonal().max(1.0) * 1e-6; - if let Some(e) = m.edges.get(index).filter(|e| near(e) < tol) { - return Some(e.clone()); + if m.edges.get(index).is_some_and(|e| near(e) < tol) { + return Some(index); } - m.edges.iter().min_by(|a, b| near(a).total_cmp(&near(b))).cloned() + m.edges.iter().enumerate().min_by(|(_, a), (_, b)| near(a).total_cmp(&near(b))).map(|(i, _)| i) } fn face_tris(m: &Mesh, f: usize) -> Vec<[Vec3; 3]> { @@ -75,15 +75,23 @@ fn describe(s: &Session, sel: &Sel) -> Option<(Geo, Option, Value)> { Sel::Vertex { point, .. } => (Geo::Points(vec![*point]), None, json!({"type": "vertex", "position": point})), Sel::Edge { body, index, point } => { let m = st.body(body)?.mesh(); - let pts = edge_points(&m, *index, *point)?; + let index = edge_index(&m, *index, *point)?; + let pts = m.edges.get(index)?; + let rim = m.circular_rim(index); + if rim.len() > 1 { + let pieces: Vec<&Vec> = rim.iter().filter_map(|&i| m.edges.get(i)).collect(); + let len: f64 = pieces.iter().map(|p| polyline_len(p)).sum(); + let points = pieces.iter().flat_map(|p| densify(p)).collect(); + return Some((Geo::Points(points), None, json!({"type": "edge", "length_mm": len}))); + } let (a, b) = (*pts.first()?, *pts.last()?); let straight = (b - a).normalized().filter(|d| pts.iter().all(|p| (*p - a).cross(*d).len() < 1e-6 * (1.0 + a.dist(b)))); - let len = polyline_len(&pts); - (Geo::Points(densify(&pts)), straight, json!({"type": "edge", "length_mm": len})) + let len = polyline_len(pts); + (Geo::Points(densify(pts)), straight, json!({"type": "edge", "length_mm": len})) } Sel::Face { body, index, .. } => { let m = st.body(body)?.mesh(); - let tris = face_tris(&m, *index); + let tris = m.surface_patch(*index).into_iter().flat_map(|f| face_tris(&m, f)).collect::>(); let area: f64 = tris.iter().map(|[a, b, c]| (*b - *a).cross(*c - *a).len() * 0.5).sum(); let normals: Vec = tris.iter().filter_map(|[a, b, c]| (*b - *a).cross(*c - *a).normalized()).collect(); let planar = normals.first().copied().filter(|n0| normals.iter().all(|n| n.dot(*n0) > 1.0 - 1e-6)); @@ -150,10 +158,41 @@ fn closest(a: &Geo, b: &Geo) -> Option<(Vec3, Vec3)> { pub(crate) fn measure_items(s: &Session, items: &Value) -> Result { let cmd = "inspect.measure"; let list = items.as_array().ok_or_else(|| bad(cmd, "`items` must be a list of selections"))?; - if list.is_empty() || list.len() > 2 { + if list.is_empty() || list.len() > 1000 { return Err(bad(cmd, "measure one or two items")); } - let sels: Vec = list.iter().map(|v| serde_json::from_value::(v.clone()).map_err(|e| bad(cmd, e.to_string()))).collect::>()?; + let st = s.world_state(); + let mut sels = Vec::new(); + for value in list { + let mut sel = serde_json::from_value::(value.clone()).map_err(|e| bad(cmd, e.to_string()))?; + // Two kernel halves of one visible item are one measurement target. + match &mut sel { + Sel::Edge { body, index, point } => { + if let Some(b) = st.body(body) { + let m = b.mesh(); + let resolved = edge_index(&m, *index, *point).unwrap_or(*index); + *index = m.circular_rim(resolved).first().copied().unwrap_or(resolved); + } + } + Sel::Face { body, index, .. } => { + if let Some(b) = st.body(body) { + *index = b.mesh().surface_patch(*index).first().copied().unwrap_or(*index); + } + } + _ => {} + } + let duplicate = sels.iter().any(|x| match (x, &sel) { + (Sel::Edge { body: a, index: i, .. }, Sel::Edge { body: b, index: j, .. }) + | (Sel::Face { body: a, index: i, .. }, Sel::Face { body: b, index: j, .. }) => a == b && i == j, + _ => false, + }); + if !duplicate { + sels.push(sel); + } + if sels.len() > 2 { + return Err(bad(cmd, "measure one or two items")); + } + } let mut described = Vec::new(); for x in &sels { described.push(describe(s, x).ok_or_else(|| bad(cmd, "an item can't be measured (pick vertices, edges, faces or bodies)"))?); diff --git a/crates/engine/src/cmd/sketch_modify.rs b/crates/engine/src/cmd/sketch_modify.rs index 14d4b739..f79c3697 100644 --- a/crates/engine/src/cmd/sketch_modify.rs +++ b/crates/engine/src/cmd/sketch_modify.rs @@ -520,7 +520,7 @@ fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize let u = (pa - pc).normalized().ok_or_else(|| bad(cmd, "degenerate line"))?; let v = (pb - pc).normalized().ok_or_else(|| bad(cmd, "degenerate line"))?; let half = 0.5 * u.cross(v).atan2(u.dot(v)).abs(); - if half < 1e-6 || half > std::f64::consts::FRAC_PI_2 - 1e-9 { + if !(1e-6..=std::f64::consts::FRAC_PI_2 - 1e-9).contains(&half) { return Err(bad(cmd, "the lines are parallel")); } let d = r / half.tan(); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 2ce89495..22a5106f 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -170,6 +170,20 @@ pub struct Session { pub analysis: Option, /// What is hidden or shown in the view (`browser.visibility`). pub visibility: Visibility, + /// States to go back to (`edit.checkpoint`, `edit.restore_checkpoint`): how batches of + /// commands are undone however long the undo history is. + pub(crate) checkpoints: Vec, + next_checkpoint: u64, +} + +/// The design, active sketch and undo history at an `edit.checkpoint`. +#[derive(Clone, Debug)] +pub(crate) struct Checkpoint { + pub id: u64, + pub doc: Arc, + pub active_sketch: Option, + pub undo: Vec, + pub redo: Vec, } /// Items hidden or shown one by one (view state, not part of the design and not an undo step). @@ -204,6 +218,8 @@ pub enum SurfaceAnalysis { } const MAX_UNDO: usize = 200; +/// Checkpoints kept (the oldest go first). +const MAX_CHECKPOINTS: usize = 16; const MAX_LOG: usize = 2000; impl Default for Session { @@ -235,6 +251,8 @@ impl Session { clipboard: Vec::new(), section: None, analysis: None, + checkpoints: Vec::new(), + next_checkpoint: 1, visibility: Visibility::default(), } } @@ -356,6 +374,44 @@ impl Session { } } + /// Remember the design, active sketch and undo history; returns the checkpoint's id. + pub fn checkpoint(&mut self) -> u64 { + let id = self.next_checkpoint; + self.next_checkpoint += 1; + self.checkpoints.push(Checkpoint { + id, + doc: self.doc.clone(), + active_sketch: self.active_sketch, + undo: self.undo.clone(), + redo: self.redo.clone(), + }); + if self.checkpoints.len() > MAX_CHECKPOINTS { + self.checkpoints.remove(0); + } + id + } + + /// Go back to checkpoint `id` as if nothing since had happened (the undo history too). It + /// and later checkpoints are used up. + pub fn restore_checkpoint(&mut self, id: u64) -> Result<()> { + let i = self.checkpoints.iter().position(|c| c.id == id).ok_or_else(|| EngineError::Other(format!("no checkpoint {id}")))?; + let Some(c) = self.checkpoints.drain(i..).next() else { return Err(EngineError::Other(format!("no checkpoint {id}"))) }; + self.doc = c.doc; + self.active_sketch = c.active_sketch; + self.undo = c.undo; + self.redo = c.redo; + self.selection.clear(); + self.refresh(); + Ok(()) + } + + /// Forget checkpoint `id` (and later ones) without going back. + pub fn drop_checkpoint(&mut self, id: u64) { + if let Some(i) = self.checkpoints.iter().position(|c| c.id == id) { + self.checkpoints.truncate(i); + } + } + /// A scratch copy of the session (same design and model, no history or log) to try /// commands on without touching this one. pub fn scratch(&self) -> Session { @@ -377,6 +433,8 @@ impl Session { clipboard: self.clipboard.clone(), section: None, analysis: None, + checkpoints: Vec::new(), + next_checkpoint: 1, visibility: Visibility::default(), } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3c4a6714..2495c83c 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -839,6 +839,51 @@ fn pipe_scale_offset_bounding_and_materials() { assert!(s.execute("material.assign", &json!({"bodies": ["Cube"], "material": "unobtainium"})).is_err()); } +/// Regression: deleting a pipe's path sketch left the pipe in the timeline ("unknown sketch"). +#[test] +fn deleting_a_path_sketch_deletes_its_pipe() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Path"})); + run(&mut s, "sketch.line", json!({"points": [[0, 0], [30, 0]], "ids": ["l1"]})); + run(&mut s, "sketch.finish", json!({})); + let pipe = run(&mut s, "solid.pipe", json!({"path_sketch": "Path", "path": ["l1"], "diameter": 4})); + let deps = run(&mut s, "timeline.dependents", json!({"feature": "Path"})); + assert_eq!(deps["deleted_with_it"].as_array().map(Vec::len), Some(1), "{deps} (pipe {pipe})"); + let r = run(&mut s, "timeline.delete", json!({"features": ["Path"]})); + assert_eq!(r["deleted"].as_array().map(Vec::len), Some(2), "{r}"); + assert!(s.doc.features.is_empty(), "{:?}", s.doc.features.iter().map(|f| &f.name).collect::>()); + assert!(s.model.results.iter().all(|r| r.error.is_none())); +} + +/// Regression: batches were rolled back by undoing as many steps as the undo history grew, +/// which is none once the history is full, leaving their sketches behind. +#[test] +fn checkpoint_restores_with_a_full_undo_history() { + let mut s = Session::default(); + for i in 0..MAX_UNDO + 5 { + run(&mut s, "parameters.add", json!({"name": format!("p{i}"), "expression": "1 mm"})); + } + assert_eq!(s.undo.len(), MAX_UNDO); + let labels: Vec = s.undo.iter().map(|u| u.label.clone()).collect(); + let doc = s.doc.clone(); + let id = run(&mut s, "edit.checkpoint", json!({}))["checkpoint"].as_u64().unwrap(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "TapHole"})); + run(&mut s, "sketch.circle.center", json!({"center": [0, 0], "radius": 2})); + assert!(s.active_sketch.is_some()); + run(&mut s, "edit.restore_checkpoint", json!({"checkpoint": id})); + assert!(Arc::ptr_eq(&s.doc, &doc) && s.active_sketch.is_none()); + assert!(s.doc.find_feature("TapHole").is_none()); + assert_eq!(s.undo.iter().map(|u| u.label.clone()).collect::>(), labels); + // Used up: a second restore is an error, not a jump somewhere else. + assert!(s.execute("edit.restore_checkpoint", &json!({"checkpoint": id})).is_err()); + // Forgetting one leaves the design alone. + let id = run(&mut s, "edit.checkpoint", json!({}))["checkpoint"].as_u64().unwrap(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Kept"})); + run(&mut s, "edit.restore_checkpoint", json!({"checkpoint": id, "forget": true})); + assert!(s.doc.find_feature("Kept").is_some()); + assert!(s.execute("edit.restore_checkpoint", &json!({"checkpoint": id})).is_err()); +} + #[test] fn components_occurrences_and_world_placement() { let mut s = Session::default(); diff --git a/crates/geom/src/mesh.rs b/crates/geom/src/mesh.rs index 32c3373a..296f4808 100644 --- a/crates/geom/src/mesh.rs +++ b/crates/geom/src/mesh.rs @@ -82,6 +82,71 @@ impl Mesh { pub fn face_edges(&self, f: u32) -> Vec { self.edge_faces.iter().enumerate().filter(|(_, fs)| fs.contains(&f)).map(|(i, _)| i).collect() } + /// Pieces of one surface joined across hidden seams, including `face` itself. + pub fn surface_patch(&self, face: usize) -> Vec { + let mut patch = vec![face]; + let mut todo = vec![face]; + while let Some(f) = todo.pop() { + for (ei, fs) in self.edge_faces.iter().enumerate() { + if !self.seams.get(ei).copied().unwrap_or(false) || !fs.iter().any(|x| *x as usize == f) { + continue; + } + for &next in fs { + let next = next as usize; + if !patch.contains(&next) { + patch.push(next); + todo.push(next); + } + } + } + } + patch.sort_unstable(); + patch + } + + /// A complete circular rim may be split into several kernel edges. Keep its pieces + /// together for picking, without joining other tangent curves or changing edge indices. + pub fn circular_rim(&self, edge: usize) -> Vec { + let single = vec![edge]; + let Some(p) = self.edges.get(edge).filter(|p| p.len() >= 3) else { return single }; + let (Some(&a), Some(&b), Some(&c)) = (p.first(), p.get(p.len() / 2), p.last()) else { return single }; + let (u, v) = (b - a, c - a); + let n = u.cross(v); + if !(n.len2() > u.len2() * v.len2() * 1e-12) { + return single; + } + let center = a + (v.cross(n) * u.len2() + n.cross(u) * v.len2()) / (2.0 * n.len2()); + let radius = center.dist(a); + let normal = n.normalized().unwrap_or(Vec3::ZERO); + let tol = (radius * 1e-6).max(1e-9); + let mut rim = self.tangent_chain(edge, 2f64.to_radians()); + if rim.len() < 2 || !radius.is_finite() { + return single; + } + // All samples must lie on this circle, and every end must join exactly one other + // piece. Open arcs, ellipses, and branching tangent chains stay separate. + for &i in &rim { + let Some(points) = self.edges.get(i) else { return single }; + if points.iter().any(|q| !q.is_finite() || (q.dist(center) - radius).abs() > tol || (*q - center).dot(normal).abs() > tol) { + return single; + } + for end in [points.first(), points.last()].into_iter().flatten() { + let joins = rim + .iter() + .filter(|&&j| j != i) + .filter_map(|&j| self.edges.get(j)) + .flat_map(|q| [q.first(), q.last()]) + .flatten() + .filter(|q| q.dist(*end) < tol) + .count(); + if joins != 1 { + return single; + } + } + } + rim.sort_unstable(); + rim + } /// The edges reached from `e` through vertices where the next edge continues smoothly /// (angle between the end tangents below `max_angle`), including `e` itself. pub fn tangent_chain(&self, e: usize, max_angle: f64) -> Vec { diff --git a/crates/geom/src/tests.rs b/crates/geom/src/tests.rs index af1b6f68..3e996d14 100644 --- a/crates/geom/src/tests.rs +++ b/crates/geom/src/tests.rs @@ -84,6 +84,46 @@ fn tangent_chains_follow_smooth_edges() { assert_eq!(m.face_edges(1), vec![1, 2]); } +#[test] +fn circular_rims_group_only_closed_cocircular_edges() { + let arc = |start: f64, sweep: f64, yscale: f64| -> Vec { + (0..=32) + .map(|i| { + let a = start + sweep * i as f64 / 32.0; + Vec3::new(10.0 * a.cos(), yscale * 10.0 * a.sin(), 5.0) + }) + .collect() + }; + let pi = std::f64::consts::PI; + let mut m = Mesh { + positions: vec![Vec3::new(-10.0, -10.0, 5.0), Vec3::new(10.0, 10.0, 5.0)], + edges: vec![arc(0.0, pi, 1.0), arc(2.0 * pi, -pi, 1.0)], + ..Default::default() + }; + assert_eq!(m.circular_rim(0), vec![0, 1]); + assert_eq!(m.circular_rim(1), vec![0, 1]); + // A hidden seam cannot form a selectable rim. + m.seams = vec![false, true]; + assert_eq!(m.circular_rim(0), vec![0]); + m.seams.clear(); + m.edges[1] = arc(pi, pi / 2.0, 1.0); + assert_eq!(m.circular_rim(0), vec![0]); + // A closed, smooth ellipse is not a circle. + m.edges = vec![arc(0.0, pi, 0.7), arc(pi, pi, 0.7)]; + assert_eq!(m.circular_rim(0), vec![0]); + assert_eq!(m.circular_rim(99), vec![99]); +} + +#[test] +fn surface_patches_cross_only_hidden_seams() { + let mut m = Mesh { edge_faces: vec![vec![0, 1], vec![1, 2], vec![2, 3]], seams: vec![true, true, false], ..Default::default() }; + assert_eq!(m.surface_patch(0), vec![0, 1, 2]); + assert_eq!(m.surface_patch(2), vec![0, 1, 2]); + assert_eq!(m.surface_patch(3), vec![3]); + m.seams.clear(); + assert_eq!(m.surface_patch(0), vec![0]); +} + #[test] fn cubic_and_conic_segments_measure_exactly() { use crate::{Loop2, Seg2, Vec2}; diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index f050cd49..64aa12f8 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -213,6 +213,35 @@ fn batch_stops_at_first_error_and_rolls_back() { assert_eq!(tool(&mut s, "measure", json!({}))["body_count"], 2); } +/// Regression: with a full undo history a failed batch reported `undone: 0` and left its +/// sketches behind (an empty "Sketch1", two sketches named "TapHole"). +#[test] +fn batch_rolls_back_with_a_full_undo_history() { + let mut s = server(); + let fill: Vec = + (0..230).map(|i| json!({"command": "parameters.add", "params": {"name": format!("p{i}"), "expression": "1 mm"}})).collect(); + tool(&mut s, "batch", json!({"commands": fill})); + let undo = tool(&mut s, "inspect_design", json!({"measure": false}))["undo"].clone(); + for _ in 0..2 { + let r = raw_tool( + &mut s, + "batch", + json!({"commands": [ + {"command": "sketch.create", "params": {"plane": "XY", "name": "TapHole"}}, + {"command": "sketch.circle.center", "params": {"center": [0, 0], "radius": 2}}, + {"command": "sketch.dimension", "params": {"entities": ["nope"], "value": 3}}, + ]}), + ); + let body: Value = serde_json::from_str(r["result"]["content"][0]["text"].as_str().unwrap()).unwrap(); + assert_eq!(body["failed"]["index"], 2, "{body}"); + assert_eq!(body["undone"], 2, "{body}"); + } + let d = tool(&mut s, "inspect_design", json!({"measure": false, "sketches": false})); + assert_eq!(d["sketches"].as_array().map(Vec::len), Some(0), "{d}"); + assert!(d["active_sketch"].is_null(), "{d}"); + assert_eq!(d["undo"], undo); +} + #[test] fn helper_sketches_can_be_hidden() { let mut s = server(); diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 0d1e993a..938e0de7 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -361,7 +361,56 @@ fn undo_depth(b: &mut dyn Backend) -> Option { b.call("document.inspect", json!({})).ok().and_then(|d| d["undo"].as_u64()) } -/// `batch`: run steps in order, stop at the first failure and (by default) undo what ran. +/// How a failed batch goes back to where it started. +enum Undo { + /// An engine checkpoint: the design, active sketch and undo history exactly as they were. + Checkpoint(u64), + /// An app without checkpoints: undo as many steps as the batch added (wrong once the undo + /// history is full, as its depth no longer grows). + Depth(u64), + None, +} + +impl Undo { + fn begin(b: &mut dyn Backend) -> Undo { + match exec(b, "edit.checkpoint", json!({})).ok().and_then(|v| v["checkpoint"].as_u64()) { + Some(id) => Undo::Checkpoint(id), + None => undo_depth(b).map_or(Undo::None, Undo::Depth), + } + } + + /// Go back; the number of steps undone. + fn rollback(self, b: &mut dyn Backend, steps: usize) -> u64 { + match self { + Undo::Checkpoint(id) => match exec(b, "edit.restore_checkpoint", json!({"checkpoint": id})) { + Ok(_) => steps as u64, + Err(_) => 0, + }, + Undo::Depth(before) => { + let after = undo_depth(b).unwrap_or(before); + let mut undone = 0u64; + for _ in before..after { + if exec(b, "edit.undo", json!({})).is_err() { + break; + } + undone += 1; + } + undone + } + Undo::None => 0, + } + } + + /// The batch went through: forget the checkpoint. + fn finish(self, b: &mut dyn Backend) { + if let Undo::Checkpoint(id) = self { + let _ = exec(b, "edit.restore_checkpoint", json!({"checkpoint": id, "forget": true})); + } + } +} + +/// `batch`: run steps in order, stop at the first failure and (by default) go back to the +/// design as it was before the batch. fn batch(b: &mut dyn Backend, a: &Map) -> Value { let Some(steps) = a.get("commands").and_then(Value::as_array) else { return text_result(&json!("`commands` must be an array of {command, params}"), true); @@ -370,7 +419,7 @@ fn batch(b: &mut dyn Backend, a: &Map) -> Value { return text_result(&json!(format!("batch too long ({} steps, at most {MAX_BATCH})", steps.len())), true); } let rollback = a.get("rollback").and_then(Value::as_bool).unwrap_or(true); - let before = if rollback { undo_depth(b) } else { None }; + let start = if rollback { Undo::begin(b) } else { Undo::None }; let mut results = Vec::with_capacity(steps.len()); for (i, step) in steps.iter().enumerate() { let command = step.get("command").or_else(|| step.get("id")).and_then(Value::as_str); @@ -381,15 +430,7 @@ fn batch(b: &mut dyn Backend, a: &Map) -> Value { match r { Ok(v) => results.push(v), Err(e) => { - let mut undone = 0u64; - if let (Some(before), Some(after)) = (before, rollback.then(|| undo_depth(b)).flatten()) { - for _ in before..after { - if exec(b, "edit.undo", json!({})).is_err() { - break; - } - undone += 1; - } - } + let undone = start.rollback(b, i); return text_result( &json!({ "ok": false, @@ -404,6 +445,7 @@ fn batch(b: &mut dyn Backend, a: &Map) -> Value { } } } + start.finish(b); text_result(&json!({"ok": true, "completed": results.len(), "results": results}), false) } diff --git a/crates/sketch/src/model.rs b/crates/sketch/src/model.rs index d86535fb..2b5a0079 100644 --- a/crates/sketch/src/model.rs +++ b/crates/sketch/src/model.rs @@ -698,10 +698,8 @@ impl Sketch { | Symmetric { l, .. } | PointLineDistance { l, .. } | LinearDiameter { l, .. } => need_line(l)?, - ArcLength { c, .. } => { - if !matches!(self.curves.get(c).map(|c| &c.kind), Some(CurveKind::Arc { .. })) { - return Err(SketchError::WrongKind(name(c), "arc length needs an arc".into())); - } + ArcLength { c, .. } if !matches!(self.curves.get(c).map(|c| &c.kind), Some(CurveKind::Arc { .. })) => { + return Err(SketchError::WrongKind(name(c), "arc length needs an arc".into())); } Smooth { a, b } => { let ends = |i: usize| self.curves.get(i).and_then(|c| c.kind.ends()); @@ -730,10 +728,8 @@ impl Sketch { return Err(SketchError::WrongKind(name(a), "tangent needs a circle or arc".into())); } } - Equal { a, b } => { - if is_line(a) != is_line(b) { - return Err(SketchError::WrongKind(name(b), "equal needs two lines or two circles/arcs".into())); - } + Equal { a, b } if is_line(a) != is_line(b) => { + return Err(SketchError::WrongKind(name(b), "equal needs two lines or two circles/arcs".into())); } Coincident { p, q } if p == q => return Err(SketchError::Invalid("a point is always coincident with itself".into())), _ => {} diff --git a/crates/sketch/src/text.rs b/crates/sketch/src/text.rs index f0f378b0..f1c9d439 100644 --- a/crates/sketch/src/text.rs +++ b/crates/sketch/src/text.rs @@ -74,7 +74,7 @@ pub fn text_geometry(text: &str, at: Vec2, height: f64, angle: f64) -> Result MAX_TEXT_CHARS { return Err(SketchError::TooLarge); } - if !(height.is_finite() && height > 1e-6 && height < 1e6) || !angle.is_finite() || !at.is_finite() { + if !(height.is_finite() && height > 1e-6 && height < 1e6 && angle.is_finite() && at.is_finite()) { return Err(SketchError::Invalid("text height must be positive".into())); } let face = ttf_parser::Face::parse(FONT, 0).map_err(|e| SketchError::Invalid(format!("font: {e}")))?; diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index d1382537..27912c30 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -480,10 +480,12 @@ impl Dialog { .collect::>() }) .unwrap_or_else(|| vec![sel.clone()]), + (Kind::Measure { .. }, _) => vec![crate::viewport::measurement_selection(s, sel)], + _ if inp.multi => crate::viewport::grouped_selection(s, sel), _ => vec![sel.clone()], }; for x in picked { - if !inp.items.contains(&x) { + if !inp.items.iter().any(|y| selection::same_item(y, &x)) { inp.items.push(x); } } @@ -631,8 +633,11 @@ impl Dialog { /// A pick in the viewport (already accepted by the active input). pub fn pick(&mut self, s: &Session, sel: Sel) { + let measure = matches!(self.kind, Kind::Measure { .. }); + let sel = if measure { crate::viewport::measurement_selection(s, &sel) } else { sel }; let chain = matches!(self.kind, Kind::Fillet { chain: true, .. }); - let mut picked = vec![sel.clone()]; + let mut picked = + if !measure && self.active_input().is_some_and(|i| i.multi) { crate::viewport::grouped_selection(s, &sel) } else { vec![sel.clone()] }; // Tangent chain: an edge brings the edges that continue it smoothly. if chain && let Sel::Edge { body, index, .. } = &sel @@ -667,13 +672,15 @@ impl Dialog { } /// Box selection result: replace (or add to) the active input. - pub fn take_box(&mut self, sels: Vec, add: bool) { + pub fn take_box(&mut self, s: &Session, sels: Vec, add: bool) { + let measure = matches!(self.kind, Kind::Measure { .. }); let Some(inp) = self.inputs.get_mut(self.active) else { return }; if !add { inp.items.clear(); } for x in sels { - if fits(inp.accept, &x) && !inp.items.contains(&x) { + let x = if measure { crate::viewport::measurement_selection(s, &x) } else { x }; + if fits(inp.accept, &x) && !inp.items.iter().any(|y| selection::same_item(y, &x)) { inp.items.push(x); } } @@ -1848,7 +1855,7 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) { Err(e) => d.error = Some(e), } } - if !(keep && !cancel) { + if !keep || cancel { crate::dialogs_motion::closed(app, &mut d, applied); } if keep && !cancel { diff --git a/crates/ui-egui/src/dim_view.rs b/crates/ui-egui/src/dim_view.rs index 66382a4c..105529eb 100644 --- a/crates/ui-egui/src/dim_view.rs +++ b/crates/ui-egui/src/dim_view.rs @@ -187,7 +187,7 @@ pub fn show(app: &mut SolveApp, ui: &egui::Ui, painter: &egui::Painter, proj: &P } let tk = Tokens::get(); let is_sel = |id: &str| app.session.selection.iter().any(|s| matches!(s, Sel::SketchConstraint { id: x } if x == id)); - let hover = ui.input(|i| i.pointer.hover_pos()).filter(|p| proj.rect.contains(*p)).and_then(hit); + let hover = ui.input(|i| i.pointer.hover_pos()).filter(|_| ui.rect_contains_pointer(proj.rect)).and_then(hit); let to = |p: Vec2| proj.to_screen(ss.plane.to_world(p)); let font = FontId::proportional(12.0); let centres = chain_centres(sk); diff --git a/crates/ui-egui/src/gizmo.rs b/crates/ui-egui/src/gizmo.rs index 5f1b1eb1..8c211e6a 100644 --- a/crates/ui-egui/src/gizmo.rs +++ b/crates/ui-egui/src/gizmo.rs @@ -255,7 +255,7 @@ pub fn show(ui: &mut egui::Ui, painter: &egui::Painter, proj: &Proj, id: egui::I } let state: Option = ui.data(|d| d.get_temp(id)); let pointer = ui.input(|i| i.pointer.hover_pos()); - let hovered = if state.is_none() { pointer.filter(|p| proj.rect.contains(*p)).and_then(|p| hit(&sh, p)) } else { None }; + let hovered = if state.is_none() { pointer.filter(|_| ui.rect_contains_pointer(proj.rect)).and_then(|p| hit(&sh, p)) } else { None }; let active = state.map(|s| s.handle).or(hovered); let hot = |h: Handle| active == Some(h); let col = |h: Handle, i: usize| if hot(h) { tk.accent } else { axis_color(i) }; diff --git a/crates/ui-egui/src/selection.rs b/crates/ui-egui/src/selection.rs index 413f660d..cfbfe9fe 100644 --- a/crates/ui-egui/src/selection.rs +++ b/crates/ui-egui/src/selection.rs @@ -8,6 +8,15 @@ use solvecraft_engine::geom::Vec3; use crate::viewport::Hit; +/// Model picks retain an on-surface point, but their identity is the body and topology index. +pub(crate) fn same_item(a: &Sel, b: &Sel) -> bool { + match (a, b) { + (Sel::Edge { body: a, index: i, .. }, Sel::Edge { body: b, index: j, .. }) + | (Sel::Face { body: a, index: i, .. }, Sel::Face { body: b, index: j, .. }) => a == b && i == j, + _ => a == b, + } +} + /// What a selection input accepts (bit set). pub type Accept = u16; pub const PROFILES: Accept = 1; @@ -73,11 +82,11 @@ impl SelInput { return; } // A pick of something already in (or a chain fully in) takes it out. - if picked.iter().all(|p| self.items.contains(p)) { - self.items.retain(|x| !picked.contains(x)); + if picked.iter().all(|p| self.items.iter().any(|x| same_item(x, p))) { + self.items.retain(|x| !picked.iter().any(|p| same_item(x, p))); } else { for p in picked { - if !self.items.contains(&p) { + if !self.items.iter().any(|x| same_item(x, &p)) { self.items.push(p); } } diff --git a/crates/ui-egui/src/sketch_tools.rs b/crates/ui-egui/src/sketch_tools.rs index 2f87a9fc..39f02e8f 100644 --- a/crates/ui-egui/src/sketch_tools.rs +++ b/crates/ui-egui/src/sketch_tools.rs @@ -1090,7 +1090,7 @@ fn snap_hint(app: &SolveApp, painter: &egui::Painter, proj: &Proj) { /// Per frame, after the viewport is drawn: constraint glyphs and the text-entry box. pub fn show(app: &mut SolveApp, ui: &egui::Ui, painter: &egui::Painter, proj: &Proj) { SHIFT.with(|c| c.set(ui.input(|i| i.modifiers.shift))); - glyphs(app, painter, proj, ui.input(|i| i.pointer.hover_pos())); + glyphs(app, painter, proj, ui.input(|i| i.pointer.hover_pos()).filter(|_| ui.rect_contains_pointer(proj.rect))); snap_hint(app, painter, proj); combs(app, painter, proj); if DEFERRED.with(|d| d.replace(false)) { diff --git a/crates/ui-egui/src/viewport.rs b/crates/ui-egui/src/viewport.rs index 2dfbd1e8..af5f3fbb 100644 --- a/crates/ui-egui/src/viewport.rs +++ b/crates/ui-egui/src/viewport.rs @@ -1091,7 +1091,14 @@ fn build_highlight(app: &SolveApp) -> GpuScene { let t = Tokens::get(); let mut sc = GpuScene::default(); let st = app.session.world_state(); - let sel = app.highlighted(); + let mut sel = Vec::new(); + for item in app.highlighted() { + for piece in grouped_selection(&app.session, &item) { + if !sel.iter().any(|x| crate::selection::same_item(x, &piece)) { + sel.push(piece); + } + } + } let hover = app.viewport.hover.as_ref(); let size = origin_size(app.cam.half_height()); // Origin planes and construction planes. @@ -1242,9 +1249,12 @@ fn build_highlight(app: &SolveApp) -> GpuScene { if let Some(b) = st.body(body) { let k = t.hover_face_lift; let c = body_rgb(app, body); - face_tris(&mut sc, &b.mesh(), *index, [c.0.saturating_add(k), c.1.saturating_add(k), c.2.saturating_add(k), 255], true); let m = b.mesh(); - for e in m.face_edges(u32::try_from(*index).unwrap_or(u32::MAX)) { + let patch = m.surface_patch(*index); + for &f in &patch { + face_tris(&mut sc, &m, f, [c.0.saturating_add(k), c.1.saturating_add(k), c.2.saturating_add(k), 255], true); + } + for (e, _) in m.edge_faces.iter().enumerate().filter(|(_, fs)| fs.iter().any(|f| patch.contains(&(*f as usize)))) { if let Some(p) = m.edges.get(e) && !m.seams.get(e).copied().unwrap_or(false) { @@ -1254,12 +1264,13 @@ fn build_highlight(app: &SolveApp) -> GpuScene { } } Some(Hit::Edge { body, index, mid }) => { - if let Some(b) = st.body(body) - && let Some(e) = edge_of(&b.mesh(), *index, *mid) - { - let selected = sel.iter().any(|x| matches!(x, Sel::Edge { point, .. } if point.dist(*mid) < 1e-9)); - if !selected { - edge_lines(&mut sc, e, t.hover_edge, t.hover_edge_halo, 2.0); + if let Some(b) = st.body(body) { + let m = b.mesh(); + for i in m.circular_rim(*index) { + let selected = sel.iter().any(|x| matches!(x, Sel::Edge { body: b, index: j, .. } if b == body && *j == i)); + if !selected && let Some(e) = if i == *index { edge_of(&m, i, *mid) } else { m.edges.get(i) } { + edge_lines(&mut sc, e, t.hover_edge, t.hover_edge_halo, 2.0); + } } } } @@ -1286,7 +1297,7 @@ fn body_rgb(app: &SolveApp, name: &str) -> (u8, u8, u8) { (c.r(), c.g(), c.b()) } -fn polyline_mid(pts: &[Vec3]) -> Vec3 { +pub(crate) fn polyline_mid(pts: &[Vec3]) -> Vec3 { let len: f64 = pts.windows(2).map(|w| w[0].dist(w[1])).sum(); let mut acc = 0.0; for w in pts.windows(2) { @@ -1360,7 +1371,8 @@ pub fn show(app: &mut SolveApp, ui: &mut egui::Ui) { let secondary_down = secondary_down && app.viewport.context_menu.is_none(); // The view cube handles its own clicks; the model underneath must not see them. let cube = Rect::from_center_size(pos2(rect.right() - 80.0, rect.top() + 80.0), vec2(150.0, 150.0)); - let inside = hover.is_some_and(|p| rect.contains(p) && !cube.contains(p)); + // Floating windows and menus own input over the model, including wheel navigation. + let inside = resp.contains_pointer() && hover.is_some_and(|p| !cube.contains(p)); app.viewport.mouse = hover.filter(|_| inside); // Ctrl or Alt held: drawing and dragging don't snap. crate::drag_snap::set_suppressed(mods.ctrl || mods.alt || mods.command); @@ -1733,12 +1745,13 @@ pub fn show(app: &mut SolveApp, ui: &mut egui::Ui) { fn select(app: &mut SolveApp, sel: Option, add: bool) { match sel { Some(x) => { - let item = serde_json::to_value(&x).unwrap_or_default(); - if add && app.session.selection.contains(&x) { - let rest: Vec = app.session.selection.iter().filter(|y| **y != x).cloned().collect(); + let items = grouped_selection(&app.session, &x); + if add && items.iter().all(|x| app.session.selection.iter().any(|y| crate::selection::same_item(x, y))) { + let rest: Vec = + app.session.selection.iter().filter(|y| !items.iter().any(|x| crate::selection::same_item(x, y))).cloned().collect(); let _ = app.run("select.set", json!({ "items": rest })); } else { - let _ = app.run("select.set", json!({"items": [item], "add": add})); + let _ = app.run("select.set", json!({"items": items, "add": add})); } } None if !add => { @@ -1748,6 +1761,75 @@ fn select(app: &mut SolveApp, sel: Option, add: bool) { } } +/// Expand a visible rim or surface into the kernel pieces a command needs. The original +/// face pick keeps its point (used by tools such as Hole and construction geometry). +pub(crate) fn grouped_selection(s: &solvecraft_engine::Session, sel: &Sel) -> Vec { + let st = s.world_state(); + match sel { + Sel::Edge { body, index, .. } => { + if let Some(b) = st.body(body) { + let m = b.mesh(); + return m + .circular_rim(*index) + .into_iter() + .filter_map(|i| m.edges.get(i).map(|e| Sel::Edge { body: body.clone(), index: i, point: polyline_mid(e) })) + .collect(); + } + } + Sel::Face { body, index, .. } => { + if let Some(b) = st.body(body) { + let m = b.mesh(); + let mut patch = m.surface_patch(*index); + patch.sort_by_key(|i| usize::from(i != index)); + return patch + .into_iter() + .filter_map(|i| { + if i == *index { + return Some(sel.clone()); + } + m.triangles.iter().zip(&m.tri_face).find_map(|(t, f)| { + if *f as usize != i { + return None; + } + let [a, b, c] = m.tri(t)?; + Some(Sel::Face { body: body.clone(), index: i, point: (a + b + c) / 3.0 }) + }) + }) + .collect(); + } + } + _ => {} + } + vec![sel.clone()] +} + +/// Measure keeps one entry per visible item, rather than using its two slots for kernel halves. +pub(crate) fn measurement_selection(s: &solvecraft_engine::Session, sel: &Sel) -> Sel { + let pieces = grouped_selection(s, sel); + let mut representative = sel.clone(); + match &mut representative { + Sel::Edge { index, .. } => { + if let Some(Sel::Edge { index: i, .. }) = pieces.first() { + *index = *i; + } + } + Sel::Face { index, .. } => { + if let Some(i) = pieces + .iter() + .filter_map(|p| match p { + Sel::Face { index, .. } => Some(*index), + _ => None, + }) + .min() + { + *index = i; + } + } + _ => {} + } + representative +} + /// What a box selects: sketch entities while sketching, otherwise what the open dialog's /// active input takes (edges, faces or bodies), otherwise bodies. fn box_select(app: &mut SolveApp, proj: &Proj, bx: BoxSel, add: bool) { @@ -1797,7 +1879,9 @@ fn box_select(app: &mut SolveApp, proj: &Proj, bx: BoxSel, add: bool) { let m = b.mesh(); if accept & EDGES != 0 { for (ei, e) in m.edges.iter().enumerate() { - if !m.seams.get(ei).copied().unwrap_or(false) && bx.takes(&to2(e)) { + let rim = m.circular_rim(ei); + let takes = if bx.crossing { bx.takes(&to2(e)) } else { rim.iter().all(|&i| m.edges.get(i).is_some_and(|p| bx.takes(&to2(p)))) }; + if !m.seams.get(ei).copied().unwrap_or(false) && takes { out.push(Sel::Edge { body: b.name.clone(), index: ei, point: polyline_mid(e) }); } } @@ -1814,8 +1898,15 @@ fn box_select(app: &mut SolveApp, proj: &Proj, bx: BoxSel, add: bool) { .find(|(_, tf)| **tf as usize == f) .and_then(|(t, _)| m.tri(t)) .map(|[a, bb, c]| (a + bb + c) / 3.0); + let takes = bx.takes(&to2(&pts)) + && (bx.crossing + || m.surface_patch(f).iter().all(|&face| { + m.face_edges(u32::try_from(face).unwrap_or(u32::MAX)) + .iter() + .all(|&e| m.edges.get(e).is_some_and(|p| bx.takes(&to2(p)))) + })); if let Some(point) = point - && bx.takes(&to2(&pts)) + && takes { out.push(Sel::Face { body: b.name.clone(), index: f, point }); } @@ -1840,8 +1931,17 @@ fn box_select(app: &mut SolveApp, proj: &Proj, bx: BoxSel, add: bool) { } } } + let mut grouped = Vec::new(); + for item in out { + for piece in grouped_selection(&app.session, &item) { + if !grouped.iter().any(|x| crate::selection::same_item(x, &piece)) { + grouped.push(piece); + } + } + } + let out = grouped; if let Some(mut d) = app.dialog.take() { - d.take_box(out, add); + d.take_box(&app.session, out, add); app.dialog = Some(d); return; } @@ -1990,7 +2090,7 @@ fn view_cube(app: &mut SolveApp, ui: &mut egui::Ui, rect: Rect) { let (r, u, b) = cube_basis(&app.cam); let (wr, wu, _) = app.cam.basis(); let painter = ui.painter_at(rect); - let hover = ui.input(|i| i.pointer.hover_pos()); + let hover = ui.input(|i| i.pointer.hover_pos()).filter(|_| ui.rect_contains_pointer(rect)); let targets = cube_targets(); let shapes = cube_projected(&targets, (r, u, b), c); let hovered = hover.and_then(|h| cube_hit(&shapes, h)); @@ -2389,6 +2489,255 @@ mod tests { assert!(app.viewport.hover.is_some()); } + fn cylinder_app() -> SolveApp { + let mut s = solvecraft_engine::Session::default(); + s.execute("solid.cylinder", &json!({"radius": 10, "height": 20})).unwrap(); + let mut app = SolveApp::new(s, crate::Services::default()); + app.ui.show_origin = false; + app + } + + #[test] + fn circular_items_measure_as_whole_rims_and_surfaces() { + let mut app = cylinder_app(); + let st = app.session.world_state(); + let b = &st.bodies[0]; + let m = b.mesh(); + let edges: Vec = m + .edges + .iter() + .enumerate() + .filter(|(_, p)| p.len() > 3 && p.iter().all(|q| (q.z - 20.0).abs() < 1e-6)) + .map(|(index, p)| Sel::Edge { body: b.name.clone(), index, point: polyline_mid(p) }) + .collect(); + let result = app.session.measure_items(&edges); + assert_eq!(result["items"].as_array().unwrap().len(), 1); + assert!((result["items"][0]["length_mm"].as_f64().unwrap() - 20.0 * std::f64::consts::PI).abs() < 0.2, "{result}"); + app.session.selection = edges; + let d = crate::dialogs::Dialog::for_command(&app, "inspect.measure").unwrap(); + assert_eq!(d.items().len(), 1); + + let faces = m.edge_faces.iter().zip(&m.seams).find(|(_, seam)| **seam).unwrap().0; + let wall = Sel::Face { body: b.name.clone(), index: faces[0] as usize, point: Vec3::new(10.0, 0.0, 10.0) }; + let wall_pieces = grouped_selection(&app.session, &wall); + let result = app.session.measure_items(&wall_pieces); + assert_eq!(result["items"].as_array().unwrap().len(), 1); + assert!((result["items"][0]["area_mm2"].as_f64().unwrap() - 400.0 * std::f64::consts::PI).abs() < 10.0, "{result}"); + let mut d = crate::dialogs::Dialog::for_command(&app, "inspect.measure").unwrap(); + d.pick(&app.session, wall.clone()); + assert_eq!(d.items().len(), 2, "a rim and a wall are two measurement targets"); + d.pick(&app.session, Sel::Face { body: b.name.clone(), index: faces[1] as usize, point: Vec3::new(-10.0, 0.0, 10.0) }); + assert_eq!(d.items().len(), 1, "the other half toggles the same wall"); + } + + #[test] + fn circular_rim_hover_click_and_dialog_pick_include_both_halves() { + let mut app = cylinder_app(); + let st = app.session.world_state(); + let b = &st.bodies[0]; + let m = b.mesh(); + let top = m.edges.iter().position(|e| e.len() > 3 && e.iter().all(|p| (p.z - 20.0).abs() < 1e-6)).unwrap(); + let rim = m.circular_rim(top); + assert_eq!(rim.len(), 2); + let line_count: usize = rim.iter().map(|&i| m.edges[i].len() - 1).sum(); + for &i in &rim { + let mid = polyline_mid(&m.edges[i]); + app.viewport.hover = Some(Hit::Edge { body: b.name.clone(), index: i, mid }); + assert_eq!(build_highlight(&app).lines.len(), 2 * line_count * crate::gpu::LINE_SIZE); + let item = hit_sel(app.viewport.hover.as_ref().unwrap()).unwrap(); + select(&mut app, Some(item.clone()), false); + assert_eq!(app.session.selection.len(), 2); + // Selecting both pieces must not draw each highlight twice. + assert_eq!(build_highlight(&app).lines.len(), 2 * line_count * crate::gpu::LINE_SIZE); + // Ctrl/Shift clicking either half deselects the entire rim. + select(&mut app, Some(item.clone()), true); + assert!(app.session.selection.is_empty()); + let mut d = crate::dialogs::Dialog::for_command(&app, "solid.chamfer").unwrap(); + if let crate::dialogs::Kind::Fillet { chain, .. } = &mut d.kind { + *chain = false; + } + d.pick(&app.session, item.clone()); + assert_eq!(d.items().len(), 2); + d.pick(&app.session, item); + assert!(d.items().is_empty()); + } + // Screen-space picking works from either side of the rim as the model turns. + for &i in &rim { + let mid = polyline_mid(&m.edges[i]); + app.cam = app.cam.looking_from(Vec3::new(mid.x, mid.y, 20.0)); + app.cam.target = Vec3::new(0.0, 0.0, 10.0); + for perspective in [false, true] { + app.ui.perspective = perspective; + let proj = projection(&app, Rect::from_min_size(pos2(0.0, 0.0), vec2(800.0, 600.0))); + let hits = pick(&app, &proj, proj.to_screen(mid).unwrap()); + let (_, sel) = candidate(&app, &hits).unwrap(); + assert!(matches!(sel, Sel::Edge { .. }), "{hits:?}"); + assert_eq!(grouped_selection(&app.session, &sel).len(), 2); + } + } + } + + #[test] + fn curved_face_hover_selection_and_dialogs_cross_the_seam() { + let mut app = cylinder_app(); + let st = app.session.world_state(); + let b = &st.bodies[0]; + let m = b.mesh(); + let faces = m.edge_faces.iter().zip(&m.seams).find(|(_, seam)| **seam).unwrap().0; + assert_eq!(faces.len(), 2); + let triangle_count = m.tri_face.iter().filter(|f| faces.contains(f)).count(); + for &f in faces { + let index = f as usize; + let t = m.triangles.iter().zip(&m.tri_face).find(|(_, tf)| **tf == f).unwrap().0; + let [a, c, d] = m.tri(t).unwrap(); + let point = (a + c + d) / 3.0; + app.viewport.hover = Some(Hit::Face { body: b.name.clone(), index, point }); + assert_eq!(build_highlight(&app).tris.len(), triangle_count * 3 * crate::gpu::TRI_SIZE); + let item = hit_sel(app.viewport.hover.as_ref().unwrap()).unwrap(); + select(&mut app, Some(item.clone()), false); + assert_eq!(app.session.selection.len(), 2); + assert_eq!(build_highlight(&app).tris.len(), triangle_count * 3 * crate::gpu::TRI_SIZE); + assert_eq!(app.session.selection.first(), Some(&item), "keep the original tool placement point first"); + let mut dialog = crate::dialogs::Dialog::for_command(&app, "solid.chamfer").unwrap(); + assert_eq!(dialog.items().len(), 2); + dialog.pick(&app.session, item.clone()); + assert!(dialog.items().is_empty()); + dialog.pick(&app.session, item); + assert_eq!(dialog.items().len(), 2); + // A point elsewhere on the opposite half still toggles the whole surface off. + let other = faces.iter().find(|&&x| x != f).unwrap(); + select(&mut app, Some(Sel::Face { body: b.name.clone(), index: *other as usize, point: m.positions[0] }), true); + assert!(app.session.selection.is_empty()); + } + // Planar caps and box corners stay separate. + let cap = (0..b.body.face_count()).find(|&i| m.surface_patch(i).len() == 1).unwrap(); + assert_eq!(m.surface_patch(cap), vec![cap]); + app.session.execute("solid.box", &json!({"length": 10, "width": 10, "height": 10})).unwrap(); + let st = app.session.world_state(); + let box_body = st.bodies.last().unwrap(); + for i in 0..box_body.body.face_count() { + assert_eq!(box_body.mesh().surface_patch(i), vec![i]); + } + } + + #[test] + fn shortcuts_window_wheel_does_not_zoom_the_viewport() { + let ctx = egui::Context::default(); + let mut app = SolveApp::new(solvecraft_engine::Session::default(), crate::Services::default()); + app.viewport.no_pixels = true; + crate::help::run(&mut app, &ctx, "shortcuts"); + let mut time = 0.0; + let mut frame = |app: &mut SolveApp, pointer: Option, wheel: f32| { + time += 1.0 / 60.0; + let mut events = Vec::new(); + if let Some(p) = pointer { + events.push(egui::Event::PointerMoved(p)); + } + if wheel != 0.0 { + events.push(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: vec2(0.0, wheel), + modifiers: egui::Modifiers::NONE, + phase: egui::TouchPhase::Move, + }); + } + let _ = ctx.run_ui( + egui::RawInput { + screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(1100.0, 800.0))), + time: Some(time), + events, + ..Default::default() + }, + |ui| { + egui::CentralPanel::default().show(ui, |ui| show(app, ui)); + crate::prefs::show(app, ui.ctx()); + }, + ); + }; + // Let egui establish the floating window's size and hit-test widgets. + for _ in 0..4 { + frame(&mut app, None, 0.0); + } + let window = ctx.memory(|m| m.area_rect(egui::Id::new("sc_prefs"))).unwrap(); + let list = window.center() + vec2(140.0, 0.0); + let height = app.cam.half_height(); + let target = app.cam.target; + frame(&mut app, Some(list), 0.0); + for n in 0..12 { + frame(&mut app, Some(list), if n == 0 { -120.0 } else { 0.0 }); + assert_eq!(app.cam.half_height(), height, "wheel input over shortcuts must stay in the popup"); + assert_eq!(app.cam.target, target); + } + // The window's non-scrolling sidebar must also shield the model. + frame(&mut app, Some(window.left_center() + vec2(30.0, 0.0)), -120.0); + assert_eq!(app.cam.half_height(), height); + // An exposed part of the viewport still accepts wheel navigation with the popup open. + let exposed = pos2(40.0, 700.0); + assert!(!window.contains(exposed)); + frame(&mut app, Some(exposed), 0.0); + frame(&mut app, Some(exposed), -120.0); + assert_ne!(app.cam.half_height(), height); + // Closing the popup returns its former area to the viewport. + app.prefs_window.open = false; + for _ in 0..12 { + frame(&mut app, Some(list), 0.0); + } + let height = app.cam.half_height(); + frame(&mut app, Some(list), -120.0); + assert_ne!(app.cam.half_height(), height); + } + + #[test] + fn floating_dialog_blocks_viewport_hover_and_clicks() { + let mut h = crate::scenario::Harness::new(); + h.call("engine.execute", json!({"command": "solid.box", "params": {"length": 40, "width": 30, "height": 20}})); + h.call("ui.view", json!({"view": "fit"})); + h.app.ui.pick_bodies = true; + let p = h.app.viewport.rect.unwrap().center(); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert!(h.app.viewport.hover.is_some(), "the test point must hover over the model"); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert!(!h.app.session.selection.is_empty(), "the uncovered model must be selectable"); + h.app.session.selection.clear(); + + h.call("ui.prefs", json!({"open": "Navigation"})); + let window = h.ctx.memory(|m| m.area_rect(egui::Id::new("sc_prefs"))).unwrap(); + assert!(window.contains(p), "the dialog must cover the model at the test point"); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert!(h.app.viewport.hover.is_none(), "the dialog must block model highlights"); + assert!(h.app.viewport.mouse.is_none()); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert!(h.app.session.selection.is_empty(), "clicks in the dialog must not select the model"); + + h.call("ui.prefs", json!({"button": "cancel"})); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert!(h.app.viewport.hover.is_some(), "closing the dialog must restore model highlights"); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert!(!h.app.session.selection.is_empty()); + } + + #[test] + fn floating_dialog_blocks_view_cube_highlights_and_clicks() { + let mut h = crate::scenario::Harness::new(); + h.call("ui.resize", json!({"width": 800, "height": 700})); + let p = crate::scenario::handle_at("cube:top").unwrap(); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert_eq!(crate::scenario::count_of("cube_highlights"), Some(1.0)); + h.call("ui.prefs", json!({"open": "Navigation"})); + let window = h.ctx.memory(|m| m.area_rect(egui::Id::new("sc_prefs"))).unwrap(); + assert!(window.contains(p), "the floating dialog must cover the cube target"); + let camera = serde_json::to_value(h.app.cam).unwrap(); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert_eq!(crate::scenario::count_of("cube_highlights"), Some(0.0)); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert_eq!(serde_json::to_value(h.app.cam).unwrap(), camera, "dialog clicks must not turn the view cube"); + h.call("ui.prefs", json!({"button": "cancel"})); + h.call("ui.move", json!({"x": p.x, "y": p.y})); + assert_eq!(crate::scenario::count_of("cube_highlights"), Some(1.0)); + h.call("ui.click", json!({"x": p.x, "y": p.y})); + assert_ne!(serde_json::to_value(h.app.cam).unwrap(), camera); + } + #[test] fn section_outlines_follow_the_cut() { let mut s = solvecraft_engine::Session::default(); diff --git a/docs/mcp.md b/docs/mcp.md index 3aec985e..63370d71 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -69,8 +69,11 @@ Errors: an unknown tool or arguments that don't match the tool's schema are JSON `-32602` errors; an engine failure (bad command parameters, a feature that fails) is a tool result with `isError: true` and the engine's message, which the model can read and correct. -`batch` rollback undoes as many steps as the batch added to the undo history; batches that -themselves call `edit.undo`, `file.new` or `doc.open` are not rolled back exactly. +`batch` rollback goes back to an engine checkpoint (`edit.checkpoint`) taken before the first +step: the design, the active sketch and the undo history are restored exactly as they were, +however long the history is. Batches that call `file.new` or `doc.open` get the design back, +not its file path. (An older app without checkpoints falls back to undoing as many steps as +the batch added to the undo history.) ## Demo: an enclosure built over MCP diff --git a/xtask/src/layers.rs b/xtask/src/layers.rs index 0a65c1e8..29abb446 100644 --- a/xtask/src/layers.rs +++ b/xtask/src/layers.rs @@ -163,11 +163,10 @@ pub fn check(crates: &[Crate]) -> Vec { match classify(&d.name) { // Unregistered deps are reported on their own entry. None => {} - Some(Class::Testkit) => { - if d.kind != DepKind::Dev { - out.push(Violation::TestkitAsNormalDep { krate: c.name.clone() }); - } + Some(Class::Testkit) if d.kind != DepKind::Dev => { + out.push(Violation::TestkitAsNormalDep { krate: c.name.clone() }); } + Some(Class::Testkit) => {} Some(dc) => { let to = dc.layer().unwrap_or(u8::MAX); if to >= layer && !(to == layer && intra_layer_allowed(&c.name, &d.name)) {