Merge pull request #3725 from Taha-Mahmoodi/fix/brush-dash-fill-opacity

Render: a fill with a dashed brush stroke fades as a whole under object opacity

crates/render/src/brush_fx.rs

@@ -168,6 +168,54 @@ mod tests {
         assert_eq!(again.pixels, img.pixels);
     }
 
+    /// Object opacity fades a brushed stroke's art as a whole, with or without a dash and a fill:
+    /// the same as the object at full opacity inside a group at that opacity.
+    #[test]
+    fn object_opacity_fades_a_brushed_stroke_once() {
+        for (dashed, fill) in [(false, false), (true, false), (false, true), (true, true)] {
+            let node = |d: &mut Document, opacity: f32| {
+                let id = d.alloc_id();
+                let fill = if fill { Paint::solid(Color::rgb(0.9, 0.1, 0.1)) } else { Paint::None };
+                let mut ap = Appearance::basic(fill, Paint::solid(Color::BLACK), 1.0);
+                let st = ap.stroke_mut().unwrap();
+                st.brush = Some("10 pt. Oval".into());
+                if dashed {
+                    st.dash = Some(vectorcraft_doc::Dash { pattern: vec![12.0, 4.0], ..Default::default() });
+                }
+                let mut n = Node::path(id, shapes::rectangle(Rect::new(30.0, 30.0, 170.0, 70.0)), ap);
+                n.opacity = opacity;
+                n
+            };
+            let mut faded = Document::new(200.0, 100.0);
+            let n = node(&mut faded, 0.5);
+            let l = faded.layers[0].id;
+            faded.insert(Some(l), 0, n).unwrap();
+            let mut grouped = Document::new(200.0, 100.0);
+            let n = node(&mut grouped, 1.0);
+            let mut g = Node::group(grouped.alloc_id(), vec![std::sync::Arc::new(n)]);
+            g.opacity = 0.5;
+            let l = grouped.layers[0].id;
+            grouped.insert(Some(l), 0, g).unwrap();
+            let (a, b) = (render(&mut Renderer::new(), &faded), render(&mut Renderer::new(), &grouped));
+            assert!(
+                dark(
+                    render(&mut Renderer::new(), &{
+                        let mut d = Document::new(200.0, 100.0);
+                        let n = node(&mut d, 1.0);
+                        let l = d.layers[0].id;
+                        d.insert(Some(l), 0, n).unwrap();
+                        d
+                    })
+                    .pixel(30, 50)
+                ),
+                "the brush paints the edge"
+            );
+            let off =
+                a.pixels.chunks(4).zip(b.pixels.chunks(4)).filter(|(x, y)| x.iter().zip(*y).any(|(p, q)| (*p as i32 - *q as i32).abs() > 2)).count();
+            assert_eq!(off, 0, "dashed {dashed}, fill {fill}: {off} pixels differ from the object faded as a whole");
+        }
+    }
+
     #[test]
     fn unknown_brush_falls_back_to_plain_stroke_and_scatter_renders() {
         let mut r = Renderer::new();

crates/render/src/lib.rs

@@ -2247,18 +2247,18 @@ fn single_plain_fill(n: &Node) -> bool {
 
 /// Number of visible painted fill/stroke items (opacity folding is exact only for one). A dashed
 /// stroke is one paint: its dashes are outlined into one path and filled once, so overlapping
-/// dashes don't darken where they overlap. A dashed brush stroke isn't: its pieces are drawn one
-/// by one.
+/// dashes don't darken where they overlap. A dashed brush stroke draws its pieces one by one, so
+/// it takes the layered path, with or without other paints.
 fn painted_items(n: &Node) -> usize {
     n.appearance
         .items
         .iter()
         .filter(|i| match i {
             AppearanceItem::Fill(f) => f.visible && !f.paint.is_none() && matches!(f.paint, vectorcraft_color::Paint::Solid { .. }),
-            AppearanceItem::Stroke(s) => s.visible && !s.paint.is_none() && s.width > 0.0 && matches!(s.paint, vectorcraft_color::Paint::Solid { .. }) && (s.dash.is_none() || s.brush.is_none()),
+            AppearanceItem::Stroke(s) => s.visible && !s.paint.is_none() && s.width > 0.0 && matches!(s.paint, vectorcraft_color::Paint::Solid { .. }),
         })
         .count()
-        .max(if n.appearance.items.iter().any(|i| matches!(i, AppearanceItem::Fill(f) if f.visible && !matches!(f.paint, vectorcraft_color::Paint::Solid { .. } | vectorcraft_color::Paint::None)) || matches!(i, AppearanceItem::Stroke(s) if s.visible && !matches!(s.paint, vectorcraft_color::Paint::Solid { .. } | vectorcraft_color::Paint::None))) { 2 } else { 0 })
+        .max(if n.appearance.items.iter().any(|i| matches!(i, AppearanceItem::Fill(f) if f.visible && !matches!(f.paint, vectorcraft_color::Paint::Solid { .. } | vectorcraft_color::Paint::None)) || matches!(i, AppearanceItem::Stroke(s) if s.visible && (!matches!(s.paint, vectorcraft_color::Paint::Solid { .. } | vectorcraft_color::Paint::None) || (s.dash.is_some() && s.brush.is_some() && !s.paint.is_none())))) { 2 } else { 0 })
 }
 
 /// Whether `st` is one of `n`'s own strokes (the strokes of the brush art drawn for `n` are not).