Bug 1374233 - Part 11: Use NonNegativeLengthOrPercentage for vector-like properties. draft
authorBoris Chiou <boris.chiou@gmail.com>
Mon, 24 Jul 2017 15:00:53 +0800
changeset 615002 08021c6fe85f8410f739deb72cef6bded7841b70
parent 615001 21536b9f0691482825763767254aee527d917ffd
child 615003 3321c2b8ddb7d5c80c960906e1499fa339788cc6
push id70205
push userbmo:boris.chiou@gmail.com
push dateTue, 25 Jul 2017 08:53:17 +0000
bugs1374233
milestone56.0a1
Bug 1374233 - Part 11: Use NonNegativeLengthOrPercentage for vector-like properties. stroke-dasyarray and background-size are vector-like properties, and their single_value is NonNegativeLengthOrPercentage. This patch implements ToAnimatedValue for them. MozReview-Commit-ID: DMcvpaqHdy9
servo/components/style/properties/data.py
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/helpers.mako.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/components/style/properties/longhand/background.mako.rs
servo/components/style/properties/longhand/inherited_svg.mako.rs
servo/components/style/values/animated/mod.rs
servo/components/style/values/computed/background.rs
--- a/servo/components/style/properties/data.py
+++ b/servo/components/style/properties/data.py
@@ -152,17 +152,17 @@ def arg_to_bool(arg):
 
 class Longhand(object):
     def __init__(self, style_struct, name, spec=None, animation_value_type=None, derived_from=None, keyword=None,
                  predefined_type=None, custom_cascade=False, experimental=False, internal=False,
                  need_clone=False, need_index=False, gecko_ffi_name=None, depend_on_viewport_size=False,
                  allowed_in_keyframe_block=True, cast_type='u8',
                  has_uncacheable_values=False, logical=False, alias=None, extra_prefixes=None, boxed=False,
                  flags=None, allowed_in_page_rule=False, allow_quirks=False, ignored_when_colors_disabled=False,
-                 gecko_pref_ident=None, vector=False):
+                 gecko_pref_ident=None, vector=False, need_animatable=False):
         self.name = name
         if not spec:
             raise TypeError("Spec should be specified for %s" % name)
         self.spec = spec
         self.keyword = keyword
         self.predefined_type = predefined_type
         self.ident = to_rust_ident(name)
         self.camel_case = to_camel_case(self.ident)
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -4513,41 +4513,48 @@ clip-path
     {
         let v = v.into_iter();
         unsafe {
             bindings::Gecko_nsStyleSVG_SetDashArrayLength(&mut self.gecko, v.len() as u32);
         }
 
         for (mut gecko, servo) in self.gecko.mStrokeDasharray.iter_mut().zip(v) {
             match servo {
-                Either::First(number) => gecko.set_value(CoordDataValue::Factor(number)),
-                Either::Second(lop) => gecko.set(lop),
+                Either::First(number) => gecko.set_value(CoordDataValue::Factor(number.0)),
+                Either::Second(lop) => gecko.set(lop.0),
             }
         }
     }
 
     pub fn copy_stroke_dasharray_from(&mut self, other: &Self) {
         unsafe {
             bindings::Gecko_nsStyleSVG_CopyDashArray(&mut self.gecko, &other.gecko);
         }
     }
 
     pub fn clone_stroke_dasharray(&self) -> longhands::stroke_dasharray::computed_value::T {
-        use values::computed::LengthOrPercentage;
+        use values::computed::{LengthOrPercentage, NonNegativeLengthOrPercentage, NonNegativeNumber};
 
         let mut vec = vec![];
         for gecko in self.gecko.mStrokeDasharray.iter() {
             match gecko.as_value() {
-                CoordDataValue::Factor(number) => vec.push(Either::First(number)),
-                CoordDataValue::Coord(coord) =>
-                    vec.push(Either::Second(LengthOrPercentage::Length(Au(coord)))),
-                CoordDataValue::Percent(p) =>
-                    vec.push(Either::Second(LengthOrPercentage::Percentage(Percentage(p)))),
+                CoordDataValue::Factor(number) => {
+                    vec.push(Either::First(NonNegativeNumber(number)))
+                },
+                CoordDataValue::Coord(coord) => {
+                    vec.push(Either::Second(NonNegativeLengthOrPercentage(
+                        LengthOrPercentage::Length(Au(coord)))))
+                },
+                CoordDataValue::Percent(p) => {
+                    vec.push(Either::Second(NonNegativeLengthOrPercentage(
+                        LengthOrPercentage::Percentage(Percentage(p)))))
+                },
                 CoordDataValue::Calc(calc) =>
-                    vec.push(Either::Second(LengthOrPercentage::Calc(calc.into()))),
+                    vec.push(Either::Second(NonNegativeLengthOrPercentage(
+                        LengthOrPercentage::Calc(calc.into())))),
                 _ => unreachable!(),
             }
         }
         longhands::stroke_dasharray::computed_value::T(vec)
     }
 
     pub fn set_stroke_dashoffset(&mut self, v: longhands::stroke_dashoffset::computed_value::T) {
         match v {
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -71,18 +71,20 @@
 <%doc>
     To be used in cases where we have a grammar like "<thing> [ , <thing> ]*".
 
     Setting allow_empty to False allows for cases where the vector
     is empty. The grammar for these is usually "none | <thing> [ , <thing> ]*".
     We assume that the default/initial value is an empty vector for these.
     `initial_value` need not be defined for these.
 </%doc>
-<%def name="vector_longhand(name, animation_value_type=None, allow_empty=False, separator='Comma', **kwargs)">
-    <%call expr="longhand(name, animation_value_type=animation_value_type, vector=True, **kwargs)">
+<%def name="vector_longhand(name, animation_value_type=None, allow_empty=False, separator='Comma',
+                            need_animatable=False, **kwargs)">
+    <%call expr="longhand(name, animation_value_type=animation_value_type, vector=True,
+                          need_animatable=need_animatable, **kwargs)">
         #[allow(unused_imports)]
         use smallvec::SmallVec;
         use std::fmt;
         #[allow(unused_imports)]
         use style_traits::HasViewportPercentage;
         use style_traits::{Separator, ToCss};
 
         pub mod single_value {
@@ -122,17 +124,17 @@
             pub struct T(
                 % if allow_empty and allow_empty != "NotInitial":
                 pub Vec<single_value::T>,
                 % else:
                 pub SmallVec<[single_value::T; 1]>,
                 % endif
             );
 
-            % if animation_value_type == "ComputedValue":
+            % if need_animatable or animation_value_type == "ComputedValue":
                 use properties::animated_properties::Animatable;
                 use values::animated::ToAnimatedZero;
 
                 impl Animatable for T {
                     fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64)
                         -> Result<Self, ()> {
                         self.0.add_weighted(&other.0, self_portion, other_portion).map(T)
                     }
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -11,20 +11,22 @@ use cssparser::{Parser, RGBA};
 use euclid::{Point2D, Size2D};
 #[cfg(feature = "gecko")] use gecko_bindings::bindings::RawServoAnimationValueMap;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::RawGeckoGfxMatrix4x4;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSPropertyID;
 #[cfg(feature = "gecko")] use gecko_bindings::sugar::ownership::{HasFFI, HasSimpleFFI};
 #[cfg(feature = "gecko")] use gecko_string_cache::Atom;
 use properties::{CSSWideKeyword, PropertyDeclaration};
 use properties::longhands;
+use properties::longhands::background_size::computed_value::T as BackgroundSizeList;
 use properties::longhands::border_spacing::computed_value::T as BorderSpacing;
 use properties::longhands::font_weight::computed_value::T as FontWeight;
 use properties::longhands::font_size_adjust::computed_value::T as FontSizeAdjust;
 use properties::longhands::font_stretch::computed_value::T as FontStretch;
+use properties::longhands::stroke_dasharray::computed_value::T as StrokeDasharray;
 use properties::longhands::transform::computed_value::ComputedMatrix;
 use properties::longhands::transform::computed_value::ComputedOperation as TransformOperation;
 use properties::longhands::transform::computed_value::T as TransformList;
 use properties::longhands::vertical_align::computed_value::T as VerticalAlign;
 use properties::longhands::visibility::computed_value::T as Visibility;
 #[cfg(feature = "gecko")] use properties::{PropertyId, PropertyDeclarationId, LonghandId};
 #[cfg(feature = "gecko")] use properties::{ShorthandId};
 use selectors::parser::SelectorParseError;
@@ -780,16 +782,19 @@ impl ToAnimatedZero for AnimationValue {
             _ => Err(()),
         }
     }
 }
 
 impl RepeatableListAnimatable for LengthOrPercentage {}
 impl RepeatableListAnimatable for Either<f32, LengthOrPercentage> {}
 
