diff --git a/crates/engine/src/cmd/eraser_cmds.rs b/crates/engine/src/cmd/eraser_cmds.rs index 8e2b16b41e..29833f949a 100644 --- a/crates/engine/src/cmd/eraser_cmds.rs +++ b/crates/engine/src/cmd/eraser_cmds.rs @@ -142,6 +142,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")?, @@ -149,6 +151,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..8e783a02c9 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,18 @@ 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), at the precision + // the document stores them. + 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).map(|&v| at_sample_precision(fmt.sample, v)).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 +386,62 @@ 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 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.