From 65b221192e531794ab49371b3042f372b7db686d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 04:27:47 -0800 Subject: [PATCH] Effects: a layer's effects gather its sublayers' ruler guides in one pass Evaluating a layer's Pathfinder or geometry effects asked each member whether a sublayer below it held a ruler guide, then asked the members of every selected sublayer or group the same question again on the way down. A sublayer nested N deep was scanned once per layer or group above it, so a chain of N sublayers cost N(N+1)/2 scans. One read walk now looks at each member once at most and records which members hold or lead to a sublayer's guides, and the move visits and copies only those. As before, the groups inside a sublayer that holds no guide are not looked into. The guides taken, their order, their hidden and locked state, the art left behind and the sharing of untouched members are the same as before. --- crates/effects/src/group.rs | 112 +++++++++++++++++++++++------- crates/effects/src/group/tests.rs | 85 +++++++++++++++++++++++ 2 files changed, 170 insertions(+), 27 deletions(-) diff --git a/crates/effects/src/group.rs b/crates/effects/src/group.rs index bb2f605a91..f8b2c00713 100644 --- a/crates/effects/src/group.rs +++ b/crates/effects/src/group.rs @@ -734,35 +734,93 @@ fn item_art(id: NodeId, item: &AppearanceItem, shapes: &[(PathData, FillRule)], Some(g) } -/// Whether a sublayer among the members of `n`, or among its groups' members, holds a ruler guide. -fn sublayer_guides(n: &Node) -> bool { - n.children().is_some_and(|ch| ch.iter().any(|c| c.holds_ruler_guides() || (!c.is_layer() && sublayer_guides(c)))) +/// The members of a container that [`take_held_guides`] visits, by index, each with the same for +/// its own members. A ruler guide's entry is empty; a container's is not. +#[derive(Default)] +struct HeldGuides(Vec<(usize, HeldGuides)>); + +/// The members of `n` holding or leading to the ruler guides of sublayers, found by looking at each +/// member at every depth once: the ruler guides (`is_guide`) of `n` when it is a layer, each +/// sublayer holding one directly or in its sublayers, and each member of another kind with such a +/// sublayer among its members or its groups' members. With them, whether `n` is a layer holding a +/// ruler guide directly or in its sublayers. The other members of a layer holding none are not +/// looked into, unless the layer is the one whose sublayers are gathered (`top`). +fn held_guides(n: &Node, top: bool, is_guide: &impl Fn(&Node) -> bool) -> (bool, HeldGuides) { + let layer = n.is_layer(); + let (mut holds, mut held, mut other) = (false, vec![], vec![]); + for (i, c) in n.children().into_iter().flatten().enumerate() { + if is_guide(c) { + if layer { + holds = true; + held.push((i, HeldGuides::default())); + } + } else if c.is_layer() { + let (sub_holds, inner) = held_guides(c, false, is_guide); + if sub_holds { + holds = true; + held.push((i, inner)); + } + } else { + other.push((i, c)); + } + } + if !layer || holds || top { + for (i, c) in other { + let (_, inner) = held_guides(c, false, is_guide); + if !inner.0.is_empty() { + held.push((i, inner)); + } + } + held.sort_unstable_by_key(|(i, _)| *i); + } + (layer && holds, HeldGuides(held)) +} + +/// Move the ruler guides of `n` and of its members that [`held_guides`] found (`held`) to `out`: +/// those of `n` first, in order, then those of each member in turn. A guide taken from a hidden or +/// locked container, or from one inside such a container, is hidden or locked itself. Only the +/// containers in `held` are copied, and only those are visited. +fn take_held_guides(n: &mut Node, held: HeldGuides, out: &mut Vec>, hidden: bool, locked: bool) { + let Some(ch) = n.children_mut() else { return }; + let (mut own, mut deeper) = (vec![], vec![]); + for (i, inner) in held.0 { + if inner.0.is_empty() { + own.push(i); + continue; + } + let Some(c) = ch.get_mut(i) else { continue }; + let c = Arc::make_mut(c); + let (hidden, locked) = (hidden || !c.visible, locked || c.locked); + take_held_guides(c, inner, &mut deeper, hidden, locked); + } + if !own.is_empty() { + let mut own = own.into_iter().peekable(); + for (i, mut c) in std::mem::take(ch).into_iter().enumerate() { + if own.next_if_eq(&i).is_none() { + ch.push(c); + continue; + } + if hidden || locked { + let g = Arc::make_mut(&mut c); + g.visible &= !hidden; + g.locked |= locked; + } + out.push(c); + } + } + out.append(&mut deeper); } /// Move the ruler guides of the sublayers among the members of `n`, and among its groups' -/// members, to `out`, in order. A guide taken from a hidden or locked sublayer, or from one inside -/// such a sublayer or group, is hidden or locked itself. Only containers holding one are copied. -fn take_sublayer_guides(n: &mut Node, out: &mut Vec>, hidden: bool, locked: bool) { - let Some(ch) = n.children_mut() else { return }; - for c in ch.iter_mut().filter(|c| c.holds_ruler_guides() || (!c.is_layer() && sublayer_guides(c))) { - let c = Arc::make_mut(c); - let (hidden, locked) = (hidden || !c.visible, locked || c.locked); - if c.is_layer() - && let Some(lc) = c.children_mut() - { - let (taken, art): (Vec<_>, Vec<_>) = std::mem::take(lc).into_iter().partition(|g| g.is_ruler_guide()); - *lc = art; - out.extend(taken.into_iter().map(|mut g| { - if hidden || locked { - let g = Arc::make_mut(&mut g); - g.visible &= !hidden; - g.locked |= locked; - } - g - })); - } - take_sublayer_guides(c, out, hidden, locked); - } +/// members, to `out`, in order: a sublayer's own guides, then those of its members. A guide taken +/// from a hidden or locked sublayer, or from one inside such a sublayer or group, is hidden or +/// locked itself. Each member at every depth is looked at once at most (`is_guide`), and only +/// containers holding one are copied. +fn take_sublayer_guides(n: &mut Node, out: &mut Vec>, is_guide: &impl Fn(&Node) -> bool) { + let (_, mut held) = held_guides(n, true, is_guide); + // The ruler guides of `n` itself stay where they are. + held.0.retain(|(_, inner)| !inner.0.is_empty()); + take_held_guides(n, held, out, false, false); } /// `n`, a group or layer, as plain art: its Pathfinder effects evaluated, its geometry effects @@ -790,7 +848,7 @@ pub fn evaluate_container(doc: Option<&Document>, n: &Node) -> Option { // So do its sublayers' guides, which would otherwise go with the art the effects replace or // copy: they stay on this layer, hidden and locked as their sublayers were. if m.is_layer() && (pathfinder || !fx.is_empty()) { - take_sublayer_guides(&mut m, &mut guides, false, false); + take_sublayer_guides(&mut m, &mut guides, &Node::is_ruler_guide); } if pathfinder { let hook = |c: &Node| crate::outline_object(doc, c); diff --git a/crates/effects/src/group/tests.rs b/crates/effects/src/group/tests.rs index c6e2a78c73..340ca9d4bc 100644 --- a/crates/effects/src/group/tests.rs +++ b/crates/effects/src/group/tests.rs @@ -1757,3 +1757,88 @@ fn a_sublayers_ruler_guide_in_a_hidden_or_locked_group_stays_hidden_or_locked() assert_eq!(guides, [(NodeId(20), !hidden, locked)], "hidden {hidden}, locked {locked}"); } } + +/// Gathering the ruler guides of nested sublayers, and of a sublayer inside nested groups, looks +/// at each member at every depth once at most, and takes the same guides, in the same order and state, as +/// a layer's effects keep. +#[test] +fn gathering_sublayer_ruler_guides_looks_at_each_member_once() { + use std::cell::Cell; + use vectorcraft_doc::LayerColor; + const CHAIN: u64 = 6; + let guide = |id| Arc::new(Node::new(NodeId(id), NodeKind::RulerGuide { vertical: true, pos: id as f64, artboard: None })); + let layer = |id, children: Vec>| { + let mut l = Node::layer(NodeId(id), "Sub", LayerColor::Preset(1)); + if let Some(ch) = l.children_mut() { + *ch = children; + } + l + }; + // Sublayers nested CHAIN deep, each holding the next before its own art and guide. The second + // is hidden and the fourth locked. + let first_guide = guide(201); + let mut chain = layer(100 + CHAIN, vec![sq(300 + CHAIN, 0.0, Color::BLACK), guide(200 + CHAIN)]); + for depth in (1..CHAIN).rev() { + let own = if depth == 1 { first_guide.clone() } else { guide(200 + depth) }; + chain = layer(100 + depth, vec![Arc::new(chain), sq(300 + depth, 0.0, Color::BLACK), own]); + (chain.visible, chain.locked) = (depth != 2, depth == 4); + } + let second_guide = chain.children().and_then(|c| c[0].children()).map(|c| c[2].clone()).unwrap(); + // A sublayer inside five nested groups, the third of them locked. + let mut groups = layer(450, vec![guide(250), sq(350, 0.0, Color::BLACK)]); + for depth in (1..=5).rev() { + groups = Node::group(NodeId(400 + depth), vec![Arc::new(groups)]); + groups.locked = depth == 3; + } + let free = Arc::new(Node::group(NodeId(500), vec![sq(501, 0.0, Color::BLACK), sq(502, 5.0, Color::BLACK)])); + // A sublayer with no guide: the art in its group is not looked at. + let quiet = Node::group(NodeId(601), vec![sq(602, 0.0, Color::BLACK), sq(603, 5.0, Color::BLACK)]); + let quiet = Arc::new(layer(600, vec![Arc::new(quiet)])); + let mut root = layer(10, vec![sq(1, 0.0, Color::rgb(1.0, 0.0, 0.0)), Arc::new(chain), Arc::new(groups), free.clone(), quiet.clone(), guide(20)]); + fn members(n: &Node) -> usize { + n.children().into_iter().flatten().map(|c| 1 + members(c)).sum() + } + let expected: Vec<_> = (1..=CHAIN).map(|depth| (NodeId(200 + depth), depth < 2, depth >= 4)).chain([(NodeId(250), true, true)]).collect(); + let state = |g: &[Arc]| g.iter().map(|g| (g.id, g.visible, g.locked)).collect::>(); + + let mut m = root.clone(); + let looked = Cell::new(0_usize); + let is_guide = |n: &Node| { + looked.set(looked.get() + 1); + n.is_ruler_guide() + }; + let mut taken = vec![]; + take_sublayer_guides(&mut m, &mut taken, &is_guide); + assert_eq!(looked.get(), members(&root) - 2, "each member is looked at once, none in the quiet sublayer's group"); + assert_eq!(state(&taken), expected, "a sublayer's own guides, then its sublayers', each once"); + assert!(Arc::ptr_eq(&taken[0], &first_guide), "a guide with nothing to change is not copied"); + assert!(!Arc::ptr_eq(&taken[1], &second_guide), "a guide made hidden is copied"); + assert!(second_guide.visible, "the guide as it was stays as it was"); + let ch = m.children().unwrap(); + assert!(Arc::ptr_eq(&ch[3], &free), "a member with no sublayer guide is not copied"); + assert!(Arc::ptr_eq(&ch[4], &quiet), "a sublayer with no guide is not copied"); + assert!(ch[5].is_ruler_guide(), "the layer's own guide stays where it is"); + let ids = |n: &Node| n.children().into_iter().flatten().map(|c| c.id).collect::>(); + assert_eq!(ids(&ch[1]), [NodeId(102), NodeId(301)], "the sublayer's art stays, in order"); + assert_eq!(ids(&root.children().unwrap()[1]), [NodeId(102), NodeId(301), NodeId(201)], "the source keeps its guides"); + + fn found(n: &Node, out: &mut Vec) { + for c in n.children().into_iter().flatten() { + if c.is_ruler_guide() { + out.push(c.id); + } + found(c, out); + } + } + for (effect, params) in [("pathfinder.add", serde_json::json!({})), ("distort.transform", serde_json::json!({"moveH": 30, "copies": 2}))] { + root.appearance.effects = vec![Effect { id: effect.into(), params, visible: true }]; + let art = evaluate_container(None, &root).unwrap(); + let ch = art.children().unwrap(); + let tail = &ch[ch.len() - expected.len() - 1..]; + assert_eq!(tail[0].id, NodeId(20), "{effect}: the layer's own guide comes first"); + assert_eq!(state(&tail[1..]), expected, "{effect}: then the sublayers' guides"); + let mut all = vec![]; + found(&art, &mut all); + assert_eq!(all.len(), expected.len() + 1, "{effect}: each guide once"); + } +}