From be428bf90ef0bdfc96dc5222a81241990be090dd Mon Sep 17 00:00:00 2001 From: DuBearte <130190089+DuBearte@users.noreply.github.com> Date: Sat, 7 Nov 2026 09:59:39 -0500 Subject: [PATCH 1/2] Brush: Clear on the Background paints as Normal and the Mode reads Normal Fixes #11897. Co-Authored-By: Claude Opus 5.5 --- .../src/brush_clear_background_tests.rs | 59 +++++++++++++++++++ crates/engine/src/cmd/brush_cmds.rs | 18 +++++- crates/engine/src/lib.rs | 2 + .../ui-egui/src/canvas/paint_mouse/tests.rs | 21 +++++++ crates/ui-egui/src/canvas/tool_input.rs | 10 ++++ 5 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 crates/engine/src/brush_clear_background_tests.rs diff --git a/crates/engine/src/brush_clear_background_tests.rs b/crates/engine/src/brush_clear_background_tests.rs new file mode 100644 index 0000000000..e786af9dd7 --- /dev/null +++ b/crates/engine/src/brush_clear_background_tests.rs @@ -0,0 +1,59 @@ +//! #11897: the Brush's Clear mode on the Background paints as Normal with the foreground colour, +//! not 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 = 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 with Clear chosen. +fn clear_stroke() -> Value { + json!({"points": [[10, 10], [30, 10]], "size": 8, "hardness": 1.0, "erase": true, "mode": "clear"}) +} + +#[test] +fn clear_on_the_background_paints_the_foreground() { + let mut s = session(); + s.execute("paint.stroke", clear_stroke()).unwrap(); + let p = pixel(&mut s, 20, 10); + assert!(close(p, BLACK), "painted {p:?}, not the background colour"); +} + +#[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:?}"); +} diff --git a/crates/engine/src/cmd/brush_cmds.rs b/crates/engine/src/cmd/brush_cmds.rs index e09ff2098c..ecdeeab4da 100644 --- a/crates/engine/src/cmd/brush_cmds.rs +++ b/crates/engine/src/cmd/brush_cmds.rs @@ -424,10 +424,24 @@ fn with_blend_mode(mut b: BrushSettings, p: &Value) -> BrushSettings { b } +/// The Brush's Clear on the Background paints as Normal with its own colour, as in Photoshop +/// (#11897): the Background has no transparency to clear. Masks, channels, other layers (even +/// with locked transparency) and the Pencil keep Clear. +fn clear_on_background(s: &Session, p: &Value, mut brush: BrushSettings) -> Result { + if !clear_mode(p) || is_mask_target(p) || crate::channel_cmds::is_channel_target(p) || crate::color_channels::target(s, p)?.is_some() { + return Ok(brush); + } + let id = layer_id(s, p)?; + if s.active().is_some_and(|st| st.doc.is_background(id)) { + brush.erase = false; + } + Ok(brush) +} + /// `paint.stroke`: the Brush (and Eraser) tool. pub fn paint_stroke(s: &mut Session, p: &Value) -> Result { let pts = parse_points(p, "paint.stroke")?; - let brush = with_blend_mode(resolve_brush(s, p, "paint.stroke")?, p); + let brush = clear_on_background(s, p, with_blend_mode(resolve_brush(s, p, "paint.stroke")?, p))?; let label = if brush.erase && !clear_mode(p) { "Eraser" } else { "Brush Tool" }; if flag(p, "fill", false) { return fill_region(s, p, label, brush, pts); @@ -605,7 +619,7 @@ impl LiveStroke { "paint.pencil" => true, other => return Err(bad(other, "live strokes are `paint.stroke` or `paint.pencil`")), }; - let brush = if pencil { pencil_brush(s, p)? } else { with_blend_mode(resolve_brush(s, p, cmd)?, p) }; + let brush = if pencil { pencil_brush(s, p)? } else { clear_on_background(s, p, with_blend_mode(resolve_brush(s, p, cmd)?, p))? }; let (seed, fg) = (brush.seed, brush.color); let (layer, mut brush, zoom) = stroke_target(s, p, brush)?; let st = s.active().ok_or(EngineError::NoDocument)?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 8f28147217..f9273cebad 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -355,4 +355,6 @@ mod tests; #[cfg(test)] mod fx_mode_tests; +#[cfg(test)] +mod brush_clear_background_tests; pub mod command_hints; diff --git a/crates/ui-egui/src/canvas/paint_mouse/tests.rs b/crates/ui-egui/src/canvas/paint_mouse/tests.rs index c31e137e82..cdc95c6158 100644 --- a/crates/ui-egui/src/canvas/paint_mouse/tests.rs +++ b/crates/ui-egui/src/canvas/paint_mouse/tests.rs @@ -1023,3 +1023,24 @@ 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:?}"); +} diff --git a/crates/ui-egui/src/canvas/tool_input.rs b/crates/ui-egui/src/canvas/tool_input.rs index f0a6b57506..949b84591f 100644 --- a/crates/ui-egui/src/canvas/tool_input.rs +++ b/crates/ui-egui/src/canvas/tool_input.rs @@ -360,6 +360,15 @@ fn handle_eyedropper_event(app: &mut PhotocraftApp, tool: Tool, ev: ToolEvent, m } /// Tool state machine. Shared by mouse input and automation. +/// The Brush's chosen Clear paints as Normal on the Background, and its Mode then reads Normal +/// (#11897). A held `~` keeps today's behaviour. +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))); + 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; + } +} + pub fn tool_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers) { if consumes_cancelled_vector_event(app, ev) { return; @@ -524,6 +533,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; From b5501b9bd1e63204698d69a430990de5d36d1991 Mon Sep 17 00:00:00 2001 From: David Mann Date: Sun, 8 Nov 2026 18:20:55 +0100 Subject: [PATCH 2/2] PR #12049: keep a held ~ erasing on the Background, keep Clear on a locked one The engine turned every "mode": "clear" stroke on the Background into Normal, so a held ~ painted the foreground there instead of erasing to the background colour as on main. The options bar already turns a chosen Clear off at the press, so the engine conversion was not needed: drop it and keep the one check in clear_off_on_background. A Background with locked pixels refuses the stroke, so a chosen Clear now stays chosen there. The "Tool state machine" doc line is back on tool_event. Co-Authored-By: DuBearte Co-Authored-By: Claude Opus 5.5 --- .../src/brush_clear_background_tests.rs | 22 +++++++++--- crates/engine/src/cmd/brush_cmds.rs | 18 ++-------- .../ui-egui/src/canvas/paint_mouse/tests.rs | 35 +++++++++++++++++++ crates/ui-egui/src/canvas/tool_input.rs | 15 ++++++-- 4 files changed, 66 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/brush_clear_background_tests.rs b/crates/engine/src/brush_clear_background_tests.rs index e786af9dd7..e295e68c0b 100644 --- a/crates/engine/src/brush_clear_background_tests.rs +++ b/crates/engine/src/brush_clear_background_tests.rs @@ -1,5 +1,7 @@ -//! #11897: the Brush's Clear mode on the Background paints as Normal with the foreground colour, -//! not the background colour; on a pixel layer Clear still erases, and the Eraser on the +//! #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; @@ -27,17 +29,27 @@ 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 with Clear chosen. +/// 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 clear_on_the_background_paints_the_foreground() { +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, BLACK), "painted {p:?}, not the background colour"); + 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] diff --git a/crates/engine/src/cmd/brush_cmds.rs b/crates/engine/src/cmd/brush_cmds.rs index 73fded269f..87a4bcdd41 100644 --- a/crates/engine/src/cmd/brush_cmds.rs +++ b/crates/engine/src/cmd/brush_cmds.rs @@ -424,24 +424,10 @@ fn with_blend_mode(mut b: BrushSettings, p: &Value) -> BrushSettings { b } -/// The Brush's Clear on the Background paints as Normal with its own colour, as in Photoshop -/// (#11897): the Background has no transparency to clear. Masks, channels, other layers (even -/// with locked transparency) and the Pencil keep Clear. -fn clear_on_background(s: &Session, p: &Value, mut brush: BrushSettings) -> Result { - if !clear_mode(p) || is_mask_target(p) || crate::channel_cmds::is_channel_target(p) || crate::color_channels::target(s, p)?.is_some() { - return Ok(brush); - } - let id = layer_id(s, p)?; - if s.active().is_some_and(|st| st.doc.is_background(id)) { - brush.erase = false; - } - Ok(brush) -} - /// `paint.stroke`: the Brush (and Eraser) tool. pub fn paint_stroke(s: &mut Session, p: &Value) -> Result { let pts = parse_points(p, "paint.stroke")?; - let brush = clear_on_background(s, p, with_blend_mode(resolve_brush(s, p, "paint.stroke")?, p))?; + let brush = with_blend_mode(resolve_brush(s, p, "paint.stroke")?, p); let label = if brush.erase && !clear_mode(p) { "Eraser" } else { "Brush Tool" }; if flag(p, "fill", false) { return fill_region(s, p, label, brush, pts); @@ -619,7 +605,7 @@ impl LiveStroke { "paint.pencil" => true, other => return Err(bad(other, "live strokes are `paint.stroke` or `paint.pencil`")), }; - let brush = if pencil { pencil_brush(s, p)? } else { clear_on_background(s, p, with_blend_mode(resolve_brush(s, p, cmd)?, p))? }; + let brush = if pencil { pencil_brush(s, p)? } else { with_blend_mode(resolve_brush(s, p, cmd)?, p) }; let (seed, fg) = (brush.seed, brush.color); let (layer, mut brush, zoom) = stroke_target(s, p, brush)?; let st = s.active().ok_or(EngineError::NoDocument)?; diff --git a/crates/ui-egui/src/canvas/paint_mouse/tests.rs b/crates/ui-egui/src/canvas/paint_mouse/tests.rs index cdc95c6158..d1092867fe 100644 --- a/crates/ui-egui/src/canvas/paint_mouse/tests.rs +++ b/crates/ui-egui/src/canvas/paint_mouse/tests.rs @@ -1044,3 +1044,38 @@ fn chosen_clear_paints_normally_on_the_background_and_the_mode_reads_normal() { 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:?}"); +} diff --git a/crates/ui-egui/src/canvas/tool_input.rs b/crates/ui-egui/src/canvas/tool_input.rs index 41e36bcd43..0f8a04f770 100644 --- a/crates/ui-egui/src/canvas/tool_input.rs +++ b/crates/ui-egui/src/canvas/tool_input.rs @@ -377,16 +377,25 @@ fn mixer_press_failed(app: &mut PhotocraftApp, message: String) { } } -/// Tool state machine. Shared by mouse input and automation. /// The Brush's chosen Clear paints as Normal on the Background, and its Mode then reads Normal -/// (#11897). A held `~` keeps today's behaviour. +/// (#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 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) { return;