Bug 1380980 - stylo: Add support for calcs in base size calculation; r?emilio draft
authorManish Goregaokar <manishearth@gmail.com>
Sat, 09 Sep 2017 01:10:50 -0700
changeset 662623 b7aedbd741d3f877138a7ac7a291feb688c2dd60
parent 662622 74a5747f6eff87b28873046bbf95ae908537d5f1
child 662624 57abbc5acc922520c49a63c78940a81f383f4bf6
push id79139
push userbmo:manishearth@gmail.com
push dateMon, 11 Sep 2017 21:27:49 +0000
reviewersemilio
bugs1380980
milestone57.0a1
Bug 1380980 - stylo: Add support for calcs in base size calculation; r?emilio This tracks an offset along with the ratio for the keyword font size data. The offset gets used when a calc is involved whilst inheriting font-size (it is the computed value of that calc ignoring em/percentage portions, which go into the ratio). If the family or language changes, the new font size can be computed by taking the keyword's size in the context of that family/language, multiplying it by the ratio, and adding the offset. MozReview-Commit-ID: L22JlbQKLCD
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/values/computed/length.rs
servo/components/style/values/computed/mod.rs
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -600,35 +600,39 @@ macro_rules! impl_gecko_keyword_conversi
     use values::computed::NonNegativeAu;
     use values::specified::{AllowQuirks, FontRelativeLength, LengthOrPercentage, NoCalcLength};
     use values::specified::length::FontBaseSize;
 
     impl ToCss for SpecifiedValue {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             match *self {
                 SpecifiedValue::Length(ref lop) => lop.to_css(dest),
-                SpecifiedValue::Keyword(kw, _) => kw.to_css(dest),
+                SpecifiedValue::Keyword(kw, _, _) => kw.to_css(dest),
                 SpecifiedValue::Smaller => dest.write_str("smaller"),
                 SpecifiedValue::Larger => dest.write_str("larger"),
                 SpecifiedValue::System(sys) => sys.to_css(dest),
             }
         }
     }
 
     #[derive(Clone, Debug, PartialEq)]
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     pub enum SpecifiedValue {
         Length(specified::LengthOrPercentage),
-        /// A keyword value, along with a ratio.
+        /// A keyword value, along with a ratio and absolute offset.
         /// The ratio in any specified keyword value
-        /// will be 1, but we cascade keywordness even
+        /// will be 1 (with offset 0), but we cascade keywordness even
         /// after font-relative (percent and em) values
-        /// have been applied, which is where the keyword
-        /// comes in. See bug 1355707
-        Keyword(KeywordSize, f32),
+        /// have been applied, which is where the ratio
+        /// comes in. The offset comes in if we cascaded a calc value,
+        /// where the font-relative portion (em and percentage) will
+        /// go into the ratio, and the remaining units all computed together
+        /// will go into the offset.
+        /// See bug 1355707.
+        Keyword(KeywordSize, f32, NonNegativeAu),
         Smaller,
         Larger,
         System(SystemFont)
     }
 
     impl From<specified::LengthOrPercentage> for SpecifiedValue {
         fn from(other: specified::LengthOrPercentage) -> Self {
             SpecifiedValue::Length(other)
@@ -797,42 +801,55 @@ macro_rules! impl_gecko_keyword_conversi
                 0 | 1 => XSmall,
                 2 => Small,
                 3 => Medium,
                 4 => Large,
                 5 => XLarge,
                 6 => XXLarge,
                 // If value is greater than 7, let it be 7.
                 _ => XXXLarge,
-            }, 1.)
+            }, 1., Au(0).into())
         }
 
         /// If this value is specified as a ratio of the parent font (em units
         /// or percent) return the ratio
