diff --git a/crates/doc/src/graph.rs b/crates/doc/src/graph.rs index d561663524..96cbf56edf 100644 --- a/crates/doc/src/graph.rs +++ b/crates/doc/src/graph.rs @@ -511,6 +511,11 @@ 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. Moves and scales of the graph group are measured + /// on these when present, so a label edit that resizes its type is not taken for one; graphs saved without them + /// are measured on [`Self::placed`]. + #[serde(skip_serializing_if = "Option::is_none")] + pub placed_art: Option, } fn at_most_one(n: &usize) -> bool { @@ -635,6 +640,7 @@ impl Default for GraphSpec { connect_points: true, edge_to_edge: false, placed: None, + placed_art: None, } } } diff --git a/crates/engine/src/cmd/graph/edit.rs b/crates/engine/src/cmd/graph/edit.rs index c7f6d52ea8..bdfd13beeb 100644 --- a/crates/engine/src/cmd/graph/edit.rs +++ b/crates/engine/src/cmd/graph/edit.rs @@ -1,6 +1,6 @@ use super::axis::{MAX_TICK_MARKS, count_given, one_line}; use super::data::apply_data; -use super::generate::{children_bounds, generate}; +use super::generate::{art_bounds, children_bounds, generate}; use super::series::{picked_series, reselect_series}; use super::{ Affine, Document, EngineError, GraphKind, GraphSpec, Node, NodeId, Rect, Result, Session, TickLength, Value, ValueAxisSide, bad, f64_req, json, @@ -18,10 +18,16 @@ fn bound_param(p: &Value, key: &str, cmd: &str) -> Result>> { } /// Regenerate graph `id` from `spec` (adjusting the plot rect for any move/scale since the last -/// generation, which is recorded in the spec's `placed` bounds). +/// generation, which is recorded in the spec's `placed_art` bounds, or `placed` in graphs saved without them). pub(super) fn regenerate(d: &mut Document, id: NodeId, mut spec: GraphSpec) -> Result<()> { let n = d.node(id).ok_or(EngineError::NoNode(id))?; - if let (Some(placed), Some(now)) = (n.extras().graph.as_ref().and_then(|g| g.placed), n.children().and_then(|c| children_bounds(c))) + let kept = n.extras().graph.as_deref(); + let measured = match (kept.and_then(|g| g.placed_art), n.children()) { + (Some(placed), Some(c)) => art_bounds(c).map(|now| (placed, now)), + (None, Some(c)) => kept.and_then(|g| g.placed).zip(children_bounds(c)), + (_, None) => None, + }; + if let Some((placed, now)) = measured && placed.width() > 1e-9 && placed.height() > 1e-9 && (placed.x0 - now.x0).abs() + (placed.y0 - now.y0).abs() + (placed.x1 - now.x1).abs() + (placed.y1 - now.y1).abs() > 1e-6 @@ -33,6 +39,7 @@ pub(super) fn regenerate(d: &mut Document, id: NodeId, mut spec: GraphSpec) -> R } let children = generate(d, &spec); spec.placed = children_bounds(&children); + spec.placed_art = art_bounds(&children); let n = d.node_mut(id).ok_or(EngineError::NoNode(id))?; if let Some(ch) = n.children_mut() { *ch = children; diff --git a/crates/engine/src/cmd/graph/generate.rs b/crates/engine/src/cmd/graph/generate.rs index 69510652b9..759c442acd 100644 --- a/crates/engine/src/cmd/graph/generate.rs +++ b/crates/engine/src/cmd/graph/generate.rs @@ -1022,3 +1022,29 @@ impl<'a> Plot<'a> { pub(super) fn children_bounds(children: &[Arc]) -> Option { children.iter().filter_map(|c| c.geometric_bounds()).reduce(|a, b| a.union(b)) } + +/// The geometric bounds of `children` leaving out their type: the plot, axes, tick marks and legend swatches without +/// the labels. None when there is nothing else. +pub(super) fn art_bounds(children: &[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: &Node) -> (Option, bool) { + if matches!(n.kind, 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) } +} diff --git a/crates/engine/src/cmd/graph/tests.rs b/crates/engine/src/cmd/graph/tests.rs index 140fb3c933..04572d10bd 100644 --- a/crates/engine/src/cmd/graph/tests.rs +++ b/crates/engine/src/cmd/graph/tests.rs @@ -5673,6 +5673,76 @@ fn formatting_one_label_keeps_that_kind_of_label() { assert!(serde_json::to_value(GraphSpec::default()).unwrap().get("labelStyles").is_none()); } +/// Character and paragraph edits that resize a graph's labels leave the plot where it is when the graph is drawn +/// again, and a move or scale of the graph made after them is still followed. +#[test] +fn restyling_a_graphs_labels_leaves_the_plot_in_place() { + let csv = ",2024,2025\nQ1,3,2\nQ2,5,4\nQ3,-1,6"; + let redraw = |s: &mut Session, id: NodeId| s.execute("graph.setData", &json!({"id": id.0, "csv": csv})).unwrap(); + 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("text.setStyle", &json!({"size": 14})).unwrap(); + redraw(&mut s, id); + assert_eq!(spec(&s, id).rect, rect, "after a size change to every label"); + let c = text_id(&s, id, "Axes", "Q2"); + s.execute("text.setFormat", &json!({"ids": [c.0], "leftIndent": 20})).unwrap(); + redraw(&mut s, id); + assert_eq!(spec(&s, id).rect, rect, "after an indent on the category labels"); + let l = text_id(&s, id, "Legend", "2025"); + s.execute("text.setStyle", &json!({"ids": [l.0], "size": 4})).unwrap(); + redraw(&mut s, id); + assert_eq!(spec(&s, id).rect, rect, "after a smaller size on the legend labels"); + // A move and a scale of the whole graph still carry the plot with them. + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("object.move", &json!({"dx": 40, "dy": -10})).unwrap(); + redraw(&mut s, id); + let moved = spec(&s, id).rect; + assert!((moved.x0 - (rect.x0 + 40.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:?}"); + s.execute("object.scale", &json!({"sx": 200, "sy": 100, "origin": [moved.x0, moved.y0]})).unwrap(); + redraw(&mut s, id); + let scaled = spec(&s, id).rect; + assert!((scaled.width() - 2.0 * rect.width()).abs() < 1e-6 && (scaled.height() - rect.height()).abs() < 1e-6, "{scaled:?}"); + assert!((scaled.x0 - moved.x0).abs() < 1e-6 && (scaled.y0 - moved.y0).abs() < 1e-6, "{scaled:?}"); +} + +/// Moving one label of a graph on its own leaves the plot where it is when the graph is drawn again. +#[test] +fn moving_one_label_leaves_the_plot_in_place() { + 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 l = text_id(&s, id, "Legend", "2025"); + s.execute("select.set", &json!({"ids": [l.0]})).unwrap(); + s.execute("object.move", &json!({"dx": 120, "dy": 60})).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); +} + +/// A graph saved before it kept the bounds of its art without type follows a move made since with its old bounds. +#[test] +fn a_graph_saved_with_only_its_whole_bounds_still_follows_a_move() { + 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 = serde_json::to_value(spec(&s, id)).unwrap(); + old.as_object_mut().unwrap().remove("placedArt"); + let old: GraphSpec = serde_json::from_value(old).unwrap(); + assert_eq!(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)); + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("object.move", &json!({"dx": 25, "dy": 5})).unwrap(); + s.execute("graph.setData", &json!({"id": id.0, "csv": ",2024,2025\nQ1,3,2\nQ2,5,4\nQ3,-1,6"})).unwrap(); + let moved = spec(&s, id).rect; + assert!((moved.x0 - (rect.x0 + 25.0)).abs() < 1e-6 && (moved.y0 - (rect.y0 + 5.0)).abs() < 1e-6, "{moved:?}"); + assert!(spec(&s, id).placed_art.is_some(), "the next drawing keeps the bounds of the art without type"); +} + /// Each Axes and Legend text after the graph is drawn again, with its object rows (fill or stroke, color, opacity, and /// for a stroke its weight) and its object opacity. type RowLooks = Vec<(bool, String, f32, f64)>;