From 63baf861526edb2fc5054daa646d7e0dd39b69b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elena=20Torr=C3=B3?= Date: Fri, 25 Sep 2026 11:50:19 +0200 Subject: [PATCH] :bug: Fix groups, masks and booleans drop shadow cases (#11911) * :bug: Fix blocky and black drop shadows on masked groups * :bug: Fix drop shadow spread on paths, bools and circles --- render-wasm/src/render.rs | 33 ++++++-- render-wasm/src/render/svg/mod.rs | 20 ++++- ...drop_spread_uses_geometric_silhouette.snap | 3 +- ...af_drop_spread_strokes_the_silhouette.snap | 11 +++ render-wasm/src/render/svg/tests.rs | 34 ++++++++- render-wasm/src/shapes.rs | 59 +++++++++++++- render-wasm/src/shapes/shadows.rs | 76 +++++++++++++++++++ render-wasm/src/shapes/strokes.rs | 31 ++++++++ 8 files changed, 250 insertions(+), 17 deletions(-) create mode 100644 render-wasm/src/render/svg/snapshots/render_wasm__render__svg__tests__path_leaf_drop_spread_strokes_the_silhouette.snap diff --git a/render-wasm/src/render.rs b/render-wasm/src/render.rs index ce0b71369b..999972bfcd 100644 --- a/render-wasm/src/render.rs +++ b/render-wasm/src/render.rs @@ -431,6 +431,9 @@ pub(crate) struct RenderState { pub nested_fills: Vec>, pub nested_blurs: Vec>, // FIXME: why is this an option? pub nested_shadows: Vec>, + /// Cumulative [`Shape::masked_group_filter_reach`] of the masked groups + /// being walked: their children cast into tiles they don't touch. + masked_group_reach: Vec, pub show_grid: Option, pub focus_mode: FocusMode, /// Viewer-only whitelist for fixed-scroll layer passes. @@ -656,6 +659,7 @@ impl RenderState { nested_fills: vec![], nested_blurs: vec![], nested_shadows: vec![], + masked_group_reach: vec![], show_grid: None, focus_mode: FocusMode::new(), include_filter: None, @@ -2435,6 +2439,7 @@ impl RenderState { self.nested_fills.clear(); self.nested_blurs.clear(); self.nested_shadows.clear(); + self.masked_group_reach.clear(); // reorder by distance to the center. self.current_tile = None; @@ -2731,6 +2736,7 @@ impl RenderState { let saved_nested_fills = std::mem::take(&mut self.nested_fills); let saved_nested_blurs = std::mem::take(&mut self.nested_blurs); let saved_nested_shadows = std::mem::take(&mut self.nested_shadows); + let saved_masked_group_reach = std::mem::take(&mut self.masked_group_reach); let saved_ignore_nested_blurs = self.ignore_nested_blurs; let saved_preview_mode = self.preview_mode; @@ -2802,6 +2808,7 @@ impl RenderState { self.nested_fills = saved_nested_fills; self.nested_blurs = saved_nested_blurs; self.nested_shadows = saved_nested_shadows; + self.masked_group_reach = saved_masked_group_reach; self.ignore_nested_blurs = saved_ignore_nested_blurs; self.preview_mode = saved_preview_mode; @@ -2954,6 +2961,10 @@ impl RenderState { } if group.masked { + let reach = self.masked_group_reach.last().copied().unwrap_or(0.0) + + element.masked_group_filter_reach(); + self.masked_group_reach.push(reach); + // A masked group's blur and shadows are applied as a single // image filter over the whole masked result. let scale = self.get_scale(); @@ -3052,6 +3063,7 @@ impl RenderState { // the blend mode 'destination-in') the content // of the group and the mask. if group.masked { + self.masked_group_reach.pop(); self.pending_nodes.push(NodeRenderState { id: element.id, visited_children: true, @@ -3530,6 +3542,13 @@ impl RenderState { // (which defers strokes to render_shape_exit for clipped frames). plain_shape_mut.clip_content = false; + let spread_outset = if shadow.spread > 0.0 && shape.spreads_through_strokes() { + plain_shape_mut.apply_shadow_spread(shadow.spread); + None + } else { + Some(shadow.spread) + }; + let Some(drop_filter) = transformed_shadow.get_drop_shadow_filter() else { return Ok(()); }; @@ -3561,7 +3580,7 @@ impl RenderState { false, Some(shadow.offset), None, - Some(shadow.spread), + spread_outset, target_surface, false, ) @@ -3605,7 +3624,7 @@ impl RenderState { false, Some(shadow.offset), // Offset is geometric None, - Some(shadow.spread), + spread_outset, target_surface, false, ) @@ -3669,7 +3688,7 @@ impl RenderState { false, Some(shadow.offset), // Offset is geometric None, - Some(shadow.spread), + spread_outset, target_surface, false, ) @@ -3944,17 +3963,21 @@ impl RenderState { ); let has_effects = transformed_element.has_effects_that_extend_bounds(); + let area = match self.masked_group_reach.last() { + Some(&reach) => self.render_area_with_margins.with_outset((reach, reach)), + None => self.render_area_with_margins, + }; let is_visible = export || mask || if is_container || has_effects { let element_extrect = extrect.get_or_insert_with(|| transformed_element.extrect(tree, scale)); - element_extrect.intersects(self.render_area_with_margins) + element_extrect.intersects(area) && !transformed_element.visually_insignificant(scale, tree) } else { let selrect = transformed_element.selrect(); - selrect.intersects(self.render_area_with_margins) + selrect.intersects(area) && !transformed_element.visually_insignificant(scale, tree) }; diff --git a/render-wasm/src/render/svg/mod.rs b/render-wasm/src/render/svg/mod.rs index 54e5f91588..6fb7207c50 100644 --- a/render-wasm/src/render/svg/mod.rs +++ b/render-wasm/src/render/svg/mod.rs @@ -1,5 +1,6 @@ use skia_safe::{self as skia}; +use std::borrow::Cow; use std::collections::HashSet; use crate::error::Result; @@ -287,7 +288,13 @@ fn render_leaf_geometry( ) -> Result<()> { let spread = builder.silhouette_spread; // Spread outsets fills only (GPU). Rect/Frame strokes ignore outset. - let fill_shape = shape_with_selrect_outset(element, spread); + // Paths and circles spread through strokes instead (see below). + let fill_outset = if element.spreads_through_strokes() { + 0.0 + } else { + spread + }; + let fill_shape = shape_with_selrect_outset(element, fill_outset); // Always from the original element (not outset selrect) so the pivot // matches content; offset comes from the silhouette pass. let draw_matrix = builder.silhouette_draw_matrix(element); @@ -313,13 +320,18 @@ fn render_leaf_geometry( )?; // Stroke geometry stays on the original selrect (GPU Rect/Frame - // drop-shadow outset is a no-op for single strokes). - let visible_strokes: Vec<_> = element.visible_strokes().collect(); + // drop-shadow outset is a no-op for single strokes). Paths and circles + // spread through widened strokes, as on the GPU. + let mut stroke_shape = Cow::Borrowed(element); + if spread > 0.0 && element.spreads_through_strokes() { + stroke_shape.to_mut().apply_shadow_spread(spread); + } + let visible_strokes: Vec<_> = stroke_shape.visible_strokes().collect(); if !visible_strokes.is_empty() { emit_strokes( builder, shared, - element, + &stroke_shape, &visible_strokes, scale, Some(draw_matrix), diff --git a/render-wasm/src/render/svg/snapshots/render_wasm__render__svg__tests__masked_leaf_drop_spread_uses_geometric_silhouette.snap b/render-wasm/src/render/svg/snapshots/render_wasm__render__svg__tests__masked_leaf_drop_spread_uses_geometric_silhouette.snap index 3a78b9ef21..f6a5cecf90 100644 --- a/render-wasm/src/render/svg/snapshots/render_wasm__render__svg__tests__masked_leaf_drop_spread_uses_geometric_silhouette.snap +++ b/render-wasm/src/render/svg/snapshots/render_wasm__render__svg__tests__masked_leaf_drop_spread_uses_geometric_silhouette.snap @@ -6,7 +6,8 @@ expression: svg - + + diff --git a/render-wasm/src/render/svg/snapshots/render_wasm__render__svg__tests__path_leaf_drop_spread_strokes_the_silhouette.snap b/render-wasm/src/render/svg/snapshots/render_wasm__render__svg__tests__path_leaf_drop_spread_strokes_the_silhouette.snap new file mode 100644 index 0000000000..8dce6d8b8e --- /dev/null +++ b/render-wasm/src/render/svg/snapshots/render_wasm__render__svg__tests__path_leaf_drop_spread_strokes_the_silhouette.snap @@ -0,0 +1,11 @@ +--- +source: src/render/svg/tests.rs +expression: svg +--- + + + + + + + \ No newline at end of file diff --git a/render-wasm/src/render/svg/tests.rs b/render-wasm/src/render/svg/tests.rs index d62d5c22ed..5643615e85 100644 --- a/render-wasm/src/render/svg/tests.rs +++ b/render-wasm/src/render/svg/tests.rs @@ -1423,6 +1423,33 @@ fn exports_masked_group_as_alpha_mask() { insta::assert_snapshot!(svg); } +#[test] +fn path_leaf_drop_spread_strokes_the_silhouette() { + // Paths had no spread at all: the selrect outset is a no-op for them. + let mut pool = ShapesPool::new(); + let id = uid(1); + add_empty_fill_closed_path(&mut pool, id, Uuid::nil(), (0.0, 0.0, 100.0, 100.0)); + let shape = pool.get_mut(&id).unwrap(); + shape.set_fills(vec![Fill::Solid(SolidColor(skia::Color::from_rgb( + 61, 123, 255, + )))]); + shape.add_shadow(Shadow::new( + skia::Color::from_rgb(255, 0, 0), + 0.0, + 20.0, + (0.0, 0.0), + ShadowStyle::Drop, + false, + )); + + let svg = render(&pool, id); + assert!( + !svg.contains("feMorphology"), + "leaf drop spread must stay geometric: {svg}" + ); + insta::assert_snapshot!(svg); +} + #[test] fn masked_leaf_drop_spread_uses_geometric_silhouette() { // Case 11 style: masked group + circle content with drop spread 16 blur 0. @@ -1474,7 +1501,7 @@ fn masked_leaf_drop_spread_uses_geometric_silhouette() { !svg.contains("feMorphology"), "leaf drop spread must not use feMorphology: {svg}" ); - // 100×100 circle + 2×16 spread → 132×132 silhouette ellipse. + // 100×100 circle + 32px center stroke → 132×132 silhouette. let filter_open = svg.find("filter=\"url(#fx").expect("drop filter"); let filter_close = svg[filter_open..] .find("") @@ -1482,9 +1509,8 @@ fn masked_leaf_drop_spread_uses_geometric_silhouette() { .expect("silhouette group close"); let silhouette = &svg[filter_open..=filter_close]; assert!( - silhouette.contains(" bool { + matches!( + self.shape_type, + Type::Path(_) | Type::Bool(_) | Type::Circle + ) + } + + /// Offsets the shape outline by `spread` with the shape's own joins: + /// every stroke widens by `spread` on each side and a black `2·spread` + /// center stroke grows the fill. + pub fn apply_shadow_spread(&mut self, spread: f32) { + let is_open = self.is_open(); + for stroke in self.strokes.iter_mut() { + stroke.grow_by_spread(spread, is_open); + } + let mut outline = + Stroke::new_center_stroke(2.0 * spread, StrokeStyle::Solid, None, None, None, None); + outline.fill = Fill::Solid(SolidColor(skia::Color::BLACK)); + self.add_stroke(outline); + } + pub fn set_last_stroke_widths(&mut self, widths: [f32; 4]) -> Result<(), String> { let stroke = self.strokes.last_mut().ok_or("Shape has no strokes")?; stroke.widths = Some(widths); @@ -974,12 +997,22 @@ impl Shape { } fn apply_shadow_bounds(&self, bounds: Bounds) -> Bounds { + // A path spread is a mitered stroke (`apply_shadow_spread`): its tips + // reach up to Skia's miter limit (4) half-widths from the vertex. + const MITER_LIMIT: f32 = 4.0; + let mitered = matches!(self.shape_type, Type::Path(_) | Type::Bool(_)); + let max_stroke = Stroke::max_bounds_width(self.strokes.iter(), self.is_open()); + let mut rect = bounds.to_rect(); for shadow in self.shadows_visible() { if !shadow.hidden() { if let Some(filter) = shadow.get_drop_shadow_filter() { - let shadow_bounds = filter.compute_fast_bounds(rect); - rect.join(shadow_bounds); + let mut source = rect; + if mitered && shadow.spread > 0.0 { + let tip = (MITER_LIMIT - 1.0) * (max_stroke + shadow.spread); + source.outset((tip, tip)); + } + rect.join(filter.compute_fast_bounds(source)); } } } @@ -1882,6 +1915,26 @@ impl Shape { } } + /// How far the masked-group layer filter reaches past the content, in + /// document units: the widest drop shadow plus the layer blur. + pub fn masked_group_filter_reach(&self) -> f32 { + let reach = |filter: Option| { + filter.map_or(0.0, |f| { + let r = f.compute_fast_bounds(math::Rect::default()); + (-r.left).max(-r.top).max(r.right).max(r.bottom) + }) + }; + let shadows = self + .drop_shadows_visible() + .map(|shadow| reach(shadow.get_drop_shadow_filter())) + .fold(0.0, f32::max); + let blur = self.masked_group_layer_blur().map_or(0.0, |blur| { + let sigma = radius_to_sigma(blur.value); + reach(skia::image_filters::blur((sigma, sigma), None, None, None)) + }); + shadows + blur + } + /// Shadows of the given style that the masked-group layer filter must /// carry, bottom-most first, already converted to device space. /// @@ -1936,7 +1989,7 @@ impl Shape { if !skip_shadows { for shadow in self.masked_group_layer_shadows(scale, ShadowStyle::Drop) { - layers.push(shadow.get_drop_shadow_filter()); + layers.push(shadow.get_layer_drop_shadow_filter()); } } diff --git a/render-wasm/src/shapes/shadows.rs b/render-wasm/src/shapes/shadows.rs index d77ad8bdd9..11a0e13295 100644 --- a/render-wasm/src/shapes/shadows.rs +++ b/render-wasm/src/shapes/shadows.rs @@ -104,6 +104,45 @@ impl Shadow { filter } + /// Same shadow as [`Self::get_drop_shadow_filter`], for a `merge` input. + /// Skia defers color filters and offsets, and `merge` can draw them + /// unresolved in 8×8 cells (untinted source, or opaque black). A `blend` + /// always renders, so the tint is a SrcIn blend of the color over the + /// offset source. + pub fn get_layer_drop_shadow_filter(&self) -> Option { + let color = image_filters::shader(skia::shaders::color(self.color), None); + let offset = image_filters::offset((self.offset.0, self.offset.1), None, None); + let mut filter = image_filters::blend(skia::BlendMode::SrcIn, offset, color, None); + + // Spread before blur, as in CSS and SVG. Dilating the blur instead + // works on Skia's downscaled blur output and comes out blocky. + if self.spread > 0. { + // Masked-group shadows are already in device space. + filter = Self::chained_dilate(self.spread, filter); + } + + let sigma = radius_to_sigma(self.blur); + if sigma > 0.0 { + filter = image_filters::blur((sigma, sigma), None, filter, None); + } + + filter + } + + /// Square dilate by a device-space `radius`, split into steps Skia will + /// not clamp. Skia caps a single morphology radius at 256 px; square + /// dilates add up exactly, so chaining keeps the full spread. + fn chained_dilate(radius: f32, input: Option) -> Option { + const MAX_RADIUS: f32 = 255.0; + let steps = (radius / MAX_RADIUS).ceil().max(1.0) as usize; + let step = radius / steps as f32; + let mut filter = input; + for _ in 0..steps { + filter = image_filters::dilate((step, step), filter, None); + } + filter + } + pub fn get_inner_shadow_paint( &self, antialias: bool, @@ -239,6 +278,43 @@ mod tests { ); } + #[test] + fn chained_dilate_reaches_the_full_radius() { + let rect = skia::Rect::from_xywh(0.0, 0.0, 10.0, 10.0); + // 600 px is three steps of 200 px. + let filter = Shadow::chained_dilate(600.0, None).expect("dilate"); + let bounds = filter.compute_fast_bounds(rect); + assert!((bounds.left + 600.0).abs() < 0.01); + assert!((bounds.right - 610.0).abs() < 0.01); + } + + #[test] + fn layer_drop_shadow_filter_matches_drop_shadow_bounds() { + let rect = skia::Rect::from_xywh(0.0, 0.0, 100.0, 50.0); + for (blur, spread, ox, oy) in [ + (0.0, 0.0, 4.0, 4.0), + (4.0, 0.0, 4.0, 4.0), + (12.0, 6.0, -3.0, 8.0), + ] { + let s = shadow(blur, spread, ox, oy); + let expected = s + .get_drop_shadow_filter() + .expect("drop shadow filter") + .compute_fast_bounds(rect); + let actual = s + .get_layer_drop_shadow_filter() + .expect("layer drop shadow filter") + .compute_fast_bounds(rect); + assert!( + (expected.left - actual.left).abs() < 0.01 + && (expected.top - actual.top).abs() < 0.01 + && (expected.right - actual.right).abs() < 0.01 + && (expected.bottom - actual.bottom).abs() < 0.01, + "bounds differ for blur {blur}: {expected:?} vs {actual:?}" + ); + } + } + #[test] fn overview_scale_vs_extent() { // At 0.038 even blur 50 is only ~1.9px — below leaf floor. diff --git a/render-wasm/src/shapes/strokes.rs b/render-wasm/src/shapes/strokes.rs index 6602af8647..e5df8fd049 100644 --- a/render-wasm/src/shapes/strokes.rs +++ b/render-wasm/src/shapes/strokes.rs @@ -86,6 +86,20 @@ impl Stroke { } } + /// Widens the band by `spread` on each side. Inner and Outer bands only + /// grow away from the path; the `2·spread` center stroke added by + /// `Shape::apply_shadow_spread` covers the other side. + pub fn grow_by_spread(&mut self, spread: f32, is_open: bool) { + let growth = match self.render_kind(is_open) { + StrokeKind::Center => 2.0 * spread, + StrokeKind::Inner | StrokeKind::Outer => spread, + }; + self.width += growth; + if let Some(widths) = self.widths.as_mut() { + widths.iter_mut().for_each(|w| *w += growth); + } + } + /// The widest side of the stroke: the uniform `width` unless per-side /// widths are set, in which case the maximum of the four sides. pub fn max_width(&self) -> f32 { @@ -636,6 +650,23 @@ mod tests { stroke } + #[test] + fn grow_by_spread_moves_both_band_edges_by_the_spread() { + let mut center = Stroke::new_center_stroke(4.0, StrokeStyle::Solid, None, None, None, None); + center.grow_by_spread(3.0, false); + assert_eq!(center.width, 10.0); + + let mut inner = stroke_with_widths(Some([2.0, 0.0, 2.0, 0.0])); + inner.grow_by_spread(3.0, false); + assert_eq!(inner.width, 5.0); + assert_eq!(inner.widths, Some([5.0, 3.0, 5.0, 3.0])); + + // Open paths render every stroke centered. + let mut open_inner = stroke_with_widths(None); + open_inner.grow_by_spread(3.0, true); + assert_eq!(open_inner.width, 8.0); + } + #[test] fn per_side_profile_needs_two_strokes_to_miter_against() { let one = [stroke_with_widths(Some([10.0, 0.0, 0.0, 0.0]))];