Bug 1532134 - Remove Options from TransformOperation. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 03 Mar 2019 11:31:54 +0000
changeset 520058 9506816277411c422b23f1b74b2e058dda615d6d
parent 520057 2b811ffbed9b8b4c71092e90cbb11201c8d9a7c6
child 520059 325cacd860797a527eddfb6652984169a3882523
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1532134
milestone67.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 1532134 - Remove Options from TransformOperation. r=xidorn This may or may not be part of the plan to get rid of nsCSSValue ;) Option is not usable via FFI, and they should not be needed (we should be following the shortest serialization principle instead). These patches also do that, which matches the other transform properties. I think that slight change is fine, if we can make it work, and consistent with other properties. Alternative is adding more TransformOperation variants or such, which I rather not do. Differential Revision: https://phabricator.services.mozilla.com/D21862
devtools/client/memory/test/browser/browser_memory_tree_map-02.js
servo/components/style/properties/gecko.mako.rs
servo/components/style/values/animated/transform.rs
servo/components/style/values/computed/transform.rs
servo/components/style/values/generics/transform.rs
servo/components/style/values/specified/angle.rs
servo/components/style/values/specified/transform.rs
testing/web-platform/tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html
testing/web-platform/tests/css/css-transforms/parsing/transform-valid.html
--- a/devtools/client/memory/test/browser/browser_memory_tree_map-02.js
+++ b/devtools/client/memory/test/browser/browser_memory_tree_map-02.js
@@ -35,25 +35,25 @@ this.test = makeMemoryTest(TEST_URL, asy
   const dragZoom = new DragZoom(canvases.container, 0, rafMock.raf);
   const style = canvases.container.style;
 
   info("Check initial state of dragZoom");
   {
     is(dragZoom.zoom, 0, "Zooming starts at 0");
     is(dragZoom.smoothZoom, 0, "Smoothed zooming starts at 0");
     is(rafMock.timesCalled, 0, "No RAFs have been queued");
-    is(style.transform, "translate(0px, 0px) scale(1)",
+    is(style.transform, "translate(0px) scale(1)",
        "No transforms have been done.");
 
     canvases.container.dispatchEvent(new WheelEvent("wheel", {
       deltaY: -PIXEL_DELTA,
       deltaMode: PIXEL_SCROLL_MODE,
     }));
 
-    is(style.transform, "translate(0px, 0px) scale(1.05)",
+    is(style.transform, "translate(0px) scale(1.05)",
        "The div has been slightly scaled.");
     is(dragZoom.zoom, PIXEL_DELTA * dragZoom.ZOOM_SPEED,
       "The zoom was increased");
     ok(floatEquality(dragZoom.smoothZoom, 0.05),
       "The smooth zoom is between the initial value and the target");
     is(rafMock.timesCalled, 1, "A RAF has been queued");
   }
 
@@ -63,17 +63,17 @@ this.test = makeMemoryTest(TEST_URL, asy
     let lastCallCount;
     for (i = 0; i < MAX_RAF_LOOP; i++) {
       if (lastCallCount === rafMock.timesCalled) {
         break;
       }
       lastCallCount = rafMock.timesCalled;
       rafMock.nextFrame();
     }
-    is(style.transform, "translate(0px, 0px) scale(1.1)",
+    is(style.transform, "translate(0px) scale(1.1)",
        "The scale has been fully applied");
     is(dragZoom.zoom, dragZoom.smoothZoom,
       "The smooth and target zoom values match");
     isnot(MAX_RAF_LOOP, i,
       "The RAF loop correctly stopped");
   }
 
   info("Dragging correctly translates the div");
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -850,43 +850,42 @@ def set_gecko_property(ffi_name, expr):
         })
     }
 </%def>
 
 <%
 transform_functions = [
     ("Matrix3D", "matrix3d", ["number"] * 16),
     ("Matrix", "matrix", ["number"] * 6),
-    ("Translate", "translate", ["lp", "optional_lp"]),
+    ("Translate", "translate", ["lp", "lp"]),
     ("Translate3D", "translate3d", ["lp", "lp", "length"]),
     ("TranslateX", "translatex", ["lp"]),
     ("TranslateY", "translatey", ["lp"]),
     ("TranslateZ", "translatez", ["length"]),
     ("Scale3D", "scale3d", ["number"] * 3),
-    ("Scale", "scale", ["number", "optional_number"]),
+    ("Scale", "scale", ["number", "number"]),
     ("ScaleX", "scalex", ["number"]),
     ("ScaleY", "scaley", ["number"]),
     ("ScaleZ", "scalez", ["number"]),
     ("Rotate", "rotate", ["angle"]),
     ("Rotate3D", "rotate3d", ["number"] * 3 + ["angle"]),
     ("RotateX", "rotatex", ["angle"]),
     ("RotateY", "rotatey", ["angle"]),
     ("RotateZ", "rotatez", ["angle"]),
-    ("Skew", "skew", ["angle", "optional_angle"]),
+    ("Skew", "skew", ["angle", "angle"]),
     ("SkewX", "skewx", ["angle"]),
     ("SkewY", "skewy", ["angle"]),
     ("Perspective", "perspective", ["length"]),
     ("InterpolateMatrix", "interpolatematrix", ["list"] * 2 + ["percentage"]),
     ("AccumulateMatrix", "accumulatematrix", ["list"] * 2 + ["integer_to_percentage"])
 ]
 %>
 
 <%def name="transform_function_arm(name, keyword, items)">
     <%
-        has_optional = items[-1].startswith("optional_")
         pattern = None
         if keyword == "matrix3d":
             # m11: number1, m12: number2, ..
             single_patterns = ["m%s: %s" % (str(a / 4 + 1) + str(a % 4 + 1), b + str(a + 1)) for (a, b)
                                 in enumerate(items)]
             pattern = "(Matrix3D { %s })" % ", ".join(single_patterns)
         elif keyword == "matrix":
             # a: number1, b: number2, ..
@@ -914,58 +913,40 @@ transform_functions = [
             "number" : "bindings::Gecko_CSSValue_SetNumber(%s, %s)",
             # Note: We use nsCSSValueSharedList here, instead of nsCSSValueList_heap
             #       because this function is not called on the main thread and
             #       nsCSSValueList_heap is not thread safe.
             "list" : "%s.set_shared_list(%s.0.iter().map(&convert_to_ns_css_value));",
         }
     %>
     crate::values::generics::transform::TransformOperation::${name}${pattern} => {
-        % if has_optional:
-            let optional_present = ${items[-1] + str(len(items))}.is_some();
-            let len = if optional_present {
-                ${len(items) + 1}
-            } else {
-                ${len(items)}
-            };
-        % else:
-            let len = ${len(items) + 1};
-        % endif
+        let len = ${len(items) + 1};
         bindings::Gecko_CSSValue_SetFunction(gecko_value, len);
         bindings::Gecko_CSSValue_SetKeyword(
             bindings::Gecko_CSSValue_GetArrayItem(gecko_value, 0),
             structs::nsCSSKeyword::eCSSKeyword_${keyword}
         );
         % for index, item in enumerate(items):
-            <% replaced_item = item.replace("optional_", "") %>
-            % if item.startswith("optional"):
-                if let Some(${replaced_item + str(index + 1)}) = ${item + str(index + 1)} {
-            % endif
             % if item == "list":
                 debug_assert!(!${item}${index + 1}.0.is_empty());
             % endif
-            ${css_value_setters[replaced_item] % (
+            ${css_value_setters[item] % (
                 "bindings::Gecko_CSSValue_GetArrayItem(gecko_value, %d)" % (index + 1),
-                replaced_item + str(index + 1)
+                item + str(index + 1)
             )};
-            % if item.startswith("optional"):
-                }
-            % endif
         % endfor
     }
 </%def>
 
 <%def name="computed_operation_arm(name, keyword, items)">
     <%
         # %s is substituted with the call to GetArrayItem.
         css_value_getters = {
             "length" : "Length::new(bindings::Gecko_CSSValue_GetNumber(%s))",
             "lp" : "%s.get_length_percentage()",
-            "lpon" : "Either::Second(%s.get_length_percentage())",
-            "lon" : "Either::First(%s.get_length())",
             "angle" : "%s.get_angle()",
             "number" : "bindings::Gecko_CSSValue_GetNumber(%s)",
             "percentage" : "Percentage(bindings::Gecko_CSSValue_GetPercentage(%s))",
             "integer_to_percentage" : "bindings::Gecko_CSSValue_GetPercentage(%s) as i32",
             "list" : "Transform(convert_shared_list_to_operations(%s))",
         }
         pre_symbols = "("
         post_symbols = ")"
@@ -993,30 +974,21 @@ transform_functions = [
             % if keyword == "matrix3d":
                 m${index / 4 + 1}${index % 4 + 1}:
             % elif keyword == "matrix":
                 ${chr(ord('a') + index)}:
             % elif keyword == "interpolatematrix" or keyword == "accumulatematrix":
                 ${field_names[index]}:
             % endif
             <%
-                getter = css_value_getters[item.replace("optional_", "")] % (
+                getter = css_value_getters[item] % (
                     "bindings::Gecko_CSSValue_GetArrayItemConst(gecko_value, %d)" % (index + 1)
                 )
             %>
-            % if item.startswith("optional_"):
-                if (**gecko_value.mValue.mArray.as_ref()).mCount == ${index + 1} {
-                    None
-                } else {
-                    Some(${getter})
-                }
-            % else:
-                ${getter}
-            % endif
-,
+            ${getter},
         % endfor
         ${post_symbols}
     },
 </%def>
 
 fn set_single_transform_function(
     servo_value: &values::computed::TransformOperation,
     gecko_value: &mut structs::nsCSSValue /* output */
--- a/servo/components/style/values/animated/transform.rs
+++ b/servo/components/style/values/animated/transform.rs
@@ -998,59 +998,43 @@ impl Animate for ComputedTransformOperat
             (&TransformOperation::Matrix3D(ref this), &TransformOperation::Matrix3D(ref other)) => {
                 Ok(TransformOperation::Matrix3D(
                     this.animate(other, procedure)?,
                 ))
             },
             (&TransformOperation::Matrix(ref this), &TransformOperation::Matrix(ref other)) => {
                 Ok(TransformOperation::Matrix(this.animate(other, procedure)?))
             },
-            (&TransformOperation::Skew(ref fx, None), &TransformOperation::Skew(ref tx, None)) => {
-                Ok(TransformOperation::Skew(fx.animate(tx, procedure)?, None))
-            },
             (
                 &TransformOperation::Skew(ref fx, ref fy),
                 &TransformOperation::Skew(ref tx, ref ty),
             ) => Ok(TransformOperation::Skew(
                 fx.animate(tx, procedure)?,
-                Some(
-                    fy.unwrap_or(Angle::zero())
-                        .animate(&ty.unwrap_or(Angle::zero()), procedure)?,
-                ),
+                fy.animate(ty, procedure)?,
             )),
             (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) => {
                 Ok(TransformOperation::SkewX(f.animate(t, procedure)?))
             },
             (&TransformOperation::SkewY(ref f), &TransformOperation::SkewY(ref t)) => {
                 Ok(TransformOperation::SkewY(f.animate(t, procedure)?))
             },
             (
                 &TransformOperation::Translate3D(ref fx, ref fy, ref fz),
                 &TransformOperation::Translate3D(ref tx, ref ty, ref tz),
             ) => Ok(TransformOperation::Translate3D(
                 fx.animate(tx, procedure)?,
                 fy.animate(ty, procedure)?,
                 fz.animate(tz, procedure)?,
             )),
             (
-                &TransformOperation::Translate(ref fx, None),
-                &TransformOperation::Translate(ref tx, None),
-            ) => Ok(TransformOperation::Translate(
-                fx.animate(tx, procedure)?,
-                None,
-            )),
-            (
                 &TransformOperation::Translate(ref fx, ref fy),
                 &TransformOperation::Translate(ref tx, ref ty),
             ) => Ok(TransformOperation::Translate(
                 fx.animate(tx, procedure)?,
-                Some(
-                    fy.unwrap_or(LengthPercentage::zero())
-                        .animate(&ty.unwrap_or(LengthPercentage::zero()), procedure)?,
-                ),
+                fy.animate(ty, procedure)?,
             )),
             (&TransformOperation::TranslateX(ref f), &TransformOperation::TranslateX(ref t)) => {
                 Ok(TransformOperation::TranslateX(f.animate(t, procedure)?))
             },
             (&TransformOperation::TranslateY(ref f), &TransformOperation::TranslateY(ref t)) => {
                 Ok(TransformOperation::TranslateY(f.animate(t, procedure)?))
             },
             (&TransformOperation::TranslateZ(ref f), &TransformOperation::TranslateZ(ref t)) => {
@@ -1068,32 +1052,22 @@ impl Animate for ComputedTransformOperat
                 TransformOperation::ScaleX(animate_multiplicative_factor(*f, *t, procedure)?),
             ),
             (&TransformOperation::ScaleY(ref f), &TransformOperation::ScaleY(ref t)) => Ok(
                 TransformOperation::ScaleY(animate_multiplicative_factor(*f, *t, procedure)?),
             ),
             (&TransformOperation::ScaleZ(ref f), &TransformOperation::ScaleZ(ref t)) => Ok(
                 TransformOperation::ScaleZ(animate_multiplicative_factor(*f, *t, procedure)?),
             ),
-            (&TransformOperation::Scale(ref f, None), &TransformOperation::Scale(ref t, None)) => {
-                Ok(TransformOperation::Scale(
-                    animate_multiplicative_factor(*f, *t, procedure)?,
-                    None,
-                ))
-            },
             (
                 &TransformOperation::Scale(ref fx, ref fy),
                 &TransformOperation::Scale(ref tx, ref ty),
             ) => Ok(TransformOperation::Scale(
                 animate_multiplicative_factor(*fx, *tx, procedure)?,
-                Some(animate_multiplicative_factor(
-                    fy.unwrap_or(*fx),
-                    ty.unwrap_or(*tx),
-                    procedure,
-                )?),
+                animate_multiplicative_factor(*fy, *ty, procedure)?,
             )),
             (
                 &TransformOperation::Rotate3D(fx, fy, fz, fa),
                 &TransformOperation::Rotate3D(tx, ty, tz, ta),
             ) => {
                 let animated = Rotate::Rotate3D(fx, fy, fz, fa)
                     .animate(&Rotate::Rotate3D(tx, ty, tz, ta), procedure)?;
                 let (fx, fy, fz, fa) = ComputedRotate::resolve(&animated);
--- a/servo/components/style/values/computed/transform.rs
+++ b/servo/components/style/values/computed/transform.rs
@@ -366,25 +366,24 @@ impl From<Transform3D<CSSFloat>> for Mat
 
 impl TransformOperation {
     /// Convert to a Translate3D.
     ///
     /// Must be called on a Translate function
     pub fn to_translate_3d(&self) -> Self {
         match *self {
             generic::TransformOperation::Translate3D(..) => self.clone(),
-            generic::TransformOperation::TranslateX(ref x) |
-            generic::TransformOperation::Translate(ref x, None) => {
+            generic::TransformOperation::TranslateX(ref x) => {
                 generic::TransformOperation::Translate3D(
                     x.clone(),
                     LengthPercentage::zero(),
                     Length::zero(),
                 )
             },
-            generic::TransformOperation::Translate(ref x, Some(ref y)) => {
+            generic::TransformOperation::Translate(ref x, ref y) => {
                 generic::TransformOperation::Translate3D(x.clone(), y.clone(), Length::zero())
             },
             generic::TransformOperation::TranslateY(ref y) => {
                 generic::TransformOperation::Translate3D(
                     LengthPercentage::zero(),
                     y.clone(),
                     Length::zero(),
                 )
@@ -421,20 +420,17 @@ impl TransformOperation {
     }
 
     /// Convert to a Scale3D.
     ///
     /// Must be called on a Scale function
     pub fn to_scale_3d(&self) -> Self {
         match *self {
             generic::TransformOperation::Scale3D(..) => self.clone(),
-            generic::TransformOperation::Scale(s, None) => {
-                generic::TransformOperation::Scale3D(s, s, 1.)
-            },
-            generic::TransformOperation::Scale(x, Some(y)) => {
+            generic::TransformOperation::Scale(x, y) => {
                 generic::TransformOperation::Scale3D(x, y, 1.)
             },
             generic::TransformOperation::ScaleX(x) => {
                 generic::TransformOperation::Scale3D(x, 1., 1.)
             },
             generic::TransformOperation::ScaleY(y) => {
                 generic::TransformOperation::Scale3D(1., y, 1.)
             },
@@ -489,17 +485,17 @@ impl ToAnimatedZero for TransformOperati
             ),
             generic::TransformOperation::TranslateZ(ref t) => Ok(
                 generic::TransformOperation::TranslateZ(t.to_animated_zero()?),
             ),
             generic::TransformOperation::Scale3D(..) => {
                 Ok(generic::TransformOperation::Scale3D(1.0, 1.0, 1.0))
             },
             generic::TransformOperation::Scale(_, _) => {
-                Ok(generic::TransformOperation::Scale(1.0, Some(1.0)))
+                Ok(generic::TransformOperation::Scale(1.0, 1.0))
             },
             generic::TransformOperation::ScaleX(..) => Ok(generic::TransformOperation::ScaleX(1.0)),
             generic::TransformOperation::ScaleY(..) => Ok(generic::TransformOperation::ScaleY(1.0)),
             generic::TransformOperation::ScaleZ(..) => Ok(generic::TransformOperation::ScaleZ(1.0)),
             generic::TransformOperation::Rotate3D(x, y, z, a) => {
                 let (x, y, z, _) = generic::get_normalized_vector_and_angle(x, y, z, a);
                 Ok(generic::TransformOperation::Rotate3D(
                     x,
@@ -580,28 +576,28 @@ impl Rotate {
 pub type Translate = generic::Translate<LengthPercentage, Length>;
 
 impl Translate {
     /// Convert TransformOperation to Translate.
     pub fn to_transform_operation(&self) -> Option<TransformOperation> {
         match *self {
             generic::Translate::None => None,
             generic::Translate::Translate(tx, ty) => {
-                Some(generic::TransformOperation::Translate(tx, Some(ty)))
+                Some(generic::TransformOperation::Translate(tx, ty))
             },
             generic::Translate::Translate3D(tx, ty, tz) => {
                 Some(generic::TransformOperation::Translate3D(tx, ty, tz))
             },
         }
     }
 
     /// Convert Translate to TransformOperation.
     pub fn from_transform_operation(operation: &TransformOperation) -> Translate {
         match *operation {
-            generic::TransformOperation::Translate(tx, Some(ty)) => {
+            generic::TransformOperation::Translate(tx, ty) => {
                 generic::Translate::Translate(tx, ty)
             },
             generic::TransformOperation::Translate3D(tx, ty, tz) => {
                 generic::Translate::Translate3D(tx, ty, tz)
             },
             _ => unreachable!("Found unexpected value for translate"),
         }
     }
@@ -610,25 +606,24 @@ impl Translate {
 /// A computed CSS `scale`
 pub type Scale = generic::Scale<Number>;
 
 impl Scale {
     /// Convert TransformOperation to Scale.
     pub fn to_transform_operation(&self) -> Option<TransformOperation> {
         match *self {
             generic::Scale::None => None,
-            generic::Scale::Scale(sx, sy) => Some(generic::TransformOperation::Scale(sx, Some(sy))),
+            generic::Scale::Scale(sx, sy) => Some(generic::TransformOperation::Scale(sx, sy)),
             generic::Scale::Scale3D(sx, sy, sz) => {
                 Some(generic::TransformOperation::Scale3D(sx, sy, sz))
             },
         }
     }
 
     /// Convert Scale to TransformOperation.
     pub fn from_transform_operation(operation: &TransformOperation) -> Scale {
         match *operation {
-            generic::TransformOperation::Scale(sx, Some(sy)) => generic::Scale::Scale(sx, sy),
-            generic::TransformOperation::Scale(sx, None) => generic::Scale::Scale(sx, sx),
+            generic::TransformOperation::Scale(sx, sy) => generic::Scale::Scale(sx, sy),
             generic::TransformOperation::Scale3D(sx, sy, sz) => generic::Scale::Scale3D(sx, sy, sz),
             _ => unreachable!("Found unexpected value for scale"),
         }
     }
 }
--- a/servo/components/style/values/generics/transform.rs
+++ b/servo/components/style/values/generics/transform.rs
@@ -101,61 +101,65 @@ impl<H, V, D> TransformOrigin<H, V, D> {
         Self {
             horizontal,
             vertical,
             depth,
         }
     }
 }
 
+fn is_same<N: PartialEq>(x: &N, y: &N) -> bool {
+    x == y
+}
+
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)]
 /// A single operation in the list of a `transform` value
-pub enum TransformOperation<Angle, Number, Length, Integer, LengthPercentage> {
+pub enum TransformOperation<Angle, Number, Length, Integer, LengthPercentage>
+where
+    Angle: Zero,
+    LengthPercentage: Zero,
+    Number: PartialEq,
+{
     /// Represents a 2D 2x3 matrix.
     Matrix(Matrix<Number>),
     /// Represents a 3D 4x4 matrix.
     Matrix3D(Matrix3D<Number>),
     /// A 2D skew.
     ///
     /// If the second angle is not provided it is assumed zero.
     ///
     /// Syntax can be skew(angle) or skew(angle, angle)
     #[css(comma, function)]
-    Skew(Angle, Option<Angle>),
+    Skew(Angle, #[css(skip_if = "Zero::is_zero")] Angle),
     /// skewX(angle)
     #[css(function = "skewX")]
     SkewX(Angle),
     /// skewY(angle)
     #[css(function = "skewY")]
     SkewY(Angle),
     /// translate(x, y) or translate(x)
     #[css(comma, function)]
-    Translate(LengthPercentage, Option<LengthPercentage>),
+    Translate(LengthPercentage, #[css(skip_if = "Zero::is_zero")] LengthPercentage),
     /// translateX(x)
     #[css(function = "translateX")]
     TranslateX(LengthPercentage),
     /// translateY(y)
     #[css(function = "translateY")]
     TranslateY(LengthPercentage),
     /// translateZ(z)
     #[css(function = "translateZ")]
     TranslateZ(Length),
     /// translate3d(x, y, z)
     #[css(comma, function = "translate3d")]
     Translate3D(LengthPercentage, LengthPercentage, Length),
     /// A 2D scaling factor.
     ///
-    /// `scale(2)` is parsed as `Scale(Number::new(2.0), None)` and is equivalent to
-    /// writing `scale(2, 2)` (`Scale(Number::new(2.0), Some(Number::new(2.0)))`).
-    ///
-    /// Negative values are allowed and flip the element.
-    ///
     /// Syntax can be scale(factor) or scale(factor, factor)
     #[css(comma, function)]
-    Scale(Number, Option<Number>),
+    Scale(Number, #[css(contextual_skip_if = "is_same")] Number),
     /// scaleX(factor)
     #[css(function = "scaleX")]
     ScaleX(Number),
     /// scaleY(factor)
     #[css(function = "scaleY")]
     ScaleY(Number),
     /// scaleZ(factor)
     #[css(function = "scaleZ")]
@@ -209,16 +213,20 @@ pub enum TransformOperation<Angle, Numbe
 }
 
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)]
 /// A value of the `transform` property
 pub struct Transform<T>(#[css(if_empty = "none", iterable)] pub Vec<T>);
 
 impl<Angle, Number, Length, Integer, LengthPercentage>
     TransformOperation<Angle, Number, Length, Integer, LengthPercentage>
+where
+    Angle: Zero,
+    LengthPercentage: Zero,
+    Number: PartialEq,
 {
     /// Check if it is any rotate function.
     pub fn is_rotate(&self) -> bool {
         use self::TransformOperation::*;
         matches!(
             *self,
             Rotate(..) | Rotate3D(..) | RotateX(..) | RotateY(..) | RotateZ(..)
         )
@@ -328,20 +336,20 @@ impl ToRadians for SpecifiedAngle {
     fn radians64(&self) -> f64 {
         computed::angle::Angle::from_degrees(self.degrees()).radians64()
     }
 }
 
 impl<Angle, Number, Length, Integer, LoP> ToMatrix
     for TransformOperation<Angle, Number, Length, Integer, LoP>
 where
-    Angle: ToRadians + Copy,
-    Number: Copy + Into<f32> + Into<f64>,
+    Angle: Zero + ToRadians + Copy,
+    Number: PartialEq + Copy + Into<f32> + Into<f64>,
     Length: ToAbsoluteLength,
-    LoP: ToAbsoluteLength,
+    LoP: Zero + ToAbsoluteLength,
 {
     #[inline]
     fn is_3d(&self) -> bool {
         use self::TransformOperation::*;
         match *self {
             Translate3D(..) | TranslateZ(..) | Rotate3D(..) | RotateX(..) | RotateY(..) |
             RotateZ(..) | Scale3D(..) | ScaleZ(..) | Perspective(..) | Matrix3D(..) => true,
             _ => false,
@@ -384,44 +392,44 @@ where
                 let theta = euclid::Angle::radians(TWO_PI - theta.radians64());
                 Transform3D::create_rotation(0., 0., 1., theta)
             },
             Perspective(ref d) => {
                 let m = create_perspective_matrix(d.to_pixel_length(None)?);
                 m.cast()
             },
             Scale3D(sx, sy, sz) => Transform3D::create_scale(sx.into(), sy.into(), sz.into()),
-            Scale(sx, sy) => Transform3D::create_scale(sx.into(), sy.unwrap_or(sx).into(), 1.),
+            Scale(sx, sy) => Transform3D::create_scale(sx.into(), sy.into(), 1.),
             ScaleX(s) => Transform3D::create_scale(s.into(), 1., 1.),
             ScaleY(s) => Transform3D::create_scale(1., s.into(), 1.),
             ScaleZ(s) => Transform3D::create_scale(1., 1., s.into()),
             Translate3D(ref tx, ref ty, ref tz) => {
                 let tx = tx.to_pixel_length(reference_width)? as f64;
                 let ty = ty.to_pixel_length(reference_height)? as f64;
                 Transform3D::create_translation(tx, ty, tz.to_pixel_length(None)? as f64)
             },
-            Translate(ref tx, Some(ref ty)) => {
+            Translate(ref tx, ref ty) => {
                 let tx = tx.to_pixel_length(reference_width)? as f64;
                 let ty = ty.to_pixel_length(reference_height)? as f64;
                 Transform3D::create_translation(tx, ty, 0.)
             },
-            TranslateX(ref t) | Translate(ref t, None) => {
+            TranslateX(ref t) => {
                 let t = t.to_pixel_length(reference_width)? as f64;
                 Transform3D::create_translation(t, 0., 0.)
             },
             TranslateY(ref t) => {
                 let t = t.to_pixel_length(reference_height)? as f64;
                 Transform3D::create_translation(0., t, 0.)
             },
             TranslateZ(ref z) => {
                 Transform3D::create_translation(0., 0., z.to_pixel_length(None)? as f64)
             },
             Skew(theta_x, theta_y) => Transform3D::create_skew(
                 euclid::Angle::radians(theta_x.radians64()),
-                euclid::Angle::radians(theta_y.map_or(0., |a| a.radians64())),
+                euclid::Angle::radians(theta_y.radians64()),
             ),
             SkewX(theta) => Transform3D::create_skew(
                 euclid::Angle::radians(theta.radians64()),
                 euclid::Angle::radians(0.),
             ),
             SkewY(theta) => Transform3D::create_skew(
                 euclid::Angle::radians(0.),
                 euclid::Angle::radians(theta.radians64()),
--- a/servo/components/style/values/specified/angle.rs
+++ b/servo/components/style/values/specified/angle.rs
@@ -4,16 +4,17 @@
 
 //! Specified angles.
 
 use crate::parser::{Parse, ParserContext};
 use crate::values::computed::angle::Angle as ComputedAngle;
 use crate::values::computed::{Context, ToComputedValue};
 use crate::values::specified::calc::CalcNode;
 use crate::values::CSSFloat;
+use crate::Zero;
 use cssparser::{Parser, Token};
 use std::f32::consts::PI;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, ToCss};
 
 /// A specified angle dimension.
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, PartialOrd, ToCss)]
@@ -27,16 +28,31 @@ pub enum AngleDimension {
     /// An angle with radian unit.
     #[css(dimension)]
     Rad(CSSFloat),
     /// An angle with turn unit.
     #[css(dimension)]
     Turn(CSSFloat),
 }
 
+impl Zero for AngleDimension {
+    fn zero() -> Self {
+        AngleDimension::Deg(0.)
+    }
+
+    fn is_zero(&self) -> bool {
+        match *self {
+            AngleDimension::Deg(ref f) |
+            AngleDimension::Grad(ref f) |
+            AngleDimension::Rad(ref f) |
+            AngleDimension::Turn(ref f) => *f == 0.,
+        }
+    }
+}
+
 impl AngleDimension {
     /// Returns the amount of degrees this angle represents.
     #[inline]
     fn degrees(&self) -> CSSFloat {
         const DEG_PER_RAD: f32 = 180.0 / PI;
         const DEG_PER_TURN: f32 = 360.0;
         const DEG_PER_GRAD: f32 = 180.0 / 200.0;
 
@@ -53,16 +69,29 @@ impl AngleDimension {
 /// was specified as `calc()` or not.
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq)]
 pub struct Angle {
     value: AngleDimension,
     was_calc: bool,
 }
 
+impl Zero for Angle {
+    fn zero() -> Self {
+        Self {
+            value: Zero::zero(),
+            was_calc: false,
+        }
+    }
+
+    fn is_zero(&self) -> bool {
+        self.value.is_zero()
+    }
+}
+
 impl ToCss for Angle {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
     {
         if self.was_calc {
             dest.write_str("calc(")?;
         }
--- a/servo/components/style/values/specified/transform.rs
+++ b/servo/components/style/values/specified/transform.rs
@@ -104,19 +104,19 @@ impl Transform {
                             m31, m32, m33, m34,
                             m41, m42, m43, m44,
                         }))
                     },
                     "translate" => {
                         let sx = specified::LengthPercentage::parse(context, input)?;
                         if input.try(|input| input.expect_comma()).is_ok() {
                             let sy = specified::LengthPercentage::parse(context, input)?;
-                            Ok(generic::TransformOperation::Translate(sx, Some(sy)))
+                            Ok(generic::TransformOperation::Translate(sx, sy))
                         } else {
-                            Ok(generic::TransformOperation::Translate(sx, None))
+                            Ok(generic::TransformOperation::Translate(sx, Zero::zero()))
                         }
                     },
                     "translatex" => {
                         let tx = specified::LengthPercentage::parse(context, input)?;
                         Ok(generic::TransformOperation::TranslateX(tx))
                     },
                     "translatey" => {
                         let ty = specified::LengthPercentage::parse(context, input)?;
@@ -133,19 +133,19 @@ impl Transform {
                         input.expect_comma()?;
                         let tz = specified::Length::parse(context, input)?;
                         Ok(generic::TransformOperation::Translate3D(tx, ty, tz))
                     },
                     "scale" => {
                         let sx = Number::parse(context, input)?;
                         if input.try(|input| input.expect_comma()).is_ok() {
                             let sy = Number::parse(context, input)?;
-                            Ok(generic::TransformOperation::Scale(sx, Some(sy)))
+                            Ok(generic::TransformOperation::Scale(sx, sy))
                         } else {
-                            Ok(generic::TransformOperation::Scale(sx, None))
+                            Ok(generic::TransformOperation::Scale(sx, sx))
                         }
                     },
                     "scalex" => {
                         let sx = Number::parse(context, input)?;
                         Ok(generic::TransformOperation::ScaleX(sx))
                     },
                     "scaley" => {
                         let sy = Number::parse(context, input)?;
@@ -189,19 +189,19 @@ impl Transform {
                         let theta = specified::Angle::parse_with_unitless(context, input)?;
                         // TODO(gw): Check that the axis can be normalized.
                         Ok(generic::TransformOperation::Rotate3D(ax, ay, az, theta))
                     },
                     "skew" => {
                         let ax = specified::Angle::parse_with_unitless(context, input)?;
                         if input.try(|input| input.expect_comma()).is_ok() {
                             let ay = specified::Angle::parse_with_unitless(context, input)?;
-                            Ok(generic::TransformOperation::Skew(ax, Some(ay)))
+                            Ok(generic::TransformOperation::Skew(ax, ay))
                         } else {
-                            Ok(generic::TransformOperation::Skew(ax, None))
+                            Ok(generic::TransformOperation::Skew(ax, Zero::zero()))
                         }
                     },
                     "skewx" => {
                         let theta = specified::Angle::parse_with_unitless(context, input)?;
                         Ok(generic::TransformOperation::SkewX(theta))
                     },
                     "skewy" => {
                         let theta = specified::Angle::parse_with_unitless(context, input)?;
--- a/testing/web-platform/tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html
+++ b/testing/web-platform/tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html
@@ -674,17 +674,17 @@ test(t => {
   const frames = getKeyframes(div);
 
   assert_equals(frames.length, 2, "number of frames");
 
   const expected = [
     { offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
       transform: "none" },
     { offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
-      transform: "translate(100px, 0px)" },
+      transform: "translate(100px)" },
   ];
   for (let i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffect.getKeyframes() returns expected values for ' +
    'animations with CSS variables as keyframe values');
 
 test(t => {
@@ -738,17 +738,17 @@ test(t => {
   div.style.animation = 'anim-only-custom-property-in-keyframe 100s';
 
   const frames = getKeyframes(div);
 
   assert_equals(frames.length, 2, "number of frames");
 
   const expected = [
     { offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
-      transform: "translate(100px, 0px)" },
+      transform: "translate(100px)" },
     { offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
       transform: "none" },
   ];
   for (let i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffect.getKeyframes() returns expected values for ' +
    'animations with only custom property in a keyframe');
--- a/testing/web-platform/tests/css/css-transforms/parsing/transform-valid.html
+++ b/testing/web-platform/tests/css/css-transforms/parsing/transform-valid.html
@@ -34,17 +34,17 @@ test_valid_value("transform", "scaleX(7)
 test_valid_value("transform", "scaleY(-8)");
 
 test_valid_value("transform", "rotate(0)", "rotate(0deg)");
 test_valid_value("transform", "rotate(90deg)");
 
 test_valid_value("transform", "skew(0)", "skew(0deg)");
 test_valid_value("transform", "skew(90deg)");
 test_valid_value("transform", "skew(0, -90deg)", "skew(0deg, -90deg)");
-test_valid_value("transform", "skew(90deg, 0)", "skew(90deg, 0deg)");
+test_valid_value("transform", "skew(90deg, 0)", ["skew(90deg)", "skew(90deg, 0deg)"]);
 
 test_valid_value("transform", "skewX(0)", "skewX(0deg)");
 test_valid_value("transform", "skewX(90deg)");
 
 test_valid_value("transform", "skewY(0)", "skewY(0deg)");
 test_valid_value("transform", "skewY(-90deg)");
 
 test_valid_value("transform", "translate(1px, 2%) scale(3, 4) rotate(-90deg)");