+impl RepeatableListAnimatable for NonNegativeLengthOrPercentage {}
+impl RepeatableListAnimatable for Either<NonNegativeNumber, NonNegativeLengthOrPercentage> {}
+
 macro_rules! repeated_vec_impl {
     ($($ty:ty),*) => {
         $(impl<T: RepeatableListAnimatable> Animatable for $ty {
             fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64)
                 -> Result<Self, ()> {
                 // If the length of either list is zero, the least common multiple is undefined.
                 if self.is_empty() || other.is_empty() {
                     return Err(());
--- a/servo/components/style/properties/longhand/background.mako.rs
+++ b/servo/components/style/properties/longhand/background.mako.rs
@@ -162,17 +162,18 @@
                          animation_value_type="discrete",
                          flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER")}
 
 ${helpers.predefined_type("background-size", "BackgroundSize",
     initial_value="computed::LengthOrPercentageOrAuto::Auto.into()",
     initial_specified_value="specified::LengthOrPercentageOrAuto::Auto.into()",
     spec="https://drafts.csswg.org/css-backgrounds/#the-background-size",
     vector=True,
-    animation_value_type="ComputedValue",
+    animation_value_type="BackgroundSizeList",
+    need_animatable=True,
     flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER",
     extra_prefixes="webkit")}
 
 // https://drafts.fxtf.org/compositing/#background-blend-mode
 ${helpers.single_keyword("background-blend-mode",
                          """normal multiply screen overlay darken lighten color-dodge
                             color-burn hard-light soft-light difference exclusion hue
                             saturation color luminosity""",
--- a/servo/components/style/properties/longhand/inherited_svg.mako.rs
+++ b/servo/components/style/properties/longhand/inherited_svg.mako.rs
@@ -84,22 +84,22 @@
                           spec="https://www.w3.org/TR/SVG11/painting.html#StrokeMiterlimitProperty")}
 
 ${helpers.predefined_type("stroke-opacity", "Opacity", "1.0",
                           products="gecko", animation_value_type="ComputedValue",
                           spec="https://www.w3.org/TR/SVG11/painting.html#StrokeOpacityProperty")}
 
 ${helpers.predefined_type(
     "stroke-dasharray",
-    "LengthOrPercentageOrNumber",
+    "NonNegativeLengthOrPercentageOrNumber",
     None,
-    "parse_non_negative",
     vector=True,
     products="gecko",
-    animation_value_type="ComputedValue",
+    animation_value_type="StrokeDasharray",
+    need_animatable=True,
     separator="CommaWithSpace",
     spec="https://www.w3.org/TR/SVG2/painting.html#StrokeDashing",
 )}
 
 ${helpers.predefined_type(
     "stroke-dashoffset", "LengthOrPercentageOrNumber",
     "Either::First(0.0)",
     products="gecko",
--- a/servo/components/style/values/animated/mod.rs
+++ b/servo/components/style/values/animated/mod.rs
@@ -4,16 +4,18 @@
 
 //! Animated values.
 //!
 //! Some values, notably colors, cannot be interpolated directly with their
 //! computed values and need yet another intermediate representation. This
 //! module's raison d'ĂȘtre is to ultimately contain all these types.
 
 use app_units::Au;
+use smallvec::SmallVec;
+use properties::longhands::stroke_dasharray::computed_value::T as StrokeDasharray;
 use values::computed::Angle as ComputedAngle;
 use values::computed::BorderCornerRadius as ComputedBorderCornerRadius;
 use values::computed::NonNegativeAu;
 use values::computed::NonNegativeNumber as ComputedNonNegativeNumber;
 use values::computed::GreaterThanOrEqualToOneNumber as ComputedGreaterThanOrEqualToOneNumber;
 use values::computed::PositiveInteger as ComputedPositiveInteger;
 use values::computed::MaxLength as ComputedMaxLength;
 use values::computed::MozLength as ComputedMozLength;
@@ -65,16 +67,33 @@ where
     }
 
     #[inline]
     fn from_animated_value(animated: Self::AnimatedValue) -> Self {
         animated.into_iter().map(T::from_animated_value).collect()
     }
 }
 
+impl<T> ToAnimatedValue for SmallVec<[T; 1]>
+where
+    T: ToAnimatedValue,
+{
+    type AnimatedValue = SmallVec<[T::AnimatedValue; 1]>;
+
+    #[inline]
+    fn to_animated_value(self) -> Self::AnimatedValue {
+        self.into_iter().map(T::to_animated_value).collect()
+    }
+
+    #[inline]
+    fn from_animated_value(animated: Self::AnimatedValue) -> Self {
+        animated.into_iter().map(T::from_animated_value).collect()
+    }
+}
+
 /// Marker trait for computed values with the same representation during animations.
 pub trait AnimatedValueAsComputed {}
 
 impl AnimatedValueAsComputed for Au {}
 impl AnimatedValueAsComputed for ComputedAngle {}
 impl AnimatedValueAsComputed for SpecifiedUrl {}
 impl AnimatedValueAsComputed for bool {}
 impl AnimatedValueAsComputed for f32 {}
@@ -234,16 +253,30 @@ impl ToAnimatedValue for ComputedMaxLeng
                 };
                 ComputedMaxLength::LengthOrPercentageOrNone(result)
             },
             _ => animated
         }
     }
 }
 
