fix(compose): redraw shapes crossing canvas bounds and include vector bounds in damage (#3946) (#4493)

This commit is contained in:
Tarek Mohamed authored and GitHub committed 2026-10-13 12:33:47 -04:00
1 parent e4f005d3dc
commit c4f0b8aa8b
3 files changed
+93 -2

No files matched your search

+24
View File
@@ -671,6 +671,19 @@ pub fn layer_bounds(layer: &Layer, canvas: Rect) -> Rect {
LayerContent::Fill(_) => canvas,
_ => {
let b = layer.surface().map_or(Rect::EMPTY, bounds::content_bounds);
let b = match &layer.content {
LayerContent::Shape(sh) => {
let p = shape_bounds(sh).map(|r| r.intersect(&canvas)).unwrap_or(Rect::EMPTY);
if !b.is_empty() && !p.is_empty() {
b.union(&p)
} else if !p.is_empty() {
p
} else {
b
}
}
_ => b,
};
// Effects follow a filled shape's outline, also where its fill is transparent.
match effect_outline(layer).and_then(|_| paint_bounds(layer)).filter(|_| effects::has_effects(layer)) {
Some(p) if !b.is_empty() => b.union(&p),
@@ -768,6 +781,17 @@ pub fn paint_bounds(layer: &Layer) -> Option<Rect> {
(!r.is_empty()).then_some(r)
}
/// The bounding box of a shape layer's vector content: its path control bounds plus stroke width.
pub fn shape_bounds(sh: &photocraft_doc::ShapeLayer) -> Option<Rect> {
if sh.fill.is_none() && sh.stroke.as_ref().is_none_or(|s| s.width <= 0.0 || s.opacity <= 0.0) {
return None;
}
let (x0, y0, x1, y1) = sh.path.control_bounds()?;
let stroke_margin = sh.stroke.as_ref().filter(|s| s.opacity > 0.0).map_or(0.0, |s| s.width.ceil()) as i32;
let r = Rect::new(x0.floor() as i32, y0.floor() as i32, x1.ceil() as i32, y1.ceil() as i32).inflate(stroke_margin);
(!r.is_empty()).then_some(r)
}
/// The layer's effective mask over `rect` (row-major), read once per tile: the pixel mask
/// times the rasterized vector mask.
fn mask_vals(layer: &Layer, rect: Rect, cx: &Ctx) -> Option<Vec<f32>> {
+24
View File
@@ -2570,3 +2570,27 @@ fn effect_cache_follows_a_group_childs_blend_if() {
assert!(before.px != cold.px, "Blend If should change the group's shadow");
assert!(warm.px == cold.px, "stale group effect maps after a child's Blend If changed");
}
#[test]
fn shape_layer_bounds_includes_vector_bounds_clipped_to_canvas_and_preserves_effects() {
let canvas = Rect::new(0, 0, 100, 100);
let subpath = photocraft_doc::vector::Subpath::polygon(&[(-20.0, -20.0), (50.0, -20.0), (50.0, 50.0), (-20.0, 50.0)]);
let path = photocraft_doc::vector::Path { subpaths: vec![subpath], ..Default::default() };
let sh = photocraft_doc::vector::ShapeLayer {
path,
fill: Some(photocraft_doc::Fill::Solid(photocraft_doc::Color::BLACK)),
stroke: Some(photocraft_doc::vector::ShapeStroke { width: 4.0, opacity: 1.0, ..Default::default() }),
..Default::default()
};
let mut layer = Layer::new("shape", LayerContent::Shape(sh));
// Vector bounds reach outside canvas (-24..54), but layer_bounds clips vector bounds to canvas:
let b = layer_bounds(&layer, canvas);
assert_eq!(b, Rect::new(0, 0, 54, 54), "shape vector bounds must clip to canvas and not reach past it");
// When the shape has layer effects, layer_bounds preserves effects union handling:
layer.effects.items = vec![Effect::DropShadow(Shadow { size: 5.0, spread: 0.0, ..shadow(5.0, 90.0) })];
let b_fx = layer_bounds(&layer, canvas);
assert!(!b_fx.is_empty());
assert!(b_fx.contains_rect(&b));
}
+45 -2
View File
@@ -423,13 +423,20 @@ fn shift_shown(doc: &Document, l: &mut Layer, dx: i32, dy: i32) {
LayerContent::Shape(sh) => {
// The rendered shape is cut at the canvas: one that reaches past it renders again.
let canvas = doc.bounds();
let inside = sh.cache.as_ref().is_some_and(|c| {
let was_inside = sh.cache.as_ref().is_some_and(|c| {
let b = photocraft_compose::bounds::content_bounds(c);
b.is_empty() || (b.x0 > canvas.x0 && b.y0 > canvas.y0 && b.x1 < canvas.x1 && b.y1 < canvas.y1)
});
crate::vector_cmds::transform_shape(sh, &a);
let stroke_pad = sh.stroke.as_ref().filter(|s| s.opacity > 0.0).map_or(0.0, |s| f64::from(s.width.ceil()));
let stays_inside = sh.path.control_bounds().is_some_and(|(x0, y0, x1, y1)| {
(x0 - stroke_pad) > canvas.x0 as f64
&& (y0 - stroke_pad) > canvas.y0 as f64
&& (x1 + stroke_pad) < canvas.x1 as f64
&& (y1 + stroke_pad) < canvas.y1 as f64
});
match &mut sh.cache {
Some(c) if inside => *c = translate_surface(c, dx, dy),
Some(c) if was_inside && stays_inside => *c = translate_surface(c, dx, dy),
_ => crate::vector_cmds::refresh_shape(doc, sh),
}
}
@@ -1806,6 +1813,42 @@ mod tests {
assert!(moved(&before, &[LayerId(999_999)], 1, 1).is_err());
}
#[test]
fn move_stroked_shape_across_canvas_edge_matches_fresh_render() {
let mut s = session(8);
let id = LayerId(
s.execute("shape.create", json!({"kind": "rect", "rect": [10, 10, 30, 30], "stroke": {"width": 4, "color": "#ff0000"}})).unwrap()["layer"]
.as_u64()
.unwrap(),
);
let before = doc(&s).clone();
let ids = move_targets(&before, &[id]);
// Drag the stroked shape across the left canvas boundary:
let moved_doc = moved(&before, &ids, -25, 0).unwrap();
let moved_layer = moved_doc.layer(id).unwrap();
let LayerContent::Shape(sh) = &moved_layer.content else { panic!("not a shape") };
let cached = sh.cache.as_ref().expect("cached render");
let mut fresh_sh = sh.clone();
crate::vector_cmds::refresh_shape(&moved_doc, &mut fresh_sh);
let fresh = fresh_sh.cache.as_ref().expect("fresh render");
assert_eq!(cached, fresh, "dragged shape cached render must match fresh render across canvas edge");
// Drag it back inside the canvas:
let moved_back = moved(&moved_doc, &ids, 30, 0).unwrap();
let back_layer = moved_back.layer(id).unwrap();
let LayerContent::Shape(back_sh) = &back_layer.content else { panic!("not a shape") };
let back_cached = back_sh.cache.as_ref().expect("cached render");
let mut fresh_back = back_sh.clone();
crate::vector_cmds::refresh_shape(&moved_back, &mut fresh_back);
let fresh_back_render = fresh_back.cache.as_ref().expect("fresh render");
assert_eq!(back_cached, fresh_back_render, "cached render must match fresh render when dragged back");
}
fn select_all(s: &mut Session, ids: &[LayerId]) {
s.execute("layer.select", json!({"layer": ids[0].0})).unwrap();
for id in &ids[1..] {