[librsvg: 9/30] convolve_matrix: move the sanity checks on targetX/Y at rendering time

[Federico Mena Quintero <federico src gnome org>]

commit 49ff91b57cf6ce7500ae6d0b33cfc21db3555468
Author: Paolo Borelli <pborelli gnome org>
Date:   Wed Dec 23 19:50:49 2020 +0100

    convolve_matrix: move the sanity checks on targetX/Y at rendering time
    
    Raise an error at rendering if the values are not compatible with
    the ones in the order attribute. Also add the corresponding error
    and add a parser for u32.

 src/filters/convolve_matrix.rs | 73 +++++++++++++++---------------------------
 src/filters/error.rs           |  3 ++
 src/parsers.rs                 | 21 ++++++++++++
 3 files changed, 50 insertions(+), 47 deletions(-)
---
diff --git a/src/filters/convolve_matrix.rs b/src/filters/convolve_matrix.rs
index 0238a81f..dfb3bcbd 100644
--- a/src/filters/convolve_matrix.rs
+++ b/src/filters/convolve_matrix.rs
@@ -69,6 +69,8 @@ impl SetAttributes for FeConvolveMatrix {
                     self.divisor = Some(d);
                 }
                 expanded_name!("", "bias") => self.bias = attr.parse(value)?,
+                expanded_name!("", "targetX") => self.target_x = attr.parse(value)?,
+                expanded_name!("", "targetY") => self.target_y = attr.parse(value)?,
                 expanded_name!("", "edgeMode") => self.edge_mode = attr.parse(value)?,
                 expanded_name!("", "kernelUnitLength") => {
                     let NumberOptionalNumber(NonNegative(x), NonNegative(y)) = attr.parse(value)?;
@@ -80,49 +82,6 @@ impl SetAttributes for FeConvolveMatrix {
             }
         }
 
-        // target_x and target_y depend on order.
-        for (attr, value) in attrs.iter() {
-            match attr.expanded() {
-                expanded_name!("", "targetX") => {
-                    self.target_x = {
-                        let v = attr.parse_and_validate(value, |v: i32| {
-                            if v >= 0 && v < self.order.0 as i32 {
-                                Ok(v)
-                            } else {
-                                Err(ValueErrorKind::value_error(
-                                    "targetX must be greater or equal to zero and less than orderX",
-                                ))
-                            }
-                        })?;
-                        Some(v as u32)
-                    }
-                }
-                expanded_name!("", "targetY") => {
-                    self.target_y = {
-                        let v = attr.parse_and_validate(value, |v: i32| {
-                            if v >= 0 && v < self.order.1 as i32 {
-                                Ok(v)
-                            } else {
-                                Err(ValueErrorKind::value_error(
-                                    "targetY must be greater or equal to zero and less than orderY",
-                                ))
-                            }
-                        })?;
-                        Some(v as u32)
-                    }
-                }
-                _ => (),
-            }
-        }
-
-        // Default values for target_x and target_y.
-        if self.target_x.is_none() {
-            self.target_x = Some(self.order.0 / 2);
-        }
-        if self.target_y.is_none() {
-            self.target_y = Some(self.order.1 / 2);
-        }
-
         // Finally, parse the kernel matrix.
         for (attr, value) in attrs
             .iter()
