Merge pull request #923 from callmeBalloch/fix/artboard-artwork-movement

Fix artwork movement when editing artboard X/Y coordinates

crates/engine/src/cmd/layer.rs

@@ -135,7 +135,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Artboard Options…",
             ["Window", "Artboards"],
             None,
-            "{index, name?, x?, y?, width?, height?, scaleArt?: bool (Scale Artwork with Artboard: resized, the artboard takes the art fully inside it (locked and hidden art only with lockedAndHidden?: bool, default prefs moveLockedWithArtboard) and its guides from its old rectangle onto the new one, each side by its own ratio; strokes/corners/patterns as object.scale)} → null, or with scaleArt {scaled: the art scaled}",
+            "{index, name?, x?, y?, width?, height?, moveArt?: bool (default false; a position-only change moves the contained art with its artboard), scaleArt?: bool (Scale Artwork with Artboard: resized, the artboard takes the art fully inside it (locked and hidden art only with lockedAndHidden?: bool, default prefs moveLockedWithArtboard) and its guides from its old rectangle onto the new one, each side by its own ratio; strokes/corners/patterns as object.scale)} → null, or with scaleArt {scaled: the art scaled}",
             has_doc,
             artboard_set
         ),
@@ -771,8 +771,13 @@ fn artboard_set(s: &mut Session, p: &Value) -> Result<Value> {
     // Artboard gathers it) and its guides from the old rectangle onto the new one.
     let scale_art = bool_or(p, "scaleArt", false);
     let fit = if scale_art && resized { rect_map(was, now) } else { None };
-    let art = if fit.is_some() {
-        // Locked and hidden art too? Noted in the journal, so a replay scales the same objects
+    // Position fields honor Move Artwork independently of Scale Artwork. A resize still
+    // leaves the art alone unless scaling was requested. Fit-to-art callers keep the default.
+    let delta = now.origin() - was.origin();
+    let moving = !resized && bool_or(p, "moveArt", false) && delta != vectorcraft_geom::Vec2::ZERO;
+    let transform = fit.or_else(|| moving.then(|| Affine::translate(delta)));
+    let art = if transform.is_some() {
+        // Locked and hidden art too? Noted in the journal, so a replay transforms the same objects
         // whatever the preference is then.
         let all = bool_or(p, "lockedAndHidden", s.prefs.move_locked_with_artboard);
         s.note_journal("lockedAndHidden", json!(all));
@@ -792,7 +797,7 @@ fn artboard_set(s: &mut Session, p: &Value) -> Result<Value> {
             a.name = n.to_string();
         }
         let id = a.id;
-        if let Some(xf) = fit {
+        if let Some(xf) = transform {
             d.map_artboard_guides(id, xf);
             for id in &art {
                 if let Some(n) = d.node_mut(*id) {
@@ -801,14 +806,14 @@ fn artboard_set(s: &mut Session, p: &Value) -> Result<Value> {
             }
         } else if !resized {
             // Moved (not resized), it takes its guides along.
-            d.move_artboard_guides(id, now.origin() - was.origin());
+            d.move_artboard_guides(id, delta);
         }
         Ok(())
     })?;
     if !scale_art {
         return ok();
     }
-    Ok(json!({ "scaled": art.iter().map(|i| i.0).collect::<Vec<_>>() }))
+    Ok(json!({ "scaled": if fit.is_some() { art.iter().map(|i| i.0).collect::<Vec<_>>() } else { vec![] } }))
 }
 
 /// The map taking artboard rectangle `from` onto `to`, each side scaled by its own ratio. None

crates/engine/src/tests_xform.rs

@@ -371,6 +371,52 @@ fn artboard_set_props_scales_art_only_when_asked_and_resized() {
     assert!(s.execute("artboard.setProps", &json!({"index": 9, "width": 10, "scaleArt": true})).is_err());
 }
 
+/// Move Artwork is independent of scaling and takes only fully contained art and board guides.
+#[test]
+fn coordinate_moves_honor_move_art_and_keep_resize_behavior() {
+    let mut s = session();
+    let inside = rect(&mut s, 100.0, 100.0, 200.0, 100.0);
+    s.execute("stroke.set", &json!({"weight": 4})).unwrap();
+    let partly = rect(&mut s, 700.0, 500.0, 200.0, 200.0);
+    s.execute("guide.add", &json!({"vertical": true, "pos": 400, "artboard": 0})).unwrap();
+    s.execute("guide.add", &json!({"vertical": true, "pos": 400})).unwrap();
+    let before = s.doc().unwrap().doc.clone();
+    for scale in [false, true] {
+        let out = s.execute("artboard.setProps", &json!({"index": 0, "x": 30, "y": -10, "moveArt": true, "scaleArt": scale})).unwrap();
+        assert_eq!(bounds(&s, inside), Rect::new(130.0, 90.0, 330.0, 190.0));
+        assert_eq!(bounds(&s, partly), Rect::new(700.0, 500.0, 900.0, 700.0));
+        assert_eq!(stroke_width(&s, inside), 4.0);
+        assert_eq!(guide_positions(&s), vec![430.0, 400.0]);
+        assert_eq!(out, if scale { json!({"scaled": []}) } else { Value::Null });
+        s.execute("edit.undo", &json!({})).unwrap();
+        assert_eq!(*s.doc().unwrap().doc, *before);
+    }
+    for p in [json!({"index": 0, "x": 30, "moveArt": false}), json!({"index": 0, "x": 30, "width": 400, "moveArt": true})] {
+        s.execute("artboard.setProps", &p).unwrap();
+        assert_eq!(bounds(&s, inside), Rect::new(100.0, 100.0, 300.0, 200.0));
+        s.execute("edit.undo", &json!({})).unwrap();
+    }
+}
+
+#[test]
+fn coordinate_moves_journal_the_locked_art_preference() {
+    let mut s = session();
+    let locked = rect(&mut s, 100.0, 100.0, 200.0, 100.0);
+    s.execute("object.lock", &json!({})).unwrap();
+    s.execute("artboard.setProps", &json!({"index": 0, "x": 30, "moveArt": true})).unwrap();
+    assert_eq!(bounds(&s, locked), Rect::new(100.0, 100.0, 300.0, 200.0));
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.prefs.move_locked_with_artboard = true;
+    s.execute("artboard.setProps", &json!({"index": 0, "x": 30, "moveArt": true})).unwrap();
+    assert_eq!(bounds(&s, locked), Rect::new(130.0, 100.0, 330.0, 200.0));
+    let (id, p) = s.journal.last().unwrap().clone();
+    assert_eq!(p["lockedAndHidden"], true);
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.prefs.move_locked_with_artboard = false;
+    s.execute(&id, &p).unwrap();
+    assert_eq!(bounds(&s, locked), Rect::new(130.0, 100.0, 330.0, 200.0));
+}
+
 /// Locked and hidden art scales only with Move Locked and Hidden Artwork with Artboard, as it
 /// moves only with it.
 #[test]

crates/engine/tests/affinity_artboards.rs

@@ -145,3 +145,42 @@ fn several_spreads_preserve_each_board_and_leave_room_between_spreads() {
     assert_eq!(loaded.doc.artboards[0].rect, Rect::new(0.0, 0.0, 100.0, 60.0));
     assert_eq!(loaded.doc.artboards[1].rect, Rect::new(136.0, 0.0, 236.0, 60.0));
 }
+
+/// Coordinate fields use setProps rather than the drag tool's artboard.move command.
+#[test]
+fn coordinate_edits_move_imported_clipping_groups_and_undo_together() {
+    use serde_json::json;
+    use vectorcraft_engine::Session;
+    use vectorcraft_geom::Vec2;
+
+    for current in [false, true] {
+        let child = shape(20, "Oversized", false, false, [-10.0, -10.0, 110.0, 70.0], IDENTITY, vec![]);
+        let mut first = shape(10, "First", true, false, [0.0, 0.0, 100.0, 60.0], IDENTITY, vec![child]);
+        if current {
+            let F::Def(_, _, fields) = &mut first else { panic!() };
+            fields.retain(|(t, _)| *t != tag(b"ABEn"));
+            fields.push(field(b"phrp", F::Def(18, vec![tag(b"aprp")], vec![])));
+        }
+        let second = shape(30, "Second", true, false, [0.0, 0.0, 40.0, 60.0], [1.0, 0.0, 200.0, 0.0, 1.0, 0.0], vec![]);
+        let loaded = vectorcraft_engine::cmd::fileio::load("synthetic.af", &document(vec![vec![first, second]], 72.0, Method::Zstd)).unwrap();
+        let mut s = Session::new();
+        s.add_document(loaded.doc, None);
+        let before = s.doc().unwrap().doc.clone();
+        let first_bounds = find(&before.layers, "First").unwrap().geometric_bounds().unwrap();
+        let second_node = find(&before.layers, "Second").unwrap().clone();
+        let child_bounds = find(&before.layers, "First").unwrap().children().unwrap()[1].geometric_bounds().unwrap();
+        let delta = Vec2::new(13.0, -7.0);
+        s.execute("artboard.setProps", &json!({"index": 0, "x": 13, "y": -7, "moveArt": true})).unwrap();
+        let after = s.doc().unwrap().doc.clone();
+        assert_eq!(after.artboards[0].rect, before.artboards[0].rect + delta);
+        let first = find(&after.layers, "First").unwrap();
+        assert_eq!(first.geometric_bounds().unwrap(), first_bounds + delta);
+        assert_eq!(first.children().unwrap()[1].geometric_bounds().unwrap(), child_bounds + delta);
+        assert_eq!(find(&after.layers, "Second").unwrap(), &second_node);
+        assert_eq!(after.artboards[1], before.artboards[1]);
+        s.execute("edit.undo", &json!({})).unwrap();
+        assert_eq!(*s.doc().unwrap().doc, *before);
+        s.execute("edit.redo", &json!({})).unwrap();
+        assert_eq!(*s.doc().unwrap().doc, *after);
+    }
+}

crates/ui-egui/src/dialogs/artboard_options.rs

@@ -35,5 +35,6 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result<Value, String> {
             p[k] = json!(d.f64(k, 0.0));
         }
     }
+    p["moveArt"] = json!(crate::panels::artboards::move_art(app));
     run_and_close(app, "artboard.setProps", p)
 }

crates/ui-egui/src/dialogs/tests.rs

@@ -446,3 +446,26 @@ fn the_transform_effect_dialog_has_its_controls_previews_them_and_shows_them_aga
     let text = crate::tests_labels::painted_text(&mut app, |app, ui| show(app, ui.ctx()));
     assert!(text.lines().any(|l| l == "Reference Point"), "{text}");
 }
