GNOME.org
 

[librsvg: 1/7] filters: move unit check at rendering time

commit f070b853bcaa78be58b49300e264733a563caa0a
Author: Paolo Borelli <pborelli gnome org>
Date:   Sat Mar 28 11:36:45 2020 +0100

    filters: move unit check at rendering time
    
    Check if the units specified for the bounds are valid when rendering.
    This removes the last use of the parent node in set_atts.

 rsvg_internals/src/filters/blend.rs              |  4 +-
 rsvg_internals/src/filters/color_matrix.rs       |  4 +-
 rsvg_internals/src/filters/component_transfer.rs |  2 +-
 rsvg_internals/src/filters/composite.rs          |  4 +-
 rsvg_internals/src/filters/convolve_matrix.rs    |  4 +-
 rsvg_internals/src/filters/displacement_map.rs   |  4 +-
 rsvg_internals/src/filters/error.rs              |  5 ++
 rsvg_internals/src/filters/flood.rs              |  5 +-
 rsvg_internals/src/filters/gaussian_blur.rs      |  4 +-
 rsvg_internals/src/filters/image.rs              |  4 +-
 rsvg_internals/src/filters/light/lighting.rs     |  2 +-
 rsvg_internals/src/filters/merge.rs              |  2 +-
 rsvg_internals/src/filters/mod.rs                | 79 +++++++++++++-----------
 rsvg_internals/src/filters/morphology.rs         |  4 +-
 rsvg_internals/src/filters/offset.rs             |  4 +-
 rsvg_internals/src/filters/tile.rs               |  7 ++-
 rsvg_internals/src/filters/turbulence.rs         |  5 +-
 17 files changed, 81 insertions(+), 62 deletions(-)
