From b96981f88ee1d319e31a61c415183acb634b4a73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 4 Mar 2019 18:19:40 +0000 Subject: [PATCH] style: Make word-spacing, letter-spacing, and line-height use Rust lengths. This also adopts the resolution from [1] while at it, making letter-spacing compute to a length, serializing 0 to normal rather than keeping normal in the computed value, which matches every other engine. This removes the SMIL tests for percentages from letter-spacing since letter-spacing does in fact not support percentages, so they were passing just by chance. [1]: https://github.com/w3c/csswg-drafts/issues/1484 Differential Revision: https://phabricator.services.mozilla.com/D21850 --- components/style/cbindgen.toml | 4 + components/style/properties/gecko.mako.rs | 120 +----------------- .../longhands/inherited_text.mako.rs | 2 +- components/style/values/computed/text.rs | 77 ++++++++++- components/style/values/generics/text.rs | 62 ++------- 5 files changed, 94 insertions(+), 171 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 205f54783164..9b7159fcba8f 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -78,8 +78,10 @@ include = [ "Resize", "Overflow", "LengthPercentage", + "LetterSpacing", "NonNegativeLengthPercentage", "LengthPercentageOrAuto", + "LineHeight", "NonNegativeLengthPercentageOrAuto", "Rect", "IntersectionObserverRootMargin", @@ -102,6 +104,7 @@ item_types = ["enums", "structs", "typedefs"] [export.body] "CSSPixelLength" = """ inline nscoord ToAppUnits() const; + inline bool IsZero() const; """ "LengthPercentage" = """ @@ -118,6 +121,7 @@ item_types = ["enums", "structs", "typedefs"] inline bool ConvertsToPercentage() const; inline bool HasLengthAndPercentage() const; inline float ToPercentage() const; + inline bool IsDefinitelyZero() const; inline CSSCoord ResolveToCSSPixels(CSSCoord aPercentageBasisInCSSPixels) const; template inline CSSCoord ResolveToCSSPixelsWith(T aPercentageGetter) const; template diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index a22e745963ed..8f4fcaec5c9e 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1227,56 +1227,23 @@ impl Clone for ${style_struct.gecko_struct_name} { # Types used with predefined_type()-defined properties that we can auto-generate. predefined_types = { - "Appearance": impl_simple, - "OverscrollBehavior": impl_simple, - "OverflowClipBox": impl_simple, - "ScrollSnapAlign": impl_simple, - "ScrollSnapType": impl_simple, - "Float": impl_simple, - "Overflow": impl_simple, - "BreakBetween": impl_simple, - "BreakWithin": impl_simple, - "Resize": impl_simple, "Color": impl_color, "ColorOrAuto": impl_color, - "GreaterThanOrEqualToOneNumber": impl_simple, - "Integer": impl_simple, "length::LengthOrAuto": impl_style_coord, "length::LengthOrNormal": impl_style_coord, "length::NonNegativeLengthOrAuto": impl_style_coord, "length::NonNegativeLengthPercentageOrNormal": impl_style_coord, - "FillRule": impl_simple, - "FlexBasis": impl_simple, "Length": impl_absolute_length, "LengthOrNormal": impl_style_coord, - "LengthPercentage": impl_simple, "LengthPercentageOrAuto": impl_style_coord, - "MaxSize": impl_simple, - "Size": impl_simple, "MozScriptMinSize": impl_absolute_length, - "MozScriptSizeMultiplier": impl_simple, - "NonNegativeLengthPercentage": impl_simple, - "NonNegativeLengthOrNumber": impl_simple, - "NonNegativeLengthOrNumberRect": impl_simple, - "BorderImageSlice": impl_simple, - "NonNegativeNumber": impl_simple, - "Number": impl_simple, - "Opacity": impl_simple, - "OverflowWrap": impl_simple, - "OverflowAnchor": impl_simple, - "Perspective": impl_simple, - "Position": impl_simple, "RGBAColor": impl_rgba_color, "SVGLength": impl_svg_length, "SVGOpacity": impl_svg_opacity, "SVGPaint": impl_svg_paint, "SVGWidth": impl_svg_length, "Transform": impl_transform, - "TransformOrigin": impl_simple, - "UserSelect": impl_simple, "url::UrlOrNone": impl_css_url, - "WordBreak": impl_simple, - "ZIndex": impl_simple, } def longhand_method(longhand): @@ -1291,15 +1258,12 @@ impl Clone for ${style_struct.gecko_struct_name} { args.update(keyword=longhand.keyword) if "font" in longhand.ident: args.update(cast_type=longhand.cast_type) - else: + elif longhand.predefined_type in predefined_types: method = predefined_types[longhand.predefined_type] + else: + method = impl_simple method(**args) - - picked_longhands = [] - for x in longhands: - if x.keyword or x.predefined_type in predefined_types or x.logical: - picked_longhands.append(x) %> impl ${style_struct.gecko_struct_name} { /* @@ -1311,7 +1275,7 @@ impl ${style_struct.gecko_struct_name} { * Auto-Generated Methods. */ <% - for longhand in picked_longhands: + for longhand in longhands: longhand_method(longhand) %> } @@ -1992,6 +1956,7 @@ fn static_assert() { <% skip_font_longhands = """font-family font-size font-size-adjust font-weight + font-style font-stretch -moz-script-level font-synthesis -x-lang font-variant-alternates font-variant-east-asian font-variant-ligatures font-variant-numeric font-language-override @@ -2783,6 +2748,7 @@ fn static_assert() { animation-iteration-count animation-timing-function clear transition-duration transition-delay transition-timing-function transition-property + transform-style rotate scroll-snap-points-x scroll-snap-points-y scroll-snap-coordinate -moz-binding will-change offset-path shape-outside contain touch-action @@ -4158,7 +4124,7 @@ fn static_assert() { <%self:impl_trait style_struct_name="InheritedText" - skip_longhands="text-align text-emphasis-style text-shadow line-height letter-spacing word-spacing + skip_longhands="text-align text-emphasis-style text-shadow -webkit-text-stroke-width text-emphasis-position"> <% text_align_keyword = Keyword("text-align", @@ -4190,78 +4156,6 @@ fn static_assert() { longhands::text_shadow::computed_value::List(buf) } - pub fn set_line_height(&mut self, v: longhands::line_height::computed_value::T) { - use crate::values::generics::text::LineHeight; - // FIXME: Align binary representations and ditch |match| for cast + static_asserts - let en = match v { - LineHeight::Normal => CoordDataValue::Normal, - LineHeight::Length(val) => CoordDataValue::Coord(val.0.to_i32_au()), - LineHeight::Number(val) => CoordDataValue::Factor(val.0), - LineHeight::MozBlockHeight => - CoordDataValue::Enumerated(structs::NS_STYLE_LINE_HEIGHT_BLOCK_HEIGHT), - }; - self.gecko.mLineHeight.set_value(en); - } - - pub fn clone_line_height(&self) -> longhands::line_height::computed_value::T { - use crate::values::generics::text::LineHeight; - return match self.gecko.mLineHeight.as_value() { - CoordDataValue::Normal => LineHeight::Normal, - CoordDataValue::Coord(coord) => LineHeight::Length(Au(coord).into()), - CoordDataValue::Factor(n) => LineHeight::Number(n.into()), - CoordDataValue::Enumerated(val) if val == structs::NS_STYLE_LINE_HEIGHT_BLOCK_HEIGHT => - LineHeight::MozBlockHeight, - _ => panic!("this should not happen"), - } - } - - <%call expr="impl_coord_copy('line_height', 'mLineHeight')"> - - pub fn set_letter_spacing(&mut self, v: longhands::letter_spacing::computed_value::T) { - use crate::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 crate::values::computed::Length; - use crate::values::generics::text::Spacing; - debug_assert!( - matches!(self.gecko.mLetterSpacing.as_value(), - CoordDataValue::Normal | - CoordDataValue::Coord(_)), - "Unexpected computed value for letter-spacing"); - Length::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) { - use crate::values::generics::text::Spacing; - match v { - Spacing::Value(lp) => self.gecko.mWordSpacing.set(lp), - // https://drafts.csswg.org/css-text-3/#valdef-word-spacing-normal - Spacing::Normal => self.gecko.mWordSpacing.set_value(CoordDataValue::Coord(0)), - } - } - - pub fn clone_word_spacing(&self) -> longhands::word_spacing::computed_value::T { - use crate::values::computed::LengthPercentage; - use crate::values::generics::text::Spacing; - debug_assert!( - matches!(self.gecko.mWordSpacing.as_value(), - CoordDataValue::Normal | - CoordDataValue::Coord(_) | - CoordDataValue::Percent(_) | - CoordDataValue::Calc(_)), - "Unexpected computed value for word-spacing"); - LengthPercentage::from_gecko_style_coord(&self.gecko.mWordSpacing).map_or(Spacing::Normal, Spacing::Value) - } - - <%call expr="impl_coord_copy('word_spacing', 'mWordSpacing')"> - fn clear_text_emphasis_style_if_string(&mut self) { if self.gecko.mTextEmphasisStyle == structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING as u8 { self.gecko.mTextEmphasisStyleString.truncate(); diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs index f11a9a7b6b2b..f61c8754b5ec 100644 --- a/components/style/properties/longhands/inherited_text.mako.rs +++ b/components/style/properties/longhands/inherited_text.mako.rs @@ -157,7 +157,7 @@ ${helpers.predefined_type( ${helpers.predefined_type( "word-spacing", "WordSpacing", - "computed::WordSpacing::normal()", + "computed::WordSpacing::zero()", animation_value_type="ComputedValue", flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER", spec="https://drafts.csswg.org/css-text/#propdef-word-spacing", diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index e921967f46d9..783e553d1cef 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -7,13 +7,14 @@ #[cfg(feature = "servo")] use crate::properties::StyleBuilder; use crate::values::computed::length::{Length, LengthPercentage}; -use crate::values::computed::{NonNegativeLength, NonNegativeNumber}; +use crate::values::computed::{Context, NonNegativeLength, NonNegativeNumber, ToComputedValue}; use crate::values::generics::text::InitialLetter as GenericInitialLetter; use crate::values::generics::text::LineHeight as GenericLineHeight; use crate::values::generics::text::Spacing; -use crate::values::specified::text::TextOverflowSide; +use crate::values::specified::text::{self as specified, TextOverflowSide}; use crate::values::specified::text::{TextEmphasisFillMode, TextEmphasisShapeKeyword}; use crate::values::{CSSFloat, CSSInteger}; +use crate::Zero; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; @@ -25,10 +26,78 @@ pub use crate::values::specified::TextEmphasisPosition; pub type InitialLetter = GenericInitialLetter; /// A computed value for the `letter-spacing` property. -pub type LetterSpacing = Spacing; +#[repr(transparent)] +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + PartialEq, + ToAnimatedValue, + ToAnimatedZero, +)] +pub struct LetterSpacing(Length); + +impl LetterSpacing { + /// Return the `normal` computed value, which is just zero. + #[inline] + pub fn normal() -> Self { + LetterSpacing(Length::zero()) + } +} + +impl ToCss for LetterSpacing { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + // https://drafts.csswg.org/css-text/#propdef-letter-spacing + // + // For legacy reasons, a computed letter-spacing of zero yields a + // resolved value (getComputedStyle() return value) of normal. + if self.0.is_zero() { + return dest.write_str("normal"); + } + self.0.to_css(dest) + } +} + +impl ToComputedValue for specified::LetterSpacing { + type ComputedValue = LetterSpacing; + fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { + match *self { + Spacing::Normal => LetterSpacing(Length::zero()), + Spacing::Value(ref v) => LetterSpacing(v.to_computed_value(context)), + } + } + + fn from_computed_value(computed: &Self::ComputedValue) -> Self { + if computed.0.is_zero() { + return Spacing::Normal; + } + Spacing::Value(ToComputedValue::from_computed_value(&computed.0)) + } +} /// A computed value for the `word-spacing` property. -pub type WordSpacing = Spacing; +pub type WordSpacing = LengthPercentage; + +impl ToComputedValue for specified::WordSpacing { + type ComputedValue = WordSpacing; + + fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { + match *self { + Spacing::Normal => LengthPercentage::zero(), + Spacing::Value(ref v) => v.to_computed_value(context), + } + } + + fn from_computed_value(computed: &Self::ComputedValue) -> Self { + Spacing::Value(ToComputedValue::from_computed_value(computed)) + } +} /// 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 029c5681318b..f339bf83432c 100644 --- a/components/style/values/generics/text.rs +++ b/components/style/values/generics/text.rs @@ -5,9 +5,7 @@ //! Generic types for text properties. use crate::parser::ParserContext; -use crate::values::animated::{Animate, Procedure, ToAnimatedZero}; -use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; -use app_units::Au; +use crate::values::animated::ToAnimatedZero; use cssparser::Parser; use style_traits::ParseError; @@ -32,7 +30,7 @@ impl InitialLetter { /// A generic spacing value for the `letter-spacing` and `word-spacing` properties. #[derive( - Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss, + Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, )] pub enum Spacing { /// `normal` @@ -63,51 +61,6 @@ impl Spacing { } 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 Animate for Spacing -where - Value: Animate + From, -{ - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> 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); - Ok(Spacing::Value(this.animate(other, procedure)?)) - } -} - -impl ComputeSquaredDistance for Spacing -where - V: ComputeSquaredDistance + From, -{ - #[inline] - fn compute_squared_distance(&self, other: &Self) -> Result { - let zero = V::from(Au(0)); - let this = self.value().unwrap_or(&zero); - let other = other.value().unwrap_or(&zero); - this.compute_squared_distance(other) - } -} - -impl ToAnimatedZero for Spacing { - #[inline] - fn to_animated_zero(&self) -> Result { - Err(()) - } } /// A generic value for the `line-height` property. @@ -123,18 +76,21 @@ impl ToAnimatedZero for Spacing { ToAnimatedValue, ToCss, )] -pub enum LineHeight { +#[repr(C, u8)] +pub enum GenericLineHeight { /// `normal` Normal, /// `-moz-block-height` #[cfg(feature = "gecko")] MozBlockHeight, /// `` - Number(Number), - /// `` - Length(LengthPercentage), + Number(N), + /// `` + Length(L), } +pub use self::GenericLineHeight as LineHeight; + impl ToAnimatedZero for LineHeight { #[inline] fn to_animated_zero(&self) -> Result {