diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 0c5bd19f0..9fc2b26bc 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -1012,7 +1012,19 @@ fn masks(cx: &mut Cx<'_>, lin: &Buf, prep: &Prep, plan: &Plan<'_>, host: &mut Ho Some(3) } MaskShape::Brush { strokes } => { - let dabs: Vec<_> = strokes.iter().map(|st| lightcraft_pipeline::masks::brush_dabs(st, frame, w, h)).collect(); + // The kernel takes at most MAX_DABS across the whole mask, so no more than + // that (plus one, to tell "at it" from "past it") is ever built: each stroke + // may otherwise construct two million dabs before the sum below rejects them + // all (#1520). + let mut room = MAX_DABS + 1; + let dabs: Vec<_> = strokes + .iter() + .map(|st| { + let d = lightcraft_pipeline::masks::brush_dabs_within(st, frame, w, h, room); + room -= d.dabs.len().min(room); + d + }) + .collect(); // pen pressure varies radius and flow per dab, which the kernel doesn't take: the CPU rasterizes it if dabs.iter().map(|d| d.dabs.len()).sum::() > MAX_DABS || dabs.iter().any(|d| !d.pressure.is_empty()) { None diff --git a/crates/pipeline/src/masks.rs b/crates/pipeline/src/masks.rs index 36cc29192..a2497a2bd 100644 --- a/crates/pipeline/src/masks.rs +++ b/crates/pipeline/src/masks.rs @@ -524,6 +524,19 @@ pub const MAX_STROKE_DABS: usize = 2_000_000; /// The dabs of stroke `s` in a `w × h` output. pub fn brush_dabs(s: &BrushStroke, frame: &Frame, w: usize, h: usize) -> BrushDabs { + brush_dabs_within(s, frame, w, h, MAX_STROKE_DABS) +} + +/// [`brush_dabs`] stopping at `max` dabs instead of [`MAX_STROKE_DABS`]. +/// +/// For a caller that discards a stroke past a threshold of its own: it builds to that threshold +/// rather than to the stroke budget and throws the difference away. The GPU path rejects a brush +/// mask past 4096 dabs in total and the Auto Mask spot window past 200,000, both far below the +/// two million a single stroke may otherwise construct first (#1520). +/// +/// Ask for one more than the threshold to tell "at it" from "past it": a result shorter than +/// `max` is the whole stroke. +pub fn brush_dabs_within(s: &BrushStroke, frame: &Frame, w: usize, h: usize, max: usize) -> BrushDabs { let to_out = frame.norm_to_out(w, h); let ppl = frame.px_per_long(w); let r = (s.size * ppl).max(0.5); @@ -536,10 +549,14 @@ pub fn brush_dabs(s: &BrushStroke, frame: &Frame, w: usize, h: usize) -> BrushDa let with_pressure = !s.pressure.is_empty(); let mut prev = None; let mut prev_p = 1.0; + let mut emitted = 0usize; for (i, p) in s.points.iter().enumerate() { + if emitted >= max { + break; + } let q = to_out.apply(*p); let before = dabs.len(); - if push_dabs(&mut dabs, prev, q, r) { + if push_dabs_within(&mut dabs, &mut emitted, prev, q, r, max) { let pq = s.pressure_at(i); if with_pressure { // the first dab of the stroke has nothing to interpolate from @@ -585,7 +602,16 @@ pub fn push_dabs(dabs: &mut Vec, prev: Option, q: Point, r: f64) - /// dabs no longer held in `dabs`. Incremental callers retain `emitted` when clearing their buffer; /// it advances only by the number actually appended. Acceptance and spacing match [`push_dabs`]. pub fn push_dabs_counted(dabs: &mut Vec, emitted: &mut usize, prev: Option, q: Point, r: f64) -> bool { - let room = MAX_STROKE_DABS.saturating_sub(*emitted); + push_dabs_within(dabs, emitted, prev, q, r, MAX_STROKE_DABS) +} + +/// [`push_dabs_counted`] against a caller's own budget rather than [`MAX_STROKE_DABS`]. +/// +/// A caller that will not look at more than `max` dabs should build no more than `max`: a length +/// check after the vector exists bounds nothing, it only throws away what was already paid for +/// (#1520). +pub fn push_dabs_within(dabs: &mut Vec, emitted: &mut usize, prev: Option, q: Point, r: f64, max: usize) -> bool { + let room = max.min(MAX_STROKE_DABS).saturating_sub(*emitted); if room == 0 || !(q.x.is_finite() && q.y.is_finite()) { return false; } @@ -856,6 +882,52 @@ mod tests { assert!(e[0].alpha.get(50, 50) < 0.01); } + /// #1520: a caller that will not look past its own threshold builds to that threshold, not to + /// the stroke budget. Each segment here is admissible on its own and together they want far + /// more than any threshold the callers apply, which is the aggregate the issue describes + /// rather than one hostile segment. + #[test] + fn a_stroke_is_built_only_as_far_as_the_caller_will_look() { + let f = frame(4000, 4000); + // Twenty long sweeps across the output: tens of thousands of dabs between them. + let points: Vec = + (0..20).map(|i| if i % 2 == 0 { Point::new(0.02, 0.02 + i as f64 * 0.04) } else { Point::new(0.98, 0.02 + i as f64 * 0.04) }).collect(); + let s = BrushStroke { points, size: 0.002, feather: 50.0, flow: 100.0, density: 100.0, ..Default::default() }; + + let whole = brush_dabs(&s, &f, 4000, 4000); + assert!(whole.dabs.len() > 20_000, "the stroke really is large: {}", whole.dabs.len()); + assert!(whole.dabs.len() <= MAX_STROKE_DABS); + + // Each caller gets what it asked for and no more, however large the stroke is. 4097 and + // 200_001 are what the GPU path and the Auto Mask spot window now ask for. + for max in [1usize, 2, 4097, 200_001] { + let got = brush_dabs_within(&s, &f, 4000, 4000, max); + assert!(got.dabs.len() <= max, "asked for at most {max}, built {}", got.dabs.len()); + assert_eq!(got.r, whole.r, "the brush itself is unchanged"); + } + + // A budget the stroke never reaches changes nothing: it only ever truncates. + let generous = brush_dabs_within(&s, &f, 4000, 4000, whole.dabs.len() + 1000); + assert_eq!(generous.dabs, whole.dabs, "an unreached budget leaves the stroke alone"); + + // The guarantee the callers rely on: a result short of the budget was never truncated, + // so it is the stroke itself. Both callers ask for one more than they will accept, so + // anything they keep is identical to what they drew before this change, and anything + // truncated is something they were going to reject anyway. + // + // A truncated one is not a prefix: running out of room makes a segment lay its remaining + // dabs further apart rather than stop mid-segment, which is how `push_dabs_counted` + // already behaved at MAX_STROKE_DABS. That only ever reaches a caller that rejects it. + let cut = brush_dabs_within(&s, &f, 4000, 4000, 500); + assert_eq!(cut.dabs.len(), 500, "a stroke past the budget fills it exactly"); + for max in [700usize, 5_000, whole.dabs.len()] { + let got = brush_dabs_within(&s, &f, 4000, 4000, max); + if got.dabs.len() < max { + assert_eq!(got.dabs, whole.dabs, "a result short of its budget is the whole stroke"); + } + } + } + /// Issue #517: dabs along a long segment (a fast stroke: two pointer samples far apart) are at /// most [`dab_spacing`] apart, painting dab by dab (the live stroke) or the whole path at once /// (the committed one). diff --git a/crates/pipeline/src/spots.rs b/crates/pipeline/src/spots.rs index 315f7f956..a875ffb7f 100644 --- a/crates/pipeline/src/spots.rs +++ b/crates/pipeline/src/spots.rs @@ -206,7 +206,9 @@ pub fn window_for_reads_checked( // refine the stroke over a couple of radii: each dab matters to pixels within 2.5 radii let mut dab_boxes: Vec<[f64; 4]> = Vec::new(); for st in autos { - let dabs = crate::masks::brush_dabs(st, frame, full_w, full_h); + // One more than the window takes, so a stroke past it is still recognised as past it + // without the other 1.8 million dabs being built first (#1520). + let dabs = crate::masks::brush_dabs_within(st, frame, full_w, full_h, MAX_DABS_CONSIDERED + 1); let r = dabs.r; if dabs.dabs.len() > MAX_DABS_CONSIDERED { continue;