Bug 1341724 - Part 3: stylo: Use gecko's font metrics; r?heycam draft
authorManish Goregaokar <manishearth@gmail.com>
Fri, 07 Apr 2017 15:49:44 -0700
changeset 559440 564cee469fad010113c1e913f61343913cf814dc
parent 559280 9872818523b2b9d3ca0fd42845526b7ef000588e
child 559441 c2fa7a37e37511c9f72773de4e997e1c61280cec
child 559446 c94f2f248eb9d848e75adcad68e8d19d0618ba8a
child 559453 10b79360c6f65efc8acbf351b23557ace52b4f9d
push id53091
push userbmo:manishearth@gmail.com
push dateMon, 10 Apr 2017 05:23:42 +0000
reviewersheycam
bugs1341724
milestone55.0a1
Bug 1341724 - Part 3: stylo: Use gecko's font metrics; r?heycam MozReview-Commit-ID: 6yYYauqCJHq
layout/style/ServoBindings.cpp
servo/components/style/font_metrics.rs
servo/components/style/gecko/media_queries.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/values/computed/mod.rs
servo/components/style/values/specified/length.rs
servo/components/style/viewport.rs
servo/ports/geckolib/glue.rs
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -1572,18 +1572,17 @@ Gecko_GetFontMetrics(RawGeckoPresContext
                      const nsStyleFont* aFont,
                      nscoord aFontSize,
                      bool aUseUserFontSet)
 {
   MutexAutoLock lock(*sServoFontMetricsLock);
   aPresContext->SetUsesExChUnits(true);
   GeckoFontMetrics ret;
   // Safe because we are locked, and this function is only
-  // ever called from Servo parallel traversal
-  MOZ_ASSERT(ServoStyleSet::IsInServoTraversal());
+  // ever called from Servo parallel traversal or the main thread
   nsPresContext* presContext = const_cast<nsPresContext*>(aPresContext);
   RefPtr<nsFontMetrics> fm = nsRuleNode::GetMetricsFor(presContext, aIsVertical,
                                                        aFont, aFontSize,
                                                        aUseUserFontSet);
   ret.mXSize = fm->XHeight();
   gfxFloat zeroWidth = fm->GetThebesFontGroup()->GetFirstValidFont()->
                            GetMetrics(fm->Orientation()).zeroOrAveCharWidth;
   ret.mChSize = ceil(aPresContext->AppUnitsPerDevPixel() * zeroWidth);
--- a/servo/components/style/font_metrics.rs
+++ b/servo/components/style/font_metrics.rs
@@ -4,42 +4,63 @@
 
 //! Access to font metrics from the style system.
 
 #![deny(missing_docs)]
 
 use Atom;
 use app_units::Au;
 use context::SharedStyleContext;
-use euclid::Size2D;
+use logical_geometry::WritingMode;
+use media_queries::Device;
+use properties::style_structs::Font;
 use std::fmt;
 
 /// Represents the font metrics that style needs from a font to compute the
 /// value of certain CSS units like `ex`.
 #[derive(Debug, PartialEq, Clone)]
 pub struct FontMetrics {
     /// The x-height of the font.
     pub x_height: Au,
-    /// The zero advance.
-    pub zero_advance_measure: Size2D<Au>,
+    /// The zero advance. This is usually writing mode dependent
+    pub zero_advance_measure: Au,
 }
 
 /// The result for querying font metrics for a given font family.
 #[derive(Debug, PartialEq, Clone)]
 pub enum FontMetricsQueryResult {
     /// The font is available, but we may or may not have found any font metrics
     /// for it.
-    Available(Option<FontMetrics>),
+    Available(FontMetrics),
     /// The font is not available.
     NotAvailable,
 }
 
+/// A trait used to represent something capable of providing us font metrics.
+pub trait FontMetricsProvider: fmt::Debug {
+    /// Obtain the metrics for given font family.
+    ///
+    /// TODO: We could make this take the full list, I guess, and save a few
+    /// virtual calls in the case we are repeatedly unable to find font metrics?
+    /// That is not too common in practice though.
+    fn query(&self, _font: &Font, _font_size: Au, _wm: WritingMode,
+             _in_media_query: bool, _device: &Device) -> FontMetricsQueryResult {
+        FontMetricsQueryResult::NotAvailable
+    }
+
+    /// Get default size of a given language and generic family
+    fn get_size(&self, font_name: &Atom, font_family: u8) -> Au;
+
+    /// Construct from a shared style context
+    fn create_from(context: &SharedStyleContext) -> Self where Self: Sized;
+}
+
+#[derive(Debug)]
 /// Dummy font metrics provider, for use by Servo
-/// and in cases where Gecko doesn't need font metrics
-#[derive(Debug)]
+/// and in cases where gecko doesn't need font metrics
 pub struct DummyProvider;
 
 #[cfg(feature = "servo")]
 /// Servo doesn't do font metrics yet, use same dummy provider.
 pub type ServoMetricsProvider = DummyProvider;
 
 impl FontMetricsProvider for DummyProvider {
     fn create_from(_: &SharedStyleContext) -> Self {
@@ -61,27 +82,8 @@ pub fn get_metrics_provider_for_product(
     ::gecko::wrapper::GeckoFontMetricsProvider::new()
 }
 
 #[cfg(feature = "servo")]
 /// Construct a font metrics provider for the current product
 pub fn get_metrics_provider_for_product() -> ServoMetricsProvider {
     ServoMetricsProvider
 }
-
-/// A trait used to represent something capable of providing us font metrics.
-pub trait FontMetricsProvider: fmt::Debug {
-    /// Obtain the metrics for given font family.
-    ///
-    /// TODO: We could make this take the full list, I guess, and save a few
-    /// virtual calls in the case we are repeatedly unable to find font metrics?
-    /// That is not too common in practice though.
-    fn query(&self, _font_name: &Atom) -> FontMetricsQueryResult {
-        FontMetricsQueryResult::NotAvailable
-    }
-
-    /// Get default size of a given language and generic family
-    fn get_size(&self, font_name: &Atom, font_family: u8) -> Au;
-
-    /// Construct from a shared style context
-    fn create_from(context: &SharedStyleContext) -> Self where Self: Sized;
-}
-
--- a/servo/components/style/gecko/media_queries.rs
+++ b/servo/components/style/gecko/media_queries.rs
@@ -508,16 +508,17 @@ impl Expression {
             is_root_element: false,
             device: device,
             inherited_style: default_values,
             layout_parent_style: default_values,
             // This cloning business is kind of dumb.... It's because Context
             // insists on having an actual ComputedValues inside itself.
             style: default_values.clone(),
             font_metrics_provider: &provider,
+            in_media_query: true,
         };
 
         let required_value = match self.value {
             Some(ref v) => v,
             None => {
                 // If there's no value, always match unless it's a zero length
                 // or a zero integer or boolean.
                 return match *actual_value {
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -17,17 +17,17 @@
 use app_units::Au;
 use atomic_refcell::AtomicRefCell;
 use context::{SharedStyleContext, UpdateAnimationsTasks};
 use data::ElementData;
 use dom::{self, AnimationRules, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode};
 use dom::{OpaqueNode, PresentationalHintsSynthetizer};
 use element_state::ElementState;
 use error_reporting::StdoutErrorReporter;
-use font_metrics::FontMetricsProvider;
+use font_metrics::{FontMetrics, FontMetricsProvider, FontMetricsQueryResult};
 use gecko::global_style_data::GLOBAL_STYLE_DATA;
 use gecko::selector_parser::{SelectorImpl, NonTSPseudoClass, PseudoElement};
 use gecko::snapshot_helpers;
 use gecko_bindings::bindings;
 use gecko_bindings::bindings::{Gecko_DropStyleChildrenIterator, Gecko_MaybeCreateStyleChildrenIterator};
 use gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetLastChild, Gecko_GetNextStyleChild};
 use gecko_bindings::bindings::{Gecko_IsRootElement, Gecko_MatchesElement, Gecko_Namespace};
 use gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags};
@@ -46,20 +46,23 @@ use gecko_bindings::structs;
 use gecko_bindings::structs::{RawGeckoElement, RawGeckoNode};
 use gecko_bindings::structs::{nsIAtom, nsIContent, nsStyleContext};
 use gecko_bindings::structs::EffectCompositor_CascadeLevel as CascadeLevel;
 use gecko_bindings::structs::NODE_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO;
 use gecko_bindings::structs::NODE_HAS_DIRTY_DESCENDANTS_FOR_SERVO;
 use gecko_bindings::structs::NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE;
 use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS;
 use gecko_bindings::sugar::ownership::HasArcFFI;
+use logical_geometry::WritingMode;
+use media_queries::Device;
 use parking_lot::RwLock;
 use properties::{ComputedValues, parse_style_attribute};
 use properties::PropertyDeclarationBlock;
 use properties::animated_properties::AnimationValueMap;
+use properties::style_structs::Font;
 use rule_tree::CascadeLevel as ServoCascadeLevel;
 use selector_parser::{ElementExt, Snapshot};
 use selectors::Element;
 use selectors::matching::{ElementSelectorFlags, StyleRelations};
 use selectors::parser::{AttrSelector, NamespaceConstraint};
 use shared_lock::Locked;
 use sink::Push;
 use std::cell::RefCell;
@@ -463,16 +466,34 @@ impl FontMetricsProvider for GeckoFontMe
             return sizes.1.size_for_generic(font_family);
         }
         let sizes = unsafe {
             Gecko_GetBaseSize(font_name.as_ptr())
         };
         cache.push((font_name.clone(), sizes));
         sizes.size_for_generic(font_family)
     }
+
+    fn query(&self, font: &Font, font_size: Au, wm: WritingMode,
+             in_media_query: bool, device: &Device) -> FontMetricsQueryResult {
+        use gecko_bindings::bindings::Gecko_GetFontMetrics;
+        let gecko_metrics = unsafe {
+            Gecko_GetFontMetrics(&*device.pres_context,
+                                 wm.is_vertical() && !wm.is_sideways(),
+                                 font.gecko(),
+                                 font_size.0,
+                                 // we don't use the user font set in a media query
+                                 !in_media_query)
+        };
+        let metrics = FontMetrics {
+            x_height: Au(gecko_metrics.mXSize),
+            zero_advance_measure: Au(gecko_metrics.mChSize),
+        };
+        FontMetricsQueryResult::Available(metrics)
+    }
 }
 
 impl structs::FontSizePrefs {
     fn size_for_generic(&self, font_family: u8) -> Au {
         Au(match font_family {
             structs::kPresContext_DefaultVariableFont_ID => self.mDefaultVariableSize,
             structs::kPresContext_DefaultFixedFont_ID => self.mDefaultFixedSize,
             structs::kGenericFont_serif => self.mDefaultSerifSize,
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -2060,16 +2060,17 @@ pub fn apply_declarations<'a, F, I>(devi
 
     let mut context = computed::Context {
         is_root_element: is_root_element,
         device: device,
         inherited_style: inherited_style,
         layout_parent_style: layout_parent_style,
         style: starting_style,
         font_metrics_provider: font_metrics_provider,
+        in_media_query: false,
     };
 
     // Set computed values, overwriting earlier declarations for the same
     // property.
     //
     // NB: The cacheable boolean is not used right now, but will be once we
     // start caching computed values in the rule nodes.
     let mut cacheable = true;
--- a/servo/components/style/values/computed/mod.rs
+++ b/servo/components/style/values/computed/mod.rs
@@ -53,16 +53,19 @@ pub struct Context<'a> {
     /// Values access through this need to be in the properties "computed
     /// early": color, text-decoration, font-size, display, position, float,
     /// border-*-style, outline-style, font-family, writing-mode...
     pub style: ComputedValues,
 
     /// A font metrics provider, used to access font metrics to implement
     /// font-relative units.
     pub font_metrics_provider: &'a FontMetricsProvider,
+
+    /// Whether or not we are computing the media list in a media query
+    pub in_media_query: bool,
 }
 
 impl<'a> Context<'a> {
     /// Whether the current element is the root element.
     pub fn is_root_element(&self) -> bool { self.is_root_element }
     /// The current viewport size.
     pub fn viewport_size(&self) -> Size2D<Au> { self.device.au_viewport_size() }
     /// The style we're inheriting from.
--- a/servo/components/style/values/specified/length.rs
+++ b/servo/components/style/values/specified/length.rs
@@ -4,17 +4,17 @@
 
 //! [Length values][length].
 //!
 //! [length]: https://drafts.csswg.org/css-values/#lengths
 
 use app_units::Au;
 use cssparser::{Parser, Token};
 use euclid::size::Size2D;
-use font_metrics::FontMetrics;
+use font_metrics::FontMetricsQueryResult;
 use parser::{Parse, ParserContext};
 use std::{cmp, fmt, mem};
 use std::ascii::AsciiExt;
 use std::ops::Mul;
 use style_traits::ToCss;
 use style_traits::values::specified::AllowedNumericType;
 use super::{Angle, Number, SimplifiedValueNode, SimplifiedSumNode, Time};
 use values::{Auto, CSSFloat, Either, FONT_MEDIUM_PX, HasViewportPercentage, None_, Normal};
@@ -64,81 +64,63 @@ impl ToCss for FontRelativeLength {
             FontRelativeLength::Ex(length) => write!(dest, "{}ex", length),
             FontRelativeLength::Ch(length) => write!(dest, "{}ch", length),
             FontRelativeLength::Rem(length) => write!(dest, "{}rem", length)
         }
     }
 }
 
 impl FontRelativeLength {
-    /// Gets the first available font metrics from the current context's
-    /// font-family list.
-    pub fn find_first_available_font_metrics(context: &Context) -> Option<FontMetrics> {
-        use font_metrics::FontMetricsQueryResult::*;
-        for family in context.style().get_font().font_family_iter() {
-            if let Available(metrics) = context.font_metrics_provider.query(family.atom()) {
-                return metrics;
-            }
-        }
-
-        None
-    }
-
     /// Computes the font-relative length. We use the use_inherited flag to
     /// special-case the computation of font-size.
     pub fn to_computed_value(&self, context: &Context, use_inherited: bool) -> Au {
+        fn query_font_metrics(context: &Context, reference_font_size: Au) -> FontMetricsQueryResult {
+            context.font_metrics_provider.query(context.style().get_font(),
+                                                reference_font_size,
+                                                context.style().writing_mode,
+                                                context.in_media_query,
+                                                context.device)
+        }
+
         let reference_font_size = if use_inherited {
             context.inherited_style().get_font().clone_font_size()
         } else {
             context.style().get_font().clone_font_size()
         };
 
         let root_font_size = context.style().root_font_size;
         match *self {
             FontRelativeLength::Em(length) => reference_font_size.scale_by(length),
             FontRelativeLength::Ex(length) => {
-                match Self::find_first_available_font_metrics(context) {
-                    Some(metrics) => metrics.x_height,
+                match query_font_metrics(context, reference_font_size) {
+                    FontMetricsQueryResult::Available(metrics) => metrics.x_height.scale_by(length),
                     // https://drafts.csswg.org/css-values/#ex
                     //
                     //     In the cases where it is impossible or impractical to
                     //     determine the x-height, a value of 0.5em must be
                     //     assumed.
                     //
-                    None => reference_font_size.scale_by(0.5 * length),
+                    FontMetricsQueryResult::NotAvailable => reference_font_size.scale_by(0.5 * length),
                 }
             },
             FontRelativeLength::Ch(length) => {
-                let wm = context.style().writing_mode;
-
-                // TODO(emilio, #14144): Compute this properly once we support
-                // all the relevant writing-mode related properties, this should
-                // be equivalent to "is the text in the block direction?".
-                let vertical = wm.is_vertical();
-
-                match Self::find_first_available_font_metrics(context) {
-                    Some(metrics) => {
-                        if vertical {
-                            metrics.zero_advance_measure.height
-                        } else {
-                            metrics.zero_advance_measure.width
-                        }
-                    }
+                match query_font_metrics(context, reference_font_size) {
+                    FontMetricsQueryResult::Available(metrics) => metrics.zero_advance_measure.scale_by(length),
                     // https://drafts.csswg.org/css-values/#ch
                     //
                     //     In the cases where it is impossible or impractical to
                     //     determine the measure of the “0” glyph, it must be
                     //     assumed to be 0.5em wide by 1em tall. Thus, the ch
                     //     unit falls back to 0.5em in the general case, and to
                     //     1em when it would be typeset upright (i.e.
                     //     writing-mode is vertical-rl or vertical-lr and
                     //     text-orientation is upright).
                     //
-                    None => {
-                        if vertical {
+                    FontMetricsQueryResult::NotAvailable => {
+                        if context.style().writing_mode.is_vertical() {
                             reference_font_size.scale_by(length)
                         } else {
                             reference_font_size.scale_by(0.5 * length)
                         }
                     }
                 }
             }
             FontRelativeLength::Rem(length) => root_font_size.scale_by(length)
--- a/servo/components/style/viewport.rs
+++ b/servo/components/style/viewport.rs
@@ -693,16 +693,17 @@ impl MaybeNew for ViewportConstraints {
         // TODO(emilio): Stop cloning `ComputedValues` around!
         let context = Context {
             is_root_element: false,
             device: device,
             inherited_style: device.default_computed_values(),
             layout_parent_style: device.default_computed_values(),
             style: device.default_computed_values().clone(),
             font_metrics_provider: &provider,
+            in_media_query: false,
         };
 
         // DEVICE-ADAPT § 9.3 Resolving 'extend-to-zoom'
         let extend_width;
         let extend_height;
         if let Some(extend_zoom) = max!(initial_zoom, max_zoom) {
             let scale_factor = 1. / extend_zoom;
             extend_width = Some(initial_viewport.width.scale_by(scale_factor));
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -1559,16 +1559,17 @@ pub extern "C" fn Servo_GetComputedKeyfr
 
     let context = Context {
         is_root_element: false,
         device: &data.stylist.device,
         inherited_style: parent_style.unwrap_or(default_values),
         layout_parent_style: parent_style.unwrap_or(default_values),
         style: (**style).clone(),
         font_metrics_provider: &metrics,
+        in_media_query: false,
     };
 
     for (index, keyframe) in keyframes.iter().enumerate() {
         let ref mut animation_values = computed_keyframes[index];
 
         let mut seen = LonghandIdSet::new();
 
         // mServoDeclarationBlock is null in the case where we have an invalid css property.