servo: Merge #18409 - Keep the unit of the serialization of specified::CalcLengthOrPercentage (from BorisChiou:stylo/calc/unit); r=emilio
authorBoris Chiou <boris.chiou@gmail.com>
Thu, 07 Sep 2017 21:34:28 -0500
changeset 379666 df88a5fb770deda614c9167c41f2a2402eb1a5d5
parent 379665 026c2d5a3ee89d6aea97349de6cd12a41b9a9a73
child 379667 7217ebcfc33e0bfe7c51ae6afdb1bfa2174211fb
push id32461
push userkwierso@gmail.com
push dateFri, 08 Sep 2017 20:15:32 +0000
treeherdermozilla-central@dd3736e98e4e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1396692
milestone57.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 #18409 - Keep the unit of the serialization of specified::CalcLengthOrPercentage (from BorisChiou:stylo/calc/unit); r=emilio For the serialization of specified values of calc(), we should keep the units of absolute lengths, so use AbsoluteLength. --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix [Bug 1396692](https://bugzilla.mozilla.org/show_bug.cgi?id=1396692). - [X] These changes do not require tests because we have wpt tests for this already. Source-Repo: https://github.com/servo/servo Source-Revision: c8bc6ca4204ff521c35ba09bfdd6921e53801bc0
servo/components/style/values/computed/length.rs
servo/components/style/values/specified/calc.rs
servo/components/style/values/specified/length.rs
--- a/servo/components/style/values/computed/length.rs
+++ b/servo/components/style/values/computed/length.rs
@@ -218,17 +218,17 @@ impl ToCss for CalcLengthOrPercentage {
 impl specified::CalcLengthOrPercentage {
     /// Compute the value, zooming any absolute units by the zoom function.
     fn to_computed_value_with_zoom<F>(&self, context: &Context, zoom_fn: F,
                                       base_size: FontBaseSize) -> CalcLengthOrPercentage
         where F: Fn(Au) -> Au {
         let mut length = Au(0);
 
         if let Some(absolute) = self.absolute {
-            length += zoom_fn(absolute);
+            length += zoom_fn(absolute.to_computed_value(context));
         }
 
         for val in &[self.vw.map(ViewportPercentageLength::Vw),
                      self.vh.map(ViewportPercentageLength::Vh),
                      self.vmin.map(ViewportPercentageLength::Vmin),
                      self.vmax.map(ViewportPercentageLength::Vmax)] {
             if let Some(val) = *val {
                 length += val.to_computed_value(context.viewport_size_for_viewport_unit_resolution());
@@ -264,17 +264,17 @@ impl ToComputedValue for specified::Calc
         // normal properties don't zoom, and compute em units against the current style's font-size
         self.to_computed_value_with_zoom(context, |abs| abs, FontBaseSize::CurrentStyle)
     }
 
     #[inline]
     fn from_computed_value(computed: &CalcLengthOrPercentage) -> Self {
         specified::CalcLengthOrPercentage {
             clamping_mode: computed.clamping_mode,
-            absolute: Some(computed.length),
+            absolute: Some(AbsoluteLength::from_computed_value(&computed.length)),
             percentage: computed.percentage,
             ..Default::default()
         }
     }
 }
 
 #[allow(missing_docs)]
 #[animate(fallback = "Self::animate_fallback")]
--- a/servo/components/style/values/specified/calc.rs
+++ b/servo/components/style/values/specified/calc.rs
@@ -1,27 +1,27 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! [Calc expressions][calc].
 //!
 //! [calc]: https://drafts.csswg.org/css-values/#calc-notation
 
-use app_units::Au;
 use cssparser::{Parser, Token, BasicParseError};
 use parser::ParserContext;
 use std::ascii::AsciiExt;
 use std::fmt;
 use style_traits::{ToCss, ParseError, StyleParseError};
 use style_traits::values::specified::AllowedLengthType;
 use values::{CSSInteger, CSSFloat};
 use values::computed;
 use values::specified::{Angle, Time};
-use values::specified::length::{FontRelativeLength, NoCalcLength, ViewportPercentageLength};
+use values::specified::length::{AbsoluteLength, FontRelativeLength, NoCalcLength};
+use values::specified::length::ViewportPercentageLength;
 
 /// A node inside a `Calc` expression's AST.
 #[derive(Clone, Debug)]
 pub enum CalcNode {
     /// `<length>`
     Length(NoCalcLength),
     /// `<angle>`
     Angle(Angle),
@@ -63,17 +63,17 @@ pub enum CalcUnit {
 }
 
 /// A struct to hold a simplified `<length>` or `<percentage>` expression.
 #[derive(Clone, Copy, Debug, Default, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[allow(missing_docs)]
 pub struct CalcLengthOrPercentage {
     pub clamping_mode: AllowedLengthType,
-    pub absolute: Option<Au>,
+    pub absolute: Option<AbsoluteLength>,
     pub vw: Option<CSSFloat>,
     pub vh: Option<CSSFloat>,
     pub vmin: Option<CSSFloat>,
     pub vmax: Option<CSSFloat>,
     pub em: Option<CSSFloat>,
     pub ex: Option<CSSFloat>,
     pub ch: Option<CSSFloat>,
     pub rem: Option<CSSFloat>,
@@ -113,39 +113,49 @@ impl ToCss for CalcLengthOrPercentage {
                         first_value_check!(val);
                         val.abs().to_css(dest)?;
                         dest.write_str(stringify!($val))?;
                     }
                 )*
             };
         }
 
+        macro_rules! serialize_abs {
+            ( $( $val:ident ),+ ) => {
+                $(
+                    if let Some(AbsoluteLength::$val(v)) = self.absolute {
+                        first_value_check!(v);
+                        AbsoluteLength::$val(v.abs()).to_css(dest)?;
+                    }
+                )+
+            };
+        }
+
         dest.write_str("calc(")?;
 
         // NOTE(emilio): Percentages first because of web-compat problems, see:
         // https://github.com/w3c/csswg-drafts/issues/1731
         if let Some(val) = self.percentage {
             first_value_check!(val.0);
             val.abs().to_css(dest)?;
         }
 
         // NOTE(emilio): The order here it's very intentional, and alphabetic
         // per the spec linked above.
-        serialize!(ch, em, ex);
+        serialize!(ch);
+        serialize_abs!(Cm);
+        serialize!(em, ex);
+        serialize_abs!(In);
 
         #[cfg(feature = "gecko")]
         {
             serialize!(mozmm);
         }
 
-        if let Some(val) = self.absolute {
-            first_value_check!(val);
-            val.abs().to_css(dest)?;
-        }
-
+        serialize_abs!(Mm, Pc, Pt, Px, Q);
         serialize!(rem, vh, vmax, vmin, vw);
 
         dest.write_str(")")
     }
 }
 
 impl CalcNode {
     /// Tries to parse a single element in the expression, that is, a
@@ -344,18 +354,20 @@ impl CalcNode {
                 ret.percentage = Some(computed::Percentage(
                     ret.percentage.map_or(0., |p| p.0) + pct * factor,
                 ));
             }
             CalcNode::Length(ref l) => {
                 match *l {
                     NoCalcLength::Absolute(abs) => {
                         ret.absolute = Some(
-                            ret.absolute.unwrap_or(Au(0)) +
-                            Au::from(abs).scale_by(factor)
+                            match ret.absolute {
+                                Some(value) => value + abs * factor,
+                                None => abs * factor,
+                            }
                         );
                     }
                     NoCalcLength::FontRelative(rel) => {
                         match rel {
                             FontRelativeLength::Em(em) => {
                                 ret.em = Some(ret.em.unwrap_or(0.) + em * factor);
                             }
                             FontRelativeLength::Ex(ex) => {
--- a/servo/components/style/values/specified/length.rs
+++ b/servo/components/style/values/specified/length.rs
@@ -8,17 +8,17 @@
 
 use app_units::{Au, MAX_AU, MIN_AU};
 use cssparser::{Parser, Token, BasicParseError};
 use euclid::Size2D;
 use font_metrics::FontMetricsQueryResult;
 use parser::{Parse, ParserContext};
 use std::{cmp, fmt, mem};
 use std::ascii::AsciiExt;
-use std::ops::Mul;
+use std::ops::{Add, Mul};
 use style_traits::{ToCss, ParseError, StyleParseError};
 use style_traits::values::specified::AllowedLengthType;
 use stylesheets::CssRuleType;
 use super::{AllowQuirks, Number, ToComputedValue, Percentage};
 use values::{Auto, CSSFloat, Either, FONT_MEDIUM_PX, None_, Normal};
 use values::{ExtremumLength, serialize_dimension};
 use values::computed::{self, Context};
 use values::generics::NonNegative;
@@ -273,16 +273,33 @@ impl AbsoluteLength {
             | AbsoluteLength::In(v)
             | AbsoluteLength::Cm(v)
             | AbsoluteLength::Mm(v)
             | AbsoluteLength::Q(v)
             | AbsoluteLength::Pt(v)
             | AbsoluteLength::Pc(v) => v == 0.,
         }
     }
+
+    /// Convert this into a pixel value.
+    #[inline]
+    pub fn to_px(&self) -> CSSFloat {
+        use std::f32;
+
+        let pixel = match *self {
+            AbsoluteLength::Px(value) => value,
+            AbsoluteLength::In(value) => value * (AU_PER_IN / AU_PER_PX),
+            AbsoluteLength::Cm(value) => value * (AU_PER_CM / AU_PER_PX),
+            AbsoluteLength::Mm(value) => value * (AU_PER_MM / AU_PER_PX),
+            AbsoluteLength::Q(value) => value * (AU_PER_Q / AU_PER_PX),
+            AbsoluteLength::Pt(value) => value * (AU_PER_PT / AU_PER_PX),
+            AbsoluteLength::Pc(value) => value * (AU_PER_PC / AU_PER_PX),
+        };
+        pixel.min(f32::MAX).max(f32::MIN)
+    }
 }
 
 impl ToComputedValue for AbsoluteLength {
     type ComputedValue = Au;
 
     fn to_computed_value(&self, _: &Context) -> Au {
         Au::from(*self)
     }
@@ -332,16 +349,34 @@ impl Mul<CSSFloat> for AbsoluteLength {
             AbsoluteLength::Mm(v) => AbsoluteLength::Mm(v * scalar),
             AbsoluteLength::Q(v) => AbsoluteLength::Q(v * scalar),
             AbsoluteLength::Pt(v) => AbsoluteLength::Pt(v * scalar),
             AbsoluteLength::Pc(v) => AbsoluteLength::Pc(v * scalar),
         }
     }
 }
 
+impl Add<AbsoluteLength> for AbsoluteLength {
+    type Output = Self;
+
+    #[inline]
+    fn add(self, rhs: Self) -> Self {
+        match (self, rhs) {
+            (AbsoluteLength::Px(x), AbsoluteLength::Px(y)) => AbsoluteLength::Px(x + y),
+            (AbsoluteLength::In(x), AbsoluteLength::In(y)) => AbsoluteLength::In(x + y),
+            (AbsoluteLength::Cm(x), AbsoluteLength::Cm(y)) => AbsoluteLength::Cm(x + y),
+            (AbsoluteLength::Mm(x), AbsoluteLength::Mm(y)) => AbsoluteLength::Mm(x + y),
+            (AbsoluteLength::Q(x), AbsoluteLength::Q(y)) => AbsoluteLength::Q(x + y),
+            (AbsoluteLength::Pt(x), AbsoluteLength::Pt(y)) => AbsoluteLength::Pt(x + y),
+            (AbsoluteLength::Pc(x), AbsoluteLength::Pc(y)) => AbsoluteLength::Pc(x + y),
+            _ => AbsoluteLength::Px(self.to_px() + rhs.to_px()),
+        }
+    }
+}
+
 /// Represents a physical length (mozmm) based on DPI
 #[derive(Clone, Copy, Debug, PartialEq)]
 #[cfg(feature = "gecko")]
 pub struct PhysicalLength(pub CSSFloat);
 
 #[cfg(feature = "gecko")]
 impl PhysicalLength {
     fn is_zero(&self) -> bool {