servo: Merge #16674 - Preserve units in computed Angle (from canaltinova:angle-unit); r=emilio

<!-- Please describe your changes on the following line: -->
It was converting all angles to radians before. But other browsers preserves the angle units. Fixed that behavior.

---
<!-- 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 #16594 and [Bug 1360659](https://bugzilla.mozilla.org/show_bug.cgi?id=1360659)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 0abd5bbabd19695a5a42437dc42ab3bdf76f6150

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 5a181ae79c46ceb680a59b0971e60c31b9f6e1b8

servo/components/style/gecko/conversions.rs

@@ -12,11 +12,11 @@ use app_units::Au;
 use gecko::values::{convert_rgba_to_nscolor, GeckoStyleCoordConvertible};
 use gecko_bindings::bindings::{Gecko_CreateGradient, Gecko_SetGradientImageValue, Gecko_SetUrlImageValue};
 use gecko_bindings::bindings::{Gecko_InitializeImageCropRect, Gecko_SetImageElement};
-use gecko_bindings::structs::{nsStyleCoord_CalcValue, nsStyleImage};
+use gecko_bindings::structs::{nsCSSUnit, nsStyleCoord_CalcValue, nsStyleImage};
 use gecko_bindings::structs::{nsresult, SheetType};
 use gecko_bindings::sugar::ns_style_coord::{CoordDataValue, CoordDataMut};
 use stylesheets::{Origin, RulesMutateError};
-use values::computed::{CalcLengthOrPercentage, Gradient, GradientItem, Image};
+use values::computed::{Angle, CalcLengthOrPercentage, Gradient, GradientItem, Image};
 use values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto};
 
 impl From<CalcLengthOrPercentage> for nsStyleCoord_CalcValue {
@@ -100,6 +100,40 @@ impl From<nsStyleCoord_CalcValue> for LengthOrPercentage {
     }
 }
 
