diff --git a/crates/compose/src/masks/tests.rs b/crates/compose/src/masks/tests.rs index b6aa42fe1f..9076770913 100644 --- a/crates/compose/src/masks/tests.rs +++ b/crates/compose/src/masks/tests.rs @@ -137,6 +137,36 @@ fn parallel_feathered_pixel_mask_returns_the_same_tiles() { assert!(outputs.iter().all(|surface| Arc::ptr_eq(surface.tiles().next().unwrap().1, outputs[0].tiles().next().unwrap().1))); } +/// A feather reaching the canvas edge sees the mask continue past it with its edge values, as +/// in Photoshop (#9475): a rectangle near the edge feathers the same on a taller canvas holding +/// the same mask (clamping each blur pass at the edge brightened it by up to 12 levels at +/// 32 px), and a mask white up to the edge stays white along it. +#[test] +fn feather_at_the_canvas_edge_continues_the_mask_past_it() { + let mut layer = Layer::raster("edge", PixelFormat::RGBA8); + let mut mask = photocraft_doc::LayerMask::hide_all(); + mask.surface.fill_rect(Rect::new(40, 20, 120, 60), &[1.0]); + mask.feather = 32.0; + layer.mask = Some(mask); + let canvas = Rect::new(0, 0, 160, 80); + let tight = combined_mask(&layer, canvas).unwrap().read_region(canvas); + let tall = combined_mask(&layer, Rect::new(0, -200, 160, 280)).unwrap().read_region(canvas); + let worst = tight.iter().zip(&tall).map(|(a, b)| (a - b).abs()).fold(0.0_f32, f32::max); + assert!(worst < 1e-4, "the canvas edge changed the feather by {worst}"); + + let mut half = photocraft_doc::LayerMask::hide_all(); + half.surface.fill_rect(Rect::new(0, 0, 80, 80), &[1.0]); + half.feather = 32.0; + layer.mask = Some(half); + let m = combined_mask(&layer, canvas).unwrap().read_region(canvas); + // Photoshop: every row the same, no darkening towards the top or bottom edge. + let middle = &m[40 * 160..41 * 160]; + for (y, row) in m.as_chunks::<160>().0.iter().enumerate() { + assert!(row.iter().zip(middle).all(|(a, b)| (a - b).abs() < 1e-5), "row {y} differs from row 40"); + } + assert!(middle[0] > 0.99, "the left edge stays white: {}", middle[0]); +} + #[cfg(not(target_arch = "wasm32"))] #[test] fn vector_mask_build_inside_a_small_rayon_pool_completes() { diff --git a/crates/compose/src/soft_shape_tests.rs b/crates/compose/src/soft_shape_tests.rs index 222b3f3e9a..37575beef5 100644 --- a/crates/compose/src/soft_shape_tests.rs +++ b/crates/compose/src/soft_shape_tests.rs @@ -91,7 +91,8 @@ fn density_shows_the_fill_beyond_the_path() { /// mask break differently: 127 and 128, both what Photoshop's "127/128" allows.) #[test] fn soft_shape_matches_an_equal_vector_mask() { - for (density, feather) in [(1.0, 10.0), (0.45, 0.0), (0.3, 6.5), (0.0, 0.0), (1.0, 0.0), (0.5, 3.0)] { + // 40 px reaches past the canvas edge: both feather it the same way there (#9475). + for (density, feather) in [(1.0, 10.0), (0.45, 0.0), (0.3, 6.5), (0.0, 0.0), (1.0, 0.0), (0.5, 3.0), (1.0, 40.0)] { for depth in [SampleType::U8, SampleType::U16] { let (s, f) = (shape_doc(depth, density, feather, None), fill_doc(depth, density, feather)); for y in [0, 45, 50, 100, 151, 199] { diff --git a/crates/vector/src/feather.rs b/crates/vector/src/feather.rs index 5048916d49..2e3cd10d08 100644 --- a/crates/vector/src/feather.rs +++ b/crates/vector/src/feather.rs @@ -42,6 +42,21 @@ pub fn path_area(path: &Path, sigma: f32) -> Rect { } } +/// The largest buffer (in `f32`s) the blur has allocated, for tests of its working set. +#[cfg(test)] +static LARGEST_BUFFER: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +/// Tests make [`line_buffers`] fail for lines of this many samples (0: never). +#[cfg(test)] +static FAIL_LINE_BUFFERS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +/// Notes a blur buffer of `len` floats ([`LARGEST_BUFFER`]); nothing outside tests. +fn note_buffer(len: usize) { + #[cfg(test)] + LARGEST_BUFFER.fetch_max(len, std::sync::atomic::Ordering::Relaxed); + let _ = len; +} + /// One box pass: weight 1 on `±r` around each sample and `end` (0..1) on the two samples just /// beyond, so the pass's variance is any real number, not only the `r(r+1)/3` of whole boxes. #[derive(Clone, Copy, Debug, PartialEq)] @@ -99,34 +114,77 @@ fn blur_line(line: &mut [f32], scratch: &mut [f32], pass: BoxPass) { /// Apply three clamped box passes to independent contiguous rows. Scoped OS threads are used /// instead of Rayon because mask construction can happen inside a Rayon tile job; nested Rayon /// work here can deadlock when every worker waits on the same OnceLock (#276). -fn blur_rows_chunk(v: &mut [f32], w: usize, pass: BoxPass) { - let mut scratch = vec![0.0; w]; +fn blur_rows_chunk(v: &mut [f32], w: usize, pass: BoxPass, halo: usize, buffers: &mut (Vec, Vec)) { + let (padded, scratch) = buffers; for row in v.chunks_mut(w) { - for _ in 0..3 { - blur_line(row, &mut scratch, pass); - } + blur_padded_line(row, padded, scratch, pass, halo); } } -fn blur_rows(v: &mut [f32], w: usize, pass: BoxPass) { - if w == 0 || v.is_empty() { +/// A padded line and its scratch for lines of `len` samples with `halo` more on each side; `None` +/// when they don't fit in memory. +fn line_buffers(len: usize, halo: usize) -> Option<(Vec, Vec)> { + #[cfg(test)] + if FAIL_LINE_BUFFERS.load(std::sync::atomic::Ordering::Relaxed) == len { + return None; + } + let n = halo.checked_mul(2)?.checked_add(len)?; + let (mut padded, mut scratch) = (Vec::new(), Vec::new()); + padded.try_reserve_exact(n).ok()?; + scratch.try_reserve_exact(n).ok()?; + padded.resize(n, 0.0); + scratch.resize(n, 0.0); + note_buffer(n); + Some((padded, scratch)) +} + +/// The three box passes on `line` continued `halo` samples past each end with its end values, as +/// Photoshop continues a mask past the canvas edge (#9475). Padding each line, row pass then +/// column pass, gives exactly what padding the whole plane would: the plane's padded corners +/// repeat its corner values, which is what the padded rows' own row pass leaves there. `padded` +/// and `scratch` hold `line.len() + 2 * halo` samples. +fn blur_padded_line(line: &mut [f32], padded: &mut [f32], scratch: &mut [f32], pass: BoxPass, halo: usize) { + let (Some(&a), Some(&b)) = (line.first(), line.last()) else { return }; + let n = line.len() + 2 * halo; + let (Some(padded), Some(scratch)) = (padded.get_mut(..n), scratch.get_mut(..n)) else { return }; + padded[..halo].fill(a); + padded[halo..halo + line.len()].copy_from_slice(line); + padded[halo + line.len()..].fill(b); + for _ in 0..3 { + blur_line(padded, scratch, pass); + } + line.copy_from_slice(&padded[halo..halo + line.len()]); +} + +/// How many workers blur `lines` lines of `len` samples. +fn row_workers(lines: usize, len: usize) -> usize { + if cfg!(target_arch = "wasm32") || lines.saturating_mul(len) < 1 << 16 { + 1 + } else { + std::thread::available_parallelism().map_or(1, |n| n.get()).clamp(1, 16).min(lines.max(1)) + } +} + +/// Line buffers for each of [`row_workers`]; `None` unless every one fits in memory. +fn worker_buffers(lines: usize, len: usize, halo: usize) -> Option, Vec)>> { + (0..row_workers(lines, len)).map(|_| line_buffers(len, halo)).collect() +} + +/// Blurs the rows of `v` on one worker per entry of `buffers` (from [`worker_buffers`]). +fn blur_rows(v: &mut [f32], w: usize, pass: BoxPass, halo: usize, buffers: &mut [(Vec, Vec)]) { + if w == 0 || v.is_empty() || buffers.is_empty() { return; } let h = v.len() / w; - let threads = if cfg!(target_arch = "wasm32") || v.len() < 1 << 16 { - 1 - } else { - std::thread::available_parallelism().map_or(1, |n| n.get()).clamp(1, 16).min(h.max(1)) - }; - if threads == 1 { - blur_rows_chunk(v, w, pass); + if let [only] = buffers { + blur_rows_chunk(v, w, pass, halo, only); return; } - let rows_per_worker = h.div_ceil(threads); + let rows_per_worker = h.div_ceil(buffers.len()); let chunk_len = rows_per_worker.saturating_mul(w); std::thread::scope(|scope| { - for chunk in v.chunks_mut(chunk_len) { - scope.spawn(move || blur_rows_chunk(chunk, w, pass)); + for (chunk, buffers) in v.chunks_mut(chunk_len).zip(buffers.iter_mut()) { + scope.spawn(move || blur_rows_chunk(chunk, w, pass, halo, buffers)); } }); } @@ -147,50 +205,62 @@ fn transpose_into(v: &[f32], out: &mut [f32], w: usize, h: usize) { } /// Approximate Gaussian blur of a `w`×`h` plane: three box passes per axis whose variance is -/// exactly `sigma²` ([`BoxPass`]; edges clamp, which is exact here since the planes extend into -/// constant mask regions). Whole-pixel boxes rounded the variance (a 6 px feather blurred like -/// 6.48 px, a 3 px one like 3.46) and missed Photoshop's feathered edges by up to 4 levels -/// (psd-tools mask-parameters-no-real-channel.psd); the exact variance is within 1.5. The axes -/// are processed in parallel on bounded OS threads, preserving each line's floating-point +/// exactly `sigma²` ([`BoxPass`]). Beyond its edges the plane continues with its edge values, as +/// Photoshop feathers a mask at the canvas edge: a mask white up to the edge stays white there, +/// one black there stays black (#9475). Whole-pixel boxes rounded the variance (a 6 px feather +/// blurred like 6.48 px, a 3 px one like 3.46) and missed Photoshop's feathered edges by up to 4 +/// levels (psd-tools mask-parameters-no-real-channel.psd); the exact variance is within 1.5. The +/// axes are processed in parallel on bounded OS threads, preserving each line's floating-point /// operation order. pub fn gaussian(v: &mut [f32], w: usize, h: usize, sigma: f32) { if w == 0 || h == 0 || v.len() != w.saturating_mul(h) { return; } let Some(pass) = BoxPass::for_sigma(sigma) else { return }; - // Reserve before modifying the input so allocation failure can use the sequential fallback - // without leaving a half-blurred mask. + // Each pass reads `r + 1` samples past a pixel, so three see `3(r + 1)` past the edge. Clamping + // every pass at the edge instead repeated the blurred values there, which brightened a 32 px + // feather by up to 12 levels where the canvas cut it off. + let m = pass.r.saturating_add(1).saturating_mul(3); + blur_plane(v, w, h, pass, m); +} + +/// Three box passes along each axis of the `w`×`h` plane, every line continued `halo` samples +/// past its ends ([`blur_padded_line`]). +fn blur_plane(v: &mut [f32], w: usize, h: usize, pass: BoxPass, halo: usize) { + // Reserve everything before modifying the input, so an allocation failure can use the + // sequential fallback (which also reserves first) and never leaves a half-blurred mask. let mut transposed = Vec::new(); - if transposed.try_reserve_exact(v.len()).is_err() { - gaussian_sequential(v, w, h, pass); + let buffers = if transposed.try_reserve_exact(v.len()).is_ok() { worker_buffers(h, w, halo).zip(worker_buffers(w, h, halo)) } else { None }; + let Some((mut rows, mut columns)) = buffers else { + gaussian_sequential(v, w, h, pass, halo); return; - } + }; transposed.resize(v.len(), 0.0); - blur_rows(v, w, pass); + note_buffer(v.len()); + blur_rows(v, w, pass, halo, &mut rows); transpose_into(v, &mut transposed, w, h); - blur_rows(&mut transposed, h, pass); + blur_rows(&mut transposed, h, pass, halo, &mut columns); transpose_into(&transposed, v, h, w); } -/// Allocation-light reference path used if parallel transpose storage cannot be reserved. -fn gaussian_sequential(v: &mut [f32], w: usize, h: usize, pass: BoxPass) { +/// Allocation-light reference path used if parallel transpose storage cannot be reserved. It +/// reserves its lines before touching `v`: without them the plane is left as it was, never half +/// blurred. +fn gaussian_sequential(v: &mut [f32], w: usize, h: usize, pass: BoxPass, halo: usize) { let mut line = Vec::new(); - let mut scratch = Vec::new(); let len = w.max(h); - if line.try_reserve_exact(len).is_err() || scratch.try_reserve_exact(len).is_err() { + if line.try_reserve_exact(len).is_err() { return; } line.resize(len, 0.0); - scratch.resize(len, 0.0); + let Some((mut padded, mut scratch)) = line_buffers(len, halo) else { return }; for axis in [false, true] { let (lines, line_len) = if axis { (w, h) } else { (h, w) }; for k in 0..lines { for i in 0..line_len { line[i] = if axis { v[i * w + k] } else { v[k * w + i] }; } - for _ in 0..3 { - blur_line(&mut line[..line_len], &mut scratch[..line_len], pass); - } + blur_padded_line(&mut line[..line_len], &mut padded, &mut scratch, pass, halo); for i in 0..line_len { if axis { v[i * w + k] = line[i]; @@ -254,9 +324,31 @@ mod tests { let mut parallel: Vec<_> = (0..w * h).map(|i| ((i * 37 % 1009) as f32) / 1008.0).collect(); let mut sequential = parallel.clone(); let pass = BoxPass::for_sigma(sigma).unwrap(); - gaussian_sequential(&mut sequential, w, h, pass); - gaussian(&mut parallel, w, h, sigma); - assert_eq!(parallel, sequential, "parallel blur must retain the exact per-line arithmetic"); + let source = parallel.clone(); + for halo in [0, 3 * (pass.r + 1)] { + parallel.copy_from_slice(&source); + sequential.copy_from_slice(&source); + gaussian_sequential(&mut sequential, w, h, pass, halo); + blur_plane(&mut parallel, w, h, pass, halo); + assert_eq!(parallel, sequential, "parallel blur must retain the exact per-line arithmetic (halo {halo})"); + } + } + + /// Padding each line (#9493) blurs exactly as padding the whole plane with its edge values + /// and cropping it afterwards does, without the padded plane. + #[test] + fn padding_each_line_matches_padding_the_plane() { + let (w, h, sigma): (usize, usize, f32) = (90, 70, 9.0); + let source: Vec = (0..w * h).map(|i| ((i * 37 % 1009) as f32) / 1008.0).collect(); + let pass = BoxPass::for_sigma(sigma).unwrap(); + let m = 3 * (pass.r + 1); + let (pw, ph) = (w + 2 * m, h + 2 * m); + let mut plane: Vec = (0..pw * ph).map(|i| source[((i / pw).clamp(m, m + h - 1) - m) * w + (i % pw).clamp(m, m + w - 1) - m]).collect(); + blur_plane(&mut plane, pw, ph, pass, 0); + let want: Vec = (0..w * h).map(|i| plane[(i / w + m) * pw + i % w + m]).collect(); + let mut got = source; + gaussian(&mut got, w, h, sigma); + assert_eq!(got, want); } #[test] @@ -269,7 +361,7 @@ mod tests { let source: Vec<_> = (0..w * h).map(|i| ((i * 37 % 1009) as f32) / 1008.0).collect(); let mut sequential = source.clone(); let start = Instant::now(); - gaussian_sequential(&mut sequential, w, h, pass); + gaussian_sequential(&mut sequential, w, h, pass, 3 * (pass.r + 1)); let sequential_time = start.elapsed(); let mut parallel = source; let start = Instant::now(); @@ -278,4 +370,36 @@ mod tests { assert_eq!(parallel, sequential); eprintln!("50 MP mask blur: sequential={sequential_time:?}, parallel={parallel_time:?}"); } + + /// CodeRabbit on #9493: continuing the mask past the plane's edge must not build a padded copy + /// of the whole plane. At the 1000 px maximum feather the halo is about 3000 px a side, so a + /// padded 400 × 400 plane would be 6400 × 6400 floats; the blur needs only the plane's + /// transpose and one padded line per worker. + #[test] + fn a_maximum_feather_does_not_pad_the_whole_plane() { + let (w, h) = (400usize, 400usize); + let mut v: Vec = (0..w * h).map(|i| if (i % w) < w / 2 { 1.0 } else { 0.0 }).collect(); + gaussian(&mut v, w, h, feather_sigma(1000.0)); + let largest = LARGEST_BUFFER.load(std::sync::atomic::Ordering::Relaxed); + assert!(largest <= w * h, "the blur allocated a buffer of {largest} floats for a {w} × {h} plane"); + assert!(v.iter().all(|x| x.is_finite())); + } + + /// CodeRabbit on #9493: when a line buffer can't be had, the mask is never left half blurred + /// (rows untouched but columns blurred, or some workers' rows only): the plane is either fully + /// blurred or untouched. + #[test] + fn a_failed_line_buffer_never_leaves_a_half_blurred_mask() { + let (w, h, sigma): (usize, usize, f32) = (433, 211, 7.0); + let source: Vec = (0..w * h).map(|i| ((i * 37 % 1009) as f32) / 1008.0).collect(); + let mut want = source.clone(); + gaussian(&mut want, w, h, sigma); + for fail in [w, h] { + FAIL_LINE_BUFFERS.store(fail, std::sync::atomic::Ordering::Relaxed); + let mut got = source.clone(); + gaussian(&mut got, w, h, sigma); + FAIL_LINE_BUFFERS.store(0, std::sync::atomic::Ordering::Relaxed); + assert!(got == want || got == source, "lines of {fail} failing left a partly blurred plane"); + } + } }