From 4515af3eb98a6993dfd48508f00bee1832c29aeb Mon Sep 17 00:00:00 2001 From: Parman Mohammadalizadeh Date: Tue, 27 Oct 2026 16:01:57 +0000 Subject: [PATCH] Update Links reads again only the linked designs that changed (#5606) (#5674) * Update Links reads again only the linked designs that changed (#5606) Update Links read, evaluated and STEP round-tripped every linked design on every run. Remember a digest of each linked file, of the STEP it gave and of its face names, and skip a link whose file, bodies and face names are all unchanged. Read a linked design through its saved model when the file has one, as opening it does. * Drop an import the rebase left unused Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: bherbruck Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/component_link.rs | 129 +++++++++++++++++++++-- crates/ui-egui/src/file_load_ui_tests.rs | 5 +- 2 files changed, 124 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/component_link.rs b/crates/engine/src/cmd/component_link.rs index 011c6b8316..33bedf07ae 100644 --- a/crates/engine/src/cmd/component_link.rs +++ b/crates/engine/src/cmd/component_link.rs @@ -8,7 +8,9 @@ //! the design gives them (`Feature::link_faces`), so a joint on a linked face stays on that face //! when the design changes, as it would inside the design, and reports it when the face is gone. -use std::path::Path; +use std::hash::{Hash, Hasher}; +use std::path::{Path, PathBuf}; +use std::sync::Mutex; use serde_json::{Value, json}; use solidcraft_doc::{DesignLink, Document, FeatureKind}; @@ -88,20 +90,87 @@ pub fn resolve_design_links(doc: &mut Document, from: Option<&str>) { /// the design gives their faces (`Feature::link_faces`), with the source's design properties. type Linked = (String, solidcraft_io::StepFeature, Vec>, solidcraft_doc::Metadata); -/// The solid bodies of the design at `file`, evaluated, as an Import feature. -fn linked_bodies(file: &Path, own: Option<&str>) -> Result { +/// What a linked file last gave, by the file: digests of its bytes, of the STEP text it gave and +/// of its face names, and that read's warnings. Update Links evaluates a file again only when its +/// bytes or the linked bodies changed since (#5606). Bounded, oldest first out. +static READS: Mutex> = Mutex::new(Vec::new()); +const READS_SIZE: usize = 256; + +struct LinkRead { + file: PathBuf, + bytes: u64, + step: u64, + faces: u64, + warnings: Vec, +} + +#[cfg(test)] +thread_local! { + /// Linked designs read and evaluated on this thread, for tests. + static EVALUATED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +fn digest(value: &T) -> u64 { + let mut h = std::collections::hash_map::DefaultHasher::new(); + value.hash(&mut h); + h.finish() +} + +/// The warnings of the last read of `file`, when it had these bytes and gave the STEP text +/// `step` and the face names `faces`: the bodies it gives are the ones the link holds. +fn unchanged_read(file: &Path, bytes: u64, step: u64, faces: u64) -> Option> { + let file = absolute(file); + let reads = READS.lock().ok()?; + reads.iter().find(|r| r.file == file && r.bytes == bytes && r.step == step && r.faces == faces).map(|r| r.warnings.clone()) +} + +fn remember_read(file: &Path, bytes: u64, feature: &solidcraft_io::StepFeature, faces: &[Vec]) { + let FeatureKind::Import { step, .. } = &feature.kind else { return }; + let file = absolute(file); + let Ok(mut reads) = READS.lock() else { return }; + reads.retain(|r| r.file != file); + if reads.len() >= READS_SIZE { + reads.remove(0); + } + reads.push(LinkRead { file, bytes, step: digest(step.as_bytes()), faces: digest(faces), warnings: feature.warnings.clone() }); +} + +/// The bytes of the design at `file`, which must not be the design saved at `own`. +fn read_link(file: &Path, own: Option<&str>) -> Result> { let shown = file.display().to_string(); if own.is_some_and(|o| absolute(Path::new(o)) == absolute(file)) { return Err(EngineError::Other(format!("{shown}: a design can't be linked into itself"))); } // A network location only when its server is allowed, a device never: judged from the text // before anything is read (on Windows, reading `\\host\share\x` connects to the host). - let bytes = solidcraft_io::vfs::read_design_path(&shown, own, solidcraft_io::MAX_DESIGN_BYTES).map_err(|e| { + solidcraft_io::vfs::read_design_path(&shown, own, solidcraft_io::MAX_DESIGN_BYTES).map_err(|e| { let e = e.to_string(); EngineError::Other(if e.starts_with(&shown) { e } else { format!("{shown}: {e}") }) - })?; - let doc = solidcraft_io::read_design(&bytes).map_err(|e| EngineError::Other(format!("{shown}: {e}")))?; - session_bodies(&Session::opened_at(doc, None, Some(&shown)), &shown) + }) +} + +/// The solid bodies of the design at `file`, evaluated, as an Import feature. +fn linked_bodies(file: &Path, own: Option<&str>) -> Result { + design_bodies(file, &read_link(file, own)?) +} + +/// The solid bodies of `bytes`, the design at `file`, as an Import feature. The model the file +/// keeps is its evaluation when this build saved it for exactly that design, as when it is opened. +fn design_bodies(file: &Path, bytes: &[u8]) -> Result { + #[cfg(test)] + EVALUATED.with(|n| n.set(n.get() + 1)); + let shown = file.display().to_string(); + let fail = |e: solidcraft_io::IoError| EngineError::Other(format!("{shown}: {e}")); + let (doc, saved) = if solidcraft_io::is_design_container(bytes) { + let mut reader = solidcraft_io::DesignContainerReader::new(std::io::Cursor::new(bytes)).map_err(fail)?; + let saved = reader.geometry(); + (reader.read_document().map_err(fail)?, saved) + } else { + (solidcraft_io::read_design(bytes).map_err(fail)?, None) + }; + let (name, feature, faces, metadata) = session_bodies(&Session::opened_at(doc, saved.as_ref(), Some(&shown)), &shown)?; + remember_read(file, digest(bytes), &feature, &faces); + Ok((name, feature, faces, metadata)) } /// The solid bodies of `part`, the design saved as `shown`, as an Import feature. @@ -224,7 +293,14 @@ fn update_links(s: &mut Session, p: &Value) -> Result { let old = old.clone(); link_refused(s.path.as_deref(), &link.path).map_err(|e| bad(cmd, format!("{cname}: {e}")))?; let file = resolve(s.path.as_deref(), &link.path); - let (_, sf, faces, _) = linked_bodies(&file, s.path.as_deref()).map_err(|e| bad(cmd, format!("{cname}: {e}")))?; + let bytes = read_link(&file, s.path.as_deref()).map_err(|e| bad(cmd, format!("{cname}: {e}")))?; + let kept_faces = s.doc.feature(link.feature).map(|f| digest(f.link_faces.as_slice())); + if let Some(known) = kept_faces.and_then(|faces| unchanged_read(&file, digest(bytes.as_slice()), digest(old.as_bytes()), faces)) { + warnings.extend(known); + unchanged.push(cname); + continue; + } + let (_, sf, faces, _) = design_bodies(&file, &bytes).map_err(|e| bad(cmd, format!("{cname}: {e}")))?; warnings.extend(sf.warnings.iter().cloned()); let same = matches!(&sf.kind, FeatureKind::Import { step, .. } if *step == old); let kept = s.doc.feature(link.feature).map(|f| &f.link_faces); @@ -525,6 +601,43 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn update_links_reads_again_only_the_designs_that_changed() { + let dir = std::env::temp_dir().join(format!("solidcraft-link-reads-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let (plate, block) = (dir.join("plate.solidcraft"), dir.join("block.solidcraft")); + save_box(&plate, 30.0); + save_box(&block, 10.0); + let mut a = Session::default(); + run(&mut a, "component.insert_design", json!({"path": plate})); + run(&mut a, "component.insert_design", json!({"path": block, "translate": [50, 0, 0]})); + let evaluated = || EVALUATED.with(std::cell::Cell::get); + // Unchanged files are not evaluated again. + let before = evaluated(); + let r = run(&mut a, "component.update_links", json!({})); + assert_eq!((r["updated"].clone(), r["unchanged"].clone()), (json!([]), json!(["plate", "block"])), "{r}"); + assert_eq!(evaluated(), before); + // An edited file is, and only that one. + save_box(&plate, 45.0); + let r = run(&mut a, "component.update_links", json!({})); + assert_eq!((r["updated"].clone(), r["unchanged"].clone()), (json!(["plate"]), json!(["block"])), "{r}"); + assert_eq!(evaluated(), before + 1); + assert!((volume(&mut a) - 11000.0).abs() < 1e-6, "{}", volume(&mut a)); + // Undone, the link holds the old bodies again: the same file brings the new ones back. + run(&mut a, "edit.undo", json!({})); + assert!((volume(&mut a) - 8000.0).abs() < 1e-6, "{}", volume(&mut a)); + assert_eq!(run(&mut a, "component.update_links", json!({}))["updated"], json!(["plate"])); + assert!((volume(&mut a) - 11000.0).abs() < 1e-6, "{}", volume(&mut a)); + // A file saved again without its model is evaluated, and gives the same bodies. + let mut part = Session::default(); + run(&mut part, "doc.open", json!({"path": block})); + run(&mut part, "file.save", json!({"cache": false})); + let before = evaluated(); + assert_eq!(run(&mut a, "component.update_links", json!({}))["unchanged"], json!(["plate", "block"])); + assert_eq!(evaluated(), before + 1); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn inserting_a_missing_file_or_bad_parameters_changes_nothing() { let mut s = Session::default(); diff --git a/crates/ui-egui/src/file_load_ui_tests.rs b/crates/ui-egui/src/file_load_ui_tests.rs index e38543c2c7..08c3b7d000 100644 --- a/crates/ui-egui/src/file_load_ui_tests.rs +++ b/crates/ui-egui/src/file_load_ui_tests.rs @@ -189,14 +189,15 @@ fn update_links_names_the_design_it_reads_while_the_window_draws() { let dir = scratch("links"); let part = dir.join("plate.solidcraft"); let case = dir.join("case.solidcraft"); - // A plate with two holes (three features: the read can be held at the third). + // A plate with two holes (three features: the read can be held at the third), saved without + // its model so reading it rebuilds the features. let save_part = |height: f64| { let mut s = Session::default(); s.execute("solid.box", &json!({"length": 40, "width": 20, "height": height})).unwrap(); for x in [10, 30] { s.execute("solid.cylinder", &json!({"radius": 2, "height": 200, "base": [x, 10, -1], "operation": "cut"})).unwrap(); } - s.execute("file.save", &json!({"path": part})).unwrap(); + s.execute("file.save", &json!({"path": part, "cache": false})).unwrap(); s.execute("inspect.measure", &json!({})).unwrap()["total"]["volume_mm3"].as_f64().unwrap() }; let volume = |app: &mut SolveApp| app.session.execute("inspect.measure", &json!({})).unwrap()["total"]["volume_mm3"].as_f64().unwrap();