Merge pull request #12049 from DuBearte/fix/11897-clear-on-background

Brush: Clear on the Background paints as Normal and the Mode reads Normal

crates/engine/src/brush_clear_background_tests.rs

@@ -0,0 +1,71 @@
+//! #11897: the Brush's chosen Clear on the Background paints as Normal. The options bar turns
+//! Clear off at the press there (`clear_off_on_background` in ui-egui), so the engine never sees
+//! it; a `"mode": "clear"` stroke that does reach the Background is a held `~`, which keeps
+//! erasing to the background colour. On a pixel layer Clear still erases, and the Eraser on the
+//! Background still paints the background colour.
+
+use crate::Session;
+use serde_json::{Value, json};
+
+const BLACK: [f32; 4] = [0.0, 0.0, 0.0, 1.0];
+const GREEN: [f32; 4] = [0.0, 1.0, 0.0, 1.0];
+const WHITE: [f32; 4] = [1.0, 1.0, 1.0, 1.0];
+
+fn session() -> Session {
+    let mut s = Session::new();
+    s.execute("file.new", json!({"width": 64, "height": 32, "background": "white"})).unwrap();
+    s.tools.foreground = BLACK;
+    s.tools.background = GREEN;
+    s
+}
+
+fn pixel(s: &mut Session, x: i32, y: i32) -> [f32; 4] {
+    let v = s.execute("document.pixel", json!({"x": x, "y": y})).unwrap();
+    let a: Vec<f32> = v.as_array().unwrap().iter().map(|c| c.as_f64().unwrap() as f32).collect();
+    [a[0], a[1], a[2], a[3]]
+}
+
+fn close(a: [f32; 4], b: [f32; 4]) -> bool {
+    a.iter().zip(&b).all(|(x, y)| (x - y).abs() <= 0.02)
+}
+
+/// A hard stroke across (10..30, 10), as the canvas sends it in Clear mode.
+fn clear_stroke() -> Value {
+    json!({"points": [[10, 10], [30, 10]], "size": 8, "hardness": 1.0, "erase": true, "mode": "clear"})
+}
+
+/// A held `~` on the Background erases to the background colour, as before #11897.
+#[test]
+fn a_clear_stroke_on_the_background_still_paints_the_background_colour() {
+    let mut s = session();
+    s.execute("paint.stroke", clear_stroke()).unwrap();
+    let p = pixel(&mut s, 20, 10);
+    assert!(close(p, GREEN), "painted {p:?}, not the foreground");
+    assert_eq!(s.active().unwrap().history.undo_label(), Some("Brush Tool"));
+}
+
+#[test]
+fn a_normal_stroke_on_the_background_paints_the_foreground() {
+    let mut s = session();
+    s.execute("paint.stroke", json!({"points": [[10, 10], [30, 10]], "size": 8, "hardness": 1.0})).unwrap();
+    let p = pixel(&mut s, 20, 10);
+    assert!(close(p, BLACK), "{p:?}");
+}
+
+#[test]
+fn clear_on_a_pixel_layer_still_erases() {
+    let mut s = session();
+    s.execute("layer.new.layer", json!({})).unwrap();
+    s.execute("edit.fill", json!({"contents": "color", "color": [1.0, 0.0, 0.0, 1.0]})).unwrap();
+    s.execute("paint.stroke", clear_stroke()).unwrap();
+    let p = pixel(&mut s, 20, 10);
+    assert!(close(p, WHITE), "erased down to the white Background: {p:?}");
+}
+
+#[test]
+fn the_eraser_on_the_background_still_paints_the_background_colour() {
+    let mut s = session();
+    s.execute("paint.stroke", json!({"points": [[10, 10], [30, 10]], "size": 8, "hardness": 1.0, "erase": true})).unwrap();
+    let p = pixel(&mut s, 20, 10);
+    assert!(close(p, GREEN), "{p:?}");
+}

crates/engine/src/lib.rs

@@ -357,4 +357,6 @@ mod tests;
 #[cfg(test)]
 mod fx_mode_tests;
 
+#[cfg(test)]
+mod brush_clear_background_tests;
 pub mod command_hints;

crates/ui-egui/src/canvas/paint_mouse/tests.rs

@@ -1023,3 +1023,59 @@ fn a_right_click_moved_and_released_in_one_frame_still_opens_its_menu() {
     assert_eq!(h.state().ui.brush_picker, None);
     assert_eq!(h.state().ui.canvas_tool_menu, None);
 }
