Merge pull request #6996 from dotCooCoo/fix/align-artboard-distribute

Align: artboard Distribute buttons need three artboards with Align to Selection

crates/engine/src/cmd/artboardalign.rs

@@ -17,7 +17,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Align",
             [],
             None,
-            "{index? | indices?: [n…], selectedArtboards?: [n…] (as artboard.duplicate; default: the artboards the Artboard tool has selected, else its active one), horizontal?: \"left\"|\"center\"|\"right\", vertical?: \"top\"|\"center\"|\"bottom\" (one of the two is needed), to?: \"selection\"|\"artboard\" (default \"selection\": the bounds of the artboards together; \"artboard\": artboard `artboard`, which stays where it is), artboard?: n (0-based; the artboard to align to with to: \"artboard\"; default: the Artboard tool's active one; it need not be one of the artboards aligned), activeArtboard?: n (the journal records that default here, for a replay; `artboard` goes ahead of it), moveArt?: bool (the art fully inside each artboard moves with it; default: the Artboard tool's Move Artwork with Artboard option), patterns?: bool, lockedAndHidden?: bool (as artboard.move)} (Align panel › Align Objects with the Artboard tool chosen) each artboard moves until its edge or center is on that edge or center of what it aligns to, with its guides, in one undo step; a locked artboard stays where it is and still counts in the bounds, and the call fails with `the artboard is locked` when every artboard it names is locked; when no artboard moves, no undo step is recorded → {moved: [index…]}",
+            "{index? | indices?: [n…], selectedArtboards?: [n…] (as artboard.duplicate; default: the artboards the Artboard tool has selected, else its active one), horizontal?: \"left\"|\"center\"|\"right\", vertical?: \"top\"|\"center\"|\"bottom\" (one of the two is needed), to?: \"selection\"|\"artboard\" (default \"selection\": the bounds of the artboards together; \"artboard\": artboard `artboard`, which stays where it is), artboard?: n (0-based; the artboard to align to with to: \"artboard\"; default: the Artboard tool's active one; it need not be one of the artboards aligned), activeArtboard?: n (the journal records that default here, for a replay; `artboard` goes ahead of it), moveArt?: bool (the art fully inside each artboard moves with it, art inside two of them with the first by index, which may stay; default: the Artboard tool's Move Artwork with Artboard option), patterns?: bool, lockedAndHidden?: bool (as artboard.move)} (Align panel › Align Objects with the Artboard tool chosen) each artboard moves until its edge or center is on that edge or center of what it aligns to, with its guides, in one undo step; a locked artboard stays where it is and still counts in the bounds, and the call fails with `the artboard is locked` when every artboard it names is locked; when no artboard moves, no undo step is recorded → {moved: [index…]}",
             has_doc,
             align
         ),
@@ -114,18 +114,27 @@ fn kept(b: &Boards, items: &[(usize, Rect)], gap: Option<f64>) -> Result<Option<
 }
 
 /// Moves each artboard of `moves` by its offset as undo step `label`, with its guides and, when
