From be270420602199454ca23d34687dfaadb132ecae Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 13:32:57 -0800 Subject: [PATCH 1/2] Guides: sublayer guides survive layer effects and the canvas check - A layer with a Pathfinder effect, or a Transform effect that copies its art, keeps the ruler guides of its sublayers on the layer, hidden and locked as their sublayers were, instead of losing them with the art. - Opening a file counts the ruler guides of sublayers removed beyond the canvas. - Opening a file moves misplaced ruler guides above the art before the canvas check, so a guide saved before a clipping path beyond the canvas no longer makes other art the clipping path. Follows #6632. --- crates/doc/src/canvas_range.rs | 46 ++++++++++++++++++++++++++----- crates/effects/src/group.rs | 38 ++++++++++++++++++++++++- crates/effects/src/group/tests.rs | 41 +++++++++++++++++++++++++++ crates/format/src/lib.rs | 38 +++++++++++++++++++++++-- 4 files changed, 152 insertions(+), 11 deletions(-) diff --git a/crates/doc/src/canvas_range.rs b/crates/doc/src/canvas_range.rs index e4f267a8b..491b79f15 100644 --- a/crates/doc/src/canvas_range.rs +++ b/crates/doc/src/canvas_range.rs @@ -15,7 +15,8 @@ pub struct BeyondCanvas { /// Objects removed whole (a layer or group keeps its children within the canvas). pub objects: usize, pub artboards: usize, - /// Guides beyond the canvas, and the artboard guides of the artboards removed. + /// Guides beyond the canvas, the artboard guides of the artboards removed, and the layer + /// guides of the layers removed. pub guides: usize, /// The ruler origin was beyond the canvas and is back at the origin. pub ruler_origin: bool, @@ -100,11 +101,20 @@ fn layers_in(n: &Node, layers: &mut Vec) { } } +/// How many ruler guides the layers in `n` hold, if it is a layer or group. +fn guides_in(n: &Node) -> usize { + match clipping(n) { + None => 0, + Some(_) => n.children().into_iter().flatten().map(|c| if c.is_ruler_guide() { 1 } else { guides_in(c) }).sum(), + } +} + /// Remove the objects in `node` that reach beyond the canvas: a layer or group keeps those of its /// objects within it, unless its clipping path reaches beyond (see [`clips_beyond`]); anything /// else goes whole. Only containers holding such an object are copied. Recursion follows the -/// nesting, which [`Document::limit_depth`] bounds. The layers removed are added to `layers`. -fn drop_children_beyond(node: &mut Arc, dropped: &mut usize, layers: &mut Vec) { +/// nesting, which [`Document::limit_depth`] bounds. The layers removed are added to `layers`, and +/// the ruler guides they held to `guides`. +fn drop_children_beyond(node: &mut Arc, dropped: &mut usize, layers: &mut Vec, guides: &mut usize) { if !node.children().is_some_and(|c| c.iter().any(|c| reaches_beyond(c))) { return; } @@ -114,10 +124,11 @@ fn drop_children_beyond(node: &mut Arc, dropped: &mut usize, layers: &mut return true; } if clipping(c).is_some() && !clips_beyond(c) { - drop_children_beyond(c, dropped, layers); + drop_children_beyond(c, dropped, layers, guides); return true; } layers_in(c, layers); + *guides += guides_in(c); *dropped += 1; false }); @@ -159,13 +170,14 @@ impl Document { for c in children.iter() { layers_in(c, &mut layers); } - // The layer's own ruler guides stay; only its art goes. + // The layer's own ruler guides stay; only its art goes, with its sublayers' guides. gone.objects += children.iter().filter(|c| !c.is_ruler_guide()).count(); + gone.guides += children.iter().filter(|c| !c.is_ruler_guide()).map(|c| guides_in(c)).sum::(); children.retain(|c| c.is_ruler_guide()); *clip = false; } } else { - drop_children_beyond(layer, &mut gone.objects, &mut layers); + drop_children_beyond(layer, &mut gone.objects, &mut layers, &mut gone.guides); } } if gone.objects > 0 { @@ -184,7 +196,7 @@ impl Document { let guides = self.saved_guides.len(); self.saved_guides .retain(|g| in_range(g.pos) && g.artboard.is_none_or(|a| !removed.contains(&a)) && g.layer.is_none_or(|l| !layers.contains(&l))); - gone.guides = guides - self.saved_guides.len(); + gone.guides += guides - self.saved_guides.len(); let out = |n: &Node| match n.kind { NodeKind::RulerGuide { pos, artboard, .. } => !in_range(pos) || artboard.is_some_and(|a| removed.contains(&a)), _ => false, @@ -324,6 +336,21 @@ mod tests { let layer = d2.layers[0].id.0; assert_eq!(d2.drop_beyond_canvas().objects, 1); assert_eq!(ids(&d2), [layer, 10]); + // The guides of a sublayer emptied out of a top-level layer go with it, counted. + let probe = Document::new(1.0, 1.0); + let sub = sublayer(&probe, 40, vec![rect(41, 0.0)], false); + let mut d3 = doc(vec![rect(10, 1e17), sub]); + if let NodeKind::Layer { clip, .. } = &mut Arc::make_mut(&mut d3.layers[0]).kind { + *clip = true; + } + d3.saved_guides = + vec![Guide { layer: Some(NodeId(40)), ..Guide::new(false, 5.0) }, Guide { layer: Some(d3.layers[0].id), ..Guide::new(true, 5.0) }]; + let mut nodes = d3.clone(); + assert_eq!(nodes.guides_into_nodes(), 2); + let gone = d3.drop_beyond_canvas(); + assert_eq!((gone.objects, gone.guides), (2, 1), "the clipping path, the sublayer and its guide; the layer's own guide stays"); + assert_eq!(nodes.drop_beyond_canvas(), gone); + assert_eq!(nodes.ruler_guides().len(), 1); assert_eq!(d.drop_beyond_canvas(), BeyondCanvas::default()); } @@ -353,8 +380,13 @@ mod tests { ]; d.variables.bindings.insert(NodeId(11), ["v".to_string()].into()); d.variables.bindings.insert(NodeId(10), ["w".to_string()].into()); + // The same guides as nodes, as a document has them once read. + let mut nodes = d.clone(); + assert_eq!(nodes.guides_into_nodes(), 2); let gone = d.drop_beyond_canvas(); assert_eq!((gone.objects, gone.guides), (2, 1), "the far rectangle, the clipping sublayer and its layer guide"); + assert_eq!(nodes.drop_beyond_canvas(), gone, "a guide node goes with its sublayer, counted as a saved one is"); + assert_eq!(nodes.ruler_guides().len(), 1); assert_eq!(d.saved_guides.len(), 1, "the guide of a layer that stays, stays"); assert_eq!(d.saved_selections[0].objects, [NodeId(10)]); assert_eq!(d.assets.iter().map(|a| (a.id, a.nodes.clone())).collect::>(), [(2, vec![NodeId(10)])]); diff --git a/crates/effects/src/group.rs b/crates/effects/src/group.rs index 99e9d9e6f..73abbffb2 100644 --- a/crates/effects/src/group.rs +++ b/crates/effects/src/group.rs @@ -719,6 +719,37 @@ 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)))) +} + +/// 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, 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) = if c.is_layer() { (hidden || !c.visible, locked || c.locked) } else { (hidden, 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); + } +} + /// `n`, a group or layer, as plain art: its Pathfinder effects evaluated, its geometry effects /// reshaping all its members as one piece (the whole art's bounds as their reference box), and its /// own fills and strokes turned into art painting the members' geometry ([`member_shapes`]) below @@ -739,7 +770,12 @@ pub fn evaluate_container(doc: Option<&Document>, n: &Node) -> Option { } let mut m = n.clone(); // A layer's ruler guides take no part: they go back above the evaluated art at the end. - let guides = m.children_mut().map(|ch| ch.split_off(Node::guide_run_start(ch))).unwrap_or_default(); + 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()) { + take_sublayer_guides(&mut m, &mut guides, false, false); + } if pathfinder { let hook = |c: &Node| crate::outline_object(doc, c); let result = pathfinder_children(doc, &m, Some(&hook)).unwrap_or_default(); diff --git a/crates/effects/src/group/tests.rs b/crates/effects/src/group/tests.rs index bf702b273..ef242c5dc 100644 --- a/crates/effects/src/group/tests.rs +++ b/crates/effects/src/group/tests.rs @@ -862,3 +862,44 @@ fn a_layers_ruler_guides_stay_last_through_its_effects() { painted.appearance.items = Appearance::basic(Paint::solid(Color::BLACK), Paint::None, 0.0).items; assert_eq!(tail(&evaluate_container(None, &painted).unwrap()), guides); } + +/// The ruler guides of a sublayer in a layer with a Pathfinder effect, or a Transform effect that +/// copies its art, stay on a layer of the evaluated art, once, hidden and locked as the sublayer is. +#[test] +fn a_sublayers_ruler_guides_survive_its_layers_effects() { + use vectorcraft_doc::LayerColor; + let layer = |effect: &str, params: serde_json::Value, shown: bool| { + let mut sub = Node::layer(NodeId(11), "Sub", LayerColor::Preset(1)); + (sub.visible, sub.locked) = (shown, !shown); + if let Some(ch) = sub.children_mut() { + *ch = vec![ + sq(2, 5.0, Color::rgb(0.0, 0.0, 1.0)), + Arc::new(Node::new(NodeId(20), NodeKind::RulerGuide { vertical: true, pos: 3.0, artboard: None })), + ]; + } + 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)), Arc::new(sub)]; + } + l.appearance.effects.push(Effect { id: effect.into(), params, visible: true }); + l + }; + // The guides found where ruler guides are looked for: directly in a layer or sublayer. + fn listed(n: &Node, out: &mut Vec<(NodeId, bool, bool)>) { + for c in n.children().into_iter().flatten() { + if c.is_ruler_guide() { + out.push((c.id, c.visible, c.locked)); + } else if c.is_layer() { + listed(c, out); + } + } + } + for (effect, params) in [("pathfinder.add", serde_json::json!({})), ("distort.transform", serde_json::json!({"moveH": 30, "copies": 2}))] { + for shown in [true, false] { + let art = evaluate_container(None, &layer(effect, params.clone(), shown)).unwrap(); + let mut found = vec![]; + listed(&art, &mut found); + assert_eq!(found, [(NodeId(20), shown, !shown)], "{effect}: the sublayer's guide, once, on a layer"); + } + } +} diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 38545b829..cd64fcbb5 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -594,6 +594,10 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result doc.raster_effects.add_around = doc.raster_effects.add_around.clamp(0.0, vectorcraft_doc::rastersettings::MAX_ADD_AROUND); let mut depth_notes = vec![]; doc.limit_depth_noting(&mut depth_notes); + // Untrusted: ruler guide nodes only directly in layers, above their art (after the depth limit: + // the walk recurses; before the canvas check, which takes a layer's first member as its + // clipping path). + doc.tidy_ruler_guides(); // Untrusted: nothing beyond the canvas, where no edit may leave anything (after the depth // limit: the walk recurses). A file whose artboards all lie there has no page to show (#3575). let had_artboards = !doc.artboards.is_empty(); @@ -605,9 +609,6 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result ))); } depth_notes.extend(beyond.note()); - // Untrusted: ruler guide nodes only directly in layers, above their art (after the depth limit: - // the walk recurses). - doc.tidy_ruler_guides(); // The saved ruler guides become nodes on their layers (after the guides beyond the canvas are // dropped, and after `fix_next_id`: new ids start above the file's). doc.guides_into_nodes(); @@ -1385,4 +1386,35 @@ mod tests { assert_eq!(base64_decode("QUI="), Some(vec![0x41, 0x42])); assert_eq!(base64_decode("QUJD"), Some(vec![0x41, 0x42, 0x43])); } + + /// A layer clipping by a path beyond the canvas is emptied on open and stops clipping, also + /// when the file puts a ruler guide before the clipping path: no other object becomes it. + #[test] + fn a_guide_before_a_clipping_path_beyond_the_canvas_does_not_change_the_clipping_path() { + let opened = |guide_first: bool| { + let mut d = Document::new(100.0, 100.0); + let rect = |d: &mut Document, y: f64| { + Arc::new(Node::path(d.alloc_id(), shapes::rectangle(Rect::new(0.0, y, 10.0, y + 10.0)), Appearance::default_art())) + }; + let (far, near) = (rect(&mut d, 1e17), rect(&mut d, 0.0)); + if let NodeKind::Layer { children, clip, .. } = &mut Arc::make_mut(&mut d.layers[0]).kind { + *children = vec![far, near]; + *clip = true; + } + // Saving writes ruler guides to the saved list: the guide node is put in the file. + let mut v: serde_json::Value = serde_json::from_slice(&save(&d, false)).unwrap(); + let guide = serde_json::json!({"id": 50, "kind": {"type": "rulerguide", "vertical": true, "pos": 5.0}}); + let kids = v["document"]["layers"][0]["kind"]["children"].as_array_mut().unwrap(); + if guide_first { + kids.insert(0, guide) + } else { + kids.push(guide) + } + let d = load(&serde_json::to_vec(&v).unwrap()).unwrap(); + let NodeKind::Layer { children, clip, .. } = &d.layers[0].kind else { panic!("a layer") }; + (*clip, children.iter().map(|c| c.is_ruler_guide()).collect::>()) + }; + assert_eq!(opened(false), (false, vec![true]), "the clipping path and what it clipped go; the guide stays"); + assert_eq!(opened(true), opened(false)); + } } From 7a9a6bd98e4ba5b75d4348d8cbc8ded3fe6d2704 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 14:23:56 -0800 Subject: [PATCH 2/2] Guides: a sublayer guide in a hidden or locked group keeps that state When a layer effect moves a sublayer's ruler guides onto the layer, a guide inside a hidden or locked group now stays hidden or locked, as it does for a hidden or locked sublayer. --- crates/effects/src/group.rs | 4 ++-- crates/effects/src/group/tests.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/effects/src/group.rs b/crates/effects/src/group.rs index 73abbffb2..57f2fc861 100644 --- a/crates/effects/src/group.rs +++ b/crates/effects/src/group.rs @@ -726,12 +726,12 @@ fn sublayer_guides(n: &Node) -> bool { /// 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, is hidden or locked itself. Only containers holding one are copied. +/// 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) = if c.is_layer() { (hidden || !c.visible, locked || c.locked) } else { (hidden, locked) }; + let (hidden, locked) = (hidden || !c.visible, locked || c.locked); if c.is_layer() && let Some(lc) = c.children_mut() { diff --git a/crates/effects/src/group/tests.rs b/crates/effects/src/group/tests.rs index ef242c5dc..16e893ce9 100644 --- a/crates/effects/src/group/tests.rs +++ b/crates/effects/src/group/tests.rs @@ -903,3 +903,29 @@ fn a_sublayers_ruler_guides_survive_its_layers_effects() { } } } + +/// A sublayer's ruler guide inside a hidden or locked group stays hidden or locked when its layer's +/// Pathfinder effect moves it onto the layer, as the group made it. +#[test] +fn a_sublayers_ruler_guide_in_a_hidden_or_locked_group_stays_hidden_or_locked() { + use vectorcraft_doc::LayerColor; + for (hidden, locked) in [(true, false), (false, true), (false, false)] { + let mut sub = Node::layer(NodeId(11), "Sub", LayerColor::Preset(1)); + if let Some(ch) = sub.children_mut() { + *ch = vec![ + sq(2, 5.0, Color::rgb(0.0, 0.0, 1.0)), + Arc::new(Node::new(NodeId(20), NodeKind::RulerGuide { vertical: true, pos: 3.0, artboard: None })), + ]; + } + let mut group = Node::group(NodeId(12), vec![Arc::new(sub)]); + (group.visible, group.locked) = (!hidden, locked); + 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)), Arc::new(group)]; + } + l.appearance.effects.push(Effect { id: "pathfinder.add".into(), params: serde_json::json!({}), visible: true }); + let art = evaluate_container(None, &l).unwrap(); + let guides: Vec<_> = art.children().into_iter().flatten().filter(|c| c.is_ruler_guide()).map(|c| (c.id, c.visible, c.locked)).collect(); + assert_eq!(guides, [(NodeId(20), !hidden, locked)], "hidden {hidden}, locked {locked}"); + } +}