+
+/// #11897: Clear chosen on a pixel layer, then the Background activated: a Brush stroke there
+/// paints the foreground, and the Mode reads Normal afterwards.
+#[test]
+fn chosen_clear_paints_normally_on_the_background_and_the_mode_reads_normal() {
+    let mut h = harness(None);
+    h.state_mut().ui.tool_options.brush_clear = true;
+    let bg = h.state().session.active().unwrap().doc.background().unwrap().id;
+    h.state_mut().run("layer.select", json!({"layer": bg.0})).unwrap();
+    h.run_steps(2);
+    let (a, b) = drag(&mut h, PointerButton::Primary);
+    assert!(!h.state().ui.tool_options.brush_clear, "the Mode reads Normal");
+    let last = strokes(&h).last().cloned().unwrap();
+    assert!(last.get("mode").is_none() && !last["erase"].as_bool().unwrap(), "{last}");
+    let app = h.state();
+    let v = app.ui.views[0].clone();
+    let d = ((a + (b - a) * 0.5) - app.last_canvas_rect.center()) / v.zoom;
+    let st = app.session.active().unwrap();
+    let px = st.doc.layer(bg).unwrap().surface().unwrap().rgba((d.x + v.center[0]) as i32, (d.y + v.center[1]) as i32);
+    assert!(px[2] > 0.9 && px[0] < 0.1, "the foreground blue, not the background colour: {px:?}");
+}
+
+/// #11897: a Background with Lock All refuses the stroke, so a chosen Clear stays chosen.
+#[test]
+fn chosen_clear_stays_on_a_locked_background() {
+    let mut h = harness(None);
+    h.state_mut().ui.tool_options.brush_clear = true;
+    let bg = h.state().session.active().unwrap().doc.background().unwrap().id;
+    h.state_mut().run("layer.select", json!({"layer": bg.0})).unwrap();
+    h.state_mut().run("layer.lockLayers", json!({"all": true})).unwrap();
+    h.run_steps(2);
+    drag(&mut h, PointerButton::Primary);
+    assert!(h.state().ui.tool_options.brush_clear, "the Mode still reads Clear");
+}
+
+/// #11897: a held `~` on the Background keeps erasing to the background colour.
+#[test]
+fn held_tilde_on_the_background_paints_the_background_colour() {
+    let mut h = harness(None);
+    h.state_mut().run("tools.setColors", json!({"background": "#00ff00"})).unwrap();
+    let bg = h.state().session.active().unwrap().doc.background().unwrap().id;
+    h.state_mut().run("layer.select", json!({"layer": bg.0})).unwrap();
+    h.run_steps(2);
+    h.event(egui::Event::Key { key: egui::Key::Backtick, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE });
+    h.run_steps(1);
+    let (a, b) = drag(&mut h, PointerButton::Primary);
+    h.event(egui::Event::Key { key: egui::Key::Backtick, physical_key: None, pressed: false, repeat: false, modifiers: Modifiers::NONE });
+    h.run_steps(1);
+    assert_eq!(strokes(&h).last().unwrap()["mode"], json!("clear"));
+    let app = h.state();
+    let v = app.ui.views[0].clone();
+    let d = ((a + (b - a) * 0.5) - app.last_canvas_rect.center()) / v.zoom;
+    let st = app.session.active().unwrap();
+    let px = st.doc.layer(bg).unwrap().surface().unwrap().rgba((d.x + v.center[0]) as i32, (d.y + v.center[1]) as i32);
+    assert!(px[1] > 0.9 && px[0] < 0.1 && px[2] < 0.1, "the background green, not the foreground blue: {px:?}");
+}

crates/ui-egui/src/canvas/tool_input.rs

@@ -377,6 +377,24 @@ fn mixer_press_failed(app: &mut PhotocraftApp, message: String) {
     }
 }
 
+/// The Brush's chosen Clear paints as Normal on the Background, and its Mode then reads Normal
+/// (#11897): at the press of a Brush stroke on the Background's own pixels, Clear (chosen in the
+/// options bar, not a held `~`) is turned off, so the stroke is sent as Normal. A Background whose
+/// pixels are locked refuses the stroke, so Clear stays chosen there.
+fn clear_off_on_background(app: &mut PhotocraftApp, tool: Tool) {
+    let on_background = app.session.active().is_some_and(|st| {
+        st.active_layer.is_some_and(|id| {
+            st.doc.is_background(id) && {
+                let locks = st.doc.effective_locks(id);
+                !locks.pixels && !locks.all
+            }
+        })
+    });
+    if tool == Tool::Brush && app.ui.tool_options.brush_clear && !app.held_erase && on_background && paint_target(app) == serde_json::json!("pixels") {
+        app.ui.tool_options.brush_clear = false;
+    }
+}
+
 /// Tool state machine. Shared by mouse input and automation.
 pub fn tool_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers) {
     if consumes_cancelled_vector_event(app, ev) {
@@ -540,6 +558,7 @@ pub fn tool_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers)
                 _ => {}
             }
             crate::paint_mouse::sync_tool_brush(app);
+            clear_off_on_background(app, tool);
             // Clear mode (`~` held, or chosen in the options bar) erases with the tool's own brush.
             let clear = crate::paint_mouse::clears(app, tool);
             let erase = std::mem::take(&mut app.secondary_erase) || tool == Tool::Eraser || clear;