@@ -189,6 +148,26 @@ impl FilterEffect for FeConvolveMatrix {
             .into_irect(draw_ctx);
         let original_bounds = bounds;
 
+        let target_x = match self.target_x {
+            Some(x) if x >= self.order.0 => {
+                return Err(FilterError::InvalidParameter(
+                    "targetX must be less than orderX".to_string(),
+                ))
+            }
+            Some(x) => x,
+            None => self.order.0 / 2,
+        };
+
+        let target_y = match self.target_y {
+            Some(y) if y >= self.order.1 => {
+                return Err(FilterError::InvalidParameter(
+                    "targetY must be less than orderY".to_string(),
+                ))
+            }
+            Some(y) => y,
+            None => self.order.1 / 2,
+        };
+
         let mut input_surface = if self.preserve_alpha {
             // preserve_alpha means we need to premultiply and unpremultiply the values.
             input.surface().unpremultiply(bounds)?
@@ -220,10 +199,10 @@ impl FilterEffect for FeConvolveMatrix {
             for (x, y, pixel) in Pixels::within(&input_surface, bounds) {
                 // Compute the convolution rectangle bounds.
                 let kernel_bounds = IRect::new(
-                    x as i32 - self.target_x.unwrap() as i32,
-                    y as i32 - self.target_y.unwrap() as i32,
-                    x as i32 - self.target_x.unwrap() as i32 + self.order.0 as i32,
-                    y as i32 - self.target_y.unwrap() as i32 + self.order.1 as i32,
+                    x as i32 - target_x as i32,
+                    y as i32 - target_y as i32,
+                    x as i32 - target_x as i32 + self.order.0 as i32,
+                    y as i32 - target_y as i32 + self.order.1 as i32,
                 );
 
                 // Do the convolution.
diff --git a/src/filters/error.rs b/src/filters/error.rs
index d5cd1043..82bf4ee2 100644
--- a/src/filters/error.rs
+++ b/src/filters/error.rs
@@ -9,6 +9,8 @@ pub enum FilterError {
     InvalidUnits,
     /// The filter was passed invalid input (the `in` attribute).
     InvalidInput,
+    /// The filter was passed an invalid parameter.
+    InvalidParameter(String),
     /// The filter input surface has an unsuccessful status.
     BadInputSurfaceStatus(cairo::Status),
     /// A Cairo error.
@@ -34,6 +36,7 @@ impl fmt::Display for FilterError {
                 "unit identifiers are not allowed with primitiveUnits set to objectBoundingBox"
             ),
             FilterError::InvalidInput => write!(f, "invalid value of the `in` attribute"),
+            FilterError::InvalidParameter(ref s) => write!(f, "invalid parameter value: {}", s),
             FilterError::BadInputSurfaceStatus(ref status) => {
                 write!(f, "invalid status of the input surface: {}", status)
             }
diff --git a/src/parsers.rs b/src/parsers.rs
index f5950d2b..509cc9eb 100644
--- a/src/parsers.rs
+++ b/src/parsers.rs
@@ -157,6 +157,18 @@ impl Parse for i32 {
     }
 }
 
+impl Parse for u32 {
+    fn parse<'i>(parser: &mut Parser<'i, '_>) -> Result<Self, ParseError<'i>> {
+        let loc = parser.current_source_location();
+        let n = parser.expect_integer()?;
+        if n >= 0 {
+            Ok(n as u32)
+        } else {
+            Err(loc.new_custom_error(ValueErrorKind::value_error("expected unsigned number")))
+        }
+    }
+}
+
 /// Parses a list of identifiers from a `cssparser::Parser`
 ///
 /// # Example
@@ -265,8 +277,12 @@ mod tests {
 
     #[test]
     fn parses_integer() {
+        assert_eq!(i32::parse_str("0").unwrap(), 0);
         assert_eq!(i32::parse_str("1").unwrap(), 1);
         assert_eq!(i32::parse_str("-1").unwrap(), -1);
+
+        assert_eq!(u32::parse_str("0").unwrap(), 0);
+        assert_eq!(u32::parse_str("1").unwrap(), 1);
     }
 
     #[test]
@@ -274,6 +290,11 @@ mod tests {
         assert!(i32::parse_str("").is_err());
         assert!(i32::parse_str("1x").is_err());
         assert!(i32::parse_str("1.5").is_err());
+
+        assert!(u32::parse_str("").is_err());
+        assert!(u32::parse_str("1x").is_err());
+        assert!(u32::parse_str("1.5").is_err());
+        assert!(u32::parse_str("-1").is_err());
     }
 
     #[test]