Graphs: restyling the labels leaves the plot where it is at the next redraw

The next redraw of a graph compared the bounds of all its generated art,
labels included, with the bounds stored at the last generation, and took
any difference for a move or scale of the graph. Label edits that change
the size of the type (a new size, an indent, a legend size) changed those
bounds without a move, so the plot shifted and stretched when the graph
was drawn again.

The spec now also stores the bounds of the art without type
(`placedArt`), and moves and scales are measured on those. Graphs saved
without them are measured on the whole bounds as before. Moving one
label with Direct Selection no longer moves or stretches the plot at the
next redraw.

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<Rect>,
+    /// 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<Rect>,
 }
 
 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,
         }
     }
 }

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<Option<Option<f64>>> {
 }
 
 /// 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;

crates/engine/src/cmd/graph/generate.rs

@@ -1022,3 +1022,29 @@ impl<'a> Plot<'a> {
 pub(super) fn children_bounds(children: &[Arc<Node>]) -> Option<Rect> {
     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<Node>]) -> Option<Rect> {
+    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<Rect>, 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<Rect> = 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) }
+}

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)>;