Remove FillPaint and StrokePaint filter inputs support.

Closes linebender#631
zimond · May 31, 2023 · 6be2f2d · 6be2f2d
1 parent cad8220
commit 6be2f2d
Show file tree

Hide file tree

Showing 16 changed files with 59 additions and 284 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 This changelog also contains important changes in dependencies.
 
 ## [Unreleased]
+### Removed
+- `FillPaint` and `StrokePaint` filter inputs support.
+  It's a mostly undocumented SVG feature that no one supports and no one uses.
+  And it was adding a significant complexity to the codebase.
+- `usvg::filter::Filter::fill_paint` and `usvg::filter::Filter::stroke_paint`.
+- `BackgroundImage`, `BackgroundAlpha`, `FillPaint` and `StrokePaint` from `usvg::filter::Input`.
+- `usvg::Group::filter_fill_paint` and `usvg::Group::filter_stroke_paint`.
 
 ## [0.34.1] - 2023-05-28
 ### Fixed

diff --git a/crates/resvg/src/filter/mod.rs b/crates/resvg/src/filter/mod.rs
@@ -458,31 +458,13 @@ impl Image {
     }
 }
 
-struct FilterInputs<'a> {
-    source: &'a mut tiny_skia::Pixmap,
-    fill_paint: Option<&'a tiny_skia::Pixmap>,
-    stroke_paint: Option<&'a tiny_skia::Pixmap>,
-}
-
 struct FilterResult {
     name: String,
     image: Image,
 }
 
