From 2c7fbb4b4c9d0468caf61d9c9b33331987ad8de7 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 31 May 2017 14:12:28 +0200 Subject: [PATCH] Use generics for text spacing properties --- components/layout/text.rs | 10 +- components/style/properties/gecko.mako.rs | 22 +-- .../longhand/inherited_text.mako.rs | 160 ++---------------- components/style/values/computed/length.rs | 7 + components/style/values/computed/mod.rs | 2 +- components/style/values/computed/text.rs | 9 +- components/style/values/generics/text.rs | 82 +++++++++ components/style/values/specified/mod.rs | 2 +- components/style/values/specified/text.rs | 26 ++- tests/unit/style/parsing/inherited_text.rs | 10 +- 10 files changed, 155 insertions(+), 175 deletions(-) diff --git a/components/layout/text.rs b/components/layout/text.rs index 969e5f0786b..481e2025201 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -166,8 +166,8 @@ impl TextRunScanner { white_space::T::pre_line => CompressionMode::CompressWhitespace, }; text_transform = inherited_text_style.text_transform; - letter_spacing = inherited_text_style.letter_spacing.0; - word_spacing = inherited_text_style.word_spacing.0 + letter_spacing = inherited_text_style.letter_spacing; + word_spacing = inherited_text_style.word_spacing.value().cloned() .map(|lop| lop.to_hash_key()) .unwrap_or((Au(0), NotNaN::new(0.0).unwrap())); text_rendering = inherited_text_style.text_rendering; @@ -289,8 +289,8 @@ impl TextRunScanner { // example, `finally` with a wide `letter-spacing` renders as `f i n a l l y` and not // `fi n a l l y`. let mut flags = ShapingFlags::empty(); - match letter_spacing { - Some(Au(0)) | None => {} + match letter_spacing.value() { + Some(&Au(0)) | None => {} Some(_) => flags.insert(IGNORE_LIGATURES_SHAPING_FLAG), } if text_rendering == text_rendering::T::optimizespeed { @@ -301,7 +301,7 @@ impl TextRunScanner { flags.insert(KEEP_ALL_FLAG); } let options = ShapingOptions { - letter_spacing: letter_spacing, + letter_spacing: letter_spacing.value().cloned(), word_spacing: word_spacing, script: Script::Common, flags: flags, diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 61210eff8a5..c7d8e879848 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3664,35 +3664,37 @@ fn static_assert() { <%call expr="impl_coord_copy('line_height', 'mLineHeight')"> pub fn set_letter_spacing(&mut self, v: longhands::letter_spacing::computed_value::T) { - match v.0 { - Some(au) => self.gecko.mLetterSpacing.set(au), - None => self.gecko.mLetterSpacing.set_value(CoordDataValue::Normal) + use values::generics::text::Spacing; + match v { + Spacing::Value(value) => self.gecko.mLetterSpacing.set(value), + Spacing::Normal => self.gecko.mLetterSpacing.set_value(CoordDataValue::Normal) } } pub fn clone_letter_spacing(&self) -> longhands::letter_spacing::computed_value::T { - use properties::longhands::letter_spacing::computed_value::T; + use values::generics::text::Spacing; debug_assert!( matches!(self.gecko.mLetterSpacing.as_value(), CoordDataValue::Normal | CoordDataValue::Coord(_)), "Unexpected computed value for letter-spacing"); - T(Au::from_gecko_style_coord(&self.gecko.mLetterSpacing)) + Au::from_gecko_style_coord(&self.gecko.mLetterSpacing).map_or(Spacing::Normal, Spacing::Value) } <%call expr="impl_coord_copy('letter_spacing', 'mLetterSpacing')"> pub fn set_word_spacing(&mut self, v: longhands::word_spacing::computed_value::T) { - match v.0 { - Some(lop) => self.gecko.mWordSpacing.set(lop), + use values::generics::text::Spacing; + match v { + Spacing::Value(lop) => self.gecko.mWordSpacing.set(lop), // https://drafts.csswg.org/css-text-3/#valdef-word-spacing-normal - None => self.gecko.mWordSpacing.set_value(CoordDataValue::Coord(0)), + Spacing::Normal => self.gecko.mWordSpacing.set_value(CoordDataValue::Coord(0)), } } pub fn clone_word_spacing(&self) -> longhands::word_spacing::computed_value::T { - use properties::longhands::word_spacing::computed_value::T; use values::computed::LengthOrPercentage; + use values::generics::text::Spacing; debug_assert!( matches!(self.gecko.mWordSpacing.as_value(), CoordDataValue::Normal | @@ -3700,7 +3702,7 @@ fn static_assert() { CoordDataValue::Percent(_) | CoordDataValue::Calc(_)), "Unexpected computed value for word-spacing"); - T(LengthOrPercentage::from_gecko_style_coord(&self.gecko.mWordSpacing)) + LengthOrPercentage::from_gecko_style_coord(&self.gecko.mWordSpacing).map_or(Spacing::Normal, Spacing::Value) } <%call expr="impl_coord_copy('word_spacing', 'mWordSpacing')"> diff --git a/components/style/properties/longhand/inherited_text.mako.rs b/components/style/properties/longhand/inherited_text.mako.rs index 25e1ce946eb..1aac10125e3 100644 --- a/components/style/properties/longhand/inherited_text.mako.rs +++ b/components/style/properties/longhand/inherited_text.mako.rs @@ -258,157 +258,17 @@ ${helpers.single_keyword("text-align-last", % endif -<%helpers:longhand name="letter-spacing" animation_value_type="ComputedValue" - spec="https://drafts.csswg.org/css-text/#propdef-letter-spacing"> - use std::fmt; - use style_traits::ToCss; - use values::specified::AllowQuirks; +${helpers.predefined_type("letter-spacing", + "LetterSpacing", + "computed::LetterSpacing::normal()", + animation_value_type="ComputedValue", + spec="https://drafts.csswg.org/css-text/#propdef-letter-spacing")} - #[derive(Clone, Debug, HasViewportPercentage, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub enum SpecifiedValue { - Normal, - Specified(specified::Length), - } - - impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match *self { - SpecifiedValue::Normal => dest.write_str("normal"), - SpecifiedValue::Specified(ref l) => l.to_css(dest), - } - } - } - - pub mod computed_value { - use app_units::Au; - use properties::animated_properties::Animatable; - - #[derive(Debug, Clone, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct T(pub Option); - - ${helpers.impl_animatable_for_option_tuple('Au(0)')} - } - - impl ToCss for computed_value::T { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match self.0 { - None => dest.write_str("normal"), - Some(l) => l.to_css(dest), - } - } - } - - #[inline] - pub fn get_initial_value() -> computed_value::T { - computed_value::T(None) - } - - impl ToComputedValue for SpecifiedValue { - type ComputedValue = computed_value::T; - - #[inline] - fn to_computed_value(&self, context: &Context) -> computed_value::T { - match *self { - SpecifiedValue::Normal => computed_value::T(None), - SpecifiedValue::Specified(ref l) => - computed_value::T(Some(l.to_computed_value(context))) - } - } - - #[inline] - fn from_computed_value(computed: &computed_value::T) -> Self { - computed.0.map(|ref au| { - SpecifiedValue::Specified(ToComputedValue::from_computed_value(au)) - }).unwrap_or(SpecifiedValue::Normal) - } - } - - pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { - if input.try(|input| input.expect_ident_matching("normal")).is_ok() { - Ok(SpecifiedValue::Normal) - } else { - specified::Length::parse_quirky(context, input, AllowQuirks::Yes).map(SpecifiedValue::Specified) - } - } - - -<%helpers:longhand name="word-spacing" animation_value_type="ComputedValue" - spec="https://drafts.csswg.org/css-text/#propdef-word-spacing"> - use std::fmt; - use style_traits::ToCss; - use values::specified::AllowQuirks; - - #[derive(Clone, Debug, HasViewportPercentage, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub enum SpecifiedValue { - Normal, - Specified(specified::LengthOrPercentage), - } - - impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match *self { - SpecifiedValue::Normal => dest.write_str("normal"), - SpecifiedValue::Specified(ref l) => l.to_css(dest), - } - } - } - - pub mod computed_value { - use properties::animated_properties::Animatable; - use values::computed::LengthOrPercentage; - #[derive(Debug, Clone, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct T(pub Option); - - ${helpers.impl_animatable_for_option_tuple('LengthOrPercentage::zero()')} - } - - impl ToCss for computed_value::T { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match self.0 { - None => dest.write_str("normal"), - Some(l) => l.to_css(dest), - } - } - } - - #[inline] - pub fn get_initial_value() -> computed_value::T { - computed_value::T(None) - } - - impl ToComputedValue for SpecifiedValue { - type ComputedValue = computed_value::T; - - #[inline] - fn to_computed_value(&self, context: &Context) -> computed_value::T { - match *self { - SpecifiedValue::Normal => computed_value::T(None), - SpecifiedValue::Specified(ref l) => - computed_value::T(Some(l.to_computed_value(context))), - } - } - - #[inline] - fn from_computed_value(computed: &computed_value::T) -> Self { - computed.0.map(|ref lop| { - SpecifiedValue::Specified(ToComputedValue::from_computed_value(lop)) - }).unwrap_or(SpecifiedValue::Normal) - } - } - - pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { - if input.try(|input| input.expect_ident_matching("normal")).is_ok() { - Ok(SpecifiedValue::Normal) - } else { - specified::LengthOrPercentage::parse_quirky(context, input, AllowQuirks::Yes) - .map(SpecifiedValue::Specified) - } - } - +${helpers.predefined_type("word-spacing", + "WordSpacing", + "computed::WordSpacing::normal()", + animation_value_type="ComputedValue", + spec="https://drafts.csswg.org/css-text/#propdef-word-spacing")} <%helpers:longhand name="-servo-text-decorations-in-effect" derived_from="display text-decoration" diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 1ed038941a7..e5b35428bef 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -222,6 +222,13 @@ pub enum LengthOrPercentage { Calc(CalcLengthOrPercentage), } +impl From for LengthOrPercentage { + #[inline] + fn from(length: Au) -> Self { + LengthOrPercentage::Length(length) + } +} + impl LengthOrPercentage { #[inline] #[allow(missing_docs)] diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index a0cf56e13d0..d18d43defbc 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -38,7 +38,7 @@ pub use self::length::{CalcLengthOrPercentage, Length, LengthOrNumber, LengthOrP pub use self::length::{LengthOrPercentageOrAutoOrContent, LengthOrPercentageOrNone, LengthOrNone}; pub use self::length::{MaxLength, MozLength}; pub use self::position::Position; -pub use self::text::LineHeight; +pub use self::text::{LetterSpacing, LineHeight, WordSpacing}; pub use self::transform::TransformOrigin; pub mod background; diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index 388b13f302e..d45242015c4 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -7,7 +7,14 @@ use app_units::Au; use properties::animated_properties::Animatable; use values::CSSFloat; -use values::generics::text::LineHeight as GenericLineHeight; +use values::computed::length::{Length, LengthOrPercentage}; +use values::generics::text::{LineHeight as GenericLineHeight, Spacing}; + +/// A computed value for the `letter-spacing` property. +pub type LetterSpacing = Spacing; + +/// A computed value for the `word-spacing` property. +pub type WordSpacing = Spacing; /// A computed value for the `line-height` property. pub type LineHeight = GenericLineHeight; diff --git a/components/style/values/generics/text.rs b/components/style/values/generics/text.rs index 611c9940cf4..2ba5ae4af98 100644 --- a/components/style/values/generics/text.rs +++ b/components/style/values/generics/text.rs @@ -4,9 +4,91 @@ //! Generic types for text properties. +use app_units::Au; +use cssparser::Parser; +use parser::ParserContext; +use properties::animated_properties::Animatable; use std::fmt; use style_traits::ToCss; +/// A generic spacing value for the `letter-spacing` and `word-spacing` properties.alloc +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +#[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq, ToComputedValue)] +pub enum Spacing { + /// `normal` + Normal, + /// `` + Value(Value), +} + +impl Spacing { + /// Returns `normal`. + #[inline] + pub fn normal() -> Self { + Spacing::Normal + } + + /// Parses. + #[inline] + pub fn parse_with( + context: &ParserContext, + input: &mut Parser, + parse: F) + -> Result + where F: FnOnce(&ParserContext, &mut Parser) -> Result + { + if input.try(|i| i.expect_ident_matching("normal")).is_ok() { + return Ok(Spacing::Normal); + } + parse(context, input).map(Spacing::Value) + } + + /// Returns the spacing value, if not `normal`. + #[inline] + pub fn value(&self) -> Option<&Value> { + match *self { + Spacing::Normal => None, + Spacing::Value(ref value) => Some(value), + } + } +} + +impl Animatable for Spacing + where Value: Animatable + From, +{ + #[inline] + fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64) -> Result { + if let (&Spacing::Normal, &Spacing::Normal) = (self, other) { + return Ok(Spacing::Normal); + } + let zero = Value::from(Au(0)); + let this = self.value().unwrap_or(&zero); + let other = other.value().unwrap_or(&zero); + this.add_weighted(other, self_portion, other_portion).map(Spacing::Value) + } + + #[inline] + fn compute_distance(&self, other: &Self) -> Result { + let zero = Value::from(Au(0)); + let this = self.value().unwrap_or(&zero); + let other = other.value().unwrap_or(&zero); + this.compute_distance(other) + } +} + +impl ToCss for Spacing + where Value: ToCss, +{ + fn to_css(&self, dest: &mut W) -> fmt::Result + where W: fmt::Write + { + match *self { + Spacing::Normal => dest.write_str("normal"), + Spacing::Value(ref value) => value.to_css(dest), + } + } +} + /// A generic value for the `line-height` property. #[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq)] diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index b397de791b0..c0bf1cdb3b7 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -44,7 +44,7 @@ pub use self::length::{Percentage, LengthOrNone, LengthOrNumber, LengthOrPercent pub use self::length::{LengthOrPercentageOrNone, LengthOrPercentageOrAutoOrContent, NoCalcLength}; pub use self::length::{MaxLength, MozLength}; pub use self::position::{Position, PositionComponent}; -pub use self::text::LineHeight; +pub use self::text::{LetterSpacing, LineHeight, WordSpacing}; pub use self::transform::TransformOrigin; #[cfg(feature = "gecko")] diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index bca8467cfd9..cc517ad4abb 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -9,13 +9,35 @@ use parser::{Parse, ParserContext}; use std::ascii::AsciiExt; use values::computed::{Context, ToComputedValue}; use values::computed::text::LineHeight as ComputedLineHeight; -use values::generics::text::LineHeight as GenericLineHeight; -use values::specified::Number; +use values::generics::text::{LineHeight as GenericLineHeight, Spacing}; +use values::specified::{AllowQuirks, Number}; use values::specified::length::{FontRelativeLength, Length, LengthOrPercentage, NoCalcLength}; +/// A specified value for the `letter-spacing` property. +pub type LetterSpacing = Spacing; + +/// A specified value for the `word-spacing` property. +pub type WordSpacing = Spacing; + /// A specified value for the `line-height` property. pub type LineHeight = GenericLineHeight; +impl Parse for LetterSpacing { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + Spacing::parse_with(context, input, |c, i| { + Length::parse_quirky(c, i, AllowQuirks::Yes) + }) + } +} + +impl Parse for WordSpacing { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + Spacing::parse_with(context, input, |c, i| { + LengthOrPercentage::parse_quirky(c, i, AllowQuirks::Yes) + }) + } +} + impl Parse for LineHeight { fn parse(context: &ParserContext, input: &mut Parser) -> Result { if let Ok(number) = input.try(|i| Number::parse_non_negative(context, i)) { diff --git a/tests/unit/style/parsing/inherited_text.rs b/tests/unit/style/parsing/inherited_text.rs index af0ec0ee5d1..5c578def660 100644 --- a/tests/unit/style/parsing/inherited_text.rs +++ b/tests/unit/style/parsing/inherited_text.rs @@ -3,27 +3,27 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use parsing::parse; +use style::values::generics::text::Spacing; #[test] fn negative_letter_spacing_should_parse_properly() { use style::properties::longhands::letter_spacing; - use style::properties::longhands::letter_spacing::SpecifiedValue; use style::values::specified::length::{Length, NoCalcLength, FontRelativeLength}; let negative_value = parse_longhand!(letter_spacing, "-0.5em"); - let expected = SpecifiedValue::Specified(Length::NoCalc(NoCalcLength::FontRelative(FontRelativeLength::Em(-0.5)))); + let expected = Spacing::Value(Length::NoCalc(NoCalcLength::FontRelative(FontRelativeLength::Em(-0.5)))); assert_eq!(negative_value, expected); } #[test] fn negative_word_spacing_should_parse_properly() { use style::properties::longhands::word_spacing; - use style::properties::longhands::word_spacing::SpecifiedValue; use style::values::specified::length::{NoCalcLength, LengthOrPercentage, FontRelativeLength}; let negative_value = parse_longhand!(word_spacing, "-0.5em"); - let expected = SpecifiedValue::Specified(LengthOrPercentage::Length(NoCalcLength::FontRelative( - FontRelativeLength::Em(-0.5)))); + let expected = Spacing::Value(LengthOrPercentage::Length( + NoCalcLength::FontRelative(FontRelativeLength::Em(-0.5)) + )); assert_eq!(negative_value, expected); }