Fix Paint Bucket sampling across visible layers

crates/algo/src/paint.rs

@@ -186,10 +186,23 @@ pub fn bucket_fill_src(
     let img = crate::selection::rgba8_image(s, area);
     let Some(region) = crate::selection::wand_region(&img, area, seed, tolerance, contiguous, anti_alias) else { return false };
     drop(img);
+    bucket_fill_region_src(s, &region, opacity, selection, lock_transparency, src);
+    true
+}
+
+/// Paints a bucket region sampled before the edit, so its boundary can come from the visible
+/// composite while only `s` receives the colour or pattern. Selection coverage still clips it.
+pub fn bucket_fill_region_src(
+    s: &mut Surface,
+    region: &crate::selection::Region,
+    opacity: f32,
+    selection: Option<&Surface>,
+    lock_transparency: bool,
+    src: impl Fn(i32, i32) -> [f32; 4] + Sync,
+) {
     // Composite only over the filled region's box.
     let b = region.bbox;
     composite_area(s, b, BlendMode::Normal, |x, y| region.at(x, y) * opacity * selection.map_or(1.0, |m| m.sample_channel(x, y, 0)), src, lock_transparency);
-    true
 }
 
 #[cfg(test)]
@@ -244,4 +257,17 @@ mod tests {
         assert_eq!(s.pixel(8, 2), vec![1.0, 1.0, 1.0, 1.0]);
         assert!(!bucket_fill(&mut s, a, (50, 1), 10.0, true, false, [1.0, 0.0, 0.0, 1.0], 1.0, None, false));
     }
+
+    #[test]
+    fn bucket_region_uses_document_coordinates_and_multiplies_coverage() {
+        let mut s = Surface::new(PixelFormat::RGBA8);
+        let region = crate::selection::Region { bbox: Rect::new(-3, 4, 0, 5), mask: vec![255, 128, 255] };
+        let mut selection = Surface::new(PixelFormat::GRAY8);
+        selection.fill_rect(Rect::new(-3, 4, -1, 5), &[0.5]);
+        bucket_fill_region_src(&mut s, &region, 0.5, Some(&selection), false, |_, _| [1.0, 0.0, 0.0, 1.0]);
+        assert!((s.rgba(-3, 4)[3] - 0.25).abs() < 0.005);
+        assert!((s.rgba(-2, 4)[3] - 0.125).abs() < 0.005);
+        assert_eq!(s.rgba(-1, 4), [0.0; 4], "outside the selection");
+        assert_eq!(s.rgba(-4, 4), [0.0; 4], "outside the region");
+    }
 }

crates/engine/src/paint_cmds.rs

@@ -1,6 +1,6 @@
 //! Paint helpers: Paint Bucket and Gradient tool.
 
-use photocraft_algo::paint::{GradientShape, bucket_fill, bucket_fill_src, paint_gradient};
+use photocraft_algo::paint::{GradientShape, bucket_fill, bucket_fill_region_src, bucket_fill_src, paint_gradient};
 use serde_json::{Value, json};
 
 use crate::commands::{CommandSpec, blend_from_str};
