style: Use mozilla::Length rather than nscoord to store font sizes.

This avoids arbitrary precision loss when computing REM units and so on,
which is particularly important if we ever change the base of our app
units (but useful regardless).

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

components/style/gecko/media_queries.rs

@@ -11,6 +11,7 @@ use crate::gecko_bindings::structs;
 use crate::media_queries::MediaType;
 use crate::properties::ComputedValues;
 use crate::string_cache::Atom;
+use crate::values::computed::Length;
 use crate::values::specified::font::FONT_MEDIUM_PX;
 use crate::values::{CustomIdent, KeyframesName};
 use app_units::{Au, AU_PER_PX};
@@ -19,7 +20,7 @@ use euclid::default::Size2D;
 use euclid::{Scale, SideOffsets2D};
 use servo_arc::Arc;
 use std::fmt;
-use std::sync::atomic::{AtomicBool, AtomicIsize, AtomicUsize, Ordering};
+use std::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize, Ordering};
 use style_traits::viewport::ViewportConstraints;
 use style_traits::{CSSPixel, DevicePixel};
 
@@ -30,15 +31,16 @@ pub struct Device {
     /// `Device`, so having a raw document pointer here is fine.
     document: *const structs::Document,
     default_values: Arc<ComputedValues>,
-    /// The font size of the root element
+    /// The font size of the root element.
-    /// This is set when computing the style of the root
-    /// element, and used for rem units in other elements.
     ///
-    /// When computing the style of the root element, there can't be any
+    /// This is set when computing the style of the root element, and used for
-    /// other style being computed at the same time, given we need the style of
+    /// rem units in other elements.
-    /// the parent to compute everything else. So it is correct to just use
+    ///
-    /// a relaxed atomic here.
+    /// When computing the style of the root element, there can't be any other
-    root_font_size: AtomicIsize,
+    /// style being computed at the same time, given we need the style of the
+    /// parent to compute everything else. So it is correct to just use a
+    /// relaxed atomic here.
+    root_font_size: AtomicU32,
     /// The body text color, stored as an `nscolor`, used for the "tables
     /// inherit from body" quirk.
     ///
@@ -85,8 +87,7 @@ impl Device {
         Device {
             document,
             default_values: ComputedValues::default_values(doc),
-            // FIXME(bz): Seems dubious?
+            root_font_size: AtomicU32::new(FONT_MEDIUM_PX.to_bits()),
-            root_font_size: AtomicIsize::new(Au::from_px(FONT_MEDIUM_PX as i32).0 as isize),
             body_text_color: AtomicUsize::new(prefs.mDefaultColor as usize),
             used_root_font_size: AtomicBool::new(false),
             used_viewport_size: AtomicBool::new(false),
@@ -131,15 +132,15 @@ impl Device {
     }
 
     /// Get the font size of the root element (for rem)
-    pub fn root_font_size(&self) -> Au {
+    pub fn root_font_size(&self) -> Length {
         self.used_root_font_size.store(true, Ordering::Relaxed);
-        Au::new(self.root_font_size.load(Ordering::Relaxed) as i32)
+        Length::new(f32::from_bits(self.root_font_size.load(Ordering::Relaxed)))
     }
 
     /// Set the font size of the root element (for rem)
-    pub fn set_root_font_size(&self, size: Au) {
+    pub fn set_root_font_size(&self, size: Length) {
         self.root_font_size
-            .store(size.0 as isize, Ordering::Relaxed)
+            .store(size.px().to_bits(), Ordering::Relaxed)
     }
 
     /// Sets the body text color for the "inherit color from body" quirk.
@@ -298,13 +299,13 @@ impl Device {
 
     /// Applies text zoom to a font-size or line-height value (see nsStyleFont::ZoomText).
     #[inline]
-    pub fn zoom_text(&self, size: Au) -> Au {
+    pub fn zoom_text(&self, size: Length) -> Length {
         size.scale_by(self.effective_text_zoom())
     }
 
     /// Un-apply text zoom.
     #[inline]
-    pub fn unzoom_text(&self, size: Au) -> Au {
+    pub fn unzoom_text(&self, size: Length) -> Length {
         size.scale_by(1. / self.effective_text_zoom())
     }
 

components/style/gecko/wrapper.rs

@@ -71,7 +71,6 @@ use crate::values::computed::font::GenericFontFamily;
 use crate::values::computed::Length;
 use crate::values::specified::length::FontBaseSize;
 use crate::CaseSensitivityExt;
-use app_units::Au;
 use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
 use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator};
 use selectors::attr::{CaseSensitivity, NamespaceConstraint};
@@ -922,14 +921,12 @@ fn get_animation_rule(
 #[derive(Debug)]
 /// Gecko font metrics provider
 pub struct GeckoFontMetricsProvider {
-    /// Cache of base font sizes for each language
+    /// Cache of base font sizes for each language. Usually will have 1 element.
     ///
-    /// Usually will have 1 element.
+    /// This may be slow on pages using more languages, might be worth
-    ///
+    /// optimizing by caching lang->group mapping separately and/or using a
-    // This may be slow on pages using more languages, might be worth optimizing
+    /// hashmap on larger loads.
-    // by caching lang->group mapping separately and/or using a hashmap on larger
+    pub font_size_cache: RefCell<Vec<(Atom, DefaultFontSizes)>>,
-    // loads.
-    pub font_size_cache: RefCell<Vec<(Atom, crate::gecko_bindings::structs::FontSizePrefs)>>,
 }
 
 impl GeckoFontMetricsProvider {
@@ -952,8 +949,9 @@ impl FontMetricsProvider for GeckoFontMetricsProvider {
             return sizes.1.size_for_generic(font_family);
         }
         let sizes = unsafe { bindings::Gecko_GetBaseSize(font_name.as_ptr()) };
+        let size = sizes.size_for_generic(font_family);
         cache.push((font_name.clone(), sizes));
-        sizes.size_for_generic(font_family)
+        size
     }
 
     fn query(
@@ -967,7 +965,7 @@ impl FontMetricsProvider for GeckoFontMetricsProvider {
             None => return Default::default(),
         };
 
-        let size = Au::from(base_size.resolve(context));
+        let size = base_size.resolve(context);
         let style = context.style();
 
         let (wm, font) = match base_size {
@@ -987,15 +985,15 @@ impl FontMetricsProvider for GeckoFontMetricsProvider {
                 pc,
                 vertical_metrics,
                 font.gecko(),
-                size.0,
+                size,
                 // we don't use the user font set in a media query
                 !context.in_media_query,
             )
         };
         FontMetrics {
-            x_height: Some(Au(gecko_metrics.mXSize).into()),
+            x_height: Some(gecko_metrics.mXSize),
-            zero_advance_measure: if gecko_metrics.mChSize >= 0 {
+            zero_advance_measure: if gecko_metrics.mChSize.px() >= 0. {
-                Some(Au(gecko_metrics.mChSize).into())
+                Some(gecko_metrics.mChSize)
             } else {
                 None
             },
@@ -1003,20 +1001,31 @@ impl FontMetricsProvider for GeckoFontMetricsProvider {
     }
 }
 
-impl structs::FontSizePrefs {
+/// The default font sizes for generic families for a given language group.
+#[derive(Debug)]
+#[repr(C)]
+pub struct DefaultFontSizes {
+    variable: Length,
+    serif: Length,
+    sans_serif: Length,
+    monospace: Length,
+    cursive: Length,
+    fantasy: Length,
+}
+
+impl DefaultFontSizes {
     fn size_for_generic(&self, font_family: GenericFontFamily) -> Length {
-        Au(match font_family {
+        match font_family {
-            GenericFontFamily::None => self.mDefaultVariableSize,
+            GenericFontFamily::None => self.variable,
-            GenericFontFamily::Serif => self.mDefaultSerifSize,
+            GenericFontFamily::Serif => self.serif,
-            GenericFontFamily::SansSerif => self.mDefaultSansSerifSize,
+            GenericFontFamily::SansSerif => self.sans_serif,
-            GenericFontFamily::Monospace => self.mDefaultMonospaceSize,
+            GenericFontFamily::Monospace => self.monospace,
-            GenericFontFamily::Cursive => self.mDefaultCursiveSize,
+            GenericFontFamily::Cursive => self.cursive,
-            GenericFontFamily::Fantasy => self.mDefaultFantasySize,
+            GenericFontFamily::Fantasy => self.fantasy,
             GenericFontFamily::MozEmoji => unreachable!(
                 "Should never get here, since this doesn't (yet) appear on font family"
             ),
-        })
+        }
-        .into()
     }
 }
 

components/style/properties/cascade.rs

@@ -845,7 +845,6 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
     fn recompute_keyword_font_size_if_needed(&mut self) {
         use crate::values::computed::ToComputedValue;
         use crate::values::specified;
-        use app_units::Au;
 
         if !self.seen.contains(LonghandId::XLang) &&
            !self.seen.contains(LonghandId::FontFamily) {
@@ -863,7 +862,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 }
             };
 
-            if font.gecko().mScriptUnconstrainedSize == Au::from(new_size.size()).0 {
+            if font.gecko().mScriptUnconstrainedSize == new_size.size {
                 return;
             }
 
@@ -878,6 +877,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
     #[cfg(feature = "gecko")]
     fn constrain_font_size_if_needed(&mut self) {
         use crate::gecko_bindings::bindings;
+        use crate::values::generics::NonNegative;
 
         if !self.seen.contains(LonghandId::XLang) &&
            !self.seen.contains(LonghandId::FontFamily) &&
@@ -896,11 +896,11 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 )
             };
 
-            if font.mFont.size >= min_font_size {
+            if font.mFont.size.0 >= min_font_size {
                 return;
             }
 
-            min_font_size
+            NonNegative(min_font_size)
         };
 
         builder.mutate_font().gecko_mut().mFont.size = min_font_size;
@@ -942,8 +942,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
     /// not clear to me. For now just pretend those don't exist here.
     #[cfg(feature = "gecko")]
     fn handle_mathml_scriptlevel_if_needed(&mut self) {
-        use app_units::Au;
+        use crate::values::generics::NonNegative;
-        use std::cmp;
 
         if !self.seen.contains(LonghandId::MozScriptLevel) &&
            !self.seen.contains(LonghandId::MozScriptMinSize) &&
@@ -968,14 +967,14 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 return;
             }
 
-            let mut min = Au(parent_font.mScriptMinSize);
+            let mut min = parent_font.mScriptMinSize;
             if font.mAllowZoomAndMinSize {
                 min = builder.device.zoom_text(min);
             }
 
             let scale = (parent_font.mScriptSizeMultiplier as f32).powi(delta as i32);
-            let parent_size = Au(parent_font.mSize);
+            let parent_size = parent_font.mSize.0;
-            let parent_unconstrained_size = Au(parent_font.mScriptUnconstrainedSize);
+            let parent_unconstrained_size = parent_font.mScriptUnconstrainedSize.0;
             let new_size = parent_size.scale_by(scale);
             let new_unconstrained_size = parent_unconstrained_size.scale_by(scale);
 
@@ -987,7 +986,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 if parent_size <= min {
                     (parent_size, new_unconstrained_size)
                 } else {
-                    (cmp::max(min, new_size), new_unconstrained_size)
+                    (min.max(new_size), new_unconstrained_size)
                 }
             } else {
                 // If the new unconstrained size is larger than the min size,
@@ -996,15 +995,15 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 // However, if the new size is even larger (perhaps due to usage
                 // of em units), use that instead.
                 (
-                    cmp::min(new_size, cmp::max(new_unconstrained_size, min)),
+                    new_size.min(new_unconstrained_size.max(min)),
                     new_unconstrained_size
                 )
             }
         };
         let font = builder.mutate_font().gecko_mut();
-        font.mFont.size = new_size.0;
+        font.mFont.size = NonNegative(new_size);
-        font.mSize = new_size.0;
+        font.mSize = NonNegative(new_size);
-        font.mScriptUnconstrainedSize = new_unconstrained_size.0;
+        font.mScriptUnconstrainedSize = NonNegative(new_unconstrained_size);
     }
 
     /// Various properties affect how font-size and font-family are computed.

components/style/properties/gecko.mako.rs

@@ -376,18 +376,6 @@ def set_gecko_property(ffi_name, expr):
 <%call expr="impl_simple_clone(ident, gecko_ffi_name)"></%call>
 </%def>
 
-<%def name="impl_absolute_length(ident, gecko_ffi_name)">
-    #[allow(non_snake_case)]
-    pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
-        ${set_gecko_property(gecko_ffi_name, "v.to_i32_au()")}
-    }
-    <%call expr="impl_simple_copy(ident, gecko_ffi_name)"></%call>
-    #[allow(non_snake_case)]
-    pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
-        Au(self.gecko.${gecko_ffi_name}).into()
-    }
-</%def>
-
 <%def name="impl_non_negative_length(ident, gecko_ffi_name, inherit_from=None,
                                      round_to_pixels=False)">
     #[allow(non_snake_case)]
@@ -593,11 +581,6 @@ impl Clone for ${style_struct.gecko_struct_name} {
     longhands = [x for x in style_struct.longhands
                 if not (skip_longhands == "*" or x.name in skip_longhands.split())]
 
-    # Types used with predefined_type()-defined properties that we can auto-generate.
-    predefined_types = {
-        "MozScriptMinSize": impl_absolute_length,
-    }
-
     def longhand_method(longhand):
         args = dict(ident=longhand.ident, gecko_ffi_name=longhand.gecko_ffi_name)
 
@@ -610,8 +593,6 @@ impl Clone for ${style_struct.gecko_struct_name} {
             args.update(keyword=longhand.keyword)
             if "font" in longhand.ident:
                 args.update(cast_type=longhand.cast_type)
-        elif longhand.predefined_type in predefined_types:
-            method = predefined_types[longhand.predefined_type]
         else:
             method = impl_simple
 
@@ -920,9 +901,10 @@ fn static_assert() {
     }
 
     pub fn unzoom_fonts(&mut self, device: &Device) {
-        self.gecko.mSize = device.unzoom_text(Au(self.gecko.mSize)).0;
+        use crate::values::generics::NonNegative;
-        self.gecko.mScriptUnconstrainedSize = device.unzoom_text(Au(self.gecko.mScriptUnconstrainedSize)).0;
+        self.gecko.mSize = NonNegative(device.unzoom_text(self.gecko.mSize.0));
-        self.gecko.mFont.size = device.unzoom_text(Au(self.gecko.mFont.size)).0;
+        self.gecko.mScriptUnconstrainedSize = NonNegative(device.unzoom_text(self.gecko.mScriptUnconstrainedSize.0));
+        self.gecko.mFont.size = NonNegative(device.unzoom_text(self.gecko.mFont.size.0));
     }
 
     pub fn copy_font_size_from(&mut self, other: &Self) {
@@ -942,25 +924,27 @@ fn static_assert() {
     }
 
     pub fn set_font_size(&mut self, v: FontSize) {
-        let size = Au::from(v.size());
+        let size = v.size;
-        self.gecko.mScriptUnconstrainedSize = size.0;
+        self.gecko.mScriptUnconstrainedSize = size;
 
         // These two may be changed from Cascade::fixup_font_stuff.
-        self.gecko.mSize = size.0;
+        self.gecko.mSize = size;
-        self.gecko.mFont.size = size.0;
+        self.gecko.mFont.size = size;
+
         self.gecko.mFontSizeKeyword = v.keyword_info.kw;
         self.gecko.mFontSizeFactor = v.keyword_info.factor;
-        self.gecko.mFontSizeOffset = v.keyword_info.offset.to_i32_au();
+        self.gecko.mFontSizeOffset = v.keyword_info.offset;
     }
 
     pub fn clone_font_size(&self) -> FontSize {
         use crate::values::specified::font::KeywordInfo;
+
         FontSize {
-            size: Au(self.gecko.mSize).into(),
+            size: self.gecko.mSize,
             keyword_info: KeywordInfo {
                 kw: self.gecko.mFontSizeKeyword,
                 factor: self.gecko.mFontSizeFactor,
-                offset: Au(self.gecko.mFontSizeOffset).into()
+                offset: self.gecko.mFontSizeOffset,
             }
         }
     }

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

@@ -307,7 +307,6 @@ ${helpers.predefined_type(
         //! variable reference. We may want to improve this behavior at some
         //! point. See also https://github.com/w3c/csswg-drafts/issues/1586.
 
-        use app_units::Au;
         use cssparser::{Parser, ToCss};
         use crate::values::computed::font::GenericFontFamily;
         use crate::properties::longhands;
@@ -397,7 +396,7 @@ ${helpers.predefined_type(
                         is_system_font: true,
                     },
                     font_size: FontSize {
-                        size: NonNegative(cx.maybe_zoom_text(Au(system.size).into())),
+                        size: NonNegative(cx.maybe_zoom_text(system.size.0)),
                         keyword_info: KeywordInfo::none()
                     },
                     font_weight,

components/style/servo/media_queries.rs

@@ -17,7 +17,7 @@ use app_units::Au;
 use cssparser::RGBA;
 use euclid::default::Size2D as UntypedSize2D;
 use euclid::{Scale, SideOffsets2D, Size2D};
-use std::sync::atomic::{AtomicBool, AtomicIsize, Ordering};
+use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
 use style_traits::viewport::ViewportConstraints;
 use style_traits::{CSSPixel, DevicePixel};
 
@@ -43,7 +43,7 @@ pub struct Device {
     /// the parent to compute everything else. So it is correct to just use
     /// a relaxed atomic here.
     #[ignore_malloc_size_of = "Pure stack type"]
-    root_font_size: AtomicIsize,
+    root_font_size: AtomicU32,
     /// Whether any styles computed in the document relied on the root font-size
     /// by using rem units.
     #[ignore_malloc_size_of = "Pure stack type"]
@@ -68,7 +68,7 @@ impl Device {
             viewport_size,
             device_pixel_ratio,
             // FIXME(bz): Seems dubious?
-            root_font_size: AtomicIsize::new(Au::from_px(FONT_MEDIUM_PX).0 as isize),
+            root_font_size: AtomicU32::new(FONT_MEDIUM_PX.to_bits()),
             used_root_font_size: AtomicBool::new(false),
             used_viewport_units: AtomicBool::new(false),
             environment: CssEnvironment,
@@ -90,15 +90,15 @@ impl Device {
     }
 
     /// Get the font size of the root element (for rem)
-    pub fn root_font_size(&self) -> Au {
+    pub fn root_font_size(&self) -> CSSPixelLength {
         self.used_root_font_size.store(true, Ordering::Relaxed);
-        Au::new(self.root_font_size.load(Ordering::Relaxed) as i32)
+        CSSPixelLength::new(f32::from_bits(self.root_font_size.load(Ordering::Relaxed)))
     }
 
     /// Set the font size of the root element (for rem)
-    pub fn set_root_font_size(&self, size: Au) {
+    pub fn set_root_font_size(&self, size: CSSPixelLength) {
         self.root_font_size
-            .store(size.0 as isize, Ordering::Relaxed)
+            .store(size.px().to_bits(), Ordering::Relaxed)
     }
 
     /// Sets the body text color for the "inherit color from body" quirk.

components/style/style_adjuster.rs

@@ -12,7 +12,6 @@ use crate::properties::longhands::float::computed_value::T as Float;
 use crate::properties::longhands::overflow_x::computed_value::T as Overflow;
 use crate::properties::longhands::position::computed_value::T as Position;
 use crate::properties::{self, ComputedValues, StyleBuilder};
-use app_units::Au;
 
 /// A struct that implements all the adjustment methods.
 ///
@@ -414,7 +413,9 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
             .none_or_hidden() &&
             self.style.get_outline().outline_has_nonzero_width()
         {
-            self.style.mutate_outline().set_outline_width(Au(0).into());
+            self.style
+                .mutate_outline()
+                .set_outline_width(crate::Zero::zero());
         }
     }
 

components/style/values/computed/font.rs

@@ -156,7 +156,7 @@ impl FontSize {
     /// Get default value of font size.
     pub fn medium() -> Self {
         Self {
-            size: NonNegative(Length::new(specified::FONT_MEDIUM_PX as CSSFloat)),
+            size: NonNegative(Length::new(specified::FONT_MEDIUM_PX)),
             keyword_info: KeywordInfo::medium(),
         }
     }

components/style/values/computed/length.rs

@@ -224,6 +224,12 @@ impl CSSPixelLength {
         CSSPixelLength(px)
     }
 
+    /// Scale the length by a given amount.
+    #[inline]
+    pub fn scale_by(self, scale: CSSFloat) -> Self {
+        CSSPixelLength(self.0 * scale)
+    }
+
     /// Return the containing pixel value.
     #[inline]
     pub fn px(self) -> CSSFloat {

components/style/values/computed/mod.rs

@@ -232,7 +232,7 @@ impl<'a> Context<'a> {
         // -x-text-zoom property, which leads to a false value
         // in mAllowZoomAndMinSize
         if self.style().get_font().gecko.mAllowZoomAndMinSize {
-            self.device().zoom_text(Au::from(size)).into()
+            self.device().zoom_text(size)
         } else {
             size
         }

components/style/values/specified/font.rs

@@ -787,13 +787,13 @@ impl ToComputedValue for FontSizeAdjust {
 const LARGER_FONT_SIZE_RATIO: f32 = 1.2;
 
 /// The default font size.
-pub const FONT_MEDIUM_PX: i32 = 16;
+pub const FONT_MEDIUM_PX: f32 = 16.0;
 
 impl FontSizeKeyword {
     #[inline]
     #[cfg(feature = "servo")]
     fn to_length(&self, _: &Context) -> NonNegativeLength {
-        let medium = Length::new(FONT_MEDIUM_PX as f32);
+        let medium = Length::new(FONT_MEDIUM_PX);
         // https://drafts.csswg.org/css-fonts-3/#font-size-prop
         NonNegative(match *self {
             FontSizeKeyword::XXSmall => medium * 3.0 / 5.0,

components/style/values/specified/length.rs

@@ -241,7 +241,7 @@ impl FontRelativeLength {
                 let reference_size = if context.builder.is_root_element || context.in_media_query {
                     reference_font_size
                 } else {
-                    computed::Length::new(context.device().root_font_size().to_f32_px())
+                    context.device().root_font_size()
                 };
                 (reference_size, length)
             },