Bug 1464791 - Add comments for the calculation of Procedure::Add on Scale and transform list. r=birtles
authorBoris Chiou <boris.chiou@gmail.com>
Fri, 16 Nov 2018 19:06:16 +0000
changeset 503297 2c3b27812cbce5ed4514753e529e4fb4d2ca3d4a
parent 503296 dbc9d48aa60b8d31cc1ac90fe41f1fc386ff77cb
child 503298 b9a2dff8c8f5963bd9384f30036731cd65cd3c8d
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1464791
milestone65.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
Bug 1464791 - Add comments for the calculation of Procedure::Add on Scale and transform list. r=birtles Add more comments to let people know the intention of the special case. Differential Revision: https://phabricator.services.mozilla.com/D12070
servo/components/style/values/animated/transform.rs
--- a/servo/components/style/values/animated/transform.rs
+++ b/servo/components/style/values/animated/transform.rs
@@ -846,16 +846,20 @@ fn is_matched_operation(first: &Computed
 }
 
 /// <https://drafts.csswg.org/css-transforms/#interpolation-of-transforms>
 impl Animate for ComputedTransform {
     #[inline]
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
         use std::borrow::Cow;
 
+        // Addition for transforms simply means appending to the list of
+        // transform functions. This is different to how we handle the other
+        // animation procedures so we treat it separately here rather than
+        // handling it in TransformOperation.
         if procedure == Procedure::Add {
             let result = self.0.iter().chain(&other.0).cloned().collect::<Vec<_>>();
             return Ok(Transform(result));
         }
 
         let this = Cow::Borrowed(&self.0);
         let other = Cow::Borrowed(&other.0);
 
@@ -1579,32 +1583,33 @@ impl Animate for ComputedScale {
         &self,
         other: &Self,
         procedure: Procedure,
     ) -> Result<Self, ()> {
         match (self, other) {
             (&Scale::None, &Scale::None) => Ok(Scale::None),
             (&Scale::Scale3D(_, ..), _) | (_, &Scale::Scale3D(_, ..)) => {
                 let (from, to) = (self.resolve(), other.resolve());
-                // FIXME(emilio, bug 1464791): why does this do something different than
-                // Scale3D / TransformOperation::Scale3D?
+                // For transform lists, we add by appending to the list of
+                // transform functions. However, ComputedScale cannot be
+                // simply concatenated, so we have to calculate the additive
+                // result here.
                 if procedure == Procedure::Add {
                     // scale(x1,y1,z1)*scale(x2,y2,z2) = scale(x1*x2, y1*y2, z1*z2)
                     return Ok(Scale::Scale3D(from.0 * to.0, from.1 * to.1, from.2 * to.2));
                 }
                 Ok(Scale::Scale3D(
                     animate_multiplicative_factor(from.0, to.0, procedure)?,
                     animate_multiplicative_factor(from.1, to.1, procedure)?,
                     animate_multiplicative_factor(from.2, to.2, procedure)?,
                 ))
             },
             (&Scale::Scale(_, ..), _) | (_, &Scale::Scale(_, ..)) => {
                 let (from, to) = (self.resolve(), other.resolve());
-                // FIXME(emilio, bug 1464791): why does this do something different than
-                // Scale / TransformOperation::Scale?
+                // As with Scale3D, addition needs special handling.
                 if procedure == Procedure::Add {
                     // scale(x1,y1)*scale(x2,y2) = scale(x1*x2, y1*y2)
                     return Ok(Scale::Scale(from.0 * to.0, from.1 * to.1));
                 }
                 Ok(Scale::Scale(
                     animate_multiplicative_factor(from.0, to.0, procedure)?,
                     animate_multiplicative_factor(from.1, to.1, procedure)?,
                 ))