style: Clean up list interpolation code

Factor out duplicated / common code to its own module, add / fix spec
links, and clean-up callers.

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

components/style/properties/helpers.mako.rs

@@ -263,8 +263,7 @@
                 Sorry, this is stupid but needed for now.
             % endif
 
-            use crate::properties::animated_properties::ListAnimation;
+            use crate::values::animated::{Animate, ToAnimatedZero, Procedure, lists};
-            use crate::values::animated::{Animate, ToAnimatedZero, Procedure};
             use crate::values::distance::{SquaredDistance, ComputeSquaredDistance};
 
             // FIXME(emilio): For some reason rust thinks that this alias is
@@ -301,7 +300,7 @@
                     procedure: Procedure,
                 ) -> Result<Self, ()> {
                     Ok(OwnedList(
-                        self.0.animate_${vector_animation_type}(&other.0, procedure)?
+                        lists::${vector_animation_type}::animate(&self.0, &other.0, procedure)?
                     ))
                 }
             }
@@ -310,7 +309,7 @@
                     &self,
                     other: &Self,
                 ) -> Result<SquaredDistance, ()> {
-                    self.0.squared_distance_${vector_animation_type}(&other.0)
+                    lists::${vector_animation_type}::squared_distance(&self.0, &other.0)
                 }
             }
             % endif

components/style/properties/helpers/animated_properties.mako.rs

@@ -10,13 +10,11 @@
 %>
 
 #[cfg(feature = "gecko")] use crate::gecko_bindings::structs::nsCSSPropertyID;
-use itertools::{EitherOrBoth, Itertools};
 use crate::properties::{CSSWideKeyword, PropertyDeclaration, NonCustomPropertyIterator};
 use crate::properties::longhands;
 use crate::properties::longhands::visibility::computed_value::T as Visibility;
 use crate::properties::LonghandId;
 use servo_arc::Arc;
-use smallvec::SmallVec;
 use std::ptr;
 use std::mem;
 use fxhash::FxHashMap;
@@ -550,127 +548,6 @@ impl ToAnimatedZero for AnimationValue {
     }
 }
 