@@ -40,6 +40,16 @@ fn bucket(s: &mut Session, p: &Value) -> Result<Value> {
     let c = color(p.get("color"), fg);
     let (x, y) = (f(p, "x", 0.0).floor() as i32, f(p, "y", 0.0).floor() as i32);
     let (tol, contiguous, aa, opacity) = (f(p, "tolerance", 32.0), b(p, "contiguous", true), b(p, "antiAlias", true), f(p, "opacity", 100.0) / 100.0);
+    let all = b(p, "sampleAllLayers", false) && crate::channel_cmds::target_of(p) == crate::channel_cmds::Target::Pixels;
+    // Other visible layers can define the region, but the fill still writes only to the target.
+    let region = if all {
+        let (area, img) = crate::selection_cmds::sample_rgba8(s, true)?;
+        let region = photocraft_algo::selection::wand_region(&img, area, (x, y), tol, contiguous, aa);
+        drop(img);
+        region
+    } else {
+        None
+    };
     // Fill source: foreground colour (default) or a pattern (the Paint Bucket "Fill" dropdown).
     let source = p.get("contents").or_else(|| p.get("source")).and_then(Value::as_str).unwrap_or("foreground");
     let pattern = if source == "pattern" {
@@ -55,14 +65,23 @@ fn bucket(s: &mut Session, p: &Value) -> Result<Value> {
         let area = doc.bounds();
         let sel = doc.selection.clone();
         let (surf, lock) = crate::channel_cmds::target_surface(doc, *active, p)?;
-        let ok = if let Some((tile, scale, angle, phase)) = &pattern {
+        let ok = if all && region.is_none() {
+            false
+        } else if let Some((tile, scale, angle, phase)) = &pattern {
             // Render the pattern over the canvas once, then sample it at each filled pixel.
             let place = photocraft_compose::pattern::Placement::new(photocraft_geom::Rect::EMPTY, false, *phase, *scale, *angle);
             let rendered = photocraft_compose::pattern::render(tile, &place, area);
             let w = area.width() as usize;
-            bucket_fill_src(surf, area, (x, y), tol, contiguous, aa, opacity, sel.as_ref(), lock, |px, py| {
-                rendered[(py - area.y0) as usize * w + (px - area.x0) as usize]
-            })
+            let src = |px: i32, py: i32| rendered[(py - area.y0) as usize * w + (px - area.x0) as usize];
+            if let Some(region) = &region {
+                bucket_fill_region_src(surf, region, opacity, sel.as_ref(), lock, src);
+                true
+            } else {
+                bucket_fill_src(surf, area, (x, y), tol, contiguous, aa, opacity, sel.as_ref(), lock, src)
+            }
+        } else if let Some(region) = &region {
+            bucket_fill_region_src(surf, region, opacity, sel.as_ref(), lock, |_, _| c);
+            true
         } else {
             bucket_fill(surf, area, (x, y), tol, contiguous, aa, c, opacity, sel.as_ref(), lock)
         };
@@ -117,7 +136,7 @@ pub fn specs() -> Vec<CommandSpec> {
             label: "Paint Bucket",
             menu: &[],
             shortcut: None,
-            params: r##"{"x":px,"y":px,"tolerance":0..255=32,"contiguous":bool=true,"antiAlias":bool=true,"contents":"foreground|pattern"="foreground","color":"#rrggbb"=foreground,"pattern":id|name (contents=pattern),"scale":%=100,"angle":deg,"opacity":1..100=100,"target":"pixels"|"mask"|"quickMask"|{"channel":i}=Channels panel target}"##,
+            params: r##"{"x":px,"y":px,"tolerance":0..255=32,"contiguous":bool=true,"antiAlias":bool=true,"sampleAllLayers":bool=false,"contents":"foreground|pattern"="foreground","color":"#rrggbb"=foreground,"pattern":id|name (contents=pattern),"scale":%=100,"angle":deg,"opacity":1..100=100,"target":"pixels"|"mask"|"quickMask"|{"channel":i}=Channels panel target}"##,
             enabled: crate::commands::has_paintable,
             run: bucket,
             journal: true,
@@ -151,6 +170,151 @@ mod tests {
         d.doc.layer(d.active_layer.unwrap()).unwrap().surface().unwrap().pixel(x, y)
     }
 
+    /// A visible wall on the Background with an empty, active layer above it.
+    fn bucket_layers(depth: u32) -> (Session, photocraft_doc::LayerId) {
+        let mut s = Session::new();
+        s.execute("file.new", json!({"width": 20, "height": 10, "depth": depth, "background": "white"})).unwrap();
+        let bottom = s.active().unwrap().active_layer.unwrap();
+        s.edit("wall", |doc, _| {
+            doc.layer_mut(bottom).unwrap().surface_mut().unwrap().fill_rect(Rect::new(10, 0, 11, 10), &[0.0, 0.0, 0.0, 1.0]);
+            Ok(())
+        })
+        .unwrap();
+        s.execute("layer.new.layer", json!({})).unwrap();
+        (s, bottom)
+    }
+
+    #[test]
+    fn bucket_sample_all_layers_uses_visible_boundaries_at_all_depths() {
+        for depth in [8, 16, 32] {
+            for all in [None, Some(false), Some(true)] {
+                let (mut s, bottom) = bucket_layers(depth);
+                let before = s.active().unwrap().doc.layer(bottom).unwrap().clone();
+                let mut p = json!({"x": 2, "y": 2, "color": "#ff0000", "antiAlias": false, "tolerance": 0});
+                if let Some(all) = all {
+                    p["sampleAllLayers"] = json!(all);
+                }
+                assert_eq!(s.execute("paint.bucket", p.clone()).unwrap()["filled"], true);
+                assert_eq!(px(&s, 5, 5), vec![1.0, 0.0, 0.0, 1.0], "depth {depth}, all {all:?}");
+                let beyond = if all == Some(true) { vec![0.0; 4] } else { vec![1.0, 0.0, 0.0, 1.0] };
+                assert_eq!(px(&s, 15, 5), beyond, "depth {depth}, all {all:?}: sample flag controls the boundary");
+                assert_eq!(s.active().unwrap().doc.layer(bottom).unwrap(), &before, "only the active layer is written");
+                if all == Some(true) {
+                    assert_eq!(px(&s, 10, 5), vec![0.0; 4], "wall itself is untouched");
+                    assert!(s.undo());
+                    p["contiguous"] = json!(false);
+                    s.execute("paint.bucket", p).unwrap();
+                    assert_eq!(px(&s, 15, 5), vec![1.0, 0.0, 0.0, 1.0], "noncontiguous finds both white regions");
+                    assert_eq!(px(&s, 10, 5), vec![0.0; 4]);
+                    assert!(s.undo());
+                    assert_eq!(s.execute("paint.bucket", json!({"x": -1, "y": 2, "sampleAllLayers": true})).unwrap()["filled"], false);
+                    assert_eq!(px(&s, 5, 5), vec![0.0; 4], "an off-canvas sample does not fill the layer");
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn bucket_sample_all_layers_ignores_hidden_layers() {
+        let (mut s, _) = bucket_layers(8);
+        s.edit("hidden wall", |doc, active| {
+            let l = doc.layer_mut(active.unwrap()).unwrap();
+            l.surface_mut().unwrap().fill_rect(Rect::new(4, 0, 5, 10), &[0.0, 0.0, 0.0, 1.0]);
+            l.visible = false;
+            Ok(())
+        })
+        .unwrap();
+        s.execute("layer.new.layer", json!({})).unwrap();
+        s.execute("paint.bucket", json!({"x": 2, "y": 2, "color": "#ff0000", "antiAlias": false, "tolerance": 0, "sampleAllLayers": true})).unwrap();
+        assert_eq!(px(&s, 4, 5), vec![1.0, 0.0, 0.0, 1.0], "hidden wall does not bound the fill");
+        assert_eq!(px(&s, 8, 5), vec![1.0, 0.0, 0.0, 1.0]);
+        assert_eq!(px(&s, 15, 5), vec![0.0; 4], "visible wall still bounds the fill");
+    }
+
+    #[test]
+    fn bucket_sample_all_layers_keeps_selection_opacity_and_history() {
+        for depth in [8, 16, 32] {
+            let (mut s, _) = bucket_layers(depth);
+            s.execute("select.rect", json!({"x": 0, "y": 2, "width": 20, "height": 6})).unwrap();
+            s.edit("feather", |doc, _| {
+                doc.selection.as_mut().unwrap().fill_rect(Rect::new(0, 2, 4, 8), &[0.5]);
+                Ok(())
+            })
+            .unwrap();
+            let selection = s.active().unwrap().doc.selection.clone();
+            s.execute("paint.bucket", json!({"x": 2, "y": 5, "color": "#ff0000", "antiAlias": false, "tolerance": 0, "opacity": 50, "sampleAllLayers": true}))
+                .unwrap();
+            let (feathered, full) = (px(&s, 2, 5), px(&s, 6, 5));
+            assert!((feathered[3] - 0.25).abs() < 0.005 && (full[3] - 0.5).abs() < 0.005, "depth {depth}: selection coverage multiplies opacity");
+            assert_eq!(full[..3], [1.0, 0.0, 0.0]);
+            assert_eq!(px(&s, 6, 1), vec![0.0; 4], "outside selection");
+            assert_eq!(px(&s, 15, 5), vec![0.0; 4], "outside sampled region");
+            assert_eq!(s.active().unwrap().history.undo_label(), Some("Paint Bucket"));
+            assert!(s.undo());
+            assert_eq!(px(&s, 2, 5), vec![0.0; 4]);
+            assert_eq!(s.active().unwrap().doc.selection, selection);
+            assert!(s.redo());
+            assert_eq!(px(&s, 2, 5), feathered);
+            assert_eq!(px(&s, 6, 5), full);
+            assert_eq!(s.active().unwrap().doc.selection, selection);
+        }
+    }
+
+    #[test]
+    fn bucket_sample_all_layers_pattern_uses_the_same_region() {
+        let (mut s, _) = bucket_layers(8);
+        s.execute(
+            "paint.bucket",
+            json!({"x": 2, "y": 2, "contents": "pattern", "pattern": "Diagonal Lines", "antiAlias": false, "tolerance": 0, "sampleAllLayers": true}),
+        )
+        .unwrap();
+        let left: Vec<f32> = (0..10).map(|x| px(&s, x, 0)[0]).collect();
+        assert!(left.iter().any(|&r| r < 0.3) && left.iter().any(|&r| r > 0.9), "pattern varies inside the region: {left:?}");
+        assert_eq!(px(&s, 10, 5), vec![0.0; 4], "wall untouched");
+        assert_eq!(px(&s, 15, 5), vec![0.0; 4], "region past the wall untouched");
+    }
+
+    #[test]
+    fn bucket_sample_all_layers_mask_and_channel_targets_keep_their_own_boundaries() {
+        for target in [json!("mask"), json!({"channel": 0}), json!("quickMask")] {
+            for all in [false, true] {
+                let (mut s, _) = bucket_layers(8);
+                match crate::channel_cmds::target_of(&json!({"target": target})) {
+                    crate::channel_cmds::Target::Mask => {
+                        s.execute("layer.layerMask.revealAll", json!({})).unwrap();
+                    }
+                    crate::channel_cmds::Target::Alpha(_) => {
+                        s.execute("channel.new", json!({"fill": "white"})).unwrap();
+                    }
+                    crate::channel_cmds::Target::QuickMask => {
+                        s.execute("select.editInQuickMaskMode", json!({"on": true})).unwrap();
+                    }
+                    _ => panic!("expected a grayscale target"),
+                }
+                let p = json!({"x": 2, "y": 2, "color": "#000000", "antiAlias": false, "tolerance": 0, "target": target, "sampleAllLayers": all});
+                s.edit("target wall", |doc, active| {
+                    let area = doc.bounds();
+                    let (surf, _) = crate::channel_cmds::target_surface(doc, *active, &p).unwrap();
+                    surf.fill_rect(area, &[1.0]);
+                    surf.fill_rect(Rect::new(5, 0, 6, 10), &[0.0]);
+                    Ok(())
+                })
+                .unwrap();
+                s.execute("paint.bucket", p.clone()).unwrap();
+                let d = &s.active().unwrap().doc;
+                let surf = match crate::channel_cmds::target_of(&p) {
+                    crate::channel_cmds::Target::Mask => &d.layer(s.active().unwrap().active_layer.unwrap()).unwrap().mask.as_ref().unwrap().surface,
+                    crate::channel_cmds::Target::Alpha(i) => &d.channels[i].surface,
+                    crate::channel_cmds::Target::QuickMask => &d.quick_mask.as_ref().unwrap().surface,
+                    _ => panic!("expected a grayscale target"),
+                };
+                assert_eq!(surf.sample_channel(2, 5, 0), 0.0, "{target}, all {all}: filled target");
+                assert_eq!(surf.sample_channel(7, 5, 0), 1.0, "{target}, all {all}: stopped at target wall");
+                assert_eq!(px(&s, 2, 5), vec![0.0; 4], "layer pixels remain untouched");
+            }
+        }
+    }
+
     #[test]
     fn bucket_fill_contiguous() {
         let mut s = session();

crates/ui-egui/src/canvas.rs

@@ -2955,7 +2955,7 @@ pub fn tool_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers)
                 Tool::PaintBucket => {
                     let o = app.ui.tool_options.clone();
                     let contents = if o.bucket_fill_pattern { "pattern" } else { "foreground" };
-                    let _ = app.run("paint.bucket", json!({"x": x.floor(), "y": y.floor(), "tolerance": o.tolerance, "contiguous": o.contiguous, "antiAlias": o.anti_alias, "opacity": o.fill_opacity, "contents": contents, "target": paint_target(app)}));
+                    let _ = app.run("paint.bucket", json!({"x": x.floor(), "y": y.floor(), "tolerance": o.tolerance, "contiguous": o.contiguous, "antiAlias": o.anti_alias, "sampleAllLayers": o.sample_all_layers, "opacity": o.fill_opacity, "contents": contents, "target": paint_target(app)}));
                     return;
                 }
                 Tool::PolygonLasso => {

crates/ui-egui/tests/paint_bucket_all_layers.rs

@@ -0,0 +1,119 @@
+//! #1118: Paint Bucket's All Layers checkbox must sample the visible image while painting
+//! only the active layer. Drive the checkbox and canvas through real egui pointer events.
+
+use egui::{Modifiers, PointerButton, Pos2};
+use egui_kittest::kittest::Queryable;
+use photocraft_doc::LayerId;
+use photocraft_geom::Rect;
+use photocraft_ui_egui::PhotocraftApp;
+use photocraft_ui_egui::canvas::ViewXform;
+use photocraft_ui_egui::control::{ControlRequest, Outcome, handle};
+use serde_json::json;
+
+type Harness = egui_kittest::Harness<'static, PhotocraftApp>;
+
+fn harness() -> Option<Harness> {
+    let built = std::panic::catch_unwind(|| {
+        egui_kittest::Harness::builder().with_size(egui::vec2(1100.0, 720.0)).with_pixels_per_point(1.0).with_max_steps(64).wgpu().build_eframe(|cc| {
+            PhotocraftApp::setup_context(&cc.egui_ctx, Default::default());
+            let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
+            app.session.prefs.edit(|p| p.interface.language = "en".into());
+            if let Some(rs) = cc.wgpu_render_state.as_ref() {
+                app.set_wgpu(rs.clone());
+            }
+            app
+        })
+    });
+    match built {
+        Ok(h) => Some(h),
+        Err(_) => {
+            eprintln!("skipping: no GPU adapter");
+            None
+        }
+    }
+}
+
+fn setup(h: &mut Harness) -> LayerId {
+    let background = {
+        let app = h.state_mut();
+        while app.session.active().is_some() {
+            app.run("file.close", json!({"discard": true})).expect("close");
+        }
+        app.run("file.new", json!({"width": 400, "height": 300, "background": "white"})).expect("new");
+        let background = app.session.active().expect("document").active_layer.expect("background");
+        app.run("select.rect", json!({"x": 0, "y": 0, "width": 200, "height": 300})).expect("select left half");
+        app.run("edit.fill", json!({"contents": "black"})).expect("black left half");
+        app.run("select.deselect", json!({})).expect("deselect");
+        app.run("layer.new.layer", json!({"name": "Bucket fill"})).expect("empty active layer");
+        app.run("tools.setColors", json!({"foreground": [1.0, 0.0, 0.0, 1.0]})).expect("red foreground");
+        app.sync_views();
+        app.ui.extras.rulers = false;
+        let o = &mut app.ui.tool_options;
+        o.tolerance = 0.0;
+        o.anti_alias = false;
+        o.contiguous = true;
+        o.sample_all_layers = false;
+        o.bucket_fill_pattern = false;
+        o.fill_opacity = 100.0;
+        background
+    };
+    let ctx = h.ctx.clone();
+    let (req, _rx) = ControlRequest::new("ui.set", json!({"tool": "paintBucket", "zoom": 1.0, "center": [200, 150]}));
+    assert!(matches!(handle(h.state_mut(), &ctx, &req), Outcome::Done(_)));
+    h.run_steps(6);
+    background
+}
+
+fn screen(h: &Harness, x: f32, y: f32) -> Pos2 {
+    let app = h.state();
+    let v = &app.ui.views[0];
+    ViewXform { rect: photocraft_ui_egui::rulers::content_rect(app, app.last_canvas_rect), zoom: v.zoom, center: v.center, flip: app.ui.view.flip_horizontal }
+        .to_screen(x, y)
+}
+
+fn pixels(h: &Harness, layer: LayerId) -> Vec<[f32; 4]> {
+    let st = h.state().session.active().expect("document");
+    let surface = st.doc.layer(layer).expect("layer").surface().expect("pixel layer");
+    let mut pixels = vec![[0.0; 4]; 400 * 300];
+    surface.read_rgba_into(Rect::new(0, 0, 400, 300), &mut pixels);
+    pixels
+}
+
+#[test]
+fn all_layers_checkbox_limits_bucket_fill_to_the_visible_color() {
+    let Some(mut h) = harness() else { return };
+    h.run_steps(4);
+    for all_layers in [false, true] {
+        let background = setup(&mut h);
+        let before = pixels(&h, background);
+        if all_layers {
+            h.get_by_label("All Layers").click();
+            h.run_steps(3);
+        }
+        assert_eq!(h.state().ui.tool_options.sample_all_layers, all_layers, "checkbox changed the option");
+
+        let pos = screen(&h, 300.0, 150.0);
+        h.event(egui::Event::PointerMoved(pos));
+        h.step();
+        for pressed in [true, false] {
+            h.event(egui::Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+            h.step();
+        }
+        h.run_steps(6);
+
+        // Optional real UI evidence, saved before assertions so a failing baseline has a frame.
+        if let Some(dir) = std::env::var_os("PHOTOCRAFT_BUCKET_EVIDENCE_DIR") {
+            std::fs::create_dir_all(&dir).expect("evidence directory");
+            let name = if all_layers { "all-layers-on.png" } else { "all-layers-off.png" };
+            h.render().expect("render UI").save(std::path::Path::new(&dir).join(name)).expect("save UI evidence");
+        }
+
+        assert_eq!(pixels(&h, background), before, "bucket must leave the sampled background unchanged");
+        let active = h.state().session.active().expect("document").active_layer.expect("active layer");
+        for (i, pixel) in pixels(&h, active).into_iter().enumerate() {
+            let x = i % 400;
+            let expected = if all_layers && x < 200 { [0.0; 4] } else { [1.0, 0.0, 0.0, 1.0] };
+            assert_eq!(pixel, expected, "All Layers={all_layers}, active-layer pixel ({x}, {})", i / 400);
+        }
+    }
+}