From 076c3e7246757c5032459edbd8f4561f3c2828e1 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Thu, 21 Mar 2019 04:50:43 +0000 Subject: [PATCH] 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. Depends on D23423 Differential Revision: https://phabricator.services.mozilla.com/D23424 --HG-- extra : moz-landing-system : lando --- servo/components/style/font_metrics.rs | 18 +++-------- servo/components/style/gecko/wrapper.rs | 19 +++++++----- .../style/values/specified/length.rs | 30 +++++++++---------- 3 files changed, 29 insertions(+), 38 deletions(-) diff --git a/servo/components/style/font_metrics.rs b/servo/components/style/font_metrics.rs index c8b4a655ad8e..c1707d1fbc1f 100644 --- a/servo/components/style/font_metrics.rs +++ b/servo/components/style/font_metrics.rs @@ -12,22 +12,12 @@ use app_units::Au; /// 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, /// 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, } /// A trait used to represent something capable of providing us font metrics. @@ -38,7 +28,7 @@ pub trait FontMetricsProvider { _context: &crate::values::computed::Context, _base_size: crate::values::specified::length::FontBaseSize, ) -> FontMetricsQueryResult { - FontMetricsQueryResult::NotAvailable + Default::default() } /// Get default size of a given language and generic family. diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs index 65a2565b3025..8055cd316ea4 100644 --- a/servo/components/style/gecko/wrapper.rs +++ b/servo/components/style/gecko/wrapper.rs @@ -20,7 +20,7 @@ use crate::context::{PostAnimationTasks, QuirksMode, SharedStyleContext, UpdateA use crate::data::ElementData; use crate::dom::{LayoutIterator, NodeInfo, OpaqueNode, TDocument, TElement, TNode, TShadowRoot}; use crate::element_state::{DocumentState, ElementState}; -use crate::font_metrics::{FontMetrics, FontMetricsProvider, FontMetricsQueryResult}; +use crate::font_metrics::{FontMetrics, FontMetricsProvider}; use crate::gecko::data::GeckoStyleSheet; use crate::gecko::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl}; use crate::gecko::snapshot_helpers; @@ -1035,10 +1035,10 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { &self, context: &crate::values::computed::Context, base_size: FontBaseSize, - ) -> FontMetricsQueryResult { + ) -> FontMetrics { let pc = match context.device().pres_context() { Some(pc) => pc, - None => return FontMetricsQueryResult::NotAvailable, + None => return Default::default(), }; let size = base_size.resolve(context); @@ -1066,11 +1066,14 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { !context.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 + }, + } } } diff --git a/servo/components/style/values/specified/length.rs b/servo/components/style/values/specified/length.rs index b3a7932dd934..38cf272c7955 100644 --- a/servo/components/style/values/specified/length.rs +++ b/servo/components/style/values/specified/length.rs @@ -7,7 +7,7 @@ //! [length]: https://drafts.csswg.org/css-values/#lengths use super::{AllowQuirks, Number, Percentage, ToComputedValue}; -use crate::font_metrics::FontMetricsQueryResult; +use crate::font_metrics::FontMetrics; use crate::parser::{Parse, ParserContext}; use crate::properties::computed_value_flags::ComputedValueFlags; use crate::values::computed::{self, CSSPixelLength, Context}; @@ -133,7 +133,7 @@ impl FontRelativeLength { fn query_font_metrics( context: &Context, base_size: FontBaseSize, - ) -> FontMetricsQueryResult { + ) -> FontMetrics { context.font_metrics_provider.query(context, base_size) } @@ -160,16 +160,16 @@ impl FontRelativeLength { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); - let reference_size = match query_font_metrics(context, base_size) { - FontMetricsQueryResult::Available(metrics) => metrics.x_height, + let metrics = query_font_metrics(context, base_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) => { @@ -177,8 +177,8 @@ impl FontRelativeLength { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); - let reference_size = match query_font_metrics(context, base_size) { - FontMetricsQueryResult::Available(metrics) => metrics.zero_advance_measure, + let metrics = query_font_metrics(context, base_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 @@ -189,14 +189,12 @@ impl FontRelativeLength { // 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) => {