style: Simplify computed::LengthOrPercentage and friends.

This is a first step to share LengthOrPercentage representation between Rust and
Gecko.

We need to preserve whether the value came from a calc() expression, for now at
least, since we do different things depending on whether we're calc or not right
now. See w3c/csswg-drafts#3482 and dependent bugs for
example.

That means that the gecko conversion code needs to handle calc() in a bit of an
awkward way until I change it to not be needed (patches for that incoming in the
next few weeks I hope).

I need to add a hack to exclude other things from the PartialEq implementation
because the new conversion code is less lossy than the old one, and we relied on
the lousiness in AnimationValue comparison (in order to start transitions and
such, in [1] for example).

I expect to remove that manual PartialEq implementation as soon as I'm done with
the conversion.

The less lossy conversion does fix a few serialization bugs for animation values
though, like not loosing 0% values in calc() when interpolating lengths and
percentages, see the two modified tests:

 * property-types.js
 * test_animation_properties.html

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

Author: emilio

components/style/gecko/conversions.rs

@@ -20,7 +20,7 @@ use crate::stylesheets::{Origin, RulesMutateError};
 use crate::values::computed::image::LineDirection;
 use crate::values::computed::transform::Matrix3D;
 use crate::values::computed::url::ComputedImageUrl;
-use crate::values::computed::{Angle, CalcLengthOrPercentage, Gradient, Image};
+use crate::values::computed::{Angle, Gradient, Image};
 use crate::values::computed::{Integer, LengthOrPercentage};
 use crate::values::computed::{LengthOrPercentageOrAuto, NonNegativeLengthOrPercentageOrAuto};
 use crate::values::computed::{Percentage, TextAlign};
@@ -31,9 +31,10 @@ use crate::values::generics::rect::Rect;
 use crate::values::generics::NonNegative;
 use app_units::Au;
 use std::f32::consts::PI;
+use style_traits::values::specified::AllowedNumericType;
 
