M8: test the finite-geometry check by the layers it visits, not by time (#1672)

The timing assertion could fail under load. finite_check_candidates is now its own function, and the test checks that a rename among 200,000 layers looks at only a handful of them.

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]

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;

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<NodeId> {
+    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(()),
         }