From 184cc1d9128b2ea338611a9884fb9b485fd8b0fa Mon Sep 17 00:00:00 2001 From: Raghav Krishna Date: Tue, 10 Nov 2026 17:34:37 +0530 Subject: [PATCH] fix(engine): Object Selection finds objects on a transparent layer (#12852) Object Finder's click sampled the layer composited over 50 % grey, so a fully transparent pixel read as mid grey. A mid-grey object then sat within the growth's colour distance of the empty area, the region reached the canvas edge, and object_in rejected it as background. Red and blue are far from 0.5 grey, which is why only they were found. Sampler gains rgba_scaled, which keeps alpha at working resolution. object_in reads through it once, never grows into a non-opaque working pixel, and gives transparency hard background in the trimap; a click on a transparent pixel finds nothing. rgb_scaled is untouched, so Quick Selection, Select Subject, sky, focus and the rectangle mode keep their results. Tests: the issue's 800x600 document and the Grayscale one in a new test file; both fail without the fix. --- crates/algo/src/segment.rs | 60 ++++++++ crates/algo/src/segment/grabcut.rs | 29 +++- crates/engine/src/cmd/smartselect_cmds.rs | 2 + .../object_transparent_layer_tests.rs | 143 ++++++++++++++++++ 4 files changed, 227 insertions(+), 7 deletions(-) create mode 100644 crates/engine/src/cmd/smartselect_cmds/object_transparent_layer_tests.rs diff --git a/crates/algo/src/segment.rs b/crates/algo/src/segment.rs index 958d3d0f6a..17d7f24b02 100644 --- a/crates/algo/src/segment.rs +++ b/crates/algo/src/segment.rs @@ -147,6 +147,66 @@ pub trait Sampler: Sync { } RgbImage { w: ow, h: oh, px } } + + /// Straight RGBA (`0..=1`) over `r`, box-downsampled by `step` like [`Self::rgb_scaled`], read + /// in the same strips. Unlike [`Self::rgb`], alpha survives, so a transparent working pixel + /// stays transparent instead of becoming mid grey: Object Finder needs that to stop at the + /// edge of a transparent layer (#12852). + fn rgba_scaled(&self, r: Rect, step: usize) -> Vec<[f32; 4]> { + if step <= 1 { + return self.rgba(r); + } + let (w, h) = (r.width() as usize, r.height() as usize); + let (ow, oh) = (w.div_ceil(step), h.div_ceil(step)); + let rows_per = (64usize.div_ceil(step)).max(1); + let strips: Vec = (0..oh.div_ceil(rows_per)).collect(); + let run = |si: &usize| -> Vec<[f32; 4]> { + let oy0 = si * rows_per; + let oy1 = (oy0 + rows_per).min(oh); + let y0 = r.y0 + (oy0 * step) as i32; + let y1 = (r.y0 + (oy1 * step) as i32).min(r.y1); + let strip = self.rgba(Rect::new(r.x0, y0, r.x1, y1)); + downsample_rgba(&strip, w, (y1 - y0) as usize, step) + }; + #[cfg(not(target_arch = "wasm32"))] + let parts: Vec> = { + use rayon::prelude::*; + strips.par_iter().map(run).collect() + }; + #[cfg(target_arch = "wasm32")] + let parts: Vec> = strips.iter().map(run).collect(); + let mut px = Vec::with_capacity(ow * oh); + for p in parts { + px.extend(p); + } + px + } +} + +/// Box-average straight RGBA by an integer `step` (`ceil(w/step) × ceil(h/step)`). +fn downsample_rgba(px: &[[f32; 4]], w: usize, h: usize, step: usize) -> Vec<[f32; 4]> { + let (ow, oh) = (w.div_ceil(step), h.div_ceil(step)); + let mut out = vec![[0.0f32; 4]; ow * oh]; + for oy in 0..oh { + for ox in 0..ow { + let mut acc = [0.0f32; 4]; + let mut n = 0.0f32; + for y in oy * step..((oy + 1) * step).min(h) { + for x in ox * step..((ox + 1) * step).min(w) { + if let Some(p) = px.get(y * w + x) { + for c in 0..4 { + acc[c] += p[c]; + } + n += 1.0; + } + } + } + if let Some(o) = out.get_mut(oy * ow + ox) { + *o = acc.map(|v| v / n.max(1.0)); + } + } + } + out } /// Samples a raster surface (any format; converted to straight RGBA). diff --git a/crates/algo/src/segment/grabcut.rs b/crates/algo/src/segment/grabcut.rs index 249763ad49..4bebd9e561 100644 --- a/crates/algo/src/segment/grabcut.rs +++ b/crates/algo/src/segment/grabcut.rs @@ -182,16 +182,23 @@ pub fn object_at(sampler: &dyn Sampler, canvas: Rect, x: f32, y: f32, work_px: u /// One [`object_at`] pass over `canvas` (the document or a window of it) at working step `step`; /// `(px, py)` lies inside `canvas`. fn object_in(sampler: &dyn Sampler, canvas: Rect, px: i32, py: i32, step: usize) -> Option { - let img = sampler.rgb_scaled(canvas, step); - let (w, h) = (img.w, img.h); - if w < 2 || h < 2 { + // Alpha is kept, so a transparent pixel is a boundary rather than the mid grey `from_rgba` + // would composite it to: a mid-grey object on a transparent layer stays an object (#12852). + let rgba = sampler.rgba_scaled(canvas, step); + let (w, h) = ((canvas.width() as usize).div_ceil(step), (canvas.height() as usize).div_ceil(step)); + if w < 2 || h < 2 || rgba.len() < w * h { return None; } + let opaque: Vec = rgba.iter().take(w * h).map(|p| p[3] >= 0.5).collect(); + let img = RgbImage::from_rgba(&rgba, w, h); // Lightly smoothed, so noise and texture don't stop the growth; edges survive. let lab: Vec<[f32; 3]> = box3(&box3(&img.px, w, h), w, h).iter().map(|c| super::slic::rgb_to_lab(*c)).collect(); let sx = (((px - canvas.x0) as usize) / step).min(w - 1); let sy = (((py - canvas.y0) as usize) / step).min(h - 1); - let grown = grow_similar(&lab, w, h, sx, sy); + if !opaque[sy * w + sx] { + return None; + } + let grown = grow_similar(&lab, w, h, sx, sy, &opaque); let count = grown.iter().filter(|g| **g).count(); let boundary = grown.iter().enumerate().filter(|(i, on)| **on && (i % w == 0 || i % w + 1 == w || i / w == 0 || i / w + 1 == h)).count(); // shortcut: cropped objects covering most of the frame need a semantic segmenter (#2788). @@ -215,6 +222,10 @@ fn object_in(sampler: &dyn Sampler, canvas: Rect, px: i32, py: i32, step: usize) let inside = xx >= bx0 && xx < bx1 && yy >= by0 && yy < by1; trimap[i] = if grown[i] { PR_FG + } else if !opaque[i] { + // Transparency is background, whatever it was composited to. + any_bg = true; + BG } else if inside { PR_BG } else { @@ -250,13 +261,17 @@ fn object_in(sampler: &dyn Sampler, canvas: Rect, px: i32, py: i32, step: usize) /// Region growing (4-connected) from `(sx, sy)` over pixels whose CIELAB colour is within a /// distance of the seed area's running mean and whose step from the neighbour it was reached -/// from is small, so it stops at edges. -fn grow_similar(lab: &[[f32; 3]], w: usize, h: usize, sx: usize, sy: usize) -> Vec { +/// from is small, so it stops at edges. A pixel that is not `opaque` is never entered, so +/// transparency stops the growth too. +fn grow_similar(lab: &[[f32; 3]], w: usize, h: usize, sx: usize, sy: usize, opaque: &[bool]) -> Vec { const MEAN_DIST: f32 = 32.0; const EDGE_STEP: f32 = 10.0; let d = |a: [f32; 3], b: [f32; 3]| ((a[0] - b[0]).powi(2) + (a[1] - b[1]).powi(2) + (a[2] - b[2]).powi(2)).sqrt(); let mut out = vec![false; w * h]; let Some(&seed) = lab.get(sy * w + sx) else { return out }; + if !opaque.get(sy * w + sx).copied().unwrap_or(false) { + return out; + } let (mut sum, mut n) = ([seed[0] as f64, seed[1] as f64, seed[2] as f64], 1.0f64); let mut stack = vec![(sx, sy)]; out[sy * w + sx] = true; @@ -269,7 +284,7 @@ fn grow_similar(lab: &[[f32; 3]], w: usize, h: usize, sx: usize, sy: usize) -> V continue; } let i = ny * w + nx; - if out[i] { + if out[i] || !opaque.get(i).copied().unwrap_or(true) { continue; } let q = lab[i]; diff --git a/crates/engine/src/cmd/smartselect_cmds.rs b/crates/engine/src/cmd/smartselect_cmds.rs index a8a3e64d36..d8d4b771b1 100644 --- a/crates/engine/src/cmd/smartselect_cmds.rs +++ b/crates/engine/src/cmd/smartselect_cmds.rs @@ -573,5 +573,7 @@ pub fn specs() -> Vec { ] } +#[cfg(test)] +mod object_transparent_layer_tests; #[cfg(test)] mod tests; diff --git a/crates/engine/src/cmd/smartselect_cmds/object_transparent_layer_tests.rs b/crates/engine/src/cmd/smartselect_cmds/object_transparent_layer_tests.rs new file mode 100644 index 0000000000..749ccdc65a --- /dev/null +++ b/crates/engine/src/cmd/smartselect_cmds/object_transparent_layer_tests.rs @@ -0,0 +1,143 @@ +//! Object Selection's Object Finder click on a transparent layer (#12852): a mid-grey object is +//! an object of its own, and transparency stops the growth instead of reading as 50 % grey. +//! +//! The documents are the ones the issue measured against the reference application. + +use super::*; + +const W: u32 = 800; +const H: u32 = 600; + +/// The issue's document: white Background, then a transparent pixel layer holding a red +/// rectangle, a blue disc and a ten-block grey ramp (100, 105, …, 145 from x 100). +fn issue_document() -> Session { + let mut s = Session::new(); + s.execute("file.new", json!({"width": W, "height": H, "depth": 8})).unwrap(); + s.execute("layer.new.layer", json!({})).unwrap(); + paint_new_layer(&mut s, W, H, |x, y| { + let (x, y) = (x as i32, y as i32); + if (100..300).contains(&x) && (100..250).contains(&y) { + return [220.0 / 255.0, 30.0 / 255.0, 30.0 / 255.0, 1.0]; + } + let (dx, dy) = (x as f32 + 0.5 - 500.0, y as f32 + 0.5 - 175.0); + if dx * dx + dy * dy <= 75.0 * 75.0 { + return [30.0 / 255.0, 30.0 / 255.0, 220.0 / 255.0, 1.0]; + } + if (100..500).contains(&x) && (350..450).contains(&y) { + let v = (100 + 5 * ((x - 100) / 40)) as f32 / 255.0; + return [v, v, v, 1.0]; + } + [0.0, 0.0, 0.0, 0.0] + }); + s +} + +/// A Grayscale 8-bit document with a grey-87 rectangle on a transparent layer. +fn gray_document() -> Session { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 400, "height": 300, "mode": "gray", "depth": 8})).unwrap(); + s.execute("layer.new.layer", json!({})).unwrap(); + paint_new_layer(&mut s, 400, 300, |x, y| { + let (x, y) = (x as i32, y as i32); + if (100..300).contains(&x) && (100..250).contains(&y) { [87.0 / 255.0, 0.0, 0.0, 1.0] } else { [0.0; 4] } + }); + s +} + +/// Writes `px` (straight RGBA) over the whole surface of the last layer, which `file.new` plus +/// `layer.new.layer` leaves transparent. +fn paint_new_layer(s: &mut Session, w: u32, h: u32, px: impl Fn(u32, u32) -> [f32; 4]) { + s.edit("paint", |doc, _| { + let Some(surf) = doc.layers.last_mut().and_then(Layer::surface_mut) else { + return Err(EngineError::Other("the new layer has no surface".into())); + }; + let n = surf.channels(); + let mut data = Vec::with_capacity((w * h) as usize * n); + for y in 0..h { + for x in 0..w { + let c = px(x, y); + if n >= 4 { + data.extend_from_slice(&c); + } else if n == 2 { + data.extend_from_slice(&[c[0], c[3]]); + } else { + data.extend_from_slice(&c[..n.min(3)]); + } + } + } + surf.write_region(Rect::new(0, 0, w as i32, h as i32), &data); + Ok(()) + }) + .unwrap(); +} + +fn cov(s: &Session, x: i32, y: i32) -> f32 { + s.active().unwrap().doc.selection.as_ref().map_or(0.0, |m| m.sample_channel(x, y, 0)) +} + +/// `[left, top, right, bottom]` of the selection, as the issue writes bounds. +fn bounds(s: &Session) -> [i32; 4] { + let b = s.active().unwrap().doc.selection.as_ref().unwrap().content_bounds(); + [b.x0, b.y0, b.x1, b.y1] +} + +#[test] +fn a_grey_ramp_on_a_transparent_layer_is_found_at_both_points() { + for (x, y) in [(120, 400), (300, 400)] { + let mut s = issue_document(); + let before = s.active().unwrap().history.past_len(); + let r = s.execute("select.object", json!({"point": [x, y]})).unwrap(); + assert_eq!(r["changed"], true, "click at {x},{y}"); + assert_eq!(bounds(&s), [99, 349, 501, 451], "click at {x},{y}"); + // The whole ramp, and nothing around it. + assert!(cov(&s, 120, 400) >= 0.5 && cov(&s, 480, 440) >= 0.5); + assert!(cov(&s, 20, 20) < 0.5 && cov(&s, 700, 500) < 0.5); + // One "Object Selection" step, undone in one go. + assert_eq!(s.active().unwrap().history.past_len(), before + 1); + assert_eq!(s.active().unwrap().history.undo_label(), Some("Object Selection")); + s.execute("edit.undo", json!({})).unwrap(); + assert!(s.active().unwrap().doc.selection.is_none()); + } +} + +#[test] +fn a_grey_rectangle_on_a_transparent_gray_layer_is_found() { + let mut s = gray_document(); + let r = s.execute("select.object", json!({"point": [150, 150]})).unwrap(); + assert_eq!(r["changed"], true); + assert_eq!(bounds(&s), [99, 99, 301, 251]); + assert!(cov(&s, 150, 150) >= 0.5 && cov(&s, 20, 20) < 0.5); +} + +#[test] +fn clicking_transparency_selects_nothing() { + let mut s = issue_document(); + // Transparent pixels inside the layer's own content: between the rectangle and the disc, and + // between the rectangle and the ramp. + for (x, y) in [(350, 150), (200, 300)] { + let r = s.execute("select.object", json!({"point": [x, y]})).unwrap(); + assert_eq!(r["changed"], false, "click at {x},{y}"); + assert!(s.active().unwrap().doc.selection.is_none()); + } +} + +#[test] +fn red_and_blue_still_select_their_own_objects() { + let mut s = issue_document(); + for (x, y) in [(200, 150), (500, 175)] { + let r = s.execute("select.object", json!({"point": [x, y]})).unwrap(); + assert_eq!(r["changed"], true, "click at {x},{y}"); + assert!(cov(&s, x, y) >= 0.5); + // The click stays on its own object. + let other = if x < 400 { (500, 175) } else { (200, 150) }; + assert!(cov(&s, other.0, other.1) < 0.5, "click at {x},{y} leaked into the other object"); + } +} + +#[test] +fn sampling_all_layers_still_finds_the_ramp() { + let mut s = issue_document(); + let r = s.execute("select.object", json!({"point": [120, 400], "sampleAllLayers": true})).unwrap(); + assert_eq!(r["changed"], true); + assert!(cov(&s, 120, 400) >= 0.5 && cov(&s, 480, 440) >= 0.5); +}