Retouch: guard every stroke command, not only Pattern Stamp (#6110)

Pattern Stamp was the only retouch command that refused a stroke it could not
render. Clone Stamp, Healing Brush, Spot Healing and Remove all reach the same
dense conversion: coverage is accumulated in tiles, but dense_coverage()
materialises the whole bounding box, and the retouch paths read a Region over
that box for every channel. The box comes from the stroke's own points, which
are validated against MAX_COORD rather than the document, so it is not bounded
by the canvas. Both allocations go through the infallible vec!, which aborts
instead of returning an error.

With the guards now shared in retouch_cmds, apply them to the rest:
reject_huge_stroke before anything is rendered, and reject_bounds before the
dense region read where the command can return one. spot_heal_surface returns a
Rect, so Spot Healing charges the stroke in the command; Remove builds a list,
so each stroke is charged as it is parsed.

The allowance is unchanged, so no stroke that worked before starts failing.

Test: every retouch stroke refuses the oversized box, leaves the pixels alone,
and still accepts a stroke across the whole canvas.
This commit is contained in:
Muhammad Saad
2026-10-15 15:47:38 +04:00
parent 205c9c1737
commit f116c3230a
3 changed files with 50 additions and 4 deletions
+5
View File
@@ -358,6 +358,7 @@ fn clamp_samples(fmt: &PixelFormat, data: &mut [f32]) {
fn clone_stamp(s: &mut Session, p: &Value) -> Result<Value> {
const CMD: &str = "paint.cloneStamp";
let (stroke, id) = parse_brush(s, p, CMD)?;
reject_huge_stroke(CMD, &stroke)?;
let which = sample_layers(p, CMD)?;
let mode = blend_param(p, CMD)?;
let aligned = flag(p, "aligned", true);
@@ -383,6 +384,7 @@ fn clone_stamp(s: &mut Session, p: &Value) -> Result<Value> {
.draw(bounds, &stroke.brush, &stroke.points, |r| clone_sample(pre, id, &before, which, r, &map, true, CMD));
}
let (bounds, cov) = stroke_coverage(&stroke);
reject_bounds(CMD, bounds)?;
// Samples come from the pre-stroke state, so pixels painted earlier in the stroke are never re-cloned.
let paint = clone_sample(pre, id, surf, which, bounds, &map, flag(p, "wrap", false), CMD)?;
Ok(apply_coverage(surf, bounds, &cov, stroke.brush.opacity, sel, lock, &paint, mode))
@@ -638,6 +640,7 @@ fn heal_region(fmt: &PixelFormat, src: &Region, dst: &Region, mask: &[bool]) ->
fn healing_brush(s: &mut Session, p: &Value) -> Result<Value> {
const CMD: &str = "paint.healingBrush";
let (stroke, id) = parse_brush(s, p, CMD)?;
reject_huge_stroke(CMD, &stroke)?;
let which = sample_layers(p, CMD)?;
let mode = blend_param(p, CMD)?;
let aligned = flag(p, "aligned", true);
@@ -645,6 +648,7 @@ fn healing_brush(s: &mut Session, p: &Value) -> Result<Value> {
let off = map.offset();
let dmg = run_stroke(s, "Healing Brush", id, p, |pre, surf, sel, lock| {
let (bounds, cov) = stroke_coverage(&stroke);
reject_bounds(CMD, bounds)?;
let (g, cov) = pad_coverage(bounds, &cov, 2);
let fmt = surf.format();
let src = clone_sample(pre, id, surf, which, g, &map, false, CMD)?;
@@ -821,6 +825,7 @@ fn spot_heal_surface(surf: &mut Surface, pre: &Document, stroke: &Stroke, kind:
fn spot_healing(s: &mut Session, p: &Value) -> Result<Value> {
const CMD: &str = "paint.spotHealing";
let (stroke, id) = parse_brush(s, p, CMD)?;
reject_huge_stroke(CMD, &stroke)?;
let kind = match string(p, "type", "contentAware") {
"contentAware" => SpotType::ContentAware,
"createTexture" => SpotType::CreateTexture,
+2
View File
@@ -71,6 +71,7 @@ fn stroke_bounds(stroke: &Stroke) -> Rect {
fn parse_strokes(s: &Session, p: &Value) -> Result<(Vec<Stroke>, Option<LayerId>)> {
let Some(list) = p.get("strokes") else {
let (stroke, id) = parse_brush(s, p, CMD)?;
reject_huge_stroke(CMD, &stroke)?;
return Ok((vec![stroke], id));
};
let list = list.as_array().ok_or_else(|| bad(CMD, "`strokes` is a list of point lists"))?;
@@ -86,6 +87,7 @@ fn parse_strokes(s: &Session, p: &Value) -> Result<(Vec<Stroke>, Option<LayerId>
o.insert("points".into(), points.clone());
}
let (stroke, layer) = parse_brush(s, &one, CMD)?;
reject_huge_stroke(CMD, &stroke)?;
strokes.push(stroke);
id = layer;
}
+43 -4
View File
@@ -544,10 +544,10 @@ fn history_brush_restores_earlier_pixels() {
}
}
/// #4961: the History Brush's coverage and its source region are both dense over the stroke's
/// bounds, so a stroke whose box dwarfs the canvas is refused from the points and brush size
/// alone, before the source surface is cloned or any coverage is rendered. The document is left
/// as it was, and ordinary strokes still paint at every depth.
/// #4961 and #6110: every retouch stroke renders its coverage densely over the stroke's bounds
/// and reads a source region over the same box, so a stroke whose box dwarfs the canvas is
/// refused from the points and brush size alone, before anything is cloned or rendered. The
/// document is left as it was, and ordinary strokes still paint at every depth.
#[test]
fn history_brush_refuses_a_stroke_too_large_to_render() {
for depth in DEPTHS {
@@ -565,6 +565,45 @@ fn history_brush_refuses_a_stroke_too_large_to_render() {
}
}
/// #6110: Pattern Stamp already refused a stroke it could not render; every other retouch stroke
/// reached the same dense conversion unguarded. Each command refuses the oversized stroke and
/// leaves the pixels alone, and a stroke across the whole canvas still works.
#[test]
fn every_retouch_stroke_refuses_a_box_it_cannot_render() {
let huge = json!([[0, 0], [900_000, 900_000]]);
let ok = json!([[0, 0], [40, 20]]);
let cases: [(&str, Value); 5] = [
("paint.cloneStamp", json!({"offset": [4, 0], "size": 8, "hardness": 100})),
("paint.healingBrush", json!({"offset": [4, 0], "size": 8, "hardness": 100})),
("paint.spotHealing", json!({"size": 8, "hardness": 100})),
("paint.historyBrush", json!({"size": 8, "hardness": 100})),
("paint.patternStamp", json!({"size": 8, "hardness": 100})),
];
for (cmd, base) in cases {
let mut s = session(40, 20, 8, "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 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}: {err}");
assert_eq!(rgba(&s, 20, 10), before, "{cmd}: a refused stroke leaves the pixels alone");
// The allowance still covers a stroke across the whole canvas.
let across = s.execute(cmd, with(&ok));
assert!(across.is_ok() || !format!("{across:?}").contains("stroke too large"), "{cmd}: {across:?}");
}
// Remove takes a list of strokes; each one is charged as it is parsed.
let mut s = session(40, 20, 8, "rgb");
let err = s.execute("paint.remove", json!({"strokes": [[[0, 0], [900_000, 900_000]]], "size": 8})).unwrap_err().to_string();
assert!(err.contains("stroke too large"), "paint.remove: {err}");
}
#[test]
fn history_brush_reads_a_source_when_its_pixels_were_locked() {
// Given a white layer locked in history state 1, then unlocked and filled red.