+impl ToAnimatedValue for StrokeDasharray {
+    type AnimatedValue = Self;
+
+    #[inline]
+    fn to_animated_value(self) -> Self {
+        self
+    }
+
+    #[inline]
+    fn from_animated_value(animated: Self::AnimatedValue) -> Self {
+        StrokeDasharray(ToAnimatedValue::from_animated_value(animated.0))
+    }
+}
+
 /// Returns a value similar to `self` that represents zero.
 pub trait ToAnimatedZero: Sized {
     /// Returns a value that, when added with an underlying value, will produce the underlying
     /// value. This is used for SMIL animation's "by-animation" where SMIL first interpolates from
     /// the zero value to the 'by' value, and then adds the result to the underlying value.
     ///
     /// This is not the necessarily the same as the initial value of a property. For example, the
     /// initial value of 'stroke-width' is 1, but the zero value is 0, since adding 1 to the
--- a/servo/components/style/values/computed/background.rs
+++ b/servo/components/style/values/computed/background.rs
@@ -1,16 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Computed types for CSS values related to backgrounds.
 
 use properties::animated_properties::{Animatable, RepeatableListAnimatable};
-use values::animated::ToAnimatedZero;
+use properties::longhands::background_size::computed_value::T as BackgroundSizeList;
+use values::animated::{ToAnimatedValue, ToAnimatedZero};
 use values::computed::length::LengthOrPercentageOrAuto;
 use values::generics::background::BackgroundSize as GenericBackgroundSize;
 
 /// A computed value for the `background-size` property.
 pub type BackgroundSize = GenericBackgroundSize<LengthOrPercentageOrAuto>;
 
 impl RepeatableListAnimatable for BackgroundSize {}
 
@@ -51,8 +52,57 @@ impl Animatable for BackgroundSize {
         }
     }
 }
 
 impl ToAnimatedZero for BackgroundSize {
     #[inline]
     fn to_animated_zero(&self) -> Result<Self, ()> { Err(()) }
 }