+impl From<Angle> for CoordDataValue {
+    fn from(reference: Angle) -> Self {
+        match reference {
+            Angle::Degree(val) => CoordDataValue::Degree(val),
+            Angle::Gradian(val) => CoordDataValue::Grad(val),
+            Angle::Radian(val) => CoordDataValue::Radian(val),
+            Angle::Turn(val) => CoordDataValue::Turn(val),
+        }
+    }
+}
+
+impl Angle {
+    /// Converts Angle struct into (value, unit) pair.
+    pub fn to_gecko_values(&self) -> (f32, nsCSSUnit) {
+        match *self {
+            Angle::Degree(val) => (val, nsCSSUnit::eCSSUnit_Degree),
+            Angle::Gradian(val) => (val, nsCSSUnit::eCSSUnit_Grad),
+            Angle::Radian(val) => (val, nsCSSUnit::eCSSUnit_Radian),
+            Angle::Turn(val) => (val, nsCSSUnit::eCSSUnit_Turn),
+        }
+    }
+
+    /// Converts gecko (value, unit) pair into Angle struct
+    pub fn from_gecko_values(value: f32, unit: nsCSSUnit) -> Angle {
+        match unit {
+            nsCSSUnit::eCSSUnit_Degree => Angle::Degree(value),
+            nsCSSUnit::eCSSUnit_Grad => Angle::Gradian(value),
+            nsCSSUnit::eCSSUnit_Radian => Angle::Radian(value),
+            nsCSSUnit::eCSSUnit_Turn => Angle::Turn(value),
+            _ => panic!("Unexpected unit {:?} for angle", unit),
+        }
+    }
+}
+
 impl nsStyleImage {
     /// Set a given Servo `Image` value into this `nsStyleImage`.
     pub fn set(&mut self, image: Image, cacheable: &mut bool) {

servo/components/style/gecko/values.rs

@@ -249,15 +249,16 @@ impl<T: GeckoStyleCoordConvertible> GeckoStyleCoordConvertible for Option<T> {
 
 impl GeckoStyleCoordConvertible for Angle {
     fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
-        coord.set_value(CoordDataValue::Radian(self.radians()))
+        coord.set_value(CoordDataValue::from(*self));
     }
 
     fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self> {
-        if let CoordDataValue::Radian(r) = coord.as_value() {
-            Some(Angle::from_radians(r))
-            // XXXManishearth should this handle Degree too?
-        } else {
-            None
+        match coord.as_value() {
+            CoordDataValue::Degree(val) => Some(Angle::Degree(val)),
+            CoordDataValue::Grad(val) => Some(Angle::Gradian(val)),
+            CoordDataValue::Radian(val) => Some(Angle::Radian(val)),
+            CoordDataValue::Turn(val) => Some(Angle::Turn(val)),
+            _ => None,
         }
     }
 }

servo/components/style/gecko_bindings/bindings.rs

@@ -1074,9 +1074,6 @@ extern "C" {
     pub fn Gecko_CSSValue_GetAbsoluteLength(css_value: nsCSSValueBorrowed)
      -> nscoord;
 }
-extern "C" {
-    pub fn Gecko_CSSValue_GetAngle(css_value: nsCSSValueBorrowed) -> f32;
-}
 extern "C" {
     pub fn Gecko_CSSValue_GetKeyword(aCSSValue: nsCSSValueBorrowed)
      -> nsCSSKeyword;
@@ -1110,10 +1107,6 @@ extern "C" {
     pub fn Gecko_CSSValue_SetPercentage(css_value: nsCSSValueBorrowedMut,
                                         percent: f32);
 }
-extern "C" {
-    pub fn Gecko_CSSValue_SetAngle(css_value: nsCSSValueBorrowedMut,
-                                   radians: f32);
-}
 extern "C" {
     pub fn Gecko_CSSValue_SetCalc(css_value: nsCSSValueBorrowedMut,
                                   calc: nsStyleCoord_CalcValue);

servo/components/style/gecko_bindings/sugar/ns_css_value.rs

@@ -12,7 +12,7 @@ use gecko_string_cache::Atom;
 use std::mem;
 use std::ops::{Index, IndexMut};
 use std::slice;
-use values::computed::LengthOrPercentage;
+use values::computed::{Angle, LengthOrPercentage};
 use values::specified::url::SpecifiedUrl;
 
 impl nsCSSValue {
@@ -173,6 +173,26 @@ impl nsCSSValue {
     pub fn set_from<T: ToNsCssValue>(&mut self, value: &T) {
         value.convert(self)
     }
+
+    /// Returns an `Angle` value from this `nsCSSValue`.
+    ///
+    /// Panics if the unit is not `eCSSUnit_Degree` `eCSSUnit_Grad`, `eCSSUnit_Turn`
+    /// or `eCSSUnit_Radian`.
+    pub fn get_angle(&self) -> Angle {
+        unsafe {
+            Angle::from_gecko_values(self.float_unchecked(), self.mUnit)
+        }
+    }
+
+    /// Sets Angle value to this nsCSSValue.
+    pub fn set_angle(&mut self, angle: Angle) {
+        debug_assert_eq!(self.mUnit, nsCSSUnit::eCSSUnit_Null);
+        let (value, unit) = angle.to_gecko_values();
+        self.mUnit = unit;
+        unsafe {
+            *self.mValue.mFloat.as_mut() = value;
+        }
+    }
 }
 
 impl Drop for nsCSSValue {

servo/components/style/properties/gecko.mako.rs

@@ -1981,7 +1981,7 @@ fn static_assert() {
                 "length" : "bindings::Gecko_CSSValue_SetAbsoluteLength(%s, %s.0)",
                 "percentage" : "bindings::Gecko_CSSValue_SetPercentage(%s, %s)",
                 "lop" : "%s.set_lop(%s)",
-                "angle" : "bindings::Gecko_CSSValue_SetAngle(%s, %s.radians())",
+                "angle" : "%s.set_angle(%s)",
                 "number" : "bindings::Gecko_CSSValue_SetNumber(%s, %s)",
             }
         %>
@@ -2058,7 +2058,7 @@ fn static_assert() {
             css_value_getters = {
                 "length" : "Au(bindings::Gecko_CSSValue_GetAbsoluteLength(%s))",
                 "lop" : "%s.get_lop()",
-                "angle" : "Angle::from_radians(bindings::Gecko_CSSValue_GetAngle(%s))",
+                "angle" : "%s.get_angle()",
                 "number" : "bindings::Gecko_CSSValue_GetNumber(%s)",
             }
         %>
@@ -2087,7 +2087,6 @@ fn static_assert() {
         use properties::longhands::transform::computed_value;
         use properties::longhands::transform::computed_value::ComputedMatrix;
         use properties::longhands::transform::computed_value::ComputedOperation;
-        use values::computed::Angle;
 
         if self.gecko.mSpecifiedTransform.mRawPtr.is_null() {
             return computed_value::T(None);
@@ -3134,7 +3133,7 @@ fn static_assert() {
                                                   CoordDataValue::Factor(factor),
                                                   gecko_filter),
                 HueRotate(angle)   => fill_filter(NS_STYLE_FILTER_HUE_ROTATE,
-                                                  CoordDataValue::Radian(angle.radians()),
+                                                  CoordDataValue::from(angle),
                                                   gecko_filter),
                 Invert(factor)     => fill_filter(NS_STYLE_FILTER_INVERT,
                                                   CoordDataValue::Factor(factor),

servo/components/style/properties/longhand/effects.mako.rs

@@ -139,7 +139,7 @@ ${helpers.predefined_type("clip",
         use app_units::Au;
         use values::CSSFloat;
         use values::computed::{CSSColor, Shadow};
-        use values::specified::Angle;
+        use values::computed::Angle;
         use values::specified::url::SpecifiedUrl;
 
         #[derive(Clone, PartialEq, Debug)]
@@ -382,7 +382,9 @@ ${helpers.predefined_type("clip",
                     SpecifiedFilter::Brightness(factor) => computed_value::Filter::Brightness(factor),
                     SpecifiedFilter::Contrast(factor) => computed_value::Filter::Contrast(factor),
                     SpecifiedFilter::Grayscale(factor) => computed_value::Filter::Grayscale(factor),
-                    SpecifiedFilter::HueRotate(factor) => computed_value::Filter::HueRotate(factor),
+                    SpecifiedFilter::HueRotate(ref factor) => {
+                        computed_value::Filter::HueRotate(factor.to_computed_value(context))
+                    },
                     SpecifiedFilter::Invert(factor) => computed_value::Filter::Invert(factor),
                     SpecifiedFilter::Opacity(factor) => computed_value::Filter::Opacity(factor),
                     SpecifiedFilter::Saturate(factor) => computed_value::Filter::Saturate(factor),
@@ -407,7 +409,9 @@ ${helpers.predefined_type("clip",
                     computed_value::Filter::Brightness(factor) => SpecifiedFilter::Brightness(factor),
                     computed_value::Filter::Contrast(factor) => SpecifiedFilter::Contrast(factor),
                     computed_value::Filter::Grayscale(factor) => SpecifiedFilter::Grayscale(factor),
-                    computed_value::Filter::HueRotate(factor) => SpecifiedFilter::HueRotate(factor),
+                    computed_value::Filter::HueRotate(ref factor) => {
+                        SpecifiedFilter::HueRotate(ToComputedValue::from_computed_value(factor))
+                    },
                     computed_value::Filter::Invert(factor) => SpecifiedFilter::Invert(factor),
                     computed_value::Filter::Opacity(factor) => SpecifiedFilter::Opacity(factor),
                     computed_value::Filter::Saturate(factor) => SpecifiedFilter::Saturate(factor),

servo/components/style/values/computed/mod.rs

@@ -11,6 +11,8 @@ use media_queries::Device;
 #[cfg(feature = "gecko")]
 use properties;
 use properties::{ComputedValues, StyleBuilder};
+use std::f32;
+use std::f32::consts::PI;
 use std::fmt;
 use style_traits::ToCss;
 use super::{CSSFloat, CSSInteger, RGBA};
@@ -139,27 +141,42 @@ impl<T> ToComputedValue for T
 /// A computed `<angle>` value.
 #[derive(Clone, PartialEq, PartialOrd, Copy, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))]
-pub struct Angle {
-    radians: CSSFloat,
+pub enum Angle {
+    /// An angle with degree unit
+    Degree(CSSFloat),
+    /// An angle with gradian unit
+    Gradian(CSSFloat),
+    /// An angle with radian unit
+    Radian(CSSFloat),
+    /// An angle with turn unit
+    Turn(CSSFloat),
 }
 
 impl Angle {
     /// Construct a computed `Angle` value from a radian amount.
     pub fn from_radians(radians: CSSFloat) -> Self {
-        Angle {
-            radians: radians,
-        }
+        Angle::Radian(radians)
     }
 
     /// Return the amount of radians this angle represents.
     #[inline]
     pub fn radians(&self) -> CSSFloat {
-        self.radians
+        const RAD_PER_DEG: CSSFloat = PI / 180.0;
+        const RAD_PER_GRAD: CSSFloat = PI / 200.0;
+        const RAD_PER_TURN: CSSFloat = PI * 2.0;
+
+        let radians = match *self {
+            Angle::Degree(val) => val * RAD_PER_DEG,
+            Angle::Gradian(val) => val * RAD_PER_GRAD,
+            Angle::Turn(val) => val * RAD_PER_TURN,
+            Angle::Radian(val) => val,
+        };
+        radians.min(f32::MAX).max(f32::MIN)
     }
 
     /// Returns an angle that represents a rotation of zero radians.
     pub fn zero() -> Self {
-        Self::from_radians(0.0)
+        Angle::Radian(0.0)
     }
 }
 
@@ -167,7 +184,12 @@ impl ToCss for Angle {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
         where W: fmt::Write,
     {
-        write!(dest, "{}rad", self.radians())
+        match *self {
+            Angle::Degree(val) => write!(dest, "{}deg", val),
+            Angle::Gradian(val) => write!(dest, "{}grad", val),
+            Angle::Radian(val) => write!(dest, "{}rad", val),
+            Angle::Turn(val) => write!(dest, "{}turn", val),
+        }
     }
 }
 

servo/components/style/values/specified/mod.rs

@@ -15,7 +15,6 @@ use self::grid::{TrackBreadth as GenericTrackBreadth, TrackSize as GenericTrackS
 use self::url::SpecifiedUrl;
 use std::ascii::AsciiExt;
 use std::f32;
-use std::f32::consts::PI;
 use std::fmt;
 use std::ops::Mul;
 use style_traits::ToCss;
@@ -300,45 +299,21 @@ impl Parse for BorderRadiusSize {
 #[derive(Clone, PartialEq, Copy, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))]
 /// An angle consisting of a value and a unit.
+///
+/// Computed Angle is essentially same as specified angle except calc
+/// value serialization. Therefore we are using computed Angle enum
+/// to hold the value and unit type.
 pub struct Angle {
-    value: CSSFloat,
-    unit: AngleUnit,
+    value: computed::Angle,
     was_calc: bool,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq)]
-#[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))]
-/// A unit used together with an angle.
-pub enum AngleUnit {
-    /// Degrees, short name "deg".
-    Degree,
-    /// Gradians, short name "grad".
-    Gradian,
-    /// Radians, short name "rad".
-    Radian,
-    /// Turns, short name "turn".
-    Turn,
-}
-
-impl ToCss for AngleUnit {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-        use self::AngleUnit::*;
-        dest.write_str(match *self {
-            Degree => "deg",
-            Gradian => "grad",
-            Radian => "rad",
-            Turn => "turn",
-        })
-    }
-}
-
 impl ToCss for Angle {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         if self.was_calc {
             dest.write_str("calc(")?;
         }
         self.value.to_css(dest)?;
-        self.unit.to_css(dest)?;
         if self.was_calc {
             dest.write_str(")")?;
         }
@@ -350,48 +325,39 @@ impl ToComputedValue for Angle {
     type ComputedValue = computed::Angle;
 
     fn to_computed_value(&self, _context: &Context) -> Self::ComputedValue {
-        computed::Angle::from_radians(self.radians())
+        self.value
     }
 
     fn from_computed_value(computed: &Self::ComputedValue) -> Self {
-        Angle::from_radians(computed.radians())
+        Angle {
+            value: *computed,
+            was_calc: false,
+        }
     }
 }
 
 impl Angle {
     /// Returns an angle with the given value in degrees.
     pub fn from_degrees(value: CSSFloat) -> Self {
-        Angle { value: value, unit: AngleUnit::Degree, was_calc: false }
+        Angle { value: computed::Angle::Degree(value), was_calc: false }
     }
     /// Returns an angle with the given value in gradians.
     pub fn from_gradians(value: CSSFloat) -> Self {
-        Angle { value: value, unit: AngleUnit::Gradian, was_calc: false }
+        Angle { value: computed::Angle::Gradian(value), was_calc: false }
     }
     /// Returns an angle with the given value in turns.
     pub fn from_turns(value: CSSFloat) -> Self {
-        Angle { value: value, unit: AngleUnit::Turn, was_calc: false }
+        Angle { value: computed::Angle::Turn(value), was_calc: false }
     }
     /// Returns an angle with the given value in radians.
     pub fn from_radians(value: CSSFloat) -> Self {
-        Angle { value: value, unit: AngleUnit::Radian, was_calc: false }
+        Angle { value: computed::Angle::Radian(value), was_calc: false }
     }
 
     #[inline]
     #[allow(missing_docs)]
     pub fn radians(self) -> f32 {
-        use self::AngleUnit::*;
-
-        const RAD_PER_DEG: CSSFloat = PI / 180.0;
-        const RAD_PER_GRAD: CSSFloat = PI / 200.0;
-        const RAD_PER_TURN: CSSFloat = PI * 2.0;
-
-        let radians = match self.unit {
-            Degree => self.value * RAD_PER_DEG,
-            Gradian => self.value * RAD_PER_GRAD,
-            Turn => self.value * RAD_PER_TURN,
-            Radian => self.value,
-        };
-        radians.min(f32::MAX).max(f32::MIN)
+        self.value.radians()
     }
 
     /// Returns an angle value that represents zero.
@@ -402,8 +368,7 @@ impl Angle {
     /// Returns an `Angle` parsed from a `calc()` expression.
     pub fn from_calc(radians: CSSFloat) -> Self {
         Angle {
-            value: radians,
-            unit: AngleUnit::Radian,
+            value: computed::Angle::Radian(radians),
             was_calc: true,
         }
     }