[librsvg: 1/2] Accept multiple URI values for filter: property




commit d7aef524c08a4fafbe94e9af1741b7014498bdb5
Author: John Ledbetter <john ledbetter gmail com>
Date:   Wed Sep 2 16:06:15 2020 -0400

    Accept multiple URI values for filter: property
    
    Addresses https://gitlab.gnome.org/GNOME/librsvg/-/issues/615
    
    We now accept a list of IRI values for the filter property, instead
    of only a single value.
    
    Prior to applying the filters, we first check that every specified
    filter is valid (it references a <filter> element); if not, according
    to the specification we do not apply any filters at all.
    
    This commit does not implement handling filter functions.

 rsvg_internals/src/drawing_ctx.rs                 |  50 +++++----
 rsvg_internals/src/filter.rs                      | 117 +++++++++++++++++++++-
 rsvg_internals/src/property_defs.rs               |   5 +-
 tests/fixtures/reftests/svg2/multi-filter-ref.png | Bin 0 -> 5829 bytes
 tests/fixtures/reftests/svg2/multi-filter.svg     |  22 ++++
 5 files changed, 173 insertions(+), 21 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index b77efb72..704fa75a 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -18,9 +18,11 @@ use crate::document::AcquiredNodes;
 use crate::dpi::Dpi;
 use crate::element::Element;
 use crate::error::{AcquireError, RenderingError};
+use crate::filter::{FilterValue, FilterValueList};
 use crate::filters;
 use crate::float_eq_cairo::ApproxEqCairo;
 use crate::gradient::{Gradient, GradientUnits, GradientVariant, SpreadMethod};
+use crate::iri::IRI;
 use crate::marker;
 use crate::node::{CascadedValues, Node, NodeBorrow, NodeDraw};
 use crate::paint_server::{PaintServer, PaintSource};
