From 6ce64abe7ecdf5b262c528617c47b1fdb4977c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 20 Jan 2023 00:23:11 +0000 Subject: [PATCH] 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 | 7 +- .../helpers/animated_properties.mako.rs | 123 ---------------- components/style/values/animated/font.rs | 23 +-- components/style/values/animated/grid.rs | 24 +--- components/style/values/animated/lists.rs | 132 ++++++++++++++++++ components/style/values/animated/mod.rs | 1 + components/style/values/animated/svg.rs | 5 +- components/style/values/animated/transform.rs | 3 +- .../style/values/generics/basic_shape.rs | 33 +---- components/style/values/specified/svg_path.rs | 32 ++--- 10 files changed, 162 insertions(+), 221 deletions(-) create mode 100644 components/style/values/animated/lists.rs diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 76c96789a40..4c051ab550f 100644 --- a/components/style/properties/helpers.mako.rs +++ b/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}; + use crate::values::animated::{Animate, ToAnimatedZero, Procedure, lists}; use crate::values::distance::{SquaredDistance, ComputeSquaredDistance}; // FIXME(emilio): For some reason rust thinks that this alias is @@ -301,7 +300,7 @@ procedure: Procedure, ) -> Result { 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 { - self.0.squared_distance_${vector_animation_type}(&other.0) + lists::${vector_animation_type}::squared_distance(&self.0, &other.0) } } % endif diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 72848a1a402..00ccbb2c62d 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/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 : Sized { - /// - fn animate_repeatable_list(&self, other: &Self, procedure: Procedure) -> Result - where - T: Animate; - - /// - fn squared_distance_repeatable_list(&self, other: &Self) -> Result - 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 - 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 - 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 - 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 - 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 - 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 - 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!( for crate::OwnedSlice); -animated_list_impl!( for SmallVec<[T; 1]>); -animated_list_impl!( for Vec); - /// impl Animate for Visibility { #[inline] diff --git a/components/style/values/animated/font.rs b/components/style/values/animated/font.rs index 63a0abf1794..ef0ce0ec6d0 100644 --- a/components/style/values/animated/font.rs +++ b/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; /// /// @@ -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 { - if self.0.len() == other.0.len() { - self.0 - .iter() - .zip(other.0.iter()) - .map(|(st, ot)| st.animate(&ot, procedure)) - .collect::, ()>>() - .map(|v| GenericFontSettings(v.into_boxed_slice())) - } else { - Err(()) - } + let result: Vec<_> = super::lists::by_computed_value::animate(&self.0, &other.0, procedure)?; + Ok(Self(result.into_boxed_slice())) } } impl ComputeSquaredDistance for FontVariationSettings { #[inline] fn compute_squared_distance(&self, other: &Self) -> Result { - if self.0.len() == other.0.len() { - self.0 - .iter() - .zip(other.0.iter()) - .map(|(st, ot)| st.compute_squared_distance(&ot)) - .sum() - } else { - Err(()) - } + super::lists::by_computed_value::squared_distance(&self.0, &other.0) } } diff --git a/components/style/values/animated/grid.rs b/components/style/values/animated/grid.rs index 7b9417a08c6..14e8fc56204 100644 --- a/components/style/values/animated/grid.rs +++ b/components/style/values/animated/grid.rs @@ -81,18 +81,8 @@ impl Animate for generics::TrackRepeat { (_, _) => 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 - .track_sizes - .iter() - .zip(other.track_sizes.iter()) - .map(|(a, b)| a.animate(b, procedure)) - .collect::, _>>()?; + let track_sizes = super::lists::by_computed_value::animate(&self.track_sizes, &other.track_sizes, procedure)?; // The length of |line_names| is always 0 or N+1, where N is the length // of |track_sizes|. Besides, is always discrete. @@ -101,7 +91,7 @@ impl Animate for generics::TrackRepeat { 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 - .values - .iter() - .zip(other.values.iter()) - .map(|(a, b)| a.animate(b, procedure)) - .collect::, _>>()?; + let values = super::lists::by_computed_value::animate(&self.values, &other.values, procedure)?; + // The length of |line_names| is always 0 or N+1, where N is the length // of |track_sizes|. Besides, 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, }) diff --git a/components/style/values/animated/lists.rs b/components/style/values/animated/lists.rs new file mode 100644 index 00000000000..1c688446cc1 --- /dev/null +++ b/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(left: &[T], right: &[T], procedure: Procedure) -> Result + where + T: Animate, + C: FromIterator, + { + 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(left: &[T], right: &[T]) -> Result + 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(left: &[T], right: &[T], procedure: Procedure) -> Result + where + T: Animate + Clone + ToAnimatedZero, + C: FromIterator, + { + 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(left: &[T], right: &[T]) -> Result + 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(left: &[T], right: &[T], procedure: Procedure) -> Result + where + T: Animate, + C: FromIterator, + { + 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(left: &[T], right: &[T]) -> Result + 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() + } +} diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index b36946c4926..4bf31f110f3 100644 --- a/components/style/values/animated/mod.rs +++ b/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; diff --git a/components/style/values/animated/svg.rs b/components/style/values/animated/svg.rs index 13ad10174b5..85f24816481 100644 --- a/components/style/values/animated/svg.rs +++ b/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 { 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(()), } diff --git a/components/style/values/animated/transform.rs b/components/style/values/animated/transform.rs index bb1b4e910cb..d4fb0985f31 100644 --- a/components/style/values/animated/transform.rs +++ b/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 { - 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. diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index 5af1580ffdf..5e1bafb2230 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/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() { - 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::, _>>()? - .into(); + let coordinates = lists::by_computed_value::animate(&self.coordinates, &other.coordinates, procedure)?; 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() { - 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() + lists::by_computed_value::squared_distance(&self.coordinates, &other.coordinates) } } diff --git a/components/style/values/specified/svg_path.rs b/components/style/values/specified/svg_path.rs index b251ce900ce..a6afc28b83a 100644 --- a/components/style/values/specified/svg_path.rs +++ b/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 - .0 - .iter() - .map(|seg| seg.normalize(&mut state)) - .collect::>(); - - SVGPathData(crate::ArcSlice::from_iter(result.into_iter())) + let iter = self.0.iter().map(|seg| seg.normalize(&mut state)); + SVGPathData(crate::ArcSlice::from_iter(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 - .normalize() - .0 - .iter() - .zip(other.normalize().0.iter()) - .map(|(a, b)| a.animate(&b, procedure)) - .collect::, _>>()?; + let left = self.normalize(); + let right = other.normalize(); - 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() - .0 - .iter() - .zip(other.normalize().0.iter()) - .map(|(this, other)| this.compute_squared_distance(&other)) - .sum() + let left = self.normalize(); + let right = other.normalize(); + lists::by_computed_value::squared_distance(&left.0, &right.0) } }