From d3f254d2e4fc6e516fe267e040db1ebfeea621b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 18 Mar 2019 16:27:54 +0000 Subject: [PATCH] style: Make all font-metrics-affecting properties cascade early. And make font-size computation work on the whole font of the parent, not just accounting for the parent's font-size. Differential Revision: https://phabricator.services.mozilla.com/D20656 --- components/style/font_metrics.rs | 16 ++------ components/style/gecko/wrapper.rs | 38 ++++++++++++------- .../style/properties/properties.mako.rs | 6 ++- components/style/values/specified/length.rs | 14 ++----- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/components/style/font_metrics.rs b/components/style/font_metrics.rs index 0c37a444658..c8b4a655ad8 100644 --- a/components/style/font_metrics.rs +++ b/components/style/font_metrics.rs @@ -7,9 +7,6 @@ #![deny(missing_docs)] use crate::context::SharedStyleContext; -use crate::logical_geometry::WritingMode; -use crate::media_queries::Device; -use crate::properties::style_structs::Font; use crate::Atom; use app_units::Au; @@ -36,22 +33,15 @@ pub enum FontMetricsQueryResult { /// 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, + _context: &crate::values::computed::Context, + _base_size: crate::values::specified::length::FontBaseSize, ) -> FontMetricsQueryResult { FontMetricsQueryResult::NotAvailable } - /// Get default size of a given language and generic family + /// 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 diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 558ee60f4e2..65a2565b302 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -58,10 +58,8 @@ use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI}; use crate::global_style_data::GLOBAL_STYLE_DATA; use crate::hash::FxHashMap; use crate::invalidation::element::restyle_hints::RestyleHint; -use crate::logical_geometry::WritingMode; use crate::media_queries::Device; use crate::properties::animated_properties::{AnimationValue, AnimationValueMap}; -use crate::properties::style_structs::Font; use crate::properties::{ComputedValues, LonghandId}; use crate::properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; use crate::rule_tree::CascadeLevel as ServoCascadeLevel; @@ -69,6 +67,7 @@ use crate::selector_parser::{AttrValue, HorizontalDirection, Lang}; use crate::shared_lock::Locked; use crate::string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; use crate::stylist::CascadeData; +use crate::values::specified::length::FontBaseSize; use crate::CaseSensitivityExt; use app_units::Au; use atomic_refcell::{AtomicRefCell, AtomicRefMut}; @@ -1023,37 +1022,48 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { } fn get_size(&self, font_name: &Atom, font_family: u8) -> Au { - use crate::gecko_bindings::bindings::Gecko_GetBaseSize; let mut cache = self.font_size_cache.borrow_mut(); if let Some(sizes) = cache.iter().find(|el| el.0 == *font_name) { return sizes.1.size_for_generic(font_family); } - let sizes = unsafe { Gecko_GetBaseSize(font_name.as_ptr()) }; + let sizes = unsafe { bindings::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, + context: &crate::values::computed::Context, + base_size: FontBaseSize, ) -> FontMetricsQueryResult { - use crate::gecko_bindings::bindings::Gecko_GetFontMetrics; - let pc = match device.pres_context() { + let pc = match context.device().pres_context() { Some(pc) => pc, None => return FontMetricsQueryResult::NotAvailable, }; + + let size = base_size.resolve(context); + let style = context.style(); + + let (wm, font) = match base_size { + FontBaseSize::CurrentStyle => { + (style.writing_mode, style.get_font()) + }, + // These are only used for font-size computation, and the first is + // really dubious... + FontBaseSize::InheritedStyleButStripEmUnits | + FontBaseSize::InheritedStyle => { + (*style.inherited_writing_mode(), style.get_parent_font()) + }, + }; + let gecko_metrics = unsafe { - Gecko_GetFontMetrics( + bindings::Gecko_GetFontMetrics( pc, wm.is_vertical() && !wm.is_sideways(), font.gecko(), - font_size.0, + size.0, // we don't use the user font set in a media query - !in_media_query, + !context.in_media_query, ) }; let metrics = FontMetrics { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 4df99668d18..40a5456c5e4 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1257,8 +1257,12 @@ impl LonghandId { LonghandId::MozScriptLevel | % endif - // Needed to compute font-relative lengths correctly. + // Needed to compute the first available font, in order to + // compute font-relative units correctly. LonghandId::FontSize | + LonghandId::FontWeight | + LonghandId::FontStretch | + LonghandId::FontStyle | LonghandId::FontFamily | // Needed to resolve currentcolor at computed value time properly. diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 8888a32c4a5..9a1d366f7fd 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -131,15 +131,9 @@ impl FontRelativeLength { ) -> (Au, CSSFloat) { fn query_font_metrics( context: &Context, - reference_font_size: Au, + base_size: FontBaseSize, ) -> FontMetricsQueryResult { - context.font_metrics_provider.query( - context.style().get_font(), - reference_font_size, - context.style().writing_mode, - context.in_media_query, - context.device(), - ) + context.font_metrics_provider.query(context, base_size) } let reference_font_size = base_size.resolve(context); @@ -164,7 +158,7 @@ impl FontRelativeLength { 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) { + let reference_size = match query_font_metrics(context, base_size) { FontMetricsQueryResult::Available(metrics) => metrics.x_height, // https://drafts.csswg.org/css-values/#ex // @@ -180,7 +174,7 @@ impl FontRelativeLength { 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) { + let reference_size = match query_font_metrics(context, base_size) { FontMetricsQueryResult::Available(metrics) => metrics.zero_advance_measure, // https://drafts.csswg.org/css-values/#ch //