diff --git a/crates/engine/src/brush_cmds.rs b/crates/engine/src/brush_cmds.rs --- a/crates/engine/src/brush_cmds.rs +++ b/crates/engine/src/brush_cmds.rs @@ -715,7 +715,10 @@ fn color_replacement_setup(s: &Session, p: &Value) -> Result<(Stroke, ReplaceSet background: s.tools.background, }; let id = layer_id(s, p)?; - Ok((Stroke { brush, points: pts }, rs, id)) + let stroke = Stroke { brush, points: pts }; + // The working copy is dense over the stroke's bounds (#8666). + crate::retouch_cmds::reject_huge_stroke(cmd, &stroke)?; + Ok((stroke, rs, id)) } fn color_replacement(s: &mut Session, p: &Value) -> Result { diff --git a/crates/engine/src/brush_cmds/mixer_live.rs b/crates/engine/src/brush_cmds/mixer_live.rs --- a/crates/engine/src/brush_cmds/mixer_live.rs +++ b/crates/engine/src/brush_cmds/mixer_live.rs @@ -132,6 +132,7 @@ pub struct LiveMixer { cmyk: Option>, original: Arc, points: Vec, + point_bounds: crate::retouch_cmds::PointBounds, original_state: MixerState, } @@ -163,6 +164,8 @@ impl LiveMixer { } let points = parse_points(p, CMD)?; let brush = resolve_brush(s, p, CMD)?; + // Dabs are tiled, but a stroke far beyond the canvas still means an unbounded dab count (#8666). + crate::retouch_cmds::reject_huge_stroke(CMD, &Stroke { brush: brush.clone(), points: points.clone() })?; let bm = &brush.mixer; let settings = MixerSettings { wet: pct(p, "wet", bm.wet * 100.0), @@ -233,6 +236,7 @@ impl LiveMixer { cmyk: doc_cmyk_space(s), original: s.active().ok_or(EngineError::NoDocument)?.doc.clone(), points: Vec::new(), + point_bounds: crate::retouch_cmds::PointBounds::EMPTY, original_state: s.tools.mixer.clone(), }; Ok((live, points)) @@ -254,6 +258,13 @@ impl LiveMixer { if self.core.preview_limits && self.points.len().saturating_add(points.len()) > 65_536 { return Err(bad(CMD, "stroke exceeds 65536 recorded input samples")); } + // A live stroke grows after it starts: admit the grown box before anything is mutated (#8666). + if !points.is_empty() { + let mut grown = self.point_bounds; + grown.add(points); + grown.reject_huge(CMD, &self.core.ctx.brush)?; + self.point_bounds = grown; + } let cmyk = self.cmyk.clone(); photocraft_color::convert::with_cmyk_space(cmyk.as_ref(), || { let old_tail = self.tail; diff --git a/crates/engine/src/brush_cmds/mixer_live_tests.rs b/crates/engine/src/brush_cmds/mixer_live_tests.rs --- a/crates/engine/src/brush_cmds/mixer_live_tests.rs +++ b/crates/engine/src/brush_cmds/mixer_live_tests.rs @@ -414,3 +414,16 @@ fn mixer_command_paints_strokes_beyond_the_preview_sample_and_dab_budgets() { assert!(target(&s).rgba(100, 48)[0] > 0.9); } } + +/// #8666: a live Mixer stroke that starts small and is pushed far away is refused, unchanged. +#[test] +fn mixer_live_push_refuses_a_stroke_that_grows_too_large() { + let s = fixture(PixelFormat::RGBA8); + let mut live = LiveMixer::begin(&s, &json!({"points":[[40,48]],"size":8,"seed":3})).unwrap(); + let (shown, bounds) = (live.doc.clone(), live.bounds()); + let err = live.push(&[StrokePoint::new(900_000.0, 900_000.0, 1.0)]).unwrap_err().to_string(); + assert!(err.contains("stroke too large"), "{err}"); + assert!(Arc::ptr_eq(&live.doc, &shown), "a refused push leaves the preview alone"); + assert_eq!(live.bounds(), bounds); + live.push(&[StrokePoint::new(60.0, 48.0, 1.0)]).unwrap(); +} diff --git a/crates/engine/src/eraser_cmds.rs b/crates/engine/src/eraser_cmds.rs --- a/crates/engine/src/eraser_cmds.rs +++ b/crates/engine/src/eraser_cmds.rs @@ -143,7 +143,10 @@ fn background_eraser_setup(s: &Session, p: &Value) -> Result<(Stroke, BgEraseSet if d.doc.effective_locks(id).transparency && !d.doc.is_background(id) { return Err(EngineError::Other(format!("Could not use the Background Eraser because the transparency of layer \"{}\" is locked", l.name))); } - Ok((Stroke { brush, points: pts }, bs, id)) + let stroke = Stroke { brush, points: pts }; + // The working copy is dense over the stroke's bounds (#8666). + crate::retouch_cmds::reject_huge_stroke(CMD, &stroke)?; + Ok((stroke, bs, id)) } /// `paint.backgroundEraser`: a stroke erasing colours similar to the sampled one. diff --git a/crates/engine/src/retouch_cmds.rs b/crates/engine/src/retouch_cmds.rs --- a/crates/engine/src/retouch_cmds.rs +++ b/crates/engine/src/retouch_cmds.rs @@ -44,42 +44,60 @@ fn bad(cmd: &str, msg: impl Into) -> EngineError { /// instead of allocating. const MAX_STROKE_PIXELS: u64 = 67_108_864; -/// Refuse a stroke whose footprint would not fit the coverage budget, from the points and brush -/// size alone, before the renderer allocates anything for it. -fn reject_huge_stroke(cmd: &str, stroke: &Stroke) -> Result<()> { - // Dynamics place dabs beyond the input points, so half the tip is not the whole reach: - // Scattering offsets a centre by up to its amount times the tip size, and a Dual Brush dab - // reaches its own radius further. Pad by what those can add so this estimate does not fall - // short of what the renderer will cover. It is only the cheap early-out; the rendered extent - // is admitted for real in `coverage` below. - let b = &stroke.brush; - let size = f64::from(b.size.max(1.0)); - let scatter = if b.scattering.enabled { size * f64::from(b.scattering.scatter.jitter.max(0.0)) } else { 0.0 }; - let dual = if b.dual_brush.enabled { f64::from(b.dual_brush.size.max(0.0)) * 0.5 } else { 0.0 }; - let pad = size.mul_add(0.5, 4.0) + scatter + dual; - let mut min_x = f64::INFINITY; - let mut min_y = f64::INFINITY; - let mut max_x = f64::NEG_INFINITY; - let mut max_y = f64::NEG_INFINITY; - for q in &stroke.points { - min_x = min_x.min(q.x); - min_y = min_y.min(q.y); - max_x = max_x.max(q.x); - max_y = max_y.max(q.y); - } - if !min_x.is_finite() || !min_y.is_finite() || !max_x.is_finite() || !max_y.is_finite() { - return Err(bad(cmd, "point coordinates must be finite")); - } - let w = (max_x - min_x + pad * 2.0).ceil().max(1.0); - let h = (max_y - min_y + pad * 2.0).ceil().max(1.0); - if !w.is_finite() || !h.is_finite() || w > 1.0e9 || h > 1.0e9 { - return Err(bad(cmd, format!("stroke too large (at most {MAX_STROKE_PIXELS} px)"))); - } - let area = (w as u64).saturating_mul(h as u64); - if area > MAX_STROKE_PIXELS { - return Err(bad(cmd, format!("stroke too large ({w:.0}×{h:.0} px, at most {MAX_STROKE_PIXELS})"))); - } - Ok(()) +/// The box the input points of a stroke span, grown as a live stroke is pushed to. +#[derive(Clone, Copy)] +pub(crate) struct PointBounds { + min_x: f64, + min_y: f64, + max_x: f64, + max_y: f64, +} + +impl PointBounds { + pub(crate) const EMPTY: Self = Self { min_x: f64::INFINITY, min_y: f64::INFINITY, max_x: f64::NEG_INFINITY, max_y: f64::NEG_INFINITY }; + + pub(crate) fn add(&mut self, points: &[StrokePoint]) { + for q in points { + self.min_x = self.min_x.min(q.x); + self.min_y = self.min_y.min(q.y); + self.max_x = self.max_x.max(q.x); + self.max_y = self.max_y.max(q.y); + } + } + + /// Refuse a footprint that would not fit the coverage budget, from these bounds and the + /// brush size alone, before the renderer allocates anything for it. + pub(crate) fn reject_huge(&self, cmd: &str, b: &BrushSettings) -> Result<()> { + // Dynamics place dabs beyond the input points, so half the tip is not the whole reach: + // Scattering offsets a centre by up to its amount times the tip size, and a Dual Brush dab + // reaches its own radius further. Pad by what those can add so this estimate does not fall + // short of what the renderer will cover. It is only the cheap early-out; the rendered extent + // is admitted for real in `coverage` below. + let size = f64::from(b.size.max(1.0)); + let scatter = if b.scattering.enabled { size * f64::from(b.scattering.scatter.jitter.max(0.0)) } else { 0.0 }; + let dual = if b.dual_brush.enabled { f64::from(b.dual_brush.size.max(0.0)) * 0.5 } else { 0.0 }; + let pad = size.mul_add(0.5, 4.0) + scatter + dual; + if !self.min_x.is_finite() || !self.min_y.is_finite() || !self.max_x.is_finite() || !self.max_y.is_finite() { + return Err(bad(cmd, "point coordinates must be finite")); + } + let w = (self.max_x - self.min_x + pad * 2.0).ceil().max(1.0); + let h = (self.max_y - self.min_y + pad * 2.0).ceil().max(1.0); + if !w.is_finite() || !h.is_finite() || w > 1.0e9 || h > 1.0e9 { + return Err(bad(cmd, format!("stroke too large (at most {MAX_STROKE_PIXELS} px)"))); + } + let area = (w as u64).saturating_mul(h as u64); + if area > MAX_STROKE_PIXELS { + return Err(bad(cmd, format!("stroke too large ({w:.0}×{h:.0} px, at most {MAX_STROKE_PIXELS})"))); + } + Ok(()) + } +} + +/// Refuse a stroke whose footprint would not fit the coverage budget (see [`PointBounds::reject_huge`]). +pub(crate) fn reject_huge_stroke(cmd: &str, stroke: &Stroke) -> Result<()> { + let mut bounds = PointBounds::EMPTY; + bounds.add(&stroke.points); + bounds.reject_huge(cmd, &stroke.brush) } /// The stroke's coverage, with the **rendered** extent admitted before the dense buffer exists. @@ -448,6 +466,7 @@ pub struct LiveRetouch { brush: Box, /// Every point rendered so far, in order. points: Vec, + point_bounds: PointBounds, } impl LiveRetouch { @@ -514,6 +533,7 @@ impl LiveRetouch { base, brush, points: Vec::new(), + point_bounds: PointBounds::EMPTY, }; live.push(&stroke.points)?; Ok(live) @@ -567,6 +587,13 @@ impl LiveRetouch { pub fn push(&mut self, pts: &[StrokePoint]) -> Result { crate::brush_cmds::check_coords(pts, &self.cmd)?; let pts: Vec<_> = pts.iter().map(|q| StrokePoint::new(q.x, q.y, q.pressure)).collect(); + // A live stroke grows after it starts: admit the grown box before anything is mutated (#8666). + if !pts.is_empty() { + let mut grown = self.point_bounds; + grown.add(&pts); + grown.reject_huge(&self.cmd, &self.brush)?; + self.point_bounds = grown; + } self.points.extend_from_slice(&pts); self.renderer.push(&pts); if self.cmd == "paint.cloneStamp" && flag(&self.params, "wrap", false) { @@ -1128,6 +1155,7 @@ impl DabTool { /// (`apply_dab_stroke`). Blur, Sharpen and Smudge can sample all layers. fn dab_cmd(s: &mut Session, p: &Value, cmd: &str) -> Result { let (stroke, id) = parse_brush(s, p, cmd)?; + reject_huge_stroke(cmd, &stroke)?; let (tool, label) = DabTool::parse(s, cmd, p, &stroke)?; let all = matches!(tool, DabTool::Focus { .. } | DabTool::Smudge { .. }) && sample_all_layers(p); // The live stroke the canvas already drew for exactly this commit (`LiveDab::into_commit`): @@ -1267,6 +1295,7 @@ pub struct LiveDab { base: Option<(photocraft_doc::DocId, u64)>, /// Every point rendered so far, in order. points: Vec, + point_bounds: PointBounds, } impl LiveDab { @@ -1339,6 +1368,8 @@ impl LiveDab { neighborhood: DabNeighborhood, effect: impl FnOnce(&Surface) -> DabEffect, ) -> Result { + // The working copy is dense over the stroke's bounds (#8666). + reject_huge_stroke(cmd, &stroke)?; let sel = doc.selection.clone(); let (surf, lock) = crate::channel_cmds::target_surface(&mut doc, id, p)?; let pre_surf = surf.clone(); @@ -1365,6 +1396,7 @@ impl LiveDab { tool: None, base: None, points: Vec::new(), + point_bounds: PointBounds::EMPTY, }; live.push(&stroke.points)?; Ok(live) @@ -1415,6 +1447,13 @@ impl LiveDab { pub fn push(&mut self, pts: &[StrokePoint]) -> Result { crate::brush_cmds::check_coords(pts, &self.cmd)?; let pts: Vec<_> = pts.iter().map(|q| StrokePoint::new(q.x, q.y, q.pressure)).collect(); + // A live stroke grows after it starts: admit the grown box before anything is mutated (#8666). + if !pts.is_empty() { + let mut grown = self.point_bounds; + grown.add(&pts); + grown.reject_huge(&self.cmd, &self.renderer.ctx.brush)?; + self.point_bounds = grown; + } self.points.extend_from_slice(&pts); self.renderer.push(&pts); let dmg = self.run_new_dabs(); diff --git a/crates/engine/src/retouch_cmds/tests.rs b/crates/engine/src/retouch_cmds/tests.rs --- a/crates/engine/src/retouch_cmds/tests.rs +++ b/crates/engine/src/retouch_cmds/tests.rs @@ -607,6 +607,53 @@ fn every_retouch_stroke_refuses_a_box_it_cannot_render() { assert!(err.contains("stroke too large"), "paint.remove: {err}"); } +/// #8666: the sequential-dab strokes (Dodge, Burn, Sponge, Blur, Sharpen, Smudge), the Mixer Brush, +/// Color Replacement and the Background Eraser all copy the target densely over the stroke's +/// bounds, and a drag far off the canvas made that copy large enough to abort the process. Each +/// refuses the oversized stroke without touching the pixels or the history, and an ordinary +/// stroke still runs, at every depth. +#[test] +fn every_dab_stroke_refuses_a_box_it_cannot_copy() { + let huge = json!([[0, 0], [900_000, 900_000]]); + let ok = json!([[2, 2], [30, 16]]); + let cases: [(&str, Value); 10] = [ + ("paint.dodge", json!({"size": 8})), + ("paint.burn", json!({"size": 8})), + ("paint.sponge", json!({"size": 8})), + ("paint.blur", json!({"size": 8})), + ("paint.sharpen", json!({"size": 8})), + ("paint.smudge", json!({"size": 8})), + ("paint.mixerBrush", json!({"size": 8})), + ("paint.mixerBrush", json!({"size": 8, "sampleAllLayers": true})), + ("paint.colorReplacement", json!({"size": 8})), + ("paint.backgroundEraser", json!({"size": 8})), + ]; + for depth in DEPTHS { + for (cmd, base) in &cases { + let mut s = session(40, 20, depth, "rgb"); + s.execute("paint.stroke", json!({"points": [[0, 10], [40, 10]], "size": 12, "hardness": 1.0, "color": "#ff0000"})).unwrap(); + let before = rgba(&s, 20, 10); + let steps = s.active().unwrap().history.past_len(); + let with = |points: &Value| { + let mut v = base.clone(); + if let Some(o) = v.as_object_mut() { + o.insert("points".into(), points.clone()); + } + v + }; + let err = s.execute(cmd, with(&huge)).unwrap_err().to_string(); + assert!(err.contains("stroke too large"), "{cmd} at {depth}: {err}"); + assert_eq!(rgba(&s, 20, 10), before, "{cmd} at {depth}: a refused stroke leaves the pixels alone"); + assert_eq!(s.active().unwrap().history.past_len(), steps, "{cmd} at {depth}: and records no history step"); + + let pixels = layer_pixels(&s); + s.execute(cmd, with(&ok)).unwrap_or_else(|e| panic!("{cmd} at {depth}: an ordinary stroke must work: {e}")); + assert_ne!(layer_pixels(&s), pixels, "{cmd} at {depth}: and changes pixels"); + assert_eq!(s.active().unwrap().history.past_len(), steps + 1, "{cmd} at {depth}: and records one step"); + } + } +} + /// The review on #6110: the estimate from the input points is not the bound. Brush dynamics place /// dabs beyond those points, so the budget is applied to the extent the renderer actually produced, /// before the dense buffer is allocated. An injected budget makes that observable without asking @@ -2413,3 +2460,42 @@ fn periodic_clone_interior_matches_ordinary_brush_masks_and_transparency() { } } } + +/// #8666: a live stroke can start small and then grow past the budget with a pushed point. The push +/// is refused with nothing changed (preview, bounds), and the stroke carries on afterwards. +#[test] +fn live_dab_push_refuses_a_stroke_that_grows_too_large() { + for (cmd, extra) in dab_cases() { + let mut s = session(64, 48, 8, "rgb"); + paint_layer(&mut s, texture); + let mut live = LiveDab::begin(&s, cmd, &dab_opts(&[[10.0, 10.0]], 50, &extra)).unwrap(); + let (shown, bounds) = (live.doc.clone(), live.bounds()); + let before = shown.layer(s.active().unwrap().active_layer.unwrap()).unwrap().surface().unwrap().clone(); + let err = live.push(&[StrokePoint::new(900_000.0, 900_000.0, 1.0)]).unwrap_err().to_string(); + assert!(err.contains("stroke too large"), "{cmd}: {err}"); + assert!(std::sync::Arc::ptr_eq(&live.doc, &shown), "{cmd}: a refused push leaves the preview alone"); + assert_eq!(live.bounds(), bounds, "{cmd}: and the bounds"); + let id = s.active().unwrap().active_layer.unwrap(); + assert_eq!(live.doc.layer(id).unwrap().surface().unwrap().read_region(shown.bounds()), before.read_region(shown.bounds()), "{cmd}: and the pixels"); + live.push(&[StrokePoint::new(30.0, 20.0, 1.0)]).unwrap_or_else(|e| panic!("{cmd}: the stroke must carry on: {e}")); + } +} + +/// #8666: the live Clone Stamp, Healing Brush, History Brush and Pattern Stamp strokes are refused +/// when a pushed point grows them past the budget, with the preview and bounds untouched. +#[test] +fn live_retouch_push_refuses_a_stroke_that_grows_too_large() { + for cmd in ["paint.cloneStamp", "paint.healingBrush", "paint.historyBrush", "paint.patternStamp"] { + let mut s = session(64, 48, 8, "rgb"); + paint_layer(&mut s, texture); + let p = json!({"points": [[10, 10]], "source": [4, 8], "size": 8, "hardness": 50, "pattern": "Checkerboard", "aligned": true}); + let mut live = LiveRetouch::begin(&s, cmd, &p).unwrap(); + let (shown, bounds) = (live.doc.clone(), live.bounds()); + let err = live.push(&[StrokePoint::new(900_000.0, 900_000.0, 1.0)]).unwrap_err().to_string(); + assert!(err.contains("stroke too large"), "{cmd}: {err}"); + assert!(std::sync::Arc::ptr_eq(&live.doc, &shown), "{cmd}: a refused push leaves the preview alone"); + assert_eq!(live.bounds(), bounds, "{cmd}"); + assert_eq!(live.points().len(), 1, "{cmd}: and the recorded points"); + live.push(&[StrokePoint::new(30.0, 20.0, 1.0)]).unwrap_or_else(|e| panic!("{cmd}: the stroke must carry on: {e}")); + } +}