diff --git a/crates/effects/src/group.rs b/crates/effects/src/group.rs index 1b64971a3a..d2a6687dd7 100644 --- a/crates/effects/src/group.rs +++ b/crates/effects/src/group.rs @@ -742,45 +742,28 @@ fn item_art(id: NodeId, item: &AppearanceItem, shapes: &[(PathData, FillRule)], } /// 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. +/// its own members. A ruler guide's entry is empty, whether its container is a layer or a group; 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![]); +/// The members of `n` that are ruler guides (`is_guide`), and those holding one at any depth, in +/// order. Every member at every depth is looked at once: a sublayer or group with no guide of its +/// own is still looked into, and a guide directly in a group counts as one in a layer does. A +/// member holding no guide gets no entry. +fn held_guides(n: &Node, is_guide: &impl Fn(&Node) -> bool) -> HeldGuides { + let mut held = 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)); + held.push((i, HeldGuides::default())); + continue; + } + let inner = held_guides(c, is_guide); + if !inner.0.is_empty() { + held.push((i, inner)); } } - 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)) + HeldGuides(held) } /// Move the ruler guides of `n` and of its members that [`held_guides`] found (`held`) to `out`: @@ -818,15 +801,14 @@ fn take_held_guides(n: &mut Node, held: HeldGuides, out: &mut Vec>, hi 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 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. +/// Move the ruler guides of `n` and those held at any depth by the sublayers and groups among its +/// members to `out`, in order: a container's own guides, then those of its members. A sublayer or +/// group with no guide of its own still gives up those of its members. A guide taken from a hidden +/// or locked sublayer or group, or from one inside such a container, is hidden or locked itself. +/// Each member at every depth is looked at once (`is_guide`), and only containers holding a guide +/// 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()); + let held = held_guides(n, is_guide); take_held_guides(n, held, out, false, false); } @@ -850,12 +832,15 @@ pub fn evaluate_container(doc: Option<&Document>, n: &Node) -> Option { return None; } let mut m = n.clone(); - // A layer's ruler guides take no part: they go back above the evaluated art at the end. - let mut guides = m.children_mut().map(|ch| ch.split_off(Node::guide_run_start(ch))).unwrap_or_default(); - // 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()) { + // Its ruler guides take no part: they go back above the evaluated art at the end. With effects + // that replace or copy the art, so do its guides among the art and those in its sublayers and + // groups, which would otherwise go with that art: they stay on `n`, in order, hidden and + // locked as their containers were. + let mut guides = vec![]; + if pathfinder || !fx.is_empty() { take_sublayer_guides(&mut m, &mut guides, &Node::is_ruler_guide); + } else if let Some(ch) = m.children_mut() { + guides = ch.split_off(Node::guide_run_start(ch)); } 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 519323e5f0..50fbf8f759 100644 --- a/crates/effects/src/group/tests.rs +++ b/crates/effects/src/group/tests.rs @@ -1808,7 +1808,7 @@ fn a_sublayers_ruler_guide_in_a_hidden_or_locked_group_stays_hidden_or_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 +/// at each member at every depth once, 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() { @@ -1840,7 +1840,7 @@ fn gathering_sublayer_ruler_guides_looks_at_each_member_once() { 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. + // A sublayer with no guide, its group holding none either. 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)]); @@ -1858,15 +1858,16 @@ fn gathering_sublayer_ruler_guides_looks_at_each_member_once() { }; 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_eq!(looked.get(), members(&root), "each member is looked at once, those of guide-free containers included"); + assert_eq!(taken[0].id, NodeId(20), "the layer's own guide comes first"); + assert_eq!(state(&taken[1..]), expected, "a sublayer's own guides, then its sublayers', each once"); + assert!(Arc::ptr_eq(&taken[1], &first_guide), "a guide with nothing to change is not copied"); + assert!(!Arc::ptr_eq(&taken[2], &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_eq!(ch.len(), 5, "the layer's own guide is taken"); 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"); @@ -1892,6 +1893,140 @@ fn gathering_sublayer_ruler_guides_looks_at_each_member_once() { } } +/// Where each ruler guide in `n` is, at every depth, with its position, visibility and lock. +fn ruler_guides_in(n: &Node, out: &mut Vec<(NodeId, NodeKind, bool, bool)>) { + for c in n.children().into_iter().flatten() { + if c.is_ruler_guide() { + out.push((c.id, c.kind.clone(), c.visible, c.locked)); + } + ruler_guides_in(c, out); + } +} + +/// A ruler guide in a layer inside a group inside a sublayer that holds no guide of its own, at +/// any depth of that pattern, survives its layer's Pathfinder effect or Transform copies once, last +/// in the layer, hidden or locked when any container on its way was. +#[test] +fn a_ruler_guide_below_a_guide_free_sublayer_and_a_group_survives_its_layers_effects() { + use vectorcraft_doc::LayerColor; + let layer = |id, children: Vec>, (hidden, locked): (bool, bool)| { + let mut l = Node::layer(NodeId(id), "Sub", LayerColor::Preset(1)); + if let Some(ch) = l.children_mut() { + *ch = children; + } + (l.visible, l.locked) = (!hidden, locked); + Arc::new(l) + }; + let group = |id, children: Vec>, (hidden, locked): (bool, bool)| { + let mut g = Node::group(NodeId(id), children); + (g.visible, g.locked) = (!hidden, locked); + Arc::new(g) + }; + let kind = NodeKind::RulerGuide { vertical: false, pos: 7.0, artboard: None }; + let states = [(false, false), (true, false), (false, true)]; + for (effect, params) in [("pathfinder.add", serde_json::json!({})), ("distort.transform", serde_json::json!({"moveH": 30, "copies": 2}))] { + for depth in 1..=3_u64 { + for sub in states { + for grp in states { + for inner in states { + // Layer 10 → sublayer → group → … → layer holding the guide, the pattern + // repeated `depth` times; only the innermost layer holds a guide. + let mut n = layer(900, vec![sq(901, 0.0, Color::BLACK), Arc::new(Node::new(NodeId(20), kind.clone()))], inner); + for d in (0..depth).rev() { + let (s, g) = if d == 0 { (sub, grp) } else { ((false, false), (false, false)) }; + n = group(700 + d, vec![sq(710 + d, 5.0, Color::BLACK), n], g); + n = layer(800 + d, vec![n, sq(810 + d, 2.0, Color::BLACK)], s); + } + let mut root = Node::layer(NodeId(10), "Layer 1", LayerColor::Preset(0)); + if let Some(ch) = root.children_mut() { + *ch = vec![sq(1, 0.0, Color::rgb(1.0, 0.0, 0.0)), n]; + } + root.appearance.effects.push(Effect { id: effect.into(), params: params.clone(), visible: true }); + let art = evaluate_container(None, &root).unwrap(); + let (hidden, locked) = (sub.0 || grp.0 || inner.0, sub.1 || grp.1 || inner.1); + let mut all = vec![]; + ruler_guides_in(&art, &mut all); + let case = format!("{effect}, depth {depth}, sublayer {sub:?}, group {grp:?}, layer {inner:?}"); + assert_eq!(all, [(NodeId(20), kind.clone(), !hidden, locked)], "{case}: the guide, once, unchanged"); + let last = art.children().and_then(|c| c.last()).unwrap(); + assert_eq!(last.id, NodeId(20), "{case}: last in the layer"); + } + } + } + } + } +} + +/// A ruler guide directly in a group, in a layer or in a group with a Pathfinder effect or +/// Transform copies, survives that effect once, last in the container, hidden or locked as its +/// groups were. So does one among a group's art rather than after it. +#[test] +fn a_ruler_guide_in_a_group_survives_its_containers_effects() { + use vectorcraft_doc::LayerColor; + let kind = NodeKind::RulerGuide { vertical: true, pos: 4.0, artboard: None }; + let guide = || Arc::new(Node::new(NodeId(20), kind.clone())); + for (effect, params) in [("pathfinder.add", serde_json::json!({})), ("distort.transform", serde_json::json!({"moveH": 30, "copies": 2}))] { + for (hidden, locked) in [(false, false), (true, false), (false, true)] { + let mut inner = Node::group(NodeId(12), vec![sq(2, 5.0, Color::BLACK), guide(), sq(3, 8.0, Color::BLACK)]); + (inner.visible, inner.locked) = (!hidden, locked); + let outer = Arc::new(Node::group(NodeId(11), vec![Arc::new(inner)])); + let fx = Effect { id: effect.into(), params: params.clone(), visible: true }; + let mut l = Node::layer(NodeId(10), "Layer 1", LayerColor::Preset(0)); + if let Some(ch) = l.children_mut() { + *ch = vec![sq(1, 0.0, Color::rgb(1.0, 0.0, 0.0)), outer.clone()]; + } + l.appearance.effects.push(fx.clone()); + let mut g = Node::group(NodeId(13), vec![sq(1, 0.0, Color::rgb(1.0, 0.0, 0.0)), outer]); + g.appearance.effects.push(fx.clone()); + for (what, n) in [("layer", l), ("group", g)] { + let art = evaluate_container(None, &n).unwrap(); + let mut all = vec![]; + ruler_guides_in(&art, &mut all); + let case = format!("{effect} on a {what}, hidden {hidden}, locked {locked}"); + assert_eq!(all, [(NodeId(20), kind.clone(), !hidden, locked)], "{case}: the guide, once, unchanged"); + assert_eq!(art.children().and_then(|c| c.last()).map(|c| c.id), Some(NodeId(20)), "{case}: last"); + } + } + // A group's guide placed before its art. + let mut g = Node::group(NodeId(13), vec![guide(), sq(1, 0.0, Color::BLACK), sq(2, 5.0, Color::BLACK)]); + g.appearance.effects.push(Effect { id: effect.into(), params: params.clone(), visible: true }); + let art = evaluate_container(None, &g).unwrap(); + let mut all = vec![]; + ruler_guides_in(&art, &mut all); + assert_eq!(all, [(NodeId(20), kind.clone(), true, false)], "{effect}: a group's leading guide, once"); + assert_eq!(art.children().and_then(|c| c.last()).map(|c| c.id), Some(NodeId(20)), "{effect}: moved last"); + } +} + +/// The ruler guides of a layer or group with a Pathfinder effect or Transform copies, one among +/// its art and one after it, go after the evaluated art in the order they were in. +#[test] +fn a_containers_ruler_guides_keep_their_order_through_its_effects() { + use vectorcraft_doc::LayerColor; + let guide = |id| Arc::new(Node::new(NodeId(id), NodeKind::RulerGuide { vertical: false, pos: 3.0, artboard: None })); + for (effect, params) in [("pathfinder.add", serde_json::json!({})), ("distort.transform", serde_json::json!({"moveH": 30, "copies": 2}))] { + let members = || vec![sq(1, 0.0, Color::BLACK), guide(20), sq(2, 5.0, Color::BLACK), guide(21)]; + let fx = Effect { id: effect.into(), params: params.clone(), visible: true }; + let mut l = Node::layer(NodeId(10), "Layer 1", LayerColor::Preset(0)); + if let Some(ch) = l.children_mut() { + *ch = members(); + } + l.appearance.effects.push(fx.clone()); + let mut g = Node::group(NodeId(11), members()); + g.appearance.effects.push(fx); + for (what, n) in [("layer", l), ("group", g)] { + let art = evaluate_container(None, &n).unwrap(); + let mut all = vec![]; + ruler_guides_in(&art, &mut all); + let ids = all.iter().map(|(id, ..)| *id).collect::>(); + assert_eq!(ids, [NodeId(20), NodeId(21)], "{effect} on a {what}: the guides, in order"); + let ch = art.children().unwrap(); + let tail = ch[ch.len() - 2..].iter().map(|c| c.id).collect::>(); + assert_eq!(tail, [NodeId(20), NodeId(21)], "{effect} on a {what}: last, in order"); + } + } +} + /// A group's own Fill + Offset Path below its contents follows the outline of a clip group among /// its members, its clipping path, not the clipped art it hides outside that path (#7289). #[test]