From 02f8d3a5e40f8489e0c853104b941560dd04fd7c Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sun, 25 Oct 2026 10:36:43 -0600 Subject: [PATCH 1/3] fix(vector): a mask feather continues the mask past the canvas edge (#9475) The feather blur clamped every one of its box passes at the plane's edge. Where the canvas cut a feathered mask off, each pass repeated the already blurred values there, brightening the edge (12 levels at a 32 px feather) and making the result depend on the canvas size. The blur now pads the plane with its edge values by the passes' reach, blurs, and keeps the original area, which matches Photoshop within one level both for a mask black at the edge and one white up to it. Co-Authored-By: Claude Opus 5.5 --- crates/compose/src/masks.rs | 30 ++++++++++++++++ crates/compose/src/soft_shape_tests.rs | 3 +- crates/vector/src/feather.rs | 49 ++++++++++++++++++++++---- 3 files changed, 75 insertions(+), 7 deletions(-) diff --git a/crates/compose/src/masks.rs b/crates/compose/src/masks.rs index 7335be90d0..63e76a8db2 100644 --- a/crates/compose/src/masks.rs +++ b/crates/compose/src/masks.rs @@ -391,6 +391,36 @@ mod tests { 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..d06a361c91 100644 --- a/crates/vector/src/feather.rs +++ b/crates/vector/src/feather.rs @@ -147,17 +147,54 @@ 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 }; + // 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); + let Some(mut padded) = padded(v, w, h, m) else { + blur_plane(v, w, h, pass); + return; + }; + let pw = w + 2 * m; + blur_plane(&mut padded, pw, h + 2 * m, pass); + for (dst, src) in v.chunks_exact_mut(w).zip(padded.chunks_exact(pw).skip(m)) { + if let Some(src) = src.get(m..m + w) { + dst.copy_from_slice(src); + } + } +} + +/// The `w`×`h` plane `v` with `m` more samples on every side, each the nearest edge value; `None` +/// when that doesn't fit in memory. +fn padded(v: &[f32], w: usize, h: usize, m: usize) -> Option> { + let pw = w.checked_add(m.checked_mul(2)?)?; + let len = pw.checked_mul(h.checked_add(m.checked_mul(2)?)?)?; + let mut out = Vec::new(); + out.try_reserve_exact(len).ok()?; + let rows: Vec<&[f32]> = v.chunks_exact(w).collect(); + let (first, last) = (*rows.first()?, *rows.last()?); + for row in std::iter::repeat_n(first, m).chain(rows.iter().copied()).chain(std::iter::repeat_n(last, m)) { + let (a, b) = (*row.first()?, *row.last()?); + out.extend(std::iter::repeat_n(a, m)); + out.extend_from_slice(row); + out.extend(std::iter::repeat_n(b, m)); + } + Some(out) +} + +fn blur_plane(v: &mut [f32], w: usize, h: usize, pass: BoxPass) { // Reserve before modifying the input so allocation failure can use the sequential fallback // without leaving a half-blurred mask. let mut transposed = Vec::new(); @@ -255,7 +292,7 @@ mod tests { 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); + blur_plane(&mut parallel, w, h, pass); assert_eq!(parallel, sequential, "parallel blur must retain the exact per-line arithmetic"); } From 3fb2b2853f6123fdfc6225af9cc214737aa03295 Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sun, 25 Oct 2026 10:57:22 -0600 Subject: [PATCH 2/3] Feather: pad each line past the edge, not the whole plane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit on #9493 (inline 4371565312): the edge halo was built as a padded copy of the whole plane, 3(r + 1) samples a side; at a 1000 px feather a 6000 × 6000 mask needed a 12000 × 12000 plane and its transpose, far past the 512 MiB working-set budget. Each row, then each column, is now continued past its ends on its own, which gives exactly the same result: the blur needs the plane's transpose and one padded line per worker, and the per-worker buffers are reserved fallibly. Tests: a_maximum_feather_does_not_pad_the_whole_plane (largest buffer for a 400 × 400 plane was 40,960,000 floats before), padding_each_line_matches_padding_the_plane (bitwise) Co-Authored-By: Claude Opus 5.5 --- crates/vector/src/feather.rs | 152 +++++++++++++++++++++++------------ 1 file changed, 100 insertions(+), 52 deletions(-) diff --git a/crates/vector/src/feather.rs b/crates/vector/src/feather.rs index d06a361c91..c639c43588 100644 --- a/crates/vector/src/feather.rs +++ b/crates/vector/src/feather.rs @@ -42,6 +42,17 @@ 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); + +/// 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,16 +110,45 @@ 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) { + let Some((mut padded, mut scratch)) = line_buffers(w, halo) else { return }; for row in v.chunks_mut(w) { - for _ in 0..3 { - blur_line(row, &mut scratch, pass); - } + blur_padded_line(row, &mut padded, &mut scratch, pass, halo); } } -fn blur_rows(v: &mut [f32], w: usize, pass: BoxPass) { +/// 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 (the lines are then left unblurred rather than aborting). +fn line_buffers(len: usize, halo: usize) -> Option<(Vec, Vec)> { + 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()]); +} + +fn blur_rows(v: &mut [f32], w: usize, pass: BoxPass, halo: usize) { if w == 0 || v.is_empty() { return; } @@ -119,14 +159,14 @@ fn blur_rows(v: &mut [f32], w: usize, pass: BoxPass) { 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); + blur_rows_chunk(v, w, pass, halo); return; } let rows_per_worker = h.div_ceil(threads); 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)); + scope.spawn(move || blur_rows_chunk(chunk, w, pass, halo)); } }); } @@ -163,71 +203,43 @@ pub fn gaussian(v: &mut [f32], w: usize, h: usize, sigma: f32) { // 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); - let Some(mut padded) = padded(v, w, h, m) else { - blur_plane(v, w, h, pass); - return; - }; - let pw = w + 2 * m; - blur_plane(&mut padded, pw, h + 2 * m, pass); - for (dst, src) in v.chunks_exact_mut(w).zip(padded.chunks_exact(pw).skip(m)) { - if let Some(src) = src.get(m..m + w) { - dst.copy_from_slice(src); - } - } + blur_plane(v, w, h, pass, m); } -/// The `w`×`h` plane `v` with `m` more samples on every side, each the nearest edge value; `None` -/// when that doesn't fit in memory. -fn padded(v: &[f32], w: usize, h: usize, m: usize) -> Option> { - let pw = w.checked_add(m.checked_mul(2)?)?; - let len = pw.checked_mul(h.checked_add(m.checked_mul(2)?)?)?; - let mut out = Vec::new(); - out.try_reserve_exact(len).ok()?; - let rows: Vec<&[f32]> = v.chunks_exact(w).collect(); - let (first, last) = (*rows.first()?, *rows.last()?); - for row in std::iter::repeat_n(first, m).chain(rows.iter().copied()).chain(std::iter::repeat_n(last, m)) { - let (a, b) = (*row.first()?, *row.last()?); - out.extend(std::iter::repeat_n(a, m)); - out.extend_from_slice(row); - out.extend(std::iter::repeat_n(b, m)); - } - Some(out) -} - -fn blur_plane(v: &mut [f32], w: usize, h: usize, pass: BoxPass) { +/// 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 before modifying the input so allocation failure can use the sequential fallback // without leaving a half-blurred mask. let mut transposed = Vec::new(); if transposed.try_reserve_exact(v.len()).is_err() { - gaussian_sequential(v, w, h, pass); + 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); transpose_into(v, &mut transposed, w, h); - blur_rows(&mut transposed, h, pass); + blur_rows(&mut transposed, h, pass, halo); 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) { +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]; @@ -291,9 +303,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); - blur_plane(&mut parallel, w, h, pass); - 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] @@ -306,7 +340,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(); @@ -315,4 +349,18 @@ 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())); + } } From 61a413d8df9e90ee545eb08e57c5615aa2b29cea Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Sun, 25 Oct 2026 11:12:24 -0600 Subject: [PATCH 3/3] Feather: reserve every line buffer before blurring CodeRabbit on #9493 (inline 4371628901): a worker whose line buffers couldn't be reserved left its rows unblurred while the rest of the plane was blurred, a half-feathered mask the cache would keep. Every worker's buffers (rows and columns) are now reserved, with the transpose, before the plane is touched; failing that, the sequential path runs, which also reserves first and otherwise leaves the plane as it was. Test: a_failed_line_buffer_never_leaves_a_half_blurred_mask Co-Authored-By: Claude Opus 5.5 --- crates/vector/src/feather.rs | 85 ++++++++++++++++++++++++++---------- 1 file changed, 62 insertions(+), 23 deletions(-) diff --git a/crates/vector/src/feather.rs b/crates/vector/src/feather.rs index c639c43588..2e3cd10d08 100644 --- a/crates/vector/src/feather.rs +++ b/crates/vector/src/feather.rs @@ -46,6 +46,10 @@ pub fn path_area(path: &Path, sigma: f32) -> Rect { #[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)] @@ -110,16 +114,20 @@ 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, halo: usize) { - let Some((mut padded, mut scratch)) = line_buffers(w, halo) else { return }; +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) { - blur_padded_line(row, &mut padded, &mut scratch, pass, halo); + blur_padded_line(row, padded, scratch, pass, halo); } } /// 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 (the lines are then left unblurred rather than aborting). +/// 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()?; @@ -148,25 +156,35 @@ fn blur_padded_line(line: &mut [f32], padded: &mut [f32], scratch: &mut [f32], p line.copy_from_slice(&padded[halo..halo + line.len()]); } -fn blur_rows(v: &mut [f32], w: usize, pass: BoxPass, halo: usize) { - if w == 0 || v.is_empty() { +/// 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, halo); + 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, halo)); + for (chunk, buffers) in v.chunks_mut(chunk_len).zip(buffers.iter_mut()) { + scope.spawn(move || blur_rows_chunk(chunk, w, pass, halo, buffers)); } }); } @@ -209,22 +227,25 @@ pub fn gaussian(v: &mut [f32], w: usize, h: usize, sigma: f32) { /// 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 before modifying the input so allocation failure can use the sequential fallback - // without leaving a half-blurred mask. + // 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() { + 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); note_buffer(v.len()); - blur_rows(v, w, pass, halo); + blur_rows(v, w, pass, halo, &mut rows); transpose_into(v, &mut transposed, w, h); - blur_rows(&mut transposed, h, pass, halo); + 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. +/// 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 len = w.max(h); @@ -363,4 +384,22 @@ mod tests { 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"); + } + } }