From e48982e02316a1b933a0f7f530b6166d2e21085f Mon Sep 17 00:00:00 2001 From: DuBearte <130190089+DuBearte@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:49:36 -0400 Subject: [PATCH 1/2] fix(engine): CMYK Split Channels reads the composite's exact inks (#9236) Split Channels read a CMYK composite through RGB, which moved each ink by a few levels. It now uses the ink compositor where the stack allows. Apply Image keeps reading the composite through RGB. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/channel_cmds/spot.rs | 11 +++++++++- crates/engine/src/cmd/channel_cmds/tests.rs | 23 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/channel_cmds/spot.rs b/crates/engine/src/cmd/channel_cmds/spot.rs index 773089faad..3807215b94 100644 --- a/crates/engine/src/cmd/channel_cmds/spot.rs +++ b/crates/engine/src/cmd/channel_cmds/spot.rs @@ -60,8 +60,17 @@ pub(super) fn split(s: &mut Session, p: &Value) -> Result { let idx = s.active_index().ok_or(EngineError::NoDocument)?; let doc = s.documents()[idx].doc.clone(); let area = doc.bounds(); - let (px, n) = native_pixels(&doc, None)?; let mode = edit_mode(&doc); + // A CMYK composite's channels are its inks: mixed on ink values where the stack allows, since + // a trip through RGB moves them by a few levels (#9236). + let inks = if mode == ColorMode::Cmyk { photocraft_compose::cmyk::render_cmyk(&doc, doc.bounds()) } else { None }; + let (px, n) = match inks { + Some(buf) => { + let n = doc.pixel_format().channels(); + (buf.px.iter().flat_map(|p| p.iter().copied().take(n)).collect(), n) + } + None => native_pixels(&doc, None)?, + }; let names = color_names(mode); let short = |name: &str| name.chars().next().map(|c| c.to_ascii_uppercase().to_string()).unwrap_or_default(); let mut docs: Vec = Vec::new(); diff --git a/crates/engine/src/cmd/channel_cmds/tests.rs b/crates/engine/src/cmd/channel_cmds/tests.rs index 079b2e4c86..25a97bd958 100644 --- a/crates/engine/src/cmd/channel_cmds/tests.rs +++ b/crates/engine/src/cmd/channel_cmds/tests.rs @@ -898,6 +898,29 @@ fn split_and_merge_channels() { assert!(s.execute("channel.merge", json!({"mode": "cmyk"})).is_err()); } +/// #9236: CMYK channels split into greyscale shown as the Channels panel shows them (white +/// paper, dark ink), and merge back into the same inks. +#[test] +fn split_and_merge_cmyk_keep_ink_dark() { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 40, "height": 20, "mode": "cmyk", "background": "white"})).unwrap(); + s.execute("select.rect", json!({"x": 0, "y": 0, "width": 20, "height": 20})).unwrap(); + s.execute("edit.fill", json!({"color": "#0080ff"})).unwrap(); + s.execute("select.deselect", json!({})).unwrap(); + let (ink, paper) = (px(&s, 1, 1), px(&s, 30, 1)); + assert!(ink[0] > 0.5 && paper[..4].iter().all(|v| *v < 0.01), "{ink:?} {paper:?}"); + s.execute("channel.split", json!({})).unwrap(); + assert_eq!(s.documents().len(), 4); + for (i, ink) in ink[..4].iter().enumerate() { + let at = |x| s.documents()[i].doc.layers[0].surface().unwrap().pixel(x, 1)[0]; + assert!((at(1) - (1.0 - ink)).abs() < 0.003, "channel {i}: {} for ink {ink}", at(1)); + assert!((at(30) - 1.0).abs() < 0.003, "channel {i}: paper is white, got {}", at(30)); + } + s.execute("channel.merge", json!({"mode": "cmyk", "documents": [0, 1, 2, 3]})).unwrap(); + let back = px(&s, 1, 1); + assert!(back[..4].iter().zip(&ink[..4]).all(|(a, b)| (a - b).abs() < 0.003), "{back:?} vs {ink:?}"); +} + #[test] fn cmyk_split_and_merge_use_tone() { // #5787: Split Channels of a white CMYK image gives white grayscale documents, and merging From ed876f35d6defb5fd998e753c5a9b4e26c23c849 Mon Sep 17 00:00:00 2001 From: DuBearte <130190089+DuBearte@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:25:59 -0500 Subject: [PATCH 2/2] Test Split Channels with rich black, and check each ink pixel's length Rich black (CMYK 100/100/100/0) doesn't survive a trip through RGB, so the test fails without the fix. Split Channels now refuses an ink buffer whose pixels give fewer values than the document has channels. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/channel_cmds/spot.rs | 6 +++++- crates/engine/src/cmd/channel_cmds/tests.rs | 7 +++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/channel_cmds/spot.rs b/crates/engine/src/cmd/channel_cmds/spot.rs index 3807215b94..81648b2d8c 100644 --- a/crates/engine/src/cmd/channel_cmds/spot.rs +++ b/crates/engine/src/cmd/channel_cmds/spot.rs @@ -67,7 +67,11 @@ pub(super) fn split(s: &mut Session, p: &Value) -> Result { let (px, n) = match inks { Some(buf) => { let n = doc.pixel_format().channels(); - (buf.px.iter().flat_map(|p| p.iter().copied().take(n)).collect(), n) + let px: Vec = buf.px.iter().flat_map(|p| p.iter().copied().take(n)).collect(); + if px.len() != buf.px.len() * n { + return Err(EngineError::Other(format!("Split Channels: an ink pixel has fewer than {n} values"))); + } + (px, n) } None => native_pixels(&doc, None)?, }; diff --git a/crates/engine/src/cmd/channel_cmds/tests.rs b/crates/engine/src/cmd/channel_cmds/tests.rs index 25a97bd958..592b34f9a0 100644 --- a/crates/engine/src/cmd/channel_cmds/tests.rs +++ b/crates/engine/src/cmd/channel_cmds/tests.rs @@ -904,11 +904,14 @@ fn split_and_merge_channels() { fn split_and_merge_cmyk_keep_ink_dark() { let mut s = Session::new(); s.execute("file.new", json!({"width": 40, "height": 20, "mode": "cmyk", "background": "white"})).unwrap(); + // Rich black (#8359): its inks don't survive a trip through RGB, so the old path fails here. + let rgb = photocraft_color::convert::cmyk_to_rgb([1.0, 1.0, 1.0, 0.0]).map(|c| c.clamp(0.0, 1.0)); + s.execute("tools.setColors", json!({"foreground": rgb, "foregroundCmyk": [1.0, 1.0, 1.0, 0.0]})).unwrap(); s.execute("select.rect", json!({"x": 0, "y": 0, "width": 20, "height": 20})).unwrap(); - s.execute("edit.fill", json!({"color": "#0080ff"})).unwrap(); + s.execute("edit.fillForeground", json!({})).unwrap(); s.execute("select.deselect", json!({})).unwrap(); let (ink, paper) = (px(&s, 1, 1), px(&s, 30, 1)); - assert!(ink[0] > 0.5 && paper[..4].iter().all(|v| *v < 0.01), "{ink:?} {paper:?}"); + assert!(ink[..3].iter().all(|v| *v > 0.99) && paper[..4].iter().all(|v| *v < 0.01), "{ink:?} {paper:?}"); s.execute("channel.split", json!({})).unwrap(); assert_eq!(s.documents().len(), 4); for (i, ink) in ink[..4].iter().enumerate() {