+
+#[test]
+fn artboard_options_position_moves_art_only_with_the_tool_option_on() {
+    for move_art in [false, true] {
+        let mut app = app();
+        let id = app.run("shape.rectangle", json!({"x": 10, "y": 20, "width": 30, "height": 40})).unwrap()["id"].as_u64().unwrap();
+        app.select_tool("artboard");
+        app.session.set_tool_option("moveArt", &json!(move_art));
+        crate::dialogs::artboard_options::open(&mut app).unwrap();
+        app.ui.dialog.as_mut().unwrap().fields.insert("x".into(), json!(50));
+        // Draw the dialog once and confirm through its registry, as the OK button does.
+        frame(&mut app, Default::default());
+        let before = app.session.doc().unwrap().doc.clone();
+        super::confirm(&mut app).unwrap();
+        assert_eq!(app.session.doc().unwrap().doc.artboards[0].rect.x0, 50.0);
+        assert_eq!(
+            app.session.doc().unwrap().doc.node(vectorcraft_doc::NodeId(id)).unwrap().geometric_bounds().unwrap().x0,
+            if move_art { 60.0 } else { 10.0 }
+        );
+        app.run("edit.undo", json!({})).unwrap();
+        assert_eq!(*app.session.doc().unwrap().doc, *before);
+    }
+}

