From ca13e355c9f1cec0d7d9907d34391e5b85b79c8d Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Fri, 23 Oct 2026 11:14:50 +0530 Subject: [PATCH] fix(algo,engine): bound brush stroke work and total brush edit area Review of #8783: - matting::brush `segments` no longer collects its pieces: they are made as they are painted, and each segment is first cut to the painted area grown by the brush. A 1 px stroke zigzagging across 2 million px (within MAX_SWEPT_AREA) split into ~134 million pieces collected up front (~2 GB) in brush_region and EdgeBrush::new; now ~5000 pieces. - select.refineEdge `edits`: the Brush edits' swept area in all is bounded by MAX_SWEPT_AREA, as refineStrokes' is. Co-Authored-By: Claude Opus 5.5 --- crates/algo/src/matting.rs | 2 +- crates/algo/src/matting/brush.rs | 82 ++++++++++++++++++-------- crates/engine/src/select_mask_edits.rs | 21 +++++-- 3 files changed, 75 insertions(+), 30 deletions(-) diff --git a/crates/algo/src/matting.rs b/crates/algo/src/matting.rs index 2008256702..892a5f145f 100644 --- a/crates/algo/src/matting.rs +++ b/crates/algo/src/matting.rs @@ -238,7 +238,7 @@ pub fn morph(src: &[f32], w: usize, h: usize, r: usize, grow: bool) -> Vec mod brush; mod morphology; -pub use brush::{EdgeBrush, EdgeStroke, MAX_SWEPT_AREA, SIZE_RANGE as BRUSH_SIZE_RANGE, brush_region, swept_area}; +pub use brush::{EdgeBrush, EdgeStroke, MAX_SWEPT_AREA, SIZE_RANGE as BRUSH_SIZE_RANGE, brush_region, stroke_area, swept_area}; /// Running max (or min) over a window of radius `r` (edge-clipped), van Herk / Gil–Werman: /// three comparisons per sample whatever the radius. diff --git a/crates/algo/src/matting/brush.rs b/crates/algo/src/matting/brush.rs index ae78e28d93..ee54ea1875 100644 --- a/crates/algo/src/matting/brush.rs +++ b/crates/algo/src/matting/brush.rs @@ -81,8 +81,9 @@ pub fn swept_area(strokes: &[EdgeStroke]) -> f64 { strokes.iter().map(|s| stroke_area(&s.points, s.size)).sum() } -/// The area one stroke of `points` sweeps with a brush `size` wide: length × size + size². -fn stroke_area(points: &[(f32, f32)], size: f32) -> f64 { +/// The area one stroke of `points` sweeps with a brush `size` wide: length × size + size²; +/// non-finite strokes count as infinite. +pub fn stroke_area(points: &[(f32, f32)], size: f32) -> f64 { let size = f64::from(size.clamp(*SIZE_RANGE.start(), *SIZE_RANGE.end())); let len: f64 = points.windows(2).filter_map(|w| Some(f64::from((w.get(1)?.0 - w.first()?.0).hypot(w.get(1)?.1 - w.first()?.1)))).sum(); if len.is_finite() { len * size + size * size } else { f64::INFINITY } @@ -107,7 +108,7 @@ pub fn brush_region(points: &[(f32, f32)], size: f32, hardness: f32, canvas: Rec } let bw = bbox.width() as usize; let mut cov = vec![0.0f32; bw.checked_mul(bbox.height() as usize)?]; - for (a, b) in segments(points, r) { + for (a, b) in segments(points, r, bbox) { let seg = dab_bounds(a, r).union(&dab_bounds(b, r)).intersect(&bbox); for y in seg.y0..seg.y1 { for x in seg.x0..seg.x1 { @@ -124,17 +125,40 @@ pub fn brush_region(points: &[(f32, f32)], size: f32, hardness: f32, canvas: Rec mask.iter().any(|v| *v > 0).then_some(Region { bbox, mask }) } -/// The stroke's path as short segments: points closer than a quarter of the radius to the last -/// kept one are dropped, and long segments are split at most `r` long, so each segment's box -/// stays close to the area it paints. -fn segments(points: &[(f32, f32)], r: f32) -> Vec<((f32, f32), (f32, f32))> { - let Some(&first) = points.first() else { return Vec::new() }; +/// The part of the segment `a`–`b` inside `r` (Liang–Barsky), or `None` when it misses it. +fn clip_segment(a: (f32, f32), b: (f32, f32), r: Rect) -> Option<((f32, f32), (f32, f32))> { + let (dx, dy) = (b.0 - a.0, b.1 - a.1); + let (mut t0, mut t1) = (0.0f32, 1.0f32); + for (p, q) in [(-dx, a.0 - r.x0 as f32), (dx, r.x1 as f32 - a.0), (-dy, a.1 - r.y0 as f32), (dy, r.y1 as f32 - a.1)] { + if p == 0.0 { + if q < 0.0 { + return None; + } + } else { + let t = q / p; + if p < 0.0 { + t0 = t0.max(t); + } else { + t1 = t1.min(t); + } + } + } + (t0 <= t1 && t0.is_finite() && t1.is_finite()).then_some(((a.0 + dx * t0, a.1 + dy * t0), (a.0 + dx * t1, a.1 + dy * t1))) +} + +/// The stroke's path as short segments within reach of `clip`: points closer than a quarter of +/// the radius to the last kept one are dropped, each segment is cut to `clip` grown by the brush +/// (what lies beyond can't paint inside it), and long ones are split at most `r` long, so each +/// piece's box stays close to the area it paints. The pieces are made as they are painted, never +/// collected, and only where they can paint: a 1 px stroke zigzagging across 2 million px is over +/// 100 million pieces in all, within [`MAX_SWEPT_AREA`] (the #8783 review). +fn segments(points: &[(f32, f32)], r: f32, clip: Rect) -> impl Iterator { let min_step = (r / 4.0).max(0.5); let max_step = r.max(2.0); - let mut kept = vec![first]; - for p in points.iter().skip(1) { - let last = kept.last().copied().unwrap_or(first); - if (p.0 - last.0).hypot(p.1 - last.1) >= min_step { + let reach = clip.inflate((r + 2.0).ceil().clamp(0.0, 1e7) as i32); + let mut kept: Vec<(f32, f32)> = Vec::new(); + for p in points { + if kept.last().is_none_or(|l| (p.0 - l.0).hypot(p.1 - l.1) >= min_step) { kept.push(*p); } } @@ -143,20 +167,17 @@ fn segments(points: &[(f32, f32)], r: f32) -> Vec<((f32, f32), (f32, f32))> { { kept.push(end); } - if kept.len() == 1 { - return vec![(first, first)]; - } - let mut out = Vec::new(); - for w in kept.windows(2) { - let (Some(&a), Some(&b)) = (w.first(), w.get(1)) else { continue }; + // A single point is one dab. + let dab = if kept.len() == 1 { kept.first().map(|p| (*p, *p)) } else { None }; + let pairs: Vec<((f32, f32), (f32, f32))> = kept.windows(2).filter_map(|w| clip_segment(*w.first()?, *w.get(1)?, reach)).collect(); + dab.into_iter().chain(pairs.into_iter().flat_map(move |(a, b)| { let pieces = ((b.0 - a.0).hypot(b.1 - a.1) / max_step).ceil().clamp(1.0, 1e6) as usize; - let at = |k: usize| { + let at = move |k: usize| { let t = k as f32 / pieces as f32; (a.0 + (b.0 - a.0) * t, a.1 + (b.1 - a.1) * t) }; - out.extend((0..pieces).map(|k| (at(k), at(k + 1)))); - } - out + (0..pieces).map(move |k| (at(k), at(k + 1))) + })) } impl EdgeBrush { @@ -193,7 +214,7 @@ impl EdgeBrush { cov.clear(); cov.resize(sw * sb.height() as usize, 0.0); // The stroke's coverage: the most any of its segments gives (a single dab for one point). - for (a, b) in segments(&s.points, r) { + for (a, b) in segments(&s.points, r, sb) { let seg = dab_bounds(a, r).union(&dab_bounds(b, r)).intersect(&sb); for y in seg.y0..seg.y1 { for x in seg.x0..seg.x1 { @@ -407,4 +428,19 @@ mod tests { // Sizes and hardness outside their ranges are clamped, not rejected. assert!(brush_region(&[(50.0, 25.0)], 0.0, 500.0, canvas).is_some()); } + /// A 1 px stroke zigzagging across 2 million px stays within [`MAX_SWEPT_AREA`] yet splits + /// into over 100 million pieces: painting it must not collect them all first (it allocated + /// about 2 GB up front; the #8783 review). + #[test] + fn a_long_thin_zigzag_paints_without_collecting_its_pieces() { + let canvas = Rect::new(0, 0, 64, 64); + let points: Vec<(f32, f32)> = (0..135).map(|i| (if i % 2 == 0 { 0.0 } else { 2_000_000.0 }, 32.0 + (i % 3) as f32)).collect(); + assert!(stroke_area(&points, 1.0) <= MAX_SWEPT_AREA); + // Only the pieces within reach of the canvas are made. + assert!(segments(&points, 0.5, canvas).count() < 134 * 40, "{}", segments(&points, 0.5, canvas).count()); + let r = brush_region(&points, 1.0, 100.0, canvas).expect("it paints its row"); + assert!(r.at(10, 32) > 0.5); + let strokes = [EdgeStroke { points, size: 1.0, add: true }]; + assert!(EdgeBrush::new(&strokes, canvas).is_some_and(|b| b.at(10, 32) > 0.5)); + } } diff --git a/crates/engine/src/select_mask_edits.rs b/crates/engine/src/select_mask_edits.rs index 10bffb5bff..7d75b50510 100644 --- a/crates/engine/src/select_mask_edits.rs +++ b/crates/engine/src/select_mask_edits.rs @@ -114,6 +114,8 @@ pub fn parse(p: &Value) -> Result> { } let size_range = matting::BRUSH_SIZE_RANGE; let mut total = 0usize; + // The area the Brush edits sweep in all, bounded as `refineStrokes`' is. + let mut swept = 0.0f64; let mut edits = Vec::with_capacity(list.len()); for e in list { let o = e.as_object().ok_or_else(|| bad("each edit must be an object with a tool"))?; @@ -125,12 +127,15 @@ pub fn parse(p: &Value) -> Result> { add: adds(o)?, sample_all_layers: flag(o, "sampleAllLayers", false)?, }, - "brush" => SelectionEdit::Brush { - points: points(o, 1, &mut total)?, - size: number(o, "size", 30.0, size_range.clone())?, - hardness: number(o, "hardness", 100.0, 0.0..=100.0)?, - add: adds(o)?, - }, + "brush" => { + let points = points(o, 1, &mut total)?; + let size = number(o, "size", 30.0, size_range.clone())?; + swept += matting::stroke_area(&points, size); + if swept > matting::MAX_SWEPT_AREA { + return Err(bad("brush edits paint too large an area")); + } + SelectionEdit::Brush { points, size, hardness: number(o, "hardness", 100.0, 0.0..=100.0)?, add: adds(o)? } + } "lasso" => SelectionEdit::Lasso { points: points(o, 3, &mut total)?, add: adds(o)? }, "object" => { let area = match (o.get("rect"), o.get("point")) { @@ -267,6 +272,8 @@ mod tests { assert!(parse(&json!({})).expect("none").is_empty()); assert!(parse(&json!({"edits": null})).expect("none").is_empty()); assert!(has_edits(&p) && !has_edits(&json!({"edits": []})) && !has_edits(&json!({}))); + // One long brush edit within the area limit is fine; two of them (rejects_hostile_edits) are not. + assert!(parse(&json!({"edits": [{"tool": "brush", "points": [[0, 0], [10_000_000, 0]], "size": 20}]})).is_ok()); } #[test] @@ -296,6 +303,8 @@ mod tests { json!([{"tool": "subject", "mode": "intersect"}]), json!([{"tool": "brush", "points": many_points}]), json!(many_edits), + // Each within the limit, together past it. + json!(vec![json!({"tool": "brush", "points": [[0, 0], [10_000_000, 0]], "size": 20}); 2]), ] { assert!(parse(&json!({"edits": bad})).is_err(), "{bad}"); }