From 1411c29fbfb6992ebb22e68224f28f68cb93f9d8 Mon Sep 17 00:00:00 2001 From: moppeeer Date: Mon, 12 Oct 2026 18:01:17 +0200 Subject: [PATCH] fix(render): A group's gradient feather masks everything the group draws The DestIn mask now covers the whole render target (and the PDF's exported area), with the gradient kept in item space through the paint transform, since past the gradient's ends the end opacity carries on: a child's long drop shadow or thick stroke is faded, not cut off at a box. feather_bounds counts only shown children and active states, walking nested groups without recursion. Co-Authored-By: Claude Opus 5.5 --- crates/doc/src/item.rs | 49 ++++++++++++++++++++++++---- crates/engine/src/cmd/paths.rs | 58 +++++++++++++++++++++++++++++++--- crates/pdf/src/export.rs | 19 +++-------- crates/render/src/fx.rs | 14 ++++---- crates/render/src/lib.rs | 5 +-- 5 files changed, 109 insertions(+), 36 deletions(-) diff --git a/crates/doc/src/item.rs b/crates/doc/src/item.rs index 9b056b625..68e55a61f 100644 --- a/crates/doc/src/item.rs +++ b/crates/doc/src/item.rs @@ -1101,15 +1101,27 @@ impl Item { self.path.bounds().unwrap_or(Rect::ZERO) } - /// The box a gradient feather spans, in item space: the path's bounds, or a group's objects' - /// (a group has no path of its own). + /// The box a gradient feather spans, in item space: the path's bounds, or a group's visible + /// objects' (a group has no path of its own). Only the active states contribute, including in + /// nested groups. Walk iteratively so deeply nested documents don't exhaust the call stack. pub fn feather_bounds(&self) -> Rect { - if self.is_group() - && let Content::Group { items } = &self.content - { - return items.iter().map(|c| c.bounds()).reduce(|a, b| a.union(b)).unwrap_or(Rect::ZERO); + if !self.is_group() { + return self.inner_bounds(); } - self.inner_bounds() + let mut stack: Vec<_> = self.shown_children().map(|c| (c.as_ref(), c.xf)).collect(); + let mut bounds: Option = None; + while let Some((it, xf)) = stack.pop() { + if it.hidden { + continue; + } + if it.is_group() { + stack.extend(it.shown_children().map(|c| (c.as_ref(), xf * c.xf))); + } else { + let b = xf.transform_rect_bbox(it.inner_bounds()); + bounds = Some(bounds.map_or(b, |have| have.union(b))); + } + } + bounds.unwrap_or(Rect::ZERO) } /// Spread-space geometric bounds (axis-aligned box of the transformed path; groups: union). @@ -1226,6 +1238,29 @@ mod tests { assert_eq!(n, 3); } + #[test] + fn feather_bounds_follow_visible_states_in_nested_groups() { + let a = Item::new(ItemId(1), LayerId(1), Shape::Rectangle, shapes::rectangle(Rect::new(10.0, 20.0, 30.0, 40.0))); + let mut b = a.clone(); + b.id = ItemId(2); + b.xf = Affine::translate((500.0, 500.0)); + let mut state = Item::new(ItemId(3), LayerId(1), Shape::Group, PathData::default()); + state.content = Content::Group { items: vec![Arc::new(a), Arc::new(b)] }; + state.states = vec!["Visible".into(), "Inactive".into()]; + assert_eq!(state.feather_bounds(), Rect::new(10.0, 20.0, 30.0, 40.0)); + state.active_state = 1; + assert_eq!(state.feather_bounds(), Rect::new(510.0, 520.0, 530.0, 540.0)); + state.active_state = 0; + state.xf = Affine::translate((5.0, 10.0)) * Affine::scale(2.0); + let mut hidden = state.clone(); + hidden.id = ItemId(4); + hidden.hidden = true; + hidden.xf = Affine::translate((-1000.0, -1000.0)); + let mut outer = Item::new(ItemId(5), LayerId(1), Shape::Group, PathData::default()); + outer.content = Content::Group { items: vec![Arc::new(state), Arc::new(hidden)] }; + assert_eq!(outer.feather_bounds(), Rect::new(25.0, 50.0, 65.0, 90.0)); + } + #[test] fn serde_roundtrip() { let mut it = Item::new(ItemId(1), LayerId(1), Shape::Oval, shapes::ellipse(Rect::new(0.0, 0.0, 10.0, 10.0))); diff --git a/crates/engine/src/cmd/paths.rs b/crates/engine/src/cmd/paths.rs index 976e892b9..d587526df 100644 --- a/crates/engine/src/cmd/paths.rs +++ b/crates/engine/src/cmd/paths.rs @@ -446,15 +446,17 @@ mod tests { ids.push(id); } let g = s.execute("object.group", &json!({"ids": ids})).unwrap()["id"].as_u64().unwrap(); + s.execute("transform.move", &json!({"ids": [g], "dx": 20, "dy": 30})).unwrap(); // The Gradient Feather tool's drag, then by angle (a group has no path of its own: the - // gradient spans its objects). - for p in [json!({"ids": [g], "from": [100, 150], "to": [300, 150]}), json!({"ids": [g], "angle": 0})] { + // gradient spans its objects). The gradient still follows the translated group even + // though the mask geometry covers the whole output. + for p in [json!({"ids": [g], "from": [120, 180], "to": [320, 180]}), json!({"ids": [g], "angle": 0})] { s.execute("object.gradientFeather", &p).unwrap(); let d = s.doc().unwrap().doc.clone(); let mut rr = designcraft_render::Renderer::new(); rr.threads = 0; let img = rr.render_page(&d, &s.cache, 0, 1.0, false, &Default::default()).unwrap(); - let (l, m, r) = (img.pixel(105, 150)[0], img.pixel(200, 150)[0], img.pixel(295, 150)[0]); + let (l, m, r) = (img.pixel(125, 180)[0], img.pixel(220, 180)[0], img.pixel(315, 180)[0]); assert!(l < 60 && m > l + 40 && r > 220, "{p}: fades left to right on screen: {l} {m} {r}"); let pdf = designcraft_pdf::export_pdf(&d, &s.cache, &Default::default()).unwrap(); let pm = designcraft_render::render_pdf_page(&pdf, 0, 400).unwrap(); @@ -463,11 +465,59 @@ mod tests { let c = pm.data()[y * pm.width() as usize + x]; c.r as u32 + 255 - c.a as u32 }; - let (l, m, r) = (at(105, 150), at(200, 150), at(295, 150)); + let (l, m, r) = (at(125, 180), at(220, 180), at(315, 180)); assert!(l < 60 && m > l + 40 && r > 220, "{p}: fades left to right in PDF: {l} {m} {r}"); } } + #[test] + fn group_gradient_feather_keeps_distant_shadows_and_thick_strokes() { + for shadow in [false, true] { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 400, "height": 400})).unwrap(); + let mut ids = vec![]; + for rect in [[120, 120, 170, 170], [175, 120, 225, 170]] { + let id = s.execute("frame.create", &json!({"rect": rect})).unwrap()["id"].as_u64().unwrap(); + s.execute("object.fill", &json!({"ids": [id], "swatch": "[Black]"})).unwrap(); + if shadow { + s.execute("object.dropShadow", &json!({"ids": [id], "on": true, "distance": 180, "angle": 90, "size": 0, "opacity": 1})).unwrap(); + } else { + s.execute("object.stroke", &json!({"ids": [id], "swatch": "[Black]", "weight": 180})).unwrap(); + } + ids.push(id); + } + let g = s.execute("object.group", &json!({"ids": ids})).unwrap()["id"].as_u64().unwrap(); + // Translate the group so mask geometry and gradient paint exercise distinct spaces. + s.execute("transform.move", &json!({"ids": [g], "dx": 30, "dy": 20})).unwrap(); + let point = if shadow { (175, 345) } else { (175, 60) }; + let sample = |s: &Session| { + let d = &s.doc().unwrap().doc; + let mut rr = designcraft_render::Renderer::new(); + rr.threads = 0; + // Zoomed viewport: the mask must extend over every device pixel. + let img = rr.render_page(d, &s.cache, 0, 2.0, false, &Default::default()).unwrap(); + let screen = img.pixel(point.0 * 2, point.1 * 2)[0]; + let pdf = designcraft_pdf::export_pdf(d, &s.cache, &Default::default()).unwrap(); + let pm = designcraft_render::render_pdf_page(&pdf, 0, 400).unwrap(); + let c = pm.data()[point.1 as usize * pm.width() as usize + point.0 as usize]; + [f32::from(screen), c.r as f32 + 255.0 - c.a as f32] + }; + let before = sample(&s); + // The sample lies past either end of the gradient, and outside the old fixed padding. + s.execute("object.gradientFeather", &json!({"ids": [g], "from": [130, 120], "to": [130, 220], "start": 40, "end": 80})).unwrap(); + let after = sample(&s); + let opacity = if shadow { 0.8 } else { 0.4 }; + for (kind, (before, after)) in ["screen", "PDF"].into_iter().zip(before.into_iter().zip(after)) { + assert!(before < 120.0, "{kind}, shadow={shadow}: sample contains the effect before feathering: {before}"); + let expected = 255.0 - (255.0 - before) * opacity; + assert!( + (after - expected).abs() < 16.0, + "{kind}, shadow={shadow}: end opacity continues past geometry: {after}, expected {expected}" + ); + } + } + } + #[test] fn create_outlines_turns_text_into_paths() { let mut s = Session::new(); diff --git a/crates/pdf/src/export.rs b/crates/pdf/src/export.rs index 3627fd954..983be9433 100644 --- a/crates/pdf/src/export.rs +++ b/crates/pdf/src/export.rs @@ -772,7 +772,7 @@ impl Exporter<'_> { cx: p0.x as f32, cy: p0.y as f32, cr: r, - transform: Transform::identity(), + transform: tf(xf), spread_method: SpreadMethod::Pad, stops, anti_alias: false, @@ -784,30 +784,21 @@ impl Exporter<'_> { y1: p0.y as f32, x2: p1.x as f32, y2: p1.y as f32, - transform: Transform::identity(), + transform: tf(xf), spread_method: SpreadMethod::Pad, stops, anti_alias: false, } .into() }; - // A group's mask covers its objects (it has no path of its own), with room for text and - // strokes past their frames. - let area = if it.is_group() { - let b = it.feather_bounds(); - let pad = 36.0 + 0.25 * b.width().max(b.height()); - b.inflate(pad, pad) - } else { - it.inner_bounds().inflate(it.stroke.extent() + 4.0, it.stroke.extent() + 4.0) - }; + // The gradient keeps its item-space geometry, but its mask covers the complete exported + // area (including bleed), so distant child shadows and thick strokes are never cut off. let mut sb = s.stream_builder(); let mut ms = sb.surface(); - ms.push_transform(&tf(xf)); - if let Some(p) = to_path(&area.to_path(0.1)) { + if let Some(p) = to_path(&self.clip.to_path(0.1)) { ms.set_fill(Some(Fill { paint, opacity: NormalizedF32::ONE, rule: FillRule::NonZero })); ms.draw_path(&p); } - ms.pop(); ms.finish(); let stream = sb.finish(); s.push_mask(krilla::mask::Mask::new(stream, krilla::mask::MaskType::Luminosity)); diff --git a/crates/render/src/fx.rs b/crates/render/src/fx.rs index 0e1ffd565..dab2e3ad0 100644 --- a/crates/render/src/fx.rs +++ b/crates/render/src/fx.rs @@ -64,9 +64,10 @@ fn shadow(sigma: f64, color: peniko::Color) -> Filter { }) } -/// Keep what was drawn in the current layer as much as `it`'s gradient feather says, over `area` -/// (item space; past the gradient's ends its end opacities carry on). -pub(crate) fn gradient_feather_mask(ctx: &mut RenderContext, f: &Frame, it: &Item, xf: Affine, area: Rect) { +/// Keep what was drawn in the current layer as much as `it`'s gradient feather says. Cover the +/// whole viewport: strokes, text and child effects can extend arbitrarily past the geometry, +/// and past the gradient's ends its end opacities carry on. +pub(crate) fn gradient_feather_mask(ctx: &mut RenderContext, f: &Frame, it: &Item, xf: Affine) { let gf = &it.effects.gradient_feather; let (p0, p1) = gf.points(it.feather_bounds()); let stop = |o: f32, a: f32| peniko::ColorStop::from((o, peniko::Color::from_rgba8(0, 0, 0, (a.clamp(0.0, 1.0) * 255.0).round() as u8))); @@ -78,9 +79,9 @@ pub(crate) fn gradient_feather_mask(ctx: &mut RenderContext, f: &Frame, it: &Ite }; ctx.set_transform(Affine::IDENTITY); ctx.push_layer(None, Some(BlendMode::new(Mix::Normal, Compose::DestIn)), None, None, None); - ctx.set_transform(f.view * xf); ctx.set_paint(grad); - ctx.fill_rect(&area); + ctx.set_paint_transform(f.view * xf); + ctx.fill_rect(&Rect::new(0.0, 0.0, f64::from(ctx.width()), f64::from(ctx.height()))); ctx.reset_paint_transform(); ctx.pop_layer(); } @@ -191,8 +192,7 @@ impl Renderer { } } if gf.on { - let area = if xf.determinant().abs() > 1e-12 { xf.inverse().transform_rect_bbox(reach) } else { it.inner_bounds() }; - gradient_feather_mask(ctx, f, it, xf, area.inflate(4.0, 4.0)); + gradient_feather_mask(ctx, f, it, xf); } if gf.on || directional { ctx.pop_layer(); diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index ea942c2c8..cc937a1a2 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -502,10 +502,7 @@ impl Renderer { } } if feather { - // Generous: text and strokes reach past their frames. - let b = it.feather_bounds(); - let pad = 36.0 + 0.25 * b.width().max(b.height()); - fx::gradient_feather_mask(ctx, f, it, xf, b.inflate(pad, pad)); + fx::gradient_feather_mask(ctx, f, it, xf); ctx.pop_layer(); } if layered {