feat: save body and occurrence visibility with the design (#531) (#592)

Hidden bodies, occurrence placements, sketches and origin items are saved with the design and come back when it is reopened. Visibility is still not an undo step, but hiding or showing something now counts as an unsaved change. Additive field under format 16. Fixes #531.

Co-authored-by: jonaswre <jonas.wrede@beyondit.gmbh>

apps/solidcraft-web/src/main.rs

@@ -110,7 +110,7 @@ mod web {
                 let changed = s.is_dirty() && self.autosaved != Some(s.revision);
                 let wait = AUTOSAVE_S - (now - self.autosaved_at);
                 if changed && wait <= 0.0 {
-                    files::autosave(&self.files, Some((&s.doc, s.path.as_deref())));
+                    files::autosave(&self.files, Some((&s.design(), s.path.as_deref())));
                     self.autosaved = Some(s.revision);
                     self.autosaved_at = now;
                 } else if changed {

crates/doc/src/document.rs

@@ -1765,6 +1765,38 @@ pub struct Document {
     /// Named views: saved cameras.
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub named_views: Vec<NamedView>,
+    /// What was hidden when the design was saved. A session takes it from here when the design
+    /// opens and keeps the live state itself (`browser.visibility`: saved, not an undo step), so
+    /// a document in a session leaves this empty; the session puts it back when it saves.
+    #[serde(default, skip_serializing_if = "Visibility::is_empty")]
+    pub visibility: Visibility,
+}
+
+/// Items hidden or shown one by one. Saved with the design, but not an undo step.
+#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]
+#[serde(default)]
+pub struct Visibility {
+    /// Bodies by name.
+    pub hidden_bodies: Vec<String>,
+    /// Exact occurrence placements hidden with all descendants.
+    pub hidden_occurrences: Vec<Vec<u64>>,
+    /// Origin items (O, X, Y, Z, XY, XZ, YZ) and construction geometry, by name.
+    pub hidden_origin: Vec<String>,
+    /// Sketches by feature id.
+    pub hidden_sketches: Vec<u64>,
+    /// Finished sketches shown although a feature uses them.
+    pub shown_sketches: Vec<u64>,
+}
+
+impl Visibility {
+    /// Nothing hidden or shown one by one (the default view).
+    pub fn is_empty(&self) -> bool {
+        self.hidden_bodies.is_empty()
+            && self.hidden_occurrences.is_empty()
+            && self.hidden_origin.is_empty()
+            && self.hidden_sketches.is_empty()
+            && self.shown_sketches.is_empty()
+    }
 }
 
 /// A saved camera (orbit camera: target, yaw, pitch, distance; `fov` 0 for orthographic).
@@ -1833,6 +1865,7 @@ impl Document {
             browser_groups: Vec::new(),
             browser_order: Default::default(),
             named_views: Vec::new(),
+            visibility: Visibility::default(),
         }
     }
 

crates/doc/src/format.rs

@@ -60,6 +60,9 @@
 //!   `font` is in the built-in font, as it always was.
 //!   A Move feature can also keep its source bodies and transform a copy (`copy: true`); a
 //!   move without it saves as before.
+//!   A design can also keep what was hidden when it was saved (`visibility`: hidden bodies,
+//!   occurrence placements, sketches and origin items; `browser.visibility`). A design without
+//!   it shows everything, as it always did; a reader that ignores it loses only the hiding.
 
 use serde_json::Value;
 

crates/engine/src/cmd/browser.rs

@@ -36,7 +36,7 @@ pub static COMMANDS: &[CommandSpec] = &[
     CommandSpec::new("browser.order", "Order Browser Items", order).params("folder, component?, order: [keys] (the folder's items in display order)"),
     CommandSpec::new("browser.visibility", "Show/Hide", visibility).icon("eye").noundo().params(
         "visible: bool; items?: [sketch id or name | body name | construction name | O|X|Y|Z|XY|XZ|YZ]; \
-         or folder?: bodies|sketches|construction|origin (all of it); or paths?: [[occurrence ids]] (exact placements and descendants), isolate?: bool (requires paths and visible:true). View state: not saved, not an undo step",
+         or folder?: bodies|sketches|construction|origin (all of it); or paths?: [[occurrence ids]] (exact placements and descendants), isolate?: bool (requires paths and visible:true). Saved with the design; not an undo step",
     ),
 ];
 

crates/engine/src/cmd/file.rs

@@ -189,7 +189,7 @@ fn write_file(s: &mut Session, path: &str, format: &str, compression: &str, prev
     };
     let bytes = match format {
         "container" => s.design_container(compression, preview, cache)?,
-        "json" => solidcraft_io::write_design(&s.doc),
+        "json" => solidcraft_io::write_design(&s.design()),
         _ => return Err(bad("file.save", "format must be container or json")),
     };
     // Atomic, keeping the version being replaced as `<file>.bak`.
@@ -205,7 +205,7 @@ impl Session {
     pub fn design_container(&self, compression: solidcraft_io::DesignCompression, preview: bool, cache: bool) -> solidcraft_io::Result<Vec<u8>> {
         let geometry = cache.then(|| saved_geometry(self)).flatten();
         let preview = preview.then(|| saved_preview(self)).flatten();
-        solidcraft_io::write_design_container_with_cache(&self.doc, compression, preview.as_ref(), geometry.as_deref())
+        solidcraft_io::write_design_container_with_cache(&self.design(), compression, preview.as_ref(), geometry.as_deref())
     }
 }
 
@@ -289,7 +289,7 @@ fn export_to(s: &Session, path: &str, format: Format, bodies: &[String]) -> Resu
         if !bodies.is_empty() {
             return Err(bad("file.export", "design JSON exports the complete document; omit bodies"));
         }
-        solidcraft_io::write_design(&s.doc)
+        solidcraft_io::write_design(&s.design())
     } else if format == Format::Step && bodies.is_empty() && !s.doc.occurrences.is_empty() {
         solidcraft_io::step_assembly(&s.doc, &s.doc.painted(&s.model.state()), &name)?
     } else {

crates/engine/src/cmd/occurrence_tests.rs

@@ -425,6 +425,87 @@ fn instance_styles_follow_source_identity_after_collision_rename_and_pending_mov
     assert!(imported.bodies[0].body.paint().unwrap().faces.iter().any(|f| f.color == [0.0, 1.0, 0.0]));
 }
 
+/// A design file path in a fresh temporary folder (removed by the caller).
+fn temp_design(tag: &str, ext: &str) -> (std::path::PathBuf, String) {
+    let dir = std::env::temp_dir().join(format!("solidcraft-visibility-{tag}-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let path = dir.join(format!("design.{ext}")).to_string_lossy().to_string();
+    (dir, path)
+}
+
+/// #531: hiding a placement and a body is saved with the design and comes back when it opens.
+/// It is not an undo step (undoing a later feature leaves it hidden), but the design then has
+/// unsaved changes, and showing it again is saved the same way.
+#[test]
+fn hidden_placements_and_bodies_are_saved_with_the_design_but_not_undone() {
+    let (dir, path) = temp_design("saved", "solidcraft");
+    let mut s = Session::default();
+    run(&mut s, "solid.box", json!({"length":4,"width":4,"height":4,"body_name":"Base"}));
+    let (component, first) = created(&mut s, "Part");
+    run(&mut s, "solid.box", json!({"length":2,"width":2,"height":2}));
+    let second = run(&mut s, "occurrence.copy", json!({"occurrence":first,"translate":[20,0,0]}))["occurrence"].as_u64().unwrap();
+    run(&mut s, "component.activate", json!({"component":0}));
+    run(&mut s, "file.save", json!({"path": path}));
+    assert!(!s.is_dirty());
+
+    let undo = s.undo.len();
+    run(&mut s, "browser.visibility", json!({"visible":false,"paths":[[second]]}));
+    run(&mut s, "browser.visibility", json!({"visible":false,"items":["Base"]}));
+    assert_eq!(s.undo.len(), undo, "hiding is not an undo step");
+    assert!(s.is_dirty(), "hiding leaves the design with unsaved changes");
+    run(&mut s, "solid.box", json!({"length":1,"width":1,"height":1,"corner":[50,0,0],"operation":"new","body_name":"Later"}));
+    run(&mut s, "edit.undo", json!({}));
+    assert!(!s.body_visible("Base") && !s.occurrence_visible(&[second]), "undo keeps the hiding");
+
+    run(&mut s, "file.save", json!({}));
+    assert!(!s.is_dirty());
+    run(&mut s, "file.new", json!({}));
+    run(&mut s, "doc.open", json!({"path": path}));
+    assert!(!s.body_visible("Base"), "the hidden body stays hidden");
+    assert!(!s.occurrence_visible(&[second]), "the hidden placement stays hidden");
+    assert!(s.occurrence_visible(&[first]));
+    assert!(s.doc.components.iter().any(|c| c.id == component));
+    assert!(!s.is_dirty(), "an opened design has no unsaved changes");
+    assert!(s.undo.is_empty());
+
+    run(&mut s, "browser.visibility", json!({"visible":true,"folder":"bodies"}));
+    assert!(s.is_dirty());
+    run(&mut s, "file.save", json!({}));
+    run(&mut s, "file.new", json!({}));
+    run(&mut s, "doc.open", json!({"path": path}));
+    assert!(s.body_visible("Base") && s.occurrence_visible(&[second]), "showing again is saved too");
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+/// #531: designs saved before visibility was kept (no `visibility` in the file) open with
+/// everything shown, and a design with nothing hidden saves exactly as before.
+#[test]
+fn designs_without_saved_visibility_open_unchanged() {
+    let (dir, path) = temp_design("legacy", "json");
+    let mut s = Session::default();
+    run(&mut s, "solid.box", json!({"length":4,"width":4,"height":4,"body_name":"Base"}));
+    let mut legacy: Value = serde_json::from_slice(&solidcraft_io::write_design(&s.doc)).unwrap();
+    assert!(legacy.get("visibility").is_none(), "nothing hidden: no visibility in the file");
+    legacy["format"] = json!("solidcraft/15");
+    std::fs::write(&path, serde_json::to_vec(&legacy).unwrap()).unwrap();
+
+    run(&mut s, "file.new", json!({}));
+    run(&mut s, "doc.open", json!({"path": path}));
+    assert!(s.body_visible("Base"));
+    assert_eq!(s.visibility, crate::Visibility::default());
+    assert!(!s.is_dirty());
+    run(&mut s, "file.save", json!({}));
+    let saved: Value = serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
+    assert!(saved.get("visibility").is_none());
+
+    run(&mut s, "browser.visibility", json!({"visible":false,"items":["Base"]}));
+    run(&mut s, "file.save", json!({}));
+    let saved: Value = serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
+    assert_eq!(saved["visibility"]["hidden_bodies"], json!(["Base"]));
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 /// Command names for linking another design are a proposal (#533); adjust them to the real API.
 const INSERT_DESIGN: &str = "component.insert_design";
 const UPDATE_LINKS: &str = "component.update_links";

crates/engine/src/lib.rs

@@ -27,6 +27,7 @@ use solidcraft_geom::Vec3;
 
 pub use cmd::{CommandInfo, CommandSpec, auto_operation, command_specs, find_command};
 pub use solidcraft_doc as doc;
+pub use solidcraft_doc::Visibility;
 pub use solidcraft_geom as geom;
 pub use solidcraft_io as io;
 pub use solidcraft_kernel as kernel;
@@ -217,8 +218,11 @@ pub struct Session {
     pub section: Option<(Vec3, Vec3)>,
     /// Surface analysis shading the model (zebra, draft, curvature map).
     pub analysis: Option<SurfaceAnalysis>,
-    /// What is hidden or shown in the view (`browser.visibility`).
+    /// What is hidden or shown in the view (`browser.visibility`): saved with the design, not an
+    /// undo step. A design brings its saved visibility when it opens (`Session::new`).
     pub visibility: Visibility,
+    /// The visibility the design file holds (what `is_dirty` compares with).
+    saved_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>,
@@ -252,22 +256,6 @@ pub(crate) struct Checkpoint {
     pub redo: Vec<Snapshot>,
 }
 
-/// Items hidden or shown one by one (view state, not part of the design and not an undo step).
-#[derive(Clone, Debug, Default, PartialEq, serde::Serialize, serde::Deserialize)]
-#[serde(default)]
-pub struct Visibility {
-    /// Bodies by name.
-    pub hidden_bodies: Vec<String>,
-    /// Exact occurrence placements hidden with all descendants.
-    pub hidden_occurrences: Vec<Vec<u64>>,
-    /// Origin items (O, X, Y, Z, XY, XZ, YZ) and construction geometry, by name.
-    pub hidden_origin: Vec<String>,
-    /// Sketches by feature id.
-    pub hidden_sketches: Vec<u64>,
-    /// Finished sketches shown although a feature uses them.
-    pub shown_sketches: Vec<u64>,
-}
-
 /// A way of shading the model's faces to judge their shape (view state, not part of the design).
 #[derive(Clone, Copy, Debug, PartialEq, serde::Serialize, serde::Deserialize)]
 #[serde(tag = "type", rename_all = "snake_case")]
@@ -297,7 +285,9 @@ impl Default for Session {
 }
 
 impl Session {
-    pub fn new(doc: Document) -> Self {
+    pub fn new(mut doc: Document) -> Self {
+        // The session keeps what is hidden (not an undo step); the design file carries it.
+        let visibility = std::mem::take(&mut doc.visibility);
         let doc = Arc::new(doc);
         let mut model = Model::new();
         model.evaluate(&doc);
@@ -324,18 +314,31 @@ impl Session {
             analysis: None,
             checkpoints: Vec::new(),
             next_checkpoint: 1,
-            visibility: Visibility::default(),
+            saved_visibility: visibility.clone(),
+            visibility,
             #[cfg(test)]
             evaluations: 0,
             crash_journal: false,
         }
     }
 
+    /// The design has changes not in its file: in the document, or in what is hidden.
     pub fn is_dirty(&self) -> bool {
-        !Arc::ptr_eq(&self.doc, &self.saved) && *self.doc != *self.saved
+        (!Arc::ptr_eq(&self.doc, &self.saved) && *self.doc != *self.saved) || self.visibility != self.saved_visibility
     }
     pub fn mark_saved(&mut self) {
         self.saved = self.doc.clone();
+        self.saved_visibility = self.visibility.clone();
+    }
+
+    /// The design as a file holds it: the document with what is hidden in it.
+    pub fn design(&self) -> std::borrow::Cow<'_, Document> {
+        if self.visibility.is_empty() {
+            return std::borrow::Cow::Borrowed(&*self.doc);
+        }
+        let mut doc = (*self.doc).clone();
+        doc.visibility = self.visibility.clone();
+        std::borrow::Cow::Owned(doc)
     }
     /// The design has changes not in its file (a recovered design).
     pub fn mark_unsaved(&mut self) {
@@ -667,6 +670,7 @@ impl Session {
             checkpoints: Vec::new(),
             next_checkpoint: 1,
             visibility: self.visibility.clone(),
+            saved_visibility: self.saved_visibility.clone(),
             #[cfg(test)]
             evaluations: 0,
             crash_journal: false,

crates/engine/src/recovery.rs

@@ -143,7 +143,8 @@ pub struct Autosaver {
     /// Autosave this often while there are unsaved changes.
     pub interval: Duration,
     lock: Option<File>,
-    last: Option<Arc<Document>>,
+    /// The design and what was hidden at the last autosave.
+    last: Option<(Arc<Document>, crate::Visibility)>,
     last_at: Instant,
     has_entry: bool,
 }
@@ -189,17 +190,17 @@ impl Autosaver {
             crate::crash::rebased(s, s.path.as_deref().map(Path::new), false);
             return Ok(false);
         }
-        if self.last.as_ref().is_some_and(|l| Arc::ptr_eq(l, &s.doc) || **l == *s.doc) {
+        if self.last.as_ref().is_some_and(|(l, v)| (Arc::ptr_eq(l, &s.doc) || **l == *s.doc) && *v == s.visibility) {
             return Ok(false);
         }
         let io = |e: std::io::Error| EngineError::Other(format!("autosave: {e}"));
-        let bytes = solidcraft_io::write_design_container(&s.doc)?;
+        let bytes = solidcraft_io::write_design_container(&s.design())?;
         solidcraft_io::write_atomic(&file(&self.dir, &self.id, solidcraft_io::DESIGN_EXT), &bytes, false).map_err(io)?;
         let entry =
             Entry { id: self.id.clone(), path: s.path.clone(), name: s.doc.name.clone(), saved_at: unix_now(), features: s.doc.features.len() };
         let meta = serde_json::to_vec_pretty(&entry).map_err(|e| EngineError::Other(e.to_string()))?;
         solidcraft_io::write_atomic(&file(&self.dir, &self.id, "json"), &meta, false).map_err(io)?;
-        self.last = Some(s.doc.clone());
+        self.last = Some((s.doc.clone(), s.visibility.clone()));
         self.has_entry = true;
         // Crash replays start from this autosave from here.
         crate::crash::rebased(s, Some(&file(&self.dir, &self.id, solidcraft_io::DESIGN_EXT)), true);

crates/mcp/src/tests.rs

@@ -483,7 +483,6 @@ fn compact_batch_reply_does_not_repeat_the_body_list() {
 /// #531: a product model keeps an optional variant hidden in its default presentation. Hiding
 /// it must survive saving and reopening the design (suppressing it would change the design).
 #[test]
-#[ignore = "fails until #531 is fixed"]
 fn hidden_body_stays_hidden_after_save_and_reopen() {
     let mut s = server();
     tool(&mut s, "execute", json!({"command": "solid.box", "params": {"length": 10, "width": 10, "height": 10, "body_name": "Base"}}));

docs/occurrence-workflows.md

@@ -32,7 +32,7 @@ Visibility is an independent view setting for exact placements. Hide a path and
 {"visible":false,"paths":[[2,4]]}
 ```
 
-`visible:true` removes that path's own hide setting. A hidden ancestor still hides its descendants. Add `isolate:true` to a visible path list to hide other branches. Show the bodies folder to clear occurrence hides. Selection, activation paths and visibility are session state; the saved document retains occurrence IDs and transforms. Undo follows the existing behavior of clearing selection.
+`visible:true` removes that path's own hide setting. A hidden ancestor still hides its descendants. Add `isolate:true` to a visible path list to hide other branches. Show the bodies folder to clear occurrence hides. Selection and activation paths are session state. Visibility is saved with the design (hidden bodies, placements, sketches and origin items) but is not an undo step; the saved document also retains occurrence IDs and transforms. Undo follows the existing behavior of clearing selection.
 
 Paths are checked for missing links, repeated IDs, component mismatches and a maximum depth of 64. Descendant expansion rejects more than 100,000 placements or bodies instead of returning incomplete results. Computed body names normally retain their readable occurrence labels; collisions use full occurrence IDs so visibility and body commands still address the correct source.