Bug 282126 - Part 2: Allow FontMetricsProvider to produce ex height and zero width independently. r=emilio draft
authorCameron McCormack <cam@mcc.id.au>
Wed, 20 Jun 2018 17:18:00 +1000
changeset 809793 97c95948d8adb360217413b5d1d0374611918b9a
parent 809792 f23e38381ed47990c4fe95823f0ce66cec9908ef
child 809794 39da73805b6f7821d19a07149bbecf61379a9b81
push id113820
push userbmo:cam@mcc.id.au
push dateSat, 23 Jun 2018 03:01:03 +0000
reviewersemilio
bugs282126
milestone62.0a1
Bug 282126 - Part 2: Allow FontMetricsProvider to produce ex height and zero width independently. r=emilio We are always able to produce an x height, but depending on whether the glyph exists, we sometimes can't produce a zero glyph width. MozReview-Commit-ID: 9UNuUJsSDxA
servo/components/style/font_metrics.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/values/specified/length.rs
--- a/servo/components/style/font_metrics.rs
+++ b/servo/components/style/font_metrics.rs
@@ -10,50 +10,40 @@ use Atom;
 use app_units::Au;
 use context::SharedStyleContext;
 use logical_geometry::WritingMode;
 use media_queries::Device;
 use properties::style_structs::Font;
 
 /// Represents the font metrics that style needs from a font to compute the
 /// value of certain CSS units like `ex`.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, Default, PartialEq)]
 pub struct FontMetrics {
     /// The x-height of the font.
-    pub x_height: Au,
+    pub x_height: Option<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(Clone, Debug, PartialEq)]
-pub enum FontMetricsQueryResult {
-    /// The font is available, but we may or may not have found any font metrics
-    /// for it.
-    Available(FontMetrics),
-    /// The font is not available.
-    NotAvailable,
+    pub zero_advance_measure: Option<Au>,
 }
 
 /// A trait used to represent something capable of providing us font metrics.
 pub trait FontMetricsProvider {
     /// 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
+    ) -> FontMetrics {
+        Default::default()
     }
 
     /// 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
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -18,17 +18,17 @@ use CaseSensitivityExt;
 use app_units::Au;
 use applicable_declarations::ApplicableDeclarationBlock;
 use atomic_refcell::{AtomicRefCell, AtomicRefMut};
 use author_styles::AuthorStyles;
 use context::{PostAnimationTasks, QuirksMode, SharedStyleContext, UpdateAnimationsTasks};
 use data::ElementData;
 use dom::{LayoutIterator, NodeInfo, OpaqueNode, TDocument, TElement, TNode, TShadowRoot};
 use element_state::{DocumentState, ElementState};
-use font_metrics::{FontMetrics, FontMetricsProvider, FontMetricsQueryResult};
+use font_metrics::{FontMetrics, FontMetricsProvider};
 use gecko::data::GeckoStyleSheet;
 use gecko::global_style_data::GLOBAL_STYLE_DATA;
 use gecko::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl};
 use gecko::snapshot_helpers;
 use gecko_bindings::bindings;
 use gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetDocumentLWTheme};
 use gecko_bindings::bindings::{Gecko_GetLastChild, Gecko_GetNextStyleChild};
 use gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags};
@@ -1006,33 +1006,36 @@ impl FontMetricsProvider for GeckoFontMe
 
     fn query(
         &self,
         font: &Font,
         font_size: Au,
         wm: WritingMode,
         in_media_query: bool,
         device: &Device,
-    ) -> FontMetricsQueryResult {
+    ) -> FontMetrics {
         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)
+        FontMetrics {
+            x_height: Some(Au(gecko_metrics.mXSize)),
+            zero_advance_measure: if gecko_metrics.mChSize >= 0 {
+                Some(Au(gecko_metrics.mChSize))
+            } else {
+                None
+            },
+        }
     }
 }
 
 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,
--- 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::Size2D;
-use font_metrics::FontMetricsQueryResult;
+use font_metrics::FontMetrics;
 use parser::{Parse, ParserContext};
 use std::cmp;
 use std::ops::{Add, Mul};
 use style_traits::{ParseError, SpecifiedValueInfo, StyleParseErrorKind};
 use style_traits::values::specified::AllowedNumericType;
 use super::{AllowQuirks, Number, Percentage, ToComputedValue};
 use values::{Auto, CSSFloat, Either, Normal};
 use values::computed::{self, CSSPixelLength, Context, ExtremumLength};
@@ -116,17 +116,17 @@ impl FontRelativeLength {
     fn reference_font_size_and_length(
         &self,
         context: &Context,
         base_size: FontBaseSize,
     ) -> (Au, CSSFloat) {
         fn query_font_metrics(
             context: &Context,
             reference_font_size: Au,
-        ) -> FontMetricsQueryResult {
+        ) -> FontMetrics {
             context.font_metrics_provider.query(
                 context.style().get_font(),
                 reference_font_size,
                 context.style().writing_mode,
                 context.in_media_query,
                 context.device(),
             )
         }
@@ -148,52 +148,50 @@ impl FontRelativeLength {
                 } else {
                     (reference_font_size, length)
                 }
             },
             FontRelativeLength::Ex(length) => {
                 if context.for_non_inherited_property.is_some() {
                     context.rule_cache_conditions.borrow_mut().set_uncacheable();
                 }
-                let reference_size = match query_font_metrics(context, reference_font_size) {
-                    FontMetricsQueryResult::Available(metrics) => metrics.x_height,
+                let metrics = query_font_metrics(context, reference_font_size);
+                let reference_size = metrics.x_height.unwrap_or_else(|| {
                     // 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.
                     //
-                    FontMetricsQueryResult::NotAvailable => reference_font_size.scale_by(0.5),
-                };
+                    reference_font_size.scale_by(0.5)
+                });
                 (reference_size, length)
             },
             FontRelativeLength::Ch(length) => {
                 if context.for_non_inherited_property.is_some() {
                     context.rule_cache_conditions.borrow_mut().set_uncacheable();
                 }
-                let reference_size = match query_font_metrics(context, reference_font_size) {
-                    FontMetricsQueryResult::Available(metrics) => metrics.zero_advance_measure,
+                let metrics = query_font_metrics(context, reference_font_size);
+                let reference_size = metrics.zero_advance_measure.unwrap_or_else(|| {
                     // 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).
                     //
-                    FontMetricsQueryResult::NotAvailable => {
-                        if context.style().writing_mode.is_vertical() {
-                            reference_font_size
-                        } else {
-                            reference_font_size.scale_by(0.5)
-                        }
-                    },
-                };
+                    if context.style().writing_mode.is_vertical() {
+                        reference_font_size
+                    } else {
+                        reference_font_size.scale_by(0.5)
+                    }
+                });
                 (reference_size, length)
             },
             FontRelativeLength::Rem(length) => {
                 // https://drafts.csswg.org/css-values/#rem:
                 //
                 //     When specified on the font-size property of the root
                 //     element, the rem units refer to the property's initial
                 //     value.