diff --git a/crates/engine/src/cmd/inspect.rs b/crates/engine/src/cmd/inspect.rs index b1d8cb59..b2949845 100644 --- a/crates/engine/src/cmd/inspect.rs +++ b/crates/engine/src/cmd/inspect.rs @@ -245,6 +245,10 @@ fn inspect(s: &mut Session, p: &Value) -> Result { 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 }) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 2d5a4faf..32b2a266 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -219,6 +219,7 @@ pub struct Session { pub pending_moves: std::collections::BTreeMap, world_cache: std::sync::Mutex>, occurrence_visibility_cache: std::sync::Mutex>, + removed_later_cache: std::sync::Mutex>, /// Features copied with `timeline.copy` (pasted by `timeline.paste`). pub clipboard: Vec, /// 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; diff --git a/crates/engine/src/occurrences.rs b/crates/engine/src/occurrences.rs index 7eeea30d..2592e375 100644 --- a/crates/engine/src/occurrences.rs +++ b/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, } +/// [`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, +} + 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 { + 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) { diff --git a/crates/engine/src/sketch_edit_removed_tests.rs b/crates/engine/src/sketch_edit_removed_tests.rs new file mode 100644 index 00000000..29733105 --- /dev/null +++ b/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()); +} diff --git a/crates/sketch/src/link.rs b/crates/sketch/src/link.rs index 005578d4..2d8c8422 100644 --- a/crates/sketch/src/link.rs +++ b/crates/sketch/src/link.rs @@ -108,6 +108,31 @@ impl LinkSource { LinkSource::Text { text, .. } => format!("text \"{}\"", text.chars().take(20).collect::()), } } + + /// The bodies this source reads geometry from (an intersection reads two). + pub fn bodies(&self) -> Vec<&str> { + match self { + LinkSource::Edge { body, .. } + | LinkSource::Face { body, .. } + | LinkSource::Body { body } + | LinkSource::Vertex { body, .. } + | LinkSource::OnSurface { body, .. } + | LinkSource::Iso { body, .. } + | LinkSource::Spun { body, .. } => vec![body.as_str()], + LinkSource::Intersection { a, b } => { + let mut bodies = a.bodies(); + bodies.extend(b.bodies()); + bodies + } + LinkSource::SketchCurve { .. } + | LinkSource::SketchPoint { .. } + | LinkSource::Origin + | LinkSource::Axis { .. } + | LinkSource::SketchAxis { .. } + | LinkSource::Plane { .. } + | LinkSource::Text { .. } => Vec::new(), + } + } } /// Half length of the line a sketch axis link produces. diff --git a/crates/ui-egui/src/browser.rs b/crates/ui-egui/src/browser.rs index 894a080e..bc76b565 100644 --- a/crates/ui-egui/src/browser.rs +++ b/crates/ui-egui/src/browser.rs @@ -1248,7 +1248,7 @@ fn entries(app: &SolveApp, comp: u64, folder: &str) -> Vec { .state() .bodies .iter() - .filter(|b| doc.model_body_component(b) == comp) + .filter(|b| doc.model_body_component(b) == comp && !app.session.body_removed_later(&b.name)) .map(|b| Entry { key: b.name.clone(), row_key: format!("b:{}", b.name), @@ -1960,6 +1960,12 @@ fn occurrence_origin(app: &SolveApp, occurrence: u64) -> Option Vec { + entries(app, 0, "bodies").into_iter().map(|e| e.key).collect() +} + #[cfg(test)] #[path = "section_browser_tests.rs"] mod section_browser_tests; diff --git a/crates/ui-egui/src/viewport.rs b/crates/ui-egui/src/viewport.rs index d381e926..9a060b2c 100644 --- a/crates/ui-egui/src/viewport.rs +++ b/crates/ui-egui/src/viewport.rs @@ -19,6 +19,10 @@ use crate::{SolveApp, icons}; #[path = "viewport_appearance_tests.rs"] mod appearance_tests; +#[cfg(test)] +#[path = "viewport_removed_later_tests.rs"] +mod removed_later_tests; + #[cfg(test)] #[path = "viewport_plane_target_tests.rs"] mod plane_target_tests; diff --git a/crates/ui-egui/src/viewport_removed_later_tests.rs b/crates/ui-egui/src/viewport_removed_later_tests.rs new file mode 100644 index 00000000..b321e1f9 --- /dev/null +++ b/crates/ui-egui/src/viewport_removed_later_tests.rs @@ -0,0 +1,55 @@ +//! Editing a sketch rolls the timeline back to it; a body deleted after the sketch is not drawn +//! then (#1167), and is back in neither the viewport nor the browser's Bodies folder. + +use super::build_scene; +use crate::{Services, SolveApp}; +use serde_json::json; +use solidcraft_engine::Session; + +/// `Kept` and `Gone` blocks, a sketch `Edited` made after both, and `Gone` deleted after it. +fn app() -> SolveApp { + let mut s = Session::default(); + for (name, x) in [("Kept", 0.0), ("Gone", 20.0)] { + s.execute("sketch.create", &json!({"plane": "XY", "name": format!("{name}Sketch")})).unwrap(); + s.execute("sketch.rectangle.two_point", &json!({"p0": [x, 0], "p1": [x + 10.0, 10]})).unwrap(); + s.execute("sketch.finish", &json!({})).unwrap(); + s.execute("solid.extrude", &json!({"sketch": format!("{name}Sketch"), "distance": 5, "body_name": name})).unwrap(); + } + s.execute("sketch.create", &json!({"plane": "XY", "offset": "30 mm", "name": "Edited"})).unwrap(); + s.execute("sketch.rectangle.two_point", &json!({"p0": [0, 0], "p1": [5, 5]})).unwrap(); + s.execute("sketch.finish", &json!({})).unwrap(); + s.execute("solid.remove", &json!({"bodies": ["Gone"]})).unwrap(); + let mut app = SolveApp::new(s, Services::default()); + app.ui.ground_shadow = false; + app.ui.show_origin = false; + app +} + +fn triangles(app: &SolveApp) -> usize { + let scene = build_scene(app).merged(); + scene.tris.len() + scene.pbr.len() + scene.trans.len() + scene.glass.len() +} + +#[test] +fn a_body_deleted_later_is_not_drawn_while_a_sketch_is_edited() { + let mut app = app(); + let drawn = triangles(&app); + assert!(drawn > 0, "the kept block is drawn"); + let sketch = app.session.doc.find_feature("Edited").map(|f| f.id).unwrap(); + app.edit_sketch(sketch); + assert_eq!(app.session.active_sketch, Some(sketch)); + assert!(app.session.model.state().body("Gone").is_some(), "the rolled-back history has it"); + assert_eq!(triangles(&app), drawn, "only the kept block is drawn"); + app.finish_sketch(); + assert_eq!(triangles(&app), drawn); +} + +#[test] +fn the_bodies_folder_leaves_out_a_body_deleted_later() { + let mut app = app(); + let sketch = app.session.doc.find_feature("Edited").map(|f| f.id).unwrap(); + let names = |app: &SolveApp| crate::browser::body_rows(app).into_iter().collect::>(); + assert_eq!(names(&app), ["Kept"]); + app.edit_sketch(sketch); + assert_eq!(names(&app), ["Kept"], "the body is not listed with an eye that cannot show it"); +} diff --git a/docs/mcp.md b/docs/mcp.md index 65025811..7ef017db 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -145,7 +145,7 @@ They cover the workflow, the command shapes, how to verify with `measure`, and t | `list_commands` | `filter?` | The command catalog: id, label, tab/panel, shortcut, parameter docs, enabled now. `filter` matches id, label, tab and panel. | | `execute` | `command`, `params?`, `verbose?` (default true) | Run any command with JSON parameters (`list_commands` shows them). Failures leave the design unchanged. A feature command replies with the feature (`feature`, `name`), every body (`bodies`) and the bodies it created, changed and removed (`created_bodies`, `changed_bodies`, `removed_bodies`). `verbose: false` uses the same compact reply as `batch`. | | `batch` | `commands: [{command, params}]`, `rollback?` (default true), `verbose?` (default true) | Run steps in order, stop at the first error and undo the completed steps. The reply names the failing step (`failed.index`, `failed.command`, `failed.error`) and every result before it. `verbose: false` keeps the reply small in large designs: feature steps leave out the full body list (`created_bodies`, `changed_bodies`, `removed_bodies` stay) and sketch steps the ids they repeat outside `created`. Bridged to an app that has `engine.batch`, the whole batch is one control request (one frame of the app). Older apps, and apps showing the agent cursor, get the steps one by one. | -| `inspect_design` | `measure?` (default true), `sketches?` (default true) | Design properties (`metadata`: project, author… and other tools' keys; set them with `execute` `document.metadata.set`), parameters, timeline (type, suppressed, rolled back, error, warning, timing), bodies (volume, area, centre of mass, bbox, face/edge/vertex counts, visible), sketches (curves, points, constraints, profiles, DOF, solve status, visible), selection, undo depth. Hide helper sketches, bodies or construction planes with `execute` `browser.visibility` (`{items, visible}` or `{folder, visible}`), and joint glyphs with `{joints: [id or name], visible}` or `{folder: "joints", visible}` (a hidden joint still constrains and drives; `joint.list` reports each joint's `visible`). | +| `inspect_design` | `measure?` (default true), `sketches?` (default true) | Design properties (`metadata`: project, author… and other tools' keys; set them with `execute` `document.metadata.set`), parameters, timeline (type, suppressed, rolled back, error, warning, timing), bodies (volume, area, centre of mass, bbox, face/edge/vertex counts, visible; `removed_later: true` while a sketch is being edited on a rolled-back timeline and a later feature deletes the body, which is then not shown), sketches (curves, points, constraints, profiles, DOF, solve status, visible), selection, undo depth. Hide helper sketches, bodies or construction planes with `execute` `browser.visibility` (`{items, visible}` or `{folder, visible}`), and joint glyphs with `{joints: [id or name], visible}` or `{folder: "joints", visible}` (a hidden joint still constrains and drives; `joint.list` reports each joint's `visible`). | | `measure` | `body?` | Volume (mm³), area (mm²), centre of mass, bbox and topology counts per body, with totals. | | `body_topology` | `body` | Edges (index, midpoint, length, ends) and faces (index, name, area, centroid, normal, `point`, and for a cylindrical face its `cylinder`: `origin`, `axis`, `radius`). An edge midpoint is the `[x,y,z]` that picks that edge for fillet and chamfer. A face's `point` lies on it and picks it (its centroid may not: that of a face with a hole is in the hole); a face's `name` picks it for the commands that take faces (`face.delete`, `solid.shell`, `solid.draft`, `solid.offset_face`, `solid.split_face`, `solid.replace_face`, and `face` of `sheet.convert`). | | `set_parameter` | `name`, `value` (number or expression), `unit?`, `comment?` | Create or change a user parameter and recompute; reports `recomputed` (features rebuilt), `restored` (features whose earlier result for the same inputs was reused, e.g. a value set back) and features that now fail. |