From a541046147e5713b0c8fb5ac3c016635515dce18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 15 Dec 2019 20:15:29 +0100 Subject: [PATCH] style: Use less Au in font code. Font code is the only thing that was using Au in the style system without interfacing with Gecko, and there was no real reason for it to do so. This slightly simplifies the code. Differential Revision: https://phabricator.services.mozilla.com/D57248 --- components/style/font_metrics.rs | 10 +++--- components/style/gecko/media_queries.rs | 10 +++--- components/style/gecko/wrapper.rs | 13 +++---- components/style/matching.rs | 2 +- components/style/properties/cascade.rs | 3 +- components/style/properties/gecko.mako.rs | 2 +- components/style/servo/media_queries.rs | 4 +-- components/style/values/computed/font.rs | 8 ++--- components/style/values/computed/length.rs | 2 +- components/style/values/specified/font.rs | 40 ++++++++++----------- components/style/values/specified/gecko.rs | 5 ++- components/style/values/specified/length.rs | 39 +++++++------------- components/style/values/specified/text.rs | 13 ++----- 13 files changed, 62 insertions(+), 89 deletions(-) diff --git a/components/style/font_metrics.rs b/components/style/font_metrics.rs index 8e69118ade4c..5ceb2b0f4cc9 100644 --- a/components/style/font_metrics.rs +++ b/components/style/font_metrics.rs @@ -8,16 +8,16 @@ use crate::context::SharedStyleContext; use crate::Atom; -use app_units::Au; +use crate::values::computed::Length; /// Represents the font metrics that style needs from a font to compute the /// value of certain CSS units like `ex`. #[derive(Clone, Debug, Default, PartialEq)] pub struct FontMetrics { /// The x-height of the font. - pub x_height: Option, + pub x_height: Option, /// The zero advance. This is usually writing mode dependent - pub zero_advance_measure: Option, + pub zero_advance_measure: Option, } /// Type of font metrics to retrieve. @@ -47,7 +47,7 @@ pub trait FontMetricsProvider { &self, font_name: &Atom, font_family: crate::values::computed::font::GenericFontFamily, - ) -> Au; + ) -> Length; /// Construct from a shared style context fn create_from(context: &SharedStyleContext) -> Self @@ -70,7 +70,7 @@ impl FontMetricsProvider for ServoMetricsProvider { ServoMetricsProvider } - fn get_size(&self, _: &Atom, _: crate::values::computed::font::GenericFontFamily) -> Au { + fn get_size(&self, _: &Atom, _: crate::values::computed::font::GenericFontFamily) -> Length { unreachable!("Dummy provider should never be used to compute font size") } } diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 3463c1f95e9f..390c1d4fe586 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -11,10 +11,9 @@ use crate::gecko_bindings::structs; use crate::media_queries::MediaType; use crate::properties::ComputedValues; use crate::string_cache::Atom; -use crate::values::computed::font::FontSize; +use crate::values::specified::font::FONT_MEDIUM_PX; use crate::values::{CustomIdent, KeyframesName}; -use app_units::Au; -use app_units::AU_PER_PX; +use app_units::{Au, AU_PER_PX}; use cssparser::RGBA; use euclid::default::Size2D; use euclid::Scale; @@ -87,7 +86,7 @@ impl Device { document, default_values: ComputedValues::default_values(doc), // FIXME(bz): Seems dubious? - root_font_size: AtomicIsize::new(FontSize::medium().size().0 as isize), + 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), @@ -139,8 +138,7 @@ impl Device { /// Set the font size of the root element (for rem) pub fn set_root_font_size(&self, size: Au) { - self.root_font_size - .store(size.0 as isize, Ordering::Relaxed) + self.root_font_size.store(size.0 as isize, Ordering::Relaxed) } /// Sets the body text color for the "inherit color from body" quirk. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 21fffb975c79..8bf5135d96fa 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -68,6 +68,7 @@ use crate::shared_lock::Locked; use crate::string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; use crate::stylist::CascadeData; use crate::values::computed::font::GenericFontFamily; +use crate::values::computed::Length; use crate::values::specified::length::FontBaseSize; use crate::CaseSensitivityExt; use app_units::Au; @@ -929,7 +930,7 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { GeckoFontMetricsProvider::new() } - fn get_size(&self, font_name: &Atom, font_family: GenericFontFamily) -> Au { + fn get_size(&self, font_name: &Atom, font_family: GenericFontFamily) -> Length { 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); @@ -950,7 +951,7 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { None => return Default::default(), }; - let size = base_size.resolve(context); + let size = Au::from(base_size.resolve(context)); let style = context.style(); let (wm, font) = match base_size { @@ -977,9 +978,9 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { ) }; FontMetrics { - x_height: Some(Au(gecko_metrics.mXSize)), + x_height: Some(Au(gecko_metrics.mXSize).into()), zero_advance_measure: if gecko_metrics.mChSize >= 0 { - Some(Au(gecko_metrics.mChSize)) + Some(Au(gecko_metrics.mChSize).into()) } else { None }, @@ -988,7 +989,7 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { } impl structs::FontSizePrefs { - fn size_for_generic(&self, font_family: GenericFontFamily) -> Au { + fn size_for_generic(&self, font_family: GenericFontFamily) -> Length { Au(match font_family { GenericFontFamily::None => self.mDefaultVariableSize, GenericFontFamily::Serif => self.mDefaultSerifSize, @@ -999,7 +1000,7 @@ impl structs::FontSizePrefs { GenericFontFamily::MozEmoji => unreachable!( "Should never get here, since this doesn't (yet) appear on font family" ), - }) + }).into() } } diff --git a/components/style/matching.rs b/components/style/matching.rs index 01246ed3878a..7ec94d35346d 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -715,7 +715,7 @@ pub trait MatchMethods: TElement { .map_or(true, |s| s.get_font().clone_font_size() != new_font_size) { debug_assert!(self.owner_doc_matches_for_testing(device)); - device.set_root_font_size(new_font_size.size()); + device.set_root_font_size(new_font_size.size().into()); // If the root font-size changed since last time, and something // in the document did use rem units, ensure we recascade the // entire tree. diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index a667cba09bce..dd87916cf7dd 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -743,6 +743,7 @@ 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) { @@ -759,7 +760,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { None => return, }; - if font.gecko().mScriptUnconstrainedSize == new_size.size().0 { + if font.gecko().mScriptUnconstrainedSize == Au::from(new_size.size()).0 { return; } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index b6cd741038c5..8f6e20fd803a 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1134,7 +1134,7 @@ fn static_assert() { pub fn set_font_size(&mut self, v: FontSize) { use crate::values::specified::font::KeywordSize; - let size = v.size(); + let size = Au::from(v.size()); self.gecko.mScriptUnconstrainedSize = size.0; // These two may be changed from Cascade::fixup_font_stuff. diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index df7bf93b8acc..be842dc0f9a1 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -10,9 +10,9 @@ use crate::media_queries::media_feature::{Evaluator, MediaFeatureDescription}; use crate::media_queries::media_feature_expression::RangeOrOperator; use crate::media_queries::MediaType; use crate::properties::ComputedValues; -use crate::values::computed::font::FontSize; use crate::values::computed::CSSPixelLength; use crate::values::KeyframesName; +use crate::values::specified::font::FONT_MEDIUM_PX; use app_units::Au; use cssparser::RGBA; use euclid::default::Size2D as UntypedSize2D; @@ -68,7 +68,7 @@ impl Device { viewport_size, device_pixel_ratio, // FIXME(bz): Seems dubious? - root_font_size: AtomicIsize::new(FontSize::medium().size().0 as isize), + root_font_size: AtomicIsize::new(Au::from_px(FONT_MEDIUM_PX).0 as isize), used_root_font_size: AtomicBool::new(false), used_viewport_units: AtomicBool::new(false), environment: CssEnvironment, diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index a4fa60132779..658d34124755 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -21,7 +21,6 @@ use crate::values::specified::font::{ use crate::values::specified::length::{FontBaseSize, NoCalcLength}; use crate::values::CSSFloat; use crate::Atom; -use app_units::Au; use byteorder::{BigEndian, ByteOrder}; use cssparser::{serialize_identifier, CssStringWriter, Parser}; #[cfg(feature = "gecko")] @@ -148,15 +147,16 @@ impl FontWeight { impl FontSize { /// The actual computed font size. - pub fn size(self) -> Au { - self.size.into() + #[inline] + pub fn size(&self) -> Length { + self.size.0 } #[inline] /// Get default value of font size. pub fn medium() -> Self { Self { - size: Au::from_px(specified::FONT_MEDIUM_PX).into(), + size: NonNegative(Length::new(specified::FONT_MEDIUM_PX as CSSFloat)), keyword_info: Some(KeywordInfo::medium()), } } diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 2423d447a6a3..cefcf213754d 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -29,7 +29,7 @@ pub use crate::values::specified::url::UrlOrNone; pub use crate::values::specified::{Angle, BorderStyle, Time}; impl ToComputedValue for specified::NoCalcLength { - type ComputedValue = CSSPixelLength; + type ComputedValue = Length; #[inline] fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index c09d5a2c34c1..7ed6429c1963 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -20,7 +20,6 @@ use crate::values::specified::{AllowQuirks, Angle, Integer, LengthPercentage}; use crate::values::specified::{NoCalcLength, NonNegativeNumber, Number, Percentage}; use crate::values::CustomIdent; use crate::Atom; -use app_units::Au; use byteorder::{BigEndian, ByteOrder}; use cssparser::{Parser, Token}; #[cfg(feature = "gecko")] @@ -773,18 +772,18 @@ impl ToComputedValue for KeywordSize { type ComputedValue = NonNegativeLength; #[inline] fn to_computed_value(&self, _: &Context) -> NonNegativeLength { + let medium = Length::new(FONT_MEDIUM_PX as f32); // https://drafts.csswg.org/css-fonts-3/#font-size-prop - match *self { - KeywordSize::XXSmall => Au::from_px(FONT_MEDIUM_PX) * 3 / 5, - KeywordSize::XSmall => Au::from_px(FONT_MEDIUM_PX) * 3 / 4, - KeywordSize::Small => Au::from_px(FONT_MEDIUM_PX) * 8 / 9, - KeywordSize::Medium => Au::from_px(FONT_MEDIUM_PX), - KeywordSize::Large => Au::from_px(FONT_MEDIUM_PX) * 6 / 5, - KeywordSize::XLarge => Au::from_px(FONT_MEDIUM_PX) * 3 / 2, - KeywordSize::XXLarge => Au::from_px(FONT_MEDIUM_PX) * 2, - KeywordSize::XXXLarge => Au::from_px(FONT_MEDIUM_PX) * 3, - } - .into() + NonNegative(match *self { + KeywordSize::XXSmall => medium * 3.0 / 5.0, + KeywordSize::XSmall => medium * 3.0 / 4.0, + KeywordSize::Small => medium * 8.0 / 9.0, + KeywordSize::Medium => medium, + KeywordSize::Large => medium * 6.0 / 5.0, + KeywordSize::XLarge => medium * 3.0 / 2.0, + KeywordSize::XXLarge => medium * 2.0, + KeywordSize::XXXLarge => medium * 3.0, + }) } #[inline] @@ -799,7 +798,6 @@ impl ToComputedValue for KeywordSize { #[inline] fn to_computed_value(&self, cx: &Context) -> NonNegativeLength { use crate::context::QuirksMode; - use crate::values::specified::length::au_to_int_px; // The tables in this function are originally from // nsRuleNode::CalcFontPointSize in Gecko: @@ -850,22 +848,21 @@ impl ToComputedValue for KeywordSize { Atom::with(gecko_font.mLanguage.mRawPtr, |atom| { cx.font_metrics_provider .get_size(atom, gecko_font.mGenericID) - .0 }) }; - let base_size_px = au_to_int_px(base_size as f32); + let base_size_px = base_size.px().round() as i32; let html_size = self.html_size() as usize; - if base_size_px >= 9 && base_size_px <= 16 { + NonNegative(if base_size_px >= 9 && base_size_px <= 16 { let mapping = if cx.quirks_mode == QuirksMode::Quirks { QUIRKS_FONT_SIZE_MAPPING } else { FONT_SIZE_MAPPING }; - Au::from_px(mapping[(base_size_px - 9) as usize][html_size]).into() + Length::new(mapping[(base_size_px - 9) as usize][html_size] as f32) } else { - Au(FONT_SIZE_FACTORS[html_size] * base_size / 100).into() - } + base_size * FONT_SIZE_FACTORS[html_size] as f32 / 100.0 + }) } #[inline] @@ -927,7 +924,7 @@ impl FontSize { // If the parent font was keyword-derived, this is too. // Tack the % onto the factor info = compose_keyword(pc.0); - base_size.resolve(context).scale_by(pc.0).into() + base_size.resolve(context) * pc.0 }, FontSize::Length(LengthPercentage::Calc(ref calc)) => { let parent = context.style().get_parent_font().clone_font_size(); @@ -964,8 +961,7 @@ impl FontSize { // others should reject negatives during parsing. But SMIL // allows parsing negatives, and relies on us _not_ doing that // clamping. That's so bonkers :( - CSSPixelLength::from(calc.to_used_value(base_size.resolve(context))) - .clamp_to_non_negative() + calc.percentage_relative_to(base_size.resolve(context)).clamp_to_non_negative() }, FontSize::Keyword(i) => { // As a specified keyword, this is keyword derived diff --git a/components/style/values/specified/gecko.rs b/components/style/values/specified/gecko.rs index 131c2a1a3149..bade419d71a1 100644 --- a/components/style/values/specified/gecko.rs +++ b/components/style/values/specified/gecko.rs @@ -5,8 +5,7 @@ //! Specified types for legacy Gecko-only properties. use crate::parser::{Parse, ParserContext}; -use crate::values::computed::length::CSSPixelLength; -use crate::values::computed::{self, LengthPercentage}; +use crate::values::computed::{self, LengthPercentage, Length}; use crate::values::generics::rect::Rect; use cssparser::{Parser, Token}; use std::fmt; @@ -24,7 +23,7 @@ fn parse_pixel_or_percent<'i, 't>( value, ref unit, .. } => { match_ignore_ascii_case! { unit, - "px" => Ok(LengthPercentage::new(CSSPixelLength::new(value), None)), + "px" => Ok(LengthPercentage::new(Length::new(value), None)), _ => Err(()), } }, diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index ebf7746ee117..f1f4b0eeec83 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -47,14 +47,6 @@ pub const AU_PER_PT: CSSFloat = AU_PER_IN / 72.; /// Number of app units per pica pub const AU_PER_PC: CSSFloat = AU_PER_PT * 12.; -/// Same as Gecko's AppUnitsToIntCSSPixels -/// -/// Converts app units to integer pixel values, -/// rounding during the conversion -pub fn au_to_int_px(au: f32) -> i32 { - (au / AU_PER_PX).round() as i32 -} - /// A font relative length. #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToCss, ToShmem)] pub enum FontRelativeLength { @@ -87,7 +79,7 @@ pub enum FontBaseSize { impl FontBaseSize { /// Calculate the actual size for a given context - pub fn resolve(&self, context: &Context) -> Au { + pub fn resolve(&self, context: &Context) -> computed::Length { match *self { FontBaseSize::CurrentStyle => context.style().get_font().clone_font_size().size(), FontBaseSize::InheritedStyleButStripEmUnits | FontBaseSize::InheritedStyle => { @@ -109,13 +101,9 @@ impl FontRelativeLength { } /// Computes the font-relative length. - pub fn to_computed_value(&self, context: &Context, base_size: FontBaseSize) -> CSSPixelLength { - use std::f32; + pub fn to_computed_value(&self, context: &Context, base_size: FontBaseSize) -> computed::Length { let (reference_size, length) = self.reference_font_size_and_length(context, base_size); - let pixel = (length * reference_size.to_f32_px()) - .min(f32::MAX) - .max(f32::MIN); - CSSPixelLength::new(pixel) + reference_size * length } /// Return reference font size. @@ -129,7 +117,7 @@ impl FontRelativeLength { &self, context: &Context, base_size: FontBaseSize, - ) -> (Au, CSSFloat) { + ) -> (computed::Length, CSSFloat) { fn query_font_metrics( context: &Context, base_size: FontBaseSize, @@ -153,7 +141,7 @@ impl FontRelativeLength { } if base_size == FontBaseSize::InheritedStyleButStripEmUnits { - (Au(0), length) + (Zero::zero(), length) } else { (reference_font_size, length) } @@ -175,7 +163,7 @@ impl FontRelativeLength { // determine the x-height, a value of 0.5em must be // assumed. // - reference_font_size.scale_by(0.5) + reference_font_size * 0.5 }); (reference_size, length) }, @@ -210,7 +198,7 @@ impl FontRelativeLength { if wm.is_vertical() && wm.is_upright() { reference_font_size } else { - reference_font_size.scale_by(0.5) + reference_font_size * 0.5 } }); (reference_size, length) @@ -225,7 +213,7 @@ impl FontRelativeLength { let reference_size = if context.is_root_element || context.in_media_query { reference_font_size } else { - context.device().root_font_size() + computed::Length::new(context.device().root_font_size().to_f32_px()) }; (reference_size, length) }, @@ -290,15 +278,14 @@ pub struct CharacterWidth(pub i32); impl CharacterWidth { /// Computes the given character width. - pub fn to_computed_value(&self, reference_font_size: Au) -> CSSPixelLength { - // This applies the *converting a character width to pixels* algorithm as specified - // in HTML5 § 14.5.4. + pub fn to_computed_value(&self, reference_font_size: computed::Length) -> computed::Length { + // This applies the *converting a character width to pixels* algorithm + // as specified in HTML5 § 14.5.4. // // TODO(pcwalton): Find these from the font. - let average_advance = reference_font_size.scale_by(0.5); + let average_advance = reference_font_size * 0.5; let max_advance = reference_font_size; - let au = average_advance.scale_by(self.0 as CSSFloat - 1.0) + max_advance; - au.into() + average_advance * (self.0 as CSSFloat - 1.0) + max_advance } } diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 860a7f5caf83..8c50a61ef945 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -77,7 +77,6 @@ impl ToComputedValue for LineHeight { #[inline] fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { - use crate::values::computed::Length as ComputedLength; use crate::values::specified::length::FontBaseSize; match *self { GenericLineHeight::Normal => GenericLineHeight::Normal, @@ -97,16 +96,8 @@ impl ToComputedValue for LineHeight { LengthPercentage::Calc(ref calc) => { let computed_calc = calc.to_computed_value_zoomed(context, FontBaseSize::CurrentStyle); - let font_relative_length = - FontRelativeLength::Em(computed_calc.percentage()) - .to_computed_value(context, FontBaseSize::CurrentStyle) - .px(); - - let absolute_length = computed_calc.unclamped_length().px(); - let pixel = computed_calc - .clamping_mode - .clamp(absolute_length + font_relative_length); - ComputedLength::new(pixel) + let base = context.style().get_font().clone_font_size().size(); + computed_calc.percentage_relative_to(base) }, }; GenericLineHeight::Length(result.into())