crates/ui-egui/src/panels/artboards.rs

@@ -30,6 +30,11 @@ pub(crate) fn scale_art(app: &VectorcraftApp) -> bool {
     app.session.tool_id() == "artboard" && app.session.tool_options()["scaleArt"].as_bool().unwrap_or(false)
 }
 
+/// Whether position edits take their art along, matching the Artboard tool's move option.
+pub(crate) fn move_art(app: &VectorcraftApp) -> bool {
+    app.session.tool_id() == "artboard" && app.session.tool_options()["moveArt"].as_bool().unwrap_or(true)
+}
+
 /// The active artboard (of `n`).
 pub(crate) fn selected(app: &VectorcraftApp, n: usize) -> usize {
     app.view().map_or(0, |v| v.artboard).min(n.saturating_sub(1))

crates/ui-egui/src/panels/properties.rs

@@ -195,6 +195,7 @@ fn artboard_sections(app: &mut VectorcraftApp, ui: &mut Ui) {
     let n = app.session.active().map_or(0, |d| d.doc.artboards.len());
     let i = super::artboards::selected(app, n);
     let scale_art = super::artboards::scale_art(app);
+    let move_art = super::artboards::move_art(app);
     if let Some(ab) = app.session.active().and_then(|d| d.doc.artboards.get(i)).cloned() {
         // Its name as it is (names are never translated).
         ui.label(egui::RichText::new(&ab.name).size(13.0).color(Tokens::get(ui.ctx()).text));
@@ -207,7 +208,7 @@ fn artboard_sections(app: &mut VectorcraftApp, ui: &mut Ui) {
                 for (label, key, v) in [(l1, k1, v1), (l2, k2, v2)] {
                     dim_label(ui, label);
                     if let Some(v) = widgets::num_field(ui, ("ab", key, row), Some(v), units, fw) {
-                        app.run("artboard.setProps", json!({"index": i, key: v, "scaleArt": scale_art})).ok();
+                        app.run("artboard.setProps", json!({"index": i, key: v, "scaleArt": scale_art, "moveArt": move_art})).ok();
                     }
                 }
                 ui.end_row();
@@ -536,8 +537,12 @@ mod tests {
 
     /// One frame of the panel with `events` → the texts painted, with their rects.
     fn frame(app: &mut VectorcraftApp, ctx: &egui::Context, events: Vec<Event>) -> Vec<(String, Rect)> {
+        panel_frame(app, ctx, events, show)
+    }
+
+    fn panel_frame(app: &mut VectorcraftApp, ctx: &egui::Context, events: Vec<Event>, draw: fn(&mut VectorcraftApp, &mut Ui)) -> Vec<(String, Rect)> {
         let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(300.0, 700.0))), events, ..Default::default() };
-        let mut out = ctx.run_ui(raw, |ui| show(app, ui));
+        let mut out = ctx.run_ui(raw, |ui| draw(app, ui));
         out.textures_delta.clear();
         out.shapes
             .iter()
@@ -586,6 +591,41 @@ mod tests {
         assert!(widths.iter().all(|w| (w - widths[0]).abs() < 1.0), "X, rotation, corner radius, opacity: {widths:?}");
     }
 
+    /// Both panels forward the Move Artwork toggle when their actual X field is edited.
+    #[test]
+    fn artboard_coordinate_fields_move_art_in_properties_and_transform() {
+        for draw in [show as fn(&mut VectorcraftApp, &mut Ui), super::super::transform::show] {
+            for move_art in [false, true] {
+                let mut app = VectorcraftApp::new(Session::new(), Default::default());
+                app.run("file.new", json!({"width": 200, "height": 100})).unwrap();
+                let id = app.run("shape.rectangle", json!({"x": 10, "y": 20, "width": 30, "height": 40})).unwrap()["id"].as_u64().unwrap();
+                app.select_tool("artboard");
+                app.session.set_tool_option("moveArt", &json!(move_art));
+                let ctx = egui::Context::default();
+                panel_frame(&mut app, &ctx, vec![], draw);
+                let texts = panel_frame(&mut app, &ctx, vec![], draw);
+                let label = texts.iter().find(|(t, _)| t == "X:").unwrap().1;
+                let at = egui::pos2(label.right() + 40.0, label.center().y);
+                let press = |pressed| Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Default::default() };
+                panel_frame(&mut app, &ctx, vec![Event::PointerMoved(at), press(true)], draw);
+                panel_frame(&mut app, &ctx, vec![press(false)], draw);
+                panel_frame(
+                    &mut app,
+                    &ctx,
+                    vec![Event::Key { key: egui::Key::ArrowUp, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }],
+                    draw,
+                );
+                let d = &app.session.doc().unwrap().doc;
+                assert_eq!(d.artboards[0].rect.x0, 1.0, "the focused X field steps its position");
+                assert_eq!(d.node(vectorcraft_doc::NodeId(id)).unwrap().geometric_bounds().unwrap().x0, if move_art { 11.0 } else { 10.0 });
+                app.run("edit.undo", json!({})).unwrap();
+                let d = &app.session.doc().unwrap().doc;
+                assert_eq!(d.artboards[0].rect.x0, 0.0);
+                assert_eq!(d.node(vectorcraft_doc::NodeId(id)).unwrap().geometric_bounds().unwrap().x0, 10.0);
+            }
+        }
+    }
+
     /// #530: Edit Artboards shows the active artboard, with New Artboard, Delete Artboard and Exit.
     /// #775: type's Paragraph section has Hyphenate, and its checkbox turns it on.
     #[test]

crates/ui-egui/src/panels/transform.rs

@@ -58,7 +58,8 @@ fn artboard_fields(app: &mut VectorcraftApp, ui: &mut Ui, index: usize, r: Rect)
     let set = |app: &mut VectorcraftApp, x, y, w, h| {
         let n = artboard_rect(r, refi, x, y, w, h, link);
         let scale_art = crate::panels::artboards::scale_art(app);
-        let p = json!({"index": index, "x": n.x0, "y": n.y0, "width": n.width(), "height": n.height(), "scaleArt": scale_art});
+        let move_art = crate::panels::artboards::move_art(app);
+        let p = json!({"index": index, "x": n.x0, "y": n.y0, "width": n.width(), "height": n.height(), "scaleArt": scale_art, "moveArt": move_art});
         if let Err(e) = app.run("artboard.setProps", p) {
             app.status(e);
         }