diff --git a/crates/engine/src/finite_geometry_tests.rs b/crates/engine/src/finite_geometry_tests.rs index a8b552a73..52ba2a575 100644 --- a/crates/engine/src/finite_geometry_tests.rs +++ b/crates/engine/src/finite_geometry_tests.rs @@ -55,31 +55,17 @@ fn large(groups: usize, per: usize) -> (figcraft_doc::Document, crate::NodeId) { (doc, last.unwrap()) } -/// Seconds `renames` renames of one layer take, one command each. -fn rename_seconds(s: &mut Session, id: crate::NodeId, renames: usize) -> f64 { - let start = std::time::Instant::now(); - for i in 0..renames { - s.execute("node.set", &json!({"ids": [id], "props": {"name": format!("rect {i}")}})).unwrap(); - } - start.elapsed().as_secs_f64() -} - #[test] fn an_edit_checks_only_what_it_changed_not_the_whole_document() { // #1672: the check went over every layer of the file after every edit, so a batch slowed down - // with the size of the file, not of the edit. - let (small, small_id) = large(1, 10); - let (big, big_id) = large(2000, 100); - let mut s = Session::with_document(small); - let mut b = Session::with_document(big); - // Warm up both (first-command bookkeeping), then time the same renames. - rename_seconds(&mut s, small_id, 3); - rename_seconds(&mut b, big_id, 3); - let (in_small, in_big) = (rename_seconds(&mut s, small_id, 100), rename_seconds(&mut b, big_id, 100)); - assert_eq!(b.doc().unwrap().node(big_id).unwrap().name, "rect 99"); - // 200,000 layers against 10: what grows with the document is a log factor, not a scan (with - // the scan these 100 renames took about 0.6 s, without it about 0.02 s). - assert!(in_big < in_small * 20.0 + 0.25, "100 renames took {in_big:.3} s among 200,000 layers, {in_small:.3} s among 10"); + // with the size of the file, not of the edit. Count the layers it looks at instead of timing it. + let (big, id) = large(2000, 100); + let mut s = Session::with_document(big); + let before = s.doc().unwrap().clone(); + s.execute("node.set", &json!({"ids": [id], "props": {"name": "renamed"}})).unwrap(); + let checked = crate::session::finite_check_candidates(s.doc().unwrap(), &before); + assert!(checked.contains(&id), "the renamed layer is checked: {checked:?}"); + assert!(checked.len() <= 10, "a rename among 200,000 layers checks {} of them", checked.len()); } #[test] diff --git a/crates/engine/src/session.rs b/crates/engine/src/session.rs index 99101dc2b..113b75cdb 100644 --- a/crates/engine/src/session.rs +++ b/crates/engine/src/session.rs @@ -10,5 +10,7 @@ use super::{ mod access; pub use access::Forward; mod exec; +#[cfg(test)] +pub(crate) use exec::finite_check_candidates; mod new; mod undo; diff --git a/crates/engine/src/session/exec.rs b/crates/engine/src/session/exec.rs index 61fa8a42d..097f44c78 100644 --- a/crates/engine/src/session/exec.rs +++ b/crates/engine/src/session/exec.rs @@ -16,6 +16,13 @@ fn command_carries_import_bytes(id: &str, params: &Value) -> bool { } } +/// The layers `check_finite_geometry` looks at: only those the command added or changed. The diff +/// skips what the two versions share, so the check costs about the size of the edit, not of the +/// document (#1672). +pub(crate) fn finite_check_candidates(doc: &Document, before: &Document) -> Vec { + doc.changed_ids(before, usize::MAX).unwrap_or_default() +} + impl Session { /// Append a live event with the active document's identity (nothing without one). Inside a /// [`Session::grouped`] call the event is buffered and committed only when the group succeeds. @@ -179,10 +186,7 @@ impl Session { } let broken = |n: &Node| !(n.transform.as_coeffs().iter().all(|c| c.is_finite()) && n.width.is_finite() && n.height.is_finite()); let untouched = |n: &Node| bdoc.node_arc(n.id).zip(ds.doc.node_arc(n.id)).is_some_and(|(a, b)| Arc::ptr_eq(a, b)); - // Only the layers the command added or changed: the diff skips what the two versions - // share, so the check costs about the size of the edit, not of the document (#1672). - let changed = ds.doc.changed_ids(bdoc, usize::MAX).unwrap_or_default(); - match changed.iter().filter_map(|i| ds.doc.node(*i)).find(|n| broken(n) && !untouched(n)) { + match finite_check_candidates(&ds.doc, bdoc).iter().filter_map(|i| ds.doc.node(*i)).find(|n| broken(n) && !untouched(n)) { Some(n) => Err(bad(id, format!("\"{}\" would get a position or size beyond the number range", n.name))), None => Ok(()), }