---
diff --git a/rsvg_internals/src/filters/blend.rs b/rsvg_internals/src/filters/blend.rs
index 13b05181..1fc53a4d 100755
--- a/rsvg_internals/src/filters/blend.rs
+++ b/rsvg_internals/src/filters/blend.rs
@@ -76,7 +76,7 @@ impl ElementTrait for FeBlend {
 impl FilterEffect for FeBlend {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -85,7 +85,7 @@ impl FilterEffect for FeBlend {
         let input_2 = ctx.get_input(acquired_nodes, draw_ctx, self.in2.as_ref())?;
         let bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .add_input(&input_2)
             .into_irect(draw_ctx);
diff --git a/rsvg_internals/src/filters/color_matrix.rs b/rsvg_internals/src/filters/color_matrix.rs
index f26be941..53fce725 100644
--- a/rsvg_internals/src/filters/color_matrix.rs
+++ b/rsvg_internals/src/filters/color_matrix.rs
@@ -153,7 +153,7 @@ impl ElementTrait for FeColorMatrix {
 impl FilterEffect for FeColorMatrix {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -161,7 +161,7 @@ impl FilterEffect for FeColorMatrix {
         let input = self.base.get_input(ctx, acquired_nodes, draw_ctx)?;
         let bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .into_irect(draw_ctx);
 
diff --git a/rsvg_internals/src/filters/component_transfer.rs 
b/rsvg_internals/src/filters/component_transfer.rs
index a4711ea6..8bab7c51 100644
--- a/rsvg_internals/src/filters/component_transfer.rs
+++ b/rsvg_internals/src/filters/component_transfer.rs
@@ -277,7 +277,7 @@ impl FilterEffect for FeComponentTransfer {
         let input = self.base.get_input(ctx, acquired_nodes, draw_ctx)?;
         let bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .into_irect(draw_ctx);
 
diff --git a/rsvg_internals/src/filters/composite.rs b/rsvg_internals/src/filters/composite.rs
index 5e7babac..e3f88a50 100644
--- a/rsvg_internals/src/filters/composite.rs
+++ b/rsvg_internals/src/filters/composite.rs
@@ -76,7 +76,7 @@ impl ElementTrait for FeComposite {
 impl FilterEffect for FeComposite {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -85,7 +85,7 @@ impl FilterEffect for FeComposite {
         let input_2 = ctx.get_input(acquired_nodes, draw_ctx, self.in2.as_ref())?;
         let bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .add_input(&input_2)
             .into_irect(draw_ctx);
diff --git a/rsvg_internals/src/filters/convolve_matrix.rs b/rsvg_internals/src/filters/convolve_matrix.rs
index 5491a30c..5c9c3871 100644
--- a/rsvg_internals/src/filters/convolve_matrix.rs
+++ b/rsvg_internals/src/filters/convolve_matrix.rs
@@ -198,7 +198,7 @@ impl ElementTrait for FeConvolveMatrix {
 impl FilterEffect for FeConvolveMatrix {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -206,7 +206,7 @@ impl FilterEffect for FeConvolveMatrix {
         let input = self.base.get_input(ctx, acquired_nodes, draw_ctx)?;
         let mut bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .into_irect(draw_ctx);
         let original_bounds = bounds;
diff --git a/rsvg_internals/src/filters/displacement_map.rs b/rsvg_internals/src/filters/displacement_map.rs
index e9ce7ade..1ccce732 100644
--- a/rsvg_internals/src/filters/displacement_map.rs
+++ b/rsvg_internals/src/filters/displacement_map.rs
@@ -74,7 +74,7 @@ impl ElementTrait for FeDisplacementMap {
 impl FilterEffect for FeDisplacementMap {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -83,7 +83,7 @@ impl FilterEffect for FeDisplacementMap {
         let displacement_input = ctx.get_input(acquired_nodes, draw_ctx, self.in2.as_ref())?;
         let bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .add_input(&displacement_input)
             .into_irect(draw_ctx);
diff --git a/rsvg_internals/src/filters/error.rs b/rsvg_internals/src/filters/error.rs
index 21055bf5..cf03a879 100644
--- a/rsvg_internals/src/filters/error.rs
+++ b/rsvg_internals/src/filters/error.rs
@@ -6,6 +6,8 @@ use crate::error::RenderingError;
 /// An enumeration of errors that can occur during filter primitive rendering.
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
 pub enum FilterError {
+    /// The units on the filter bounds are invalid
+    InvalidUnits,
     /// The filter was passed invalid input (the `in` attribute).
     InvalidInput,
     /// The filter input surface has an unsuccessful status.
@@ -27,6 +29,9 @@ impl Error for FilterError {
     #[inline]
     fn description(&self) -> &str {
         match *self {
+            FilterError::InvalidUnits => {
+                "unit identifiers are not allowed with primitiveUnits set to objectBoundingBox"
+            }
             FilterError::InvalidInput => "invalid value of the `in` attribute",
             FilterError::BadInputSurfaceStatus(_) => "invalid status of the input surface",
             FilterError::CairoError(_) => "Cairo error",
diff --git a/rsvg_internals/src/filters/flood.rs b/rsvg_internals/src/filters/flood.rs
index 69eb6ef3..c4df15d2 100644
--- a/rsvg_internals/src/filters/flood.rs
+++ b/rsvg_internals/src/filters/flood.rs
@@ -39,7 +39,10 @@ impl FilterEffect for FeFlood {
         _acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
     ) -> Result<FilterResult, FilterError> {
-        let bounds = self.base.get_bounds(ctx).into_irect(draw_ctx);
+        let bounds = self
+            .base
+            .get_bounds(ctx, node.parent().as_ref())?
+            .into_irect(draw_ctx);
 
         let cascaded = CascadedValues::new_from_node(node);
         let values = cascaded.get();
diff --git a/rsvg_internals/src/filters/gaussian_blur.rs b/rsvg_internals/src/filters/gaussian_blur.rs
index 13570360..a23e443d 100644
--- a/rsvg_internals/src/filters/gaussian_blur.rs
+++ b/rsvg_internals/src/filters/gaussian_blur.rs
@@ -202,7 +202,7 @@ fn gaussian_blur(
 impl FilterEffect for FeGaussianBlur {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -210,7 +210,7 @@ impl FilterEffect for FeGaussianBlur {
         let input = self.base.get_input(ctx, acquired_nodes, draw_ctx)?;
         let bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .into_irect(draw_ctx);
 
diff --git a/rsvg_internals/src/filters/image.rs b/rsvg_internals/src/filters/image.rs
index 7beb15f2..1fdc2720 100644
--- a/rsvg_internals/src/filters/image.rs
+++ b/rsvg_internals/src/filters/image.rs
@@ -140,12 +140,12 @@ impl ElementTrait for FeImage {
 impl FilterEffect for FeImage {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
     ) -> Result<FilterResult, FilterError> {
-        let bounds_builder = self.base.get_bounds(ctx);
+        let bounds_builder = self.base.get_bounds(ctx, node.parent().as_ref())?;
         let bounds = bounds_builder.into_rect(draw_ctx);
 
         match self.href.as_ref() {
diff --git a/rsvg_internals/src/filters/light/lighting.rs b/rsvg_internals/src/filters/light/lighting.rs
index 9e0ba8eb..d2a68992 100644
--- a/rsvg_internals/src/filters/light/lighting.rs
+++ b/rsvg_internals/src/filters/light/lighting.rs
@@ -261,7 +261,7 @@ macro_rules! impl_lighting_filter {
                 let mut bounds = self
                     .common()
                     .base
-                    .get_bounds(ctx)
+                    .get_bounds(ctx, node.parent().as_ref())?
                     .add_input(&input)
                     .into_irect(draw_ctx);
                 let original_bounds = bounds;
diff --git a/rsvg_internals/src/filters/merge.rs b/rsvg_internals/src/filters/merge.rs
index fc32c54e..c1ec6016 100644
--- a/rsvg_internals/src/filters/merge.rs
+++ b/rsvg_internals/src/filters/merge.rs
@@ -88,7 +88,7 @@ impl FilterEffect for FeMerge {
         draw_ctx: &mut DrawingCtx,
     ) -> Result<FilterResult, FilterError> {
         // Compute the filter bounds, taking each child node's input into account.
-        let mut bounds = self.base.get_bounds(ctx);
+        let mut bounds = self.base.get_bounds(ctx, node.parent().as_ref())?;
         for child in node
             .children()
             .filter(|c| c.is_element() && c.borrow_element().get_type() == ElementType::FeMergeNode)
diff --git a/rsvg_internals/src/filters/mod.rs b/rsvg_internals/src/filters/mod.rs
index 28023537..535c1725 100644
--- a/rsvg_internals/src/filters/mod.rs
+++ b/rsvg_internals/src/filters/mod.rs
@@ -10,7 +10,7 @@ use crate::coord_units::CoordUnits;
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
 use crate::element::{ElementResult, ElementTrait, ElementType};
-use crate::error::{RenderingError, ValueErrorKind};
+use crate::error::RenderingError;
 use crate::filter::Filter;
 use crate::length::*;
 use crate::node::{CascadedValues, Node, NodeBorrow};
@@ -106,16 +106,13 @@ impl Primitive {
         }
     }
 
-    /// Returns the `BoundsBuilder` for bounds computation.
+    /// Validates attributes and returns the `BoundsBuilder` for bounds computation.
     #[inline]
-    fn get_bounds<'a>(&self, ctx: &'a FilterContext) -> BoundsBuilder<'a> {
-        BoundsBuilder::new(ctx, self.x, self.y, self.width, self.height)
-    }
-}
-
-impl ElementTrait for Primitive {
-    fn set_atts(&mut self, parent: Option<&Node>, pbag: &PropertyBag<'_>) -> ElementResult {
-        // With ObjectBoundingBox, only fractions and percents are allowed.
+    fn get_bounds<'a>(
+        &self,
+        ctx: &'a FilterContext,
+        parent: Option<&Node>,
+    ) -> Result<BoundsBuilder<'a>, FilterError> {
         let primitiveunits = parent
             .and_then(|parent| {
                 assert!(parent.is_element());
@@ -129,6 +126,7 @@ impl ElementTrait for Primitive {
             })
             .unwrap_or(CoordUnits::UserSpaceOnUse);
 
+        // With ObjectBoundingBox, only fractions and percents are allowed.
         let no_units_allowed = primitiveunits == CoordUnits::ObjectBoundingBox;
 
         let check_units_horizontal = |length: Length<Horizontal>| {
@@ -138,9 +136,7 @@ impl ElementTrait for Primitive {
 
             match length.unit {
                 LengthUnit::Px | LengthUnit::Percent => Ok(length),
-                _ => Err(ValueErrorKind::parse_error(
-                    "unit identifiers are not allowed with primitiveUnits set to objectBoundingBox",
-                )),
+                _ => Err(FilterError::InvalidUnits),
             }
         };
 
@@ -151,41 +147,50 @@ impl ElementTrait for Primitive {
 
             match length.unit {
                 LengthUnit::Px | LengthUnit::Percent => Ok(length),
-                _ => Err(ValueErrorKind::parse_error(
-                    "unit identifiers are not allowed with primitiveUnits set to objectBoundingBox",
-                )),
+                _ => Err(FilterError::InvalidUnits),
             }
         };
 
-        let check_units_horizontal_and_ensure_nonnegative = |length: Length<Horizontal>| {
-            check_units_horizontal(length).and_then(Length::<Horizontal>::check_nonnegative)
-        };
+        if let Some(x) = self.x {
+            check_units_horizontal(x)?;
+        }
 
-        let check_units_vertical_and_ensure_nonnegative = |length: Length<Vertical>| {
-            check_units_vertical(length).and_then(Length::<Vertical>::check_nonnegative)
-        };
+        if let Some(y) = self.y {
+            check_units_vertical(y)?;
+        }
+
+        if let Some(w) = self.width {
+            check_units_horizontal(w)?;
+        }
 
+        if let Some(h) = self.height {
+            check_units_vertical(h)?;
+        }
+
+        Ok(BoundsBuilder::new(
+            ctx,
+            self.x,
+            self.y,
+            self.width,
+            self.height,
+        ))
+    }
+}
+
+impl ElementTrait for Primitive {
+    fn set_atts(&mut self, _parent: Option<&Node>, pbag: &PropertyBag<'_>) -> ElementResult {
         for (attr, value) in pbag.iter() {
             match attr.expanded() {
-                expanded_name!("", "x") => {
-                    self.x = Some(attr.parse_and_validate(value, check_units_horizontal)?)
-                }
-                expanded_name!("", "y") => {
-                    self.y = Some(attr.parse_and_validate(value, check_units_vertical)?)
-                }
+                expanded_name!("", "x") => self.x = Some(attr.parse(value)?),
+                expanded_name!("", "y") => self.y = Some(attr.parse(value)?),
                 expanded_name!("", "width") => {
-                    self.width =
-                        Some(attr.parse_and_validate(
-                            value,
-                            check_units_horizontal_and_ensure_nonnegative,
-                        )?)
+                    self.width = Some(
+                        attr.parse_and_validate(value, Length::<Horizontal>::check_nonnegative)?,
+                    )
                 }
                 expanded_name!("", "height") => {
                     self.height =
-                        Some(attr.parse_and_validate(
-                            value,
-                            check_units_vertical_and_ensure_nonnegative,
-                        )?)
+                        Some(attr.parse_and_validate(value, Length::<Vertical>::check_nonnegative)?)
                 }
                 expanded_name!("", "result") => self.result = Some(attr.parse(value)?),
                 _ => (),
diff --git a/rsvg_internals/src/filters/morphology.rs b/rsvg_internals/src/filters/morphology.rs
index 7c4b252e..915a351c 100644
--- a/rsvg_internals/src/filters/morphology.rs
+++ b/rsvg_internals/src/filters/morphology.rs
@@ -77,7 +77,7 @@ impl ElementTrait for FeMorphology {
 impl FilterEffect for FeMorphology {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -85,7 +85,7 @@ impl FilterEffect for FeMorphology {
         let input = self.base.get_input(ctx, acquired_nodes, draw_ctx)?;
         let bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .into_irect(draw_ctx);
 
diff --git a/rsvg_internals/src/filters/offset.rs b/rsvg_internals/src/filters/offset.rs
index 0994a9b2..ac64f4cd 100644
--- a/rsvg_internals/src/filters/offset.rs
+++ b/rsvg_internals/src/filters/offset.rs
@@ -50,7 +50,7 @@ impl ElementTrait for FeOffset {
 impl FilterEffect for FeOffset {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -58,7 +58,7 @@ impl FilterEffect for FeOffset {
         let input = self.base.get_input(ctx, acquired_nodes, draw_ctx)?;
         let bounds = self
             .base
-            .get_bounds(ctx)
+            .get_bounds(ctx, node.parent().as_ref())?
             .add_input(&input)
             .into_irect(draw_ctx);
 
diff --git a/rsvg_internals/src/filters/tile.rs b/rsvg_internals/src/filters/tile.rs
index 574d29ee..1029ac1b 100644
--- a/rsvg_internals/src/filters/tile.rs
+++ b/rsvg_internals/src/filters/tile.rs
@@ -33,7 +33,7 @@ impl ElementTrait for FeTile {
 impl FilterEffect for FeTile {
     fn render(
         &self,
-        _node: &Node,
+        node: &Node,
         ctx: &FilterContext,
         acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
@@ -41,7 +41,10 @@ impl FilterEffect for FeTile {
         let input = self.base.get_input(ctx, acquired_nodes, draw_ctx)?;
 
         // feTile doesn't consider its inputs in the filter primitive subregion calculation.
-        let bounds = self.base.get_bounds(ctx).into_irect(draw_ctx);
+        let bounds = self
+            .base
+            .get_bounds(ctx, node.parent().as_ref())?
+            .into_irect(draw_ctx);
 
         let surface = match input {
             FilterInput::StandardInput(input_surface) => input_surface,
diff --git a/rsvg_internals/src/filters/turbulence.rs b/rsvg_internals/src/filters/turbulence.rs
index 5cd059a4..ec7a6bbe 100644
--- a/rsvg_internals/src/filters/turbulence.rs
+++ b/rsvg_internals/src/filters/turbulence.rs
@@ -340,7 +340,10 @@ impl FilterEffect for FeTurbulence {
         _acquired_nodes: &mut AcquiredNodes,
         draw_ctx: &mut DrawingCtx,
     ) -> Result<FilterResult, FilterError> {
-        let bounds = self.base.get_bounds(ctx).into_irect(draw_ctx);
+        let bounds = self
+            .base
+            .get_bounds(ctx, node.parent().as_ref())?
+            .into_irect(draw_ctx);
 
         let affine = ctx.paffine().invert().unwrap();
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]