style: Simplify the SVG animation code.

It's overly generic for no good reason.

Differential Revision: https://phabricator.services.mozilla.com/D10844

components/style/properties/gecko.mako.rs

@@ -561,17 +561,24 @@ def set_gecko_property(ffi_name, expr):
             return SVGLength::ContextValue;
         }
         let length = match self.gecko.${gecko_ffi_name}.as_value() {
-            CoordDataValue::Factor(number) =>
+            CoordDataValue::Factor(number) => {
-                SvgLengthOrPercentageOrNumber::Number(number),
+                SvgLengthOrPercentageOrNumber::Number(number)
-            CoordDataValue::Coord(coord) =>
+            },
+            CoordDataValue::Coord(coord) => {
                 SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                    LengthOrPercentage::Length(Au(coord).into())),
+                    LengthOrPercentage::Length(Au(coord).into())
-            CoordDataValue::Percent(p) =>
+                )
+            },
+            CoordDataValue::Percent(p) => {
                 SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                    LengthOrPercentage::Percentage(Percentage(p))),
+                    LengthOrPercentage::Percentage(Percentage(p))
-            CoordDataValue::Calc(calc) =>
+                )
+            },
+            CoordDataValue::Calc(calc) => {
                 SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                    LengthOrPercentage::Calc(calc.into())),
+                    LengthOrPercentage::Calc(calc.into())
+                )
+            },
             _ => unreachable!("Unexpected coordinate in ${ident}"),
         };
         SVGLength::Length(length.into())

components/style/properties/longhands/inherited_svg.mako.rs

