servo: Merge #19074 - style: Avoid double-applying text-zoom for keywords (from emilio:font-size-woes); r=Manishearth
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 31 Oct 2017 15:08:24 -0500
changeset 389366 95e8712dbf3baa0bb59d676aec75df3fbccd6240
parent 389365 0c00ba0de42e8710c791205c2fb1cc5aa28f07ca
child 389367 46a4ddbc054104db241fcf4b337824e2ef387b03
push id54462
push userservo-vcs-sync@mozilla.com
push dateTue, 31 Oct 2017 22:06:32 +0000
treeherderautoland@95e8712dbf3b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersManishearth
bugs19074, 1412743
milestone58.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 #19074 - style: Avoid double-applying text-zoom for keywords (from emilio:font-size-woes); r=Manishearth Bug: 1412743 Reviewed-by: Manishearth Source-Repo: https://github.com/servo/servo Source-Revision: c882266c8915b9f424cc4efbc860a9e417bf1326
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/values/computed/font.rs
servo/components/style/values/specified/font.rs
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -847,18 +847,17 @@ macro_rules! impl_gecko_keyword_conversi
         % if product == "gecko":
             // 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.mRawPtr !=
                context.builder.get_parent_font().gecko().mLanguage.mRawPtr ||
                context.builder.get_font().gecko().mGenericID !=
                context.builder.get_parent_font().gecko().mGenericID {
                 if let Some(info) = computed.keyword_info {
-                    computed.size = context.maybe_zoom_text(info.kw.to_computed_value(context)
-                                                                .scale_by(info.factor) + info.offset)
+                    computed.size = info.to_computed_value(context);
                 }
             }
         % endif
 
         let device = context.builder.device;
         let mut font = context.builder.take_font();
         let parent_unconstrained = {
             let parent_font = context.builder.get_parent_font();
@@ -880,18 +879,17 @@ macro_rules! impl_gecko_keyword_conversi
     /// 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.get_parent_font()
                                        .clone_font_size()
                                        .keyword_info.map(|info| {
-            context.maybe_zoom_text(SpecifiedValue::Keyword(info)
-                  .to_computed_value(context).size)
+            SpecifiedValue::Keyword(info).to_computed_value(context).size
         });
         let mut font = context.builder.take_font();
         font.inherit_font_size_from(context.builder.get_parent_font(),
                                     kw_inherited_size,
                                     context.builder.device);
         context.builder.put_font(font);
     }
 
@@ -899,19 +897,19 @@ macro_rules! impl_gecko_keyword_conversi
     ///
     /// FIXME(emilio): This is the only function that is outside of the
     /// `StyleBuilder`, and should really move inside!
     ///
     /// Can we move the font stuff there?
     pub fn cascade_initial_font_size(context: &mut Context) {
         // font-size's default ("medium") does not always
         // compute to the same value and depends on the font
-        let mut computed = longhands::font_size::get_initial_specified_value()
-                                            .to_computed_value(context);
-        computed.size = context.maybe_zoom_text(computed.size);
+        let computed =
+            longhands::font_size::get_initial_specified_value()
+                .to_computed_value(context);
         context.builder.mutate_font().set_font_size(computed);
         % if product == "gecko":
             let device = context.builder.device;
             context.builder.mutate_font().fixup_font_min_size(device);
         % endif
     }
 </%helpers:longhand>
 
--- a/servo/components/style/values/computed/font.rs
+++ b/servo/components/style/values/computed/font.rs
@@ -33,16 +33,23 @@ pub struct KeywordInfo {
     pub kw: specified::KeywordSize,
     /// A factor to be multiplied by the computed size of the keyword
     pub factor: f32,
     /// An additional Au offset to add to the kw*factor in the case of calcs
     pub offset: NonNegativeLength,
 }
 
 impl KeywordInfo {
+    /// Computes the final size for this font-size keyword, accounting for
+    /// text-zoom.
+    pub fn to_computed_value(&self, context: &Context) -> NonNegativeLength {
+        let base = context.maybe_zoom_text(self.kw.to_computed_value(context));
+        base.scale_by(self.factor) + context.maybe_zoom_text(self.offset)
+    }
+
     /// Given a parent keyword info (self), apply an additional factor/offset to it
     pub fn compose(self, factor: f32, offset: NonNegativeLength) -> Self {
         KeywordInfo {
             kw: self.kw,
             factor: self.factor * factor,
             offset: self.offset.scale_by(factor) + offset,
         }
     }
--- a/servo/components/style/values/specified/font.rs
+++ b/servo/components/style/values/specified/font.rs
@@ -311,17 +311,17 @@ impl FontSize {
                     info = parent.keyword_info.map(|i| i.compose(ratio, abs));
                 }
                 let calc = calc.to_computed_value_zoomed(context, base_size);
                 calc.to_used_value(Some(base_size.resolve(context))).unwrap().into()
             }
             FontSize::Keyword(i) => {
                 // As a specified keyword, this is keyword derived
                 info = Some(i);
-                context.maybe_zoom_text(i.kw.to_computed_value(context).scale_by(i.factor) + i.offset)
+                i.to_computed_value(context)
             }
             FontSize::Smaller => {
                 info = compose_keyword(1. / LARGER_FONT_SIZE_RATIO);
                 FontRelativeLength::Em(1. / LARGER_FONT_SIZE_RATIO)
                     .to_computed_value(context, base_size).into()
             }
             FontSize::Larger => {
                 info = compose_keyword(LARGER_FONT_SIZE_RATIO);