@@ -487,14 +489,13 @@ impl DrawingCtx {
                 let clip_uri = clip_path_value.0.get();
                 let mask = mask_value.0.get();
 
-                // The `filter` property does not apply to masks.
-                let filter = if node.is_element() {
+                let filters = if node.is_element() {
                     match *node.borrow_element() {
-                        Element::Mask(_) => None,
-                        _ => values.filter().0.get().cloned(),
+                        Element::Mask(_) => FilterValueList::default(),
+                        _ => values.filter().0,
                     }
                 } else {
-                    values.filter().0.get().cloned()
+                    values.filter().0
                 };
 
                 let UnitInterval(opacity) = values.opacity().0;
@@ -509,7 +510,7 @@ impl DrawingCtx {
 
                 let is_opaque = approx_eq!(f64, opacity, 1.0);
                 let needs_temporary_surface = !(is_opaque
-                    && filter.is_none()
+                    && filters.is_empty()
                     && mask.is_none()
                     && values.mix_blend_mode() == MixBlendMode::Normal
                     && clip_in_object_space.is_none());
@@ -525,12 +526,12 @@ impl DrawingCtx {
 
                     // Create temporary surface and its cr
 
-                    let cr = if filter.is_some() {
-                        cairo::Context::new(&*dc.create_surface_for_toplevel_viewport()?)
-                    } else {
+                    let cr = if filters.is_empty() {
                         cairo::Context::new(
                             &dc.create_similar_surface_for_toplevel_viewport(&dc.cr.get_target())?,
                         )
+                    } else {
+                        cairo::Context::new(&*dc.create_surface_for_toplevel_viewport()?)
                     };
 
                     cr.set_matrix(affines.for_temporary_surface.into());
@@ -549,7 +550,7 @@ impl DrawingCtx {
 
                     // Filter
 
-                    let source_surface = if let Some(filter_uri) = filter {
+                    let source_surface = if filters.is_applicable(node, acquired_nodes) {
                         // The target surface has multiple references.
                         // We need to copy it to a new surface to have a unique
                         // reference to be able to safely access the pixel data.
@@ -557,17 +558,27 @@ impl DrawingCtx {
                             &cairo::ImageSurface::try_from(dc.cr.get_target()).unwrap(),
                         )?;
 
-                        let img_surface = dc
-                            .run_filter(
-                                acquired_nodes,
-                                &filter_uri,
-                                node,
-                                values,
+                        let img_surface = filters
+                            .0
+                            .iter()
+                            .try_fold(
                                 child_surface,
-                                bbox,
+                                |surface, filter| -> Result<_, RenderingError> {
+                                    if let FilterValue::URL(IRI::Resource(filter_uri)) = filter {
+                                        dc.run_filter(
+                                            acquired_nodes,
+                                            &filter_uri,
+                                            node,
+                                            values,
+                                            surface,
+                                            bbox,
+                                        )
+                                    } else {
+                                        Ok(surface)
+                                    }
+                                },
                             )?
                             .into_image_surface()?;
-
                         // turn ImageSurface into a Surface
                         (*img_surface).clone()
                     } else {
@@ -767,6 +778,9 @@ impl DrawingCtx {
         child_surface: SharedImageSurface,
         node_bbox: BoundingBox,
     ) -> Result<SharedImageSurface, RenderingError> {
+        // TODO: since we check is_applicable before we get here, these checks are redundant
+        // do we want to remove them and directly grab the filter node? or keep for future error
+        // handling?
         match acquired_nodes.acquire(filter_uri) {
             Ok(acquired) => {
                 let filter_node = acquired.get();
diff --git a/rsvg_internals/src/filter.rs b/rsvg_internals/src/filter.rs
index 74d6b131..90718732 100644
--- a/rsvg_internals/src/filter.rs
+++ b/rsvg_internals/src/filter.rs
@@ -1,13 +1,17 @@
 //! The `filter` element.
 
+use cssparser::Parser;
 use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
 use crate::bbox::BoundingBox;
 use crate::coord_units::CoordUnits;
+use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
-use crate::element::{Draw, ElementResult, SetAttributes};
+use crate::element::{Draw, Element, ElementResult, SetAttributes};
 use crate::error::ValueErrorKind;
+use crate::iri::IRI;
 use crate::length::*;
+use crate::node::{Node, NodeBorrow};
 use crate::parsers::{Parse, ParseValue};
 use crate::properties::ComputedValues;
 use crate::property_bag::PropertyBag;
@@ -185,3 +189,114 @@ impl SetAttributes for Filter {
 }
 
 impl Draw for Filter {}
+
+#[derive(Debug, Clone, PartialEq)]
+pub enum FilterValue {
+    URL(IRI),
+}
+#[derive(Debug, Clone, PartialEq)]
+pub struct FilterValueList(pub Vec<FilterValue>);
+
+impl Default for FilterValueList {
+    fn default() -> FilterValueList {
+        FilterValueList(vec![])
+    }
+}
+
+impl FilterValueList {
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    /// Check that at least one filter URI exists and that all contained
+    /// URIs reference existing <filter> elements.
+    pub fn is_applicable(&self, node: &Node, acquired_nodes: &mut AcquiredNodes) -> bool {
+        if self.is_empty() {
+            return false;
+        }
+
+        self.0.iter().all(|filter| match filter {
+            FilterValue::URL(IRI::Resource(filter_uri)) => {
+                match acquired_nodes.acquire(filter_uri) {
+                    Ok(acquired) => {
+                        let filter_node = acquired.get();
+
+                        match *filter_node.borrow_element() {
+                            Element::Filter(_) => true,
+                            _ => {
+                                rsvg_log!(
+                                    "element {} will not be filtered since \"{}\" is not a filter",
+                                    node,
+                                    filter_uri,
+                                );
+                                false
+                            }
+                        }
+                    }
+                    _ => {
+                        rsvg_log!(
+                            "element {} will not be filtered since its filter \"{}\" was not found",
+                            node,
+                            filter_uri,
+                        );
+                        false
+                    }
+                }
+            }
+            FilterValue::URL(IRI::None) => {
+                unreachable!("Unexpected IRI::None in FilterValueList");
+            }
+        })
+    }
+}
+
+impl Parse for FilterValueList {
+    fn parse<'i>(parser: &mut Parser<'i, '_>) -> Result<Self, crate::error::ParseError<'i>> {
+        let mut result = FilterValueList::default();
+
+        if parser
+            .try_parse(|p| p.expect_ident_matching("none"))
+            .is_ok()
+        {
+            return Ok(result);
+        }
+
+        while !parser.is_exhausted() {
+            let loc = parser.current_source_location();
+
+            if let Ok(iri) = IRI::parse(parser) {
+                result.0.push(FilterValue::URL(iri));
+            } else {
+                let token = parser.next()?;
+                return Err(loc.new_basic_unexpected_token_error(token.clone()).into());
+            }
+        }
+
+        Ok(result)
+    }
+}
+
+#[cfg(test)]
+#[test]
+fn parses_filter_value_list() {
+    use crate::allowed_url::Fragment;
+    use crate::filter::FilterValue;
+    use crate::iri::IRI;
+
+    assert_eq!(
+        FilterValueList::parse_str("none"),
+        Ok(FilterValueList::default())
+    );
+
+    let f1 = Fragment::new(Some("foo.svg".to_string()), "bar".to_string());
+    let f2 = Fragment::new(Some("test.svg".to_string()), "baz".to_string());
+    assert_eq!(
+        FilterValueList::parse_str("url(foo.svg#bar) url(test.svg#baz)"),
+        Ok(FilterValueList(vec![
+            FilterValue::URL(IRI::Resource(f1)),
+            FilterValue::URL(IRI::Resource(f2))
+        ]))
+    );
+
+    assert!(FilterValueList::parse_str("fail").is_err());
+}
diff --git a/rsvg_internals/src/property_defs.rs b/rsvg_internals/src/property_defs.rs
index 5ca37dcc..9eab1d4a 100644
--- a/rsvg_internals/src/property_defs.rs
+++ b/rsvg_internals/src/property_defs.rs
@@ -47,6 +47,7 @@ use cssparser::{Parser, Token};
 
 use crate::dasharray::Dasharray;
 use crate::error::*;
+use crate::filter::FilterValueList;
 use crate::font_props::{Font, FontFamily, FontSize, FontWeight, LetterSpacing, LineHeight};
 use crate::iri::IRI;
 use crate::length::*;
@@ -250,9 +251,9 @@ make_property!(
 make_property!(
     ComputedValues,
     Filter,
-    default: IRI::None,
+    default: FilterValueList::default(),
     inherits_automatically: false,
-    newtype_parse: IRI,
+    newtype_parse: FilterValueList,
 );
 
 // https://www.w3.org/TR/SVG/filters.html#FloodColorProperty
diff --git a/tests/fixtures/reftests/svg2/multi-filter-ref.png 
b/tests/fixtures/reftests/svg2/multi-filter-ref.png
new file mode 100644
index 00000000..c5739cbd
Binary files /dev/null and b/tests/fixtures/reftests/svg2/multi-filter-ref.png differ
diff --git a/tests/fixtures/reftests/svg2/multi-filter.svg b/tests/fixtures/reftests/svg2/multi-filter.svg
new file mode 100644
index 00000000..a5b08415
--- /dev/null
+++ b/tests/fixtures/reftests/svg2/multi-filter.svg
@@ -0,0 +1,22 @@
+<svg width="123" height="114" xmlns="http://www.w3.org/2000/svg";>
+ <defs>
+  <filter id="filter1">
+   <feGaussianBlur stdDeviation="3"/>
+  </filter>
+    <filter id="filter2">
+      <feColorMatrix type="hueRotate" values="45"/>
+    </filter>
+ </defs>
+ <metadata id="metadata5">image/svg+xml</metadata>
+ <g>
+  <title>background</title>
+  <rect fill="none" id="canvas_background" height="116" width="125" y="-1" x="-1"/>
+ </g>
+ <g>
+  <title>Layer 1</title>
+  <g id="layer1">
+   <rect fill="#0000ff" fill-rule="evenodd" stroke-width="0.26458" id="rect833" width="73.38349" 
height="60.49095" x="8.5901" y="12.87481"/>
+   <ellipse fill="#ff0000" fill-rule="evenodd" stroke-width="0.26458" filter="url(#filter1) url(#filter2)" 
id="path835" cx="67.3905" cy="70.02157" rx="37.57762" ry="22.7273"/>
+  </g>
+ </g>
+</svg>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]