diff --git a/apps/solidcraft-cli/src/main.rs b/apps/solidcraft-cli/src/main.rs index 8d9db6ff..15aa2883 100644 --- a/apps/solidcraft-cli/src/main.rs +++ b/apps/solidcraft-cli/src/main.rs @@ -410,10 +410,13 @@ fn cmd_bench_open(args: &[String]) -> Result<(), String> { let read_ms = t.elapsed().as_secs_f64() * 1000.0; println!("{}", json!({"step": "read", "ms": read_ms, "bytes": bytes.len()})); let t = std::time::Instant::now(); + // The file as read, held until the timeline has evaluated it (as the import command does). + let mut read = None; let doc = if step { let imported = solidcraft_engine::io::step_import_feature(&bytes, path).map_err(|e| e.to_string())?; let mut doc = solidcraft_engine::doc::Document::new(&imported.name); doc.add_feature(imported.kind, Some(&imported.name)).map_err(|e| e.to_string())?; + read = Some(imported.import); doc } else { solidcraft_engine::io::read_design(&bytes).map_err(|e| e.to_string())? @@ -425,6 +428,7 @@ fn cmd_bench_open(args: &[String]) -> Result<(), String> { } let t = std::time::Instant::now(); let s = Session::new(doc); + drop(read); println!("{}", json!({"step": "evaluate", "ms": t.elapsed().as_secs_f64() * 1000.0})); for r in &s.model.results { println!("{}", json!({"step": "feature", "id": r.id, "name": r.name, "ms": r.ms, "error": r.error})); diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index 41868e2d..10c63384 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -1032,8 +1032,9 @@ pub enum FeatureKind { Import { /// Source file name (display only). file: String, - /// The STEP (ISO 10303-21) text. - step: String, + /// The STEP (ISO 10303-21) text, shared by every copy of the design (undo states, + /// previews, scratch sessions): a large file is held once, not once per edit. + step: std::sync::Arc, /// The file's product/assembly tree, kept for components. #[serde(default, skip_serializing_if = "Vec::is_empty")] components: Vec, diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index 9146d40a..a7e61b10 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -4407,9 +4407,12 @@ fn eval_feature(doc: &Document, vals: &BTreeMap, f: &Feature, st: FeatureKind::Import { step, .. } => { let imp = kernel::step_import_shared(step)?; let digest = crate::config::payload_digest(step); + // Files from board and assembly tools give thousands of bodies the same name: name + // them from a set, not by searching the bodies for every candidate (cubic). + let mut names = BodyNames::new(st); for (k, b) in imp.bodies.iter().enumerate() { let base = f.body_names.get(k).filter(|n| !n.trim().is_empty()).unwrap_or(&b.name); - let name = unique_body_name(st, base); + let name = names.unique(base); let b = crate::lineage::numbered_tool(b.body.clone().with_import_certificate(&digest, f.id)?); st.bodies.push(ModelBody::traced_new(name, b, f.id)); } @@ -4442,6 +4445,35 @@ fn mesh_bodies(meshes: &[crate::MeshData]) -> Result> { Ok(out) } +/// [`unique_body_name`] for many bodies in a row: the same names, in time linear in the bodies. +struct BodyNames { + taken: std::collections::HashSet, + /// The next suffix worth trying for each base (smaller ones are all taken). + next: std::collections::HashMap, +} + +impl BodyNames { + fn new(st: &ModelState) -> Self { + BodyNames { taken: st.bodies.iter().map(|b| b.name.clone()).collect(), next: std::collections::HashMap::new() } + } + fn unique(&mut self, base: &str) -> String { + let base = if base.trim().is_empty() { "Body" } else { base.trim() }; + if self.taken.insert(base.to_string()) { + return base.to_string(); + } + let mut i = self.next.get(base).copied().unwrap_or(2); + let name = loop { + let candidate = format!("{base} ({i})"); + i += 1; + if self.taken.insert(candidate.clone()) { + break candidate; + } + }; + self.next.insert(base.to_string(), i); + name + } +} + /// `base`, or `base (2)`, `base (3)`… if a body already has that name. fn unique_body_name(st: &ModelState, base: &str) -> String { let base = if base.trim().is_empty() { "Body" } else { base.trim() }; @@ -4780,3 +4812,33 @@ mod chain_tests { assert_eq!(faces.len(), 3, "{faces:?}"); } } + +#[cfg(test)] +mod body_name_tests { + use super::*; + + /// Imported bodies get the names one-at-a-time naming gives them, in time linear in the + /// bodies: a board file with thousands of same-named parts once took minutes (#883). + #[test] + fn many_same_named_bodies_are_named_in_linear_time() { + let mut st = ModelState::default(); + let body = kernel::box_solid(Vec3::ZERO, Vec3::new(1.0, 1.0, 1.0)).unwrap(); + for name in ["R", "R (3)", "C"] { + st.bodies.push(ModelBody::new(name.into(), body.clone(), 1)); + } + // The same names as asking the bodies one at a time. + let mut names = BodyNames::new(&st); + let mut slow = st.clone(); + for base in ["R", "R", "C", " ", "Body", "R", "R (2)", "X"] { + let expected = unique_body_name(&slow, base); + assert_eq!(names.unique(base), expected); + slow.bodies.push(ModelBody::new(expected, body.clone(), 2)); + } + // 50,000 parts named alike: milliseconds (one-at-a-time naming would take hours). + let began = std::time::Instant::now(); + let mut names = BodyNames::new(&ModelState::default()); + let last = (0..50_000).map(|_| names.unique("R_0603")).last().unwrap(); + assert_eq!(last, "R_0603 (50000)"); + assert!(began.elapsed() < std::time::Duration::from_secs(5), "{:?}", began.elapsed()); + } +} diff --git a/crates/doc/src/progress.rs b/crates/doc/src/progress.rs index 568d5742..ae063976 100644 --- a/crates/doc/src/progress.rs +++ b/crates/doc/src/progress.rs @@ -17,11 +17,23 @@ pub struct Progress { total: AtomicUsize, /// The feature the evaluation waits at until cancelled ([`Progress::hold_at`]). hold: AtomicUsize, + /// Long work inside one feature (reading a large STEP file): its step and how far it is. + work_stage: std::sync::Mutex<&'static str>, + work_done: AtomicUsize, + work_total: AtomicUsize, } impl Default for Progress { fn default() -> Self { - Progress { cancelled: AtomicBool::new(false), done: AtomicUsize::new(0), total: AtomicUsize::new(0), hold: AtomicUsize::new(usize::MAX) } + Progress { + cancelled: AtomicBool::new(false), + done: AtomicUsize::new(0), + total: AtomicUsize::new(0), + hold: AtomicUsize::new(usize::MAX), + work_stage: std::sync::Mutex::new(""), + work_done: AtomicUsize::new(0), + work_total: AtomicUsize::new(0), + } } } @@ -42,6 +54,29 @@ impl Progress { pub fn features(&self) -> (usize, usize) { (self.done.load(Ordering::Relaxed), self.total.load(Ordering::Relaxed)) } + /// Long work inside a feature, such as reading a large STEP file: (step, done, total); + /// `None` while there is none to show. + pub fn work(&self) -> Option<(&'static str, usize, usize)> { + let total = self.work_total.load(Ordering::Relaxed); + let stage = self.work_stage.lock().map(|s| *s).unwrap_or(""); + (total > 0 && !stage.is_empty()).then(|| (stage, self.work_done.load(Ordering::Relaxed).min(total), total)) + } +} + +/// The kernel's long work (STEP import) reports to the same progress and stops on Cancel. +struct KernelWatch(Arc); + +impl solidcraft_kernel::watch::Watch for KernelWatch { + fn cancelled(&self) -> bool { + self.0.is_cancelled() + } + fn progress(&self, stage: &'static str, done: usize, total: usize) { + if let Ok(mut s) = self.0.work_stage.lock() { + *s = stage; + } + self.0.work_total.store(total, Ordering::Relaxed); + self.0.work_done.store(done, Ordering::Relaxed); + } } struct Current { @@ -56,6 +91,7 @@ thread_local! { /// Run `f` with `progress` watching every evaluation it makes on this thread. pub fn watch(progress: Arc, f: impl FnOnce() -> R) -> R { + let kernel = Arc::new(KernelWatch(progress.clone())); let previous = CURRENT.with(|c| c.replace(Some(Current { progress, depth: 0 }))); struct Restore(Option>); impl Drop for Restore { @@ -66,7 +102,7 @@ pub fn watch(progress: Arc, f: impl FnOnce() -> R) -> R { } } let _restore = Restore(Some(previous)); - f() + solidcraft_kernel::watch::watching(kernel, f) } /// Entered at the start of an evaluation; the guard leaves it. diff --git a/crates/engine/src/cmd/component_clone.rs b/crates/engine/src/cmd/component_clone.rs index c5895f4e..a52f831c 100644 --- a/crates/engine/src/cmd/component_clone.rs +++ b/crates/engine/src/cmd/component_clone.rs @@ -319,7 +319,7 @@ pub(super) fn paste( bodies.insert(body.name.clone(), new_name.clone()); let feature_name = unique(&format!("{} Base", body.name), &mut used); let id = scratch.doc_mut().add_feature( - FeatureKind::Import { file: format!("{} independent base", body.name), step, components: Vec::new() }, + FeatureKind::Import { file: format!("{} independent base", body.name), step: step.into(), components: Vec::new() }, Some(&feature_name), )?; if let Some(f) = scratch.doc_mut().feature_mut(id) { diff --git a/crates/engine/src/cmd/config_certificate_tests.rs b/crates/engine/src/cmd/config_certificate_tests.rs index ec0eceb6..75f850a3 100644 --- a/crates/engine/src/cmd/config_certificate_tests.rs +++ b/crates/engine/src/cmd/config_certificate_tests.rs @@ -71,7 +71,10 @@ fn certified_annulus_side_rows_follow_radius_edits_and_clone() { fn imported(session: &mut Session, step: String, name: &str) -> u64 { let owner = session.active_component; - let id = session.doc_mut().add_feature(FeatureKind::Import { file: "fixture.step".into(), step, components: vec![] }, Some(name)).unwrap(); + let id = session + .doc_mut() + .add_feature(FeatureKind::Import { file: "fixture.step".into(), step: step.into(), components: vec![] }, Some(name)) + .unwrap(); let feature = session.doc_mut().feature_mut(id).unwrap(); feature.body_names = vec![name.into()]; feature.component = owner; @@ -262,8 +265,8 @@ fn certified_import_payload_binding_roundtrip_clone_and_replacement_refusal() { assert_eq!(painted_radius(&s, "Imported", 10.), pieces); // Identical geometry, entity IDs and item IDs, but a replaced embedded payload. let FeatureKind::Import { step: payload, .. } = &mut s.doc_mut().feature_mut(source).unwrap().kind else { panic!() }; - *payload = step.replace("certificate fixture", "replacement fixture"); - assert_ne!(*payload, step); + *payload = step.replace("certificate fixture", "replacement fixture").into(); + assert_ne!(&**payload, step.as_str()); s.refresh(); assert!(s.doc.face_colors(s.model.state().body("Imported").unwrap()).is_empty()); assert_eq!(painted_radius(&s, "Imported (copy)", 10.), pieces); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ef56c232..ef284aa0 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -254,6 +254,7 @@ fn join_of_two_overlapping_rings_is_not_refused() { mod boolean_diagnostics; mod component_frames; +mod large_step_import; mod loft_cut; mod mirror_planes; mod path_scope; diff --git a/crates/engine/src/tests/large_step_import.rs b/crates/engine/src/tests/large_step_import.rs new file mode 100644 index 00000000..58122ea6 --- /dev/null +++ b/crates/engine/src/tests/large_step_import.rs @@ -0,0 +1,80 @@ +//! Large STEP imports (#883): the file's text is held once however many undo states share +//! it, and an import cancelled from another thread stops and keeps nothing. + +use std::sync::Arc; + +use serde_json::json; +use solidcraft_doc::{FeatureKind, progress::Progress}; + +use crate::*; + +fn step_file(name: &str, count: usize) -> String { + let bodies: Vec<_> = (0..count) + .map(|i| { + let at = solidcraft_geom::Vec3::new((i % 32) as f64 * 12.0, (i / 32) as f64 * 12.0, 0.0); + solidcraft_kernel::box_solid(at, at + solidcraft_geom::Vec3::new(10.0, 10.0, 10.0)).unwrap() + }) + .collect(); + let text = solidcraft_kernel::step_export(&bodies.iter().collect::>(), name).unwrap(); + let path = std::env::temp_dir().join(format!("solidcraft-883-{}-{name}.step", std::process::id())); + std::fs::write(&path, text).unwrap(); + path.to_string_lossy().to_string() +} + +fn step_text(s: &Session) -> Arc { + s.doc.features.iter().find_map(|f| if let FeatureKind::Import { step, .. } = &f.kind { Some(step.clone()) } else { None }).unwrap() +} + +/// Memory budget: 30 edits after a large import keep one copy of the STEP text, not 31. +#[test] +fn edits_after_a_large_import_share_one_copy_of_its_text() { + let path = step_file("shared", 300); + let mut s = Session::default(); + s.execute("file.insert_step", &json!({"path": path})).unwrap(); + let text = step_text(&s); + assert!(text.len() > 500_000, "{} bytes", text.len()); + for i in 0..30 { + s.execute("parameters.add", &json!({"name": format!("p{i}"), "expression": format!("{i} mm")})).unwrap(); + } + assert!(s.undo.len() >= 30); + for snap in &s.undo { + for f in &snap.doc.features { + if let FeatureKind::Import { step, .. } = &f.kind { + assert!(Arc::ptr_eq(step, &text), "an undo state holds its own copy of the STEP text"); + } + } + } + assert!(Arc::ptr_eq(&step_text(&s), &text)); + // Every Document clone (preview, scratch session) shares it too. + let copy = (*s.doc).clone(); + assert!(copy.features.iter().any(|f| matches!(&f.kind, FeatureKind::Import { step, .. } if Arc::ptr_eq(step, &text)))); + let _ = std::fs::remove_file(path); +} + +#[test] +fn cancelled_import_stops_and_leaves_the_design_alone() { + let path = step_file("cancelled", 300); + let mut s = Session::default(); + s.execute("solid.box", &json!({"length": 10, "width": 10, "height": 10})).unwrap(); + let before = s.model.state().bodies.len(); + // Cancel from the window thread while the worker reads the file. + let progress = Arc::new(Progress::default()); + let canceller = { + let progress = progress.clone(); + std::thread::spawn(move || { + while progress.work().is_none() { + std::thread::yield_now(); + } + progress.cancel(); + }) + }; + let r = solidcraft_doc::progress::watch(progress.clone(), || s.execute("file.insert_step", &json!({"path": path}))); + canceller.join().unwrap(); + let err = r.unwrap_err().to_string(); + assert!(err.contains("cancelled"), "{err}"); + assert_eq!(s.model.state().bodies.len(), before); + assert!(!s.doc.features.iter().any(|f| matches!(f.kind, FeatureKind::Import { .. }))); + let (_, done, total) = progress.work().unwrap(); + assert!(done < total, "stopped part way ({done} of {total})"); + let _ = std::fs::remove_file(path); +} diff --git a/crates/io/src/design.rs b/crates/io/src/design.rs index 5436e022..e04476ff 100644 --- a/crates/io/src/design.rs +++ b/crates/io/src/design.rs @@ -568,7 +568,7 @@ fn read_document(reader: &mut PackageReader) -> Result { for (index, mesh) in meshes.iter_mut().enumerate() { @@ -640,7 +640,7 @@ mod tests { fn seekable_preview_defers_large_source_blob_until_document_read() { let mut doc = Document::new("Deferred source"); let step = "ISO-10303-21;".repeat(200_000); - doc.add_feature(FeatureKind::Import { file: "large.step".into(), step: step.clone(), components: Vec::new() }, None).unwrap(); + doc.add_feature(FeatureKind::Import { file: "large.step".into(), step: step.clone().into(), components: Vec::new() }, None).unwrap(); let preview = DesignPreview { meshes: vec![mesh()] }; let bytes = write_design_container_with_preview(&doc, DesignCompression::Fast, Some(&preview)).unwrap(); let count = std::rc::Rc::new(std::cell::Cell::new(0)); @@ -699,7 +699,7 @@ mod tests { fn native_cache_round_trip_and_preview_defer_original_sources() { let mut doc = Document::new("Native cache"); let step = "ISO-10303-21;".repeat(200_000); - doc.add_feature(FeatureKind::Import { file: "large.step".into(), step: step.clone(), components: Vec::new() }, None).unwrap(); + doc.add_feature(FeatureKind::Import { file: "large.step".into(), step: step.clone().into(), components: Vec::new() }, None).unwrap(); let bodies = native_geometry(); let bytes = write_design_container_with_cache(&doc, DesignCompression::Fast, None, Some(&bodies)).unwrap(); let entries = parts(&bytes); @@ -868,7 +868,7 @@ mod tests { png.extend(150u32.to_be_bytes()); let mut doc = Document::from_json(&document().to_json()).unwrap(); if let FeatureKind::Import { step, .. } = &mut doc.features[0].kind { - *step = "source data".repeat(250_000); + *step = "source data".repeat(250_000).into(); } let bytes = write_design_container_with_thumbnail(&doc, DesignCompression::Fast, None, None, Some(&png)).unwrap(); let count = std::rc::Rc::new(std::cell::Cell::new(0)); @@ -1157,7 +1157,7 @@ mod tests { .collect(); let step = solidcraft_kernel::step_export(&bodies.iter().collect::>(), "SolidCraft storage benchmark").unwrap(); let mut doc = Document::new("Generated STEP benchmark"); - doc.add_feature(FeatureKind::Import { file: "generated.step".into(), step, components: Vec::new() }, None).unwrap(); + doc.add_feature(FeatureKind::Import { file: "generated.step".into(), step: step.into(), components: Vec::new() }, None).unwrap(); measure_storage(&doc, "step"); } diff --git a/crates/io/src/lib.rs b/crates/io/src/lib.rs index 3a380344..c4d62eb9 100644 --- a/crates/io/src/lib.rs +++ b/crates/io/src/lib.rs @@ -95,6 +95,9 @@ pub struct StepFeature { /// Body names from the file (solid names, else product names). pub body_names: Vec, pub warnings: Vec, + /// The file as read. Hold it until the feature has been evaluated: the timeline then reuses + /// it instead of reading the file again (it is freed with the last holder). + pub import: std::sync::Arc, } /// Read STEP bytes (`file` is the source file name) into an Import base feature. The file is @@ -125,8 +128,9 @@ pub fn step_import_feature(bytes: &[u8], file: &str) -> Result { Ok(StepFeature { name, body_names: imp.bodies.iter().map(|b| b.name.clone()).collect(), - kind: solidcraft_doc::FeatureKind::Import { file: file_name, step: text, components: imp.tree.clone() }, + kind: solidcraft_doc::FeatureKind::Import { file: file_name, step: text.into(), components: imp.tree.clone() }, warnings: iges_warnings.into_iter().chain(imp.warnings.iter().cloned()).collect(), + import: imp, }) } diff --git a/crates/kernel/src/lib.rs b/crates/kernel/src/lib.rs index a3641986..2ecf305b 100644 --- a/crates/kernel/src/lib.rs +++ b/crates/kernel/src/lib.rs @@ -93,6 +93,7 @@ mod topo; mod tspline; #[cfg(test)] mod untouched_faces_tests; +pub mod watch; pub use blend::{ChamferSide, chamfer, chamfer_sides, fillet, fillet_each, fillet_g2, fillet_max, fillet_max_each}; pub use blend_corner::fillet_radii; @@ -119,7 +120,7 @@ pub use prism_measure::certified_prism_equivalent; pub use sew::{EdgeSpec, FaceSpec, SurfSpec, sew}; pub use splitface::{SplitTool, split_body, split_faces, split_faces_with_map}; pub use step::step_export; -pub use step_in::{ImportedBody, StepImport, StepNode, step_import, step_import_shared, step_orientation_errors, step_validate}; +pub use step_in::{ImportedBody, StepImport, StepNode, step_cache_alive, step_import, step_import_shared, step_orientation_errors, step_validate}; pub use step_out::{ExportBody, ExportProduct, StepHeader, step_export_bodies, step_export_products}; pub use surface_kind::{FaceSurface, face_surface}; pub use surface_offset::offset_surface; diff --git a/crates/kernel/src/step_in/mod.rs b/crates/kernel/src/step_in/mod.rs index 6070e017..53818e6a 100644 --- a/crates/kernel/src/step_in/mod.rs +++ b/crates/kernel/src/step_in/mod.rs @@ -257,6 +257,12 @@ struct Reader<'a> { unmeshed_checked: HashSet, bodies: Vec, workers: usize, + /// Whoever waits for the import (progress, Cancel). + watch: Option>, + /// Solid items in the file, the measure of the conversion's progress. + solid_items: usize, + /// The watch cancelled the import: the walk stops and the import fails. + cancelled: bool, } impl<'a> Reader<'a> { @@ -280,9 +286,27 @@ impl<'a> Reader<'a> { unmeshed_checked: HashSet::new(), bodies: Vec::new(), workers, + watch: None, + solid_items: 0, + cancelled: false, } } + /// Report progress; true once the import is cancelled (the walk then unwinds). + fn stop(&mut self) -> bool { + if self.cancelled { + return true; + } + if let Some(w) = &self.watch { + if w.cancelled() { + self.cancelled = true; + return true; + } + w.progress("converting", self.bodies.len().min(self.solid_items), self.solid_items); + } + false + } + fn warn(&mut self, msg: impl Into) { *self.warnings.entry(msg.into()).or_default() += 1; } @@ -584,6 +608,9 @@ impl<'a> Reader<'a> { let u = self.units_of(rep); let mut items = self.reps.get(&rep).map(|r| r.0.clone()).unwrap_or_default().into_iter().peekable(); while let Some(item) = items.next() { + if self.stop() { + return; + } let Some(e) = self.ex.get(item) else { continue }; match e.name() { "MANIFOLD_SOLID_BREP" | "BREP_WITH_VOIDS" | "FACETED_BREP" | "SHELL_BASED_SURFACE_MODEL" => { @@ -749,6 +776,9 @@ impl<'a> Reader<'a> { fn node(&mut self, pd: u64, local: mt::Matrix4, world: mt::Matrix4, path: &[String], depth: usize, stack: &mut Vec) -> StepNode { let name = self.product_name(pd); let mut node = StepNode { name: name.clone(), transform: to_array(&local), bodies: Vec::new(), children: Vec::new() }; + if self.stop() { + return node; + } if depth > MAX_ASSEMBLY_DEPTH || stack.contains(&pd) { self.warn("assembly nested too deeply or recursive: some components were left out"); return node; @@ -933,40 +963,73 @@ pub fn step_validate(text: &str) -> std::result::Result { Ok(n) } +/// A recent import: held weakly, so it lives only as long as someone uses it (the command that +/// read the file, until the timeline has evaluated it), and strongly only when it is small. +struct Recent { + key: (u64, usize), + import: std::sync::Weak, + /// Keeps a small import alive (never read). + _small: Option>, +} + /// Recent imports, so a file read for a command and then evaluated in the timeline is only /// read once. -static RECENT: std::sync::Mutex)>> = std::sync::Mutex::new(Vec::new()); +static RECENT: std::sync::Mutex> = std::sync::Mutex::new(Vec::new()); -/// [`step_import`] through a small cache of recent results (keyed by the text). +/// Files up to this size stay cached after their last user is gone (a few cheap re-reads +/// saved); a larger import is freed with the design that holds it. +const KEEP_SMALL_BYTES: usize = 1 << 20; + +/// [`step_import`] through a small cache of recent results (keyed by the text). Keep the result +/// while the same text is still to be read elsewhere (in the timeline): that read reuses it. pub fn step_import_shared(text: &str) -> Result> { use std::hash::{Hash, Hasher}; let mut h = std::collections::hash_map::DefaultHasher::new(); text.hash(&mut h); let key = (h.finish(), text.len()); if let Ok(cache) = RECENT.lock() - && let Some((_, _, imp)) = cache.iter().find(|(k, n, _)| (*k, *n) == key) + && let Some(imp) = cache.iter().filter(|r| r.key == key).find_map(|r| r.import.upgrade()) { - return Ok(imp.clone()); + return Ok(imp); } let imp = std::sync::Arc::new(step_import(text)?); if let Ok(mut cache) = RECENT.lock() { - cache.retain(|(k, n, _)| (*k, *n) != key); - cache.push((key.0, key.1, imp.clone())); - if cache.len() > 4 { + cache.retain(|r| r.key != key && r.import.strong_count() > 0); + let small = (text.len() <= KEEP_SMALL_BYTES).then(|| imp.clone()); + cache.push(Recent { key, import: Arc::downgrade(&imp), _small: small }); + while cache.len() > 4 { cache.remove(0); } } Ok(imp) } +/// Imports the cache still holds alive, and how many STEP bytes they came from (tests and +/// diagnostics: a closed design must not leave its import behind). +pub fn step_cache_alive() -> (usize, usize) { + RECENT.lock().map(|c| c.iter().filter(|r| r.import.strong_count() > 0).fold((0, 0), |(n, b), r| (n + 1, b + r.key.1))).unwrap_or((0, 0)) +} + /// Read a STEP file: its solids (as bodies, placed by the assembly structure) and the tree. pub fn step_import(text: &str) -> Result { step_import_with_workers(text, parallel::workers()) } fn step_import_with_workers(text: &str, workers: usize) -> Result { - let ex = p21::parse(text).map_err(KernelError::Invalid)?; + let watch = crate::watch::current(); + let cancelled = || watch.as_ref().is_some_and(|w| w.cancelled()); + let ex = match p21::parse(text) { + Ok(ex) => ex, + Err(_) if cancelled() => return Err(crate::watch::cancelled_error("STEP import")), + Err(e) => return Err(KernelError::Invalid(e)), + }; let mut rd = Reader::new(&ex, workers); + rd.solid_items = ex + .entities + .values() + .filter(|e| matches!(e.name(), "MANIFOLD_SOLID_BREP" | "BREP_WITH_VOIDS" | "FACETED_BREP" | "SHELL_BASED_SURFACE_MODEL")) + .count(); + rd.watch = watch.clone(); rd.index(); // Roots: product definitions that are nobody's component. let child_pds: HashSet = @@ -1006,6 +1069,10 @@ fn step_import_with_workers(text: &str, workers: usize) -> Result { tree.push(StepNode { name: "Import".into(), transform: to_array(&identity), bodies, children: Vec::new() }); } } + // Cancelled: what was read so far is dropped here, not cached. + if rd.cancelled || cancelled() { + return Err(crate::watch::cancelled_error("STEP import")); + } if let Some(first) = ex.duplicates.first() { rd.warn(format!("{} entity ids are defined twice (first #{first}); the first definitions were used", ex.duplicates.len())); } diff --git a/crates/kernel/src/step_in/p21.rs b/crates/kernel/src/step_in/p21.rs index 996583cb..e749f40b 100644 --- a/crates/kernel/src/step_in/p21.rs +++ b/crates/kernel/src/step_in/p21.rs @@ -399,6 +399,7 @@ pub fn parse(text: &str) -> Result { } lx.expect(b';')?; let mut ex = Exchange::default(); + let watch = crate::watch::current(); loop { lx.ws()?; if lx.peek().is_none() { @@ -469,6 +470,14 @@ pub fn parse(text: &str) -> Result { vec![lx.record()?] }; lx.expect(b';')?; + if ex.entities.len() % 4096 == 0 + && let Some(w) = &watch + { + if w.cancelled() { + return err("STEP import cancelled"); + } + w.progress("reading", lx.i, lx.s.len()); + } if ex.entities.len() >= MAX_ENTITIES { return err(format!("STEP file has more than {MAX_ENTITIES} entities")); } diff --git a/crates/kernel/src/step_in/tests.rs b/crates/kernel/src/step_in/tests.rs index f943651a..6932be2a 100644 --- a/crates/kernel/src/step_in/tests.rs +++ b/crates/kernel/src/step_in/tests.rs @@ -915,3 +915,119 @@ fn faces_with_too_many_bounds_are_refused() { assert!(imp.warnings.iter().any(|w| w.contains("boundary loops")), "{:?}", imp.warnings); assert!(start.elapsed() < std::time::Duration::from_secs(10), "took {:?}", start.elapsed()); } + +/// A board-like assembly for issue #883: `SOLIDCRAFT_STEP_PCB_FIXTURE=path` writes it (with +/// `SOLIDCRAFT_STEP_PCB_COMPONENTS` and `SOLIDCRAFT_STEP_PCB_PINS`; 600 and 15 give 52 MB). Every component is +/// its own product with a package body and its own pins (nothing shared), the worst case for +/// a large import. +#[test] +#[ignore = "manual fixture generator for large STEP imports"] +fn generate_board_assembly_fixture() { + use crate::step_out::{ExportBody, ExportProduct, StepHeader, step_export_products}; + let count: usize = solidcraft_geom::env::var("STEP_PCB_COMPONENTS").map(|s| s.parse().unwrap()).unwrap_or(500); + let pins: usize = solidcraft_geom::env::var("STEP_PCB_PINS").map(|s| s.parse().unwrap()).unwrap_or(16); + let mut owned: Vec> = Vec::new(); + for c in 0..count { + let w = 4.0 + (c % 7) as f64 * 0.37; + let d = 3.0 + (c % 5) as f64 * 0.29; + let mut bodies = vec![fillet_box(w, d, 1.2 + (c % 3) as f64 * 0.1)]; + for p in 0..pins { + let side = if p % 2 == 0 { -0.6 } else { d + 0.6 }; + let x = 0.3 + (p / 2) as f64 * (w - 0.6) / (pins / 2).max(1) as f64 + c as f64 * 1e-4; + bodies.push(cylinder(Vec3::new(x, side, -1.5), Vec3::Z, 0.25, 2.0 + (p % 3) as f64 * 0.01).unwrap()); + } + owned.push(bodies); + } + let mut products: Vec = owned + .iter() + .enumerate() + .map(|(c, bodies)| ExportProduct { + name: format!("U{c}"), + bodies: bodies.iter().enumerate().map(|(k, b)| ExportBody { name: format!("U{c}_{k}"), body: b, color: Some([0.2, 0.2, 0.2]) }).collect(), + children: Vec::new(), + }) + .collect(); + let side = (count as f64).sqrt().ceil() as usize; + let children = (0..count) + .map(|c| { + let (x, y) = ((c % side) as f64 * 12.0, (c / side) as f64 * 10.0); + (c, [[1.0, 0.0, 0.0, 0.0], [0.0, 1.0, 0.0, 0.0], [0.0, 0.0, 1.0, 0.0], [x, y, 1.6, 1.0]], format!("U{c}")) + }) + .collect(); + products.push(ExportProduct { name: "Board".into(), bodies: Vec::new(), children }); + let root = products.len() - 1; + let text = step_export_products(&products, root, &StepHeader { file_name: "board.step".into(), ..Default::default() }).unwrap(); + let path = solidcraft_geom::env::var("STEP_PCB_FIXTURE").unwrap_or_else(|| "board.step".into()); + std::fs::write(&path, &text).unwrap(); + println!("wrote {path}: {} bytes, {} components", text.len(), count); +} + +fn fillet_box(w: f64, d: f64, h: f64) -> Body { + let b = box_solid(Vec3::ZERO, Vec3::new(w, d, h)).unwrap(); + let vertical = [Vec3::new(0.0, 0.0, h / 2.0), Vec3::new(w, 0.0, h / 2.0), Vec3::new(w, d, h / 2.0), Vec3::new(0.0, d, h / 2.0)]; + let top = [Vec3::new(w / 2.0, 0.0, h), Vec3::new(w, d / 2.0, h), Vec3::new(w / 2.0, d, h), Vec3::new(0.0, d / 2.0, h)]; + let b = fillet(&b, &vertical, 0.3).unwrap_or(b); + fillet(&b, &top, 0.1).unwrap_or(b) +} + +/// Independent boxes as one STEP file (about 3 KB a box). +fn boxes_step(count: usize, name: &str) -> String { + let bodies: Vec<_> = (0..count) + .map(|i| { + let at = Vec3::new((i % 32) as f64 * 12.0, (i / 32) as f64 * 12.0, 0.0); + box_solid(at, at + Vec3::new(10.0, 10.0, 10.0)).unwrap() + }) + .collect(); + step_export(&bodies.iter().collect::>(), name).unwrap() +} + +struct TestWatch { + cancel_after: usize, + reports: std::sync::atomic::AtomicUsize, + last: std::sync::Mutex<(&'static str, usize, usize)>, +} + +impl crate::watch::Watch for TestWatch { + fn cancelled(&self) -> bool { + self.reports.load(std::sync::atomic::Ordering::Relaxed) >= self.cancel_after + } + fn progress(&self, stage: &'static str, done: usize, total: usize) { + self.reports.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + *self.last.lock().unwrap() = (stage, done, total); + } +} + +#[test] +fn watched_import_reports_progress_and_stops_on_cancel_without_caching() { + let text = boxes_step(40, "watched import"); + let watch = Arc::new(TestWatch { cancel_after: usize::MAX, reports: Default::default(), last: std::sync::Mutex::new(("", 0, 0)) }); + let imp = crate::watch::watching(watch.clone(), || step_import(&text)).unwrap(); + assert_eq!(imp.bodies.len(), 40); + let (stage, done, total) = *watch.last.lock().unwrap(); + assert_eq!((stage, total), ("converting", 40)); + assert!(done <= total && watch.reports.load(std::sync::atomic::Ordering::Relaxed) > 1); + + // Cancelled part way: an error, no partial result, and nothing cached for the text. + let text = boxes_step(41, "cancelled import"); + let watch = Arc::new(TestWatch { cancel_after: 3, reports: Default::default(), last: std::sync::Mutex::new(("", 0, 0)) }); + let err = crate::watch::watching(watch.clone(), || step_import_shared(&text).map(|_| ())).unwrap_err(); + assert!(err.to_string().contains("cancelled"), "{err}"); + let (_, done, _) = *watch.last.lock().unwrap(); + assert!(done < 41, "stopped before converting everything ({done})"); + // Unwatched, the same text imports in full (no cached failure or partial import). + assert_eq!(step_import_shared(&text).unwrap().bodies.len(), 41); + // Without a watch nothing is reported or stopped. + assert_eq!(step_import(&boxes_step(2, "unwatched")).unwrap().bodies.len(), 2); +} + +#[test] +fn large_imports_are_freed_with_their_last_user() { + let text = boxes_step(400, "large shared import"); + assert!(text.len() > KEEP_SMALL_BYTES, "{} bytes", text.len()); + let first = step_import_shared(&text).unwrap(); + let again = step_import_shared(&text).unwrap(); + assert!(Arc::ptr_eq(&first, &again), "read once while it is in use"); + let weak = Arc::downgrade(&first); + drop((first, again)); + assert!(weak.upgrade().is_none(), "the cache must not keep a large import alive after its design is gone"); +} diff --git a/crates/kernel/src/watch.rs b/crates/kernel/src/watch.rs new file mode 100644 index 00000000..840988c2 --- /dev/null +++ b/crates/kernel/src/watch.rs @@ -0,0 +1,44 @@ +//! Progress and cooperative cancellation for long kernel work (reading a large STEP file) run +//! off the window thread. The caller installs a [`Watch`] around the work with [`watching`]; +//! the work reports how far it is and stops at its next check once the watch says so. Work +//! that nobody watches runs as before. + +use std::cell::RefCell; +use std::sync::Arc; + +/// Someone waiting for long work: told how far it is, and able to stop it. +pub trait Watch: Send + Sync { + /// Stop at the next check. + fn cancelled(&self) -> bool; + /// `done` of `total` units of the current step (`stage`, such as "reading" or "converting"). + fn progress(&self, stage: &'static str, done: usize, total: usize); +} + +thread_local! { + static CURRENT: RefCell>> = const { RefCell::new(None) }; +} + +/// Run `f` with `watch` seeing the kernel work it does on this thread. +pub fn watching(watch: Arc, f: impl FnOnce() -> R) -> R { + let previous = CURRENT.with(|c| c.replace(Some(watch))); + struct Restore(Option>>); + impl Drop for Restore { + fn drop(&mut self) { + if let Some(previous) = self.0.take() { + CURRENT.with(|c| *c.borrow_mut() = previous); + } + } + } + let _restore = Restore(Some(previous)); + f() +} + +/// The watch over this thread's work, to hand to the worker threads it starts. +pub(crate) fn current() -> Option> { + CURRENT.with(|c| c.borrow().clone()) +} + +/// The error long work returns when its watch cancelled it. +pub(crate) fn cancelled_error(what: &str) -> crate::KernelError { + crate::KernelError::Failed(format!("{what} cancelled")) +} diff --git a/crates/ui-egui/src/file_load.rs b/crates/ui-egui/src/file_load.rs index 4e4b5eb7..1a37924b 100644 --- a/crates/ui-egui/src/file_load.rs +++ b/crates/ui-egui/src/file_load.rs @@ -474,6 +474,8 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) { }; let phase = r.phase.load(Ordering::Relaxed); let (done, total) = r.progress.features(); + // Long work inside one feature (a large STEP file) has progress of its own. + let work = r.progress.work().filter(|(_, d, t)| phase == 0 && d < t); let screen = ctx.content_rect(); // Leave the title bar to the window. The resize edges are a foreground layer above this. let cover = egui::Rect::from_min_max(egui::pos2(screen.left(), screen.top() + crate::toolbar::APP_BAR_HEIGHT), screen.max); @@ -538,16 +540,22 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) { ui.label(if cancelled { "Waiting for the previous operation to stop...".to_string() } else { - match phase { - 1 => "Preparing the view...".into(), - 2 => "Preparing the thumbnail...".into(), + match (phase, work) { + (_, Some(("reading", d, t))) => format!("Reading the file... {}%", d * 100 / t.max(1)), + (_, Some((_, d, t))) => format!("Converting solid {} of {t}", d + 1), + (1, _) => "Preparing the view...".into(), + (2, _) => "Preparing the thumbnail...".into(), _ if total > 0 => format!("Rebuilding feature {} of {total}", (done + 1).min(total)), _ => "Reading the file...".into(), } }); }); // Always there, so the card keeps its height as the phases change. - let fraction = if cancelled || total == 0 { + let fraction = if cancelled { + 0.0 + } else if let Some((_, d, t)) = work { + d as f32 / t.max(1) as f32 + } else if total == 0 { 0.0 } else if phase == 0 { done as f32 / total as f32 diff --git a/crates/ui-egui/src/gpu.rs b/crates/ui-egui/src/gpu.rs index 62fa2935..af854ea0 100644 --- a/crates/ui-egui/src/gpu.rs +++ b/crates/ui-egui/src/gpu.rs @@ -179,9 +179,13 @@ pub struct AccessMap { pub scale: f32, } +/// Vertices (or line instances) on the GPU, in as many buffers as the device's buffer size +/// limit needs: a large import's triangles can exceed one buffer (#883). struct Batch { - buffer: wgpu::Buffer, - count: u32, + /// Each buffer and its vertex (instance) count. + parts: Vec<(wgpu::Buffer, u32)>, + /// Bytes per buffer (all but the last are full). + chunk: usize, } /// A chunk of planned bodies on the GPU: their evaluated mesh and a colour per vertex. @@ -289,8 +293,8 @@ impl Batches { let faces = mesh.vertex_face.get(part.vertices.start as usize..part.vertices.end as usize).unwrap_or_default(); bytes.extend(faces.iter().flat_map(|f| b.face_colors.get(*f as usize).copied().unwrap_or(last))); } - if let Some(colors) = upload(device, "sc_planned_colors", &bytes, 4) { - out.push(PlannedDraw { mesh, colors: colors.buffer }); + if let Some((colors, _)) = upload(device, "sc_planned_colors", &bytes, 4).and_then(|c| c.parts.into_iter().next()) { + out.push(PlannedDraw { mesh, colors }); } } out @@ -663,13 +667,27 @@ fn upload(device: &wgpu::Device, label: &str, bytes: &[u8], stride: usize) -> Op } fn upload_with(device: &wgpu::Device, label: &str, bytes: &[u8], stride: usize, usage: wgpu::BufferUsages) -> Option { - if bytes.len() < stride { + if bytes.len() < stride || stride == 0 { return None; } - Some(Batch { - buffer: device.create_buffer_init(&wgpu::util::BufferInitDescriptor { label: Some(label), contents: bytes, usage }), - count: u32::try_from(bytes.len() / stride).unwrap_or(0), - }) + let chunk = buffer_chunk(stride, device.limits().max_buffer_size); + let parts = bytes + .chunks(chunk) + .filter(|part| part.len() >= stride) + .map(|part| { + let buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor { label: Some(label), contents: part, usage }); + (buffer, u32::try_from(part.len() / stride).unwrap_or(0)) + }) + .collect(); + Some(Batch { parts, chunk }) +} + +/// Bytes per GPU buffer for vertices of `stride` bytes: within the device's limit and a +/// whole number of triangles and line pairs, so no primitive straddles two buffers. +fn buffer_chunk(stride: usize, max_buffer_size: u64) -> usize { + let unit = stride.max(1) * 6; + let max = usize::try_from(max_buffer_size).unwrap_or(usize::MAX).min(1 << 30); + (max / unit).max(1) * unit } /// Triangles (3 vertices of `TRI_SIZE` bytes each, position first) ordered far to near along @@ -691,7 +709,10 @@ impl Batches { let (Some(g), false) = (&self.glass, self.glass_cpu.is_empty()) else { return }; let turned = self.glass_sorted_for.is_none_or(|s| s[0] * back[0] + s[1] * back[1] + s[2] * back[2] < 0.9994); if turned { - queue.write_buffer(&g.buffer, 0, &sort_back_to_front(&self.glass_cpu, back)); + let sorted = sort_back_to_front(&self.glass_cpu, back); + for ((buffer, _), bytes) in g.parts.iter().zip(sorted.chunks(g.chunk.max(1))) { + queue.write_buffer(buffer, 0, bytes); + } self.glass_sorted_for = Some(back); } } @@ -796,15 +817,19 @@ impl egui_wgpu::CallbackTrait for ViewportCallback { let tris = |pass: &mut wgpu::RenderPass<'static>, p: &wgpu::RenderPipeline, b: &Option| { if let Some(b) = b { pass.set_pipeline(p); - pass.set_vertex_buffer(0, b.buffer.slice(..)); - pass.draw(0..b.count, 0..1); + for (buffer, count) in &b.parts { + pass.set_vertex_buffer(0, buffer.slice(..)); + pass.draw(0..*count, 0..1); + } } }; let lines = |pass: &mut wgpu::RenderPass<'static>, p: &wgpu::RenderPipeline, b: &Option| { if let Some(b) = b { pass.set_pipeline(p); - pass.set_vertex_buffer(0, b.buffer.slice(..)); - pass.draw(0..6, 0..b.count); + for (buffer, count) in &b.parts { + pass.set_vertex_buffer(0, buffer.slice(..)); + pass.draw(0..6, 0..*count); + } } }; let (m, sk, h, pv) = (&res.model, &res.sketch, &res.highlight, &res.preview); @@ -848,8 +873,10 @@ impl egui_wgpu::CallbackTrait for ViewportCallback { if let Some((_, bind)) = res.textures.get(id) { pass.set_pipeline(&res.image); pass.set_bind_group(1, bind, &[]); - pass.set_vertex_buffer(0, q.buffer.slice(..)); - pass.draw(0..q.count, 0..1); + for (buffer, count) in &q.parts { + pass.set_vertex_buffer(0, buffer.slice(..)); + pass.draw(0..*count, 0..1); + } } } tris(pass, &res.ghost, &pv.ghost); @@ -1079,6 +1106,22 @@ mod tests { mod glass_tests { use super::*; + /// A large import's triangles once went into one buffer past the device's limit (524 MB + /// against 256 MB) and the app crashed (#883): they are split into whole triangles. + #[test] + fn large_scenes_are_split_into_buffers_within_the_device_limit() { + let limit = 256u64 << 20; + for stride in [TRI_SIZE, LINE_SIZE, 4] { + let chunk = buffer_chunk(stride, limit); + assert!(chunk as u64 <= limit && chunk > limit as usize / 2); + assert_eq!(chunk % (stride * 3), 0, "whole triangles"); + assert_eq!(chunk % (stride * 2), 0, "whole line pairs"); + } + let scene = 524_434_680usize; + assert_eq!(scene.div_ceil(buffer_chunk(TRI_SIZE, limit)), 2); + assert!(buffer_chunk(TRI_SIZE, 1) >= TRI_SIZE * 6, "a tiny limit still holds one unit"); + } + #[test] fn glass_is_drawn_far_to_near() { // Two triangles at z = 5 and z = -5, given near first; looking down from +Z. diff --git a/crates/ui-egui/src/viewport_appearance_tests.rs b/crates/ui-egui/src/viewport_appearance_tests.rs index 7f65f589..b7f2fffb 100644 --- a/crates/ui-egui/src/viewport_appearance_tests.rs +++ b/crates/ui-egui/src/viewport_appearance_tests.rs @@ -14,7 +14,8 @@ fn placed_step_body_uses_imported_opacity_until_a_design_look_overrides_it() { .unwrap(); let mut document = doc::Document::default(); let component = document.add_component(Some("Imported part"), 0).unwrap(); - let feature = document.add_feature(doc::FeatureKind::Import { file: "glass.step".into(), step, components: Vec::new() }, None).unwrap(); + let feature = + document.add_feature(doc::FeatureKind::Import { file: "glass.step".into(), step: step.into(), components: Vec::new() }, None).unwrap(); document.features.iter_mut().find(|f| f.id == feature).unwrap().component = component; let mut session = Session::new(document); let source = session.model.state().bodies[0].name.clone();