Skip to content

Commit 7aa0de5

Browse files
committed
fix(paint): include box-shadow and descendant ink in the layer bounds
bleed is derived only from node.css.filter (FilterFn::Blur / DropShadow in filter_bleed, line 610). It ignores the node's own box_shadow, which step 5 paints *inside* this layer (line 411-418) at layout.x + dx - spread, width + spread*2 — i.e. outside the border-box — and it ignores the whole subtree painted at steps 9-10, which with the default overflow: visible may legitimately extend past the parent box (absolutely-positioned children, a child's own scale/pulse transform, a child glow, marquee, which CLAUDE.md explicitly documents as "exempté (leur rôle est de bleed)"). The file's own comment on filter_bleed (line 605-609) states the rule: "a *too-tight* one would silently clip filter bleed, trading a perf bug for a correctness one". Concretely: a card with box-shadow: 0 20px 40px rgba(0,0,0,.5) loses its shadow the moment any fade_in drives opacity below 1.0, and regains it on the frame opacity reaches 1.0 — a visible pop mid-entrance. The repo already treats this exact invariant as load-bearing for overflow: hidden (test overflow_hidden_does_not_clip_own_outset_box_shadow, line 2909); the opacity layer violates it for the same shadow. Fix: Compute the layer bounds from the union of: the border-box, the filter bleed, the outset box_shadow extents (|offset| + blur*1.5 + spread per shadow), and — when overflow is visible — the descendants' layout union. Alternatively move the outset box-shadow painting outside the opacity layer and multiply its paint alpha by opacity, and fall back to an unbounded layer when overflow: visible and children exist. Refs #220
1 parent b629e65 commit 7aa0de5

2 files changed

Lines changed: 228 additions & 13 deletions

File tree

‎crates/rustmotion-core/src/engine/paint_pass.rs‎

Lines changed: 95 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -356,17 +356,28 @@ fn paint_node(canvas: &Canvas, node: &BoxNode, ctx: &PaintContext, tree_depth: u
356356
}
357357
}
358358