-/// A trait to abstract away the different kind of animations over a list that
-/// there may be.
-pub trait ListAnimation<T> : Sized {
-    /// <https://drafts.csswg.org/css-transitions/#animtype-repeatable-list>
-    fn animate_repeatable_list(&self, other: &Self, procedure: Procedure) -> Result<Self, ()>
-    where
-        T: Animate;
-
-    /// <https://drafts.csswg.org/css-transitions/#animtype-repeatable-list>
-    fn squared_distance_repeatable_list(&self, other: &Self) -> Result<SquaredDistance, ()>
-    where
-        T: ComputeSquaredDistance;
-
-    /// This is the animation used for some of the types like shadows and
-    /// filters, where the interpolation happens with the zero value if one of
-    /// the sides is not present.
-    fn animate_with_zero(&self, other: &Self, procedure: Procedure) -> Result<Self, ()>
-    where
-        T: Animate + Clone + ToAnimatedZero;
-
-    /// This is the animation used for some of the types like shadows and
-    /// filters, where the interpolation happens with the zero value if one of
-    /// the sides is not present.
-    fn squared_distance_with_zero(&self, other: &Self) -> Result<SquaredDistance, ()>
-    where
-        T: ToAnimatedZero + ComputeSquaredDistance;
-}
-
-macro_rules! animated_list_impl {
-    (<$t:ident> for $ty:ty) => {
-        impl<$t> ListAnimation<$t> for $ty {
-            fn animate_repeatable_list(
-                &self,
-                other: &Self,
-                procedure: Procedure,
-            ) -> Result<Self, ()>
-            where
-                T: Animate,
-            {
-                // If the length of either list is zero, the least common multiple is undefined.
-                if self.is_empty() || other.is_empty() {
-                    return Err(());
-                }
-                use num_integer::lcm;
-                let len = lcm(self.len(), other.len());
-                self.iter().cycle().zip(other.iter().cycle()).take(len).map(|(this, other)| {
-                    this.animate(other, procedure)
-                }).collect()
-            }
-
-            fn squared_distance_repeatable_list(
-                &self,
-                other: &Self,
-            ) -> Result<SquaredDistance, ()>
-            where
-                T: ComputeSquaredDistance,
-            {
-                if self.is_empty() || other.is_empty() {
-                    return Err(());
-                }
-                use num_integer::lcm;
-                let len = lcm(self.len(), other.len());
-                self.iter().cycle().zip(other.iter().cycle()).take(len).map(|(this, other)| {
-                    this.compute_squared_distance(other)
-                }).sum()
-            }
-
-            fn animate_with_zero(
-                &self,
-                other: &Self,
-                procedure: Procedure,
-            ) -> Result<Self, ()>
-            where
-                T: Animate + Clone + ToAnimatedZero
-            {
-                if procedure == Procedure::Add {
-                    return Ok(
-                        self.iter().chain(other.iter()).cloned().collect()
-                    );
-                }
-                self.iter().zip_longest(other.iter()).map(|it| {
-                    match it {
-                        EitherOrBoth::Both(this, other) => {
-                            this.animate(other, procedure)
-                        },
-                        EitherOrBoth::Left(this) => {
-                            this.animate(&this.to_animated_zero()?, procedure)
-                        },
-                        EitherOrBoth::Right(other) => {
-                            other.to_animated_zero()?.animate(other, procedure)
-                        }
-                    }
-                }).collect()
-            }
-
-            fn squared_distance_with_zero(
-                &self,
-                other: &Self,
-            ) -> Result<SquaredDistance, ()>
-            where
-                T: ToAnimatedZero + ComputeSquaredDistance
-            {
-                self.iter().zip_longest(other.iter()).map(|it| {
-                    match it {
-                        EitherOrBoth::Both(this, other) => {
-                            this.compute_squared_distance(other)
-                        },
-                        EitherOrBoth::Left(list) | EitherOrBoth::Right(list) => {
-                            list.to_animated_zero()?.compute_squared_distance(list)
-                        },
-                    }
-                }).sum()
-            }
-        }
-    }
-}
-
-animated_list_impl!(<T> for crate::OwnedSlice<T>);
-animated_list_impl!(<T> for SmallVec<[T; 1]>);
-animated_list_impl!(<T> for Vec<T>);
-
 /// <https://drafts.csswg.org/web-animations-1/#animating-visibility>
 impl Animate for Visibility {
     #[inline]

components/style/values/animated/font.rs

@@ -7,7 +7,6 @@
 use super::{Animate, Procedure, ToAnimatedZero};
 use crate::values::computed::font::FontVariationSettings;
 use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
-use crate::values::generics::font::FontSettings as GenericFontSettings;
 
 /// <https://drafts.csswg.org/css-fonts-4/#font-variation-settings-def>
 ///
@@ -17,31 +16,15 @@ use crate::values::generics::font::FontSettings as GenericFontSettings;
 impl Animate for FontVariationSettings {
     #[inline]
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
-        if self.0.len() == other.0.len() {
+        let result: Vec<_> = super::lists::by_computed_value::animate(&self.0, &other.0, procedure)?;
-            self.0
+        Ok(Self(result.into_boxed_slice()))
-                .iter()
-                .zip(other.0.iter())
-                .map(|(st, ot)| st.animate(&ot, procedure))
-                .collect::<Result<Vec<_>, ()>>()
-                .map(|v| GenericFontSettings(v.into_boxed_slice()))
-        } else {
-            Err(())
-        }
     }
 }
 
 impl ComputeSquaredDistance for FontVariationSettings {
     #[inline]
     fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
-        if self.0.len() == other.0.len() {
+        super::lists::by_computed_value::squared_distance(&self.0, &other.0)
-            self.0
-                .iter()
-                .zip(other.0.iter())
-                .map(|(st, ot)| st.compute_squared_distance(&ot))
-                .sum()
-        } else {
-            Err(())
-        }
     }
 }
 

components/style/values/animated/grid.rs

@@ -81,18 +81,8 @@ impl Animate for generics::TrackRepeat<LengthPercentage, Integer> {
             (_, _) => return Err(()),
         }
 
-        // The length of track_sizes should be matched.
-        if self.track_sizes.len() != other.track_sizes.len() {
-            return Err(());
-        }
-
         let count = self.count;
-        let track_sizes = self
+        let track_sizes = super::lists::by_computed_value::animate(&self.track_sizes, &other.track_sizes, procedure)?;
-            .track_sizes
-            .iter()
-            .zip(other.track_sizes.iter())
-            .map(|(a, b)| a.animate(b, procedure))
-            .collect::<Result<Vec<_>, _>>()?;
 
         // The length of |line_names| is always 0 or N+1, where N is the length
         // of |track_sizes|. Besides, <line-names> is always discrete.
