Merge pull request #9303 from davidmann4/fix/artboard-shift-samplers

Artboards: Color Sampler points follow the document when a board re-bases the canvas (#9302)

crates/engine/src/cmd.rs

@@ -587,6 +587,7 @@ fn build() -> Vec<CommandSpec> {
                 let add_copy = layer_panel_option(s, "addCopyToCopiedLayersAndGroups", true);
                 // The dialog offers Duplicate Layer's own name: only a different one was typed (#9023).
                 let mut typed = None;
+                let mut shift = (0, 0);
                 let nid = s.edit("Duplicate Layer", |doc, active| {
                     let mut dup = layer_copy(doc, id, add_copy)?;
                     if let Some(name) = name.as_ref().filter(|n| **n != dup.name) {
@@ -597,11 +598,12 @@ fn build() -> Vec<CommandSpec> {
                     // A lone copy joins its original's link group, as in the reference application (#11217).
                     crate::layer_multi_cmds::relink_copies(doc, &[nid])?;
                     if !in_place {
-                        crate::artboard_cmds::place_copy(doc, nid)?;
+                        shift = crate::artboard_cmds::place_copy(doc, nid)?;
                     }
                     *active = Some(nid);
                     Ok(nid)
                 })?;
+                crate::artboard_cmds::follow_shift(s, shift);
                 if let Some(name) = typed {
                     crate::type_cmds::note_typed_name(s, nid, &name);
                 }

crates/engine/src/cmd/artboard_cmds.rs

@@ -152,9 +152,11 @@ fn xywh(r: Rect) -> [i64; 4] {
 /// it puts it right of the board, a [`GAP`] away, sliding further right past any board in the
 /// way. Moves the copy, layer `id` (just inserted on top of its original), there with its
 /// contents and grows the canvas to show it; anything that isn't an artboard is left alone.
-pub(crate) fn place_copy(doc: &mut Document, id: LayerId) -> Result<()> {
+/// Returns [`fit_canvas`]'s shift (a board already past the origin, as a PSD can hold, re-bases the
+/// document), for [`follow_shift`].
+pub(crate) fn place_copy(doc: &mut Document, id: LayerId) -> Result<(i32, i32)> {
     let l = doc.layer(id).ok_or(EngineError::NoLayer(id))?;
-    let Some(r) = l.artboard().map(|a| a.rect) else { return Ok(()) };
+    let Some(r) = l.artboard().map(|a| a.rect) else { return Ok((0, 0)) };
     let name = l.name.clone();
     let boards: Vec<Rect> = doc.artboards().iter().filter(|b| b.0 != id).map(|b| b.2.rect).collect();
     let (y0, y1, w) = (i64::from(r.y0), i64::from(r.y1), i64::from(r.width()));
@@ -180,8 +182,7 @@ pub(crate) fn place_copy(doc: &mut Document, id: LayerId) -> Result<()> {
     let copy = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?;
     crate::commands::translate_layer(&snapshot, copy, dx, 0);
     crate::vector_cmds::translate_vectors(&snapshot, copy, f64::from(dx), 0.0);
-    fit_canvas(doc);
-    Ok(())
+    Ok(fit_canvas(doc))
 }
 
 /// Record a Duplicate's `inPlace` only when a copy (the selection after it) is an artboard: for
@@ -203,8 +204,11 @@ pub(crate) fn recorded_duplicate(s: &Session, mut p: Value) -> Value {
 /// contents, guides, paths…) right/down by its overhang, then the canvas grows to show it. An
 /// axis whose boards span more than the largest canvas, or a shift that would push pixels past
 /// the 32-bit range, stays unshifted: the board stays partly off-canvas, as one past the size
-/// limit does.
-pub(crate) fn fit_canvas(doc: &mut Document) {
+/// limit does. Returns the shift, for [`follow_shift`]: `translate_doc` moves the document's own
+/// geometry, but the session's Color Sampler points and painting symmetry stay behind unless the
+/// caller passes it on.
+#[must_use = "pass the shift to `follow_shift`, or the Color Samplers and symmetry stay behind (#9302)"]
+pub(crate) fn fit_canvas(doc: &mut Document) -> (i32, i32) {
     let max = i64::from(crate::document_preset_cmds::MAX_DIMENSION);
     let boards: Vec<Rect> = doc.artboards().iter().map(|b| b.2.rect).collect();
     // The canvas and every board, per axis (`lo` ≤ 0 is the canvas origin or a board past it).
@@ -226,6 +230,20 @@ pub(crate) fn fit_canvas(doc: &mut Document) {
         // shapes against it, leaving them missing or clipped (#2389).
         crate::canvas_geom::refresh(doc, crate::canvas_geom::Refresh::Shapes);
     }
+    (sx, sy)
+}
+
+/// Keep the Color Sampler points and the painting symmetry on their pixels after an edit shifted
+/// the document by `(dx, dy)` to show a board past the origin ([`fit_canvas`]), as Canvas Size's
+/// sampler map does (#9302). View state: no history step. The canvas only grew, so every point
+/// stays on it.
+pub(crate) fn follow_shift(s: &mut Session, (dx, dy): (i32, i32)) {
+    let Some(st) = s.active_mut().filter(|_| (dx, dy) != (0, 0)) else { return };
+    for p in &mut st.color_samplers {
+        *p = [p[0] + f64::from(dx), p[1] + f64::from(dy)];
+    }
+    let a = photocraft_geom::Affine::translate(f64::from(dx), f64::from(dy));
+    st.symmetry = st.symmetry.take().and_then(|sym| sym.transform(&a));
 }
 
 fn next_artboard_name(doc: &Document) -> String {
@@ -294,7 +312,7 @@ fn new_artboard(s: &mut Session, p: &Value) -> Result<Value> {
         None => false,
         Some(v) => v.as_bool().ok_or_else(|| bad(cmd, "`takeBackground` must be true or false"))?,
     };
-    let id = s.edit("New Artboard", |doc, active| {
+    let (id, shift) = s.edit("New Artboard", |doc, active| {
         let mut g = Layer::group(name.unwrap_or_else(|| next_artboard_name(doc)), vec![]);
         set_artboard(&mut g, Artboard { rect, background, preset });
         // Only the first board takes the Background in (decided before the board exists).
@@ -303,10 +321,11 @@ fn new_artboard(s: &mut Session, p: &Value) -> Result<Value> {
         if take {
             move_background_into(doc, id);
         }
-        fit_canvas(doc);
+        let shift = fit_canvas(doc);
         *active = Some(id);
-        Ok(id)
+        Ok((id, shift))
     })?;
+    follow_shift(s, shift);
     crate::layer_multi_cmds::set_selection(s, vec![id], Some(id), Some(id))?;
     Ok(json!({"layer": id.0, "rect": [rect.x0, rect.y0, rect.width(), rect.height()]}))
 }
@@ -344,17 +363,18 @@ fn artboard_from_group(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let canvas = d.doc.bounds();
     let background = if apply_background { background_param(p, cmd)?.unwrap_or_default() } else { ArtboardBackground::White };
-    let rect = s.edit("Artboard from Group", |doc, active| {
+    let (rect, shift) = s.edit("Artboard from Group", |doc, active| {
         if !doc.layers.iter().any(|t| t.id == id && t.is_group() && t.artboard().is_none()) {
             return Err(EngineError::Other("artboards are made from a top-level layer group".into()));
         }
         let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?;
         let rect = crate::layer_multi_cmds::layer_bounds(l).unwrap_or(canvas);
         set_artboard(l, Artboard { rect, background, preset });
-        fit_canvas(doc);
+        let shift = fit_canvas(doc);
         *active = Some(id);
-        Ok(rect)
+        Ok((rect, shift))
     })?;
+    follow_shift(s, shift);
     crate::layer_multi_cmds::set_selection(s, vec![id], Some(id), Some(id))?;
     Ok(json!({"layer": id.0, "rect": [rect.x0, rect.y0, rect.width(), rect.height()]}))
 }
@@ -388,7 +408,7 @@ fn artboard_from_layers(s: &mut Session, p: &Value) -> Result<Value> {
     // A size of the user's own (a post, a story…) from the layers' top left; 0 or none keeps
     // their bounds on that axis (#9087).
     let size = [int(p, "width").filter(|v| *v != 0), int(p, "height").filter(|v| *v != 0)];
-    let (gid, rect) = s.edit("Artboard from Layers", |doc, active| {
+    let (gid, rect, shift) = s.edit("Artboard from Layers", |doc, active| {
         let ids = crate::layer_multi_cmds::top_level(doc, &sel);
         if ids.is_empty() || ids.iter().any(|id| doc.artboard_of(*id).is_some()) {
             return Err(EngineError::Other("select layers that are not in an artboard".into()));
@@ -403,10 +423,11 @@ fn artboard_from_layers(s: &mut Session, p: &Value) -> Result<Value> {
         set_artboard(&mut g, Artboard { rect, background, preset: String::new() });
         let gid = doc.insert_above(None, g);
         crate::layer_multi_cmds::check_group_depth(doc, "Artboard from Layers")?;
-        fit_canvas(doc);
+        let shift = fit_canvas(doc);
         *active = Some(gid);
-        Ok((gid, rect))
+        Ok((gid, rect, shift))
     })?;
+    follow_shift(s, shift);
     crate::layer_multi_cmds::set_selection(s, vec![gid], Some(gid), Some(gid))?;
     Ok(json!({"layer": gid.0, "rect": [rect.x0, rect.y0, rect.width(), rect.height()]}))
 }
@@ -445,7 +466,7 @@ fn set_props(s: &mut Session, p: &Value) -> Result<Value> {
             ));
         }
     };
-    s.edit("Edit Artboard", |doc, _| {
+    let shift = s.edit("Edit Artboard", |doc, _| {
         if move_contents && (dx, dy) != (0, 0) {
             let snapshot = doc.clone();
             let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?;
@@ -467,9 +488,9 @@ fn set_props(s: &mut Session, p: &Value) -> Result<Value> {
             None if rect.width() != old.rect.width() || rect.height() != old.rect.height() => a.preset.clear(),
             None => {}
         }
-        fit_canvas(doc);
-        Ok(())
+        Ok(fit_canvas(doc))
     })?;
+    follow_shift(s, shift);
     Ok(json!({"layer": id.0, "rect": [rect.x0, rect.y0, rect.width(), rect.height()]}))
 }
 
@@ -546,7 +567,7 @@ fn add_beside(s: &mut Session, p: &Value) -> Result<Value> {
     let grown = |len: u32, by: i32| (i64::from(len) + i64::from(by)).clamp(1, 300_000) as u32;
     let label = if duplicate { "Duplicate Artboard" } else { "New Artboard" };
     let add_copy = crate::commands::layer_panel_option(s, "addCopyToCopiedLayersAndGroups", true);
-    let (nid, rect) = s.edit(label, |doc, active| {
+    let (nid, rect, (fx, fy)) = s.edit(label, |doc, active| {
         if (sx, sy) != (0, 0) {
             crate::image_cmds::translate_doc(doc, cmd, sx, sy)?;
             doc.size = Size::new(grown(doc.size.width, sx), grown(doc.size.height, sy));
@@ -575,10 +596,11 @@ fn add_beside(s: &mut Session, p: &Value) -> Result<Value> {
             set_artboard(&mut g, Artboard { rect, background: src.background, preset: src.preset.clone() });
             doc.insert_above(Some(id), g)
         };
-        fit_canvas(doc);
+        let fitted = fit_canvas(doc);
         *active = Some(nid);
-        Ok((nid, rect))
+        Ok((nid, rect, fitted))
     })?;
+    follow_shift(s, (sx.saturating_add(fx), sy.saturating_add(fy)));
     crate::layer_multi_cmds::set_selection(s, vec![nid], Some(nid), Some(nid))?;
     Ok(json!({"layer": nid.0, "rect": [rect.x0, rect.y0, rect.width(), rect.height()]}))
 }

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

@@ -723,6 +723,87 @@ fn moving_an_artboard_left_of_or_above_the_origin_grows_the_canvas_there() {
     }
 }
 
+#[test]
+fn a_board_past_the_origin_moves_the_color_samplers_and_symmetry_with_the_document() {
+    // #9302: the re-based document moved everything but the Color Sampler points and the
+    // painting symmetry, which then sat on other pixels. They follow, as with Canvas Size.
+    let samplers = |s: &Session| s.active().unwrap().color_samplers.clone();
+    let center = |s: &Session| s.active().unwrap().symmetry.as_ref().and_then(|y| y.preset()).map(|p| p.center);
+    for (cmd, p) in [("layer.translate", json!({"dx": -60, "dy": -30})), ("layer.artboard.set", json!({"x": -50, "y": -20}))] {
+        let mut s = session(8);
+        let gid = board_with_square(&mut s);
+        s.execute("view.colorSamplers.add", json!({"x": 15, "y": 15})).unwrap();
+        s.execute("paint.setSymmetry", json!({"mode": "vertical", "center": [70.0, 20.0]})).unwrap();
+        s.execute("layer.select", json!({"layer": gid.0})).unwrap();
+        s.execute(cmd, p).unwrap();
+        // The board lands at (0, 0) after the document shifted right 50 and down 20; the sampler
+        // is pinned to the canvas, not the board, so it shifts with the document only.
+        assert_eq!(board_rect(&s, gid.0), Rect::new(0, 0, 40, 40), "{cmd}");
+        assert_eq!(samplers(&s), vec![[65.0, 35.0]], "{cmd}");
+        assert_eq!(center(&s), Some([120.0, 40.0]), "{cmd}: the symmetry stays on its pixels");
+    }
+    // A new board left of the canvas shifts the photo right by its overhang.
+    let mut s = session(8);
+    add_square(&mut s, Rect::new(10, 10, 30, 30), [1.0, 0.0, 0.0, 1.0]);
+    s.execute("view.colorSamplers.add", json!({"x": 15, "y": 15})).unwrap();
+    s.execute("layer.new.artboard", json!({"rect": [-200, 0, 100, 80]})).unwrap();
+    assert_eq!(samplers(&s), vec![[215.0, 15.0]]);
+    assert!(red_at(&s, 215, 15));
+    // A board made from layers (or a group) left of the canvas shifts by their overhang.
+    for cmd in ["layer.new.artboardFromLayers", "layer.new.artboardFromGroup"] {
+        let mut s = session(8);
+        add_square(&mut s, Rect::new(10, 10, 30, 30), [1.0, 0.0, 0.0, 1.0]);
+        s.execute("view.colorSamplers.add", json!({"x": 15, "y": 15})).unwrap();
+        add_square(&mut s, Rect::new(-30, 40, -10, 60), [0.0, 0.0, 1.0, 1.0]);
+        if cmd.ends_with("Group") {
+            s.execute("layer.groupLayers", json!({})).unwrap();
+        }
+        s.execute(cmd, json!({})).unwrap();
+        assert_eq!(samplers(&s), vec![[45.0, 15.0]], "{cmd}");
+        assert!(red_at(&s, 45, 15), "{cmd}");
+    }
+    // The "+" button left of a board at the origin shifts by the board and the gap.
+    let mut s = session(8);
+    let gid = board_with_square(&mut s);
+    s.execute("layer.artboard.set", json!({"x": 0, "y": 0})).unwrap();
+    s.execute("view.colorSamplers.add", json!({"x": 5, "y": 5})).unwrap();
+    s.execute("layer.artboard.addBeside", json!({"layer": gid.0, "side": "left"})).unwrap();
+    let by = 40 + GAP;
+    assert_eq!(board_rect(&s, gid.0), Rect::new(by, 0, by + 40, 40));
+    assert_eq!(samplers(&s), vec![[f64::from(by) + 5.0, 5.0]]);
+    assert!(red_at(&s, by + 5, 5));
+    // A board right of the canvas shifts nothing.
+    s.execute("layer.artboard.addBeside", json!({"layer": gid.0, "side": "right"})).unwrap();
+    assert_eq!(samplers(&s), vec![[f64::from(by) + 5.0, 5.0]]);
+}
+
+#[test]
+fn duplicating_a_board_beside_one_past_the_origin_moves_the_color_samplers_with_the_document() {
+    // #9302 review: Layer › Duplicate puts the copy right of its board, but `fit_canvas` still
+    // re-bases a document whose boards were already past the origin (as a PSD can hold them).
+    // Single (`layer`) and several selected layers (`duplicate_selected`) both follow the shift.
+    for multi in [false, true] {
+        let mut s = session(8);
+        let gid = board_with_square(&mut s);
+        s.edit("past the origin", |doc, _| {
+            doc.layer_mut(gid).and_then(|l| l.artboard_mut()).unwrap().rect = Rect::new(-40, 10, 0, 50);
+            Ok(())
+        })
+        .unwrap();
+        let sq = add_square(&mut s, Rect::new(60, 10, 80, 30), [1.0, 0.0, 0.0, 1.0]);
+        s.execute("view.colorSamplers.add", json!({"x": 65, "y": 15})).unwrap();
+        if multi {
+            crate::layer_multi_cmds::set_selection(&mut s, vec![gid, sq], Some(gid), Some(gid)).unwrap();
+            s.execute("layer.duplicate", json!({})).unwrap();
+        } else {
+            s.execute("layer.duplicate", json!({"layer": gid.0})).unwrap();
+        }
+        assert_eq!(board_rect(&s, gid.0), Rect::new(0, 10, 40, 50), "multi {multi}: the document shifted right 40");
+        assert_eq!(s.active().unwrap().color_samplers, vec![[105.0, 15.0]], "multi {multi}");
+        assert!(red_at(&s, 105, 15), "multi {multi}: the sampler is still on the square");
+    }
+}
+
 #[test]
 fn add_beside_places_a_board_on_each_side_and_alt_duplicates_contents() {
     // #3033: the Artboard tool's "+" buttons. Board 40×40 at (10, 10) holding a red square.

crates/engine/src/cmd/layer_multi_cmds/moves.rs

@@ -152,6 +152,7 @@ pub fn translate(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let mask_target = p.get("target").and_then(Value::as_str) == Some("mask");
     let before = s.active().ok_or(EngineError::NoDocument)?.doc.clone();
+    let mut shift = (0, 0);
     let nested = s.edit("Move", |doc, _| {
         let (alone, rest): (Vec<LayerId>, Vec<LayerId>) = roots.iter().partition(|id| mask_target && doc.layer(**id).is_some_and(moves_mask_alone));
         let mut ids = move_targets(doc, &rest);
@@ -175,7 +176,7 @@ pub fn translate(s: &mut Session, p: &Value) -> Result<Value> {
         }
         // A board moved past any canvas edge grows the canvas, as a new one does (#1531, #3070).
         if ids.iter().any(|id| doc.layer(*id).is_some_and(|l| l.artboard().is_some())) {
-            crate::artboard_cmds::fit_canvas(doc);
+            shift = crate::artboard_cmds::fit_canvas(doc);
         }
         // The Move tool carries the selection along when it moves several layers whole, as
         // Photoshop does (#7538): one step, so Undo takes both back.
@@ -184,6 +185,7 @@ pub fn translate(s: &mut Session, p: &Value) -> Result<Value> {
         }
         Ok(nested)
     })?;
+    crate::artboard_cmds::follow_shift(s, shift);
     if nested.is_empty() {
         return Ok(Value::Null);
     }

crates/engine/src/cmd/layer_multi_cmds/structure.rs

@@ -272,6 +272,7 @@ pub fn duplicate_selected(s: &mut Session, in_place: bool, label: &str) -> Resul
     let sel = selected(s);
     let old_active = s.active().and_then(|d| d.active_layer);
     let add_copy = crate::commands::layer_panel_option(s, "addCopyToCopiedLayersAndGroups", true);
+    let mut shift = (0i32, 0i32);
     let (copies, active) = s.edit(label, |doc, active| {
         let mut copies = Vec::new();
         let mut new_active = None;
@@ -281,7 +282,9 @@ pub fn duplicate_selected(s: &mut Session, in_place: bool, label: &str) -> Resul
             let dup = crate::commands::layer_copy(doc, id, add_copy)?;
             let nid = doc.insert_above(Some(id), dup);
             if !in_place {
-                crate::artboard_cmds::place_copy(doc, nid)?;
+                // Each copy's shift moved the whole document, so they add up.
+                let (dx, dy) = crate::artboard_cmds::place_copy(doc, nid)?;
+                shift = (shift.0.saturating_add(dx), shift.1.saturating_add(dy));
             }
             if Some(id) == old_active {
                 new_active = Some(nid);
@@ -293,6 +296,7 @@ pub fn duplicate_selected(s: &mut Session, in_place: bool, label: &str) -> Resul
         *active = a;
         Ok((copies, a))
     })?;
+    crate::artboard_cmds::follow_shift(s, shift);
     let out = copies.iter().map(|l| l.0).collect::<Vec<_>>();
     reselect(s, copies, active);
     Ok(json!({"layers": out, "layer": active.map(|l| l.0)}))