From 44a3c25a9edca46d1c918486bcf2bd55059b8cea Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Sun, 8 Nov 2026 18:06:50 +0100 Subject: [PATCH 1/3] Background Eraser: a CMYK document compares against the picked inks (#12352) Protect Foreground Color and Sampling Background Swatch converted the foreground and background from their sRGB stand-ins, so a colour picked as CMYK inks (100/0/0/0) came back as other inks and pixels painted with exactly those inks were erased or missed. BgEraseSettings now carries the picked inks, as BrushSettings::color_cmyk does, and a CMYK target compares against them while the colour is still that sRGB colour. RGB, Gray and Lab documents are unchanged. --- crates/engine/src/cmd/eraser_cmds.rs | 4 ++ crates/engine/src/cmd/eraser_cmds/tests.rs | 49 ++++++++++++++++++ crates/paint/src/bg_erase.rs | 60 ++++++++++++++++++++-- 3 files changed, 109 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/eraser_cmds.rs b/crates/engine/src/cmd/eraser_cmds.rs index b2a36fdd18..daafe4279d 100644 --- a/crates/engine/src/cmd/eraser_cmds.rs +++ b/crates/engine/src/cmd/eraser_cmds.rs @@ -138,6 +138,8 @@ fn background_eraser_setup(s: &Session, p: &Value) -> Result<(Stroke, BgEraseSet return Err(bad(CMD, "`points` must be finite canvas coordinates")); } let brush = crate::brush_cmds::resolve_brush(s, p, CMD)?; + // The inks a colour was picked as, for a CMYK document (#12352). + let picked = |c: [f32; 4]| s.tools.picked_cmyk([c[0], c[1], c[2]]).map(|k| ([c[0], c[1], c[2]], k)); let bs = BgEraseSettings { sampling: enum_param(p, CMD, "sampling", Sampling::Continuous, "continuous|once|backgroundSwatch")?, limits: enum_param(p, CMD, "limits", Limits::Contiguous, "discontiguous|contiguous|findEdges")?, @@ -145,6 +147,8 @@ fn background_eraser_setup(s: &Session, p: &Value) -> Result<(Stroke, BgEraseSet protect_foreground: flag(p, "protectForegroundColor", false), foreground: s.tools.foreground, background: s.tools.background, + foreground_cmyk: picked(s.tools.foreground), + background_cmyk: picked(s.tools.background), }; let id = active_pixel_layer(s, p, CMD)?; let d = s.active().ok_or(EngineError::NoDocument)?; diff --git a/crates/engine/src/cmd/eraser_cmds/tests.rs b/crates/engine/src/cmd/eraser_cmds/tests.rs index fd965b62e8..50d11699d9 100644 --- a/crates/engine/src/cmd/eraser_cmds/tests.rs +++ b/crates/engine/src/cmd/eraser_cmds/tests.rs @@ -215,6 +215,55 @@ fn background_eraser_options() { assert_eq!(alpha(&s, 6, 20), 1.0); } +/// #12352: in a CMYK document, Protect Foreground Color and Sampling Background Swatch compare +/// against the inks the colour was picked as, not what its sRGB stand-in converts back to. +#[test] +fn background_eraser_compares_against_picked_cmyk_inks() { + const STAND_IN: &str = "#00aeef"; + const CYAN: [f32; 4] = [1.0, 0.0, 0.0, 0.0]; + let cyan_square = |key: &str| { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 64, "height": 48, "mode": "cmyk", "depth": 8})).unwrap(); + s.execute("tools.setColors", json!({key: STAND_IN, format!("{key}Cmyk"): CYAN})).unwrap(); + s.execute("select.rect", json!({"x": 10, "y": 10, "width": 20, "height": 20, "antiAlias": false})).unwrap(); + s.execute("edit.fill", json!({"color": STAND_IN})).unwrap(); + s.execute("select.deselect", json!({})).unwrap(); + assert_eq!(surface(&s).pixel(20, 20)[..4], CYAN, "the square holds the picked inks"); + s + }; + let stroke = |extra: Value| { + let mut p = json!({"points": [[4, 20], [40, 20]], "size": 16, "hardness": 1, "limits": "discontiguous", "tolerance": 1}); + p.as_object_mut().unwrap().extend(extra.as_object().unwrap().clone()); + p + }; + let mut s = cyan_square("foreground"); + s.execute("paint.backgroundEraser", stroke(json!({"sampling": "continuous", "protectForegroundColor": true}))).unwrap(); + assert_eq!(alpha(&s, 20, 20), 1.0, "the foreground's inks are protected"); + assert_eq!(alpha(&s, 6, 20), 0.0, "white erased"); + let mut s = cyan_square("background"); + s.execute("paint.backgroundEraser", stroke(json!({"sampling": "backgroundSwatch"}))).unwrap(); + assert_eq!(alpha(&s, 20, 20), 0.0, "the background's inks are erased fully"); + assert_eq!(alpha(&s, 6, 20), 1.0, "white kept"); + // Other modes ignore the picked inks: the stand-in is compared as it always was. + for mode in ["rgb", "gray", "lab"] { + let run = |cmyk: bool| { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 64, "height": 48, "mode": mode, "depth": 8})).unwrap(); + s.execute("select.rect", json!({"x": 10, "y": 10, "width": 20, "height": 20, "antiAlias": false})).unwrap(); + s.execute("edit.fill", json!({"color": STAND_IN})).unwrap(); + s.execute("select.deselect", json!({})).unwrap(); + let picks = if cmyk { json!({"foregroundCmyk": CYAN, "backgroundCmyk": CYAN}) } else { json!({}) }; + let mut colors = json!({"foreground": STAND_IN, "background": STAND_IN}); + colors.as_object_mut().unwrap().extend(picks.as_object().unwrap().clone()); + s.execute("tools.setColors", colors).unwrap(); + s.execute("paint.backgroundEraser", stroke(json!({"sampling": "continuous", "protectForegroundColor": true}))).unwrap(); + s.execute("paint.backgroundEraser", stroke(json!({"points": [[4, 30], [40, 30]], "sampling": "backgroundSwatch"}))).unwrap(); + surface(&s).clone() + }; + assert!(run(true) == run(false), "{mode}: a CMYK pick changes nothing"); + } +} + #[test] fn background_eraser_fails_gracefully() { let mut s = session("rgb", 8); diff --git a/crates/paint/src/bg_erase.rs b/crates/paint/src/bg_erase.rs index facf783e83..8ee09a7481 100644 --- a/crates/paint/src/bg_erase.rs +++ b/crates/paint/src/bg_erase.rs @@ -4,7 +4,7 @@ //! Colours are compared in the surface's own colour channels (RGB, CMYK, Gray, Lab… at any depth), //! so nothing is converted through RGB; only alpha changes. -use photocraft_color::{SampleType, read_sample, write_sample}; +use photocraft_color::{ColorMode, SampleType, read_sample, write_sample}; use photocraft_geom::Rect; use photocraft_raster::{Surface, from_rgba}; use serde::{Deserialize, Serialize}; @@ -33,6 +33,15 @@ pub struct BgEraseSettings { pub foreground: [f32; 4], /// Straight RGBA background swatch (for [`Sampling::BackgroundSwatch`]). pub background: [f32; 4], + /// The inks `foreground` was picked as (its sRGB colour, the CMYK values), for a CMYK target: + /// Protect Foreground Color compares against exactly these, since an sRGB round trip does not + /// give them back (#12352). Only valid while `foreground` is that sRGB colour. + #[serde(skip)] + pub foreground_cmyk: Option<([f32; 3], [f32; 4])>, + /// The inks `background` was picked as, like [`Self::foreground_cmyk`] (for + /// [`Sampling::BackgroundSwatch`]). + #[serde(skip)] + pub background_cmyk: Option<([f32; 3], [f32; 4])>, } impl Default for BgEraseSettings { @@ -44,6 +53,8 @@ impl Default for BgEraseSettings { protect_foreground: false, foreground: [0.0, 0.0, 0.0, 1.0], background: [1.0; 4], + foreground_cmyk: None, + background_cmyk: None, } } } @@ -121,9 +132,15 @@ pub fn background_eraser_effect(pre: &Surface, bs: &BgEraseSettings) -> Option Vec { from_rgba(&fmt, [c[0], c[1], c[2], 1.0])[..nc].to_vec() }; - let swatch = native(bs.background); - let fg: Vec = native(bs.foreground).into_iter().map(|v| at_sample_precision(fmt.sample, v)).collect(); + // A colour picked as CMYK is compared by its inks in a CMYK target (#12352). + let native = |c: [f32; 4], picked: Option<([f32; 3], [f32; 4])>| -> Vec { + match picked { + Some((rgb, inks)) if fmt.mode == ColorMode::Cmyk && rgb == [c[0], c[1], c[2]] => inks.iter().take(nc).copied().collect(), + _ => from_rgba(&fmt, [c[0], c[1], c[2], 1.0])[..nc].to_vec(), + } + }; + let swatch = native(bs.background, bs.background_cmyk); + let fg: Vec = native(bs.foreground, bs.foreground_cmyk).into_iter().map(|v| at_sample_precision(fmt.sample, v)).collect(); let (sampling, limits, protect_foreground) = (bs.sampling, bs.limits, bs.protect_foreground); let float = fmt.sample == SampleType::F32; let mut once: Option>> = None; @@ -366,6 +383,41 @@ mod tests { assert_eq!(t, s); } + #[test] + fn cmyk_targets_compare_the_picked_inks() { + // #12352: cyan inks whose sRGB stand-in converts back to other inks. + const STAND_IN: [f32; 4] = [0.0, 0.68, 0.94, 1.0]; + let picked = Some(([STAND_IN[0], STAND_IN[1], STAND_IN[2]], [1.0, 0.0, 0.0, 0.0])); + let fmt = PixelFormat::new(ColorMode::Cmyk, SampleType::U8, true); + let mut s = Surface::new(fmt); + s.fill_rect(Rect::new(0, 0, 80, 40), &from_rgba(&fmt, [1.0; 4])); + s.fill_rect(Rect::new(30, 0, 50, 40), &[1.0, 0.0, 0.0, 0.0, 1.0]); + let st = stroke(&[(10.0, 20.0), (70.0, 20.0)]); + let base = BgEraseSettings { limits: Limits::Discontiguous, tolerance: 0.01, ..Default::default() }; + let protect = BgEraseSettings { protect_foreground: true, foreground: STAND_IN, foreground_cmyk: picked, ..base.clone() }; + let mut t = s.clone(); + apply_background_eraser(&mut t, &st, &protect, None); + assert_eq!(alpha(&t, 40, 20), 1.0, "the picked foreground inks are protected"); + assert_eq!(alpha(&t, 10, 20), 0.0, "white erased"); + let swatch = BgEraseSettings { sampling: Sampling::BackgroundSwatch, background: STAND_IN, background_cmyk: picked, ..base.clone() }; + let mut t = s.clone(); + apply_background_eraser(&mut t, &st, &swatch, None); + assert_eq!(alpha(&t, 40, 20), 0.0, "the picked background inks are erased fully"); + assert_eq!(alpha(&t, 10, 20), 1.0, "white kept"); + // Inks picked for another colour, and non-CMYK targets, use the stand-in as before. + let stale = BgEraseSettings { background_cmyk: Some(([0.5; 3], [1.0, 0.0, 0.0, 0.0])), ..swatch.clone() }; + let mut t = s.clone(); + apply_background_eraser(&mut t, &st, &stale, None); + assert_eq!(alpha(&t, 40, 20), 1.0, "stale inks are ignored"); + let rgb = PixelFormat::RGBA8; + let mut r = Surface::new(rgb); + r.fill_rect(Rect::new(0, 0, 80, 40), &from_rgba(&rgb, STAND_IN)); + let (mut with, mut without) = (r.clone(), r.clone()); + apply_background_eraser(&mut with, &st, &swatch, None); + apply_background_eraser(&mut without, &st, &BgEraseSettings { background_cmyk: None, ..swatch }, None); + assert_eq!(with, without, "an RGB target ignores the inks"); + } + #[test] fn one_pass_clears_the_whole_stroke_when_dabs_overlap_erased_pixels() { // #1280: later dabs land their hotspot on pixels earlier dabs already erased. From 658609f82c3fc5037acd21ad5ae1f2c42c4e9f97 Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Mon, 9 Nov 2026 16:25:28 +0100 Subject: [PATCH 2/3] Background Eraser: picked swatch inks compare at the stored precision (#12352) Typed inks between two 8-bit levels (30% cyan is stored as 77/255) missed the pixels filled with them at a low Tolerance and left partial alpha. Round them as the foreground already is. --- crates/paint/src/bg_erase.rs | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/paint/src/bg_erase.rs b/crates/paint/src/bg_erase.rs index 8ee09a7481..8e783a02c9 100644 --- a/crates/paint/src/bg_erase.rs +++ b/crates/paint/src/bg_erase.rs @@ -132,10 +132,13 @@ pub fn background_eraser_effect(pre: &Surface, bs: &BgEraseSettings) -> Option| -> Vec { match picked { - Some((rgb, inks)) if fmt.mode == ColorMode::Cmyk && rgb == [c[0], c[1], c[2]] => inks.iter().take(nc).copied().collect(), + Some((rgb, inks)) if fmt.mode == ColorMode::Cmyk && rgb == [c[0], c[1], c[2]] => { + inks.iter().take(nc).map(|&v| at_sample_precision(fmt.sample, v)).collect() + } _ => from_rgba(&fmt, [c[0], c[1], c[2], 1.0])[..nc].to_vec(), } }; @@ -418,6 +421,27 @@ mod tests { assert_eq!(with, without, "an RGB target ignores the inks"); } + #[test] + fn picked_swatch_inks_match_their_stored_levels() { + // #12352: typed inks between two 8-bit levels (30% is stored as 77/255) still match the + // pixels filled with them, as the foreground already does. + const INKS: [f32; 4] = [0.3, 0.0, 0.0, 0.0]; + const STAND_IN: [f32; 4] = [0.7, 1.0, 1.0, 1.0]; + let fmt = PixelFormat::new(ColorMode::Cmyk, SampleType::U8, true); + let mut s = Surface::new(fmt); + s.fill_rect(Rect::new(0, 0, 80, 40), &[INKS[0], INKS[1], INKS[2], INKS[3], 1.0]); + let bs = BgEraseSettings { + sampling: Sampling::BackgroundSwatch, + limits: Limits::Discontiguous, + tolerance: 0.01, + background: STAND_IN, + background_cmyk: Some(([STAND_IN[0], STAND_IN[1], STAND_IN[2]], INKS)), + ..Default::default() + }; + apply_background_eraser(&mut s, &stroke(&[(10.0, 20.0), (70.0, 20.0)]), &bs, None); + assert_eq!(alpha(&s, 40, 20), 0.0, "the stored inks are erased fully"); + } + #[test] fn one_pass_clears_the_whole_stroke_when_dabs_overlap_erased_pixels() { // #1280: later dabs land their hotspot on pixels earlier dabs already erased. From c92e934e8ab07938c3dbe7e3696bcb2f6c8ea259 Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Mon, 9 Nov 2026 17:41:07 +0100 Subject: [PATCH 3/3] Re-run CI after a flaky Liquify test (#12352)