@@ -101,7 +91,7 @@ impl Animate for generics::TrackRepeat<LengthPercentage, Integer> {
         Ok(generics::TrackRepeat {
             count,
             line_names,
-            track_sizes: track_sizes.into(),
+            track_sizes,
         })
     }
 }
@@ -130,18 +120,14 @@ impl Animate for TrackList {
             return Err(());
         }
 
-        let values = self
+        let values = super::lists::by_computed_value::animate(&self.values, &other.values, procedure)?;
-            .values
+
-            .iter()
-            .zip(other.values.iter())
-            .map(|(a, b)| a.animate(b, procedure))
-            .collect::<Result<Vec<_>, _>>()?;
         // The length of |line_names| is always 0 or N+1, where N is the length
         // of |track_sizes|. Besides, <line-names> is always discrete.
         let line_names = discrete(&self.line_names, &other.line_names, procedure)?;
 
         Ok(TrackList {
-            values: values.into(),
+            values,
             line_names,
             auto_repeat_index: self.auto_repeat_index,
         })

components/style/values/animated/lists.rs

@@ -0,0 +1,132 @@
+/* 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 https://mozilla.org/MPL/2.0/. */
+
+//! Lists have various ways of being animated, this module implements them.
+//!
+//! See https://drafts.csswg.org/web-animations-1/#animating-properties
+
+/// https://drafts.csswg.org/web-animations-1/#by-computed-value
+pub mod by_computed_value {
+    use crate::values::{animated::{Animate, Procedure}, distance::{ComputeSquaredDistance, SquaredDistance}};
+    use std::iter::FromIterator;
+
+    #[allow(missing_docs)]
+    pub fn animate<T, C>(left: &[T], right: &[T], procedure: Procedure) -> Result<C, ()>
+    where
+        T: Animate,
+        C: FromIterator<T>,
+    {
+        if left.len() != right.len() {
+            return Err(())
+        }
+        left.iter().zip(right.iter()).map(|(left, right)| {
+            left.animate(right, procedure)
+        }).collect()
+    }
+
+    #[allow(missing_docs)]
+    pub fn squared_distance<T>(left: &[T], right: &[T]) -> Result<SquaredDistance, ()>
+    where
+        T: ComputeSquaredDistance,
+    {
+        if left.len() != right.len() {
+            return Err(())
+        }
+        left.iter().zip(right.iter()).map(|(left, right)| {
+            left.compute_squared_distance(right)
+        }).sum()
+    }
+}
+
+/// This is the animation used for some of the types like shadows and filters, where the
+/// interpolation happens with the zero value if one of the sides is not present.
+///
+/// https://drafts.csswg.org/web-animations-1/#animating-shadow-lists
+pub mod with_zero {
+    use crate::values::{animated::{Animate, Procedure}, distance::{ComputeSquaredDistance, SquaredDistance}};
+    use crate::values::animated::ToAnimatedZero;
+    use itertools::{EitherOrBoth, Itertools};
+    use std::iter::FromIterator;
+
+    #[allow(missing_docs)]
+    pub fn animate<T, C>(left: &[T], right: &[T], procedure: Procedure) -> Result<C, ()>
+    where
+        T: Animate + Clone + ToAnimatedZero,
+        C: FromIterator<T>,
+    {
+        if procedure == Procedure::Add {
+            return Ok(
+                left.iter().chain(right.iter()).cloned().collect()
+            );
+        }
+        left.iter().zip_longest(right.iter()).map(|it| {
+            match it {
+                EitherOrBoth::Both(left, right) => {
+                    left.animate(right, procedure)
+                },
+                EitherOrBoth::Left(left) => {
+                    left.animate(&left.to_animated_zero()?, procedure)
+                },
+                EitherOrBoth::Right(right) => {
+                    right.to_animated_zero()?.animate(right, procedure)
+                }
+            }
+        }).collect()
+    }
+
+    #[allow(missing_docs)]
+    pub fn squared_distance<T>(left: &[T], right: &[T]) -> Result<SquaredDistance, ()>
+    where
+        T: ToAnimatedZero + ComputeSquaredDistance,
+    {
+        left.iter().zip_longest(right.iter()).map(|it| {
+            match it {
+                EitherOrBoth::Both(left, right) => {
+                    left.compute_squared_distance(right)
+                },
+                EitherOrBoth::Left(item) | EitherOrBoth::Right(item) => {
+                    item.to_animated_zero()?.compute_squared_distance(item)
+                },
+            }
+        }).sum()
+    }
+}
+
+/// https://drafts.csswg.org/web-animations-1/#repeatable-list
+pub mod repeatable_list  {
+    use crate::values::{animated::{Animate, Procedure}, distance::{ComputeSquaredDistance, SquaredDistance}};
+    use std::iter::FromIterator;
+
+    #[allow(missing_docs)]
+    pub fn animate<T, C>(left: &[T], right: &[T], procedure: Procedure) -> Result<C, ()>
+    where
+        T: Animate,
+        C: FromIterator<T>,
+    {
+        use num_integer::lcm;
+        // If the length of either list is zero, the least common multiple is undefined.
+        if left.is_empty() || right.is_empty() {
+            return Err(());
+        }
+        let len = lcm(left.len(), right.len());
+        left.iter().cycle().zip(right.iter().cycle()).take(len).map(|(left, right)| {
+            left.animate(right, procedure)
+        }).collect()
+    }
+
+    #[allow(missing_docs)]
+    pub fn squared_distance<T>(left: &[T], right: &[T]) -> Result<SquaredDistance, ()>
+    where
+        T: ComputeSquaredDistance,
+    {
+        use num_integer::lcm;
+        if left.is_empty() || right.is_empty() {
+            return Err(());
+        }
+        let len = lcm(left.len(), right.len());
+        left.iter().cycle().zip(right.iter().cycle()).take(len).map(|(left, right)| {
+            left.compute_squared_distance(right)
+        }).sum()
+    }
+}