-        pub fn as_font_ratio(&self) -> Option<f32> {
+        pub fn as_font_ratio(&self, context: &Context) -> Option<(f32, NonNegativeAu)> {
             match *self {
                 SpecifiedValue::Length(ref lop) => {
                     match *lop {
                         LengthOrPercentage::Percentage(pc) => {
-                            Some(pc.0)
+                            Some((pc.0, Au(0).into()))
                         }
                         LengthOrPercentage::Length(ref nocalc) => {
                             match *nocalc {
                                 NoCalcLength::FontRelative(FontRelativeLength::Em(em)) => {
-                                    Some(em)
+                                    Some((em, Au(0).into()))
                                 }
                                 _ => None,
                             }
                         }
-                        // FIXME(emilio): This looks super fishy!
-                        LengthOrPercentage::Calc(..) => None,
+                        LengthOrPercentage::Calc(ref calc) => {
+                            if calc.em.is_none() && calc.percentage.is_none() {
+                                return None;
+                            }
+                            let ratio = calc.em.unwrap_or(0.) + calc.percentage.map_or(0., |pc| pc.0);
+                            // Compute it, but shave off the font-relative part (em, %)
+                            // This will mean that other font-relative units like ex and ch will be computed against
+                            // the old font even when the font changes. There's no particular "right answer" for what
+                            // to do here -- Gecko recascades as if the font had changed, we instead track the changes
+                            // and reapply, which means that we carry over old computed ex/ch values whilst Gecko
+                            // recomputes new ones. This is enough of an edge case to not really matter.
+                            let abs = calc.to_computed_value_zoomed(context, FontBaseSize::Custom(Au(0).into()))
+                                          .length_component().into();
+                            Some((ratio, abs))
+                        }
                     }
                 }
-                SpecifiedValue::Larger => Some(LARGER_FONT_SIZE_RATIO),
-                SpecifiedValue::Smaller => Some(1. / LARGER_FONT_SIZE_RATIO),
+                SpecifiedValue::Larger => Some((LARGER_FONT_SIZE_RATIO, Au(0).into())),
+                SpecifiedValue::Smaller => Some((1. / LARGER_FONT_SIZE_RATIO, Au(0).into())),
                 _ => None,
             }
         }
 
         /// Compute it against a given base font size
         pub fn to_computed_value_against(
             &self,
             context: &Context,
@@ -857,18 +874,18 @@ macro_rules! impl_gecko_keyword_conversi
                 }
                 SpecifiedValue::Length(LengthOrPercentage::Percentage(pc)) => {
                     base_size.resolve(context).scale_by(pc.0).into()
                 }
                 SpecifiedValue::Length(LengthOrPercentage::Calc(ref calc)) => {
                     let calc = calc.to_computed_value_zoomed(context, base_size);
                     calc.to_used_value(Some(base_size.resolve(context))).unwrap().into()
                 }
-                SpecifiedValue::Keyword(ref key, fraction) => {
-                    context.maybe_zoom_text(key.to_computed_value(context).scale_by(fraction))
+                SpecifiedValue::Keyword(ref key, fraction, offset) => {
+                    context.maybe_zoom_text(key.to_computed_value(context).scale_by(fraction) + offset)
                 }
                 SpecifiedValue::Smaller => {
                     FontRelativeLength::Em(1. / LARGER_FONT_SIZE_RATIO)
                         .to_computed_value(context, base_size).into()
                 }
                 SpecifiedValue::Larger => {
                     FontRelativeLength::Em(LARGER_FONT_SIZE_RATIO)
                         .to_computed_value(context, base_size).into()
@@ -886,17 +903,17 @@ macro_rules! impl_gecko_keyword_conversi
     #[inline]
     #[allow(missing_docs)]
     pub fn get_initial_value() -> computed_value::T {
         NonNegativeAu::from_px(FONT_MEDIUM_PX)
     }
 
     #[inline]
     pub fn get_initial_specified_value() -> SpecifiedValue {
-        SpecifiedValue::Keyword(Medium, 1.)
+        SpecifiedValue::Keyword(Medium, 1., Au(0).into())
     }
 
 
     impl ToComputedValue for SpecifiedValue {
         type ComputedValue = computed_value::T;
 
         #[inline]
         fn to_computed_value(&self, context: &Context) -> computed_value::T {
@@ -923,17 +940,17 @@ macro_rules! impl_gecko_keyword_conversi
                                 allow_quirks: AllowQuirks)
                                 -> Result<SpecifiedValue, ParseError<'i>> {
         use self::specified::LengthOrPercentage;
         if let Ok(lop) = input.try(|i| LengthOrPercentage::parse_non_negative_quirky(context, i, allow_quirks)) {
             return Ok(SpecifiedValue::Length(lop))
         }
 
         if let Ok(kw) = input.try(KeywordSize::parse) {
-            return Ok(SpecifiedValue::Keyword(kw, 1.))
+            return Ok(SpecifiedValue::Keyword(kw, 1., Au(0).into()))
         }
 
         try_match_ident_ignore_ascii_case! { input.expect_ident()?,
             "smaller" => Ok(SpecifiedValue::Smaller),
             "larger" => Ok(SpecifiedValue::Larger),
         }
     }
 
@@ -949,27 +966,27 @@ macro_rules! impl_gecko_keyword_conversi
             }
         }
     }
 
     #[allow(unused_mut)]
     pub fn cascade_specified_font_size(context: &mut Context,
                                        specified_value: &SpecifiedValue,
                                        mut computed: NonNegativeAu) {
-        if let SpecifiedValue::Keyword(kw, fraction) = *specified_value {
-            context.builder.font_size_keyword = Some((kw, fraction, Au(0).into()));
-        } else if let Some(ratio) = specified_value.as_font_ratio() {
+        if let SpecifiedValue::Keyword(kw, fraction, offset) = *specified_value {
+            context.builder.font_size_keyword = Some((kw, fraction, offset));
+        } else if let Some((ratio, abs)) = specified_value.as_font_ratio(context) {
             // In case a font-size-relative value was applied to a keyword
             // value, we must preserve this fact in case the generic font family
             // changes. relative values (em and %) applied to keywords must be
             // recomputed from the base size for the keyword and the relative size.
             //
             // See bug 1355707
-            if let Some((kw, fraction, _)) = context.builder.inherited_font_computation_data().font_size_keyword {
-                context.builder.font_size_keyword = Some((kw, fraction * ratio, Au(0).into()));
+            if let Some((kw, fraction, old_abs)) = context.builder.inherited_font_computation_data().font_size_keyword {
+                context.builder.font_size_keyword = Some((kw, fraction * ratio, abs + old_abs.0.scale_by(ratio).into()));
             } else {
                 context.builder.font_size_keyword = None;
             }
         } else {
             context.builder.font_size_keyword = None;
         }
 
         // we could use clone_language and clone_font_family() here but that's
@@ -977,18 +994,18 @@ macro_rules! impl_gecko_keyword_conversi
         % if product == "gecko":
             use gecko_bindings::structs::nsIAtom;
             // if the language or generic changed, we need to recalculate
             // the font size from the stored font-size origin information.
             if context.builder.get_font().gecko().mLanguage.raw::<nsIAtom>() !=
                context.builder.get_parent_font().gecko().mLanguage.raw::<nsIAtom>() ||
                context.builder.get_font().gecko().mGenericID !=
                context.builder.get_parent_font().gecko().mGenericID {
-                if let Some((kw, ratio, _)) = context.builder.font_size_keyword {
-                    computed = context.maybe_zoom_text(kw.to_computed_value(context).scale_by(ratio));
+                if let Some((kw, ratio, offset)) = context.builder.font_size_keyword {
+                    computed = context.maybe_zoom_text(kw.to_computed_value(context).scale_by(ratio) + offset);
                 }
             }
         % endif
 
         let device = context.builder.device;
         let mut font = context.builder.take_font();
         let parent_unconstrained = {
             let parent_font = context.builder.get_parent_font();
@@ -1007,18 +1024,18 @@ macro_rules! impl_gecko_keyword_conversi
     }
 
     /// FIXME(emilio): This is very complex. Also, it should move to
     /// StyleBuilder.
     pub fn cascade_inherit_font_size(context: &mut Context) {
         // If inheriting, we must recompute font-size in case of language
         // changes using the font_size_keyword. We also need to do this to
         // handle mathml scriptlevel changes
-        let kw_inherited_size = context.builder.font_size_keyword.map(|(kw, ratio, _)| {
-            context.maybe_zoom_text(SpecifiedValue::Keyword(kw, ratio).to_computed_value(context))
+        let kw_inherited_size = context.builder.font_size_keyword.map(|(kw, ratio, offset)| {
+            context.maybe_zoom_text(SpecifiedValue::Keyword(kw, ratio, offset).to_computed_value(context))
         });
         let parent_kw;
         let device = context.builder.device;
         let mut font = context.builder.take_font();
         let used_kw = {
             let parent_font = context.builder.get_parent_font();
             parent_kw = context.builder.inherited_font_computation_data().font_size_keyword;
 
--- a/servo/components/style/values/computed/length.rs
+++ b/servo/components/style/values/computed/length.rs
@@ -108,16 +108,22 @@ impl CalcLengthOrPercentage {
     }
 
     /// Returns this `calc()` as a `<length>`.
     ///
     /// Panics in debug mode if a percentage is present in the expression.
     #[inline]
     pub fn length(&self) -> Au {
         debug_assert!(self.percentage.is_none());
+        self.length_component()
+    }
+
+    /// Returns the length component of this `calc()`
+    #[inline]
+    pub fn length_component(&self) -> Au {
         self.clamping_mode.clamp(self.length)
     }
 
     /// Returns the `<length>` component of this `calc()`, unclamped.
     #[inline]
     pub fn unclamped_length(&self) -> Au {
         self.length
     }
--- a/servo/components/style/values/computed/mod.rs
+++ b/servo/components/style/values/computed/mod.rs
@@ -9,18 +9,17 @@ use context::QuirksMode;
 use euclid::Size2D;
 use font_metrics::FontMetricsProvider;
 use media_queries::Device;
 #[cfg(feature = "gecko")]
 use properties;
 use properties::{ComputedValues, StyleBuilder};
 #[cfg(feature = "servo")]
 use servo_url::ServoUrl;
-use std::f32;
-use std::fmt;
+use std::{f32, fmt, ops};
 #[cfg(feature = "servo")]
 use std::sync::Arc;
 use style_traits::ToCss;
 use super::{CSSFloat, CSSInteger};
 use super::generics::{GreaterThanOrEqualToOne, NonNegative};
 use super::generics::grid::{TrackBreadth as GenericTrackBreadth, TrackSize as GenericTrackSize};
 use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent;
 use super::generics::grid::TrackList as GenericTrackList;
@@ -549,16 +548,23 @@ impl NonNegativeAu {
     /// Scale this NonNegativeAu.
     #[inline]
     pub fn scale_by(self, factor: f32) -> Self {
         // scale this by zero if factor is negative.
         NonNegative::<Au>(self.0.scale_by(factor.max(0.)))
     }
 }
 
+impl ops::Add<NonNegativeAu> for NonNegativeAu {
+    type Output = NonNegativeAu;
+    fn add(self, other: Self) -> Self {
+        (self.0 + other.0).into()
+    }
+}
+
 impl From<Au> for NonNegativeAu {
     #[inline]
     fn from(au: Au) -> NonNegativeAu {
         NonNegative::<Au>(au)
     }
 }
 
 /// The computed value of a CSS `url()`, resolved relative to the stylesheet URL.