-/// `b` says so, the art inside it. Locked artboards stay where they are. When no artboard moves
+/// `b` says so, the art inside it; art inside two of the artboards `b` names goes with the one
+/// first in artboard order, and stays when that one stays. Locked artboards stay where they are.
+/// When no artboard moves
 /// more than 1e-9 pt on either axis, the document and its undo history stay as they are. The edit
 /// fails with nothing changed when an artboard would end up past the canvas, as every edit does.
 fn apply(s: &mut Session, label: &str, p: &Value, b: &Boards, moves: &[(usize, Vec2)]) -> Result<Value> {
     let d = &s.doc()?.doc;
-    let moves: Vec<(usize, Vec2)> =
+    let mut moves: Vec<(usize, Vec2)> =
         moves.iter().copied().filter(|(i, dv)| (dv.x.abs() > 1e-9 || dv.y.abs() > 1e-9) && d.artboards.get(*i).is_some_and(|a| !a.locked)).collect();
+    // In artboard order, as the other artboard edits take them, whatever order the Distribute
+    // commands spaced them in.
+    moves.sort_by_key(|(i, _)| *i);
     let moved: Vec<usize> = moves.iter().map(|(i, _)| *i).collect();
     if moves.is_empty() {
         return Ok(json!({ "moved": moved }));
     }
-    let arts = artboards_art(s, &moved, b.move_art, b.locked_art, true)?;
+    // The art of every artboard named, those that stay included, so that art on overlapping
+    // artboards goes with the first by index; then that of the artboards that move.
+    let named: Vec<usize> = b.items.iter().map(|(i, _)| *i).collect();
+    let arts: Vec<(usize, Vec<NodeId>)> =
+        artboards_art(s, &named, b.move_art, b.locked_art, true)?.into_iter().filter(|(i, _)| moved.contains(i)).collect();
     let ids: Vec<NodeId> = arts.iter().flat_map(|(_, a)| a.iter().copied()).collect();
     let sc = Scaling { patterns: super::object::transform_patterns(s, p, &ids)?, ..Scaling::default() };
     s.edit(label, |d, _| {

crates/engine/src/cmd/artboardalign/tests.rs

@@ -265,6 +265,39 @@ fn bad_params_are_errors() {
     assert_eq!((undo_steps(&s), corners(&s)), (steps, before));
 }
 
+/// Art inside two overlapping artboards moves with the one first in artboard order, as with the
+/// other artboard edits, even when Distribute spaces the other one first: artboard 2 at x 140 is
+/// left of artboard 1 at x 200, and the square inside both goes with artboard 1.
+#[test]
+fn art_on_overlapping_artboards_moves_with_the_first_by_index() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 100, "height": 100})).unwrap();
+    s.execute("artboard.new", &json!({"x": 200, "y": 0, "width": 100, "height": 100})).unwrap();
+    s.execute("artboard.new", &json!({"x": 140, "y": 0, "width": 100, "height": 100})).unwrap();
+    let sq = s.execute("shape.rectangle", &json!({"x": 210, "y": 10, "width": 10, "height": 10})).unwrap()["id"].as_u64().unwrap();
+    s.execute("select.none", &json!({})).unwrap();
+    let p = json!({"indices": ALL, "to": "artboard", "artboard": 0, "spacing": 10});
+    assert_eq!(s.execute("artboard.distributeSpacing", &p).unwrap(), json!({"moved": [1, 2]}));
+    assert_eq!(corners(&s), [(0.0, 0.0), (220.0, 0.0), (110.0, 0.0)]);
+    assert_eq!(corner(&s, sq), (230.0, 10.0), "with artboard 1, 20 pt to the right");
+}
+
+/// Art inside an artboard that stays and a later one that moves stays with the first: artboard 2
+/// overlaps artboard 0, and aligning it to artboard 0 leaves the square inside both in place.
+#[test]
+fn art_shared_with_an_artboard_that_stays_stays() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 100, "height": 100})).unwrap();
+    s.execute("artboard.new", &json!({"x": 200, "y": 0, "width": 100, "height": 100})).unwrap();
+    s.execute("artboard.new", &json!({"x": 60, "y": 0, "width": 100, "height": 100})).unwrap();
+    let sq = s.execute("shape.rectangle", &json!({"x": 70, "y": 10, "width": 10, "height": 10})).unwrap()["id"].as_u64().unwrap();
+    s.execute("select.none", &json!({})).unwrap();
+    let p = json!({"indices": [0, 2], "horizontal": "left", "to": "artboard", "artboard": 0});
+    assert_eq!(s.execute("artboard.align", &p).unwrap(), json!({"moved": [2]}));
+    assert_eq!(corners(&s)[2], (0.0, 0.0));
+    assert_eq!(corner(&s, sq), (70.0, 10.0), "with artboard 0, which stays");
+}
+
 /// An artboard's own guides move with it.
 #[test]
 fn an_artboards_guides_move_with_it() {

crates/ui-egui/src/chrome/control_bar.rs

@@ -283,8 +283,9 @@ fn groups_before_transform(app: &mut VectorcraftApp, ui: &mut Ui, objects: usize
 }
 
 /// Draws the Control bar's Align group after the artboard controls, with the Artboard tool in use:
-/// the Align buttons, the Align To menu and, for two or more selected artboards, the Distribute
-/// buttons. They align and distribute the selected artboards
+/// the Align buttons, the Align To menu and, while they can space the selected artboards
+/// ([`crate::panels::align::artboards_distribute`]), the Distribute buttons. They align and
+/// distribute the selected artboards
 /// ([`crate::panels::align::run_button`]). Each shows only while the bar has room for it, and none
 /// while the panel menu has the group off.
 fn artboard_align_group(app: &mut VectorcraftApp, ui: &mut Ui) {
@@ -295,7 +296,7 @@ fn artboard_align_group(app: &mut VectorcraftApp, ui: &mut Ui) {
     crate::panels::align::align_buttons(app, ui, BUTTON);
     if ui.available_width() >= crate::panels::align::align_to_menu_width(BUTTON) + ui.spacing().item_spacing.x {
         crate::panels::align::align_to_menu(app, ui, BUTTON);
-        if crate::panels::align::artboards_selected(app) > 1 && ui.available_width() >= GROUP_GAP + buttons_width(ui, 6) {
+        if crate::panels::align::artboards_distribute(app) && ui.available_width() >= GROUP_GAP + buttons_width(ui, 6) {
             ui.add_space(GROUP_GAP);
             crate::panels::align::distribute_buttons(app, ui, BUTTON);
         }

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

@@ -2670,7 +2670,8 @@ fn menus_stop_above_a_control_bar_docked_at_the_bottom() {
 }
 
 /// With the Artboard tool in use, the Control bar shows the Align buttons after the artboard
-/// controls while it has room, and the Distribute buttons for two or more selected artboards. A
+/// controls while it has room, and the Distribute buttons for two or more selected artboards with
+/// Align to Artboard and three or more with Align to Selection. A
 /// click aligns the selected artboards as the Align panel's button does. The group is left out
 /// while the panel menu has it off.
 #[test]
@@ -2699,7 +2700,10 @@ fn the_control_bar_aligns_the_selected_artboards_with_the_artboard_tool() {
     assert!(button(&mut app, "Vertical Align Top").is_some(), "one artboard: the Align buttons");
     assert!(button(&mut app, "Horizontal Distribute Left").is_none(), "one artboard: no Distribute buttons");
     app.session.set_tool_option("selected", &json!([1, 0]));
-    assert!(button(&mut app, "Horizontal Distribute Left").is_some(), "two artboards: the Distribute buttons");
+    assert!(button(&mut app, "Horizontal Distribute Left").is_none(), "two artboards, Align to Selection: no Distribute buttons");
+    app.ui.align_to = crate::panels::align::AlignTo::Artboard;
+    assert!(button(&mut app, "Horizontal Distribute Left").is_some(), "two artboards, Align to Artboard: the Distribute buttons");
+    app.ui.align_to = crate::panels::align::AlignTo::Selection;
     let r = button(&mut app, "Vertical Align Top").expect("Vertical Align Top");
     let press =
         |pressed| egui::Event::PointerButton { pos: r.center(), button: egui::PointerButton::Primary, pressed, modifiers: Default::default() };

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

@@ -142,6 +142,14 @@ pub(crate) fn artboards_selected(app: &VectorcraftApp) -> usize {
     if n == 0 { 0 } else { super::artboards::picked(app, n).len() }
 }
 
+/// With the Artboard tool chosen, whether the Distribute buttons of the Align panel and the Control
+/// bar can space the selected artboards: two or more with Align to Artboard, whose spacing places
+/// the second from the active one (the Artboard tool's selection always holds its active artboard),
+/// else three or more, since the first and the last stay where they are.
+pub(crate) fn artboards_distribute(app: &VectorcraftApp) -> bool {
+    artboards_selected(app) >= if app.ui.align_to == AlignTo::Artboard { 2 } else { 3 }
+}
+
 /// Runs what a click on button `b` runs with the Artboard tool chosen: the artboard command on
 /// the artboards the tool has selected. With Align to Artboard chosen, it sends `to: "artboard"`,
 /// which keeps the tool's active artboard in place, and the Distribute buttons send the
@@ -219,6 +227,12 @@ fn spacing_sent(app: &VectorcraftApp) -> Option<f64> {
     sent.then_some(app.ui.align_spacing)
 }
 
+/// The lowest Distribute Spacing value the Align panel's field takes: none with the Artboard tool
+/// chosen, since `artboard.distributeSpacing` overlaps the artboards with a negative spacing, else 0.
+fn spacing_min(app: &VectorcraftApp) -> f64 {
+    if on_artboards(app) { f64::NEG_INFINITY } else { 0.0 }
+}
+
 /// The six align buttons of the Control bar and the Properties panel, `size` points square.
 /// They align to what the Align panel's Align To says (the key object while there is one), as
 /// its own buttons do (#789), and the selected artboards with the Artboard tool chosen.
@@ -405,8 +419,9 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
     let spacing = app.ui.align_spacing;
     let key = spacing_sent(app).is_some();
     // Two or more objects and ruler guides (#1597), or anchor points, which may all be on one
-    // path; or two or more artboards.
-    let distributes = boards.map_or(n + selected_guides(app) >= 2 || has_anchors(app), |b| b >= 2);
+    // path; or the artboards [`artboards_distribute`] asks for.
+    let distributes = boards.map_or(n + selected_guides(app) >= 2 || has_anchors(app), |_| artboards_distribute(app));
+    let (unit, min_spacing) = (app.session.general_unit(), spacing_min(app));
     widgets::subheader(ui, tl!("Align Objects:"));
     ui.horizontal(|ui| {
         ui.spacing_mut().item_spacing.x = 5.0;
@@ -452,7 +467,7 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
                     run_button(app, Button::Spacing("horizontal")).ok();
                 }
                 ui.add_enabled_ui(key, |ui| {
-                    if let Some(v) = widgets::spin_field(ui, "align-spacing", Some(spacing), app.session.general_unit(), 70.0, 1.0, 0.0, &[]) {
+                    if let Some(v) = widgets::spin_field(ui, "align-spacing", Some(spacing), unit, 70.0, 1.0, min_spacing, &[]) {
                         app.ui.align_spacing = v;
                     }
                 });

crates/ui-egui/src/panels/align/tests.rs

@@ -800,6 +800,63 @@ fn the_distribute_buttons_space_the_selected_artboards() {
     assert_eq!(app.session.journal.len(), ran, "one artboard: the Distribute buttons are off");
 }
 
+/// Whether the widget at `rect` in the last frame takes clicks (an enabled button).
+fn clickable(ctx: &egui::Context, rect: egui::Rect) -> bool {
+    ctx.viewport(|vp| vp.prev_pass.widgets.layers().flat_map(|(_, w)| w.iter()).any(|w| w.rect == rect && w.sense.senses_click()))
+}
+
+/// With Align to Selection chosen, two selected artboards are the ends of the distribution
+/// already, so the Distribute buttons are off; with Align to Artboard and the active artboard
+/// selected, the spacing places the second from it, and they are on.
+#[test]
+fn the_distribute_buttons_need_three_artboards_without_a_spacing() {
+    let mut app = selected_artboards();
+    // Making an artboard active selects it alone, so the selection is set after.
+    crate::panels::artboards::select(&mut app, 1);
+    app.session.set_tool_option("selected", &json!([1, 2]));
+    let ctx = egui::Context::default();
+    let (left, space) = distribute_buttons(&mut app, &ctx);
+    assert!(!clickable(&ctx, left) && !clickable(&ctx, space), "Align to Selection: off for two artboards");
+    app.ui.align_to = AlignTo::Artboard;
+    let (left, space) = distribute_buttons(&mut app, &ctx);
+    assert!(clickable(&ctx, left) && clickable(&ctx, space), "Align to Artboard: on for two artboards");
+    app.ui.align_spacing = 20.0;
+    click(&ctx, &mut app, space.center(), show);
+    assert_eq!(artboard_corners(&app), [(0.0, 0.0), (150.0, 40.0), (230.0, 10.0)], "a gap of 20: {}", app.ui.status);
+}
+
+/// With Align to Artboard chosen, the spacing is measured from the active artboard, which is
+/// always one of the selected ones: selecting artboards 0 and 2 makes artboard 0 active, and
+/// Horizontal Distribute Space with 20 keeps it in place.
+#[test]
+fn the_artboard_spacing_is_measured_from_a_selected_artboard() {
+    let mut app = selected_artboards();
+    app.session.set_tool_option("selected", &json!([0, 2]));
+    assert_eq!((app.session.tool_options()["active"].clone(), artboards_selected(&app)), (json!(0), 2));
+    app.ui.align_to = AlignTo::Artboard;
+    app.ui.align_spacing = 20.0;
+    let ctx = egui::Context::default();
+    let (_, space) = distribute_buttons(&mut app, &ctx);
+    click(&ctx, &mut app, space.center(), show);
+    assert_eq!(last_sent(&app), ("artboard.distributeSpacing".to_string(), json!("artboard"), Some(json!(20.0))));
+    assert_eq!(artboard_corners(&app), [(0.0, 0.0), (150.0, 40.0), (120.0, 10.0)], "a gap of 20 after artboard 0: {}", app.ui.status);
+}
+
+/// With the Artboard tool chosen, the Distribute Spacing value can be negative, as
+/// `artboard.distributeSpacing` takes it: the artboards then overlap. With another tool its
+/// lowest value is 0.
+#[test]
+fn the_artboard_spacing_can_be_negative() {
+    let mut app = selected_artboards();
+    assert_eq!(spacing_min(&app), f64::NEG_INFINITY);
+    app.ui.align_to = AlignTo::Artboard;
+    app.ui.align_spacing = -10.0;
+    run_button(&mut app, Button::Spacing("horizontal")).unwrap();
+    assert_eq!(artboard_corners(&app), [(60.0, 0.0), (150.0, 40.0), (200.0, 10.0)], "10 pt overlaps: {}", app.ui.status);
+    app.select_tool("selection");
+    assert_eq!(spacing_min(&app), 0.0);
+}
+
 /// A button's shortcut entry is enabled by, and runs, the artboard command with the Artboard tool
 /// chosen and the object command with another tool.
 #[test]

docs/mcp.md

@@ -5069,8 +5069,9 @@ the Align To choice, and while there is a key object or a key guide they send th
 With the Artboard tool chosen, the same buttons and UI commands, and the Control bar's Align and Distribute buttons,
 run `artboard.align`, `artboard.distribute` and `artboard.distributeSpacing` on the artboards the tool has selected.
 Align To then offers Align to Selection and Align to Artboard. Align to Artboard sends `to: "artboard"`, which keeps the
-tool's active artboard in place, and the Distribute buttons then send the Distribute Spacing value as `spacing`. Align
-to Selection sends `to: "selection"`. The Distribute buttons need two or more selected artboards.
+tool's active artboard in place, and the Distribute buttons then send the Distribute Spacing value as `spacing`, which
+can then be negative. Align to Selection sends `to: "selection"`. The Distribute buttons need two or more selected
+artboards with Align to Artboard, and three or more with Align to Selection.
 
 Edit › Keyboard Shortcuts and `shortcuts.list` leave out a command with no default shortcut when a key press cannot
 run it: no item of the menu bar runs it, it opens no dialog when run without params, and it is one of these:
@@ -6037,7 +6038,8 @@ artboard it names is locked. When no artboard changes, the command records no un
 Align and distribute artboards ([Align and distribute artboards](https://helpx.adobe.com/illustrator/desktop/create-manage-artboards/organize-manage-artboards/align-and-distribute-artboards.html)):
 three commands move whole artboards, by default those the Artboard tool has selected, else its active one. Each takes
 `index? | indices?`, `to?: "selection"|"artboard"`, `artboard?: n`, `moveArt?`, `patterns?` and `lockedAndHidden?`, and
-returns `{moved: [index…]}`.
+returns `{moved: [index…]}`. With `moveArt`, art inside two of the artboards goes with the one first in artboard order, and stays when that one
+stays.
 
 - `artboard.align {horizontal?: "left"|"center"|"right", vertical?: "top"|"center"|"bottom"}` moves each artboard until
   the named edge or center is on that edge or center of the bounds of all the artboards named. One of the two