components/style/values/animated/mod.rs

@@ -21,6 +21,7 @@ use std::cmp;
 
 pub mod color;
 pub mod effects;
+pub mod lists;
 mod font;
 mod grid;
 mod svg;

components/style/values/animated/svg.rs

@@ -5,7 +5,6 @@
 //! Animation implementations for various SVG-related types.
 
 use super::{Animate, Procedure};
-use crate::properties::animated_properties::ListAnimation;
 use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
 use crate::values::generics::svg::SVGStrokeDashArray;
 
@@ -22,7 +21,7 @@ where
         }
         match (self, other) {
             (&SVGStrokeDashArray::Values(ref this), &SVGStrokeDashArray::Values(ref other)) => Ok(
-                SVGStrokeDashArray::Values(this.animate_repeatable_list(other, procedure)?),
+                SVGStrokeDashArray::Values(super::lists::repeatable_list::animate(this, other, procedure)?),
             ),
             _ => Err(()),
         }
@@ -37,7 +36,7 @@ where
     fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
         match (self, other) {
             (&SVGStrokeDashArray::Values(ref this), &SVGStrokeDashArray::Values(ref other)) => {
-                this.squared_distance_repeatable_list(other)
+                super::lists::repeatable_list::squared_distance(this, other)
             },
             _ => Err(()),
         }

components/style/values/animated/transform.rs

@@ -8,7 +8,6 @@
 
 use super::animate_multiplicative_factor;
 use super::{Animate, Procedure, ToAnimatedZero};
-use crate::properties::animated_properties::ListAnimation;
 use crate::values::computed::transform::Rotate as ComputedRotate;
 use crate::values::computed::transform::Scale as ComputedScale;
 use crate::values::computed::transform::Transform as ComputedTransform;
