Edit Sketch no longer shows bodies a later feature deletes (#1329)

Fixes #1167. While a sketch is edited, a body that a rolled-off, unsuppressed Remove deletes is not shown (unless the sketch is built on it); the history stays rolled back, only the view changes. Drawing, picking, screenshots, thumbnails and the browser's Bodies folder agree; document.inspect reports removed_later. Checked on main + this branch: cargo check --workspace, the engine sketch_edit_removed/occurrences tests and the UI viewport/browser tests.

crates/engine/src/cmd/inspect.rs

@@ -245,6 +245,10 @@ fn inspect(s: &mut Session, p: &Value) -> Result<Value> {
             let mut v = if with_measure { measure_json(b) } else { json!({"name": b.name, "feature": b.feature}) };
             if let Some(o) = v.as_object_mut() {
                 o.insert("visible".into(), json!(s.body_visible(&b.name)));
+                if s.body_removed_later(&b.name) {
+                    // Editing a sketch rolls the timeline back; this body is deleted further on.
+                    o.insert("removed_later".into(), json!(true));
+                }
             }
             v
         })

crates/engine/src/lib.rs

@@ -219,6 +219,7 @@ pub struct Session {
     pub pending_moves: std::collections::BTreeMap<u64, solidcraft_doc::Mat>,
     world_cache: std::sync::Mutex<Option<WorldCache>>,
     occurrence_visibility_cache: std::sync::Mutex<Option<occurrences::VisibilityCache>>,
+    removed_later_cache: std::sync::Mutex<Option<occurrences::RemovedLaterCache>>,
     /// Features copied with `timeline.copy` (pasted by `timeline.paste`).
     pub clipboard: Vec<solidcraft_doc::Feature>,
     /// Section Analysis: a view cut by a plane (origin, unit normal; the normal side is hidden).
@@ -323,6 +324,7 @@ impl Session {
             pending_moves: Default::default(),
             world_cache: Default::default(),
             occurrence_visibility_cache: Default::default(),
+            removed_later_cache: Default::default(),
             clipboard: Vec::new(),
             section: None,
             analysis: None,
@@ -788,6 +790,7 @@ impl Session {
             pending_moves: self.pending_moves.clone(),
             world_cache: std::sync::Mutex::new(self.world_cache.lock().ok().and_then(|cache| cache.clone())),
             occurrence_visibility_cache: Default::default(),
+            removed_later_cache: Default::default(),
             clipboard: self.clipboard.clone(),
             section: None,
             analysis: None,
@@ -946,6 +949,8 @@ mod profile_curves_tests;
 #[cfg(test)]
 mod profile_refs_tests;
 #[cfg(test)]
+mod sketch_edit_removed_tests;
+#[cfg(test)]
 mod split_face_tests;
 #[cfg(test)]
 mod tests;

crates/engine/src/occurrences.rs

@@ -1,6 +1,6 @@
 //! Exact placement identity and session view/authoring state.
 use crate::{EngineError, Result, Sel, Session};
-use solidcraft_doc::{IDENTITY, Mat, mat_mul};
+use solidcraft_doc::{FeatureKind, IDENTITY, Mat, PlaneRef, mat_mul};
 
 #[derive(Clone, Debug)]
 pub struct OccurrenceBody {
@@ -31,6 +31,14 @@ pub(crate) struct VisibilityCache {
     bodies: std::collections::BTreeSet<String>,
 }
 
+/// [`Session::body_removed_later`] for one revision, sketch and history marker.
+pub(crate) struct RemovedLaterCache {
+    revision: u64,
+    sketch: u64,
+    marker: usize,
+    bodies: std::collections::BTreeSet<String>,
+}
+
 impl Session {
     /// Document with uncaptured occurrence positions applied, without changing saved geometry.
     pub fn placed_document(&self) -> std::borrow::Cow<'_, solidcraft_doc::Document> {
@@ -183,7 +191,7 @@ impl Session {
     }
 
     pub fn body_visible(&self, name: &str) -> bool {
-        if self.visibility.hidden_bodies.iter().any(|n| n == name) {
+        if self.visibility.hidden_bodies.iter().any(|n| n == name) || self.body_removed_later(name) {
             return false;
         }
         if self.visibility.hidden_occurrences.is_empty() {
@@ -205,6 +213,66 @@ impl Session {
         self.doc.body_frames(&self.model.state()).iter().find(|(n, _, _, _)| n == name).is_none_or(|(_, _, path, _)| self.occurrence_visible(path))
     }
 
+    /// Is this body deleted by a feature the history marker has rolled off, while a sketch is
+    /// being edited? Edit Sketch rolls the timeline back to the sketch, so a body made before the
+    /// sketch and removed after it is part of the shown model again; it stays out of the view,
+    /// as it does once the timeline runs to the end. Nothing else changes: the model, its state
+    /// and every command still see the history as it was rolled to. A body the sketch is
+    /// built on (the face it sits on, an edge it projects) stays.
+    pub fn body_removed_later(&self, name: &str) -> bool {
+        let (Some(sketch), Some(marker)) = (self.active_sketch, self.doc.marker) else { return false };
+        if marker >= self.doc.features.len() {
+            return false;
+        }
+        let Ok(mut cache) = self.removed_later_cache.lock() else { return false };
+        if cache.as_ref().is_none_or(|c| c.revision != self.revision || c.sketch != sketch || c.marker != marker) {
+            *cache = Some(RemovedLaterCache { revision: self.revision, sketch, marker, bodies: self.removed_later_bodies(sketch, marker) });
+        }
+        cache.as_ref().is_some_and(|c| c.bodies.contains(name))
+    }
+
+    /// The names (as shown in the world) of the bodies that features from `marker` on remove,
+    /// leaving out the bodies `sketch` is built on.
+    fn removed_later_bodies(&self, sketch: u64, marker: usize) -> std::collections::BTreeSet<String> {
+        let none = std::collections::BTreeSet::new();
+        let Some(FeatureKind::Sketch { plane, sketch: made }) = self.doc.feature(sketch).map(|f| &f.kind) else { return none };
+        let mut kept: std::collections::BTreeSet<&str> = std::collections::BTreeSet::new();
+        let mut plane = plane;
+        loop {
+            match plane {
+                PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => plane = base,
+                // A face attachment that names no exact source: which body it sits on is unknown,
+                // so none is taken from the view.
+                PlaneRef::Face { anchor: None, .. } => return none,
+                PlaneRef::Face { anchor: Some(anchor), .. } => {
+                    kept.insert(anchor.target.body.as_str());
+                    break;
+                }
+                _ => break,
+            }
+        }
+        for link in &made.links {
+            kept.extend(link.source.bodies());
+        }
+        let shown = self.model.state();
+        let mut removed = std::collections::BTreeSet::new();
+        for feature in self.doc.features.iter().skip(marker).filter(|f| !f.suppressed) {
+            if let FeatureKind::Remove { bodies } = &feature.kind {
+                removed.extend(bodies.iter().filter(|b| !kept.contains(b.as_str()) && shown.body(b).is_some()).cloned());
+            }
+        }
+        if removed.is_empty() || self.doc.occurrences.is_empty() {
+            return removed;
+        }
+        // A component's bodies are shown once per placement, each under its own name.
+        self.doc
+            .body_frames_with_source(&shown)
+            .into_iter()
+            .filter(|(_, source, _, _, _)| removed.contains(source))
+            .map(|(world, ..)| world)
+            .collect()
+    }
+
     pub(crate) fn prune_occurrence_state(&mut self) {
         self.pending_moves.retain(|id, _| self.doc.occurrences.iter().any(|o| o.id == *id));
         if self.active_component != 0 && !self.doc.components.iter().any(|c| c.id == self.active_component) {

crates/engine/src/sketch_edit_removed_tests.rs

@@ -0,0 +1,161 @@
+//! Edit Sketch rolls the timeline back to the sketch. A body made before the sketch and deleted
+//! after it is then part of the model again, but it must not be shown while the sketch is edited
+//! (#1167): the history is rolled back, the view of it is not. A body the sketch is built on stays.
+
+use serde_json::{Value, json};
+use solidcraft_geom::Vec3;
+use solidcraft_sketch::LinkSource;
+
+use crate::Session;
+
+fn run(s: &mut Session, id: &str, p: Value) -> Value {
+    match s.execute(id, &p) {
+        Ok(v) => v,
+        Err(e) => panic!("{id} {p}: {e}"),
+    }
+}
+
+/// A 10 x 10 x 5 block `name`, drawn on the XY plane at `x`.
+fn block(s: &mut Session, name: &str, x: f64) {
+    run(s, "sketch.create", json!({"plane": "XY", "name": format!("{name}Sketch")}));
+    run(s, "sketch.rectangle.two_point", json!({"p0": [x, 0], "p1": [x + 10.0, 10]}));
+    run(s, "sketch.finish", json!({}));
+    run(s, "solid.extrude", json!({"sketch": format!("{name}Sketch"), "distance": 5, "body_name": name}));
+}
+
+/// Whether `inspect` lists the body, and what it says of it.
+fn inspected(s: &mut Session, body: &str) -> Value {
+    let r = run(s, "document.inspect", json!({"measure": false, "sketches": false}));
+    r["bodies"].as_array().and_then(|bodies| bodies.iter().find(|b| b["name"] == body)).cloned().unwrap_or(Value::Null)
+}
+
+/// `Kept` and `Gone` (removed after the sketch `Edited`), then Edit Sketch on `Edited`.
+fn edit_with_a_later_removal(s: &mut Session) {
+    block(s, "Kept", 0.0);
+    block(s, "Gone", 20.0);
+    run(s, "sketch.create", json!({"plane": "XY", "offset": "30 mm", "name": "Edited"}));
+    run(s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [5, 5]}));
+    run(s, "sketch.finish", json!({}));
+    run(s, "solid.remove", json!({"bodies": ["Gone"]}));
+    run(s, "timeline.roll_to", json!({"feature": "Edited"}));
+    run(s, "sketch.edit", json!({"sketch": "Edited"}));
+}
+
+#[test]
+fn a_body_deleted_later_is_not_shown_while_a_sketch_is_edited() {
+    let mut s = Session::default();
+    edit_with_a_later_removal(&mut s);
+    // The rolled-back history still has the body: commands and measurements see it.
+    assert!(s.model.state().body("Gone").is_some());
+    assert!(!s.body_visible("Gone"));
+    assert!(s.body_removed_later("Gone"));
+    let gone = inspected(&mut s, "Gone");
+    assert_eq!(gone["visible"], json!(false), "{gone}");
+    assert_eq!(gone["removed_later"], json!(true), "{gone}");
+    // What is not deleted is shown as before.
+    assert!(s.body_visible("Kept"));
+    let kept = inspected(&mut s, "Kept");
+    assert_eq!(kept["visible"], json!(true), "{kept}");
+    assert!(kept.get("removed_later").is_none(), "{kept}");
+}
+
+#[test]
+fn the_body_is_shown_again_when_the_sketch_is_finished() {
+    let mut s = Session::default();
+    edit_with_a_later_removal(&mut s);
+    run(&mut s, "sketch.finish", json!({}));
+    // Rolled back and not editing: the history as it was, which has the body.
+    assert!(s.body_visible("Gone"));
+    assert!(inspected(&mut s, "Gone").get("removed_later").is_none());
+    // Rolled forward, it is gone.
+    run(&mut s, "timeline.roll_to", json!({}));
+    assert!(s.model.state().body("Gone").is_none());
+}
+
+#[test]
+fn rolling_back_by_hand_shows_the_history() {
+    let mut s = Session::default();
+    block(&mut s, "Gone", 0.0);
+    run(&mut s, "solid.remove", json!({"bodies": ["Gone"]}));
+    run(&mut s, "timeline.roll_to", json!({"position": 2}));
+    assert!(s.body_visible("Gone"));
+}
+
+#[test]
+fn a_suppressed_removal_does_not_hide_the_body() {
+    let mut s = Session::default();
+    edit_with_a_later_removal(&mut s);
+    run(&mut s, "sketch.finish", json!({}));
+    let removal = s.doc.features.iter().find(|f| f.kind.type_name() == "RemoveFeature").map(|f| f.id).unwrap_or(0);
+    run(&mut s, "timeline.suppress", json!({"feature": removal, "suppressed": true}));
+    run(&mut s, "sketch.edit", json!({"sketch": "Edited"}));
+    assert!(s.body_visible("Gone"));
+}
+
+#[test]
+fn a_body_the_sketch_projects_stays_shown() {
+    let mut s = Session::default();
+    block(&mut s, "Gone", 0.0);
+    run(&mut s, "sketch.create", json!({"plane": "XY", "offset": "5 mm", "name": "Edited"}));
+    run(&mut s, "sketch.project", json!({"refs": [{"edge": [10, 5, 5], "body": "Gone"}]}));
+    run(&mut s, "sketch.finish", json!({}));
+    run(&mut s, "solid.remove", json!({"bodies": ["Gone"]}));
+    run(&mut s, "timeline.roll_to", json!({"feature": "Edited"}));
+    run(&mut s, "sketch.edit", json!({"sketch": "Edited"}));
+    assert!(s.body_visible("Gone"));
+}
+
+#[test]
+fn a_body_the_sketch_sits_on_stays_shown() {
+    let mut s = Session::default();
+    block(&mut s, "Gone", 0.0);
+    let top = {
+        let world = s.world_state();
+        let face = world
+            .body("Gone")
+            .and_then(|body| body.body.faces(0.01).ok())
+            .and_then(|faces| faces.into_iter().find(|f| f.plane_normal.is_some_and(|n| n.dot(Vec3::Z) > 0.999)))
+            .unwrap();
+        json!({"face": [face.centroid.x, face.centroid.y, face.centroid.z], "body": "Gone", "index": face.index})
+    };
+    run(&mut s, "sketch.create", json!({"plane": top, "name": "Edited"}));
+    let anchored = match s.doc.find_feature("Edited").map(|f| &f.kind) {
+        Some(solidcraft_doc::FeatureKind::Sketch { plane: solidcraft_doc::PlaneRef::Face { anchor, .. }, .. }) => anchor.is_some(),
+        other => panic!("not attached to a face: {other:?}"),
+    };
+    assert!(anchored, "the face attachment names its body");
+    run(&mut s, "sketch.finish", json!({}));
+    run(&mut s, "solid.remove", json!({"bodies": ["Gone"]}));
+    run(&mut s, "timeline.roll_to", json!({"feature": "Edited"}));
+    run(&mut s, "sketch.edit", json!({"sketch": "Edited"}));
+    assert!(s.body_visible("Gone"));
+}
+
+#[test]
+fn every_placement_of_a_component_body_is_left_out() {
+    let mut s = Session::default();
+    let component = run(&mut s, "component.create", json!({"name": "Block"}))["component"].as_u64().unwrap_or(0);
+    block(&mut s, "Gone", 0.0);
+    run(&mut s, "sketch.create", json!({"plane": "XY", "offset": "30 mm", "name": "Edited"}));
+    run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [5, 5]}));
+    run(&mut s, "sketch.finish", json!({}));
+    run(&mut s, "solid.remove", json!({"bodies": ["Gone"]}));
+    run(&mut s, "occurrence.copy", json!({"component": component, "translate": [50, 0, 0]}));
+    run(&mut s, "timeline.roll_to", json!({"feature": "Edited"}));
+    run(&mut s, "sketch.edit", json!({"sketch": "Edited"}));
+    let world = s.world_state();
+    assert_eq!(world.bodies.len(), 2, "one body, placed twice");
+    for body in &world.bodies {
+        assert!(!s.body_visible(&body.name), "{} is deleted later", body.name);
+    }
+}
+
+#[test]
+fn a_link_reads_the_bodies_of_its_sources() {
+    let at = Vec3::ZERO;
+    let edge = LinkSource::Edge { body: "A".into(), at };
+    let face = LinkSource::Face { body: "B".into(), at };
+    assert_eq!(edge.bodies(), ["A"]);
+    assert_eq!(LinkSource::Intersection { a: Box::new(edge), b: Box::new(face) }.bodies(), ["A", "B"]);
+    assert!(LinkSource::Origin.bodies().is_empty());
+}