Merge main and preserve viewport regression tests

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<u64> {
+        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<Vec<u64>> {
         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<bool> = self.configs.columns.iter().map(|c| !matches!(c, crate::config::Column::Suppress(id) if gone.contains(id))).collect();

crates/doc/src/project.rs

@@ -485,7 +485,7 @@ fn tilted_circle(plane: &Plane, pts: &[Vec3]) -> Option<Vec<LinkGeom>> {
     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 {

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,

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"
+        );
+    }
+}

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<Value> {
     Ok(json!({"redone": label}))
 }
 
+fn checkpoint(s: &mut Session, _p: &Value) -> Result<Value> {
+    Ok(json!({"checkpoint": s.checkpoint()}))
+}
+
+fn restore_checkpoint(s: &mut Session, p: &Value) -> Result<Value> {
+    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<Value> {
     s.model = solvecraft_doc::Model::new();
     s.refresh();

crates/engine/src/cmd/measure_sel.rs

@@ -55,13 +55,13 @@ fn densify(pts: &[Vec3]) -> Vec<Vec3> {
 
 /// 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<Vec<Vec3>> {
+fn edge_index(m: &Mesh, index: usize, point: Vec3) -> Option<usize> {
     let near = |e: &Vec<Vec3>| 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<Vec3>, 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<Vec3>> = 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::<Vec<_>>();
             let area: f64 = tris.iter().map(|[a, b, c]| (*b - *a).cross(*c - *a).len() * 0.5).sum();
             let normals: Vec<Vec3> = 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<Value> {
     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<Sel> = list.iter().map(|v| serde_json::from_value::<Sel>(v.clone()).map_err(|e| bad(cmd, e.to_string()))).collect::<Result<_>>()?;
+    let st = s.world_state();
+    let mut sels = Vec::new();
+    for value in list {
+        let mut sel = serde_json::from_value::<Sel>(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)"))?);

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();

crates/engine/src/lib.rs

@@ -170,6 +170,20 @@ pub struct Session {
     pub analysis: Option<SurfaceAnalysis>,
     /// 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<Checkpoint>,
+    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<Document>,
+    pub active_sketch: Option<u64>,
+    pub undo: Vec<Snapshot>,
+    pub redo: Vec<Snapshot>,
 }
 
 /// 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(),
         }
     }

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::<Vec<_>>());
+    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<String> = 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::<Vec<_>>(), 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();

crates/geom/src/mesh.rs

@@ -82,6 +82,71 @@ impl Mesh {
     pub fn face_edges(&self, f: u32) -> Vec<usize> {
         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<usize> {
+        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<usize> {
+        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<usize> {

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<Vec3> {
+        (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};

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<Value> =
+        (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();

crates/mcp/src/tools.rs

@@ -361,7 +361,56 @@ fn undo_depth(b: &mut dyn Backend) -> Option<u64> {
     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<String, Value>) -> 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<String, Value>) -> 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<String, Value>) -> 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<String, Value>) -> Value {
             }
         }
     }
+    start.finish(b);
     text_result(&json!({"ok": true, "completed": results.len(), "results": results}), false)
 }
 

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())),
             _ => {}

crates/sketch/src/text.rs

@@ -74,7 +74,7 @@ pub fn text_geometry(text: &str, at: Vec2, height: f64, angle: f64) -> Result<Ve
     if text.chars().count() > 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}")))?;

crates/ui-egui/src/dialogs.rs

@@ -480,10 +480,12 @@ impl Dialog {
                                 .collect::<Vec<_>>()
                         })
                         .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<Sel>, add: bool) {
+    pub fn take_box(&mut self, s: &Session, sels: Vec<Sel>, 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 {

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);

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<State> = 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) };

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);
                 }
             }

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)) {

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<Sel>, 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<Sel> = 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<Sel> =
+                    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<Sel>, 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<Sel> {
+    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<Sel> = 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<Pos2>, 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();

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
 

xtask/src/layers.rs

@@ -163,11 +163,10 @@ pub fn check(crates: &[Crate]) -> Vec<Violation> {
                 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)) {