From 234d2c1b3274faf830d3e33bf4b8c98522fe962c Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Fri, 21 Jul 2017 15:38:55 +0800 Subject: [PATCH] Bug 1374233 - Part 3: Use NonNegativeAu as computed values for font-size related properties. For font-size and font-size-adjust. MozReview-Commit-ID: 5rrfVSzB7WF --- components/gfx/font_context.rs | 10 +-- components/layout/multicol.rs | 2 +- components/layout/text.rs | 2 +- components/style/gecko/media_queries.rs | 3 +- components/style/matching.rs | 2 +- components/style/properties/gecko.mako.rs | 24 +++---- .../style/properties/longhand/font.mako.rs | 66 ++++++++++++------- .../style/properties/properties.mako.rs | 9 +-- components/style/servo/media_queries.rs | 3 +- components/style/values/computed/length.rs | 4 +- components/style/values/computed/mod.rs | 13 +++- components/style/values/specified/length.rs | 4 +- components/style/values/specified/text.rs | 2 +- 13 files changed, 87 insertions(+), 57 deletions(-) diff --git a/components/gfx/font_context.rs b/components/gfx/font_context.rs index 289939ee9d5..67214cf5e40 100644 --- a/components/gfx/font_context.rs +++ b/components/gfx/font_context.rs @@ -116,7 +116,7 @@ impl FontContext { let layout_font_group_cache_key = LayoutFontGroupCacheKey { pointer: style.clone(), - size: style.font_size, + size: style.font_size.0, }; if let Some(ref cached_font_group) = self.layout_font_group_cache.get( &layout_font_group_cache_key) { @@ -146,7 +146,7 @@ impl FontContext { Some(ref cached_font_ref) => { let cached_font = (*cached_font_ref).borrow(); if cached_font.descriptor == desc && - cached_font.requested_pt_size == style.font_size && + cached_font.requested_pt_size == style.font_size.0 && cached_font.variant == style.font_variant_caps { fonts.push((*cached_font_ref).clone()); cache_hit = true; @@ -164,7 +164,7 @@ impl FontContext { Some(template_info) => { let layout_font = self.create_layout_font(template_info.font_template, desc.clone(), - style.font_size, + style.font_size.0, style.font_variant_caps, template_info.font_key .expect("No font key present!")); @@ -198,7 +198,7 @@ impl FontContext { for cached_font_entry in &self.fallback_font_cache { let cached_font = cached_font_entry.font.borrow(); if cached_font.descriptor == desc && - cached_font.requested_pt_size == style.font_size && + cached_font.requested_pt_size == style.font_size.0 && cached_font.variant == style.font_variant_caps { fonts.push(cached_font_entry.font.clone()); cache_hit = true; @@ -210,7 +210,7 @@ impl FontContext { let template_info = self.font_cache_thread.last_resort_font_template(desc.clone()); let layout_font = self.create_layout_font(template_info.font_template, desc.clone(), - style.font_size, + style.font_size.0, style.font_variant_caps, template_info.font_key.expect("No font key present!")); match layout_font { diff --git a/components/layout/multicol.rs b/components/layout/multicol.rs index 3321faae926..cdc6b2de08a 100644 --- a/components/layout/multicol.rs +++ b/components/layout/multicol.rs @@ -99,7 +99,7 @@ impl Flow for MulticolFlow { let column_gap = match column_style.column_gap { Either::First(len) => len, - Either::Second(_normal) => self.block_flow.fragment.style.get_font().font_size, + Either::Second(_normal) => self.block_flow.fragment.style.get_font().font_size.0, }; let mut column_count; diff --git a/components/layout/text.rs b/components/layout/text.rs index 6e1c7b2a43a..da779ecb240 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -446,7 +446,7 @@ pub fn font_metrics_for_style(font_context: &mut FontContext, font_style: ::Serv /// Returns the line block-size needed by the given computed style and font size. pub fn line_height_from_style(style: &ComputedValues, metrics: &FontMetrics) -> Au { - let font_size = style.get_font().font_size; + let font_size = style.get_font().font_size.0; match style.get_inheritedtext().line_height { LineHeight::Normal => metrics.line_gap, LineHeight::Number(l) => font_size.scale_by(l), diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index babefa95ebb..a7ba27d8414 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -69,7 +69,8 @@ impl Device { Device { pres_context: pres_context, default_values: ComputedValues::default_values(unsafe { &*pres_context }), - root_font_size: AtomicIsize::new(font_size::get_initial_value().0 as isize), // FIXME(bz): Seems dubious? + // FIXME(bz): Seems dubious? + root_font_size: AtomicIsize::new(font_size::get_initial_value().value() as isize), used_root_font_size: AtomicBool::new(false), used_viewport_size: AtomicBool::new(false), } diff --git a/components/style/matching.rs b/components/style/matching.rs index 72e110ce1a2..d16dee6f3a1 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -534,7 +534,7 @@ pub trait MatchMethods : TElement { if old_styles.primary.as_ref().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); + device.set_root_font_size(new_font_size.0); // 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/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 0085da6c518..c8c2c0806f3 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -62,7 +62,7 @@ use std::mem::{forget, uninitialized, transmute, zeroed}; use std::{cmp, ops, ptr}; use stylesheets::{MallocSizeOfWithRepeats, SizeOfState}; use values::{self, Auto, CustomIdent, Either, KeyframesName}; -use values::computed::ToComputedValue; +use values::computed::{NonNegativeAu, ToComputedValue}; use values::computed::effects::{BoxShadow, Filter, SimpleShadow}; use values::computed::length::Percentage; use computed_values::border_style; @@ -2259,15 +2259,15 @@ fn static_assert() { } pub fn set_font_size(&mut self, v: longhands::font_size::computed_value::T) { - self.gecko.mSize = v.0; - self.gecko.mScriptUnconstrainedSize = v.0; + self.gecko.mSize = v.value(); + self.gecko.mScriptUnconstrainedSize = v.value(); } /// Set font size, taking into account scriptminsize and scriptlevel /// Returns Some(size) if we have to recompute the script unconstrained size pub fn apply_font_size(&mut self, v: longhands::font_size::computed_value::T, parent: &Self, - device: &Device) -> Option { + device: &Device) -> Option { let (adjusted_size, adjusted_unconstrained_size) = self.calculate_script_level_size(parent, device); // In this case, we have been unaffected by scriptminsize, ignore it @@ -2277,9 +2277,9 @@ fn static_assert() { self.fixup_font_min_size(device); None } else { - self.gecko.mSize = v.0; + self.gecko.mSize = v.value(); self.fixup_font_min_size(device); - Some(Au(parent.gecko.mScriptUnconstrainedSize)) + Some(Au(parent.gecko.mScriptUnconstrainedSize).into()) } } @@ -2287,8 +2287,8 @@ fn static_assert() { unsafe { bindings::Gecko_nsStyleFont_FixupMinFontSize(&mut self.gecko, device.pres_context()) } } - pub fn apply_unconstrained_font_size(&mut self, v: Au) { - self.gecko.mScriptUnconstrainedSize = v.0; + pub fn apply_unconstrained_font_size(&mut self, v: NonNegativeAu) { + self.gecko.mScriptUnconstrainedSize = v.value(); } /// Calculates the constrained and unconstrained font sizes to be inherited @@ -2398,7 +2398,7 @@ fn static_assert() { /// /// Returns true if the inherited keyword size was actually used pub fn inherit_font_size_from(&mut self, parent: &Self, - kw_inherited_size: Option, + kw_inherited_size: Option, device: &Device) -> bool { let (adjusted_size, adjusted_unconstrained_size) = self.calculate_script_level_size(parent, device); @@ -2424,9 +2424,9 @@ fn static_assert() { false } else if let Some(size) = kw_inherited_size { // Parent element was a keyword-derived size. - self.gecko.mSize = size.0; + self.gecko.mSize = size.value(); // MathML constraints didn't apply here, so we can ignore this. - self.gecko.mScriptUnconstrainedSize = size.0; + self.gecko.mScriptUnconstrainedSize = size.value(); self.fixup_font_min_size(device); true } else { @@ -2440,7 +2440,7 @@ fn static_assert() { } pub fn clone_font_size(&self) -> longhands::font_size::computed_value::T { - Au(self.gecko.mSize) + Au(self.gecko.mSize).into() } pub fn set_font_weight(&mut self, v: longhands::font_weight::computed_value::T) { diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs index cbf6df1f44e..6e39540194c 100644 --- a/components/style/properties/longhand/font.mako.rs +++ b/components/style/properties/longhand/font.mako.rs @@ -566,7 +566,7 @@ ${helpers.single_keyword_system("font-variant-caps", } -<%helpers:longhand name="font-size" need_clone="True" animation_value_type="ComputedValue" +<%helpers:longhand name="font-size" need_clone="True" animation_value_type="NonNegativeAu" flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER" allow_quirks="True" spec="https://drafts.csswg.org/css-fonts/#propdef-font-size"> use app_units::Au; @@ -574,6 +574,7 @@ ${helpers.single_keyword_system("font-variant-caps", use std::fmt; use style_traits::{HasViewportPercentage, ToCss}; use values::FONT_MEDIUM_PX; + use values::computed::NonNegativeAu; use values::specified::{AllowQuirks, FontRelativeLength, LengthOrPercentage, NoCalcLength}; use values::specified::length::FontBaseSize; @@ -621,8 +622,8 @@ ${helpers.single_keyword_system("font-variant-caps", } pub mod computed_value { - use app_units::Au; - pub type T = Au; + use values::computed::NonNegativeAu; + pub type T = NonNegativeAu; } /// CSS font keywords @@ -697,7 +698,7 @@ ${helpers.single_keyword_system("font-variant-caps", % if product == "servo": impl ToComputedValue for KeywordSize { - type ComputedValue = Au; + type ComputedValue = NonNegativeAu; #[inline] fn to_computed_value(&self, _: &Context) -> computed_value::T { // https://drafts.csswg.org/css-fonts-3/#font-size-prop @@ -711,7 +712,7 @@ ${helpers.single_keyword_system("font-variant-caps", XLarge => Au::from_px(FONT_MEDIUM_PX) * 3 / 2, XXLarge => Au::from_px(FONT_MEDIUM_PX) * 2, XXXLarge => Au::from_px(FONT_MEDIUM_PX) * 3, - } + }.into() } #[inline] @@ -721,7 +722,7 @@ ${helpers.single_keyword_system("font-variant-caps", } % else: impl ToComputedValue for KeywordSize { - type ComputedValue = Au; + type ComputedValue = NonNegativeAu; #[inline] fn to_computed_value(&self, cx: &Context) -> computed_value::T { use gecko_bindings::structs::nsIAtom; @@ -757,9 +758,9 @@ ${helpers.single_keyword_system("font-variant-caps", let base_size_px = au_to_int_px(base_size as f32); let html_size = self.html_size() as usize; if base_size_px >= 9 && base_size_px <= 16 { - Au::from_px(FONT_SIZE_MAPPING[(base_size_px - 9) as usize][html_size]) + NonNegativeAu::from_px(FONT_SIZE_MAPPING[(base_size_px - 9) as usize][html_size]) } else { - Au(FONT_SIZE_FACTORS[html_size] * base_size / 100) + Au(FONT_SIZE_FACTORS[html_size] * base_size / 100).into() } } @@ -818,37 +819,38 @@ ${helpers.single_keyword_system("font-variant-caps", } /// Compute it against a given base font size - pub fn to_computed_value_against(&self, context: &Context, base_size: FontBaseSize) -> Au { + pub fn to_computed_value_against(&self, context: &Context, base_size: FontBaseSize) + -> NonNegativeAu { use values::specified::length::FontRelativeLength; match *self { SpecifiedValue::Length(LengthOrPercentage::Length( NoCalcLength::FontRelative(value))) => { - value.to_computed_value(context, base_size) + value.to_computed_value(context, base_size).into() } SpecifiedValue::Length(LengthOrPercentage::Length( NoCalcLength::ServoCharacterWidth(value))) => { - value.to_computed_value(base_size.resolve(context)) + value.to_computed_value(base_size.resolve(context)).into() } SpecifiedValue::Length(LengthOrPercentage::Length(ref l)) => { - context.maybe_zoom_text(l.to_computed_value(context)) + context.maybe_zoom_text(l.to_computed_value(context).into()) } SpecifiedValue::Length(LengthOrPercentage::Percentage(pc)) => { - base_size.resolve(context).scale_by(pc.0) + base_size.resolve(context).scale_by(pc.0).into() } SpecifiedValue::Length(LengthOrPercentage::Calc(ref calc)) => { let calc = calc.to_computed_value_zoomed(context); - calc.to_used_value(Some(base_size.resolve(context))).unwrap() + calc.to_used_value(Some(base_size.resolve(context))).unwrap().into() } SpecifiedValue::Keyword(ref key, fraction) => { context.maybe_zoom_text(key.to_computed_value(context).scale_by(fraction)) } SpecifiedValue::Smaller => { FontRelativeLength::Em(1. / LARGER_FONT_SIZE_RATIO) - .to_computed_value(context, base_size) + .to_computed_value(context, base_size).into() } SpecifiedValue::Larger => { FontRelativeLength::Em(LARGER_FONT_SIZE_RATIO) - .to_computed_value(context, base_size) + .to_computed_value(context, base_size).into() } SpecifiedValue::System(_) => { @@ -863,7 +865,7 @@ ${helpers.single_keyword_system("font-variant-caps", #[inline] #[allow(missing_docs)] pub fn get_initial_value() -> computed_value::T { - Au::from_px(FONT_MEDIUM_PX) + NonNegativeAu::from_px(FONT_MEDIUM_PX) } #[inline] @@ -883,7 +885,7 @@ ${helpers.single_keyword_system("font-variant-caps", #[inline] fn from_computed_value(computed: &computed_value::T) -> Self { SpecifiedValue::Length(LengthOrPercentage::Length( - ToComputedValue::from_computed_value(computed) + ToComputedValue::from_computed_value(&computed.0) )) } } @@ -930,7 +932,7 @@ ${helpers.single_keyword_system("font-variant-caps", #[allow(unused_mut)] pub fn cascade_specified_font_size(context: &mut Context, specified_value: &SpecifiedValue, - mut computed: Au) { + mut computed: NonNegativeAu) { if let SpecifiedValue::Keyword(kw, fraction) = *specified_value { context.builder.font_size_keyword = Some((kw, fraction)); } else if let Some(ratio) = specified_value.as_font_ratio() { @@ -976,7 +978,7 @@ ${helpers.single_keyword_system("font-variant-caps", if let Some(parent) = parent_unconstrained { let new_unconstrained = specified_value - .to_computed_value_against(context, FontBaseSize::Custom(parent)); + .to_computed_value_against(context, FontBaseSize::Custom(parent.0)); context.builder .mutate_font() .apply_unconstrained_font_size(new_unconstrained); @@ -1028,7 +1030,8 @@ ${helpers.single_keyword_system("font-variant-caps", } -<%helpers:longhand products="gecko" name="font-size-adjust" animation_value_type="ComputedValue" +<%helpers:longhand products="gecko" name="font-size-adjust" + animation_value_type="longhands::font_size_adjust::computed_value::T" flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER" spec="https://drafts.csswg.org/css-fonts/#propdef-font-size-adjust"> use properties::longhands::system_font::SystemFont; @@ -1082,7 +1085,7 @@ ${helpers.single_keyword_system("font-variant-caps", pub mod computed_value { use properties::animated_properties::Animatable; use values::CSSFloat; - use values::animated::ToAnimatedZero; + use values::animated::{ToAnimatedValue, ToAnimatedZero}; #[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[derive(Copy, Clone, Debug, PartialEq, ToCss)] @@ -1125,6 +1128,23 @@ ${helpers.single_keyword_system("font-variant-caps", #[inline] fn to_animated_zero(&self) -> Result { Err(()) } } + + impl ToAnimatedValue for T { + type AnimatedValue = Self; + + #[inline] + fn to_animated_value(self) -> Self { + self + } + + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + match animated { + T::Number(number) => T::Number(number.max(0.)), + _ => animated + } + } + } } #[inline] @@ -2514,7 +2534,7 @@ ${helpers.single_keyword("-moz-math-variant", let weight = longhands::font_weight::computed_value::T::from_gecko_weight(system.weight); let ret = ComputedSystemFont { font_family: longhands::font_family::computed_value::T(family), - font_size: Au(system.size), + font_size: Au(system.size).into(), font_weight: weight, font_size_adjust: longhands::font_size_adjust::computed_value ::T::from_gecko_adjust(system.sizeAdjust), diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index e46a659ee97..7973e7f2c0f 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1648,12 +1648,12 @@ pub use gecko_properties::style_structs; /// The module where all the style structs are defined. #[cfg(feature = "servo")] pub mod style_structs { - use app_units::Au; use fnv::FnvHasher; use super::longhands; use std::hash::{Hash, Hasher}; use logical_geometry::WritingMode; use media_queries::Device; + use values::computed::NonNegativeAu; % for style_struct in data.active_style_structs(): % if style_struct.name == "Font": @@ -1771,7 +1771,8 @@ pub mod style_structs { /// (Servo does not handle MathML, so this just calls copy_font_size_from) pub fn inherit_font_size_from(&mut self, parent: &Self, - _: Option, _: &Device) -> bool { + _: Option, + _: &Device) -> bool { self.copy_font_size_from(parent); false } @@ -1779,12 +1780,12 @@ pub mod style_structs { pub fn apply_font_size(&mut self, v: longhands::font_size::computed_value::T, _: &Self, - _: &Device) -> Option { + _: &Device) -> Option { self.set_font_size(v); None } /// (Servo does not handle MathML, so this does nothing) - pub fn apply_unconstrained_font_size(&mut self, _: Au) { + pub fn apply_unconstrained_font_size(&mut self, _: NonNegativeAu) { } % elif style_struct.name == "Outline": diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index 6904051af18..9b47d231c97 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -60,7 +60,8 @@ impl Device { media_type: media_type, viewport_size: viewport_size, device_pixel_ratio: device_pixel_ratio, - root_font_size: AtomicIsize::new(font_size::get_initial_value().0 as isize), // FIXME(bz): Seems dubious? + // FIXME(bz): Seems dubious? + root_font_size: AtomicIsize::new(font_size::get_initial_value().value() as isize), used_root_font_size: AtomicBool::new(false), } } diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 29117a5cb8d..b17606036ed 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -71,7 +71,7 @@ impl ToComputedValue for specified::NoCalcLength { specified::NoCalcLength::ViewportPercentage(length) => length.to_computed_value(context.viewport_size()), specified::NoCalcLength::ServoCharacterWidth(length) => - length.to_computed_value(context.style().get_font().clone_font_size()), + length.to_computed_value(context.style().get_font().clone_font_size().0), #[cfg(feature = "gecko")] specified::NoCalcLength::Physical(length) => length.to_computed_value(context), @@ -265,7 +265,7 @@ impl specified::CalcLengthOrPercentage { /// Compute font-size or line-height taking into account text-zoom if necessary. pub fn to_computed_value_zoomed(&self, context: &Context) -> CalcLengthOrPercentage { - self.to_computed_value_with_zoom(context, |abs| context.maybe_zoom_text(abs)) + self.to_computed_value_with_zoom(context, |abs| context.maybe_zoom_text(abs.into()).0) } } diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 271135b677c..a00461f6998 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -137,12 +137,12 @@ impl<'a> Context<'a> { /// Apply text-zoom if enabled #[cfg(feature = "gecko")] - pub fn maybe_zoom_text(&self, size: Au) -> Au { + pub fn maybe_zoom_text(&self, size: NonNegativeAu) -> NonNegativeAu { // We disable zoom for by unsetting the // -x-text-zoom property, which leads to a false value // in mAllowZoom if self.style().get_font().gecko.mAllowZoom { - self.device().zoom_text(size) + self.device().zoom_text(size.0).into() } else { size } @@ -150,7 +150,7 @@ impl<'a> Context<'a> { /// (Servo doesn't do text-zoom) #[cfg(feature = "servo")] - pub fn maybe_zoom_text(&self, size: Au) -> Au { + pub fn maybe_zoom_text(&self, size: NonNegativeAu) -> NonNegativeAu { size } } @@ -613,6 +613,13 @@ impl NonNegativeAu { pub fn value(self) -> i32 { (self.0).0 } + + /// Scale this NonNegativeAu. + #[inline] + pub fn scale_by(self, factor: f32) -> Self { + // scale this by zero if factor is negative. + NonNegative::(self.0.scale_by(factor.max(0.))) + } } impl From for NonNegativeAu { diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index bf4b83b5293..73c77076ee7 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -92,8 +92,8 @@ impl FontBaseSize { pub fn resolve(&self, context: &Context) -> Au { match *self { FontBaseSize::Custom(size) => size, - FontBaseSize::CurrentStyle => context.style().get_font().clone_font_size(), - FontBaseSize::InheritedStyle => context.style().get_parent_font().clone_font_size(), + FontBaseSize::CurrentStyle => context.style().get_font().clone_font_size().0, + FontBaseSize::InheritedStyle => context.style().get_parent_font().clone_font_size().0, } } } diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 495b3eebd33..2316d12eaeb 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -95,7 +95,7 @@ impl ToComputedValue for LineHeight { GenericLineHeight::Number(number.to_computed_value(context)) }, GenericLineHeight::Length(LengthOrPercentage::Length(ref length)) => { - GenericLineHeight::Length(context.maybe_zoom_text(length.to_computed_value(context))) + GenericLineHeight::Length(context.maybe_zoom_text(length.to_computed_value(context).into()).0) }, GenericLineHeight::Length(LengthOrPercentage::Percentage(p)) => { let font_relative_length =