From b12c78a7312a1d15ef2a568c002cbefae2a8e36f Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 13:45:46 -0800 Subject: [PATCH] Graphs: Outline Stroke and a graph left with only labels keep the plot in place - Outline Stroke on a graph's strokes, including Object > Expand with Stroke on, adjusts the bounds the graph keeps, the same way Create Outlines on its labels does. Before, the stroke outlines changed the bounds of the art without type, and the next redraw stretched the plot by the width the outlines added. - A graph that keeps `placedArt` and has no art other than type left is no longer measured on the bounds of all its art. A label edit on such a graph moved and stretched the plot at the next redraw; the plot now stays where it is. - A redraw no longer measures a move or scale on art that has no width or no height now, such as a graph left with one axis line. Before, the plot collapsed to no width. - A clipping group in a graph that holds type is measured on its clip when the clip does not depend on type, so moving art outside the clip does not change the bounds. Blends, envelopes and repeats are measured whole, as they are drawn. - The doc crate's helper is named `graph_art_without_type` and sits with the file's other free functions. - docs/mcp.md lists Outline Stroke and states that moving an outlined label, a swatch, an axis or a tick mark on its own can move or scale the plot, and that a graph left with only labels, or with art that has no width or height, does not follow a move. - Tests: a scale of the graph made before outlining a label, a move of a graph opened from `.ai` after its label is restyled and moved, Outline Stroke after a move, a graph left with only its labels or with one line, and clipping groups holding type. --- crates/doc/src/graph.rs | 65 ++++++----- crates/engine/src/cmd/graph/edit.rs | 36 ++++-- crates/engine/src/cmd/graph/tests.rs | 147 ++++++++++++++++++++++++- crates/engine/src/cmd/pathops.rs | 3 + crates/eps/src/import/ai/read/graph.rs | 2 +- docs/mcp.md | 12 +- 6 files changed, 222 insertions(+), 43 deletions(-) diff --git a/crates/doc/src/graph.rs b/crates/doc/src/graph.rs index 0b726cd0c1..a74f6d0f18 100644 --- a/crates/doc/src/graph.rs +++ b/crates/doc/src/graph.rs @@ -527,7 +527,7 @@ pub struct GraphSpec { /// Bounds of the art when it was last generated (detects moves/scales of the graph group). #[serde(skip_serializing_if = "Option::is_none")] pub placed: Option, - /// Bounds of the art other than type when it was last generated ([`art_bounds`]). Moves and scales of the graph + /// Bounds of the art other than type when it was last generated ([`graph_art_without_type`]). Moves and scales of the graph /// group are measured on these when present; graphs saved without them are measured on [`Self::placed`]. #[serde(skip_serializing_if = "Option::is_none")] pub placed_art: Option, @@ -546,6 +546,43 @@ fn edge_to_edge_before_the_option() -> bool { true } +/// The geometric bounds of a graph's art `children` leaving out their type: the plot, axes, tick marks and legend +/// swatches without the labels. None when there is nothing else. [`GraphSpec::placed_art`] keeps these. +pub fn graph_art_without_type(children: &[std::sync::Arc]) -> Option { + children.iter().filter_map(|c| without_type(c).0).reduce(|a, b| a.union(b)) +} + +/// The geometric bounds of `n` leaving out the type in it, and whether its own geometric bounds depend on that type. A +/// group without such type gives its own bounds, and a clipping group whose clip does not depend on type gives the +/// bounds of its clip. Objects other than groups, layers and compound paths, blends, envelopes and repeats among them, +/// give their own bounds. +fn without_type(n: &crate::Node) -> (Option, bool) { + use crate::NodeKind::{Compound, Group, Layer, Text}; + if matches!(n.kind, Text(_)) { + return (None, true); + } + let children: &[std::sync::Arc] = match &n.kind { + Group { clip: true, children } | Layer { clip: true, children, .. } => children, + Group { .. } | Layer { .. } | Compound { .. } => n.editing_children(), + _ => return (n.geometric_bounds(), false), + }; + let mut bounds: Option = None; + let mut had_type = false; + let mut clip_has_type = false; + let hidden_layer_member = |c: &crate::Node| !c.visible && matches!(n.kind, Layer { clip: false, .. }); + for (i, c) in children.iter().enumerate().filter(|(_, c)| !hidden_layer_member(c)) { + let (b, t) = without_type(c); + had_type |= t; + clip_has_type |= i == 0 && t; + bounds = match (bounds, b) { + (Some(a), Some(b)) => Some(a.union(b)), + (a, b) => a.or(b), + }; + } + let clip = matches!(n.kind, Group { clip: true, .. } | Layer { clip: true, .. }); + if had_type && (!clip || clip_has_type) { (bounds, true) } else { (n.geometric_bounds(), false) } +} + impl GraphSpec { /// The type series `s` is drawn as: its own type when it combines with the graph's, else the graph's. pub fn series_kind(&self, s: usize) -> GraphKind { @@ -614,32 +651,6 @@ impl GraphSpec { } } -/// The geometric bounds of a graph's art `children` leaving out their type: the plot, axes, tick marks and legend -/// swatches without the labels. None when there is nothing else. [`GraphSpec::placed_art`] keeps these. -pub fn art_bounds(children: &[std::sync::Arc]) -> Option { - children.iter().filter_map(|c| art_bounds_of(c).0).reduce(|a, b| a.union(b)) -} - -/// The geometric bounds of `n` leaving out the type in it, and whether there was type to leave out. A group without type -/// gives its own bounds, so a clipping group gives the bounds of its clip. -fn art_bounds_of(n: &crate::Node) -> (Option, bool) { - if matches!(n.kind, crate::NodeKind::Text(_)) { - return (None, true); - } - let Some(children) = n.children() else { return (n.geometric_bounds(), false) }; - let mut bounds: Option = None; - let mut had_type = false; - for c in children { - let (b, t) = art_bounds_of(c); - had_type |= t; - bounds = match (bounds, b) { - (Some(a), Some(b)) => Some(a.union(b)), - (a, b) => a.or(b), - }; - } - if had_type { (bounds, true) } else { (n.geometric_bounds(), false) } -} - impl Default for GraphSpec { fn default() -> Self { Self { diff --git a/crates/engine/src/cmd/graph/edit.rs b/crates/engine/src/cmd/graph/edit.rs index e1c5c336b3..77ce7a7b2a 100644 --- a/crates/engine/src/cmd/graph/edit.rs +++ b/crates/engine/src/cmd/graph/edit.rs @@ -1,4 +1,4 @@ -use vectorcraft_doc::graph::art_bounds; +use vectorcraft_doc::graph::graph_art_without_type as art_bounds; use super::axis::{MAX_TICK_MARKS, count_given, one_line}; use super::data::apply_data; @@ -21,12 +21,16 @@ fn bound_param(p: &Value, key: &str, cmd: &str) -> Result>> { /// The bounds a redraw of graph `n` measures a move or scale on, as kept at the last generation and as they are now: /// those of its art without type ([`GraphSpec::placed_art`]), or those of all its art when the graph keeps no such -/// bounds with a width and a height, or has no art without type now. +/// bounds with a width and a height. `None` when the art measured has no width or no height now, or the graph keeps +/// bounds of its art without type and has no such art now. fn measured(n: &Node) -> Option<(Rect, Rect)> { let kept = n.extras().graph.as_deref()?; let c = n.children()?; - let art = kept.placed_art.filter(|p| p.width() > 1e-9 && p.height() > 1e-9).zip(art_bounds(c)); - art.or_else(|| kept.placed.zip(children_bounds(c))) + let solid = |r: &Rect| r.width() > 1e-9 && r.height() > 1e-9; + match kept.placed_art.filter(solid) { + Some(placed) => art_bounds(c).filter(solid).map(|now| (placed, now)), + None => kept.placed.zip(children_bounds(c).filter(solid)), + } } /// `r` carried by the change from bounds `from` to bounds `to`: each edge stays at the same place relative to them. @@ -39,14 +43,28 @@ fn carried(r: Rect, from: Rect, to: Rect) -> Option { Some(Rect::new(to.x0 + (r.x0 - from.x0) * sx, to.y0 + (r.y0 - from.y0) * sy, to.x0 + (r.x1 - from.x0) * sx, to.y0 + (r.y1 - from.y0) * sy)) } -/// The graphs holding objects `ids`, each with the bounds of its art without type and of all its art. Give them to -/// [`keep_placed`] after an edit that changes a graph's art without moving the graph. +/// The graphs holding objects `ids`, that are among them or that are inside them, each with the bounds of its art +/// without type and of all its art. Give them to [`keep_placed`] after an edit that changes a graph's art without +/// moving the graph. pub(crate) fn graphs_holding(d: &Document, ids: &[NodeId]) -> Vec<(NodeId, Option, Option)> { - let mut out: Vec<(NodeId, Option, Option)> = vec![]; + let is_graph = |n: &Node| n.extras().graph.is_some(); + let mut found: Vec = vec![]; for id in ids { let Some(chain) = d.ancestry(*id) else { continue }; - let holder = chain.iter().rev().skip(1).copied().find(|a| d.node(*a).is_some_and(|n| n.extras().graph.is_some())); - let Some(gid) = holder.filter(|g| !out.iter().any(|(o, ..)| o == g)) else { continue }; + found.extend(chain.iter().rev().copied().find(|a| d.node(*a).is_some_and(is_graph))); + if let Some(n) = d.node(*id) { + n.walk(&mut |c| { + if is_graph(c) { + found.push(c.id); + } + }); + } + } + let mut out: Vec<(NodeId, Option, Option)> = vec![]; + for gid in found { + if out.iter().any(|(o, ..)| *o == gid) { + continue; + } let children = d.node(gid).and_then(Node::children); out.push((gid, children.and_then(|c| art_bounds(c)), children.and_then(|c| children_bounds(c)))); } diff --git a/crates/engine/src/cmd/graph/tests.rs b/crates/engine/src/cmd/graph/tests.rs index c160a78833..7737477ae9 100644 --- a/crates/engine/src/cmd/graph/tests.rs +++ b/crates/engine/src/cmd/graph/tests.rs @@ -5796,6 +5796,141 @@ fn outlining_a_graphs_labels_leaves_the_plot_in_place() { let moved = spec(&s, id).rect; assert!((moved.x0 - (rect.x0 + 30.0)).abs() < 1e-6 && (moved.y0 - (rect.y0 + 15.0)).abs() < 1e-6, "{moved:?}"); assert!((moved.width() - rect.width()).abs() < 1e-6 && (moved.height() - rect.height()).abs() < 1e-6, "{moved:?}"); + // A scale of the graph made before outlining is still followed. + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("object.scale", &json!({"sx": 150, "sy": 50, "origin": [moved.x0, moved.y0]})).unwrap(); + s.execute("select.set", &json!({"ids": [text_id(&s, id, "Axes", "Q3").0]})).unwrap(); + s.execute("type.createOutlines", &json!({})).unwrap(); + s.execute("graph.setData", &json!({"id": id.0, "csv": csv})).unwrap(); + let scaled = spec(&s, id).rect; + assert!((scaled.x0 - moved.x0).abs() < 1e-6 && (scaled.y0 - moved.y0).abs() < 1e-6, "{scaled:?}"); + assert!((scaled.width() - 1.5 * rect.width()).abs() < 1e-6 && (scaled.height() - 0.5 * rect.height()).abs() < 1e-6, "{scaled:?}"); +} + +/// Outline Stroke on a graph's strokes leaves the plot where it is when the graph is drawn again, and a move of the +/// graph made before it is still followed. +#[test] +fn outlining_a_graphs_strokes_leaves_the_plot_in_place() { + let csv = ",2024,2025\nQ1,3,2\nQ2,5,4\nQ3,-1,6"; + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 800, "height": 800})).unwrap(); + let id = graph(&mut s, "column"); + let rect = spec(&s, id).rect; + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("object.move", &json!({"dx": 20, "dy": 10})).unwrap(); + s.execute("object.path.outlineStroke", &json!({})).unwrap(); + s.execute("graph.setData", &json!({"id": id.0, "csv": csv})).unwrap(); + let moved = spec(&s, id).rect; + assert!((moved.x0 - (rect.x0 + 20.0)).abs() < 1e-6 && (moved.y0 - (rect.y0 + 10.0)).abs() < 1e-6, "{moved:?}"); + assert!((moved.width() - rect.width()).abs() < 1e-6 && (moved.height() - rect.height()).abs() < 1e-6, "{moved:?}"); +} + +/// A graph left with only its labels, or with its labels and one vertical line, leaves the plot where it is when it is +/// drawn again after a label edit. +#[test] +fn a_graph_left_with_only_its_labels_leaves_the_plot_in_place() { + for keep_a_line in [false, true] { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 800, "height": 800})).unwrap(); + let id = graph(&mut s, "column"); + let rect = spec(&s, id).rect; + let (mut art, mut line) = (vec![], None); + s.doc().unwrap().doc.node(id).unwrap().walk(&mut |n| { + if n.children().is_none() && !matches!(n.kind, NodeKind::Text(_)) { + let vertical = n.geometric_bounds().is_some_and(|b| b.width() < 1e-9 && b.height() > 1.0); + if keep_a_line && vertical && line.is_none() { + line = n.geometric_bounds(); + } else { + art.push(n.id.0); + } + } + }); + assert!(!art.is_empty() && line.is_some() == keep_a_line); + s.execute("select.set", &json!({"ids": art})).unwrap(); + s.execute("edit.clear", &json!({})).unwrap(); + let left = s.doc().unwrap().doc.node(id).unwrap().clone(); + assert_eq!(vectorcraft_doc::graph::graph_art_without_type(left.children().unwrap()), line, "{:#?}", left.children()); + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("text.setStyle", &json!({"size": 20})).unwrap(); + s.execute("graph.setData", &json!({"id": id.0, "csv": ",2024,2025\nQ1,3,2\nQ2,5,4\nQ3,-1,6"})).unwrap(); + assert_eq!(spec(&s, id).rect, rect, "keeping a line: {keep_a_line}"); + } +} + +/// A graph saved without the bounds of its art without type, left with one vertical line, keeps its plot's size when +/// it is drawn again. +#[test] +fn a_graph_saved_with_only_its_whole_bounds_and_left_with_a_line_keeps_its_plot() { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 800, "height": 800})).unwrap(); + let id = graph(&mut s, "column"); + let rect = spec(&s, id).rect; + let mut old = spec(&s, id); + old.placed_art = None; + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc).node_mut(id).unwrap().extras_mut().graph = Some(Box::new(old)); + let (mut art, mut line) = (vec![], None); + s.doc().unwrap().doc.node(id).unwrap().walk(&mut |n| { + if n.children().is_none() { + let vertical = !matches!(n.kind, NodeKind::Text(_)) && n.geometric_bounds().is_some_and(|b| b.width() < 1e-9 && b.height() > 1.0); + if vertical && line.is_none() { + line = Some(n.id); + } else { + art.push(n.id.0); + } + } + }); + assert!(line.is_some()); + s.execute("select.set", &json!({"ids": art})).unwrap(); + s.execute("edit.clear", &json!({})).unwrap(); + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("graph.setData", &json!({"id": id.0, "csv": ",2024,2025\nQ1,3,2\nQ2,5,4\nQ3,-1,6"})).unwrap(); + assert_eq!(spec(&s, id).rect, rect); +} + +/// The bounds a graph measures moves on take a clipping group holding type at the bounds of its clip, so art outside the +/// clip does not change them; a clipping group whose clip is type is measured on its other art. +#[test] +fn a_clipping_group_holding_type_is_measured_on_its_clip() { + use std::sync::Arc; + use vectorcraft_doc::graph::graph_art_without_type as measure; + use vectorcraft_geom::Affine; + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 800, "height": 800})).unwrap(); + let id = graph(&mut s, "column"); + let n = s.doc().unwrap().doc.node(id).unwrap().clone(); + let label: Arc = + group(&n, "Legend").children().unwrap().iter().find(|c| matches!(c.kind, NodeKind::Text(_))).unwrap().clone(); + let mut paths = vec![]; + n.walk(&mut |c| { + if matches!(c.kind, NodeKind::Path { .. }) { + paths.push(c.clone()); + } + }); + let (clip, art) = (Arc::new(paths[0].clone()), paths[1].clone()); + let clipped = |dx: f64, first: Arc, second: Arc| { + let mut far = art.clone(); + far.transform(Affine::translate((dx, 0.0)), false); + let mut g = vectorcraft_doc::Node::group(NodeId(999_999), vec![first, second, Arc::new(far)]); + if let NodeKind::Group { clip, .. } = &mut g.kind { + *clip = true; + } + measure(&[Arc::new(g)]) + }; + assert_eq!(clipped(500.0, clip.clone(), label.clone()), clip.geometric_bounds()); + assert_eq!(clipped(500.0, clip.clone(), label.clone()), clipped(900.0, clip.clone(), label.clone())); + // A clip that is type leaves the other art to measure. + assert_ne!(clipped(500.0, label.clone(), clip.clone()), clipped(900.0, label.clone(), clip.clone())); + // A clip that is a clipping group holding type in its clipped art is measured on its own clip. + let mut inner = vectorcraft_doc::Node::group(NodeId(999_998), vec![clip.clone(), label.clone()]); + if let NodeKind::Group { clip, .. } = &mut inner.kind { + *clip = true; + } + let inner = Arc::new(inner); + assert_eq!(clipped(500.0, inner.clone(), label.clone()), clip.geometric_bounds()); + assert_eq!(clipped(500.0, inner.clone(), label.clone()), clipped(900.0, inner.clone(), label.clone())); + // A blend holding type is measured whole, as it is drawn. + let blend = vectorcraft_doc::Node::new(NodeId(999_997), NodeKind::Blend { children: vec![clip.clone(), label.clone()], spec: Box::default() }); + assert_eq!(measure(&[Arc::new(blend.clone())]), blend.geometric_bounds()); } /// A graph whose kept bounds of its art without type have no width follows a move with the bounds of all of its art. @@ -7113,9 +7248,17 @@ fn a_graph_opened_from_ai_measures_moves_without_its_labels() { assert_eq!((graphs.len(), labels.len()), (1, 1)); let (id, label) = (graphs[0], labels[0]); let before = spec(&s, id); - assert!(before.placed_art.is_some()); s.execute("select.set", &json!({"ids": [label.0]})).unwrap(); + s.execute("text.setStyle", &json!({"size": 30})).unwrap(); s.execute("object.move", &json!({"dx": 80, "dy": -40})).unwrap(); s.execute("graph.setData", &json!({"id": id.0, "csv": ",East\nQ1,8\nQ2,2"})).unwrap(); - assert_eq!(spec(&s, id).rect, before.rect); + let rect = spec(&s, id).rect; + assert_eq!(rect, before.rect, "after restyling and moving a label"); + // A move of the whole opened graph is still followed. + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("object.move", &json!({"dx": 15, "dy": 25})).unwrap(); + s.execute("graph.setData", &json!({"id": id.0, "csv": ",East\nQ1,8\nQ2,2"})).unwrap(); + let moved = spec(&s, id).rect; + assert!((moved.x0 - (rect.x0 + 15.0)).abs() < 1e-6 && (moved.y0 - (rect.y0 + 25.0)).abs() < 1e-6, "{moved:?}"); + assert!((moved.width() - rect.width()).abs() < 1e-6 && (moved.height() - rect.height()).abs() < 1e-6, "{moved:?}"); } diff --git a/crates/engine/src/cmd/pathops.rs b/crates/engine/src/cmd/pathops.rs index 878b3743cf..b5267fa990 100644 --- a/crates/engine/src/cmd/pathops.rs +++ b/crates/engine/src/cmd/pathops.rs @@ -1188,6 +1188,8 @@ fn outline_stroke(s: &mut Session, _: &Value) -> Result { let roots = selected_roots(s)?; let ids = s.edit("Outline Stroke", |d, sel| { let before = d.clone(); + // Outlining the strokes in a graph does not move or scale its plot at the next redraw. + let graphs = super::graph::graphs_holding(d, &roots); // A dimension selected with the art it measures keeps its own lines: it is out of the // document while the art is outlined. let lifted = super::dimension::lift(d, &roots); @@ -1207,6 +1209,7 @@ fn outline_stroke(s: &mut Session, _: &Value) -> Result { if changed == 0 || (kept && d.layers == before.layers) { return Err(EngineError::Other("Outline Stroke: no stroked paths selected".into())); } + super::graph::keep_placed(d, &graphs); sel.set(new_sel.iter().copied()); Ok(new_sel) })?; diff --git a/crates/eps/src/import/ai/read/graph.rs b/crates/eps/src/import/ai/read/graph.rs index 9fb60408c7..609713fe16 100644 --- a/crates/eps/src/import/ai/read/graph.rs +++ b/crates/eps/src/import/ai/read/graph.rs @@ -631,7 +631,7 @@ impl<'a> Reader<'a> { let Some(paints) = series_paints(art, &tags, spec.kind == GraphKind::Line) else { return false }; spec.series_paints = paints; spec.placed = Some(placed); - spec.placed_art = art.children().and_then(|c| vectorcraft_doc::graph::art_bounds(c)); + spec.placed_art = art.children().and_then(|c| vectorcraft_doc::graph::graph_art_without_type(c)); tag_series(art, &tags); art.extras_mut().graph = Some(Box::new(spec)); true diff --git a/docs/mcp.md b/docs/mcp.md index fc03d070b6..76d53693cb 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -870,10 +870,14 @@ leaves room for a legend label's stored indents, spacing and leading. When a graph is drawn again, its plot moves and scales with any move or scale of the graph made since it was last drawn. The graph saves the bounds of its art other than type as `placedArt` (and the bounds of all of its art as -`placed`), and measures the move or scale on those. Changing the size, indents or spacing of the labels, moving a label -on its own and `type.createOutlines` on labels do not move or scale the plot. A graph opened from a `.ai` file saves -`placedArt` too. A graph saved without `placedArt`, or with a `placedArt` that has no width or height, is measured on -`placed`. +`placed`), and measures the move or scale on those. Changing the size or indents of the labels, moving a label on its +own, `type.createOutlines` on labels and `object.path.outlineStroke` on the graph's strokes do not move or scale the +plot. Outlined labels are no longer type: moving one of them on its own, like moving a swatch, an axis or a tick mark on +its own, can move or scale the plot at the next redraw. When all of a graph's art other than type has been deleted, or +the art measured has no width or no height, a redraw leaves the plot where it is and does not follow a move or scale of +the graph. A graph opened from a `.ai` file +saves `placedArt` too. A graph saved without `placedArt`, or with a `placedArt` that has no width or height, is +measured on `placed`. The `appearance.*` commands, `graphicStyle.apply` and `graphicStyle.redefine`, and the paint, stroke, gradient and transparency commands given an `item` (or the Appearance panel's active row) store a graph label's object rows for its