Merge #10905: Crop with Delete Cropped Pixels off turns the Background into Layer 0 (#10867)

crates/engine/src/cmd/extra_cmds.rs

@@ -877,9 +877,10 @@ pub fn specs() -> Vec<CommandSpec> {
             if all == d.doc.bounds() {
                 return Ok(json!({"changed": false}));
             }
-            // One History step named as in Photoshop, not the crop's "Crop".
+            // One History step named as in Photoshop, not the crop's "Crop". The Background stays a
+            // Background, unlike a Crop tool crop with Delete Cropped Pixels off (#10867).
             let p = json!({"x": all.x0, "y": all.y0, "width": all.width(), "height": all.height(), "deleteCroppedPixels": false});
-            crate::analysis_cmds::compound(s, "Reveal All", |s| s.execute("image.crop", p))?;
+            crate::analysis_cmds::compound(s, "Reveal All", |s| crate::image_cmds::crop_keeping_background(s, &p))?;
             Ok(json!({"changed": true}))
         }),
         spec!(

crates/engine/src/cmd/image_cmds.rs

@@ -614,6 +614,26 @@ const MAX_CROP_SIDE: i32 = 300_000;
 
 /// Image → Crop (to the selection bounds).
 fn crop(s: &mut Session, p: &Value) -> Result<Value> {
+    crop_with(s, p, false)
+}
+
+/// Image › Reveal All's crop: as `image.crop` with Delete Cropped Pixels off, but the Background
+/// stays a Background and an extension takes the background colour (Reveal All keeps its own
+/// behaviour; only the Crop tool's was measured, #10867).
+pub(crate) fn crop_keeping_background(s: &mut Session, p: &Value) -> Result<Value> {
+    crop_with(s, p, true)
+}
+
+/// A Delete Cropped Pixels off crop turns the Background into a normal layer ("Layer 0", as Layer ›
+/// New › Layer from Background names it), as in Photoshop: it keeps the pixels past the canvas,
+/// and an extension is transparent on every layer (#10867).
+fn unlock_background_for_crop(doc: &mut Document) {
+    if let Some(bg) = doc.background_mut() {
+        crate::extra_cmds::unlock_background(bg);
+    }
+}
+
+fn crop_with(s: &mut Session, p: &Value, keep_background: bool) -> Result<Value> {
     let delete = p.get("deleteCroppedPixels").and_then(Value::as_bool).unwrap_or(true);
     // The Crop tool's Fill: Content-Aware Fill of what the crop uncovers (#3823).
     let content_aware = match p.get("contentAware") {
@@ -621,6 +641,8 @@ fn crop(s: &mut Session, p: &Value) -> Result<Value> {
         Some(Value::Bool(b)) => *b,
         Some(v) => return Err(bad("image.crop", format!("`contentAware` must be true or false, not {v}"))),
     };
+    // Content-Aware Fill with Delete off wasn't measured: it keeps filling the Background (#10867).
+    let unlock = !delete && !content_aware && !keep_background;
     // Values beyond i32 would wrap through the narrowing casts into a rectangle unrelated to
     // the numbers passed; reject them (see `commands::int_i32`).
     let explicit = match (
@@ -652,7 +674,7 @@ fn crop(s: &mut Session, p: &Value) -> Result<Value> {
     }
     if let Some(a) = crop_angle(p)? {
         let rect = explicit.ok_or_else(|| bad("image.crop", "an `angle` needs an explicit `x`, `y`, `width` and `height`"))?;
-        return rotated_crop(s, rect, a, delete, content_aware, &target);
+        return rotated_crop(s, rect, a, delete, content_aware, unlock, &target);
     }
     // An explicit rectangle (the Crop tool) may extend past the canvas; a selection crop is clamped.
     let r = explicit
@@ -665,6 +687,9 @@ fn crop(s: &mut Session, p: &Value) -> Result<Value> {
     let extension = s.tools.background;
     s.edit("Crop", |doc, active| {
         let original = doc.size;
+        if unlock {
+            unlock_background_for_crop(doc);
+        }
         crop_doc(doc, "image.crop", r, delete, extension)?;
         // A straight crop fills all of the area past the old canvas, pixels an earlier Delete Cropped
         // Pixels off crop hid there included, as Photoshop does; only a turned frame keeps them (#8060).
@@ -796,8 +821,10 @@ fn crop_angle(p: &Value) -> Result<Option<f64>> {
 /// background colour, other layers keep transparency there and every pixel of theirs), which makes
 /// the frame upright, then cropped to it (`deleteCroppedPixels` as for an unrotated crop), all in
 /// one undo step. With `content_aware`, the corners the rotation uncovers (and any area past the
-/// rotated image) are then filled from their surroundings, in the same step (#3823).
-fn rotated_crop(s: &mut Session, r: Rect, angle: f64, delete: bool, content_aware: bool, crop_target: &CropTarget) -> Result<Value> {
+/// rotated image) are then filled from their surroundings, in the same step (#3823). With `unlock`
+/// the Background becomes a normal layer before the turn, so it turns like the others, keeping its
+/// overhang and leaving the uncovered corners transparent (#10867).
+fn rotated_crop(s: &mut Session, r: Rect, angle: f64, delete: bool, content_aware: bool, unlock: bool, crop_target: &CropTarget) -> Result<Value> {
     const CMD: &str = "image.crop";
     let size = s.active().ok_or(EngineError::NoDocument)?.doc.size;
     let (w, h) = (f64::from(r.width()), f64::from(r.height()));
@@ -860,6 +887,12 @@ fn rotated_crop(s: &mut Session, r: Rect, angle: f64, delete: bool, content_awar
                 Ok(())
             })?;
         }
+        if unlock {
+            s.edit("Crop", |doc, _| {
+                unlock_background_for_crop(doc);
+                Ok(())
+            })?;
+        }
         crate::mode_cmds::rotate_arbitrary(s, &json!({"angle": angle, "direction": "ccw"}))?;
         // Color Sampler points follow the crop's origin shift too (the rotation moved them above).
         s.set_sampler_map(crop_target.sampler_map(target));
@@ -1042,7 +1075,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "image.crop",
             "Crop",
             ["Image"],
-            r##"{"x":px,"y":px,"width":px,"height":px,"angle":-3600..3600=0,"deleteCroppedPixels":bool=true,"contentAware":bool=false,"targetWidth":px,"targetHeight":px,"resolution":ppi,"resample":"bicubic|bilinear|nearest|lanczos|preserveDetails"="bicubic"} → {x,y,width,height,documentWidth,documentHeight,resolution} (targetWidth/targetHeight, both or neither: the cropped area is resampled to exactly that size, as Image Size, in the same undo step; resolution: the document's new ppi; contentAware: the Crop tool's Fill › Content-Aware Fill, the area a rotated or extending crop uncovers filled from its surroundings on the Background, else the active pixel layer)"##,
+            r##"{"x":px,"y":px,"width":px,"height":px,"angle":-3600..3600=0,"deleteCroppedPixels":bool=true,"contentAware":bool=false,"targetWidth":px,"targetHeight":px,"resolution":ppi,"resample":"bicubic|bilinear|nearest|lanczos|preserveDetails"="bicubic"} → {x,y,width,height,documentWidth,documentHeight,resolution} (targetWidth/targetHeight, both or neither: the cropped area is resampled to exactly that size, as Image Size, in the same undo step; resolution: the document's new ppi; deleteCroppedPixels false: the pixels past the frame are kept and the Background becomes a normal layer, "Layer 0", so an extension is transparent on every layer; contentAware: the Crop tool's Fill › Content-Aware Fill, the area a rotated or extending crop uncovers filled from its surroundings on the Background, else the active pixel layer)"##,
             has_doc,
             crop
         ),

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

@@ -486,6 +486,17 @@ fn extending_the_canvas_keeps_the_backgrounds_hidden_pixels() {
             })
             .unwrap();
             s.execute("image.crop", json!({"x": 50, "y": 0, "width": 50, "height": 100, "deleteCroppedPixels": false})).unwrap();
+            // That crop makes the Background "Layer 0" now (#10867); a Background with pixels past
+            // the canvas is still in documents saved before, so lock it back into one.
+            s.edit("relock", |doc, _| {
+                let l = doc.layers.first_mut().unwrap();
+                l.name = "Background".into();
+                l.locks.transparency = true;
+                l.locks.position = true;
+                Ok(())
+            })
+            .unwrap();
+            assert!(doc(s).background().is_some());
             s.tools.background = [0.0, 0.0, 0.0, 1.0];
         };
         let mut s = Session::new();

crates/engine/tests/it/crop_delete_off_background.rs

@@ -0,0 +1,191 @@
+//! Crop with Delete Cropped Pixels off turns the Background into a normal layer, "Layer 0", as
+//! Photoshop does: it keeps the pixels past the canvas, and an extension is transparent on every
+//! layer. Delete on keeps the Background, and Image › Reveal All is unchanged (#10867).
+
+use photocraft_engine::Session;
+use photocraft_geom::Rect;
+use serde_json::{Value, json};
+
+const MAGENTA: [f32; 4] = [1.0, 0.0, 1.0, 1.0];
+const WHITE: [f32; 4] = [1.0, 1.0, 1.0, 1.0];
+const RED: [f32; 4] = [1.0, 0.0, 0.0, 1.0];
+
+/// The issue's test document at a tenth of the size: 80 × 50 px, a white Background and a pixel
+/// layer "Quads" covering the canvas in red, with magenta as the background colour.
+fn doc() -> Session {
+    let mut s = Session::new();
+    s.execute("file.new", json!({"width": 80, "height": 50, "background": "white"})).unwrap();
+    s.execute("layer.new.layer", json!({"name": "Quads"})).unwrap();
+    s.execute("tools.setColors", json!({"foreground": "#ff0000", "background": "#ff00ff"})).unwrap();
+    s.execute("select.all", json!({})).unwrap();
+    s.execute("edit.fill", json!({"contents": "foreground"})).unwrap();
+    s.execute("select.deselect", json!({})).unwrap();
+    s
+}
+
+fn d(s: &Session) -> &photocraft_doc::Document {
+    &s.active().unwrap().doc
+}
+
+fn size(s: &Session) -> (u32, u32) {
+    (d(s).size.width, d(s).size.height)
+}
+
+fn past(s: &Session) -> usize {
+    s.active().unwrap().history.past_len()
+}
+
+fn bottom(s: &Session) -> &photocraft_doc::Layer {
+    &d(s).layers[0]
+}
+
+fn content(s: &Session, i: usize) -> Rect {
+    d(s).layers[i].surface().unwrap().content_bounds()
+}
+
+/// The bottom layer is no longer a Background: a normal, unlocked layer named "Layer 0".
+fn assert_layer_0(s: &Session, what: &str) {
+    assert!(d(s).background().is_none(), "{what}: no Background left");
+    let l = bottom(s);
+    assert_eq!(l.name, "Layer 0", "{what}");
+    assert!(!l.locks.transparency && !l.locks.position, "{what}: unlocked");
+}
+
+fn crop(s: &mut Session, mut p: Value, delete: bool) {
+    p["deleteCroppedPixels"] = json!(delete);
+    let before = past(s);
+    s.execute("image.crop", p).unwrap();
+    assert_eq!(past(s), before + 1, "one undo step");
+}
+
+#[test]
+fn shrinking_turns_the_background_into_layer_0_keeping_the_hidden_pixels() {
+    let mut s = doc();
+    crop(&mut s, json!({"x": 0, "y": 0, "width": 70, "height": 50}), false);
+    assert_eq!(size(&s), (70, 50));
+    assert_layer_0(&s, "shrink");
+    // Both layers keep their 80 px, hidden past the canvas.
+    assert_eq!(content(&s, 0), Rect::new(0, 0, 80, 50), "Layer 0");
+    assert_eq!(content(&s, 1), Rect::new(0, 0, 80, 50), "Quads");
+    assert_eq!(bottom(&s).surface().unwrap().rgba(75, 25), WHITE);
+}
+
+#[test]
+fn extending_leaves_the_new_area_transparent_on_every_layer() {
+    let mut s = doc();
+    crop(&mut s, json!({"x": 0, "y": 0, "width": 90, "height": 50}), false);
+    assert_eq!(size(&s), (90, 50));
+    assert_layer_0(&s, "extend");
+    for i in 0..2 {
+        let surf = d(&s).layers[i].surface().unwrap();
+        assert_eq!(surf.rgba(85, 25)[3], 0.0, "layer {i}: the added strip is transparent");
+    }
+    assert_eq!(bottom(&s).surface().unwrap().rgba(40, 25), WHITE);
+    assert_eq!(d(&s).layers[1].surface().unwrap().rgba(40, 25), RED);
+}
+
+#[test]
+fn a_turned_crop_turns_layer_0_with_the_others() {
+    // The issue's 6.3° frame (687 × 429 of 800 × 500), centred.
+    let mut s = doc();
+    crop(&mut s, json!({"x": 6, "y": 4, "width": 68, "height": 42, "angle": 6.3}), false);
+    assert_eq!(size(&s), (68, 42));
+    assert_layer_0(&s, "turned");
+    // Every layer turned, keeping its overhang past the canvas.
+    let canvas = Rect::new(0, 0, 68, 42);
+    for i in 0..2 {
+        let c = content(&s, i);
+        assert!(c.x0 < canvas.x0 && c.y0 < canvas.y0 && c.x1 > canvas.x1 && c.y1 > canvas.y1, "layer {i} keeps its overhang: {c:?}");
+    }
+    // The corners the turn uncovered are transparent, not the background colour.
+    let surf = bottom(&s).surface().unwrap();
+    let c = content(&s, 0);
+    assert_eq!(surf.rgba(c.x0, c.y0)[3], 0.0, "an uncovered corner of Layer 0");
+    assert_eq!(surf.rgba(34, 21), WHITE);
+}
+
+#[test]
+fn the_action_steps_turned_crop_unlocks_the_background() {
+    // The action step `Crop` with `Dlt` false, rect (100,100)–(700,400) and angle 10°, scaled down.
+    let mut s = doc();
+    crop(&mut s, json!({"x": 10, "y": 10, "width": 60, "height": 30, "angle": 10}), false);
+    assert_eq!(size(&s), (60, 30));
+    assert_layer_0(&s, "action step");
+}
+
+#[test]
+fn a_target_size_crop_unlocks_the_background() {
+    let mut s = doc();
+    crop(&mut s, json!({"x": 0, "y": 0, "width": 40, "height": 50, "targetWidth": 20, "targetHeight": 25, "resolution": 150}), false);
+    assert_eq!(size(&s), (20, 25));
+    assert_layer_0(&s, "W x H x Resolution");
+}
+
+#[test]
+fn delete_on_keeps_the_background_and_fills_an_extension() {
+    for (frame, want) in [
+        (json!({"x": 0, "y": 0, "width": 70, "height": 50}), (70, 50)),
+        (json!({"x": 0, "y": 0, "width": 90, "height": 50}), (90, 50)),
+        (json!({"x": 6, "y": 4, "width": 68, "height": 42, "angle": 6.3}), (68, 42)),
+    ] {
+        let mut s = doc();
+        crop(&mut s, frame.clone(), true);
+        assert_eq!(size(&s), want, "{frame}");
+        let bg = d(&s).background().unwrap_or_else(|| panic!("{frame}: the Background stays"));
+        assert_eq!(bg.name, "Background", "{frame}");
+    }
+    let mut s = doc();
+    crop(&mut s, json!({"x": 0, "y": 0, "width": 90, "height": 50}), true);
+    assert_eq!(bottom(&s).surface().unwrap().rgba(85, 25), MAGENTA, "the extension takes the background colour");
+}
+
+#[test]
+fn undo_restores_the_background() {
+    for frame in [json!({"x": 0, "y": 0, "width": 90, "height": 50}), json!({"x": 6, "y": 4, "width": 68, "height": 42, "angle": 6.3})] {
+        let mut s = doc();
+        let before = bottom(&s).clone();
+        crop(&mut s, frame.clone(), false);
+        assert_layer_0(&s, "cropped");
+        s.execute("edit.undo", json!({})).unwrap();
+        assert_eq!(size(&s), (80, 50), "{frame}");
+        let bg = d(&s).background().unwrap_or_else(|| panic!("{frame}: undo restores the Background"));
+        assert_eq!(bg.id, before.id, "{frame}");
+        assert_eq!(bg.name, "Background", "{frame}");
+        assert!(bg.locks.transparency && bg.locks.position, "{frame}");
+        let (a, b) = (bg.surface().unwrap(), before.surface().unwrap());
+        let r = Rect::new(0, 0, 80, 50);
+        assert_eq!(a.read_region(r), b.read_region(r), "{frame}: the pixels");
+        s.execute("edit.redo", json!({})).unwrap();
+        assert_layer_0(&s, "redo");
+    }
+}
+
+#[test]
+fn reveal_all_keeps_the_background() {
+    let mut s = doc();
+    // Quads reaches 10 px past the right edge.
+    s.edit("paint past the canvas", |doc, _| {
+        let quads = doc.layers.last_mut().unwrap();
+        quads.surface_mut().unwrap().fill_rect(Rect::new(80, 0, 90, 50), &RED);
+        Ok(())
+    })
+    .unwrap();
+    let before = past(&s);
+    s.execute("image.revealAll", json!({})).unwrap();
+    assert_eq!(past(&s), before + 1);
+    assert_eq!(size(&s), (90, 50));
+    let bg = d(&s).background().expect("Reveal All keeps the Background");
+    assert_eq!(bg.name, "Background");
+    assert_eq!(bg.surface().unwrap().rgba(85, 25), MAGENTA, "the extension takes the background colour, as before");
+}
+
+#[test]
+fn a_document_without_a_background_has_nothing_to_convert() {
+    let mut s = Session::new();
+    s.execute("file.new", json!({"width": 80, "height": 50, "background": "transparent"})).unwrap();
+    let names: Vec<String> = d(&s).layers.iter().map(|l| l.name.clone()).collect();
+    crop(&mut s, json!({"x": 0, "y": 0, "width": 90, "height": 50}), false);
+    assert_eq!(size(&s), (90, 50));
+    assert!(d(&s).background().is_none());
+    assert_eq!(d(&s).layers.iter().map(|l| l.name.clone()).collect::<Vec<_>>(), names, "no layer renamed");
+}

crates/engine/tests/it/main.rs

@@ -20,6 +20,7 @@ mod convert_keeps_references;
 mod copy_shape_placement;
 mod corpus_atn;
 mod crop_content_aware;
+mod crop_delete_off_background;
 mod current_affinity_artboards;
 mod duplicate_layer_comps;
 mod duplicate_slices;

crates/ui-egui/tests/it/crop_beyond_canvas.rs

@@ -110,7 +110,8 @@ fn editing_a_crop_frame_shows_the_pixels_past_the_canvas() {
         let app = h.state_mut();
         // 300 × 120: red down the left 60 px and blue down the right 60 px of a new layer, then
         // cropped to (50, 0) 200 × 100 keeping the pixels: 50 px of red past the left edge, 50 of
-        // blue past the right, and 20 rows of the white Background below.
+        // blue past the right, and 20 rows of the white Background below (the crop makes it "Layer 0",
+        // which keeps them, #10867).
         app.run("file.new", json!({"width": 300, "height": 120, "background": "white"})).expect("new");
         app.run("layer.new.layer", json!({})).expect("layer");
         for (x, color) in [(0, "#ff0000"), (240, "#0000ff")] {