servo: Merge #17898 - stylo: Fix compute_squared_distance for AnimatedFilterList (from BorisChiou:stylo/animation/filter_distance); r=birtles
authorBoris Chiou <boris.chiou@gmail.com>
Fri, 28 Jul 2017 02:08:31 -0500
changeset 420261 e65c70c5c833aed67f952323a971554b1fe05c6e
parent 420260 18cb88892539e7dd4c5b9076cb78cff0283261e1
child 420262 8079b6c0ef641059e716d897f23fabeeeeecd396
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs17898, 1384014
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #17898 - stylo: Fix compute_squared_distance for AnimatedFilterList (from BorisChiou:stylo/animation/filter_distance); r=birtles We implement compute_distance for Angle to avoid returning Err(()) from it, and then rewrite compute_squared_distance of AnimatedFilterLIst to avoid using unwrap() and make it simpler. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix [Bug 1384014](https://bugzilla.mozilla.org/show_bug.cgi?id=1384014). - [X] These changes do not require tests because there is a DevTools test for this already. Source-Repo: https://github.com/servo/servo Source-Revision: 3ab6b1cf8fa64cb31e169eece9e5e8224e195149
servo/Cargo.lock
servo/components/style/Cargo.toml
servo/components/style/lib.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
--- a/servo/Cargo.lock
+++ b/servo/Cargo.lock
@@ -3006,16 +3006,17 @@ dependencies = [
  "cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "cssparser 0.18.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "encoding 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "heapsize_derive 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "html5ever 0.18.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itertools 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)",
  "itoa 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "nsstring_vendor 0.1.0",
  "num-integer 0.1.34 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-traits 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)",
--- a/servo/components/style/Cargo.toml
+++ b/servo/components/style/Cargo.toml
@@ -39,16 +39,17 @@ bit-vec = "0.4.3"
 byteorder = "1.0"
 cfg-if = "0.1.0"
 cssparser = "0.18"
 encoding = {version = "0.2", optional = true}
 euclid = "0.15"
 fnv = "1.0"
 heapsize = {version = "0.4", optional = true}
 heapsize_derive = {version = "0.1", optional = true}
+itertools = "0.5"
 itoa = "0.3"
 html5ever = {version = "0.18", optional = true}
 lazy_static = "0.2"
 log = "0.3"
 matches = "0.1"
 nsstring_vendor = {path = "gecko_bindings/nsstring_vendor", optional = true}
 num_cpus = {version = "1.1.0", optional = true}
 num-integer = "0.1.32"
--- a/servo/components/style/lib.rs
+++ b/servo/components/style/lib.rs
@@ -46,16 +46,17 @@ extern crate bitflags;
 #[allow(unused_extern_crates)] extern crate byteorder;
 #[cfg(feature = "gecko")] #[macro_use] #[no_link] extern crate cfg_if;
 #[macro_use] extern crate cssparser;
 extern crate euclid;
 extern crate fnv;
 #[cfg(feature = "gecko")] #[macro_use] pub mod gecko_string_cache;
 #[cfg(feature = "servo")] extern crate heapsize;
 #[cfg(feature = "servo")] #[macro_use] extern crate heapsize_derive;
+extern crate itertools;
 extern crate itoa;
 #[cfg(feature = "servo")] #[macro_use] extern crate html5ever;
 #[macro_use]
 extern crate lazy_static;
 #[macro_use]
 extern crate log;
 #[allow(unused_extern_crates)]
 #[macro_use]
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -920,16 +920,23 @@ impl Animatable for Angle {
             % endfor
             _ => {
                 self.radians()
                     .add_weighted(&other.radians(), self_portion, other_portion)
                     .map(Angle::from_radians)
             }
         }
     }
+
+    #[inline]
+    fn compute_distance(&self, other: &Self) -> Result<f64, ()> {
+        // Use the formula for calculating the distance between angles defined in SVG:
+        // https://www.w3.org/TR/SVG/animate.html#complexDistances
+        Ok((self.radians64() - other.radians64()).abs())
+    }
 }
 
 /// https://drafts.csswg.org/css-transitions/#animtype-percentage
 impl Animatable for Percentage {
     #[inline]
     fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64) -> Result<Self, ()> {
         Ok(Percentage((self.0 as f64 * self_portion + other.0 as f64 * other_portion) as f32))
     }
@@ -3167,47 +3174,31 @@ impl Animatable for AnimatedFilterList {
 
     #[inline]
     fn compute_distance(&self, other: &Self) -> Result<f64, ()> {
         self.compute_squared_distance(other).map(|sd| sd.sqrt())
     }
 
     #[inline]
     fn compute_squared_distance(&self, other: &Self) -> Result<f64, ()> {
-        let mut square_distance: f64 = 0.0;
-        let mut from_iter = self.0.iter();
-        let mut to_iter = other.0.iter();
-
-        let mut from = from_iter.next();
-        let mut to = to_iter.next();
-        while from.is_some() || to.is_some() {
-            let current_square_distance: f64 ;
-            if from.is_none() {
-                let none = try!(add_weighted_filter_function(to, to, 0.0, 0.0));
-                current_square_distance =
-                    compute_filter_square_distance(&none, &(to.unwrap())).unwrap();
+        use itertools::{EitherOrBoth, Itertools};
 
-                to = to_iter.next();
-            } else if to.is_none() {
-                let none = try!(add_weighted_filter_function(from, from, 0.0, 0.0));
-                current_square_distance =
-                    compute_filter_square_distance(&none, &(from.unwrap())).unwrap();
-
-                from = from_iter.next();
-            } else {
-                current_square_distance =
-                    compute_filter_square_distance(&(from.unwrap()),
-                                                   &(to.unwrap())).unwrap();
-
-                from = from_iter.next();
-                to = to_iter.next();
-            }
-            square_distance += current_square_distance;
+        let mut square_distance: f64 = 0.0;
+        for it in self.0.iter().zip_longest(other.0.iter()) {
+            square_distance += match it {
+                EitherOrBoth::Both(from, to) => {
+                    compute_filter_square_distance(&from, &to)?
+                },
+                EitherOrBoth::Left(list) | EitherOrBoth::Right(list)=> {
+                    let none = add_weighted_filter_function(Some(list), Some(list), 0.0, 0.0)?;
+                    compute_filter_square_distance(&none, &list)?
+                },
+            };
         }
-        Ok(square_distance.sqrt())
+        Ok(square_distance)
     }
 }
 
 /// A comparator to sort PropertyIds such that longhands are sorted before shorthands,
 /// shorthands with fewer components are sorted before shorthands with more components,
 /// and otherwise shorthands are sorted by IDL name as defined by [Web Animations][property-order].
 ///
 /// Using this allows us to prioritize values specified by longhands (or smaller