From 34427c8a280a3684d47c3395b6dd8245113347c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Wed, 23 Sep 2026 12:45:02 +0200 Subject: [PATCH] :bug: Fix borders not fully rendered next to masked groups (#11807) A masked group now closes its focus scope on the mask pass, pops the nested stacks on the content pass only, and draws its exit strokes on the content pass. The focus depth also resets when each tile's walk starts. Closes #11805 AI-assisted-by: claude-opus-5 --- render-wasm/src/render.rs | 178 +++++++++++++++++++++++++++++++++++--- 1 file changed, 164 insertions(+), 14 deletions(-) diff --git a/render-wasm/src/render.rs b/render-wasm/src/render.rs index 4743bd5124..cd211bff1f 100644 --- a/render-wasm/src/render.rs +++ b/render-wasm/src/render.rs @@ -349,6 +349,26 @@ fn sort_z_index(tree: ShapesPoolRef, element: &Shape, children_ids: Vec) - } } +/// Whether this `render_shape_exit` pass closes the shape's focus scope. A +/// masked group keeps it open across the mask pass. +fn exit_closes_focus_scope(element: &Shape, visited_mask: bool) -> bool { + visited_mask || !element.is_masked_group() +} + +/// Whether this pass pops the nested fill/blur/shadow stacks, pushed once on +/// enter. The mask pass skips them. +fn exit_pops_nested_stacks(element: &Shape, visited_mask: bool) -> bool { + !(visited_mask && element.is_masked_group()) +} + +/// Whether this pass draws strokes over the children: all of them for a +/// clipped frame, only the inner one otherwise. +fn exit_draws_strokes_over_children(element: &Shape, visited_mask: bool) -> bool { + !visited_mask + && (element.clip() + || (matches!(element.shape_type, Type::Frame(_)) && element.has_inner_stroke())) +} + struct RenderStats { pub counts: HashMap, } @@ -3050,22 +3070,21 @@ impl RenderState { } } - match element.shape_type { - Type::Frame(_) | Type::Group(_) => { - self.nested_fills.pop(); - self.nested_blurs.pop(); - self.nested_shadows.pop(); + if exit_pops_nested_stacks(element, visited_mask) { + match element.shape_type { + Type::Frame(_) | Type::Group(_) => { + self.nested_fills.pop(); + self.nested_blurs.pop(); + self.nested_shadows.pop(); + } + _ => {} } - _ => {} } - // Strokes are drawn over children for clipped frames (all strokes), and for non-clipped - // frames with inner strokes (inner strokes only — non-inner were rendered before children). - // Skip when focus mode excludes this subtree (focus_mode.exit runs after this, so - // is_active() still reflects this element's focus state here). - let needs_exit_strokes = self.focus_mode.is_active() - && (element.clip() - || (matches!(element.shape_type, Type::Frame(_)) && element.has_inner_stroke())); + // Skip when focus mode excludes this subtree (the focus scope closes after this, + // so is_active() still reflects this element's focus state here). + let needs_exit_strokes = + self.focus_mode.is_active() && exit_draws_strokes_over_children(element, visited_mask); if needs_exit_strokes { let mut element_strokes: Cow = Cow::Borrowed(element); @@ -3117,7 +3136,9 @@ impl RenderState { self.surfaces.canvas(target_surface).restore(); } - self.focus_mode.exit(&element.id); + if exit_closes_focus_scope(element, visited_mask) { + self.focus_mode.exit(&element.id); + } Ok(()) } @@ -4318,6 +4339,9 @@ impl RenderState { self.current_tile_had_shapes = false; self.tile_atlas_flushed = false; self.drop_shadows_ops_warmed = false; + // Every tile walks from the root shapes with depth zero. Only a + // drained walk reaches here; an interrupted one returns Partial. + self.focus_mode.reset(); let viewer_masked_pass = self.viewer_masked_pass(); let current_scale = self.get_scale(); @@ -4842,6 +4866,132 @@ mod tests { shape } + fn group(masked: bool) -> Shape { + let mut shape = Shape::new(Uuid::new_v4()); + shape.set_shape_type(Type::Group(Group { masked })); + shape.set_selrect(10.0, 20.0, 110.0, 120.0); + shape + } + + fn rect() -> Shape { + let mut shape = Shape::new(Uuid::new_v4()); + shape.set_shape_type(Type::Rect(RectType::default())); + shape.set_selrect(10.0, 20.0, 110.0, 120.0); + shape + } + + /// Both exit passes of a masked group, in walker order. + fn exit_masked_group(focus: &mut FocusMode, group: &Shape) { + for visited_mask in [false, true] { + if exit_closes_focus_scope(group, visited_mask) { + focus.exit(&group.id); + } + } + } + + #[test] + fn a_masked_group_closes_its_focus_scope_on_the_mask_pass_only() { + let masked = group(true); + + assert!(!exit_closes_focus_scope(&masked, false)); + assert!(exit_closes_focus_scope(&masked, true)); + } + + #[test] + fn every_other_shape_closes_its_focus_scope_on_its_single_exit() { + for shape in [frame(true), frame(false), group(false), rect()] { + assert!( + exit_closes_focus_scope(&shape, false), + "{:?} exits once and must close its scope there", + shape.shape_type + ); + } + } + + /// The mask shape renders between the two exit passes, so it must still be + /// inside the group's focus scope. + #[test] + fn a_masked_group_leaves_the_focus_depth_as_it_found_it() { + let masked = group(true); + let mut focus = FocusMode::new(); + + focus.enter(&masked.id); + if exit_closes_focus_scope(&masked, false) { + focus.exit(&masked.id); + } + assert!(focus.is_active(), "the mask pass renders inside the group"); + + if exit_closes_focus_scope(&masked, true) { + focus.exit(&masked.id); + } + assert!(!focus.is_active()); + } + + /// GH-11805: a clipped frame paints its border on exit, gated on the focus + /// depth, so a masked group sibling must not eat a level of it. + #[test] + fn a_frame_keeps_its_focus_scope_after_a_masked_group_sibling() { + let board = frame(true); + let first = (frame(true), group(true)); + let second = (frame(true), group(true)); + + let mut focus = FocusMode::new(); + focus.enter(&board.id); + + for (container, masked) in [&first, &second] { + focus.enter(&container.id); + focus.enter(&masked.id); + exit_masked_group(&mut focus, masked); + + assert!( + focus.is_active(), + "the frame must still be in focus to draw its border" + ); + focus.exit(&container.id); + } + + assert!(focus.is_active(), "the board is still open"); + focus.exit(&board.id); + assert!(!focus.is_active()); + } + + /// The mask shape is an alpha silhouette and must not inherit the group's + /// fills, so the content pass is the one that pops. + #[test] + fn a_masked_group_pops_the_nested_stacks_on_the_content_pass_only() { + let masked = group(true); + + assert!(exit_pops_nested_stacks(&masked, false)); + assert!(!exit_pops_nested_stacks(&masked, true)); + } + + #[test] + fn a_masked_group_leaves_the_nested_stacks_as_it_found_them() { + let board = frame(true); + let masked = group(true); + let mut nested_fills: Vec> = vec![]; + + nested_fills.push(board.fills.to_vec()); + let depth_inside_the_board = nested_fills.len(); + + nested_fills.push(masked.fills.to_vec()); + for visited_mask in [false, true] { + if exit_pops_nested_stacks(&masked, visited_mask) { + nested_fills.pop(); + } + } + + assert_eq!(nested_fills.len(), depth_inside_the_board); + } + + #[test] + fn only_the_content_pass_draws_strokes_over_children() { + let masked = group(true); + + assert!(exit_draws_strokes_over_children(&masked, false)); + assert!(!exit_draws_strokes_over_children(&masked, true)); + } + #[test] fn append_child_clip_leaves_the_stack_alone_for_a_non_clipping_shape() { let shape = frame(false);