359+
// Hoisted from step 8 below: the layer-bounds computation right after
360+
// this needs it too, to decide whether descendant ink painted outside
361+
// the border-box (legitimate under `overflow: visible`) must stay
362+
// reachable by the opacity/filter layer opened next.
363+
let overflow = node.css.overflow.unwrap_or(Overflow::Visible);
364+
359365
// 4. opacity / filter layer — one shared layer carries both the group
360366
// alpha and the CSS `filter` chain (applies to the node and its
361-
// subtree). Bounded to the node's own box (padded by the filter chain's
362-
// blur/drop-shadow bleed so those still bleed past the edge, unclipped):
363-
// an unbounded `SaveLayerRec` sizes the layer against the current clip —
364-
// usually the whole viewport — so every faded/filtered node allocates
365-
// and composites a full-frame layer regardless of how small it is
366-
// (measured on this repo's release binary, 1080x1920/60 frames, 30 small
367-
// `opacity: 0.5` shapes, `--threads 1`: ~42-60s wall time unbounded vs.
368-
// ~0.5s bounded — roughly two orders of magnitude, not a rounding
369-
// error; cost scales with viewport area, not node size).
367+
// subtree). Bounded to the node's own box, padded by: the filter
368+
// chain's blur/drop-shadow bleed, this node's own outset box-shadow
369+
// extent (painted inside this same layer at step 5, outside the
370+
// border-box), and — when `overflow` leaves descendant ink free to
371+
// paint past the border-box — the union of the whole subtree's layout
372+
// boxes. An unbounded `SaveLayerRec` sizes the layer against the
373+
// current clip — usually the whole viewport — so every faded/filtered
374+
// node allocates and composites a full-frame layer regardless of how
375+
// small it is (measured on this repo's release binary, 1080x1920/60
376+
// frames, 30 small `opacity: 0.5` shapes, `--threads 1`: ~42-60s wall
377+
// time unbounded vs. ~0.5s bounded — roughly two orders of magnitude,
378+
// not a rounding error; cost scales with viewport area, not node
379+
// size), so the bound stays tight to the content that can actually
380+
// paint rather than falling back to the viewport.
370381
let opacity = node.css.opacity.unwrap_or(1.0).clamp(0.0, 1.0);
371382
let content_filter = node
372383
.css
@@ -381,18 +392,30 @@ fn paint_node(canvas: &Canvas, node: &BoxNode, ctx: &PaintContext, tree_depth: u
381392
if let Some(filter) = content_filter {
382393
paint.set_image_filter(filter);
383394
}
384-
let bleed = node
395+
let filter_bleed_px = node
385396
.css
386397
.filter
387398
.as_deref()
388399
.map(|list| filter_bleed(list, &length_ctx))
389400
.unwrap_or(0.0);
390-
let bounds = Rect::from_xywh(
401+
let shadow_bleed_px = node
402+
.css
403+
.box_shadow
404+
.as_deref()
405+
.map(|shadows| box_shadow_bleed(shadows, &length_ctx))
406+
.unwrap_or(0.0);
407+
let bleed = filter_bleed_px.max(shadow_bleed_px);
408+
let mut bounds = Rect::from_xywh(
391409
box_layout.x - bleed,
392410
box_layout.y - bleed,
393411
box_layout.width + bleed * 2.0,
394412
box_layout.height + bleed * 2.0,
395413
);
414+
if overflow == Overflow::Visible {
415+
if let Some(descendants) = subtree_layout_bounds(node, ctx.layout) {
416+
bounds = Rect::join2(bounds, descendants);
417+
}
418+
}
396419
let rec = SaveLayerRec::default().paint(&paint).bounds(&bounds);
397420
canvas.save_layer(&rec);
398421
true
@@ -448,8 +471,8 @@ fn paint_node(canvas: &Canvas, node: &BoxNode, ctx: &PaintContext, tree_depth: u
448471

449472
// 8. clip overflow:hidden / clip — scoped to this node's own content and
450473
// its children only (see step 5-7's comment for why the box's own
451-
// decorations must stay outside this clip).
452-
let overflow = node.css.overflow.unwrap_or(Overflow::Visible);
474+
// decorations must stay outside this clip). `overflow` was hoisted
475+
// above step 4.
453476
let opened_overflow_clip = if matches!(
454477
overflow,
455478
Overflow::Hidden | Overflow::Clip | Overflow::Scroll | Overflow::Auto
@@ -632,6 +655,65 @@ fn filter_bleed(list: &[crate::css::style::FilterFn], ctx: &LengthContext) -> f3
632655
bleed
633656
}
634657

658+
/// Conservative outward bleed (px) a node's own outset `box_shadow` list
659+
/// paints beyond its border-box — the same role `filter_bleed` plays for
660+
/// `filter`, and sized the same way (offset + spread pushes the shadow rect
661+
/// out, `1.5x` blur radius covers the Gaussian falloff). Inset shadows are
662+
/// clipped to the padding-box by `paint_box_shadow` and never bleed outward,
663+
/// so they are skipped here.
664+
fn box_shadow_bleed(shadows: &[BoxShadow], ctx: &LengthContext) -> f32 {
665+
let mut bleed = 0.0f32;
666+
for shadow in shadows {
667+
if shadow.inset.unwrap_or(false) {
668+
continue;
669+
}
670+
let offset = shadow
671+
.offset_x
672+
.resolve(ctx)
673+
.abs()
674+
.max(shadow.offset_y.resolve(ctx).abs());
675+
let spread = shadow
676+
.spread
677+
.as_ref()
678+
.map(|s| s.resolve(ctx).max(0.0))
679+
.unwrap_or(0.0);
680+
let blur_bleed = shadow
681+
.blur
682+
.as_ref()
683+
.map(|b| b.resolve(ctx).max(0.0) * 1.5)
684+
.unwrap_or(0.0);
685+
bleed = bleed.max(offset + spread + blur_bleed);
686+
}
687+
bleed
688+
}
689+
690+
/// Bounding box (viewport coordinates) of every descendant's own layout box,
691+
/// recursively — the same "leave the layer big enough to hold what can
692+
/// legitimately paint outside the border-box" contract as `filter_bleed`,
693+
/// applied to `overflow: visible` subtrees instead of a filter chain. Each
694+
/// descendant contributes only its plain layout rect (not its own
695+
/// filter/shadow bleed or transform): a tight bound for the common cases —
696+
/// absolutely-positioned children, `marquee`, a taller-than-parent flow —
697+
/// without walking the whole subtree's CSS.
698+
fn subtree_layout_bounds(node: &BoxNode, layout: &LayoutResult) -> Option<Rect> {
699+
let mut bounds: Option<Rect> = None;
700+
for child in &node.children {
701+
if let Some(child_layout) = layout.get(child.id) {
702+
let rect = Rect::from_xywh(
703+
child_layout.x,
704+
child_layout.y,
705+
child_layout.width,
706+
child_layout.height,
707+
);
708+
bounds = Some(bounds.map_or(rect, |b| Rect::join2(b, rect)));
709+
}
710+
if let Some(child_bounds) = subtree_layout_bounds(child, layout) {
711+
bounds = Some(bounds.map_or(child_bounds, |b| Rect::join2(b, child_bounds)));
712+
}
713+
}
714+
bounds
715+
}
716+
635717
// ---- CSS filters ----
636718

637719
/// Build a Skia `ImageFilter` chain from a CSS `filter`/`backdrop-filter`
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
//! Regression tests for the workstream A (animation & paint) audit findings
2+
//! tracked in issue #220.
3+
4+
use rustmotion_core::css::style::{
5+
Background, BoxShadow, Color as CssColor, CssStyle, Display, FlexDirection, Position,
6+
Size as CSize,
7+
};
8+
use rustmotion_core::css::taffy_bridge::ConversionContext;
9+
use rustmotion_core::css::units::{Length, LengthPercentage as CLP};
10+
use rustmotion_core::engine::box_tree::{BoxKind, BoxNode};
11+
use rustmotion_core::engine::layout_pass::run_layout;
12+
use rustmotion_core::engine::paint_pass::{paint_tree, NoopDispatcher, PaintFrame};
13+
14+
fn test_frame(w: u32, h: u32) -> PaintFrame {
15+
PaintFrame {
16+
time: 0.0,
17+
scenario_time: 0.0,
18+
frame_index: 0,
19+
fps: 30,
20+
video_width: w,
21+
video_height: h,
22+
scene_duration: 1.0,
23+
camera: None,
24+
}
25+
}
26+
27+
fn render_pixels(root: &mut BoxNode, w: u32, h: u32) -> Vec<u8> {
28+
root.assign_ids(0);
29+
let layout = run_layout(root, (w as f32, h as f32), &ConversionContext::default());
30+
let mut surface = skia_safe::surfaces::raster_n32_premul((w as i32, h as i32)).unwrap();
31+
paint_tree(
32+
surface.canvas(),
33+
root,
34+
&layout,
35+
&test_frame(w, h),
36+
&NoopDispatcher,
37+
);
38+
let info = skia_safe::ImageInfo::new(
39+
(w as i32, h as i32),
40+
skia_safe::ColorType::RGBA8888,
41+
skia_safe::AlphaType::Unpremul,
42+
None,
43+
);
44+
let mut buf = vec![0u8; (w * h * 4) as usize];
45+
surface.read_pixels(&info, &mut buf, (w * 4) as usize, (0, 0));
46+
buf
47+
}
48+
49+
fn root_node(w: f32, h: f32, background: &str, children: Vec<BoxNode>) -> BoxNode {
50+
BoxNode {
51+
id: 0,
52+
kind: BoxKind::Container,
53+
css: CssStyle {
54+
display: Some(Display::Flex),
55+
flex_direction: Some(FlexDirection::Column),
56+
width: Some(CSize::Length(CLP::Px(w))),
57+
height: Some(CSize::Length(CLP::Px(h))),
58+
background: Some(Background::Color(CssColor::String(background.to_string()))),
59+
..Default::default()
60+
},
61+
children,
62+
intrinsic: None,
63+
source_path: None,
64+
window: None,
65+
}
66+
}
67+
68+
fn probe(buf: &[u8], w: u32, x: usize, y: usize) -> (u8, u8, u8) {
69+
let i = (y * w as usize + x) * 4;
70+
(buf[i], buf[i + 1], buf[i + 2])
71+
}
72+
73+
// ---- opacity layer must not clip the node's own outset box-shadow ----
74+
75+
fn card_with_shadow(opacity: Option<f32>) -> BoxNode {
76+
let css = CssStyle {
77+
position: Some(Position::Absolute),
78+
left: Some(CLP::Px(50.0)),
79+
top: Some(CLP::Px(50.0)),
80+
width: Some(CSize::Length(CLP::Px(100.0))),
81+
height: Some(CSize::Length(CLP::Px(100.0))),
82+
background: Some(Background::Color(CssColor::String("#ffffff".into()))),
83+
box_shadow: Some(vec![BoxShadow {
84+
offset_x: Length::Px(0.0),
85+
offset_y: Length::Px(0.0),
86+
blur: None,
87+
spread: Some(Length::Px(20.0)),
88+
color: Some(CssColor::String("#ff0000".into())),
89+
inset: None,
90+
}]),
91+
opacity,
92+
..Default::default()
93+
};
94+
BoxNode {
95+
id: 0,
96+
kind: BoxKind::Container,
97+
css,
98+
children: vec![],
99+
intrinsic: None,
100+
source_path: None,
101+
window: None,
102+
}
103+
}
104+
105+
#[test]
106+
fn opacity_layer_does_not_clip_own_outset_box_shadow() {
107+
// 100x100 white card at (50,50) on a 200x200 black canvas, outset
108+
// box-shadow (red, spread 20, blur 0 -> hard-edged halo rect from
109+
// (30,30) to (170,170)). Probe point (100,45) sits in the halo band
110+
// above the card, outside its own border-box. `opacity: 0.999` forces
111+
// the opacity/filter SaveLayerRec open without visibly dimming the
112+
// probed color.
113+
let opaque = {
114+
let mut root = root_node(200.0, 200.0, "#000000", vec![card_with_shadow(None)]);
115+
render_pixels(&mut root, 200, 200)
116+
};
117+
let faded = {
118+
let mut root = root_node(200.0, 200.0, "#000000", vec![card_with_shadow(Some(0.999))]);
119+
render_pixels(&mut root, 200, 200)
120+
};
121+
122+
let above_opaque = probe(&opaque, 200, 100, 45);
123+
assert!(
124+
above_opaque.0 > 200 && above_opaque.1 < 50,
125+
"sanity: shadow halo must be visible without an opacity layer, got {above_opaque:?}"
126+
);
127+
128+
let above_faded = probe(&faded, 200, 100, 45);
129+
assert!(
130+
above_faded.0 > 200 && above_faded.1 < 50,
131+
"an opacity<1 layer must not clip the node's own outset box-shadow, got {above_faded:?}"
132+
);
133+
}

0 commit comments

Comments
 (0)