-impl From<CalcLengthOrPercentage> for nsStyleCoord_CalcValue {
-    fn from(other: CalcLengthOrPercentage) -> nsStyleCoord_CalcValue {
+impl From<LengthOrPercentage> for nsStyleCoord_CalcValue {
+    fn from(other: LengthOrPercentage) -> nsStyleCoord_CalcValue {
         let has_percentage = other.percentage.is_some();
         nsStyleCoord_CalcValue {
             mLength: other.unclamped_length().to_i32_au(),
@@ -43,93 +44,54 @@ impl From<CalcLengthOrPercentage> for nsStyleCoord_CalcValue {
     }
 }
 
-impl From<nsStyleCoord_CalcValue> for CalcLengthOrPercentage {
-    fn from(other: nsStyleCoord_CalcValue) -> CalcLengthOrPercentage {
+impl From<nsStyleCoord_CalcValue> for LengthOrPercentage {
+    fn from(other: nsStyleCoord_CalcValue) -> LengthOrPercentage {
         let percentage = if other.mHasPercent {
             Some(Percentage(other.mPercent))
         } else {
             None
         };
-        Self::new(Au(other.mLength).into(), percentage)
-    }
-}
-
-impl From<LengthOrPercentage> for nsStyleCoord_CalcValue {
-    fn from(other: LengthOrPercentage) -> nsStyleCoord_CalcValue {
-        match other {
-            LengthOrPercentage::Length(px) => nsStyleCoord_CalcValue {
-                mLength: px.to_i32_au(),
-                mPercent: 0.0,
-                mHasPercent: false,
-            },
-            LengthOrPercentage::Percentage(pc) => nsStyleCoord_CalcValue {
-                mLength: 0,
-                mPercent: pc.0,
-                mHasPercent: true,
-            },
-            LengthOrPercentage::Calc(calc) => calc.into(),
-        }
+        Self::with_clamping_mode(
+            Au(other.mLength).into(),
+            percentage,
+            AllowedNumericType::All,
+            /* was_calc = */ true,
+        )
     }
 }
 
 impl LengthOrPercentageOrAuto {
     /// Convert this value in an appropriate `nsStyleCoord::CalcValue`.
     pub fn to_calc_value(&self) -> Option<nsStyleCoord_CalcValue> {
         match *self {
-            LengthOrPercentageOrAuto::Length(px) => Some(nsStyleCoord_CalcValue {
-                mLength: px.to_i32_au(),
-                mPercent: 0.0,
-                mHasPercent: false,
-            }),
-            LengthOrPercentageOrAuto::Percentage(pc) => Some(nsStyleCoord_CalcValue {
-                mLength: 0,
-                mPercent: pc.0,
-                mHasPercent: true,
-            }),
-            LengthOrPercentageOrAuto::Calc(calc) => Some(calc.into()),
+            LengthOrPercentageOrAuto::LengthOrPercentage(len) => Some(From::from(len)),
             LengthOrPercentageOrAuto::Auto => None,
         }
     }
 }
 
-impl From<nsStyleCoord_CalcValue> for LengthOrPercentage {
-    fn from(other: nsStyleCoord_CalcValue) -> LengthOrPercentage {
-        match (other.mHasPercent, other.mLength) {
-            (false, _) => LengthOrPercentage::Length(Au(other.mLength).into()),
-            (true, 0) => LengthOrPercentage::Percentage(Percentage(other.mPercent)),
-            _ => LengthOrPercentage::Calc(other.into()),
-        }
-    }
-}
-
 impl From<nsStyleCoord_CalcValue> for LengthOrPercentageOrAuto {
     fn from(other: nsStyleCoord_CalcValue) -> LengthOrPercentageOrAuto {
-        match (other.mHasPercent, other.mLength) {
-            (false, _) => LengthOrPercentageOrAuto::Length(Au(other.mLength).into()),
-            (true, 0) => LengthOrPercentageOrAuto::Percentage(Percentage(other.mPercent)),
-            _ => LengthOrPercentageOrAuto::Calc(other.into()),
-        }
+        LengthOrPercentageOrAuto::LengthOrPercentage(LengthOrPercentage::from(other))
     }
 }
 
 // FIXME(emilio): A lot of these impl From should probably become explicit or
 // disappear as we move more stuff to cbindgen.
 impl From<nsStyleCoord_CalcValue> for NonNegativeLengthOrPercentageOrAuto {
     fn from(other: nsStyleCoord_CalcValue) -> Self {
-        use style_traits::values::specified::AllowedNumericType;
-        NonNegative(if other.mLength < 0 || other.mPercent < 0. {
-            LengthOrPercentageOrAuto::Calc(CalcLengthOrPercentage::with_clamping_mode(
+        NonNegative(
+            LengthOrPercentageOrAuto::LengthOrPercentage(LengthOrPercentage::with_clamping_mode(
                 Au(other.mLength).into(),
                 if other.mHasPercent {
                     Some(Percentage(other.mPercent))
                 } else {
                     None
                 },
                 AllowedNumericType::NonNegative,
+                /* was_calc = */ true,
             ))
-        } else {
-            other.into()
-        })
+        )
     }
 }
 
@@ -143,30 +105,23 @@ fn line_direction(horizontal: LengthOrPercentage, vertical: LengthOrPercentage)
     use crate::values::computed::position::Position;
     use crate::values::specified::position::{X, Y};
 
-    let horizontal_percentage = match horizontal {
-        LengthOrPercentage::Percentage(percentage) => Some(percentage.0),
-        _ => None,
-    };
-
-    let vertical_percentage = match vertical {
-        LengthOrPercentage::Percentage(percentage) => Some(percentage.0),
-        _ => None,
-    };
+    let horizontal_percentage = horizontal.as_percentage();
+    let vertical_percentage = vertical.as_percentage();
 
     let horizontal_as_corner = horizontal_percentage.and_then(|percentage| {
-        if percentage == 0.0 {
+        if percentage.0 == 0.0 {
             Some(X::Left)
-        } else if percentage == 1.0 {
+        } else if percentage.0 == 1.0 {
             Some(X::Right)
         } else {
             None
         }
     });
 
     let vertical_as_corner = vertical_percentage.and_then(|percentage| {
-        if percentage == 0.0 {
+        if percentage.0 == 0.0 {
             Some(Y::Top)
-        } else if percentage == 1.0 {
+        } else if percentage.0 == 1.0 {
             Some(Y::Bottom)
         } else {
             None
@@ -178,13 +133,13 @@ fn line_direction(horizontal: LengthOrPercentage, vertical: LengthOrPercentage)
     }
 
     if let Some(hc) = horizontal_as_corner {
-        if vertical_percentage == Some(0.5) {
+        if vertical_percentage == Some(Percentage(0.5)) {
             return LineDirection::Horizontal(hc);
         }
     }
 
     if let Some(vc) = vertical_as_corner {
-        if horizontal_percentage == Some(0.5) {
+        if horizontal_percentage == Some(Percentage(0.5)) {
             return LineDirection::Vertical(vc);
         }
     }

components/style/gecko/values.rs

@@ -148,19 +148,21 @@ impl GeckoStyleCoordConvertible for NumberOrPercentage {
 
 impl GeckoStyleCoordConvertible for LengthOrPercentage {
     fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
-        let value = match *self {
-            LengthOrPercentage::Length(px) => CoordDataValue::Coord(px.to_i32_au()),
-            LengthOrPercentage::Percentage(p) => CoordDataValue::Percent(p.0),
-            LengthOrPercentage::Calc(calc) => CoordDataValue::Calc(calc.into()),
-        };
-        coord.set_value(value);
+        if self.was_calc {
+            return coord.set_value(CoordDataValue::Calc((*self).into()))
+        }
+        debug_assert!(self.percentage.is_none() || self.unclamped_length() == Length::zero());
+        if let Some(p) = self.percentage {
+            return coord.set_value(CoordDataValue::Percent(p.0));
+        }
+        coord.set_value(CoordDataValue::Coord(self.unclamped_length().to_i32_au()))
     }
 
     fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self> {
         match coord.as_value() {
-            CoordDataValue::Coord(coord) => Some(LengthOrPercentage::Length(Au(coord).into())),
-            CoordDataValue::Percent(p) => Some(LengthOrPercentage::Percentage(Percentage(p))),
-            CoordDataValue::Calc(calc) => Some(LengthOrPercentage::Calc(calc.into())),
+            CoordDataValue::Coord(coord) => Some(LengthOrPercentage::new(Au(coord).into(), None)),
+            CoordDataValue::Percent(p) => Some(LengthOrPercentage::new(Au(0).into(), Some(Percentage(p)))),
+            CoordDataValue::Calc(calc) => Some(calc.into()),
             _ => None,
         }
     }
@@ -181,48 +183,32 @@ impl GeckoStyleCoordConvertible for Length {
 
 impl GeckoStyleCoordConvertible for LengthOrPercentageOrAuto {
     fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
-        let value = match *self {
-            LengthOrPercentageOrAuto::Length(px) => CoordDataValue::Coord(px.to_i32_au()),
-            LengthOrPercentageOrAuto::Percentage(p) => CoordDataValue::Percent(p.0),
-            LengthOrPercentageOrAuto::Auto => CoordDataValue::Auto,
-            LengthOrPercentageOrAuto::Calc(calc) => CoordDataValue::Calc(calc.into()),
-        };
-        coord.set_value(value);
+        match *self {
+            LengthOrPercentageOrAuto::Auto => coord.set_value(CoordDataValue::Auto),
+            LengthOrPercentageOrAuto::LengthOrPercentage(ref lop) => lop.to_gecko_style_coord(coord),
+        }
     }
 
     fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self> {
         match coord.as_value() {
-            CoordDataValue::Coord(coord) => {
-                Some(LengthOrPercentageOrAuto::Length(Au(coord).into()))
-            },
-            CoordDataValue::Percent(p) => Some(LengthOrPercentageOrAuto::Percentage(Percentage(p))),
             CoordDataValue::Auto => Some(LengthOrPercentageOrAuto::Auto),
-            CoordDataValue::Calc(calc) => Some(LengthOrPercentageOrAuto::Calc(calc.into())),
-            _ => None,
+            _ => LengthOrPercentage::from_gecko_style_coord(coord).map(LengthOrPercentageOrAuto::LengthOrPercentage),
         }
     }
 }
 
 impl GeckoStyleCoordConvertible for LengthOrPercentageOrNone {
     fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
-        let value = match *self {
-            LengthOrPercentageOrNone::Length(px) => CoordDataValue::Coord(px.to_i32_au()),
-            LengthOrPercentageOrNone::Percentage(p) => CoordDataValue::Percent(p.0),
-            LengthOrPercentageOrNone::None => CoordDataValue::None,
-            LengthOrPercentageOrNone::Calc(calc) => CoordDataValue::Calc(calc.into()),
-        };
-        coord.set_value(value);
+        match *self {
+            LengthOrPercentageOrNone::None => coord.set_value(CoordDataValue::None),
+            LengthOrPercentageOrNone::LengthOrPercentage(ref lop) => lop.to_gecko_style_coord(coord),
+        }
     }
 
     fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self> {
         match coord.as_value() {
-            CoordDataValue::Coord(coord) => {
-                Some(LengthOrPercentageOrNone::Length(Au(coord).into()))
-            },
-            CoordDataValue::Percent(p) => Some(LengthOrPercentageOrNone::Percentage(Percentage(p))),
             CoordDataValue::None => Some(LengthOrPercentageOrNone::None),
-            CoordDataValue::Calc(calc) => Some(LengthOrPercentageOrNone::Calc(calc.into())),
-            _ => None,
+            _ => LengthOrPercentage::from_gecko_style_coord(coord).map(LengthOrPercentageOrNone::LengthOrPercentage),
         }
     }
 }

components/style/gecko_bindings/sugar/ns_css_value.rs

@@ -69,11 +69,14 @@ impl nsCSSValue {
 
     /// Sets LengthOrPercentage value to this nsCSSValue.
     pub unsafe fn set_lop(&mut self, lop: LengthOrPercentage) {
-        match lop {
-            LengthOrPercentage::Length(px) => self.set_px(px.px()),
-            LengthOrPercentage::Percentage(pc) => self.set_percentage(pc.0),
-            LengthOrPercentage::Calc(calc) => bindings::Gecko_CSSValue_SetCalc(self, calc.into()),
+        if lop.was_calc {
+            return bindings::Gecko_CSSValue_SetCalc(self, lop.into())
         }
+        debug_assert!(lop.percentage.is_none() || lop.unclamped_length() == Length::zero());
+        if let Some(p) = lop.percentage {
+            return self.set_percentage(p.0);
+        }
+        self.set_px(lop.unclamped_length().px());
     }
 
     /// Sets a px value to this nsCSSValue.
@@ -90,13 +93,16 @@ impl nsCSSValue {
     pub unsafe fn get_lop(&self) -> LengthOrPercentage {
         match self.mUnit {
             nsCSSUnit::eCSSUnit_Pixel => {
-                LengthOrPercentage::Length(Length::new(bindings::Gecko_CSSValue_GetNumber(self)))
+                LengthOrPercentage::new(
+                    Length::new(bindings::Gecko_CSSValue_GetNumber(self)),
+                    None,
+                )
             },
-            nsCSSUnit::eCSSUnit_Percent => LengthOrPercentage::Percentage(Percentage(
+            nsCSSUnit::eCSSUnit_Percent => LengthOrPercentage::new_percent(Percentage(
                 bindings::Gecko_CSSValue_GetPercentage(self),
             )),
             nsCSSUnit::eCSSUnit_Calc => {
-                LengthOrPercentage::Calc(bindings::Gecko_CSSValue_GetCalc(self).into())
+                bindings::Gecko_CSSValue_GetCalc(self).into()
             },
             _ => panic!("Unexpected unit"),
         }

components/style/properties/gecko.mako.rs

@@ -558,18 +558,16 @@ def set_gecko_property(ffi_name, expr):
             },
             CoordDataValue::Coord(coord) => {
                 SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                    LengthOrPercentage::Length(Au(coord).into())
+                    LengthOrPercentage::new(Au(coord).into(), None)
                 )
             },
             CoordDataValue::Percent(p) => {
                 SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                    LengthOrPercentage::Percentage(Percentage(p))
+                    LengthOrPercentage::new(Au(0).into(), Some(Percentage(p)))
                 )
             },
             CoordDataValue::Calc(calc) => {
-                SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                    LengthOrPercentage::Calc(calc.into())
-                )
+                SvgLengthOrPercentageOrNumber::LengthOrPercentage(calc.into())
             },
             _ => unreachable!("Unexpected coordinate in ${ident}"),
         };
@@ -5062,6 +5060,7 @@ clip-path
     pub fn clone_stroke_dasharray(&self) -> longhands::stroke_dasharray::computed_value::T {
         use crate::gecko_bindings::structs::nsStyleSVG_STROKE_DASHARRAY_CONTEXT as CONTEXT_VALUE;
         use crate::values::computed::LengthOrPercentage;
+        use crate::values::generics::NonNegative;
         use crate::values::generics::svg::{SVGStrokeDashArray, SvgLengthOrPercentageOrNumber};
 
         if self.gecko.mContextFlags & CONTEXT_VALUE != 0 {
@@ -5075,13 +5074,13 @@ clip-path
                     vec.push(SvgLengthOrPercentageOrNumber::Number(number.into())),
                 CoordDataValue::Coord(coord) =>
                     vec.push(SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                        LengthOrPercentage::Length(Au(coord).into()).into())),
+                        NonNegative(LengthOrPercentage::new(Au(coord).into(), None).into()))),
                 CoordDataValue::Percent(p) =>
                     vec.push(SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                        LengthOrPercentage::Percentage(Percentage(p)).into())),
+                        NonNegative(LengthOrPercentage::new_percent(Percentage(p)).into()))),
                 CoordDataValue::Calc(calc) =>
                     vec.push(SvgLengthOrPercentageOrNumber::LengthOrPercentage(
-                        LengthOrPercentage::Calc(calc.into()).into())),
+                        NonNegative(LengthOrPercentage::from(calc).clamp_to_non_negative()))),
                 _ => unreachable!(),
             }
         }

components/style/properties/longhands/inherited_text.mako.rs

@@ -54,7 +54,7 @@ ${helpers.single_keyword(
 ${helpers.predefined_type(
     "text-indent",
     "LengthOrPercentage",
-    "computed::LengthOrPercentage::Length(computed::Length::new(0.))",
+    "computed::LengthOrPercentage::zero()",
     animation_value_type="ComputedValue",
     spec="https://drafts.csswg.org/css-text/#propdef-text-indent",
     allow_quirks=True,

components/style/properties/longhands/margin.mako.rs

@@ -15,7 +15,7 @@
     ${helpers.predefined_type(
         "margin-%s" % side[0],
         "LengthOrPercentageOrAuto",
-        "computed::LengthOrPercentageOrAuto::Length(computed::Length::new(0.))",
+        "computed::LengthOrPercentageOrAuto::zero()",
         alias=maybe_moz_logical_alias(product, side, "-moz-margin-%s"),
         allow_quirks=not side[1],
         animation_value_type="ComputedValue",