+
+impl ToAnimatedValue for BackgroundSize {
+    type AnimatedValue = Self;
+
+    #[inline]
+    fn to_animated_value(self) -> Self {
+        self
+    }
+
+    #[inline]
+    fn from_animated_value(animated: Self::AnimatedValue) -> Self {
+        use app_units::Au;
+        use values::computed::Percentage;
+        let clamp_animated_value = |value: LengthOrPercentageOrAuto| -> LengthOrPercentageOrAuto {
+            match value {
+                LengthOrPercentageOrAuto::Length(len) => {
+                    LengthOrPercentageOrAuto::Length(Au(::std::cmp::max(len.0, 0)))
+                },
+                LengthOrPercentageOrAuto::Percentage(percent) => {
+                    LengthOrPercentageOrAuto::Percentage(Percentage(percent.0.max(0.)))
+                },
+                _ => value
+            }
+        };
+        match animated {
+            GenericBackgroundSize::Explicit { width, height } => {
+                GenericBackgroundSize::Explicit {
+                    width: clamp_animated_value(width),
+                    height: clamp_animated_value(height)
+                }
+            },
+            _ => animated
+        }
+    }
+}
+
+impl ToAnimatedValue for BackgroundSizeList {
+    type AnimatedValue = Self;
+
+    #[inline]
+    fn to_animated_value(self) -> Self {
+        self
+    }
+
+    #[inline]
+    fn from_animated_value(animated: Self::AnimatedValue) -> Self {
+        BackgroundSizeList(ToAnimatedValue::from_animated_value(animated.0))
+    }
+}