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"); + } +}