@@ -947,7 +946,7 @@ impl Animate for ComputedTransform {
 impl ComputeSquaredDistance for ComputedTransform {
     #[inline]
     fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
-        let squared_dist = self.0.squared_distance_with_zero(&other.0);
+        let squared_dist = super::lists::with_zero::squared_distance(&self.0, &other.0);
 
         // Roll back to matrix interpolation if there is any Err(()) in the
         // transform lists, such as mismatched transform functions.

components/style/values/generics/basic_shape.rs

@@ -5,7 +5,7 @@
 //! CSS handling for the [`basic-shape`](https://drafts.csswg.org/css-shapes/#typedef-basic-shape)
 //! types that are generic over their `ToCss` implementations.
 
-use crate::values::animated::{Animate, Procedure, ToAnimatedZero};
+use crate::values::animated::{Animate, Procedure, ToAnimatedZero, lists};
 use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
 use crate::values::generics::border::GenericBorderRadius;
 use crate::values::generics::position::GenericPosition;
@@ -320,7 +320,9 @@ pub use self::GenericPolygon as Polygon;
 
 /// Coordinates for Polygon.
 #[derive(
+    Animate,
     Clone,
+    ComputeSquaredDistance,
     Debug,
     MallocSizeOf,
     PartialEq,
@@ -477,21 +479,7 @@ where
         if self.fill != other.fill {
             return Err(());
         }
-        if self.coordinates.len() != other.coordinates.len() {
+        let coordinates = lists::by_computed_value::animate(&self.coordinates, &other.coordinates, procedure)?;
-            return Err(());
-        }
-        let coordinates = self
-            .coordinates
-            .iter()
-            .zip(other.coordinates.iter())
-            .map(|(this, other)| {
-                Ok(PolygonCoord(
-                    this.0.animate(&other.0, procedure)?,
-                    this.1.animate(&other.1, procedure)?,
-                ))
-            })
-            .collect::<Result<Vec<_>, _>>()?
-            .into();
         Ok(Polygon {
             fill: self.fill,
             coordinates,
@@ -507,18 +495,7 @@ where
         if self.fill != other.fill {
             return Err(());
         }
-        if self.coordinates.len() != other.coordinates.len() {
+        lists::by_computed_value::squared_distance(&self.coordinates, &other.coordinates)
-            return Err(());
-        }
-        self.coordinates
-            .iter()
-            .zip(other.coordinates.iter())
-            .map(|(this, other)| {
-                let d1 = this.0.compute_squared_distance(&other.0)?;
-                let d2 = this.1.compute_squared_distance(&other.1)?;
-                Ok(d1 + d2)
-            })
-            .sum()
     }
 }
 

components/style/values/specified/svg_path.rs

@@ -5,7 +5,7 @@
 //! Specified types for SVG Path.
 
 use crate::parser::{Parse, ParserContext};
-use crate::values::animated::{Animate, Procedure, ToAnimatedZero};
+use crate::values::animated::{Animate, Procedure, ToAnimatedZero, lists};
 use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
 use crate::values::CSSFloat;
 use cssparser::Parser;
@@ -52,13 +52,8 @@ impl SVGPathData {
             subpath_start: CoordPair::new(0.0, 0.0),
             pos: CoordPair::new(0.0, 0.0),
         };
-        let result = self
+        let iter = self.0.iter().map(|seg| seg.normalize(&mut state));
-            .0
+        SVGPathData(crate::ArcSlice::from_iter(iter))
-            .iter()
-            .map(|seg| seg.normalize(&mut state))
-            .collect::<Vec<_>>();
-
-        SVGPathData(crate::ArcSlice::from_iter(result.into_iter()))
     }
 
     // FIXME: Bug 1714238, we may drop this once we use the same data structure for both SVG and
@@ -220,15 +215,11 @@ impl Animate for SVGPathData {
         // FIXME(emilio): This allocates three copies of the path, that's not
         // great! Specially, once we're normalized once, we don't need to
         // re-normalize again.
-        let result = self
+        let left = self.normalize();
-            .normalize()
+        let right = other.normalize();
-            .0
-            .iter()
-            .zip(other.normalize().0.iter())
-            .map(|(a, b)| a.animate(&b, procedure))
-            .collect::<Result<Vec<_>, _>>()?;
 
-        Ok(SVGPathData(crate::ArcSlice::from_iter(result.into_iter())))
+        let items: Vec<_> = lists::by_computed_value::animate(&left.0, &right.0, procedure)?;
+        Ok(SVGPathData(crate::ArcSlice::from_iter(items.into_iter())))
     }
 }
 
@@ -237,12 +228,9 @@ impl ComputeSquaredDistance for SVGPathData {
         if self.0.len() != other.0.len() {
             return Err(());
         }
-        self.normalize()
+        let left = self.normalize();
-            .0
+        let right = other.normalize();
-            .iter()
+        lists::by_computed_value::squared_distance(&left.0, &right.0)
-            .zip(other.normalize().0.iter())
-            .map(|(this, other)| this.compute_squared_distance(&other))
-            .sum()
     }
 }