From 372af0f21e5aa228212edb395f50b49d87916a45 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 25 Oct 2026 20:13:34 -0700 Subject: [PATCH] Shapes: upgrading a saved project walks nested shape groups without recursion upgrade_shape_contents now visits nested groups with a work list, so a project with very deeply nested shape groups does not run out of stack while Wiggle Paths gains Spatial Phase. The new test nests a Wiggle Paths 100,000 groups deep and upgrades it on a 2 MiB stack. --- crates/project/src/build.rs | 20 ++++++++++++-------- crates/project/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/crates/project/src/build.rs b/crates/project/src/build.rs index 3f9e232aa..b78e3d8c1 100644 --- a/crates/project/src/build.rs +++ b/crates/project/src/build.rs @@ -615,17 +615,21 @@ fn wiggle_spatial_phase(ids: &mut Ids) -> Property { /// Adds the properties that shape path operations have gained since a project was saved (Wiggle /// Paths' Spatial Phase), at their defaults, to the operations in `contents` and its nested -/// groups. Returns whether anything was added. +/// groups. Returns whether anything was added. It walks the nested groups with a work list and +/// does not recurse. pub fn upgrade_shape_contents(ids: &mut Ids, contents: &mut PropGroup) -> bool { let mut changed = false; - for n in &mut contents.children { - let Node::Group(g) = n else { continue }; - if g.match_id == "wiggle" && !g.children.iter().any(|c| c.match_id() == "spatialPhase") { - let at = g.children.iter().position(|c| c.match_id() == "phase").map_or(g.children.len(), |i| i + 1); - g.children.insert(at, wiggle_spatial_phase(ids).into()); - changed = true; + let mut todo = vec![contents]; + while let Some(group) = todo.pop() { + for n in &mut group.children { + let Node::Group(g) = n else { continue }; + if g.match_id == "wiggle" && !g.children.iter().any(|c| c.match_id() == "spatialPhase") { + let at = g.children.iter().position(|c| c.match_id() == "phase").map_or(g.children.len(), |i| i + 1); + g.children.insert(at, wiggle_spatial_phase(ids).into()); + changed = true; + } + todo.push(g); } - changed |= upgrade_shape_contents(ids, g); } changed } diff --git a/crates/project/src/tests.rs b/crates/project/src/tests.rs index fe36c7724..f9ee16167 100644 --- a/crates/project/src/tests.rs +++ b/crates/project/src/tests.rs @@ -290,3 +290,35 @@ fn comp_contains_ends_on_cycles_long_chains_and_shared_precomps() { assert!(p.comp_contains(c[0], c[n - 1])); assert!(!p.comp_contains(c[n - 1], c[0])); } + +/// A Wiggle Paths 100 000 groups deep gets Spatial Phase on a 2 MiB stack. +#[test] +fn upgrading_deeply_nested_shape_groups_fits_a_small_stack() { + let n = 100_000; + let mut next = 1; + let mut contents = build::shape_simple_op(&mut Ids(&mut next), "wiggle").unwrap(); + contents.children.retain(|c| c.match_id() != "spatialPhase"); + for _ in 0..n { + let uid = Ids(&mut next).alloc(); + contents = crate::PropGroup::new(uid, "group", "Group 1").with(contents); + } + let h = std::thread::Builder::new() + .stack_size(2 << 20) + .spawn(move || { + let added = build::upgrade_shape_contents(&mut Ids(&mut next), &mut contents); + (added, contents) + }) + .unwrap(); + let (added, mut g) = h.join().unwrap(); + assert!(added); + // One level at a time: dropping the whole tree would recurse as deep as it is nested. + let mut depth = 0; + while g.match_id == "group" { + let Some(crate::Node::Group(child)) = g.children.pop() else { panic!("a nested group") }; + g = child; + depth += 1; + } + assert_eq!(depth, n); + let ids: Vec<&str> = g.children.iter().map(|c| c.match_id()).collect(); + assert_eq!(ids, ["size", "detail", "points", "speed", "correlation", "phase", "spatialPhase", "seed"]); +}