-pub fn apply(
-    filter: &Filter,
-    ts: tiny_skia::Transform,
-    fill_paint: Option<&tiny_skia::Pixmap>,
-    stroke_paint: Option<&tiny_skia::Pixmap>,
-    source: &mut tiny_skia::Pixmap,
-) {
-    let inputs = FilterInputs {
-        source,
-        fill_paint,
-        stroke_paint,
-    };
-
-    let result = apply_inner(filter, &inputs, ts);
+pub fn apply(filter: &Filter, ts: tiny_skia::Transform, source: &mut tiny_skia::Pixmap) {
+    let result = apply_inner(filter, ts, source);
     let result = result.and_then(|image| apply_to_canvas(image, source));
 
     // Clear on error.
@@ -501,8 +483,8 @@ pub fn apply(
 
 fn apply_inner(
     filter: &Filter,
-    inputs: &FilterInputs,
     ts: usvg::Transform,
+    source: &mut tiny_skia::Pixmap,
 ) -> Result<Image, Error> {
     let mut results: Vec<FilterResult> = Vec::new();
 
@@ -531,62 +513,62 @@ fn apply_inner(
 
         let mut result = match primitive.kind {
             usvg::filter::Kind::Blend(ref fe) => {
-                let input1 = get_input(&fe.input1, region, inputs, &results)?;
-                let input2 = get_input(&fe.input2, region, inputs, &results)?;
+                let input1 = get_input(&fe.input1, region, source, &results)?;
+                let input2 = get_input(&fe.input2, region, source, &results)?;
                 apply_blend(fe, cs, region, input1, input2)
             }
             usvg::filter::Kind::DropShadow(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_drop_shadow(fe, cs, ts, input)
             }
             usvg::filter::Kind::Flood(ref fe) => apply_flood(fe, region),
             usvg::filter::Kind::GaussianBlur(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_blur(fe, cs, ts, input)
             }
             usvg::filter::Kind::Offset(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_offset(fe, ts, input)
             }
             usvg::filter::Kind::Composite(ref fe) => {
-                let input1 = get_input(&fe.input1, region, inputs, &results)?;
-                let input2 = get_input(&fe.input2, region, inputs, &results)?;
+                let input1 = get_input(&fe.input1, region, source, &results)?;
+                let input2 = get_input(&fe.input2, region, source, &results)?;
                 apply_composite(fe, cs, region, input1, input2)
             }
-            usvg::filter::Kind::Merge(ref fe) => apply_merge(fe, cs, region, inputs, &results),
+            usvg::filter::Kind::Merge(ref fe) => apply_merge(fe, cs, region, source, &results),
             usvg::filter::Kind::Tile(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_tile(input, region)
             }
             usvg::filter::Kind::Image(ref fe) => apply_image(fe, region, subregion, ts),
             usvg::filter::Kind::ComponentTransfer(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_component_transfer(fe, cs, input)
             }
             usvg::filter::Kind::ColorMatrix(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_color_matrix(fe, cs, input)
             }
             usvg::filter::Kind::ConvolveMatrix(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_convolve_matrix(fe, cs, input)
             }
             usvg::filter::Kind::Morphology(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_morphology(fe, cs, ts, input)
             }
             usvg::filter::Kind::DisplacementMap(ref fe) => {
-                let input1 = get_input(&fe.input1, region, inputs, &results)?;
-                let input2 = get_input(&fe.input2, region, inputs, &results)?;
+                let input1 = get_input(&fe.input1, region, source, &results)?;
+                let input2 = get_input(&fe.input2, region, source, &results)?;
                 apply_displacement_map(fe, region, cs, ts, input1, input2)
             }
             usvg::filter::Kind::Turbulence(ref fe) => apply_turbulence(fe, region, cs, ts),
             usvg::filter::Kind::DiffuseLighting(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_diffuse_lighting(fe, region, cs, ts, input)
             }
             usvg::filter::Kind::SpecularLighting(ref fe) => {
-                let input = get_input(&fe.input, region, inputs, &results)?;
+                let input = get_input(&fe.input, region, source, &results)?;
                 apply_specular_lighting(fe, region, cs, ts, input)
             }
         }?;
@@ -748,41 +730,12 @@ fn calc_subregion(
 fn get_input(
     input: &usvg::filter::Input,
     region: IntRect,
-    inputs: &FilterInputs,
+    source: &tiny_skia::Pixmap,
     results: &[FilterResult],
 ) -> Result<Image, Error> {
-    let convert = |in_image: Option<&tiny_skia::Pixmap>, region: IntRect| {
-        let image = if let Some(image) = in_image {
-            image.clone()
-        } else {
-            tiny_skia::Pixmap::try_create(region.width(), region.height())?
-        };
-
-        Ok(Image {
-            image: Rc::new(image),
-            region,
-            color_space: usvg::filter::ColorInterpolation::SRGB,
-        })
-    };
-
-    let convert_alpha = |mut image: tiny_skia::Pixmap| {
-        // Set RGB to black. Keep alpha as is.
-        for p in image.data_mut().as_rgba_mut() {
-            p.r = 0;
-            p.g = 0;
-            p.b = 0;
-        }
-
-        Ok(Image {
-            image: Rc::new(image),
-            region,
-            color_space: usvg::filter::ColorInterpolation::SRGB,
-        })
-    };
-
     match input {
         usvg::filter::Input::SourceGraphic => {
-            let image = inputs.source.clone();
+            let image = source.clone();
 
             Ok(Image {
                 image: Rc::new(image),
@@ -791,24 +744,27 @@ fn get_input(
             })
         }
         usvg::filter::Input::SourceAlpha => {
-            let image = inputs.source.clone();
-            convert_alpha(image)
-        }
-        usvg::filter::Input::BackgroundImage => {
-            get_input(&usvg::filter::Input::SourceGraphic, region, inputs, results)
-        }
-        usvg::filter::Input::BackgroundAlpha => {
-            get_input(&usvg::filter::Input::SourceAlpha, region, inputs, results)
+            let mut image = source.clone();
+            // Set RGB to black. Keep alpha as is.
+            for p in image.data_mut().as_rgba_mut() {
+                p.r = 0;
+                p.g = 0;
+                p.b = 0;
+            }
+
+            Ok(Image {
+                image: Rc::new(image),
+                region,
+                color_space: usvg::filter::ColorInterpolation::SRGB,
+            })
         }
-        usvg::filter::Input::FillPaint => convert(inputs.fill_paint, region),
-        usvg::filter::Input::StrokePaint => convert(inputs.stroke_paint, region),
         usvg::filter::Input::Reference(ref name) => {
             if let Some(v) = results.iter().rev().find(|v| v.name == *name) {
                 Ok(v.image.clone())
             } else {
                 // Technically unreachable.
                 log::warn!("Unknown filter primitive reference '{}'.", name);
-                get_input(&usvg::filter::Input::SourceGraphic, region, inputs, results)
+                get_input(&usvg::filter::Input::SourceGraphic, region, source, results)
             }
         }
     }
@@ -1043,13 +999,13 @@ fn apply_merge(
     fe: &usvg::filter::Merge,
     cs: usvg::filter::ColorInterpolation,
     region: IntRect,
-    inputs: &FilterInputs,
+    source: &tiny_skia::Pixmap,
     results: &[FilterResult],
 ) -> Result<Image, Error> {
     let mut pixmap = tiny_skia::Pixmap::try_create(region.width(), region.height())?;
 
     for input in &fe.inputs {
-        let input = get_input(input, region, inputs, results)?;
+        let input = get_input(input, region, source, results)?;
         let input = input.into_color_space(cs)?;
         pixmap.draw_pixmap(
             0,

diff --git a/crates/resvg/src/render.rs b/crates/resvg/src/render.rs
@@ -140,16 +140,8 @@ fn render_group(
     render_nodes(&group.children, ctx, transform, &mut sub_pixmap.as_mut());
 
     if !group.filters.is_empty() {
-        let fill_paint = prepare_filter_paint(group.filter_fill.as_ref(), ctx, &sub_pixmap);
-        let stroke_paint = prepare_filter_paint(group.filter_stroke.as_ref(), ctx, &sub_pixmap);
         for filter in &group.filters {
-            crate::filter::apply(
-                filter,
-                transform,
-                fill_paint.as_ref(),
-                stroke_paint.as_ref(),
-                &mut sub_pixmap,
-            );
+            crate::filter::apply(filter, transform, &mut sub_pixmap);
         }
     }
 
@@ -179,45 +171,6 @@ fn render_group(
     Some(())
 }
 
-/// Renders an image used by `FillPaint`/`StrokePaint` filter input.
-///
-/// FillPaint/StrokePaint is mostly an undefined behavior and will produce different results
-/// in every application.
-/// And since there are no expected behaviour, we will simply fill the filter region.
-///
-/// https://github.com/w3c/fxtf-drafts/issues/323
-fn prepare_filter_paint(
-    paint: Option<&crate::paint_server::Paint>,
-    ctx: &Context,
-    pixmap: &tiny_skia::Pixmap,
-) -> Option<tiny_skia::Pixmap> {
-    use std::rc::Rc;
-
-    let paint = paint?;
-    let mut sub_pixmap = tiny_skia::Pixmap::new(pixmap.width(), pixmap.height()).unwrap();
-
-    let rect = tiny_skia::Rect::from_xywh(0.0, 0.0, pixmap.width() as f32, pixmap.height() as f32)?;
-    let path = tiny_skia::PathBuilder::from_rect(rect);
-
-    let path = crate::path::FillPath {
-        transform: tiny_skia::Transform::default(),
-        paint: paint.clone(), // TODO: remove clone
-        rule: tiny_skia::FillRule::Winding,
-        anti_alias: true,
-        path: Rc::new(path),
-    };
-
-    crate::path::render_fill_path(
-        &path,
-        tiny_skia::BlendMode::SourceOver,
-        ctx,
-        tiny_skia::Transform::default(),
-        &mut sub_pixmap.as_mut(),
-    );
-
-    Some(sub_pixmap)
-}
-
 pub trait TinySkiaPixmapMutExt {
     fn create_rect_mask(
         &self,

diff --git a/crates/resvg/src/tree.rs b/crates/resvg/src/tree.rs
@@ -7,7 +7,6 @@ use usvg::NodeExt;
 use crate::clip::ClipPath;
 use crate::image::Image;
 use crate::mask::Mask;
-use crate::paint_server::Paint;
 use crate::path::{FillPath, StrokePath};
 
 pub struct Group {
@@ -16,11 +15,8 @@ pub struct Group {
     pub blend_mode: tiny_skia::BlendMode,
     pub clip_path: Option<ClipPath>,
     pub mask: Option<Mask>,
-    pub isolate: bool,
-
     pub filters: Vec<crate::filter::Filter>,
-    pub filter_fill: Option<Paint>,
-    pub filter_stroke: Option<Paint>,
+    pub isolate: bool,
     /// Group's layer bounding box in canvas coordinates.
     pub bbox: tiny_skia::Rect,
 
@@ -183,24 +179,6 @@ fn convert_group(
         bboxes.layer = usvg::BBox::from(filter_bbox);
     }
 
-    let mut filter_fill = None;
-    if let Some(ref paint) = ugroup.filter_fill_paint() {
-        filter_fill = crate::paint_server::convert(
-            paint,
-            usvg::Opacity::ONE,
-            bboxes.layer.to_non_zero_rect(),
-        );
-    }
-
-    let mut filter_stroke = None;
-    if let Some(ref paint) = ugroup.filter_stroke_paint() {
-        filter_stroke = crate::paint_server::convert(
-            paint,
-            usvg::Opacity::ONE,
-            bboxes.layer.to_non_zero_rect(),
-        );
-    }
-
     let group = Group {
         transform: ugroup.transform,
         opacity: ugroup.opacity,
@@ -209,8 +187,6 @@ fn convert_group(
         mask: crate::mask::convert(ugroup.mask.clone(), bboxes.object.to_rect()?),
         isolate: ugroup.isolate,
         filters,
-        filter_fill,
-        filter_stroke,
         bbox: bboxes.layer.to_rect()?,
         children: group_children,
     };
@@ -231,18 +207,6 @@ fn convert_empty_group(ugroup: &usvg::Group, children: &mut Vec<Node>) -> Option
     let (filters, layer_bbox) = crate::filter::convert(&ugroup.filters, None);
     let layer_bbox = layer_bbox?;
 
-    let mut filter_fill = None;
-    if let Some(ref paint) = ugroup.filter_fill_paint() {
-        filter_fill =
-            crate::paint_server::convert(paint, usvg::Opacity::ONE, layer_bbox.to_non_zero_rect());
-    }
-
-    let mut filter_stroke = None;
-    if let Some(ref paint) = ugroup.filter_stroke_paint() {
-        filter_stroke =
-            crate::paint_server::convert(paint, usvg::Opacity::ONE, layer_bbox.to_non_zero_rect());
-    }
-
     let group = Group {
         transform: ugroup.transform,
         opacity: ugroup.opacity,
@@ -251,8 +215,6 @@ fn convert_empty_group(ugroup: &usvg::Group, children: &mut Vec<Node>) -> Option
         mask: None,
         isolate: ugroup.isolate,
         filters,
-        filter_fill,
-        filter_stroke,
         bbox: layer_bbox,
         children: Vec::new(),
     };

diff --git a/crates/resvg/tests/tests/filters/filter/in=BackgroundAlpha.png b/crates/resvg/tests/tests/filters/filter/in=BackgroundAlpha.png
diff --git a/crates/resvg/tests/tests/filters/filter/in=FillPaint-on-g-without-children.png b/crates/resvg/tests/tests/filters/filter/in=FillPaint-on-g-without-children.png
diff --git a/crates/resvg/tests/tests/filters/filter/in=FillPaint-with-gradient.png b/crates/resvg/tests/tests/filters/filter/in=FillPaint-with-gradient.png
diff --git a/crates/resvg/tests/tests/filters/filter/in=FillPaint-with-pattern.png b/crates/resvg/tests/tests/filters/filter/in=FillPaint-with-pattern.png
diff --git a/crates/resvg/tests/tests/filters/filter/in=FillPaint-with-target-on-g.png b/crates/resvg/tests/tests/filters/filter/in=FillPaint-with-target-on-g.png
diff --git a/crates/resvg/tests/tests/filters/filter/in=FillPaint.png b/crates/resvg/tests/tests/filters/filter/in=FillPaint.png
diff --git a/crates/resvg/tests/tests/filters/filter/in=StrokePaint.png b/crates/resvg/tests/tests/filters/filter/in=StrokePaint.png