@@ -85,7 +85,7 @@ ${helpers.predefined_type(
 
 ${helpers.predefined_type(
     "stroke-width", "SVGWidth",
-    "::values::computed::NonNegativeLength::new(1.).into()",
+    "computed::SVGWidth::one()",
     products="gecko",
     animation_value_type="::values::computed::SVGWidth",
     spec="https://www.w3.org/TR/SVG2/painting.html#StrokeWidth",
@@ -135,8 +135,9 @@ ${helpers.predefined_type(
 )}
 
 ${helpers.predefined_type(
-    "stroke-dashoffset", "SVGLength",
+    "stroke-dashoffset",
-    "Au(0).into()",
+    "SVGLength",
+    "computed::SVGLength::zero()",
     products="gecko",
     animation_value_type="ComputedValue",
     spec="https://www.w3.org/TR/SVG2/painting.html#StrokeDashing",

components/style/values/animated/svg.rs

@@ -6,12 +6,11 @@
 
 use properties::animated_properties::ListAnimation;
 use values::animated::color::Color as AnimatedColor;
-use values::computed::{NonNegativeNumber, Number, NumberOrPercentage, LengthOrPercentage};
+use values::computed::{Number, NumberOrPercentage, LengthOrPercentage};
 use values::computed::url::ComputedUrl;
-use values::computed::length::NonNegativeLengthOrPercentage;
 use values::distance::{ComputeSquaredDistance, SquaredDistance};
 use values::generics::svg::{SVGLength,  SvgLengthOrPercentageOrNumber, SVGPaint};
-use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity};
+use values::generics::svg::{SVGStrokeDashArray, SVGOpacity};
 use super::{Animate, Procedure, ToAnimatedZero};
 
 /// Animated SVGPaint.
@@ -27,102 +26,44 @@ impl ToAnimatedZero for IntermediateSVGPaint {
     }
 }
 
-impl From<NonNegativeLengthOrPercentage> for NumberOrPercentage {
+// FIXME: We need to handle calc here properly, see
-    fn from(lop: NonNegativeLengthOrPercentage) -> NumberOrPercentage {
+// https://bugzilla.mozilla.org/show_bug.cgi?id=1386967
-        lop.0.into()
+fn to_number_or_percentage(
+    value: &SvgLengthOrPercentageOrNumber<LengthOrPercentage, Number>,
+) -> Result<NumberOrPercentage, ()> {
+    Ok(match *value {
+        SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref l) => {
+            match *l {
+                LengthOrPercentage::Length(ref l) => NumberOrPercentage::Number(l.px()),
+                LengthOrPercentage::Percentage(ref p) => NumberOrPercentage::Percentage(*p),
+                LengthOrPercentage::Calc(..) => return Err(()),
             }
+        }
+        SvgLengthOrPercentageOrNumber::Number(ref n) => NumberOrPercentage::Number(*n),
+    })
 }
 
-impl From<NonNegativeNumber> for NumberOrPercentage {
+impl Animate for SvgLengthOrPercentageOrNumber<LengthOrPercentage, Number> {
-    fn from(num: NonNegativeNumber) -> NumberOrPercentage {
-        num.0.into()
-    }
-}
-
-impl From<LengthOrPercentage> for NumberOrPercentage {
-    fn from(lop: LengthOrPercentage) -> NumberOrPercentage {
-        match lop {
-            LengthOrPercentage::Length(len) => NumberOrPercentage::Number(len.px()),
-            LengthOrPercentage::Percentage(p) => NumberOrPercentage::Percentage(p),
-            LengthOrPercentage::Calc(_) => {
-                panic!("We dont't expected calc interpolation for SvgLengthOrPercentageOrNumber");
-            },
-        }
-    }
-}
-
-impl From<Number> for NumberOrPercentage {
-    fn from(num: Number) -> NumberOrPercentage {
-        NumberOrPercentage::Number(num)
-    }
-}
-
-fn convert_to_number_or_percentage<LengthOrPercentageType, NumberType>(
-    from: SvgLengthOrPercentageOrNumber<LengthOrPercentageType, NumberType>,
-) -> NumberOrPercentage
-where
-    LengthOrPercentageType: Into<NumberOrPercentage>,
-    NumberType: Into<NumberOrPercentage>,
-{
-    match from {
-        SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop) => {
-            lop.into()
-        }
-        SvgLengthOrPercentageOrNumber::Number(num) => {
-            num.into()
-        }
-    }
-}
-
-fn convert_from_number_or_percentage<LengthOrPercentageType, NumberType>(
-    from: NumberOrPercentage,
-) -> SvgLengthOrPercentageOrNumber<LengthOrPercentageType, NumberType>
-where
-    LengthOrPercentageType: From<LengthOrPercentage>,
-    NumberType: From<Number>,
-{
-    match from {
-        NumberOrPercentage::Number(num) =>
-            SvgLengthOrPercentageOrNumber::Number(num.into()),
-        NumberOrPercentage::Percentage(p) =>
-            SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                (LengthOrPercentage::Percentage(p)).into())
-    }
-}
-
-impl <L, N> Animate for SvgLengthOrPercentageOrNumber<L, N>
-where
-    L: Animate + From<LengthOrPercentage> + Into<NumberOrPercentage> + Copy,
-    N: Animate + From<Number> + Into<NumberOrPercentage>,
-    LengthOrPercentage: From<L>,
-    Self: Copy,
-{
     #[inline]
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
-        if self.has_calc() || other.has_calc() {
+        let this = to_number_or_percentage(self)?;
-            // TODO: We need to treat calc value.
+        let other = to_number_or_percentage(other)?;
-            // https://bugzilla.mozilla.org/show_bug.cgi?id=1386967
-            return Err(());
-        }
-
-        let this = convert_to_number_or_percentage(*self);
-        let other = convert_to_number_or_percentage(*other);
 
         match (this, other) {
             (
                 NumberOrPercentage::Number(ref this),
                 NumberOrPercentage::Number(ref other),
             ) => {
-                Ok(convert_from_number_or_percentage(
+                Ok(SvgLengthOrPercentageOrNumber::Number(
-                    NumberOrPercentage::Number(this.animate(other, procedure)?)
+                    this.animate(other, procedure)?
                 ))
             },
             (
                 NumberOrPercentage::Percentage(ref this),
                 NumberOrPercentage::Percentage(ref other),
             ) => {
-                Ok(convert_from_number_or_percentage(
+                Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                    NumberOrPercentage::Percentage(this.animate(other, procedure)?)
+                    LengthOrPercentage::Percentage(this.animate(other, procedure)?)
                 ))
             },
             _ => Err(()),
@@ -130,6 +71,13 @@ where
     }
 }
 
+impl ComputeSquaredDistance for SvgLengthOrPercentageOrNumber<LengthOrPercentage, Number> {
+    fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
+        to_number_or_percentage(self)?
+            .compute_squared_distance(&to_number_or_percentage(other)?)
+    }
+}
+
 impl<L> Animate for SVGLength<L>
 where
     L: Animate + Clone,

components/style/values/computed/svg.rs

@@ -4,9 +4,8 @@
 
 //! Computed types for SVG properties.
 
-use app_units::Au;
 use values::RGBA;
-use values::computed::{LengthOrPercentage, NonNegativeLength};
+use values::computed::LengthOrPercentage;
 use values::computed::{NonNegativeLengthOrPercentage, NonNegativeNumber, Number};
 use values::computed::Opacity;
 use values::computed::color::Color;
@@ -50,10 +49,11 @@ pub type SvgLengthOrPercentageOrNumber =
 /// <length> | <percentage> | <number> | context-value
 pub type SVGLength = generic::SVGLength<SvgLengthOrPercentageOrNumber>;
 
-impl From<Au> for SVGLength {
+impl SVGLength {
-    fn from(length: Au) -> Self {
+    /// `0px`
+    pub fn zero() -> Self {
         generic::SVGLength::Length(generic::SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-            length.into(),
+            LengthOrPercentage::zero()
         ))
     }
 }
@@ -63,6 +63,8 @@ impl From<Au> for SVGLength {
 pub type NonNegativeSvgLengthOrPercentageOrNumber =
     generic::SvgLengthOrPercentageOrNumber<NonNegativeLengthOrPercentage, NonNegativeNumber>;
 
+// FIXME(emilio): This is really hacky, and can go away with a bit of work on
+// the clone_stroke_width code in gecko.mako.rs.
 impl Into<NonNegativeSvgLengthOrPercentageOrNumber> for SvgLengthOrPercentageOrNumber {
     fn into(self) -> NonNegativeSvgLengthOrPercentageOrNumber {
         match self {
@@ -79,10 +81,12 @@ impl Into<NonNegativeSvgLengthOrPercentageOrNumber> for SvgLengthOrPercentageOrN
 /// An non-negative wrapper of SVGLength.
 pub type SVGWidth = generic::SVGLength<NonNegativeSvgLengthOrPercentageOrNumber>;
 
-impl From<NonNegativeLength> for SVGWidth {
+impl SVGWidth {
-    fn from(length: NonNegativeLength) -> Self {
+    /// `1px`.
+    pub fn one() -> Self {
+        use values::generics::NonNegative;
         generic::SVGLength::Length(generic::SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-            length.into(),
+            NonNegative(LengthOrPercentage::one())
         ))
     }
 }

components/style/values/generics/svg.rs

@@ -8,9 +8,6 @@ use cssparser::Parser;
 use parser::{Parse, ParserContext};
 use style_traits::{ParseError, StyleParseErrorKind};
 use values::{Either, None_};
-use values::computed::NumberOrPercentage;
-use values::computed::length::LengthOrPercentage;
-use values::distance::{ComputeSquaredDistance, SquaredDistance};
 
 /// An SVG paint value
 ///
@@ -152,54 +149,6 @@ pub enum SvgLengthOrPercentageOrNumber<LengthOrPercentage, Number> {
     Number(Number),
 }
 
-impl<L, N> ComputeSquaredDistance for SvgLengthOrPercentageOrNumber<L, N>
-where
-    L: ComputeSquaredDistance + Copy + Into<NumberOrPercentage>,
-    N: ComputeSquaredDistance + Copy + Into<NumberOrPercentage>,
-{
-    #[inline]
-    fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
-        match (self, other) {
-            (
-                &SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref from),
-                &SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref to),
-            ) => from.compute_squared_distance(to),
-            (
-                &SvgLengthOrPercentageOrNumber::Number(ref from),
-                &SvgLengthOrPercentageOrNumber::Number(ref to),
-            ) => from.compute_squared_distance(to),
-            (
-                &SvgLengthOrPercentageOrNumber::LengthOrPercentage(from),
-                &SvgLengthOrPercentageOrNumber::Number(to),
-            ) => from.into().compute_squared_distance(&to.into()),
-            (
-                &SvgLengthOrPercentageOrNumber::Number(from),
-                &SvgLengthOrPercentageOrNumber::LengthOrPercentage(to),
-            ) => from.into().compute_squared_distance(&to.into()),
-        }
-    }
-}
-
-impl<LengthOrPercentageType, NumberType>
-    SvgLengthOrPercentageOrNumber<LengthOrPercentageType, NumberType>
-where
-    LengthOrPercentage: From<LengthOrPercentageType>,
-    LengthOrPercentageType: Copy,
-{
-    /// return true if this struct has calc value.
-    pub fn has_calc(&self) -> bool {
-        match self {
-            &SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop) => {
-                match LengthOrPercentage::from(lop) {
-                    LengthOrPercentage::Calc(_) => true,
-                    _ => false,
-                }
-            },
-            _ => false,
-        }
-    }
-}
-
 /// Parsing the SvgLengthOrPercentageOrNumber. At first, we need to parse number
 /// since prevent converting to the length.
 impl<LengthOrPercentageType: Parse, NumberType: Parse> Parse
@@ -213,10 +162,8 @@ impl<LengthOrPercentageType: Parse, NumberType: Parse> Parse
             return Ok(SvgLengthOrPercentageOrNumber::Number(num));
         }
 
-        if let Ok(lop) = input.try(|i| LengthOrPercentageType::parse(context, i)) {
+        let lop = LengthOrPercentageType::parse(context, input)?;
-            return Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop));
+        Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop))
-        }
-        Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
 }