From ebeb7b228fb93a7631bf8a4903e94ed8055d13fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 25 Feb 2019 03:03:15 +0000 Subject: [PATCH 01/31] style: Simplify border-radius serialization. Differential Revision: https://phabricator.services.mozilla.com/D20958 --- components/style/values/generics/border.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/components/style/values/generics/border.rs b/components/style/values/generics/border.rs index ffd30e98678..a722548e732 100644 --- a/components/style/values/generics/border.rs +++ b/components/style/values/generics/border.rs @@ -145,11 +145,7 @@ where W: Write, { widths.to_css(dest)?; - if widths.0 != heights.0 || - widths.1 != heights.1 || - widths.2 != heights.2 || - widths.3 != heights.3 - { + if widths != heights { dest.write_str(" / ")?; heights.to_css(dest)?; } From aad4dac5b422ff107484d7dccbc0f33ce8251755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 25 Feb 2019 03:05:39 +0000 Subject: [PATCH 02/31] style: Make the generic size not use euclid under the hood. The euclid size is not really used for anything. Also rename it to Size2D to avoid cbindgen conflicts with values::length::Size. Differential Revision: https://phabricator.services.mozilla.com/D20959 --- components/style/values/computed/border.rs | 6 +- components/style/values/generics/border.rs | 14 ++--- components/style/values/generics/size.rs | 61 +++++++-------------- components/style/values/specified/border.rs | 6 +- 4 files changed, 32 insertions(+), 55 deletions(-) diff --git a/components/style/values/computed/border.rs b/components/style/values/computed/border.rs index e85b5a3e5d6..7b42dd0e879 100644 --- a/components/style/values/computed/border.rs +++ b/components/style/values/computed/border.rs @@ -12,7 +12,7 @@ use crate::values::generics::border::BorderImageSlice as GenericBorderImageSlice use crate::values::generics::border::BorderRadius as GenericBorderRadius; use crate::values::generics::border::BorderSpacing as GenericBorderSpacing; use crate::values::generics::rect::Rect; -use crate::values::generics::size::Size; +use crate::values::generics::size::Size2D; use crate::values::generics::NonNegative; use app_units::Au; @@ -59,7 +59,7 @@ impl BorderImageSlice { impl BorderSpacing { /// Returns `0 0`. pub fn zero() -> Self { - GenericBorderSpacing(Size::new( + GenericBorderSpacing(Size2D::new( NonNegativeLength::zero(), NonNegativeLength::zero(), )) @@ -79,7 +79,7 @@ impl BorderSpacing { impl BorderCornerRadius { /// Returns `0 0`. pub fn zero() -> Self { - GenericBorderCornerRadius(Size::new( + GenericBorderCornerRadius(Size2D::new( NonNegativeLengthPercentage::zero(), NonNegativeLengthPercentage::zero(), )) diff --git a/components/style/values/generics/border.rs b/components/style/values/generics/border.rs index a722548e732..148f74e3834 100644 --- a/components/style/values/generics/border.rs +++ b/components/style/values/generics/border.rs @@ -5,7 +5,7 @@ //! Generic types for CSS values related to borders. use crate::values::generics::rect::Rect; -use crate::values::generics::size::Size; +use crate::values::generics::size::Size2D; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; @@ -53,12 +53,12 @@ pub use self::GenericBorderImageSlice as BorderImageSlice; ToComputedValue, ToCss, )] -pub struct BorderCornerRadius(#[css(field_bound)] pub Size); +pub struct BorderCornerRadius(#[css(field_bound)] pub Size2D); impl BorderCornerRadius { /// Trivially create a `BorderCornerRadius`. pub fn new(w: L, h: L) -> Self { - BorderCornerRadius(Size::new(w, h)) + BorderCornerRadius(Size2D::new(w, h)) } } @@ -77,12 +77,12 @@ impl BorderCornerRadius { ToComputedValue, ToCss, )] -pub struct BorderSpacing(#[css(field_bound)] pub Size); +pub struct BorderSpacing(#[css(field_bound)] pub Size2D); impl BorderSpacing { /// Trivially create a `BorderCornerRadius`. pub fn new(w: L, h: L) -> Self { - BorderSpacing(Size::new(w, h)) + BorderSpacing(Size2D::new(w, h)) } } @@ -168,8 +168,8 @@ where bottom_left: BorderCornerRadius(ref bl), } = *self; - let widths = Rect::new(&tl.0.width, &tr.0.width, &br.0.width, &bl.0.width); - let heights = Rect::new(&tl.0.height, &tr.0.height, &br.0.height, &bl.0.height); + let widths = Rect::new(&tl.width, &tr.width, &br.width, &bl.width); + let heights = Rect::new(&tl.height, &tr.height, &br.height, &bl.height); Self::serialize_rects(widths, heights, dest) } diff --git a/components/style/values/generics/size.rs b/components/style/values/generics/size.rs index a5a5c86eac6..c67d6e82781 100644 --- a/components/style/values/generics/size.rs +++ b/components/style/values/generics/size.rs @@ -5,11 +5,9 @@ //! Generic type for CSS properties that are composed by two dimensions. use crate::parser::ParserContext; -use crate::values::animated::ToAnimatedValue; use cssparser::Parser; -use euclid::Size2D; use std::fmt::{self, Write}; -use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, ToCss}; +use style_traits::{CssWriter, ParseError, ToCss}; /// A generic size, for `border-*-radius` longhand properties, or /// `border-spacing`. @@ -21,29 +19,35 @@ use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, ToCss}; Debug, MallocSizeOf, PartialEq, + SpecifiedValueInfo, ToAnimatedZero, + ToAnimatedValue, ToComputedValue, )] -pub struct Size(pub Size2D); +#[allow(missing_docs)] +pub struct Size2D { + pub width: L, + pub height: L, +} -impl Size { +impl Size2D { #[inline] - /// Create a new `Size` for an area of given width and height. - pub fn new(width: L, height: L) -> Size { - Size(Size2D::new(width, height)) + /// Create a new `Size2D` for an area of given width and height. + pub fn new(width: L, height: L) -> Self { + Self { width, height } } /// Returns the width component. pub fn width(&self) -> &L { - &self.0.width + &self.width } /// Returns the height component. pub fn height(&self) -> &L { - &self.0.height + &self.height } - /// Parse a `Size` with a given parsing function. + /// Parse a `Size2D` with a given parsing function. pub fn parse_with<'i, 't, F>( context: &ParserContext, input: &mut Parser<'i, 't>, @@ -61,7 +65,7 @@ impl Size { } } -impl ToCss for Size +impl ToCss for Size2D where L: ToCss + PartialEq, { @@ -69,40 +73,13 @@ where where W: Write, { - self.0.width.to_css(dest)?; + self.width.to_css(dest)?; - if self.0.height != self.0.width { + if self.height != self.width { dest.write_str(" ")?; - self.0.height.to_css(dest)?; + self.height.to_css(dest)?; } Ok(()) } } - -impl ToAnimatedValue for Size -where - L: ToAnimatedValue, -{ - type AnimatedValue = Size; - - #[inline] - fn to_animated_value(self) -> Self::AnimatedValue { - Size(Size2D::new( - self.0.width.to_animated_value(), - self.0.height.to_animated_value(), - )) - } - - #[inline] - fn from_animated_value(animated: Self::AnimatedValue) -> Self { - Size(Size2D::new( - L::from_animated_value(animated.0.width), - L::from_animated_value(animated.0.height), - )) - } -} - -impl SpecifiedValueInfo for Size { - const SUPPORTED_TYPES: u8 = L::SUPPORTED_TYPES; -} diff --git a/components/style/values/specified/border.rs b/components/style/values/specified/border.rs index e16d37b4786..0b1e9026b8e 100644 --- a/components/style/values/specified/border.rs +++ b/components/style/values/specified/border.rs @@ -12,7 +12,7 @@ use crate::values::generics::border::BorderImageSlice as GenericBorderImageSlice use crate::values::generics::border::BorderRadius as GenericBorderRadius; use crate::values::generics::border::BorderSpacing as GenericBorderSpacing; use crate::values::generics::rect::Rect; -use crate::values::generics::size::Size; +use crate::values::generics::size::Size2D; use crate::values::specified::length::{NonNegativeLength, NonNegativeLengthPercentage}; use crate::values::specified::{AllowQuirks, NonNegativeNumber, NonNegativeNumberOrPercentage}; use cssparser::Parser; @@ -228,7 +228,7 @@ impl Parse for BorderCornerRadius { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - Size::parse_with(context, input, NonNegativeLengthPercentage::parse) + Size2D::parse_with(context, input, NonNegativeLengthPercentage::parse) .map(GenericBorderCornerRadius) } } @@ -238,7 +238,7 @@ impl Parse for BorderSpacing { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - Size::parse_with(context, input, |context, input| { + Size2D::parse_with(context, input, |context, input| { NonNegativeLength::parse_quirky(context, input, AllowQuirks::Yes).map(From::from) }) .map(GenericBorderSpacing) From 197065f6bc425592a6d0b59a4e0e13ebe9c12b52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 24 Feb 2019 10:47:53 -0800 Subject: [PATCH 03/31] style: Use rust lengths for border corners. The test in https://github.com/web-platform-tests/wpt/pull/15423 hasn't been synced over yet, but it passes with this patch of course. Differential Revision: https://phabricator.services.mozilla.com/D20960 --- components/style/cbindgen.toml | 7 ++ components/style/gecko/conversions.rs | 67 +------------------ .../gecko_bindings/sugar/ns_style_coord.rs | 60 +---------------- components/style/properties/gecko.mako.rs | 61 +++++------------ components/style/values/computed/border.rs | 10 +++ components/style/values/generics/border.rs | 19 ++++-- components/style/values/generics/size.rs | 1 + 7 files changed, 51 insertions(+), 174 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 0b8719c4dbf..82ed48ab290 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -13,6 +13,7 @@ autogen_warning = """/* DO NOT MODIFY THIS MANUALLY! This file was generated usi class nsAtom; namespace mozilla { class WritingMode; + enum HalfCorner : uint8_t; enum LogicalSide : uint8_t; namespace css { struct URLValue; @@ -88,6 +89,8 @@ include = [ "Position", "BackgroundSize", "BorderImageSlice", + "BorderSpacing", + "BorderRadius", "NonNegativeLengthOrNumberRect", "Perspective", "ZIndex", @@ -185,3 +188,7 @@ item_types = ["enums", "structs", "typedefs"] inline const T& GetIEnd(mozilla::WritingMode) const; inline const T& GetBEnd(mozilla::WritingMode) const; """ + +"GenericBorderRadius" = """ + inline const StyleLengthPercentage& Get(mozilla::HalfCorner) const; +""" diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 2a2ae268395..4cbf759c2d8 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -575,17 +575,15 @@ pub mod basic_shape { //! Conversions from and to CSS shape representations. use crate::gecko::values::GeckoStyleCoordConvertible; - use crate::gecko_bindings::structs::{nsStyleCoord, nsStyleCorners}; + use crate::gecko_bindings::structs::nsStyleCoord; use crate::gecko_bindings::structs::{StyleBasicShape, StyleBasicShapeType}; use crate::gecko_bindings::structs::{ StyleGeometryBox, StyleShapeSource, StyleShapeSourceType, }; - use crate::gecko_bindings::sugar::ns_style_coord::{CoordDataMut, CoordDataValue}; use crate::gecko_bindings::sugar::refptr::RefPtr; use crate::values::computed::basic_shape::{ BasicShape, ClippingShape, FloatAreaShape, ShapeRadius, }; - use crate::values::computed::border::{BorderCornerRadius, BorderRadius}; use crate::values::computed::length::LengthPercentage; use crate::values::computed::motion::OffsetPath; use crate::values::computed::url::ComputedUrl; @@ -594,9 +592,7 @@ pub mod basic_shape { }; use crate::values::generics::basic_shape::{Circle, Ellipse, Path, PolygonCoord}; use crate::values::generics::basic_shape::{GeometryBox, ShapeBox, ShapeSource}; - use crate::values::generics::border::BorderRadius as GenericBorderRadius; use crate::values::generics::rect::Rect; - use crate::values::generics::NonNegative; use crate::values::specified::SVGPathData; use std::borrow::Borrow; @@ -706,7 +702,7 @@ pub mod basic_shape { let r = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[1]); let b = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[2]); let l = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[3]); - let round: BorderRadius = (&other.mRadius).into(); + let round = other.mRadius; let round = if round.all_zero() { None } else { Some(round) }; let rect = Rect::new( t.expect("inset() offset should be a length, percentage, or calc value"), @@ -752,65 +748,6 @@ pub mod basic_shape { } } - impl<'a> From<&'a nsStyleCorners> for BorderRadius { - fn from(other: &'a nsStyleCorners) -> Self { - let get_corner = |index| { - BorderCornerRadius::new( - NonNegative( - LengthPercentage::from_gecko_style_coord(&other.data_at(index)).expect( - " should be a length, percentage, or calc value", - ), - ), - NonNegative( - LengthPercentage::from_gecko_style_coord(&other.data_at(index + 1)).expect( - " should be a length, percentage, or calc value", - ), - ), - ) - }; - - GenericBorderRadius { - top_left: get_corner(0), - top_right: get_corner(2), - bottom_right: get_corner(4), - bottom_left: get_corner(6), - } - } - } - - // Can't be a From impl since we need to set an existing - // nsStyleCorners, not create a new one - impl BorderRadius { - /// Set this `BorderRadius` into a given `nsStyleCoord`. - pub fn set_corners(&self, other: &mut nsStyleCorners) { - let mut set_corner = |field: &BorderCornerRadius, index| { - field - .0 - .width() - .to_gecko_style_coord(&mut other.data_at_mut(index)); - field - .0 - .height() - .to_gecko_style_coord(&mut other.data_at_mut(index + 1)); - }; - set_corner(&self.top_left, 0); - set_corner(&self.top_right, 2); - set_corner(&self.bottom_right, 4); - set_corner(&self.bottom_left, 6); - } - } - - /// We use None for a nonexistant radius, but Gecko uses (0 0 0 0 / 0 0 0 0) - pub fn set_corners_from_radius(radius: Option, other: &mut nsStyleCorners) { - if let Some(radius) = radius { - radius.set_corners(other); - } else { - for i in 0..8 { - other.data_at_mut(i).set_value(CoordDataValue::Coord(0)); - } - } - } - impl<'a> From<&'a nsStyleCoord> for ShapeRadius { fn from(other: &'a nsStyleCoord) -> Self { let other = other.borrow(); diff --git a/components/style/gecko_bindings/sugar/ns_style_coord.rs b/components/style/gecko_bindings/sugar/ns_style_coord.rs index 4f188297d5e..133ce4d9b7b 100644 --- a/components/style/gecko_bindings/sugar/ns_style_coord.rs +++ b/components/style/gecko_bindings/sugar/ns_style_coord.rs @@ -6,7 +6,7 @@ use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs::{nsStyleCoord, nsStyleCoord_Calc, nsStyleCoord_CalcValue}; -use crate::gecko_bindings::structs::{nsStyleCorners, nsStyleSides}; +use crate::gecko_bindings::structs::nsStyleSides; use crate::gecko_bindings::structs::{nsStyleUnion, nsStyleUnit, nscoord}; use std::mem; @@ -123,64 +123,6 @@ unsafe impl<'a> CoordDataMut for SidesDataMut<'a> { } } -impl nsStyleCorners { - /// Get a `nsStyleCoord` like object representing the given index's value - /// and unit. - #[inline] - pub fn data_at(&self, index: usize) -> CornersData { - CornersData { - corners: self, - index: index, - } - } - - /// Get a `nsStyleCoord` like object representing the mutable given index's - /// value and unit. - #[inline] - pub fn data_at_mut(&mut self, index: usize) -> CornersDataMut { - CornersDataMut { - corners: self, - index: index, - } - } -} - -/// A `nsStyleCoord`-like struct on top of `nsStyleCorners`. -pub struct CornersData<'a> { - corners: &'a nsStyleCorners, - index: usize, -} - -/// A `nsStyleCoord`-like struct on top of a mutable `nsStyleCorners` reference. -pub struct CornersDataMut<'a> { - corners: &'a mut nsStyleCorners, - index: usize, -} - -unsafe impl<'a> CoordData for CornersData<'a> { - fn unit(&self) -> nsStyleUnit { - unsafe { self.corners.get_mUnits()[self.index] } - } - fn union(&self) -> nsStyleUnion { - unsafe { self.corners.get_mValues()[self.index] } - } -} -unsafe impl<'a> CoordData for CornersDataMut<'a> { - fn unit(&self) -> nsStyleUnit { - unsafe { self.corners.get_mUnits()[self.index] } - } - fn union(&self) -> nsStyleUnion { - unsafe { self.corners.get_mValues()[self.index] } - } -} -unsafe impl<'a> CoordDataMut for CornersDataMut<'a> { - unsafe fn values_mut(&mut self) -> (&mut nsStyleUnit, &mut nsStyleUnion) { - let unit = &mut self.corners.get_mUnits_mut()[self.index] as *mut _; - let value = &mut self.corners.get_mValues_mut()[self.index] as *mut _; - (&mut *unit, &mut *value) - } -} - /// Enum representing the tagged union that is CoordData. /// /// In release mode this should never actually exist in the code, and will be diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index b1be315e7a0..65ee8bc1053 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -52,7 +52,7 @@ use crate::rule_tree::StrongRuleNode; use crate::selector_parser::PseudoElement; use servo_arc::{Arc, RawOffsetArc}; use std::marker::PhantomData; -use std::mem::{forget, uninitialized, transmute, zeroed}; +use std::mem::{forget, uninitialized, zeroed}; use std::{cmp, ops, ptr}; use crate::values::{self, CustomIdent, Either, KeyframesName, None_}; use crate::values::computed::{NonNegativeLength, Percentage, TransitionProperty}; @@ -817,34 +817,23 @@ def set_gecko_property(ffi_name, expr): } -<%def name="impl_corner_style_coord(ident, gecko_ffi_name, x_index, y_index)"> +<%def name="impl_corner_style_coord(ident, gecko_ffi_name, corner)"> #[allow(non_snake_case)] pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { - v.0.width().to_gecko_style_coord(&mut self.gecko.${gecko_ffi_name}.data_at_mut(${x_index})); - v.0.height().to_gecko_style_coord(&mut self.gecko.${gecko_ffi_name}.data_at_mut(${y_index})); + self.gecko.${gecko_ffi_name}.${corner} = v; } #[allow(non_snake_case)] pub fn copy_${ident}_from(&mut self, other: &Self) { - self.gecko.${gecko_ffi_name}.data_at_mut(${x_index}) - .copy_from(&other.gecko.${gecko_ffi_name}.data_at(${x_index})); - self.gecko.${gecko_ffi_name}.data_at_mut(${y_index}) - .copy_from(&other.gecko.${gecko_ffi_name}.data_at(${y_index})); + self.gecko.${gecko_ffi_name}.${corner} = + other.gecko.${gecko_ffi_name}.${corner}; } #[allow(non_snake_case)] pub fn reset_${ident}(&mut self, other: &Self) { self.copy_${ident}_from(other) } - #[allow(non_snake_case)] pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T { - use crate::values::computed::border::BorderCornerRadius; - let width = GeckoStyleCoordConvertible::from_gecko_style_coord( - &self.gecko.${gecko_ffi_name}.data_at(${x_index})) - .expect("Failed to clone ${ident}"); - let height = GeckoStyleCoordConvertible::from_gecko_style_coord( - &self.gecko.${gecko_ffi_name}.data_at(${y_index})) - .expect("Failed to clone ${ident}"); - BorderCornerRadius::new(width, height) + self.gecko.${gecko_ffi_name}.${corner} } @@ -1387,14 +1376,6 @@ class Side(object): self.ident = name.lower() self.index = index -class Corner(object): - def __init__(self, vert, horiz, index): - self.x_name = "HalfCorner::eCorner" + vert + horiz + "X" - self.y_name = "HalfCorner::eCorner" + vert + horiz + "Y" - self.ident = (vert + "_" + horiz).lower() - self.x_index = 2 * index - self.y_index = 2 * index + 1 - class GridLine(object): def __init__(self, name): self.ident = "grid-" + name.lower() @@ -1402,19 +1383,12 @@ class GridLine(object): self.gecko = "m" + to_camel_case(self.ident) SIDES = [Side("Top", 0), Side("Right", 1), Side("Bottom", 2), Side("Left", 3)] -CORNERS = [Corner("Top", "Left", 0), Corner("Top", "Right", 1), - Corner("Bottom", "Right", 2), Corner("Bottom", "Left", 3)] +CORNERS = ["top_left", "top_right", "bottom_right", "bottom_left"] GRID_LINES = map(GridLine, ["row-start", "row-end", "column-start", "column-end"]) %> #[allow(dead_code)] fn static_assert() { - unsafe { - % for corner in CORNERS: - transmute::<_, [u32; ${corner.x_index}]>([1; structs::${corner.x_name} as usize]); - transmute::<_, [u32; ${corner.y_index}]>([1; structs::${corner.y_name} as usize]); - % endfor - } // Note: using the above technique with an enum hits a rust bug when |structs| is in a different crate. % for side in SIDES: { const DETAIL: u32 = [0][(structs::Side::eSide${side.name} as usize != ${side.index}) as usize]; let _ = DETAIL; } @@ -1425,7 +1399,7 @@ fn static_assert() { <% skip_border_longhands = " ".join(["border-{0}-{1}".format(x.ident, y) for x in SIDES for y in ["color", "style", "width"]] + - ["border-{0}-radius".format(x.ident.replace("_", "-")) + ["border-{0}-radius".format(x.replace("_", "-")) for x in CORNERS]) %> <%self:impl_trait style_struct_name="Border" @@ -1494,10 +1468,9 @@ fn static_assert() { % endfor % for corner in CORNERS: - <% impl_corner_style_coord("border_%s_radius" % corner.ident, + <% impl_corner_style_coord("border_%s_radius" % corner, "mBorderRadius", - corner.x_index, - corner.y_index) %> + corner) %> % endfor pub fn set_border_image_source(&mut self, image: longhands::border_image_source::computed_value::T) { @@ -2027,7 +2000,7 @@ fn static_assert() { <% skip_outline_longhands = " ".join("outline-style outline-width".split() + - ["-moz-outline-radius-{0}".format(x.ident.replace("_", "")) + ["-moz-outline-radius-{0}".format(x.replace("_", "")) for x in CORNERS]) %> <%self:impl_trait style_struct_name="Outline" skip_longhands="${skip_outline_longhands}"> @@ -2059,10 +2032,9 @@ fn static_assert() { round_to_pixels=True) %> % for corner in CORNERS: - <% impl_corner_style_coord("_moz_outline_radius_%s" % corner.ident.replace("_", ""), + <% impl_corner_style_coord("_moz_outline_radius_%s" % corner.replace("_", ""), "mOutlineRadius", - corner.x_index, - corner.y_index) %> + corner) %> % endfor pub fn outline_has_nonzero_width(&self) -> bool { @@ -4593,7 +4565,6 @@ fn set_style_svg_path( use crate::gecko_bindings::bindings::{Gecko_NewBasicShape, Gecko_DestroyShapeSource}; use crate::gecko_bindings::structs::{StyleBasicShape, StyleBasicShapeType, StyleShapeSourceType}; use crate::gecko_bindings::structs::{StyleGeometryBox, StyleShapeSource}; - use crate::gecko::conversions::basic_shape::set_corners_from_radius; use crate::gecko::values::GeckoStyleCoordConvertible; use crate::values::generics::basic_shape::{BasicShape, ShapeSource}; @@ -4658,8 +4629,10 @@ fn set_style_svg_path( inset.rect.2.to_gecko_style_coord(&mut shape.mCoordinates[2]); shape.mCoordinates[3].leaky_set_null(); inset.rect.3.to_gecko_style_coord(&mut shape.mCoordinates[3]); - - set_corners_from_radius(inset.round, &mut shape.mRadius); + shape.mRadius = match inset.round { + Some(radius) => radius, + None => crate::values::computed::BorderRadius::zero(), + }; } BasicShape::Circle(circ) => { let shape = init_shape(${ident}, StyleBasicShapeType::Circle); diff --git a/components/style/values/computed/border.rs b/components/style/values/computed/border.rs index 7b42dd0e879..0d4862ee469 100644 --- a/components/style/values/computed/border.rs +++ b/components/style/values/computed/border.rs @@ -87,6 +87,16 @@ impl BorderCornerRadius { } impl BorderRadius { + /// Returns a `0` border radius. + pub fn zero() -> Self { + Self { + top_left: BorderCornerRadius::zero(), + top_right: BorderCornerRadius::zero(), + bottom_right: BorderCornerRadius::zero(), + bottom_left: BorderCornerRadius::zero(), + } + } + /// Returns whether all the values are `0px`. pub fn all_zero(&self) -> bool { fn all(corner: &BorderCornerRadius) -> bool { diff --git a/components/style/values/generics/border.rs b/components/style/values/generics/border.rs index 148f74e3834..bfc7ac5814a 100644 --- a/components/style/values/generics/border.rs +++ b/components/style/values/generics/border.rs @@ -53,7 +53,10 @@ pub use self::GenericBorderImageSlice as BorderImageSlice; ToComputedValue, ToCss, )] -pub struct BorderCornerRadius(#[css(field_bound)] pub Size2D); +#[repr(C)] +pub struct GenericBorderCornerRadius(#[css(field_bound)] pub Size2D); + +pub use self::GenericBorderCornerRadius as BorderCornerRadius; impl BorderCornerRadius { /// Trivially create a `BorderCornerRadius`. @@ -77,6 +80,7 @@ impl BorderCornerRadius { ToComputedValue, ToCss, )] +#[repr(transparent)] pub struct BorderSpacing(#[css(field_bound)] pub Size2D); impl BorderSpacing { @@ -101,17 +105,20 @@ impl BorderSpacing { ToAnimatedValue, ToComputedValue, )] -pub struct BorderRadius { +#[repr(C)] +pub struct GenericBorderRadius { /// The top left radius. - pub top_left: BorderCornerRadius, + pub top_left: GenericBorderCornerRadius, /// The top right radius. - pub top_right: BorderCornerRadius, + pub top_right: GenericBorderCornerRadius, /// The bottom right radius. - pub bottom_right: BorderCornerRadius, + pub bottom_right: GenericBorderCornerRadius, /// The bottom left radius. - pub bottom_left: BorderCornerRadius, + pub bottom_left: GenericBorderCornerRadius, } +pub use self::GenericBorderRadius as BorderRadius; + impl BorderRadius { /// Returns a new `BorderRadius`. #[inline] diff --git a/components/style/values/generics/size.rs b/components/style/values/generics/size.rs index c67d6e82781..28c1ed5a36b 100644 --- a/components/style/values/generics/size.rs +++ b/components/style/values/generics/size.rs @@ -25,6 +25,7 @@ use style_traits::{CssWriter, ParseError, ToCss}; ToComputedValue, )] #[allow(missing_docs)] +#[repr(C)] pub struct Size2D { pub width: L, pub height: L, From 52db71c0bf6d6e5bb65273b9add31a19f7658ed2 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Tue, 26 Feb 2019 10:17:25 +0000 Subject: [PATCH 04/31] style: Explicitly specify animation_value_type for scroll-padding. Otherwise the Trait for clamping negative animation value isn't generated thus negative animating results are exposed in computed values. Differential Revision: https://phabricator.services.mozilla.com/D21153 --- components/style/properties/longhands/padding.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/longhands/padding.mako.rs b/components/style/properties/longhands/padding.mako.rs index 6c4e2b32cf9..a1262aee0fc 100644 --- a/components/style/properties/longhands/padding.mako.rs +++ b/components/style/properties/longhands/padding.mako.rs @@ -39,6 +39,6 @@ logical=side[1], logical_group="scroll-padding", spec="https://drafts.csswg.org/css-scroll-snap-1/#propdef-scroll-padding-%s" % side[0], - animation_value_type="ComputedValue", + animation_value_type="NonNegativeLengthPercentageOrAuto", )} % endfor From 4496411edc09d3f3b2c4287ab316f1c2921142af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Feb 2019 23:34:25 +0000 Subject: [PATCH 05/31] style: Fix computed value clamping. Differential Revision: https://phabricator.services.mozilla.com/D21268 --- components/style/values/computed/length.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index f578bbbd5ce..23b7474e778 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -203,12 +203,7 @@ impl LengthPercentage { return None; } - if self.clamping_mode.clamp(self.percentage.0) != self.percentage.0 { - debug_assert!(self.was_calc); - return None; - } - - Some(self.percentage) + Some(Percentage(self.clamping_mode.clamp(self.percentage.0))) } /// Convert the computed value into used value. @@ -433,14 +428,13 @@ impl ToComputedValue for specified::LengthPercentage { } fn from_computed_value(computed: &LengthPercentage) -> Self { - let length = computed.unclamped_length(); if let Some(p) = computed.as_percentage() { return specified::LengthPercentage::Percentage(p); } - if !computed.has_percentage && computed.clamping_mode.clamp(length.px()) == length.px() { + if !computed.has_percentage { return specified::LengthPercentage::Length(ToComputedValue::from_computed_value( - &length, + &computed.length(), )); } From 7d01114cbfe7e3f991521317cd4499df0507f35c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 28 Feb 2019 19:03:03 +0000 Subject: [PATCH 06/31] style: Add a Zero trait that doesn't require Add, and use it in place of num_traits and IsZeroLength. Use it to be consistent in InsetRect serialization and storage between Servo and Gecko. Differential Revision: https://phabricator.services.mozilla.com/D21493 --- components/style/gecko/conversions.rs | 2 +- components/style/gecko/values.rs | 2 +- .../gecko_bindings/sugar/ns_css_value.rs | 1 + components/style/lib.rs | 23 +++++ .../media_queries/media_feature_expression.rs | 3 +- components/style/properties/gecko.mako.rs | 5 +- components/style/properties/helpers.mako.rs | 2 + .../style/properties/properties.mako.rs | 5 +- components/style/values/animated/transform.rs | 2 +- components/style/values/computed/angle.rs | 2 +- components/style/values/computed/border.rs | 37 +------- components/style/values/computed/length.rs | 44 +++------- .../style/values/computed/percentage.rs | 23 +++-- components/style/values/computed/position.rs | 1 + components/style/values/computed/svg.rs | 1 + components/style/values/computed/transform.rs | 2 +- .../style/values/generics/basic_shape.rs | 9 +- components/style/values/generics/border.rs | 35 ++++++-- components/style/values/generics/length.rs | 32 +++++-- components/style/values/generics/mod.rs | 2 +- components/style/values/generics/size.rs | 11 +++ components/style/values/generics/transform.rs | 15 +--- components/style/values/specified/angle.rs | 11 ++- .../style/values/specified/basic_shape.rs | 5 +- components/style/values/specified/border.rs | 1 + components/style/values/specified/effects.rs | 1 + components/style/values/specified/length.rs | 86 +++++++------------ components/style/values/specified/position.rs | 1 + .../style/values/specified/transform.rs | 1 + 29 files changed, 179 insertions(+), 186 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 4cbf759c2d8..64f0726b21e 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -27,6 +27,7 @@ use crate::values::generics::box_::VerticalAlign; use crate::values::generics::grid::{TrackListValue, TrackSize}; use crate::values::generics::image::{CompatMode, GradientItem, Image as GenericImage}; use crate::values::generics::rect::Rect; +use crate::Zero; use app_units::Au; use std::f32::consts::PI; use style_traits::values::specified::AllowedNumericType; @@ -703,7 +704,6 @@ pub mod basic_shape { let b = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[2]); let l = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[3]); let round = other.mRadius; - let round = if round.all_zero() { None } else { Some(round) }; let rect = Rect::new( t.expect("inset() offset should be a length, percentage, or calc value"), r.expect("inset() offset should be a length, percentage, or calc value"), diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index fa1c7d1f6d3..6b3e7a676e0 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -19,7 +19,7 @@ use crate::values::generics::grid::{TrackBreadth, TrackKeyword}; use crate::values::generics::length::LengthPercentageOrAuto; use crate::values::generics::{CounterStyleOrNone, NonNegative}; use crate::values::{Auto, Either, None_, Normal}; -use crate::Atom; +use crate::{Atom, Zero}; use app_units::Au; use cssparser::RGBA; use nsstring::{nsACString, nsCStr}; diff --git a/components/style/gecko_bindings/sugar/ns_css_value.rs b/components/style/gecko_bindings/sugar/ns_css_value.rs index ef971c669ad..b59732bcc8e 100644 --- a/components/style/gecko_bindings/sugar/ns_css_value.rs +++ b/components/style/gecko_bindings/sugar/ns_css_value.rs @@ -10,6 +10,7 @@ use crate::gecko_bindings::structs::{nsCSSUnit, nsCSSValue}; use crate::gecko_bindings::structs::{nsCSSValueList, nsCSSValue_Array}; use crate::gecko_string_cache::Atom; use crate::values::computed::{Angle, Length, LengthPercentage, Percentage}; +use crate::Zero; use std::marker::PhantomData; use std::mem; use std::ops::{Index, IndexMut}; diff --git a/components/style/lib.rs b/components/style/lib.rs index 2ea20088ed0..37b464afb47 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -244,3 +244,26 @@ impl CaseSensitivityExt for selectors::attr::CaseSensitivity { } } } + +/// A trait pretty much similar to num_traits::Zero, but without the need of +/// implementing `Add`. +pub trait Zero { + /// Returns the zero value. + fn zero() -> Self; + + /// Returns whether this value is zero. + fn is_zero(&self) -> bool; +} + +impl Zero for T +where + T: num_traits::Zero, +{ + fn zero() -> Self { + ::zero() + } + + fn is_zero(&self) -> bool { + ::is_zero(self) + } +} diff --git a/components/style/media_queries/media_feature_expression.rs b/components/style/media_queries/media_feature_expression.rs index ec864510c72..6a9db93e889 100644 --- a/components/style/media_queries/media_feature_expression.rs +++ b/components/style/media_queries/media_feature_expression.rs @@ -21,9 +21,8 @@ use crate::stylesheets::Origin; use crate::values::computed::{self, ToComputedValue}; use crate::values::specified::{Integer, Length, Number, Resolution}; use crate::values::{serialize_atom_identifier, CSSFloat}; -use crate::Atom; +use crate::{Atom, Zero}; use cssparser::{Parser, Token}; -use num_traits::Zero; use std::cmp::{Ordering, PartialOrd}; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 65ee8bc1053..ecad0b419d5 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -4629,10 +4629,7 @@ fn set_style_svg_path( inset.rect.2.to_gecko_style_coord(&mut shape.mCoordinates[2]); shape.mCoordinates[3].leaky_set_null(); inset.rect.3.to_gecko_style_coord(&mut shape.mCoordinates[3]); - shape.mRadius = match inset.round { - Some(radius) => radius, - None => crate::values::computed::BorderRadius::zero(), - }; + shape.mRadius = inset.round; } BasicShape::Circle(circ) => { let shape = init_shape(${ident}, StyleBasicShapeType::Circle); diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 59c9785292c..1129a36908d 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -19,6 +19,8 @@ #[allow(unused_imports)] use crate::values::specified::AllowQuirks; #[allow(unused_imports)] + use crate::Zero; + #[allow(unused_imports)] use smallvec::SmallVec; pub use crate::values::specified::${type} as SpecifiedValue; pub mod computed_value { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 7e35d318c30..ec859a42ea7 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -42,6 +42,7 @@ use crate::values::computed; use crate::values::computed::NonNegativeLength; use crate::values::serialize_atom_name; use crate::rule_tree::StrongRuleNode; +use crate::Zero; use self::computed_value_flags::*; use crate::str::{CssString, CssStringBorrow, CssStringWriter}; @@ -2585,7 +2586,7 @@ pub mod style_structs { /// Whether the border-${side} property has nonzero width. #[allow(non_snake_case)] pub fn border_${side}_has_nonzero_width(&self) -> bool { - self.border_${side}_width != NonNegativeLength::zero() + !self.border_${side}_width.is_zero() } % endfor % elif style_struct.name == "Font": @@ -2624,7 +2625,7 @@ pub mod style_structs { /// Whether the outline-width property is non-zero. #[inline] pub fn outline_has_nonzero_width(&self) -> bool { - self.outline_width != NonNegativeLength::zero() + !self.outline_width.is_zero() } % elif style_struct.name == "Text": /// Whether the text decoration has an underline. diff --git a/components/style/values/animated/transform.rs b/components/style/values/animated/transform.rs index c341ca770ed..507c4ba4671 100644 --- a/components/style/values/animated/transform.rs +++ b/components/style/values/animated/transform.rs @@ -22,7 +22,7 @@ use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; use crate::values::generics::transform::{self, Transform, TransformOperation}; use crate::values::generics::transform::{Rotate, Scale, Translate}; use crate::values::CSSFloat; -use num_traits::Zero; +use crate::Zero; use std::cmp; // ------------------------------------ diff --git a/components/style/values/computed/angle.rs b/components/style/values/computed/angle.rs index d30f02d7200..76e44575dfb 100644 --- a/components/style/values/computed/angle.rs +++ b/components/style/values/computed/angle.rs @@ -6,7 +6,7 @@ use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; use crate::values::CSSFloat; -use num_traits::Zero; +use crate::Zero; use std::f64::consts::PI; use std::fmt::{self, Write}; use std::{f32, f64}; diff --git a/components/style/values/computed/border.rs b/components/style/values/computed/border.rs index 0d4862ee469..62c8e3883f6 100644 --- a/components/style/values/computed/border.rs +++ b/components/style/values/computed/border.rs @@ -14,6 +14,7 @@ use crate::values::generics::border::BorderSpacing as GenericBorderSpacing; use crate::values::generics::rect::Rect; use crate::values::generics::size::Size2D; use crate::values::generics::NonNegative; +use crate::Zero; use app_units::Au; pub use crate::values::specified::border::BorderImageRepeat; @@ -75,39 +76,3 @@ impl BorderSpacing { Au::from(*self.0.height()) } } - -impl BorderCornerRadius { - /// Returns `0 0`. - pub fn zero() -> Self { - GenericBorderCornerRadius(Size2D::new( - NonNegativeLengthPercentage::zero(), - NonNegativeLengthPercentage::zero(), - )) - } -} - -impl BorderRadius { - /// Returns a `0` border radius. - pub fn zero() -> Self { - Self { - top_left: BorderCornerRadius::zero(), - top_right: BorderCornerRadius::zero(), - bottom_right: BorderCornerRadius::zero(), - bottom_left: BorderCornerRadius::zero(), - } - } - - /// Returns whether all the values are `0px`. - pub fn all_zero(&self) -> bool { - fn all(corner: &BorderCornerRadius) -> bool { - fn is_zero(l: &NonNegativeLengthPercentage) -> bool { - *l == NonNegativeLengthPercentage::zero() - } - is_zero(corner.0.width()) && is_zero(corner.0.height()) - } - all(&self.top_left) && - all(&self.top_right) && - all(&self.bottom_left) && - all(&self.bottom_right) - } -} diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 23b7474e778..38cecd7339d 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -12,11 +12,11 @@ use crate::values::generics::length as generics; use crate::values::generics::length::{ GenericLengthOrNumber, MaxSize as GenericMaxSize, Size as GenericSize, }; -use crate::values::generics::transform::IsZeroLength; use crate::values::generics::NonNegative; use crate::values::specified::length::ViewportPercentageLength; use crate::values::specified::length::{AbsoluteLength, FontBaseSize, FontRelativeLength}; use crate::values::{specified, Auto, CSSFloat, Either, Normal}; +use crate::Zero; use app_units::Au; use ordered_float::NotNan; use std::fmt::{self, Write}; @@ -342,12 +342,6 @@ impl ToComputedValue for specified::CalcLengthPercentage { } impl LengthPercentage { - #[inline] - #[allow(missing_docs)] - pub fn zero() -> LengthPercentage { - LengthPercentage::new(Length::new(0.), None) - } - /// 1px length value for SVG defaults #[inline] pub fn one() -> LengthPercentage { @@ -442,9 +436,13 @@ impl ToComputedValue for specified::LengthPercentage { } } -impl IsZeroLength for LengthPercentage { +impl Zero for LengthPercentage { + fn zero() -> Self { + LengthPercentage::new(Length::zero(), None) + } + #[inline] - fn is_zero_length(&self) -> bool { + fn is_zero(&self) -> bool { self.is_definitely_zero() } } @@ -453,12 +451,6 @@ impl IsZeroLength for LengthPercentage { /// length-percentage or auto. macro_rules! computed_length_percentage_or_auto { ($inner:ty) => { - /// Returns the `0` value. - #[inline] - pub fn zero() -> Self { - generics::LengthPercentageOrAuto::LengthPercentage(<$inner>::zero()) - } - /// Returns the used value. #[inline] pub fn to_used_value(&self, percentage_basis: Au) -> Option { @@ -547,12 +539,6 @@ impl From for LengthPercentage { } impl NonNegativeLengthPercentage { - /// Get zero value. - #[inline] - pub fn zero() -> Self { - NonNegative(LengthPercentage::zero()) - } - /// Returns true if the computed value is absolute 0 or 0%. #[inline] pub fn is_definitely_zero(&self) -> bool { @@ -656,12 +642,16 @@ impl CSSPixelLength { pub fn clamp_to_non_negative(self) -> Self { CSSPixelLength::new(self.0.max(0.)) } +} - /// Zero value - #[inline] - pub fn zero() -> Self { +impl Zero for CSSPixelLength { + fn zero() -> Self { CSSPixelLength::new(0.) } + + fn is_zero(&self) -> bool { + self.px() == 0. + } } impl ToCss for CSSPixelLength { @@ -743,12 +733,6 @@ impl NonNegativeLength { NonNegative(Length::new(px.max(0.))) } - /// Return a zero value. - #[inline] - pub fn zero() -> Self { - Self::new(0.) - } - /// Return the pixel value of |NonNegativeLength|. #[inline] pub fn px(&self) -> CSSFloat { diff --git a/components/style/values/computed/percentage.rs b/components/style/values/computed/percentage.rs index 67bdda76033..4f39a7a7274 100644 --- a/components/style/values/computed/percentage.rs +++ b/components/style/values/computed/percentage.rs @@ -7,6 +7,7 @@ use crate::values::animated::ToAnimatedValue; use crate::values::generics::NonNegative; use crate::values::{serialize_percentage, CSSFloat}; +use crate::Zero; use std::fmt; use style_traits::{CssWriter, ToCss}; @@ -31,12 +32,6 @@ use style_traits::{CssWriter, ToCss}; pub struct Percentage(pub CSSFloat); impl Percentage { - /// 0% - #[inline] - pub fn zero() -> Self { - Percentage(0.) - } - /// 100% #[inline] pub fn hundred() -> Self { @@ -56,6 +51,16 @@ impl Percentage { } } +impl Zero for Percentage { + fn zero() -> Self { + Percentage(0.) + } + + fn is_zero(&self) -> bool { + self.0 == 0. + } +} + impl ToCss for Percentage { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where @@ -69,12 +74,6 @@ impl ToCss for Percentage { pub type NonNegativePercentage = NonNegative; impl NonNegativePercentage { - /// 0% - #[inline] - pub fn zero() -> Self { - NonNegative(Percentage::zero()) - } - /// 100% #[inline] pub fn hundred() -> Self { diff --git a/components/style/values/computed/position.rs b/components/style/values/computed/position.rs index d6f9161903b..f0f8fe198a6 100644 --- a/components/style/values/computed/position.rs +++ b/components/style/values/computed/position.rs @@ -11,6 +11,7 @@ use crate::values::computed::{Integer, LengthPercentage, Percentage}; use crate::values::generics::position::Position as GenericPosition; use crate::values::generics::position::ZIndex as GenericZIndex; pub use crate::values::specified::position::{GridAutoFlow, GridTemplateAreas}; +use crate::Zero; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; diff --git a/components/style/values/computed/svg.rs b/components/style/values/computed/svg.rs index f3ca86aa5b0..3d47df32ab8 100644 --- a/components/style/values/computed/svg.rs +++ b/components/style/values/computed/svg.rs @@ -10,6 +10,7 @@ use crate::values::computed::{LengthPercentage, NonNegativeLengthPercentage}; use crate::values::computed::{NonNegativeNumber, Number, Opacity}; use crate::values::generics::svg as generic; use crate::values::RGBA; +use crate::Zero; pub use crate::values::specified::SVGPaintOrder; diff --git a/components/style/values/computed/transform.rs b/components/style/values/computed/transform.rs index dd3fe20754c..f3b40c79f9e 100644 --- a/components/style/values/computed/transform.rs +++ b/components/style/values/computed/transform.rs @@ -9,8 +9,8 @@ use crate::values::animated::transform::{Perspective, Scale3D, Translate3D}; use crate::values::animated::ToAnimatedZero; use crate::values::computed::{Angle, Integer, Length, LengthPercentage, Number, Percentage}; use crate::values::generics::transform as generic; +use crate::Zero; use euclid::{Transform3D, Vector3D}; -use num_traits::Zero; pub use crate::values::generics::transform::TransformStyle; diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index f224f936ccb..975fbdb797d 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -11,6 +11,7 @@ use crate::values::generics::border::BorderRadius; use crate::values::generics::position::Position; use crate::values::generics::rect::Rect; use crate::values::specified::SVGPathData; +use crate::Zero; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; @@ -127,7 +128,7 @@ pub enum BasicShape { )] pub struct InsetRect { pub rect: Rect, - pub round: Option>, + pub round: BorderRadius, } /// @@ -311,7 +312,7 @@ impl ToAnimatedZero for ShapeSource { impl ToCss for InsetRect where Length: ToCss + PartialEq, - NonNegativeLength: ToCss + PartialEq, + NonNegativeLength: ToCss + PartialEq + Zero, { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where @@ -319,9 +320,9 @@ where { dest.write_str("inset(")?; self.rect.to_css(dest)?; - if let Some(ref radius) = self.round { + if !self.round.is_zero() { dest.write_str(" round ")?; - radius.to_css(dest)?; + self.round.to_css(dest)?; } dest.write_str(")") } diff --git a/components/style/values/generics/border.rs b/components/style/values/generics/border.rs index bfc7ac5814a..df6427e888f 100644 --- a/components/style/values/generics/border.rs +++ b/components/style/values/generics/border.rs @@ -6,6 +6,7 @@ use crate::values::generics::rect::Rect; use crate::values::generics::size::Size2D; +use crate::Zero; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; @@ -65,6 +66,16 @@ impl BorderCornerRadius { } } +impl Zero for BorderCornerRadius { + fn zero() -> Self { + BorderCornerRadius(Size2D::zero()) + } + + fn is_zero(&self) -> bool { + self.0.is_zero() + } +} + /// A generic value for the `border-spacing` property. #[derive( Animate, @@ -135,12 +146,7 @@ impl BorderRadius { bottom_left: bl, } } -} -impl BorderRadius -where - L: PartialEq + ToCss, -{ /// Serialises two given rects following the syntax of the `border-radius`` /// property. pub fn serialize_rects( @@ -149,6 +155,7 @@ where dest: &mut CssWriter, ) -> fmt::Result where + L: PartialEq + ToCss, W: Write, { widths.to_css(dest)?; @@ -160,6 +167,24 @@ where } } +impl Zero for BorderRadius { + fn zero() -> Self { + Self::new( + BorderCornerRadius::::zero(), + BorderCornerRadius::::zero(), + BorderCornerRadius::::zero(), + BorderCornerRadius::::zero(), + ) + } + + fn is_zero(&self) -> bool { + self.top_left.is_zero() && + self.top_right.is_zero() && + self.bottom_right.is_zero() && + self.bottom_left.is_zero() + } +} + impl ToCss for BorderRadius where L: PartialEq + ToCss, diff --git a/components/style/values/generics/length.rs b/components/style/values/generics/length.rs index 1a83f58054c..076a02a4a0f 100644 --- a/components/style/values/generics/length.rs +++ b/components/style/values/generics/length.rs @@ -7,8 +7,8 @@ use crate::parser::{Parse, ParserContext}; #[cfg(feature = "gecko")] use crate::values::computed::ExtremumLength; +use crate::Zero; use cssparser::Parser; -use num_traits::Zero; use style_traits::ParseError; /// A ` | auto` value. @@ -67,6 +67,19 @@ impl LengthPercentageOrAuto { } } +impl Zero for LengthPercentageOrAuto { + fn zero() -> Self { + LengthPercentageOrAuto::LengthPercentage(Zero::zero()) + } + + fn is_zero(&self) -> bool { + match *self { + LengthPercentageOrAuto::LengthPercentage(ref l) => l.is_zero(), + LengthPercentageOrAuto::Auto => false, + } + } +} + impl Parse for LengthPercentageOrAuto { fn parse<'i, 't>( context: &ParserContext, @@ -186,12 +199,15 @@ pub enum GenericLengthOrNumber { pub use self::GenericLengthOrNumber as LengthOrNumber; -impl LengthOrNumber { - /// Returns `0`. - pub fn zero() -> Self - where - N: Zero, - { - LengthOrNumber::Number(num_traits::Zero::zero()) +impl Zero for LengthOrNumber { + fn zero() -> Self { + LengthOrNumber::Number(Zero::zero()) + } + + fn is_zero(&self) -> bool { + match *self { + LengthOrNumber::Number(ref n) => n.is_zero(), + LengthOrNumber::Length(..) => false, + } } } diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index 737c88e7ba7..be1c10a5ee4 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -8,8 +8,8 @@ use super::CustomIdent; use crate::counter_style::{parse_counter_style_name, Symbols}; use crate::parser::{Parse, ParserContext}; +use crate::Zero; use cssparser::Parser; -use num_traits::Zero; use std::ops::Add; use style_traits::{KeywordsCollectFn, ParseError}; use style_traits::{SpecifiedValueInfo, StyleParseErrorKind}; diff --git a/components/style/values/generics/size.rs b/components/style/values/generics/size.rs index 28c1ed5a36b..96e0003128f 100644 --- a/components/style/values/generics/size.rs +++ b/components/style/values/generics/size.rs @@ -5,6 +5,7 @@ //! Generic type for CSS properties that are composed by two dimensions. use crate::parser::ParserContext; +use crate::Zero; use cssparser::Parser; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, ToCss}; @@ -84,3 +85,13 @@ where Ok(()) } } + +impl Zero for Size2D { + fn zero() -> Self { + Self::new(L::zero(), L::zero()) + } + + fn is_zero(&self) -> bool { + self.width.is_zero() && self.height.is_zero() + } +} diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index 6c60e76cbf6..d25d2d3a653 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -10,9 +10,9 @@ use crate::values::specified::angle::Angle as SpecifiedAngle; use crate::values::specified::length::Length as SpecifiedLength; use crate::values::specified::length::LengthPercentage as SpecifiedLengthPercentage; use crate::values::{computed, CSSFloat}; +use crate::Zero; use app_units::Au; use euclid::{self, Rect, Transform3D}; -use num_traits::Zero; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; @@ -643,16 +643,7 @@ pub enum Translate { Translate3D(LengthPercentage, LengthPercentage, Length), } -/// A trait to check if this is a zero length. -/// An alternative way is use num_traits::Zero. However, in order to implement num_traits::Zero, -/// we also have to implement Add, which may be complicated for LengthPercentage::Calc. -/// We could do this if other types also need it. If so, we could drop this trait. -pub trait IsZeroLength { - /// Returns true if this is a zero length. - fn is_zero_length(&self) -> bool; -} - -impl ToCss for Translate { +impl ToCss for Translate { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write, @@ -670,7 +661,7 @@ impl ToCss for Translate { Translate::None => dest.write_str("none"), Translate::Translate(ref x, ref y) => { x.to_css(dest)?; - if !y.is_zero_length() { + if !y.is_zero() { dest.write_char(' ')?; y.to_css(dest)?; } diff --git a/components/style/values/specified/angle.rs b/components/style/values/specified/angle.rs index 35a0944aed5..98233608ebb 100644 --- a/components/style/values/specified/angle.rs +++ b/components/style/values/specified/angle.rs @@ -101,6 +101,11 @@ impl Angle { } } + /// Return `0deg`. + pub fn zero() -> Self { + Self::from_degrees(0.0, false) + } + /// Returns the value of the angle in degrees, mostly for `calc()`. #[inline] pub fn degrees(&self) -> CSSFloat { @@ -113,12 +118,6 @@ impl Angle { self.was_calc } - /// Returns `0deg`. - #[inline] - pub fn zero() -> Self { - Self::from_degrees(0.0, false) - } - /// Returns an `Angle` parsed from a `calc()` expression. pub fn from_calc(degrees: CSSFloat) -> Self { Angle { diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index b757c8eec48..014ce439d3c 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -18,6 +18,7 @@ use crate::values::specified::position::{HorizontalPosition, Position, VerticalP use crate::values::specified::url::SpecifiedUrl; use crate::values::specified::SVGPathData; use crate::values::specified::{LengthPercentage, NonNegativeLengthPercentage}; +use crate::Zero; use cssparser::Parser; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; @@ -202,9 +203,9 @@ impl InsetRect { ) -> Result> { let rect = Rect::parse_with(context, input, LengthPercentage::parse)?; let round = if input.try(|i| i.expect_ident_matching("round")).is_ok() { - Some(BorderRadius::parse(context, input)?) + BorderRadius::parse(context, input)? } else { - None + BorderRadius::zero() }; Ok(generic::InsetRect { rect, round }) } diff --git a/components/style/values/specified/border.rs b/components/style/values/specified/border.rs index 0b1e9026b8e..d0ef8126482 100644 --- a/components/style/values/specified/border.rs +++ b/components/style/values/specified/border.rs @@ -15,6 +15,7 @@ use crate::values::generics::rect::Rect; use crate::values::generics::size::Size2D; use crate::values::specified::length::{NonNegativeLength, NonNegativeLengthPercentage}; use crate::values::specified::{AllowQuirks, NonNegativeNumber, NonNegativeNumberOrPercentage}; +use crate::Zero; use cssparser::Parser; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, ToCss}; diff --git a/components/style/values/specified/effects.rs b/components/style/values/specified/effects.rs index 42ec0ce0005..38a53749605 100644 --- a/components/style/values/specified/effects.rs +++ b/components/style/values/specified/effects.rs @@ -20,6 +20,7 @@ use crate::values::specified::url::SpecifiedUrl; use crate::values::specified::{Angle, NumberOrPercentage}; #[cfg(not(feature = "gecko"))] use crate::values::Impossible; +use crate::Zero; use cssparser::{self, BasicParseErrorKind, Parser, Token}; use style_traits::{ParseError, StyleParseErrorKind, ValueParseErrorKind}; diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index ffdd8d026ef..102bffab658 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -14,11 +14,11 @@ use crate::values::generics::length as generics; use crate::values::generics::length::{ GenericLengthOrNumber, MaxSize as GenericMaxSize, Size as GenericSize, }; -use crate::values::generics::transform::IsZeroLength; use crate::values::generics::NonNegative; use crate::values::specified::calc::CalcNode; use crate::values::specified::NonNegativeNumber; use crate::values::{Auto, CSSFloat, Either, Normal}; +use crate::Zero; use app_units::Au; use cssparser::{Parser, Token}; use euclid::Size2D; @@ -477,21 +477,6 @@ impl NoCalcLength { }) } - #[inline] - /// Returns a `zero` length. - pub fn zero() -> NoCalcLength { - NoCalcLength::Absolute(AbsoluteLength::Px(0.)) - } - - #[inline] - /// Checks whether the length value is zero. - pub fn is_zero(&self) -> bool { - match *self { - NoCalcLength::Absolute(length) => length.is_zero(), - _ => false, - } - } - /// Get a px value without context. #[inline] pub fn to_computed_pixel_length_without_context(&self) -> Result { @@ -510,9 +495,12 @@ impl NoCalcLength { impl SpecifiedValueInfo for NoCalcLength {} -impl IsZeroLength for NoCalcLength { - #[inline] - fn is_zero_length(&self) -> bool { +impl Zero for NoCalcLength { + fn zero() -> Self { + NoCalcLength::Absolute(AbsoluteLength::Px(0.)) + } + + fn is_zero(&self) -> bool { match *self { NoCalcLength::Absolute(v) => v.is_zero(), NoCalcLength::FontRelative(v) => v.is_zero(), @@ -584,12 +572,6 @@ impl Mul for ViewportPercentageLength { } impl Length { - #[inline] - /// Returns a `zero` length. - pub fn zero() -> Length { - Length::NoCalc(NoCalcLength::zero()) - } - #[inline] fn parse_internal<'i, 't>( context: &ParserContext, @@ -672,6 +654,21 @@ impl Parse for Length { } } +impl Zero for Length { + fn zero() -> Self { + Length::NoCalc(NoCalcLength::zero()) + } + + fn is_zero(&self) -> bool { + // FIXME(emilio): Seems a bit weird to treat calc() unconditionally as + // non-zero here? + match *self { + Length::NoCalc(ref l) => l.is_zero(), + Length::Calc(..) => false, + } + } +} + impl Length { /// Parses a length, with quirks. pub fn parse_quirky<'i, 't>( @@ -711,12 +708,6 @@ impl From for NonNegativeLength { } impl NonNegativeLength { - /// Returns a `zero` length. - #[inline] - pub fn zero() -> Self { - Length::zero().into() - } - /// Get an absolute length from a px value. #[inline] pub fn from_px(px_value: CSSFloat) -> Self { @@ -801,12 +792,6 @@ impl Parse for LengthPercentage { } impl LengthPercentage { - #[inline] - /// Returns a `zero` length. - pub fn zero() -> LengthPercentage { - LengthPercentage::Length(NoCalcLength::zero()) - } - #[inline] /// Returns a `0%` value. pub fn zero_percent() -> LengthPercentage { @@ -898,11 +883,14 @@ impl LengthPercentage { } } -impl IsZeroLength for LengthPercentage { - #[inline] - fn is_zero_length(&self) -> bool { +impl Zero for LengthPercentage { + fn zero() -> Self { + LengthPercentage::Length(NoCalcLength::zero()) + } + + fn is_zero(&self) -> bool { match *self { - LengthPercentage::Length(l) => l.is_zero_length(), + LengthPercentage::Length(l) => l.is_zero(), LengthPercentage::Percentage(p) => p.0 == 0.0, LengthPercentage::Calc(_) => false, } @@ -913,11 +901,6 @@ impl IsZeroLength for LengthPercentage { pub type LengthPercentageOrAuto = generics::LengthPercentageOrAuto; impl LengthPercentageOrAuto { - /// Returns a value representing a `0` length. - pub fn zero() -> Self { - generics::LengthPercentageOrAuto::LengthPercentage(LengthPercentage::zero()) - } - /// Returns a value representing `0%`. #[inline] pub fn zero_percent() -> Self { @@ -943,11 +926,6 @@ pub type NonNegativeLengthPercentageOrAuto = generics::LengthPercentageOrAuto; impl NonNegativeLengthPercentageOrAuto { - /// Returns a value representing a `0` length. - pub fn zero() -> Self { - generics::LengthPercentageOrAuto::LengthPercentage(NonNegativeLengthPercentage::zero()) - } - /// Returns a value representing `0%`. #[inline] pub fn zero_percent() -> Self { @@ -997,12 +975,6 @@ impl Parse for NonNegativeLengthPercentage { } impl NonNegativeLengthPercentage { - #[inline] - /// Returns a `zero` length. - pub fn zero() -> Self { - NonNegative(LengthPercentage::zero()) - } - #[inline] /// Returns a `0%` value. pub fn zero_percent() -> Self { diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index 0212763ec6d..f60767f1656 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -17,6 +17,7 @@ use crate::values::generics::position::ZIndex as GenericZIndex; use crate::values::specified::transform::OriginComponent; use crate::values::specified::{AllowQuirks, Integer, LengthPercentage}; use crate::values::{Either, None_}; +use crate::Zero; use cssparser::Parser; use selectors::parser::SelectorParseErrorKind; use servo_arc::Arc; diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index c4f43735498..4758a0c4c05 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -11,6 +11,7 @@ use crate::values::generics::transform as generic; use crate::values::generics::transform::{Matrix, Matrix3D}; use crate::values::specified::position::{Side, X, Y}; use crate::values::specified::{self, Angle, Integer, Length, LengthPercentage, Number}; +use crate::Zero; use cssparser::Parser; use style_traits::{ParseError, StyleParseErrorKind}; From 9e0d38a64fcfc74b288e8eeec0c1a807e08670cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 1 Mar 2019 02:33:07 +0000 Subject: [PATCH 07/31] style: Make word-break: break-word behave like word-break: normal; overflow-wrap: anywhere. Differential Revision: https://phabricator.services.mozilla.com/D21398 --- components/style/cbindgen.toml | 1 + components/style/properties/data.py | 1 + components/style/properties/gecko.mako.rs | 1 + .../longhands/inherited_text.mako.rs | 8 +++--- components/style/values/computed/mod.rs | 2 +- components/style/values/computed/text.rs | 2 +- components/style/values/specified/mod.rs | 2 +- components/style/values/specified/text.rs | 26 +++++++++++++++++++ 8 files changed, 36 insertions(+), 7 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 82ed48ab290..205f5478316 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -95,6 +95,7 @@ include = [ "Perspective", "ZIndex", "TransformOrigin", + "WordBreak", ] item_types = ["enums", "structs", "typedefs"] diff --git a/components/style/properties/data.py b/components/style/properties/data.py index f3b4e43c225..9347d9b9428 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -347,6 +347,7 @@ class Longhand(object): "TouchAction", "TransformStyle", "UserSelect", + "WordBreak", "XSpan", "XTextZoom", "ZIndex", diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index ecad0b419d5..cca1bd540f9 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1328,6 +1328,7 @@ impl Clone for ${style_struct.gecko_struct_name} { "TransformOrigin": impl_simple, "UserSelect": impl_simple, "url::UrlOrNone": impl_css_url, + "WordBreak": impl_simple, "ZIndex": impl_simple, } diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs index 97d551120a4..f11a9a7b6b2 100644 --- a/components/style/properties/longhands/inherited_text.mako.rs +++ b/components/style/properties/longhands/inherited_text.mako.rs @@ -74,13 +74,13 @@ ${helpers.predefined_type( servo_restyle_damage="rebuild_and_reflow", )} -// TODO(pcwalton): Support `word-break: keep-all` once we have better CJK support. -${helpers.single_keyword( +${helpers.predefined_type( "word-break", - "normal break-all keep-all", - gecko_constant_prefix="NS_STYLE_WORDBREAK", + "WordBreak", + "computed::WordBreak::Normal", animation_value_type="discrete", spec="https://drafts.csswg.org/css-text/#propdef-word-break", + needs_context=False, servo_restyle_damage="rebuild_and_reflow", )} diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 6fe36d684f2..70d1e243f02 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -78,7 +78,7 @@ pub use self::svg::{SVGLength, SVGOpacity, SVGPaint, SVGPaintKind}; pub use self::svg::{SVGPaintOrder, SVGStrokeDashArray, SVGWidth}; pub use self::table::XSpan; pub use self::text::{InitialLetter, LetterSpacing, LineHeight}; -pub use self::text::{OverflowWrap, TextOverflow, WordSpacing}; +pub use self::text::{OverflowWrap, TextOverflow, WordSpacing, WordBreak}; pub use self::text::{TextAlign, TextEmphasisPosition, TextEmphasisStyle}; pub use self::time::Time; pub use self::transform::{Rotate, Scale, Transform, TransformOperation}; diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index 2c9a9593ecc..e921967f46d 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -17,7 +17,7 @@ use crate::values::{CSSFloat, CSSInteger}; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; -pub use crate::values::specified::OverflowWrap; +pub use crate::values::specified::{OverflowWrap, WordBreak}; pub use crate::values::specified::TextAlignKeyword as TextAlign; pub use crate::values::specified::TextEmphasisPosition; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 4956d53f177..fabc595c416 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -80,7 +80,7 @@ pub use self::svg::{SVGPaintOrder, SVGStrokeDashArray, SVGWidth}; pub use self::svg_path::SVGPathData; pub use self::table::XSpan; pub use self::text::{InitialLetter, LetterSpacing, LineHeight, TextAlign}; -pub use self::text::{OverflowWrap, TextEmphasisPosition, TextEmphasisStyle}; +pub use self::text::{OverflowWrap, TextEmphasisPosition, TextEmphasisStyle, WordBreak}; pub use self::text::{TextAlignKeyword, TextDecorationLine, TextOverflow, WordSpacing}; pub use self::time::Time; pub use self::transform::{Rotate, Scale, Transform}; diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 1879dc4159a..33394857f68 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -813,6 +813,32 @@ impl From for u8 { } } +/// Values for the `word-break` property. +#[repr(u8)] +#[derive( + Clone, + Copy, + Debug, + Eq, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, +)] +#[allow(missing_docs)] +pub enum WordBreak { + Normal, + BreakAll, + KeepAll, + /// The break-word value, needed for compat. + /// + /// Specifying `word-break: break-word` makes `overflow-wrap` behave as + /// `anywhere`, and `word-break` behave like `normal`. + BreakWord, +} + /// Values for the `overflow-wrap` property. #[repr(u8)] #[derive( From f1b5d5c06a4e63f45162473fca730e4c0fe3a893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 1 Mar 2019 23:17:12 +0100 Subject: [PATCH 08/31] style: Cleanup and fix interpolation of SVG lengths. Instead of storing them as LengthPercentage | Number, always store as LengthPercentage, and use the unitless length quirk to parse numbers instead. Further cleanups to use the rust representation can happen as a followup, which will also get rid of the boolean argument (since we can poke at the rust length itself). That's why I didn't bother to convert it to an enum class yet. Differential Revision: https://phabricator.services.mozilla.com/D21804 --- components/style/properties/gecko.mako.rs | 69 ++++----------- components/style/values/animated/mod.rs | 22 +---- components/style/values/animated/svg.rs | 100 +--------------------- components/style/values/computed/svg.rs | 42 ++------- components/style/values/generics/svg.rs | 54 +++--------- components/style/values/specified/mod.rs | 12 ++- components/style/values/specified/svg.rs | 75 ++++++++++------ 7 files changed, 94 insertions(+), 280 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index cca1bd540f9..1c8fb3121b3 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -463,27 +463,22 @@ def set_gecko_property(ffi_name, expr): // set on mContextFlags, and the length field is set to the initial value. pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { - use crate::values::generics::svg::{SVGLength, SvgLengthPercentageOrNumber}; + use crate::values::generics::svg::SVGLength; use crate::gecko_bindings::structs::nsStyleSVG_${ident.upper()}_CONTEXT as CONTEXT_VALUE; let length = match v { - SVGLength::Length(length) => { + SVGLength::LengthPercentage(length) => { self.gecko.mContextFlags &= !CONTEXT_VALUE; length } SVGLength::ContextValue => { self.gecko.mContextFlags |= CONTEXT_VALUE; match longhands::${ident}::get_initial_value() { - SVGLength::Length(length) => length, + SVGLength::LengthPercentage(length) => length, _ => unreachable!("Initial value should not be context-value"), } } }; - match length { - SvgLengthPercentageOrNumber::LengthPercentage(lp) => - self.gecko.${gecko_ffi_name}.set(lp), - SvgLengthPercentageOrNumber::Number(num) => - self.gecko.${gecko_ffi_name}.set_value(CoordDataValue::Factor(num.into())), - } + self.gecko.${gecko_ffi_name}.set(length) } pub fn copy_${ident}_from(&mut self, other: &Self) { @@ -499,32 +494,16 @@ def set_gecko_property(ffi_name, expr): } pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T { - use crate::values::generics::svg::{SVGLength, SvgLengthPercentageOrNumber}; + use crate::values::generics::svg::SVGLength; use crate::values::computed::LengthPercentage; use crate::gecko_bindings::structs::nsStyleSVG_${ident.upper()}_CONTEXT as CONTEXT_VALUE; if (self.gecko.mContextFlags & CONTEXT_VALUE) != 0 { return SVGLength::ContextValue; } - let length = match self.gecko.${gecko_ffi_name}.as_value() { - CoordDataValue::Factor(number) => { - SvgLengthPercentageOrNumber::Number(number) - }, - CoordDataValue::Coord(coord) => { - SvgLengthPercentageOrNumber::LengthPercentage( - LengthPercentage::new(Au(coord).into(), None) - ) - }, - CoordDataValue::Percent(p) => { - SvgLengthPercentageOrNumber::LengthPercentage( - LengthPercentage::new(Au(0).into(), Some(Percentage(p))) - ) - }, - CoordDataValue::Calc(calc) => { - SvgLengthPercentageOrNumber::LengthPercentage(calc.into()) - }, - _ => unreachable!("Unexpected coordinate in ${ident}"), - }; - SVGLength::Length(length.into()) + // TODO(emilio): Use the Rust representation instead. + let length = + LengthPercentage::from_gecko_style_coord(&self.gecko.${gecko_ffi_name}).unwrap(); + SVGLength::LengthPercentage(length.into()) } @@ -4718,7 +4697,7 @@ clip-path pub fn set_stroke_dasharray(&mut self, v: longhands::stroke_dasharray::computed_value::T) { use crate::gecko_bindings::structs::nsStyleSVG_STROKE_DASHARRAY_CONTEXT as CONTEXT_VALUE; - use crate::values::generics::svg::{SVGStrokeDashArray, SvgLengthPercentageOrNumber}; + use crate::values::generics::svg::SVGStrokeDashArray; match v { SVGStrokeDashArray::Values(v) => { @@ -4728,12 +4707,7 @@ clip-path bindings::Gecko_nsStyleSVG_SetDashArrayLength(&mut self.gecko, v.len() as u32); } for (gecko, servo) in self.gecko.mStrokeDasharray.iter_mut().zip(v) { - match servo { - SvgLengthPercentageOrNumber::LengthPercentage(lp) => - gecko.set(lp), - SvgLengthPercentageOrNumber::Number(num) => - gecko.set_value(CoordDataValue::Factor(num.into())), - } + gecko.set(servo) } } SVGStrokeDashArray::ContextValue => { @@ -4761,30 +4735,17 @@ clip-path pub fn clone_stroke_dasharray(&self) -> longhands::stroke_dasharray::computed_value::T { use crate::gecko_bindings::structs::nsStyleSVG_STROKE_DASHARRAY_CONTEXT as CONTEXT_VALUE; - use crate::values::computed::LengthPercentage; - use crate::values::generics::NonNegative; - use crate::values::generics::svg::{SVGStrokeDashArray, SvgLengthPercentageOrNumber}; + use crate::values::computed::NonNegativeLengthPercentage; + use crate::values::generics::svg::SVGStrokeDashArray; if self.gecko.mContextFlags & CONTEXT_VALUE != 0 { debug_assert_eq!(self.gecko.mStrokeDasharray.len(), 0); return SVGStrokeDashArray::ContextValue; } + // TODO(emilio): Use the rust representation instead. let mut vec = vec![]; for gecko in self.gecko.mStrokeDasharray.iter() { - match gecko.as_value() { - CoordDataValue::Factor(number) => - vec.push(SvgLengthPercentageOrNumber::Number(number.into())), - CoordDataValue::Coord(coord) => - vec.push(SvgLengthPercentageOrNumber::LengthPercentage( - NonNegative(LengthPercentage::new(Au(coord).into(), None).into()))), - CoordDataValue::Percent(p) => - vec.push(SvgLengthPercentageOrNumber::LengthPercentage( - NonNegative(LengthPercentage::new_percent(Percentage(p)).into()))), - CoordDataValue::Calc(calc) => - vec.push(SvgLengthPercentageOrNumber::LengthPercentage( - NonNegative(LengthPercentage::from(calc).clamp_to_non_negative()))), - _ => unreachable!(), - } + vec.push(NonNegativeLengthPercentage::from_gecko_style_coord(gecko).unwrap()); } SVGStrokeDashArray::Values(vec) } diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index 6bc7f624e34..3b83c71e6e2 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -16,7 +16,7 @@ use crate::values::computed::Image; use crate::values::specified::SVGPathData; use crate::values::CSSFloat; use app_units::Au; -use euclid::{Point2D, Size2D}; +use euclid::Point2D; use smallvec::SmallVec; use std::cmp; @@ -241,19 +241,6 @@ impl Animate for Au { } } -impl Animate for Size2D -where - T: Animate, -{ - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - Ok(Size2D::new( - self.width.animate(&other.width, procedure)?, - self.height.animate(&other.height, procedure)?, - )) - } -} - impl Animate for Point2D where T: Animate, @@ -397,16 +384,13 @@ where } } -impl ToAnimatedZero for Size2D +impl ToAnimatedZero for Vec where T: ToAnimatedZero, { #[inline] fn to_animated_zero(&self) -> Result { - Ok(Size2D::new( - self.width.to_animated_zero()?, - self.height.to_animated_zero()?, - )) + self.iter().map(|v| v.to_animated_zero()).collect() } } diff --git a/components/style/values/animated/svg.rs b/components/style/values/animated/svg.rs index 4564b787c6d..15c6580cd74 100644 --- a/components/style/values/animated/svg.rs +++ b/components/style/values/animated/svg.rs @@ -8,10 +8,8 @@ use super::{Animate, Procedure, ToAnimatedZero}; use crate::properties::animated_properties::ListAnimation; use crate::values::animated::color::Color as AnimatedColor; use crate::values::computed::url::ComputedUrl; -use crate::values::computed::{LengthPercentage, Number, NumberOrPercentage}; -use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; -use crate::values::generics::svg::{SVGLength, SVGPaint, SvgLengthPercentageOrNumber}; -use crate::values::generics::svg::{SVGOpacity, SVGStrokeDashArray}; +use crate::values::distance::{SquaredDistance, ComputeSquaredDistance}; +use crate::values::generics::svg::{SVGPaint, SVGStrokeDashArray}; /// Animated SVGPaint. pub type IntermediateSVGPaint = SVGPaint; @@ -26,67 +24,6 @@ impl ToAnimatedZero for IntermediateSVGPaint { } } -// FIXME: We need to handle calc here properly, see -// https://bugzilla.mozilla.org/show_bug.cgi?id=1386967 -fn to_number_or_percentage( - value: &SvgLengthPercentageOrNumber, -) -> Result { - Ok(match *value { - SvgLengthPercentageOrNumber::LengthPercentage(ref l) => match l.specified_percentage() { - Some(p) => { - if l.unclamped_length().px() != 0. { - return Err(()); - } - NumberOrPercentage::Percentage(p) - }, - None => NumberOrPercentage::Number(l.length().px()), - }, - SvgLengthPercentageOrNumber::Number(ref n) => NumberOrPercentage::Number(*n), - }) -} - -impl Animate for SvgLengthPercentageOrNumber { - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - let this = to_number_or_percentage(self)?; - let other = to_number_or_percentage(other)?; - - match (this, other) { - (NumberOrPercentage::Number(ref this), NumberOrPercentage::Number(ref other)) => Ok( - SvgLengthPercentageOrNumber::Number(this.animate(other, procedure)?), - ), - ( - NumberOrPercentage::Percentage(ref this), - NumberOrPercentage::Percentage(ref other), - ) => Ok(SvgLengthPercentageOrNumber::LengthPercentage( - LengthPercentage::new_percent(this.animate(other, procedure)?), - )), - _ => Err(()), - } - } -} - -impl ComputeSquaredDistance for SvgLengthPercentageOrNumber { - fn compute_squared_distance(&self, other: &Self) -> Result { - to_number_or_percentage(self)?.compute_squared_distance(&to_number_or_percentage(other)?) - } -} - -impl Animate for SVGLength -where - L: Animate + Clone, -{ - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - match (self, other) { - (&SVGLength::Length(ref this), &SVGLength::Length(ref other)) => { - Ok(SVGLength::Length(this.animate(other, procedure)?)) - }, - _ => Err(()), - } - } -} - /// impl Animate for SVGStrokeDashArray where @@ -121,36 +58,3 @@ where } } } - -impl ToAnimatedZero for SVGStrokeDashArray -where - L: ToAnimatedZero, -{ - #[inline] - fn to_animated_zero(&self) -> Result { - match *self { - SVGStrokeDashArray::Values(ref values) => Ok(SVGStrokeDashArray::Values( - values - .iter() - .map(ToAnimatedZero::to_animated_zero) - .collect::, _>>()?, - )), - SVGStrokeDashArray::ContextValue => Ok(SVGStrokeDashArray::ContextValue), - } - } -} - -impl Animate for SVGOpacity -where - O: Animate + Clone, -{ - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - match (self, other) { - (&SVGOpacity::Opacity(ref this), &SVGOpacity::Opacity(ref other)) => { - Ok(SVGOpacity::Opacity(this.animate(other, procedure)?)) - }, - _ => Err(()), - } - } -} diff --git a/components/style/values/computed/svg.rs b/components/style/values/computed/svg.rs index 3d47df32ab8..27d4dd6adc3 100644 --- a/components/style/values/computed/svg.rs +++ b/components/style/values/computed/svg.rs @@ -6,8 +6,7 @@ use crate::values::computed::color::Color; use crate::values::computed::url::ComputedUrl; -use crate::values::computed::{LengthPercentage, NonNegativeLengthPercentage}; -use crate::values::computed::{NonNegativeNumber, Number, Opacity}; +use crate::values::computed::{LengthPercentage, NonNegativeLengthPercentage, Opacity}; use crate::values::generics::svg as generic; use crate::values::RGBA; use crate::Zero; @@ -41,58 +40,31 @@ impl SVGPaint { } } -/// A value of | | for stroke-dashoffset. -/// -pub type SvgLengthPercentageOrNumber = - generic::SvgLengthPercentageOrNumber; - /// | | | context-value -pub type SVGLength = generic::SVGLength; +pub type SVGLength = generic::SVGLength; impl SVGLength { /// `0px` pub fn zero() -> Self { - generic::SVGLength::Length(generic::SvgLengthPercentageOrNumber::LengthPercentage( - LengthPercentage::zero(), - )) - } -} - -/// A value of | | for stroke-width/stroke-dasharray. -/// -pub type NonNegativeSvgLengthPercentageOrNumber = - generic::SvgLengthPercentageOrNumber; - -// FIXME(emilio): This is really hacky, and can go away with a bit of work on -// the clone_stroke_width code in gecko.mako.rs. -impl Into for SvgLengthPercentageOrNumber { - fn into(self) -> NonNegativeSvgLengthPercentageOrNumber { - match self { - generic::SvgLengthPercentageOrNumber::LengthPercentage(lop) => { - generic::SvgLengthPercentageOrNumber::LengthPercentage(lop.into()) - }, - generic::SvgLengthPercentageOrNumber::Number(num) => { - generic::SvgLengthPercentageOrNumber::Number(num.into()) - }, - } + generic::SVGLength::LengthPercentage(LengthPercentage::zero()) } } /// An non-negative wrapper of SVGLength. -pub type SVGWidth = generic::SVGLength; +pub type SVGWidth = generic::SVGLength; impl SVGWidth { /// `1px`. pub fn one() -> Self { use crate::values::generics::NonNegative; - generic::SVGLength::Length(generic::SvgLengthPercentageOrNumber::LengthPercentage( + generic::SVGLength::LengthPercentage( NonNegative(LengthPercentage::one()), - )) + ) } } /// [ | | ]# | context-value -pub type SVGStrokeDashArray = generic::SVGStrokeDashArray; +pub type SVGStrokeDashArray = generic::SVGStrokeDashArray; impl Default for SVGStrokeDashArray { fn default() -> Self { diff --git a/components/style/values/generics/svg.rs b/components/style/values/generics/svg.rs index 7f52126a748..6a4d2411e76 100644 --- a/components/style/values/generics/svg.rs +++ b/components/style/values/generics/svg.rs @@ -128,64 +128,26 @@ impl Parse for SVGPaint | | for svg which allow unitless length. -/// -#[derive( - Clone, - Copy, - Debug, - MallocSizeOf, - PartialEq, - Parse, - SpecifiedValueInfo, - ToAnimatedValue, - ToAnimatedZero, - ToComputedValue, - ToCss, -)] -pub enum SvgLengthPercentageOrNumber { - /// - /// - /// Note that this needs to be before, so it gets parsed before the length, - /// to handle `0` correctly as a number instead of a ``. - Number(Number), - /// | - LengthPercentage(LengthPercentage), -} - -/// Whether the `context-value` value is enabled. -#[cfg(feature = "gecko")] -pub fn is_context_value_enabled(_: &ParserContext) -> bool { - use crate::gecko_bindings::structs::mozilla; - unsafe { mozilla::StaticPrefs_sVarCache_gfx_font_rendering_opentype_svg_enabled } -} - -/// Whether the `context-value` value is enabled. -#[cfg(not(feature = "gecko"))] -pub fn is_context_value_enabled(_: &ParserContext) -> bool { - false -} - /// An SVG length value supports `context-value` in addition to length. #[derive( + Animate, Clone, ComputeSquaredDistance, Copy, Debug, MallocSizeOf, PartialEq, - Parse, SpecifiedValueInfo, ToAnimatedValue, ToAnimatedZero, ToComputedValue, ToCss, )] -pub enum SVGLength { +pub enum SVGLength { /// ` | | ` - Length(LengthType), + LengthPercentage(L), /// `context-value` - #[parse(condition = "is_context_value_enabled")] + #[animation(error)] ContextValue, } @@ -197,13 +159,14 @@ pub enum SVGLength { PartialEq, SpecifiedValueInfo, ToAnimatedValue, + ToAnimatedZero, ToComputedValue, ToCss, )] -pub enum SVGStrokeDashArray { +pub enum SVGStrokeDashArray { /// `[ | | ]#` #[css(comma)] - Values(#[css(if_empty = "none", iterable)] Vec), + Values(#[css(if_empty = "none", iterable)] Vec), /// `context-value` ContextValue, } @@ -211,6 +174,7 @@ pub enum SVGStrokeDashArray { /// An SVG opacity value accepts `context-{fill,stroke}-opacity` in /// addition to opacity value. #[derive( + Animate, Clone, ComputeSquaredDistance, Copy, @@ -227,7 +191,9 @@ pub enum SVGOpacity { /// `` Opacity(OpacityType), /// `context-fill-opacity` + #[animation(error)] ContextFillOpacity, /// `context-stroke-opacity` + #[animation(error)] ContextStrokeOpacity, } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index fabc595c416..d82a3246a13 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -782,16 +782,22 @@ impl ClipRectOrAuto { /// Whether quirks are allowed in this context. #[derive(Clone, Copy, PartialEq)] pub enum AllowQuirks { - /// Quirks are allowed. - Yes, /// Quirks are not allowed. No, + /// Quirks are allowed, in quirks mode. + Yes, + /// Quirks are always allowed, used for SVG lengths. + Always, } impl AllowQuirks { /// Returns `true` if quirks are allowed in this context. pub fn allowed(self, quirks_mode: QuirksMode) -> bool { - self == AllowQuirks::Yes && quirks_mode == QuirksMode::Quirks + match self { + AllowQuirks::Always => true, + AllowQuirks::No => false, + AllowQuirks::Yes => quirks_mode == QuirksMode::Quirks, + } } } diff --git a/components/style/values/specified/svg.rs b/components/style/values/specified/svg.rs index 98db273e61c..fa54b5338b4 100644 --- a/components/style/values/specified/svg.rs +++ b/components/style/values/specified/svg.rs @@ -6,11 +6,11 @@ use crate::parser::{Parse, ParserContext}; use crate::values::generics::svg as generic; +use crate::values::specified::AllowQuirks; use crate::values::specified::color::Color; use crate::values::specified::url::SpecifiedUrl; use crate::values::specified::LengthPercentage; -use crate::values::specified::{NonNegativeLengthPercentage, NonNegativeNumber}; -use crate::values::specified::{Number, Opacity}; +use crate::values::specified::{NonNegativeLengthPercentage, Opacity}; use crate::values::CustomIdent; use cssparser::Parser; use std::fmt::{self, Write}; @@ -23,36 +23,53 @@ pub type SVGPaint = generic::SVGPaint; /// Specified SVG Paint Kind value pub type SVGPaintKind = generic::SVGPaintKind; -/// A value of | | for stroke-dashoffset. -/// -pub type SvgLengthPercentageOrNumber = - generic::SvgLengthPercentageOrNumber; - /// | | | context-value -pub type SVGLength = generic::SVGLength; - -impl From for SVGLength { - fn from(length: SvgLengthPercentageOrNumber) -> Self { - generic::SVGLength::Length(length) - } -} - -/// A value of | | for stroke-width/stroke-dasharray. -/// -pub type NonNegativeSvgLengthPercentageOrNumber = - generic::SvgLengthPercentageOrNumber; +pub type SVGLength = generic::SVGLength; /// A non-negative version of SVGLength. -pub type SVGWidth = generic::SVGLength; +pub type SVGWidth = generic::SVGLength; -impl From for SVGWidth { - fn from(length: NonNegativeSvgLengthPercentageOrNumber) -> Self { - generic::SVGLength::Length(length) +/// [ | | ]# | context-value +pub type SVGStrokeDashArray = generic::SVGStrokeDashArray; + +/// Whether the `context-value` value is enabled. +#[cfg(feature = "gecko")] +pub fn is_context_value_enabled() -> bool { + use crate::gecko_bindings::structs::mozilla; + unsafe { mozilla::StaticPrefs_sVarCache_gfx_font_rendering_opentype_svg_enabled } +} + +/// Whether the `context-value` value is enabled. +#[cfg(not(feature = "gecko"))] +pub fn is_context_value_enabled() -> bool { + false +} + +macro_rules! parse_svg_length { + ($ty:ty, $lp:ty) => { + impl Parse for $ty { + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + if let Ok(lp) = input.try(|i| { + <$lp>::parse_quirky(context, i, AllowQuirks::Always) + }) { + return Ok(generic::SVGLength::LengthPercentage(lp)) + } + + try_match_ident_ignore_ascii_case! { input, + "context-value" if is_context_value_enabled() => { + Ok(generic::SVGLength::ContextValue) + }, + } + } + } } } -/// [ | | ]# | context-value -pub type SVGStrokeDashArray = generic::SVGStrokeDashArray; +parse_svg_length!(SVGLength, LengthPercentage); +parse_svg_length!(SVGWidth, NonNegativeLengthPercentage); impl Parse for SVGStrokeDashArray { fn parse<'i, 't>( @@ -61,14 +78,18 @@ impl Parse for SVGStrokeDashArray { ) -> Result> { if let Ok(values) = input.try(|i| { CommaWithSpace::parse(i, |i| { - NonNegativeSvgLengthPercentageOrNumber::parse(context, i) + NonNegativeLengthPercentage::parse_quirky( + context, + i, + AllowQuirks::Always, + ) }) }) { return Ok(generic::SVGStrokeDashArray::Values(values)); } try_match_ident_ignore_ascii_case! { input, - "context-value" if generic::is_context_value_enabled(context) => { + "context-value" if is_context_value_enabled() => { Ok(generic::SVGStrokeDashArray::ContextValue) }, "none" => Ok(generic::SVGStrokeDashArray::Values(vec![])), From 94686df11babb96c4dbfb9afc7d6801a6d8f92bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 2 Mar 2019 02:09:19 +0100 Subject: [PATCH 09/31] style: Use rust lengths for the SVG lengths. As it turns out we need this to avoid losing precision both during painting and during serialization. This patch also changes to serialize `context-value` if it's the computed value. I could keep the previous behavior, but it makes no sense to serialize the initial value. We're the only ones to support this value anyway, and I couldn't find a definition or spec for this. Also update tests and expectations for: * New unexpected passes. * Always serializing the unit in getComputedStyle. * Calc and interpolation support. Chrome also always serializes the unit in getComputedStyle, so I'm pretty sure this is compatible with them. Chrome is inconsistent and keeps numbers in specified style, but that's inconsistent with itself and with other quirky lengths, so I updated the tests instead. Differential Revision: https://phabricator.services.mozilla.com/D21819 --- components/style/properties/gecko.mako.rs | 20 +++++-------------- .../longhands/inherited_svg.mako.rs | 3 ++- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 1c8fb3121b3..9ee8ab3c656 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -478,12 +478,12 @@ def set_gecko_property(ffi_name, expr): } } }; - self.gecko.${gecko_ffi_name}.set(length) + self.gecko.${gecko_ffi_name} = length; } pub fn copy_${ident}_from(&mut self, other: &Self) { use crate::gecko_bindings::structs::nsStyleSVG_${ident.upper()}_CONTEXT as CONTEXT_VALUE; - self.gecko.${gecko_ffi_name}.copy_from(&other.gecko.${gecko_ffi_name}); + self.gecko.${gecko_ffi_name} = other.gecko.${gecko_ffi_name}; self.gecko.mContextFlags = (self.gecko.mContextFlags & !CONTEXT_VALUE) | (other.gecko.mContextFlags & CONTEXT_VALUE); @@ -495,15 +495,11 @@ def set_gecko_property(ffi_name, expr): pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T { use crate::values::generics::svg::SVGLength; - use crate::values::computed::LengthPercentage; use crate::gecko_bindings::structs::nsStyleSVG_${ident.upper()}_CONTEXT as CONTEXT_VALUE; if (self.gecko.mContextFlags & CONTEXT_VALUE) != 0 { return SVGLength::ContextValue; } - // TODO(emilio): Use the Rust representation instead. - let length = - LengthPercentage::from_gecko_style_coord(&self.gecko.${gecko_ffi_name}).unwrap(); - SVGLength::LengthPercentage(length.into()) + SVGLength::LengthPercentage(self.gecko.${gecko_ffi_name}) } @@ -4707,7 +4703,7 @@ clip-path bindings::Gecko_nsStyleSVG_SetDashArrayLength(&mut self.gecko, v.len() as u32); } for (gecko, servo) in self.gecko.mStrokeDasharray.iter_mut().zip(v) { - gecko.set(servo) + *gecko = servo; } } SVGStrokeDashArray::ContextValue => { @@ -4735,19 +4731,13 @@ clip-path pub fn clone_stroke_dasharray(&self) -> longhands::stroke_dasharray::computed_value::T { use crate::gecko_bindings::structs::nsStyleSVG_STROKE_DASHARRAY_CONTEXT as CONTEXT_VALUE; - use crate::values::computed::NonNegativeLengthPercentage; use crate::values::generics::svg::SVGStrokeDashArray; if self.gecko.mContextFlags & CONTEXT_VALUE != 0 { debug_assert_eq!(self.gecko.mStrokeDasharray.len(), 0); return SVGStrokeDashArray::ContextValue; } - // TODO(emilio): Use the rust representation instead. - let mut vec = vec![]; - for gecko in self.gecko.mStrokeDasharray.iter() { - vec.push(NonNegativeLengthPercentage::from_gecko_style_coord(gecko).unwrap()); - } - SVGStrokeDashArray::Values(vec) + SVGStrokeDashArray::Values(self.gecko.mStrokeDasharray.iter().cloned().collect()) } #[allow(non_snake_case)] diff --git a/components/style/properties/longhands/inherited_svg.mako.rs b/components/style/properties/longhands/inherited_svg.mako.rs index fcd16833b5f..35d0a02aab2 100644 --- a/components/style/properties/longhands/inherited_svg.mako.rs +++ b/components/style/properties/longhands/inherited_svg.mako.rs @@ -84,7 +84,8 @@ ${helpers.predefined_type( )} ${helpers.predefined_type( - "stroke-width", "SVGWidth", + "stroke-width", + "SVGWidth", "computed::SVGWidth::one()", products="gecko", animation_value_type="crate::values::computed::SVGWidth", From 35b8b95263d4f93dd791a44a7f570e2240655fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 3 Mar 2019 11:31:06 +0000 Subject: [PATCH 10/31] style: Implement a version of #[css(skip_if)] that takes more context. I called it contextual_skip_if, though better names welcome. Differential Revision: https://phabricator.services.mozilla.com/D21858 --- components/style_derive/to_css.rs | 22 ++++++++++++++++++++-- components/style_traits/values.rs | 4 ++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/components/style_derive/to_css.rs b/components/style_derive/to_css.rs index 4eabdcb8357..3a1bc9f4c6c 100644 --- a/components/style_derive/to_css.rs +++ b/components/style_derive/to_css.rs @@ -148,12 +148,20 @@ fn derive_variant_fields_expr( } } } + + if let Some(condition) = attrs.contextual_skip_if { + expr = quote! { + if !#condition(#(#bindings),*) { + #expr + } + } + } return expr; } - let mut expr = derive_single_field_expr(first, attrs, where_clause); + let mut expr = derive_single_field_expr(first, attrs, where_clause, bindings); for (binding, attrs) in iter { - derive_single_field_expr(binding, attrs, where_clause).to_tokens(&mut expr) + derive_single_field_expr(binding, attrs, where_clause, bindings).to_tokens(&mut expr) } quote! {{ @@ -167,6 +175,7 @@ fn derive_single_field_expr( field: &BindingInfo, attrs: CssFieldAttrs, where_clause: &mut Option, + bindings: &[BindingInfo], ) -> TokenStream { let mut expr = if attrs.iterable { if let Some(if_empty) = attrs.if_empty { @@ -216,6 +225,14 @@ fn derive_single_field_expr( } } + if let Some(condition) = attrs.contextual_skip_if { + expr = quote! { + if !#condition(#(#bindings),*) { + #expr + } + } + } + expr } @@ -249,5 +266,6 @@ pub struct CssFieldAttrs { pub iterable: bool, pub skip: bool, pub represents_keyword: bool, + pub contextual_skip_if: Option, pub skip_if: Option, } diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index 582e34eb3db..46751541319 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -37,6 +37,10 @@ use std::fmt::{self, Write}; /// * if `#[css(skip_if = "function")]` is found on a field, the `ToCss` call /// for that field is skipped if `function` returns true. This function is /// provided the field as an argument; +/// * if `#[css(contextual_skip_if = "function")]` is found on a field, the +/// `ToCss` call for that field is skipped if `function` returns true. This +/// function is given all the fields in the current struct or variant as an +/// argument; /// * `#[css(represents_keyword)]` can be used on bool fields in order to /// serialize the field name if the field is true, or nothing otherwise. It /// also collects those keywords for `SpecifiedValueInfo`. From 9b24a451f6d77ecefc43a5b690930c19eb704c23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 3 Mar 2019 11:31:21 +0000 Subject: [PATCH 11/31] style: When deriving something with an output type, map preconditions as well. Otherwise, deriving ToComputedValue and ToAnimatedValue in structs or enums with other where clauses just doesn't work. Differential Revision: https://phabricator.services.mozilla.com/D21859 --- components/style_derive/cg.rs | 64 ++++++++++++++++++++ components/style_derive/to_animated_value.rs | 6 ++ components/style_derive/to_computed_value.rs | 6 ++ 3 files changed, 76 insertions(+) diff --git a/components/style_derive/cg.rs b/components/style_derive/cg.rs index 6eeca384bfa..880cb07378b 100644 --- a/components/style_derive/cg.rs +++ b/components/style_derive/cg.rs @@ -12,6 +12,70 @@ use syn::{TypeParam, TypeParen, TypePath, TypeSlice, TypeTuple}; use syn::{Variant, WherePredicate}; use synstructure::{self, BindStyle, BindingInfo, VariantAst, VariantInfo}; +/// Given an input type which has some where clauses already, like: +/// +/// struct InputType +/// where +/// T: Zero, +/// { +/// ... +/// } +/// +/// Add the necessary `where` clauses so that the output type of a trait +/// fulfils them. +/// +/// For example: +/// +/// ::ComputedValue: Zero, +/// +/// This needs to run before adding other bounds to the type parameters. +pub fn propagate_clauses_to_output_type( + where_clause: &mut Option, + generics: &syn::Generics, + trait_path: Path, + trait_output: Ident, +) { + let where_clause = match *where_clause { + Some(ref mut clause) => clause, + None => return, + }; + let mut extra_bounds = vec![]; + for pred in &where_clause.predicates { + let ty = match *pred { + syn::WherePredicate::Type(ref ty) => ty, + ref predicate => panic!("Unhanded complex where predicate: {:?}", predicate), + }; + + let path = match ty.bounded_ty { + syn::Type::Path(ref p) => &p.path, + ref ty => panic!("Unhanded complex where type: {:?}", ty), + }; + + assert!( + ty.lifetimes.is_none(), + "Unhanded complex lifetime bound: {:?}", + ty, + ); + + let ident = match path_to_ident(path) { + Some(i) => i, + None => panic!("Unhanded complex where type path: {:?}", path), + }; + + if generics.type_params().any(|param| param.ident == *ident) { + extra_bounds.push(ty.clone()); + } + } + + for bound in extra_bounds { + let ty = bound.bounded_ty; + let bounds = bound.bounds; + where_clause.predicates.push( + parse_quote!(<#ty as #trait_path>::#trait_output: #bounds), + ) + } +} + pub fn add_predicate(where_clause: &mut Option, pred: WherePredicate) { where_clause .get_or_insert(parse_quote!(where)) diff --git a/components/style_derive/to_animated_value.rs b/components/style_derive/to_animated_value.rs index b53cf9c3d41..1aba9b3327e 100644 --- a/components/style_derive/to_animated_value.rs +++ b/components/style_derive/to_animated_value.rs @@ -9,6 +9,12 @@ use synstructure::BindStyle; pub fn derive(mut input: DeriveInput) -> TokenStream { let mut where_clause = input.generics.where_clause.take(); + cg::propagate_clauses_to_output_type( + &mut where_clause, + &input.generics, + parse_quote!(crate::values::animated::ToAnimatedValue), + parse_quote!(AnimatedValue), + ); for param in input.generics.type_params() { cg::add_predicate( &mut where_clause, diff --git a/components/style_derive/to_computed_value.rs b/components/style_derive/to_computed_value.rs index 68261740502..b0b2aa6af63 100644 --- a/components/style_derive/to_computed_value.rs +++ b/components/style_derive/to_computed_value.rs @@ -9,6 +9,12 @@ use synstructure::BindStyle; pub fn derive(mut input: DeriveInput) -> TokenStream { let mut where_clause = input.generics.where_clause.take(); + cg::propagate_clauses_to_output_type( + &mut where_clause, + &input.generics, + parse_quote!(crate::values::computed::ToComputedValue), + parse_quote!(ComputedValue), + ); let (to_body, from_body) = { let params = input.generics.type_params().collect::>(); for param in ¶ms { From 93d3004c17552b8db2855c3add483bc4e44f2e13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 3 Mar 2019 11:31:30 +0000 Subject: [PATCH 12/31] style: Use skip_if for translate serialization. Trivial drive-by cleanup. Differential Revision: https://phabricator.services.mozilla.com/D21860 --- components/style/values/generics/transform.rs | 57 ++++++------------- 1 file changed, 18 insertions(+), 39 deletions(-) diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index d25d2d3a653..b0a121fd6e9 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -629,55 +629,34 @@ impl ToCss for Scale { } #[derive( - Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToAnimatedZero, ToComputedValue, + Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToAnimatedZero, ToComputedValue, ToCss )] -/// A value of the `Translate` property +/// A value of the `translate` property +/// +/// https://drafts.csswg.org/css-transforms-2/#individual-transform-serialization: +/// +/// If a 2d translation is specified, the property must serialize with only one +/// or two values (per usual, if the second value is 0px, the default, it must +/// be omitted when serializing). +/// +/// If a 3d translation is specified, all three values must be serialized. +/// +/// We don't omit the 3rd component even if it is 0px for now, and the +/// related spec issue is https://github.com/w3c/csswg-drafts/issues/3305 /// /// -pub enum Translate { +pub enum Translate +where + LengthPercentage: Zero, +{ /// 'none' None, /// '' or ' ' - Translate(LengthPercentage, LengthPercentage), + Translate(LengthPercentage, #[css(skip_if = "Zero::is_zero")] LengthPercentage), /// ' ' Translate3D(LengthPercentage, LengthPercentage, Length), } -impl ToCss for Translate { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: fmt::Write, - { - // The spec says: - // 1. If a 2d translation is specified, the property must serialize with only one or two - // values (per usual, if the second value is 0px, the default, it must be omitted when - // serializing). - // 2. If a 3d translation is specified, all three values must be serialized. - // https://drafts.csswg.org/css-transforms-2/#individual-transform-serialization - // - // We don't omit the 3rd component even if it is 0px for now, and the related - // spec issue is https://github.com/w3c/csswg-drafts/issues/3305 - match *self { - Translate::None => dest.write_str("none"), - Translate::Translate(ref x, ref y) => { - x.to_css(dest)?; - if !y.is_zero() { - dest.write_char(' ')?; - y.to_css(dest)?; - } - Ok(()) - }, - Translate::Translate3D(ref x, ref y, ref z) => { - x.to_css(dest)?; - dest.write_char(' ')?; - y.to_css(dest)?; - dest.write_char(' ')?; - z.to_css(dest) - }, - } - } -} - #[allow(missing_docs)] #[derive( Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss, From 1418ddc6850b000c0e3fa293103c237e5baafb66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 3 Mar 2019 11:31:40 +0000 Subject: [PATCH 13/31] style: Use contextual_skip_if for background-size. Also drive-by cleanup. Differential Revision: https://phabricator.services.mozilla.com/D21861 --- .../style/values/generics/background.rs | 40 ++++++------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/components/style/values/generics/background.rs b/components/style/values/generics/background.rs index 14ac9744b14..3780f9a4fc3 100644 --- a/components/style/values/generics/background.rs +++ b/components/style/values/generics/background.rs @@ -5,8 +5,13 @@ //! Generic types for CSS values related to backgrounds. use crate::values::generics::length::{GenericLengthPercentageOrAuto, LengthPercentageOrAuto}; -use std::fmt::{self, Write}; -use style_traits::{CssWriter, ToCss}; + +fn width_and_height_are_auto( + width: &LengthPercentageOrAuto, + height: &LengthPercentageOrAuto, +) -> bool { + width.is_auto() && height.is_auto() +} /// A generic value for the `background-size` property. #[derive( @@ -21,6 +26,7 @@ use style_traits::{CssWriter, ToCss}; ToAnimatedValue, ToAnimatedZero, ToComputedValue, + ToCss, )] #[repr(C, u8)] pub enum GenericBackgroundSize { @@ -29,6 +35,10 @@ pub enum GenericBackgroundSize { /// Explicit width. width: GenericLengthPercentageOrAuto, /// Explicit height. + /// NOTE(emilio): We should probably simplify all these in case `width` + /// and `height` are the same, but all other browsers agree on only + /// special-casing `auto`. + #[css(contextual_skip_if = "width_and_height_are_auto")] height: GenericLengthPercentageOrAuto, }, /// `cover` @@ -41,32 +51,6 @@ pub enum GenericBackgroundSize { pub use self::GenericBackgroundSize as BackgroundSize; -impl ToCss for BackgroundSize -where - LengthPercentage: ToCss, -{ - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - match self { - BackgroundSize::ExplicitSize { width, height } => { - width.to_css(dest)?; - // NOTE(emilio): We should probably simplify all these in case - // `width == `height`, but all other browsers agree on only - // special-casing `auto`. - if !width.is_auto() || !height.is_auto() { - dest.write_str(" ")?; - height.to_css(dest)?; - } - Ok(()) - }, - BackgroundSize::Cover => dest.write_str("cover"), - BackgroundSize::Contain => dest.write_str("contain"), - } - } -} - impl BackgroundSize { /// Returns `auto auto`. pub fn auto() -> Self { From c16e88d2294deffd39ce540547617f46a90894fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 3 Mar 2019 11:31:54 +0000 Subject: [PATCH 14/31] style: Remove Options from TransformOperation. This may or may not be part of the plan to get rid of nsCSSValue ;) Option is not usable via FFI, and they should not be needed (we should be following the shortest serialization principle instead). These patches also do that, which matches the other transform properties. I think that slight change is fine, if we can make it work, and consistent with other properties. Alternative is adding more TransformOperation variants or such, which I rather not do. Differential Revision: https://phabricator.services.mozilla.com/D21862 --- components/style/properties/gecko.mako.rs | 44 ++++--------------- components/style/values/animated/transform.rs | 32 ++------------ components/style/values/computed/transform.rs | 21 ++++----- components/style/values/generics/transform.rs | 40 ++++++++++------- components/style/values/specified/angle.rs | 29 ++++++++++++ .../style/values/specified/transform.rs | 12 ++--- 6 files changed, 78 insertions(+), 100 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 9ee8ab3c656..a22e745963e 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -855,13 +855,13 @@ def set_gecko_property(ffi_name, expr): transform_functions = [ ("Matrix3D", "matrix3d", ["number"] * 16), ("Matrix", "matrix", ["number"] * 6), - ("Translate", "translate", ["lp", "optional_lp"]), + ("Translate", "translate", ["lp", "lp"]), ("Translate3D", "translate3d", ["lp", "lp", "length"]), ("TranslateX", "translatex", ["lp"]), ("TranslateY", "translatey", ["lp"]), ("TranslateZ", "translatez", ["length"]), ("Scale3D", "scale3d", ["number"] * 3), - ("Scale", "scale", ["number", "optional_number"]), + ("Scale", "scale", ["number", "number"]), ("ScaleX", "scalex", ["number"]), ("ScaleY", "scaley", ["number"]), ("ScaleZ", "scalez", ["number"]), @@ -870,7 +870,7 @@ transform_functions = [ ("RotateX", "rotatex", ["angle"]), ("RotateY", "rotatey", ["angle"]), ("RotateZ", "rotatez", ["angle"]), - ("Skew", "skew", ["angle", "optional_angle"]), + ("Skew", "skew", ["angle", "angle"]), ("SkewX", "skewx", ["angle"]), ("SkewY", "skewy", ["angle"]), ("Perspective", "perspective", ["length"]), @@ -881,7 +881,6 @@ transform_functions = [ <%def name="transform_function_arm(name, keyword, items)"> <% - has_optional = items[-1].startswith("optional_") pattern = None if keyword == "matrix3d": # m11: number1, m12: number2, .. @@ -919,36 +918,20 @@ transform_functions = [ } %> crate::values::generics::transform::TransformOperation::${name}${pattern} => { - % if has_optional: - let optional_present = ${items[-1] + str(len(items))}.is_some(); - let len = if optional_present { - ${len(items) + 1} - } else { - ${len(items)} - }; - % else: - let len = ${len(items) + 1}; - % endif + let len = ${len(items) + 1}; bindings::Gecko_CSSValue_SetFunction(gecko_value, len); bindings::Gecko_CSSValue_SetKeyword( bindings::Gecko_CSSValue_GetArrayItem(gecko_value, 0), structs::nsCSSKeyword::eCSSKeyword_${keyword} ); % for index, item in enumerate(items): - <% replaced_item = item.replace("optional_", "") %> - % if item.startswith("optional"): - if let Some(${replaced_item + str(index + 1)}) = ${item + str(index + 1)} { - % endif % if item == "list": debug_assert!(!${item}${index + 1}.0.is_empty()); % endif - ${css_value_setters[replaced_item] % ( + ${css_value_setters[item] % ( "bindings::Gecko_CSSValue_GetArrayItem(gecko_value, %d)" % (index + 1), - replaced_item + str(index + 1) + item + str(index + 1) )}; - % if item.startswith("optional"): - } - % endif % endfor } @@ -959,8 +942,6 @@ transform_functions = [ css_value_getters = { "length" : "Length::new(bindings::Gecko_CSSValue_GetNumber(%s))", "lp" : "%s.get_length_percentage()", - "lpon" : "Either::Second(%s.get_length_percentage())", - "lon" : "Either::First(%s.get_length())", "angle" : "%s.get_angle()", "number" : "bindings::Gecko_CSSValue_GetNumber(%s)", "percentage" : "Percentage(bindings::Gecko_CSSValue_GetPercentage(%s))", @@ -998,20 +979,11 @@ transform_functions = [ ${field_names[index]}: % endif <% - getter = css_value_getters[item.replace("optional_", "")] % ( + getter = css_value_getters[item] % ( "bindings::Gecko_CSSValue_GetArrayItemConst(gecko_value, %d)" % (index + 1) ) %> - % if item.startswith("optional_"): - if (**gecko_value.mValue.mArray.as_ref()).mCount == ${index + 1} { - None - } else { - Some(${getter}) - } - % else: - ${getter} - % endif -, + ${getter}, % endfor ${post_symbols} }, diff --git a/components/style/values/animated/transform.rs b/components/style/values/animated/transform.rs index 507c4ba4671..0c322d8fccc 100644 --- a/components/style/values/animated/transform.rs +++ b/components/style/values/animated/transform.rs @@ -1003,18 +1003,12 @@ impl Animate for ComputedTransformOperation { (&TransformOperation::Matrix(ref this), &TransformOperation::Matrix(ref other)) => { Ok(TransformOperation::Matrix(this.animate(other, procedure)?)) }, - (&TransformOperation::Skew(ref fx, None), &TransformOperation::Skew(ref tx, None)) => { - Ok(TransformOperation::Skew(fx.animate(tx, procedure)?, None)) - }, ( &TransformOperation::Skew(ref fx, ref fy), &TransformOperation::Skew(ref tx, ref ty), ) => Ok(TransformOperation::Skew( fx.animate(tx, procedure)?, - Some( - fy.unwrap_or(Angle::zero()) - .animate(&ty.unwrap_or(Angle::zero()), procedure)?, - ), + fy.animate(ty, procedure)?, )), (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) => { Ok(TransformOperation::SkewX(f.animate(t, procedure)?)) @@ -1030,22 +1024,12 @@ impl Animate for ComputedTransformOperation { fy.animate(ty, procedure)?, fz.animate(tz, procedure)?, )), - ( - &TransformOperation::Translate(ref fx, None), - &TransformOperation::Translate(ref tx, None), - ) => Ok(TransformOperation::Translate( - fx.animate(tx, procedure)?, - None, - )), ( &TransformOperation::Translate(ref fx, ref fy), &TransformOperation::Translate(ref tx, ref ty), ) => Ok(TransformOperation::Translate( fx.animate(tx, procedure)?, - Some( - fy.unwrap_or(LengthPercentage::zero()) - .animate(&ty.unwrap_or(LengthPercentage::zero()), procedure)?, - ), + fy.animate(ty, procedure)?, )), (&TransformOperation::TranslateX(ref f), &TransformOperation::TranslateX(ref t)) => { Ok(TransformOperation::TranslateX(f.animate(t, procedure)?)) @@ -1073,22 +1057,12 @@ impl Animate for ComputedTransformOperation { (&TransformOperation::ScaleZ(ref f), &TransformOperation::ScaleZ(ref t)) => Ok( TransformOperation::ScaleZ(animate_multiplicative_factor(*f, *t, procedure)?), ), - (&TransformOperation::Scale(ref f, None), &TransformOperation::Scale(ref t, None)) => { - Ok(TransformOperation::Scale( - animate_multiplicative_factor(*f, *t, procedure)?, - None, - )) - }, ( &TransformOperation::Scale(ref fx, ref fy), &TransformOperation::Scale(ref tx, ref ty), ) => Ok(TransformOperation::Scale( animate_multiplicative_factor(*fx, *tx, procedure)?, - Some(animate_multiplicative_factor( - fy.unwrap_or(*fx), - ty.unwrap_or(*tx), - procedure, - )?), + animate_multiplicative_factor(*fy, *ty, procedure)?, )), ( &TransformOperation::Rotate3D(fx, fy, fz, fa), diff --git a/components/style/values/computed/transform.rs b/components/style/values/computed/transform.rs index f3b40c79f9e..cdb7fb39b47 100644 --- a/components/style/values/computed/transform.rs +++ b/components/style/values/computed/transform.rs @@ -371,15 +371,14 @@ impl TransformOperation { pub fn to_translate_3d(&self) -> Self { match *self { generic::TransformOperation::Translate3D(..) => self.clone(), - generic::TransformOperation::TranslateX(ref x) | - generic::TransformOperation::Translate(ref x, None) => { + generic::TransformOperation::TranslateX(ref x) => { generic::TransformOperation::Translate3D( x.clone(), LengthPercentage::zero(), Length::zero(), ) }, - generic::TransformOperation::Translate(ref x, Some(ref y)) => { + generic::TransformOperation::Translate(ref x, ref y) => { generic::TransformOperation::Translate3D(x.clone(), y.clone(), Length::zero()) }, generic::TransformOperation::TranslateY(ref y) => { @@ -426,10 +425,7 @@ impl TransformOperation { pub fn to_scale_3d(&self) -> Self { match *self { generic::TransformOperation::Scale3D(..) => self.clone(), - generic::TransformOperation::Scale(s, None) => { - generic::TransformOperation::Scale3D(s, s, 1.) - }, - generic::TransformOperation::Scale(x, Some(y)) => { + generic::TransformOperation::Scale(x, y) => { generic::TransformOperation::Scale3D(x, y, 1.) }, generic::TransformOperation::ScaleX(x) => { @@ -494,7 +490,7 @@ impl ToAnimatedZero for TransformOperation { Ok(generic::TransformOperation::Scale3D(1.0, 1.0, 1.0)) }, generic::TransformOperation::Scale(_, _) => { - Ok(generic::TransformOperation::Scale(1.0, Some(1.0))) + Ok(generic::TransformOperation::Scale(1.0, 1.0)) }, generic::TransformOperation::ScaleX(..) => Ok(generic::TransformOperation::ScaleX(1.0)), generic::TransformOperation::ScaleY(..) => Ok(generic::TransformOperation::ScaleY(1.0)), @@ -585,7 +581,7 @@ impl Translate { match *self { generic::Translate::None => None, generic::Translate::Translate(tx, ty) => { - Some(generic::TransformOperation::Translate(tx, Some(ty))) + Some(generic::TransformOperation::Translate(tx, ty)) }, generic::Translate::Translate3D(tx, ty, tz) => { Some(generic::TransformOperation::Translate3D(tx, ty, tz)) @@ -596,7 +592,7 @@ impl Translate { /// Convert Translate to TransformOperation. pub fn from_transform_operation(operation: &TransformOperation) -> Translate { match *operation { - generic::TransformOperation::Translate(tx, Some(ty)) => { + generic::TransformOperation::Translate(tx, ty) => { generic::Translate::Translate(tx, ty) }, generic::TransformOperation::Translate3D(tx, ty, tz) => { @@ -615,7 +611,7 @@ impl Scale { pub fn to_transform_operation(&self) -> Option { match *self { generic::Scale::None => None, - generic::Scale::Scale(sx, sy) => Some(generic::TransformOperation::Scale(sx, Some(sy))), + generic::Scale::Scale(sx, sy) => Some(generic::TransformOperation::Scale(sx, sy)), generic::Scale::Scale3D(sx, sy, sz) => { Some(generic::TransformOperation::Scale3D(sx, sy, sz)) }, @@ -625,8 +621,7 @@ impl Scale { /// Convert Scale to TransformOperation. pub fn from_transform_operation(operation: &TransformOperation) -> Scale { match *operation { - generic::TransformOperation::Scale(sx, Some(sy)) => generic::Scale::Scale(sx, sy), - generic::TransformOperation::Scale(sx, None) => generic::Scale::Scale(sx, sx), + generic::TransformOperation::Scale(sx, sy) => generic::Scale::Scale(sx, sy), generic::TransformOperation::Scale3D(sx, sy, sz) => generic::Scale::Scale3D(sx, sy, sz), _ => unreachable!("Found unexpected value for scale"), } diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index b0a121fd6e9..15e0ea2d274 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -106,9 +106,18 @@ impl TransformOrigin { } } +fn is_same(x: &N, y: &N) -> bool { + x == y +} + #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)] /// A single operation in the list of a `transform` value -pub enum TransformOperation { +pub enum TransformOperation +where + Angle: Zero, + LengthPercentage: Zero, + Number: PartialEq, +{ /// Represents a 2D 2x3 matrix. Matrix(Matrix), /// Represents a 3D 4x4 matrix. @@ -119,7 +128,7 @@ pub enum TransformOperation { /// /// Syntax can be skew(angle) or skew(angle, angle) #[css(comma, function)] - Skew(Angle, Option), + Skew(Angle, #[css(skip_if = "Zero::is_zero")] Angle), /// skewX(angle) #[css(function = "skewX")] SkewX(Angle), @@ -128,7 +137,7 @@ pub enum TransformOperation { SkewY(Angle), /// translate(x, y) or translate(x) #[css(comma, function)] - Translate(LengthPercentage, Option), + Translate(LengthPercentage, #[css(skip_if = "Zero::is_zero")] LengthPercentage), /// translateX(x) #[css(function = "translateX")] TranslateX(LengthPercentage), @@ -143,14 +152,9 @@ pub enum TransformOperation { Translate3D(LengthPercentage, LengthPercentage, Length), /// A 2D scaling factor. /// - /// `scale(2)` is parsed as `Scale(Number::new(2.0), None)` and is equivalent to - /// writing `scale(2, 2)` (`Scale(Number::new(2.0), Some(Number::new(2.0)))`). - /// - /// Negative values are allowed and flip the element. - /// /// Syntax can be scale(factor) or scale(factor, factor) #[css(comma, function)] - Scale(Number, Option), + Scale(Number, #[css(contextual_skip_if = "is_same")] Number), /// scaleX(factor) #[css(function = "scaleX")] ScaleX(Number), @@ -214,6 +218,10 @@ pub struct Transform(#[css(if_empty = "none", iterable)] pub Vec); impl TransformOperation +where + Angle: Zero, + LengthPercentage: Zero, + Number: PartialEq, { /// Check if it is any rotate function. pub fn is_rotate(&self) -> bool { @@ -333,10 +341,10 @@ impl ToRadians for SpecifiedAngle { impl ToMatrix for TransformOperation where - Angle: ToRadians + Copy, - Number: Copy + Into + Into, + Angle: Zero + ToRadians + Copy, + Number: PartialEq + Copy + Into + Into, Length: ToAbsoluteLength, - LoP: ToAbsoluteLength, + LoP: Zero + ToAbsoluteLength, { #[inline] fn is_3d(&self) -> bool { @@ -389,7 +397,7 @@ where m.cast() }, Scale3D(sx, sy, sz) => Transform3D::create_scale(sx.into(), sy.into(), sz.into()), - Scale(sx, sy) => Transform3D::create_scale(sx.into(), sy.unwrap_or(sx).into(), 1.), + Scale(sx, sy) => Transform3D::create_scale(sx.into(), sy.into(), 1.), ScaleX(s) => Transform3D::create_scale(s.into(), 1., 1.), ScaleY(s) => Transform3D::create_scale(1., s.into(), 1.), ScaleZ(s) => Transform3D::create_scale(1., 1., s.into()), @@ -398,12 +406,12 @@ where let ty = ty.to_pixel_length(reference_height)? as f64; Transform3D::create_translation(tx, ty, tz.to_pixel_length(None)? as f64) }, - Translate(ref tx, Some(ref ty)) => { + Translate(ref tx, ref ty) => { let tx = tx.to_pixel_length(reference_width)? as f64; let ty = ty.to_pixel_length(reference_height)? as f64; Transform3D::create_translation(tx, ty, 0.) }, - TranslateX(ref t) | Translate(ref t, None) => { + TranslateX(ref t) => { let t = t.to_pixel_length(reference_width)? as f64; Transform3D::create_translation(t, 0., 0.) }, @@ -416,7 +424,7 @@ where }, Skew(theta_x, theta_y) => Transform3D::create_skew( euclid::Angle::radians(theta_x.radians64()), - euclid::Angle::radians(theta_y.map_or(0., |a| a.radians64())), + euclid::Angle::radians(theta_y.radians64()), ), SkewX(theta) => Transform3D::create_skew( euclid::Angle::radians(theta.radians64()), diff --git a/components/style/values/specified/angle.rs b/components/style/values/specified/angle.rs index 98233608ebb..01eb58368b9 100644 --- a/components/style/values/specified/angle.rs +++ b/components/style/values/specified/angle.rs @@ -9,6 +9,7 @@ use crate::values::computed::angle::Angle as ComputedAngle; use crate::values::computed::{Context, ToComputedValue}; use crate::values::specified::calc::CalcNode; use crate::values::CSSFloat; +use crate::Zero; use cssparser::{Parser, Token}; use std::f32::consts::PI; use std::fmt::{self, Write}; @@ -32,6 +33,21 @@ pub enum AngleDimension { Turn(CSSFloat), } +impl Zero for AngleDimension { + fn zero() -> Self { + AngleDimension::Deg(0.) + } + + fn is_zero(&self) -> bool { + match *self { + AngleDimension::Deg(ref f) | + AngleDimension::Grad(ref f) | + AngleDimension::Rad(ref f) | + AngleDimension::Turn(ref f) => *f == 0., + } + } +} + impl AngleDimension { /// Returns the amount of degrees this angle represents. #[inline] @@ -58,6 +74,19 @@ pub struct Angle { was_calc: bool, } +impl Zero for Angle { + fn zero() -> Self { + Self { + value: Zero::zero(), + was_calc: false, + } + } + + fn is_zero(&self) -> bool { + self.value.is_zero() + } +} + impl ToCss for Angle { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index 4758a0c4c05..92ca953159f 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -109,9 +109,9 @@ impl Transform { let sx = specified::LengthPercentage::parse(context, input)?; if input.try(|input| input.expect_comma()).is_ok() { let sy = specified::LengthPercentage::parse(context, input)?; - Ok(generic::TransformOperation::Translate(sx, Some(sy))) + Ok(generic::TransformOperation::Translate(sx, sy)) } else { - Ok(generic::TransformOperation::Translate(sx, None)) + Ok(generic::TransformOperation::Translate(sx, Zero::zero())) } }, "translatex" => { @@ -138,9 +138,9 @@ impl Transform { let sx = Number::parse(context, input)?; if input.try(|input| input.expect_comma()).is_ok() { let sy = Number::parse(context, input)?; - Ok(generic::TransformOperation::Scale(sx, Some(sy))) + Ok(generic::TransformOperation::Scale(sx, sy)) } else { - Ok(generic::TransformOperation::Scale(sx, None)) + Ok(generic::TransformOperation::Scale(sx, sx)) } }, "scalex" => { @@ -194,9 +194,9 @@ impl Transform { let ax = specified::Angle::parse_with_unitless(context, input)?; if input.try(|input| input.expect_comma()).is_ok() { let ay = specified::Angle::parse_with_unitless(context, input)?; - Ok(generic::TransformOperation::Skew(ax, Some(ay))) + Ok(generic::TransformOperation::Skew(ax, ay)) } else { - Ok(generic::TransformOperation::Skew(ax, None)) + Ok(generic::TransformOperation::Skew(ax, Zero::zero())) } }, "skewx" => { 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 15/31] 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 205f5478316..9b7159fcba8 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 a22e745963e..8f4fcaec5c9 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 f11a9a7b6b2..f61c8754b5e 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 e921967f46d..783e553d1ce 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 029c5681318..f339bf83432 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 { From 33814a9afefcc2e39d11598ceca4c354e422bd36 Mon Sep 17 00:00:00 2001 From: Daniel Holbert Date: Wed, 6 Mar 2019 20:44:37 +0000 Subject: [PATCH 16/31] style: Drop css parser support for 'contain:style'. We're not sure we're going to ship support for this value, and it's been marked as 'at-risk' in the spec, so let's stop pretending we support it in the parser. Spec issue: https://github.com/w3c/csswg-drafts/issues/3280 Differential Revision: https://phabricator.services.mozilla.com/D22203 --- components/style/properties/gecko.mako.rs | 8 -------- components/style/values/specified/box.rs | 20 ++++++++------------ 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 8f4fcaec5c9..bf7bc9aceec 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3177,7 +3177,6 @@ fn static_assert() { use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_SIZE; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_LAYOUT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_STYLE; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_PAINT; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_ALL_BITS; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT_BITS; @@ -3201,9 +3200,6 @@ fn static_assert() { if v.contains(SpecifiedValue::LAYOUT) { bitfield |= NS_STYLE_CONTAIN_LAYOUT; } - if v.contains(SpecifiedValue::STYLE) { - bitfield |= NS_STYLE_CONTAIN_STYLE; - } if v.contains(SpecifiedValue::PAINT) { bitfield |= NS_STYLE_CONTAIN_PAINT; } @@ -3219,7 +3215,6 @@ fn static_assert() { use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_SIZE; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_LAYOUT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_STYLE; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_PAINT; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_ALL_BITS; use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT_BITS; @@ -3249,9 +3244,6 @@ fn static_assert() { if gecko_flags & (NS_STYLE_CONTAIN_LAYOUT as u8) != 0 { servo_flags.insert(SpecifiedValue::LAYOUT); } - if gecko_flags & (NS_STYLE_CONTAIN_STYLE as u8) != 0 { - servo_flags.insert(SpecifiedValue::STYLE); - } if gecko_flags & (NS_STYLE_CONTAIN_PAINT as u8) != 0 { servo_flags.insert(SpecifiedValue::PAINT); } diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 2495cca3d6c..d4b66e42ab9 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -752,25 +752,23 @@ pub fn assert_touch_action_matches() { bitflags! { #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue)] - #[value_info(other_values = "none,strict,content,size,layout,style,paint")] + #[value_info(other_values = "none,strict,content,size,layout,paint")] /// Constants for contain: https://drafts.csswg.org/css-contain/#contain-property pub struct Contain: u8 { /// 'size' variant, turns on size containment const SIZE = 0x01; /// `layout` variant, turns on layout containment const LAYOUT = 0x02; - /// `style` variant, turns on style containment - const STYLE = 0x04; /// `paint` variant, turns on paint containment - const PAINT = 0x08; + const PAINT = 0x04; /// `strict` variant, turns on all types of containment - const STRICT = 0x10; - /// 'content' variant, turns on style, layout, and paint containment - const CONTENT = 0x20; + const STRICT = 0x08; + /// 'content' variant, turns on layout and paint containment + const CONTENT = 0x10; /// variant with all the bits that contain: strict turns on - const STRICT_BITS = Contain::LAYOUT.bits | Contain::STYLE.bits | Contain::PAINT.bits | Contain::SIZE.bits; + const STRICT_BITS = Contain::LAYOUT.bits | Contain::PAINT.bits | Contain::SIZE.bits; /// variant with all the bits that contain: content turns on - const CONTENT_BITS = Contain::STYLE.bits | Contain::LAYOUT.bits | Contain::PAINT.bits; + const CONTENT_BITS = Contain::LAYOUT.bits | Contain::PAINT.bits; } } @@ -803,7 +801,6 @@ impl ToCss for Contain { } maybe_write_value!(Contain::SIZE => "size"); maybe_write_value!(Contain::LAYOUT => "layout"); - maybe_write_value!(Contain::STYLE => "style"); maybe_write_value!(Contain::PAINT => "paint"); debug_assert!(has_any); @@ -812,7 +809,7 @@ impl ToCss for Contain { } impl Parse for Contain { - /// none | strict | content | [ size || layout || style || paint ] + /// none | strict | content | [ size || layout || paint ] fn parse<'i, 't>( _context: &ParserContext, input: &mut Parser<'i, 't>, @@ -822,7 +819,6 @@ impl Parse for Contain { let flag = match_ignore_ascii_case! { &name, "size" => Some(Contain::SIZE), "layout" => Some(Contain::LAYOUT), - "style" => Some(Contain::STYLE), "paint" => Some(Contain::PAINT), "strict" if result.is_empty() => return Ok(Contain::STRICT | Contain::STRICT_BITS), "content" if result.is_empty() => return Ok(Contain::CONTENT | Contain::CONTENT_BITS), From 4e3e4c106a6ce694c028166956ab03cb3b3e7881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 6 Mar 2019 21:34:30 +0000 Subject: [PATCH 17/31] style: Refactor preference stylesheet prefs to not require a pres context. We really only have two sets of prefs, one for chrome-like documents (stuff in chrome docshells + chrome-origin images), and one for the rest. Differential Revision: https://phabricator.services.mozilla.com/D20946 --- components/style/gecko/data.rs | 5 ++--- components/style/gecko/media_queries.rs | 16 +++++++++++----- components/style/gecko/wrapper.rs | 2 +- components/style/values/specified/color.rs | 12 ++++++------ 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 80e34fe2e7e..e48656ee80f 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -150,7 +150,7 @@ impl PerDocumentStyleData { // right now not always honored, see bug 1405543... // // Should we just force XBL Stylists to be NoQuirks? - let quirks_mode = unsafe { (*device.pres_context().mDocument.mRawPtr).mCompatMode }; + let quirks_mode = device.document().mCompatMode; PerDocumentStyleData(AtomicRefCell::new(PerDocumentStyleDataImpl { stylist: Stylist::new(device, quirks_mode.into()), @@ -191,8 +191,7 @@ impl PerDocumentStyleDataImpl { /// Returns whether visited styles are enabled. #[inline] pub fn visited_styles_enabled(&self) -> bool { - let doc = self.stylist.device().pres_context().mDocument.mRawPtr; - unsafe { bindings::Gecko_VisitedStylesEnabled(doc) } + unsafe { bindings::Gecko_VisitedStylesEnabled(self.stylist.device().document()) } } /// Measure heap usage. diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 8fa2258eaaf..2d146c5dcf0 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -83,14 +83,14 @@ impl Device { /// Trivially constructs a new `Device`. pub fn new(pres_context: RawGeckoPresContextBorrowed) -> Self { assert!(!pres_context.is_null()); + let doc = unsafe { &*(*pres_context).mDocument.mRawPtr }; + let prefs = unsafe { &*bindings::Gecko_GetPrefSheetPrefs(doc) }; Device { pres_context, - default_values: ComputedValues::default_values(unsafe { - &*(*pres_context).mDocument.mRawPtr - }), + default_values: ComputedValues::default_values(doc), // FIXME(bz): Seems dubious? root_font_size: AtomicIsize::new(FontSize::medium().size().0 as isize), - body_text_color: AtomicUsize::new(unsafe { &*pres_context }.mDefaultColor as usize), + body_text_color: AtomicUsize::new(prefs.mDefaultColor as usize), used_root_font_size: AtomicBool::new(false), used_viewport_size: AtomicBool::new(false), environment: CssEnvironment, @@ -168,6 +168,12 @@ impl Device { unsafe { &*self.pres_context().mDocument.mRawPtr } } + /// Gets the preference stylesheet prefs for our document. + #[inline] + pub fn pref_sheet_prefs(&self) -> &structs::PreferenceSheet_Prefs { + unsafe { &*bindings::Gecko_GetPrefSheetPrefs(self.document()) } + } + /// Recreates the default computed values. pub fn reset_computed_values(&mut self) { self.default_values = ComputedValues::default_values(self.document()); @@ -243,7 +249,7 @@ impl Device { /// Returns the default background color. pub fn default_background_color(&self) -> RGBA { - convert_nscolor_to_rgba(self.pres_context().mBackgroundColor) + convert_nscolor_to_rgba(self.pref_sheet_prefs().mDefaultBackgroundColor) } /// Applies text zoom to a font-size or line-height value (see nsStyleFont::ZoomText). diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 97f0f887a00..4db2da6d55b 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1243,7 +1243,7 @@ impl<'le> TElement for GeckoElement<'le> { fn owner_doc_matches_for_testing(&self, device: &Device) -> bool { self.as_node().owner_doc().0 as *const structs::Document == - device.pres_context().mDocument.mRawPtr + device.document() as *const _ } fn style_attribute(&self) -> Option>> { diff --git a/components/style/values/specified/color.rs b/components/style/values/specified/color.rs index 7a42e834236..9ef9ab57f0a 100644 --- a/components/style/values/specified/color.rs +++ b/components/style/values/specified/color.rs @@ -351,13 +351,13 @@ impl Color { Color::Special(special) => { use self::gecko::SpecialColorKeyword as Keyword; _context.map(|context| { - let pres_context = context.device().pres_context(); + let prefs = context.device().pref_sheet_prefs(); convert_nscolor_to_computedcolor(match special { - Keyword::MozDefaultColor => pres_context.mDefaultColor, - Keyword::MozDefaultBackgroundColor => pres_context.mBackgroundColor, - Keyword::MozHyperlinktext => pres_context.mLinkColor, - Keyword::MozActivehyperlinktext => pres_context.mActiveLinkColor, - Keyword::MozVisitedhyperlinktext => pres_context.mVisitedLinkColor, + Keyword::MozDefaultColor => prefs.mDefaultColor, + Keyword::MozDefaultBackgroundColor => prefs.mDefaultBackgroundColor, + Keyword::MozHyperlinktext => prefs.mLinkColor, + Keyword::MozActivehyperlinktext => prefs.mActiveLinkColor, + Keyword::MozVisitedhyperlinktext => prefs.mVisitedLinkColor, }) }) }, From e723a5b7d6b10b48014253bd11dddc96ff57aad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 6 Mar 2019 21:36:12 +0000 Subject: [PATCH 18/31] style: Make the pres context optional in the style system. Differential Revision: https://phabricator.services.mozilla.com/D21239 --- components/style/gecko/data.rs | 8 +- components/style/gecko/media_features.rs | 13 ++-- components/style/gecko/media_queries.rs | 96 +++++++++++++++++------- components/style/gecko/wrapper.rs | 6 +- 4 files changed, 85 insertions(+), 38 deletions(-) diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index e48656ee80f..b70f376d7e6 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -7,7 +7,7 @@ use crate::context::QuirksMode; use crate::dom::TElement; use crate::gecko_bindings::bindings::{self, RawServoStyleSet}; -use crate::gecko_bindings::structs::{RawGeckoPresContextBorrowed, ServoStyleSetSizes}; +use crate::gecko_bindings::structs::{self, ServoStyleSetSizes}; use crate::gecko_bindings::structs::{StyleSheet as DomStyleSheet, StyleSheetInfo}; use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasBoxFFI, HasFFI, HasSimpleFFI}; use crate::invalidation::media_queries::{MediaListKey, ToMediaListKey}; @@ -142,9 +142,9 @@ pub struct PerDocumentStyleDataImpl { pub struct PerDocumentStyleData(AtomicRefCell); impl PerDocumentStyleData { - /// Create a dummy `PerDocumentStyleData`. - pub fn new(pres_context: RawGeckoPresContextBorrowed) -> Self { - let device = Device::new(pres_context); + /// Create a `PerDocumentStyleData`. + pub fn new(document: *const structs::Document) -> Self { + let device = Device::new(document); // FIXME(emilio, tlin): How is this supposed to work with XBL? This is // right now not always honored, see bug 1405543... diff --git a/components/style/gecko/media_features.rs b/components/style/gecko/media_features.rs index 2547f2a45d4..9c48836b4d9 100644 --- a/components/style/gecko/media_features.rs +++ b/components/style/gecko/media_features.rs @@ -17,12 +17,13 @@ use app_units::Au; use euclid::Size2D; fn viewport_size(device: &Device) -> Size2D { - let pc = device.pres_context(); - if pc.mIsRootPaginatedDocument() != 0 { - // We want the page size, including unprintable areas and margins. - // FIXME(emilio, bug 1414600): Not quite! - let area = &pc.mPageSize; - return Size2D::new(Au(area.width), Au(area.height)); + if let Some(pc) = device.pres_context() { + if pc.mIsRootPaginatedDocument() != 0 { + // We want the page size, including unprintable areas and margins. + // FIXME(emilio, bug 1414600): Not quite! + let area = &pc.mPageSize; + return Size2D::new(Au(area.width), Au(area.height)); + } } device.au_viewport_size() } diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 2d146c5dcf0..3fef4974c33 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -8,7 +8,6 @@ use crate::custom_properties::CssEnvironment; use crate::gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor}; use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs; -use crate::gecko_bindings::structs::{nsPresContext, RawGeckoPresContextBorrowed}; use crate::media_queries::MediaType; use crate::properties::ComputedValues; use crate::string_cache::Atom; @@ -28,10 +27,9 @@ use style_traits::{CSSPixel, DevicePixel}; /// The `Device` in Gecko wraps a pres context, has a default values computed, /// and contains all the viewport rule state. pub struct Device { - /// NB: The pres context lifetime is tied to the styleset, who owns the - /// stylist, and thus the `Device`, so having a raw pres context pointer - /// here is fine. - pres_context: RawGeckoPresContextBorrowed, + /// NB: The document owns the styleset, who owns the stylist, and thus the + /// `Device`, so having a raw document pointer here is fine. + document: *const structs::Document, default_values: Arc, /// The font size of the root element /// This is set when computing the style of the root @@ -81,12 +79,12 @@ unsafe impl Send for Device {} impl Device { /// Trivially constructs a new `Device`. - pub fn new(pres_context: RawGeckoPresContextBorrowed) -> Self { - assert!(!pres_context.is_null()); - let doc = unsafe { &*(*pres_context).mDocument.mRawPtr }; + pub fn new(document: *const structs::Document) -> Self { + assert!(!document.is_null()); + let doc = unsafe { &*document }; let prefs = unsafe { &*bindings::Gecko_GetPrefSheetPrefs(doc) }; Device { - pres_context, + document, default_values: ComputedValues::default_values(doc), // FIXME(bz): Seems dubious? root_font_size: AtomicIsize::new(FontSize::medium().size().0 as isize), @@ -112,9 +110,14 @@ impl Device { /// Whether any animation name may be referenced from the style of any /// element. pub fn animation_name_may_be_referenced(&self, name: &KeyframesName) -> bool { + let pc = match self.pres_context() { + Some(pc) => pc, + None => return false, + }; + unsafe { bindings::Gecko_AnimationNameMayBeReferencedFromStyle( - self.pres_context(), + pc, name.as_atom().as_ptr(), ) } @@ -156,16 +159,18 @@ impl Device { convert_nscolor_to_rgba(self.body_text_color.load(Ordering::Relaxed) as u32) } - /// Gets the pres context associated with this document. - #[inline] - pub fn pres_context(&self) -> &nsPresContext { - unsafe { &*self.pres_context } - } - /// Gets the document pointer. #[inline] pub fn document(&self) -> &structs::Document { - unsafe { &*self.pres_context().mDocument.mRawPtr } + unsafe { &*self.document } + } + + /// Gets the pres context associated with this document. + #[inline] + pub fn pres_context(&self) -> Option<&structs::nsPresContext> { + unsafe { + self.document().mPresShell.as_ref()?.mPresContext.mRawPtr.as_ref() + } } /// Gets the preference stylesheet prefs for our document. @@ -201,13 +206,17 @@ impl Device { /// Returns the current media type of the device. pub fn media_type(&self) -> MediaType { + let pc = match self.pres_context() { + Some(pc) => pc, + None => return MediaType::screen(), + }; + // Gecko allows emulating random media with mIsEmulatingMedia and // mMediaEmulated. - let context = self.pres_context(); - let medium_to_use = if context.mIsEmulatingMedia() != 0 { - context.mMediaEmulated.mRawPtr + let medium_to_use = if pc.mIsEmulatingMedia() != 0 { + pc.mMediaEmulated.mRawPtr } else { - context.mMedium + pc.mMedium }; MediaType(CustomIdent(unsafe { Atom::from_raw(medium_to_use) })) @@ -215,7 +224,11 @@ impl Device { /// Returns the current viewport size in app units. pub fn au_viewport_size(&self) -> Size2D { - let area = &self.pres_context().mVisibleArea; + let pc = match self.pres_context() { + Some(pc) => pc, + None => return Size2D::new(Au(0), Au(0)), + }; + let area = &pc.mVisibleArea; Size2D::new(Au(area.width), Au(area.height)) } @@ -233,18 +246,37 @@ impl Device { /// Returns the device pixel ratio. pub fn device_pixel_ratio(&self) -> TypedScale { - let override_dppx = self.pres_context().mOverrideDPPX; + let pc = match self.pres_context() { + Some(pc) => pc, + None => return TypedScale::new(1.), + }; + + let override_dppx = pc.mOverrideDPPX; if override_dppx > 0.0 { return TypedScale::new(override_dppx); } - let au_per_dpx = self.pres_context().mCurAppUnitsPerDevPixel as f32; + + let au_per_dpx = pc.mCurAppUnitsPerDevPixel as f32; let au_per_px = AU_PER_PX as f32; TypedScale::new(au_per_px / au_per_dpx) } /// Returns whether document colors are enabled. + #[inline] pub fn use_document_colors(&self) -> bool { - self.pres_context().mUseDocumentColors() != 0 + let doc = self.document(); + if doc.mIsBeingUsedAsImage() { + return true; + } + let document_color_use = unsafe { + structs::StaticPrefs_sVarCache_browser_display_document_color_use + }; + let prefs = self.pref_sheet_prefs(); + match document_color_use { + 1 => true, + 2 => prefs.mIsChrome, + _ => !prefs.mUseAccessibilityTheme, + } } /// Returns the default background color. @@ -252,15 +284,25 @@ impl Device { convert_nscolor_to_rgba(self.pref_sheet_prefs().mDefaultBackgroundColor) } + /// Returns the current effective text zoom. + #[inline] + fn effective_text_zoom(&self) -> f32 { + let pc = match self.pres_context() { + Some(pc) => pc, + None => return 1., + }; + pc.mEffectiveTextZoom + } + /// Applies text zoom to a font-size or line-height value (see nsStyleFont::ZoomText). #[inline] pub fn zoom_text(&self, size: Au) -> Au { - size.scale_by(self.pres_context().mEffectiveTextZoom) + size.scale_by(self.effective_text_zoom()) } /// Un-apply text zoom. #[inline] pub fn unzoom_text(&self, size: Au) -> Au { - size.scale_by(1. / self.pres_context().mEffectiveTextZoom) + size.scale_by(1. / self.effective_text_zoom()) } } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 4db2da6d55b..f5bb7d8a94c 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1044,9 +1044,13 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { device: &Device, ) -> FontMetricsQueryResult { use crate::gecko_bindings::bindings::Gecko_GetFontMetrics; + let pc = match device.pres_context() { + Some(pc) => pc, + None => return FontMetricsQueryResult::NotAvailable, + }; let gecko_metrics = unsafe { Gecko_GetFontMetrics( - device.pres_context(), + pc, wm.is_vertical() && !wm.is_sideways(), font.gecko(), font_size.0, From e0b3e5f691cd43fc24f352f0acb29c50fae18b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 6 Mar 2019 21:37:48 +0000 Subject: [PATCH 19/31] style: Derive more stuff for clip rects. I feel a bit weird for using LenghtPercentageOrAuto to implement LengthOrAuto, but I don't think much other code will use it so it seemed a bit better to me. Differential Revision: https://phabricator.services.mozilla.com/D21863 --- components/style/properties/gecko.mako.rs | 143 ++++++++++-------- .../helpers/animated_properties.mako.rs | 28 ++-- .../style/properties/longhands/column.mako.rs | 4 +- components/style/values/computed/length.rs | 10 +- components/style/values/computed/mod.rs | 54 +------ components/style/values/generics/mod.rs | 11 ++ components/style/values/specified/length.rs | 25 ++- components/style/values/specified/mod.rs | 122 ++------------- 8 files changed, 143 insertions(+), 254 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index bf7bc9aceec..6a4d4d87a99 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3748,6 +3748,7 @@ fn static_assert() { #[allow(non_snake_case)] pub fn set__moz_image_region(&mut self, v: longhands::_moz_image_region::computed_value::T) { use crate::values::Either; + use crate::values::generics::length::LengthPercentageOrAuto::*; match v { Either::Second(_auto) => { @@ -3757,15 +3758,21 @@ fn static_assert() { self.gecko.mImageRegion.height = 0; } Either::First(rect) => { - self.gecko.mImageRegion.x = rect.left.map(Au::from).unwrap_or(Au(0)).0; - self.gecko.mImageRegion.y = rect.top.map(Au::from).unwrap_or(Au(0)).0; + self.gecko.mImageRegion.x = match rect.left { + LengthPercentage(v) => v.to_i32_au(), + Auto => 0, + }; + self.gecko.mImageRegion.y = match rect.top { + LengthPercentage(v) => v.to_i32_au(), + Auto => 0, + }; self.gecko.mImageRegion.height = match rect.bottom { - Some(value) => (Au::from(value) - Au(self.gecko.mImageRegion.y)).0, - None => 0, + LengthPercentage(value) => (Au::from(value) - Au(self.gecko.mImageRegion.y)).0, + Auto => 0, }; self.gecko.mImageRegion.width = match rect.right { - Some(value) => (Au::from(value) - Au(self.gecko.mImageRegion.x)).0, - None => 0, + LengthPercentage(value) => (Au::from(value) - Au(self.gecko.mImageRegion.x)).0, + Auto => 0, }; } } @@ -3774,6 +3781,7 @@ fn static_assert() { #[allow(non_snake_case)] pub fn clone__moz_image_region(&self) -> longhands::_moz_image_region::computed_value::T { use crate::values::{Auto, Either}; + use crate::values::generics::length::LengthPercentageOrAuto::*; use crate::values::computed::ClipRect; // There is no ideal way to detect auto type for structs::nsRect and its components, so @@ -3786,10 +3794,10 @@ fn static_assert() { } Either::First(ClipRect { - top: Some(Au(self.gecko.mImageRegion.y).into()), - right: Some(Au(self.gecko.mImageRegion.width + self.gecko.mImageRegion.x).into()), - bottom: Some(Au(self.gecko.mImageRegion.height + self.gecko.mImageRegion.y).into()), - left: Some(Au(self.gecko.mImageRegion.x).into()), + top: LengthPercentage(Au(self.gecko.mImageRegion.y).into()), + right: LengthPercentage(Au(self.gecko.mImageRegion.width + self.gecko.mImageRegion.x).into()), + bottom: LengthPercentage(Au(self.gecko.mImageRegion.height + self.gecko.mImageRegion.y).into()), + left: LengthPercentage(Au(self.gecko.mImageRegion.x).into()), }) } @@ -3846,38 +3854,43 @@ fn static_assert() { use crate::gecko_bindings::structs::NS_STYLE_CLIP_TOP_AUTO; use crate::gecko_bindings::structs::NS_STYLE_CLIP_RIGHT_AUTO; use crate::gecko_bindings::structs::NS_STYLE_CLIP_BOTTOM_AUTO; + use crate::values::generics::length::LengthPercentageOrAuto::*; use crate::values::Either; match v { Either::First(rect) => { self.gecko.mClipFlags = NS_STYLE_CLIP_RECT as u8; - if let Some(left) = rect.left { - self.gecko.mClip.x = left.to_i32_au(); - } else { - self.gecko.mClip.x = 0; - self.gecko.mClipFlags |= NS_STYLE_CLIP_LEFT_AUTO as u8; - } + self.gecko.mClip.x = match rect.left { + LengthPercentage(l) => l.to_i32_au(), + Auto => { + self.gecko.mClipFlags |= NS_STYLE_CLIP_LEFT_AUTO as u8; + 0 + } + }; - if let Some(top) = rect.top { - self.gecko.mClip.y = top.to_i32_au(); - } else { - self.gecko.mClip.y = 0; - self.gecko.mClipFlags |= NS_STYLE_CLIP_TOP_AUTO as u8; - } + self.gecko.mClip.y = match rect.top { + LengthPercentage(l) => l.to_i32_au(), + Auto => { + self.gecko.mClipFlags |= NS_STYLE_CLIP_TOP_AUTO as u8; + 0 + } + }; - if let Some(bottom) = rect.bottom { - self.gecko.mClip.height = (Au::from(bottom) - Au(self.gecko.mClip.y)).0; - } else { - self.gecko.mClip.height = 1 << 30; // NS_MAXSIZE - self.gecko.mClipFlags |= NS_STYLE_CLIP_BOTTOM_AUTO as u8; - } + self.gecko.mClip.height = match rect.bottom { + LengthPercentage(l) => (Au::from(l) - Au(self.gecko.mClip.y)).0, + Auto => { + self.gecko.mClipFlags |= NS_STYLE_CLIP_BOTTOM_AUTO as u8; + 1 << 30 // NS_MAXSIZE + } + }; - if let Some(right) = rect.right { - self.gecko.mClip.width = (Au::from(right) - Au(self.gecko.mClip.x)).0; - } else { - self.gecko.mClip.width = 1 << 30; // NS_MAXSIZE - self.gecko.mClipFlags |= NS_STYLE_CLIP_RIGHT_AUTO as u8; - } + self.gecko.mClip.width = match rect.right { + LengthPercentage(l) => (Au::from(l) - Au(self.gecko.mClip.x)).0, + Auto => { + self.gecko.mClipFlags |= NS_STYLE_CLIP_RIGHT_AUTO as u8; + 1 << 30 // NS_MAXSIZE + } + }; }, Either::Second(_auto) => { self.gecko.mClipFlags = NS_STYLE_CLIP_AUTO as u8; @@ -3904,42 +3917,42 @@ fn static_assert() { use crate::gecko_bindings::structs::NS_STYLE_CLIP_LEFT_AUTO; use crate::gecko_bindings::structs::NS_STYLE_CLIP_RIGHT_AUTO; use crate::gecko_bindings::structs::NS_STYLE_CLIP_TOP_AUTO; + use crate::values::generics::length::LengthPercentageOrAuto::*; use crate::values::computed::{ClipRect, ClipRectOrAuto}; use crate::values::Either; if self.gecko.mClipFlags == NS_STYLE_CLIP_AUTO as u8 { - ClipRectOrAuto::auto() - } else { - let left = if self.gecko.mClipFlags & NS_STYLE_CLIP_LEFT_AUTO as u8 != 0 { - debug_assert_eq!(self.gecko.mClip.x, 0); - None - } else { - Some(Au(self.gecko.mClip.x).into()) - }; - - let top = if self.gecko.mClipFlags & NS_STYLE_CLIP_TOP_AUTO as u8 != 0 { - debug_assert_eq!(self.gecko.mClip.y, 0); - None - } else { - Some(Au(self.gecko.mClip.y).into()) - }; - - let bottom = if self.gecko.mClipFlags & NS_STYLE_CLIP_BOTTOM_AUTO as u8 != 0 { - debug_assert_eq!(self.gecko.mClip.height, 1 << 30); // NS_MAXSIZE - None - } else { - Some(Au(self.gecko.mClip.y + self.gecko.mClip.height).into()) - }; - - let right = if self.gecko.mClipFlags & NS_STYLE_CLIP_RIGHT_AUTO as u8 != 0 { - debug_assert_eq!(self.gecko.mClip.width, 1 << 30); // NS_MAXSIZE - None - } else { - Some(Au(self.gecko.mClip.x + self.gecko.mClip.width).into()) - }; - - Either::First(ClipRect { top, right, bottom, left }) + return ClipRectOrAuto::auto() } + let left = if self.gecko.mClipFlags & NS_STYLE_CLIP_LEFT_AUTO as u8 != 0 { + debug_assert_eq!(self.gecko.mClip.x, 0); + Auto + } else { + LengthPercentage(Au(self.gecko.mClip.x).into()) + }; + + let top = if self.gecko.mClipFlags & NS_STYLE_CLIP_TOP_AUTO as u8 != 0 { + debug_assert_eq!(self.gecko.mClip.y, 0); + Auto + } else { + LengthPercentage(Au(self.gecko.mClip.y).into()) + }; + + let bottom = if self.gecko.mClipFlags & NS_STYLE_CLIP_BOTTOM_AUTO as u8 != 0 { + debug_assert_eq!(self.gecko.mClip.height, 1 << 30); // NS_MAXSIZE + Auto + } else { + LengthPercentage(Au(self.gecko.mClip.y + self.gecko.mClip.height).into()) + }; + + let right = if self.gecko.mClipFlags & NS_STYLE_CLIP_RIGHT_AUTO as u8 != 0 { + debug_assert_eq!(self.gecko.mClip.width, 1 << 30); // NS_MAXSIZE + Auto + } else { + LengthPercentage(Au(self.gecko.mClip.x + self.gecko.mClip.width).into()) + }; + + Either::First(ClipRect { top, right, bottom, left }) } <% diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index fcc6e48b41b..5669101351d 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -802,29 +802,29 @@ impl ToAnimatedZero for Visibility { impl Animate for ClipRect { #[inline] fn animate(&self, other: &Self, procedure: Procedure) -> Result { - use crate::values::computed::Length; - let animate_component = |this: &Option, other: &Option| { - match (this.animate(other, procedure)?, procedure) { - (None, Procedure::Interpolate { .. }) => Ok(None), - (None, _) => Err(()), - (result, _) => Ok(result), + use crate::values::computed::LengthOrAuto; + let animate_component = |this: &LengthOrAuto, other: &LengthOrAuto| { + let result = this.animate(other, procedure)?; + if let Procedure::Interpolate { .. } = procedure { + return Ok(result); } + if result.is_auto() { + // FIXME(emilio): Why? A couple SMIL tests fail without this, + // but it seems extremely fishy. + return Err(()); + } + Ok(result) }; Ok(ClipRect { - top: animate_component(&self.top, &other.top)?, - right: animate_component(&self.right, &other.right)?, + top: animate_component(&self.top, &other.top)?, + right: animate_component(&self.right, &other.right)?, bottom: animate_component(&self.bottom, &other.bottom)?, - left: animate_component(&self.left, &other.left)?, + left: animate_component(&self.left, &other.left)?, }) } } -impl ToAnimatedZero for ClipRect { - #[inline] - fn to_animated_zero(&self) -> Result { Err(()) } -} - <% FILTER_FUNCTIONS = [ 'Blur', 'Brightness', 'Contrast', 'Grayscale', 'HueRotate', 'Invert', 'Opacity', 'Saturate', diff --git a/components/style/properties/longhands/column.mako.rs b/components/style/properties/longhands/column.mako.rs index 90c37a9710f..f61b78acf46 100644 --- a/components/style/properties/longhands/column.mako.rs +++ b/components/style/properties/longhands/column.mako.rs @@ -9,8 +9,8 @@ ${helpers.predefined_type( "column-width", "length::NonNegativeLengthOrAuto", - "Either::Second(Auto)", - initial_specified_value="Either::Second(Auto)", + "computed::length::NonNegativeLengthOrAuto::auto()", + initial_specified_value="specified::length::NonNegativeLengthOrAuto::auto()", extra_prefixes="moz", animation_value_type="NonNegativeLengthOrAuto", servo_pref="layout.columns.enabled", diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 38cecd7339d..1262e4dd1d5 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -15,7 +15,7 @@ use crate::values::generics::length::{ use crate::values::generics::NonNegative; use crate::values::specified::length::ViewportPercentageLength; use crate::values::specified::length::{AbsoluteLength, FontBaseSize, FontRelativeLength}; -use crate::values::{specified, Auto, CSSFloat, Either, Normal}; +use crate::values::{specified, CSSFloat, Either, Normal}; use crate::Zero; use app_units::Au; use ordered_float::NotNan; @@ -701,7 +701,10 @@ impl From for CSSPixelLength { pub type Length = CSSPixelLength; /// Either a computed `` or the `auto` keyword. -pub type LengthOrAuto = Either; +pub type LengthOrAuto = generics::GenericLengthPercentageOrAuto; + +/// Either a non-negative `` or the `auto` keyword. +pub type NonNegativeLengthOrAuto = generics::GenericLengthPercentageOrAuto; /// Either a computed `` or a `` value. pub type LengthOrNumber = GenericLengthOrNumber; @@ -779,9 +782,6 @@ impl From for Au { } } -/// Either a computed NonNegativeLength or the `auto` keyword. -pub type NonNegativeLengthOrAuto = Either; - /// Either a computed NonNegativeLength or the `normal` keyword. pub type NonNegativeLengthOrNormal = Either; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 70d1e243f02..299b8bc1ead 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -10,7 +10,7 @@ use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth}; use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize}; use super::generics::transform::IsParallelTo; -use super::generics::{GreaterThanOrEqualToOne, NonNegative}; +use super::generics::{self, GreaterThanOrEqualToOne, NonNegative}; use super::specified; use super::{CSSFloat, CSSInteger}; use crate::context::QuirksMode; @@ -27,8 +27,6 @@ use euclid::Size2D; use std::cell::RefCell; use std::cmp; use std::f32; -use std::fmt::{self, Write}; -use style_traits::{CssWriter, ToCss}; #[cfg(feature = "gecko")] pub use self::align::{AlignContent, AlignItems, JustifyContent, JustifyItems, SelfAlignment}; @@ -62,7 +60,7 @@ pub use self::gecko::ScrollSnapPoint; pub use self::image::{Gradient, GradientItem, Image, ImageLayer, LineDirection, MozImageRect}; pub use self::length::{CSSPixelLength, ExtremumLength, NonNegativeLength}; pub use self::length::{Length, LengthOrNumber, LengthPercentage, NonNegativeLengthOrNumber}; -pub use self::length::{LengthPercentageOrAuto, MaxSize, Size}; +pub use self::length::{LengthOrAuto, LengthPercentageOrAuto, MaxSize, Size}; pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageOrAuto}; #[cfg(feature = "gecko")] pub use self::list::ListStyleType; @@ -637,52 +635,8 @@ impl From for PositiveInteger { } } -#[allow(missing_docs)] -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] -#[derive(Clone, ComputeSquaredDistance, Copy, Debug, PartialEq)] -/// A computed cliprect for clip and image-region -pub struct ClipRect { - pub top: Option, - pub right: Option, - pub bottom: Option, - pub left: Option, -} - -impl ToCss for ClipRect { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - dest.write_str("rect(")?; - if let Some(top) = self.top { - top.to_css(dest)?; - dest.write_str(", ")?; - } else { - dest.write_str("auto, ")?; - } - - if let Some(right) = self.right { - right.to_css(dest)?; - dest.write_str(", ")?; - } else { - dest.write_str("auto, ")?; - } - - if let Some(bottom) = self.bottom { - bottom.to_css(dest)?; - dest.write_str(", ")?; - } else { - dest.write_str("auto, ")?; - } - - if let Some(left) = self.left { - left.to_css(dest)?; - } else { - dest.write_str("auto")?; - } - dest.write_str(")") - } -} +/// rect(...) +pub type ClipRect = generics::ClipRect; /// rect(...) | auto pub type ClipRectOrAuto = Either; diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index be1c10a5ee4..425ddf62dcf 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -212,3 +212,14 @@ impl Zero for NonNegative { ToCss, )] pub struct GreaterThanOrEqualToOne(pub T); + +/// A clip rect for clip and image-region +#[allow(missing_docs)] +#[derive(Clone, ComputeSquaredDistance, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToAnimatedValue, ToAnimatedZero, ToComputedValue, ToCss)] +#[css(function = "rect", comma)] +pub struct ClipRect { + pub top: LengthOrAuto, + pub right: LengthOrAuto, + pub bottom: LengthOrAuto, + pub left: LengthOrAuto, +} diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 102bffab658..840991a0dac 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -17,7 +17,7 @@ use crate::values::generics::length::{ use crate::values::generics::NonNegative; use crate::values::specified::calc::CalcNode; use crate::values::specified::NonNegativeNumber; -use crate::values::{Auto, CSSFloat, Either, Normal}; +use crate::values::{CSSFloat, Either, Normal}; use crate::Zero; use app_units::Au; use cssparser::{Parser, Token}; @@ -729,9 +729,6 @@ impl NonNegativeLength { } } -/// Either a NonNegativeLength or the `auto` keyword. -pub type NonNegativeLengthOrAuto = Either; - /// A `` value. This can be either a ``, a /// ``, or a combination of both via `calc()`. /// @@ -997,7 +994,25 @@ impl NonNegativeLengthPercentage { pub type LengthOrNormal = Either; /// Either a `` or the `auto` keyword. -pub type LengthOrAuto = Either; +pub type LengthOrAuto = generics::LengthPercentageOrAuto; + +impl LengthOrAuto { + /// Parses a length, allowing the unitless length quirk. + /// + #[inline] + pub fn parse_quirky<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + allow_quirks: AllowQuirks, + ) -> Result> { + Self::parse_with(context, input, |context, input| { + Length::parse_quirky(context, input, allow_quirks) + }) + } +} + +/// Either a non-negative `` or the `auto` keyword. +pub type NonNegativeLengthOrAuto = generics::LengthPercentageOrAuto; /// Either a `` or a ``. pub type LengthOrNumber = GenericLengthOrNumber; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index d82a3246a13..48bba0a5a77 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -11,7 +11,7 @@ use super::computed::{Context, ToComputedValue}; use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth}; use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize}; use super::generics::transform::IsParallelTo; -use super::generics::{GreaterThanOrEqualToOne, NonNegative}; +use super::generics::{self, GreaterThanOrEqualToOne, NonNegative}; use super::{Auto, CSSFloat, CSSInteger, Either}; use crate::context::QuirksMode; use crate::parser::{Parse, ParserContext}; @@ -60,7 +60,7 @@ pub use self::image::{ColorStop, EndingShape as GradientEndingShape, Gradient}; pub use self::image::{GradientItem, GradientKind, Image, ImageLayer, MozImageRect}; pub use self::length::{AbsoluteLength, CalcLengthPercentage, CharacterWidth}; pub use self::length::{FontRelativeLength, Length, LengthOrNumber, NonNegativeLengthOrNumber}; -pub use self::length::{LengthPercentage, LengthPercentageOrAuto}; +pub use self::length::{LengthOrAuto, LengthPercentage, LengthPercentageOrAuto}; pub use self::length::{MaxSize, Size}; pub use self::length::{NoCalcLength, ViewportPercentageLength}; pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageOrAuto}; @@ -605,99 +605,8 @@ pub type GridLine = GenericGridLine; /// ` | ` pub type GridTemplateComponent = GenericGridTemplateComponent; -#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo)] -/// rect(, , , ) used by clip and image-region -#[css(function = "rect")] -pub struct ClipRect { - /// ( | ) - pub top: Option, - /// ( | ) - pub right: Option, - /// ( | ) - pub bottom: Option, - /// ( | ) - pub left: Option, -} - -impl ToCss for ClipRect { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - dest.write_str("rect(")?; - - if let Some(ref top) = self.top { - top.to_css(dest)?; - dest.write_str(", ")?; - } else { - dest.write_str("auto, ")?; - } - - if let Some(ref right) = self.right { - right.to_css(dest)?; - dest.write_str(", ")?; - } else { - dest.write_str("auto, ")?; - } - - if let Some(ref bottom) = self.bottom { - bottom.to_css(dest)?; - dest.write_str(", ")?; - } else { - dest.write_str("auto, ")?; - } - - if let Some(ref left) = self.left { - left.to_css(dest)?; - } else { - dest.write_str("auto")?; - } - - dest.write_str(")")?; - Ok(()) - } -} - -impl ToComputedValue for ClipRect { - type ComputedValue = super::computed::ClipRect; - - #[inline] - fn to_computed_value(&self, context: &Context) -> super::computed::ClipRect { - super::computed::ClipRect { - top: self.top.as_ref().map(|top| top.to_computed_value(context)), - right: self - .right - .as_ref() - .map(|right| right.to_computed_value(context)), - bottom: self - .bottom - .as_ref() - .map(|bottom| bottom.to_computed_value(context)), - left: self - .left - .as_ref() - .map(|left| left.to_computed_value(context)), - } - } - - #[inline] - fn from_computed_value(computed: &super::computed::ClipRect) -> Self { - ClipRect { - top: computed - .top - .map(|top| ToComputedValue::from_computed_value(&top)), - right: computed - .right - .map(|right| ToComputedValue::from_computed_value(&right)), - bottom: computed - .bottom - .map(|bottom| ToComputedValue::from_computed_value(&bottom)), - left: computed - .left - .map(|left| ToComputedValue::from_computed_value(&left)), - } - } -} +/// rect(...) +pub type ClipRect = generics::ClipRect; impl Parse for ClipRect { fn parse<'i, 't>( @@ -715,25 +624,16 @@ impl ClipRect { input: &mut Parser<'i, 't>, allow_quirks: AllowQuirks, ) -> Result> { - use crate::values::specified::Length; + input.expect_function_matching("rect")?; fn parse_argument<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, allow_quirks: AllowQuirks, - ) -> Result, ParseError<'i>> { - if input - .try(|input| input.expect_ident_matching("auto")) - .is_ok() - { - Ok(None) - } else { - Length::parse_quirky(context, input, allow_quirks).map(Some) - } + ) -> Result> { + LengthOrAuto::parse_quirky(context, input, allow_quirks) } - input.expect_function_matching("rect")?; - input.parse_nested_block(|input| { let top = parse_argument(context, input, allow_quirks)?; let right; @@ -751,12 +651,8 @@ impl ClipRect { bottom = parse_argument(context, input, allow_quirks)?; left = parse_argument(context, input, allow_quirks)?; } - Ok(ClipRect { - top: top, - right: right, - bottom: bottom, - left: left, - }) + + Ok(ClipRect { top, right, bottom, left }) }) } } From 14e6875ef9bcce0c2d93cc5605bba112c9c46edc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 6 Mar 2019 23:01:11 +0000 Subject: [PATCH 20/31] style: Use more cbindgen features. We can get back the fancy flag syntax as soon as we get C++17 inline variables, which I sent an email to dev-platform@ about, with no reply. Differential Revision: https://phabricator.services.mozilla.com/D22382 --- components/style/cbindgen.toml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 9b7159fcba8..5de390cc8a6 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -40,8 +40,13 @@ include = ["cssparser", "style_traits"] derive_eq = true derive_neq = true +[macro_expansion] +bitflags = true + [enum] derive_helper_methods = true +derive_const_casts = true +cast_assert_name = "MOZ_ASSERT" [export] prefix = "Style" @@ -133,7 +138,6 @@ item_types = ["enums", "structs", "typedefs"] """ "GenericLengthPercentageOrAuto" = """ - inline const StyleLengthPercentage& AsLengthPercentage() const; inline bool ConvertsToLength() const; inline nscoord ToLength() const; inline bool ConvertsToPercentage() const; @@ -143,8 +147,6 @@ item_types = ["enums", "structs", "typedefs"] """ "GenericSize" = """ - inline const StyleLengthPercentage& AsLengthPercentage() const; - inline StyleExtremumLength AsExtremumLength() const; inline bool ConvertsToLength() const; inline nscoord ToLength() const; inline bool ConvertsToPercentage() const; @@ -156,12 +158,9 @@ item_types = ["enums", "structs", "typedefs"] "GenericFlexBasis" = """ inline bool IsAuto() const; - inline const StyleSize& AsSize() const; """ "GenericMaxSize" = """ - inline const StyleLengthPercentage& AsLengthPercentage() const; - inline StyleExtremumLength AsExtremumLength() const; inline bool ConvertsToLength() const; inline nscoord ToLength() const; inline bool ConvertsToPercentage() const; From 18a0c9b03335caa974c140f6d246ef411a29e280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 7 Mar 2019 00:14:06 +0100 Subject: [PATCH 21/31] style: followup: Add a comment as requested on Phabricator. --- components/style/values/specified/length.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 840991a0dac..acee43f4b38 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -994,6 +994,10 @@ impl NonNegativeLengthPercentage { pub type LengthOrNormal = Either; /// Either a `` or the `auto` keyword. +/// +/// Note that we use LengthPercentage just for convenience, since it pretty much +/// is everything we care about, but we could just add a similar LengthOrAuto +/// instead if we think getting rid of this weirdness is worth it. pub type LengthOrAuto = generics::LengthPercentageOrAuto; impl LengthOrAuto { From 80ed0706397f0767f7c1c4f419e6061d79437d99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 7 Mar 2019 00:14:43 +0100 Subject: [PATCH 22/31] style: fix a typo. --- components/style/traversal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/traversal.rs b/components/style/traversal.rs index f1821900bb4..d5a9a5c5033 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -686,7 +686,7 @@ where // sibling or cousin. Otherwise, recascading a bunch of identical // elements would unnecessarily flood the cache with identical entries. // - // This is analagous to the obvious "don't insert an element that just + // This is analogous to the obvious "don't insert an element that just // got a hit in the style sharing cache" behavior in the MatchAndCascade // handling above. // From 6fd17ccb35b665f8518d3a11a532e217fdb9f3cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 7 Mar 2019 11:59:36 +0000 Subject: [PATCH 23/31] style: Implement CSS revert keyword. The only fishy bit is the animation stuff. In particular, there are two places where we just mint the revert behavior: * When serializing web-animations keyframes (the custom properties stuff in declaration_block.rs). That codepath is already not sound and I wanted to get rid of it in bug 1501530, but what do I know. * When getting an animation value from a property declaration. At that point we no longer have the CSS rules that apply to the element to compute the right revert value handy. It'd also use the wrong style anyway, I think, given the way StyleBuilder::for_animation works. We _could_ probably get them out of somewhere, but it seems like a whole lot of code reinventing the wheel which is probably not useful, and that Blink and WebKit just cannot implement either since they don't have a rule tree, so it just doesn't seem worth the churn. The custom properties code looks a bit different in order to minimize hash lookups in the common case. FWIW, `revert` for custom properties doesn't seem very useful either, but oh well. Differential Revision: https://phabricator.services.mozilla.com/D21877 --- components/style/custom_properties.rs | 34 +++++++++++++++---- components/style/properties/cascade.rs | 29 +++++++++++----- .../style/properties/declaration_block.rs | 2 +- components/style/properties/helpers.mako.rs | 1 + .../helpers/animated_properties.mako.rs | 9 +++++ .../style/properties/properties.mako.rs | 11 ++++++ components/style/rule_tree/mod.rs | 23 ++++++++++++- components/style/stylesheets/origin.rs | 28 +++++++++++++-- components/style/values/mod.rs | 2 +- 9 files changed, 119 insertions(+), 20 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index f21929c7079..93cd3488f53 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -7,8 +7,9 @@ //! [custom]: https://drafts.csswg.org/css-variables/ use crate::hash::map::Entry; -use crate::properties::{CSSWideKeyword, CustomDeclarationValue}; +use crate::properties::{CSSWideKeyword, CustomDeclaration, CustomDeclarationValue}; use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, PrecomputedHasher}; +use crate::stylesheets::{Origin, PerOrigin}; use crate::Atom; use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType}; use indexmap::IndexMap; @@ -490,6 +491,7 @@ fn parse_env_function<'i, 't>( /// properties. pub struct CustomPropertiesBuilder<'a> { seen: PrecomputedHashSet<&'a Name>, + reverted: PerOrigin>, may_have_cycles: bool, custom_properties: Option, inherited: Option<&'a Arc>, @@ -504,6 +506,7 @@ impl<'a> CustomPropertiesBuilder<'a> { ) -> Self { Self { seen: PrecomputedHashSet::default(), + reverted: Default::default(), may_have_cycles: false, custom_properties: None, inherited, @@ -512,13 +515,26 @@ impl<'a> CustomPropertiesBuilder<'a> { } /// Cascade a given custom property declaration. - pub fn cascade(&mut self, name: &'a Name, specified_value: &CustomDeclarationValue) { + pub fn cascade( + &mut self, + declaration: &'a CustomDeclaration, + origin: Origin, + ) { + let CustomDeclaration { + ref name, + ref value, + } = *declaration; + + if self.reverted.borrow_for_origin(&origin).contains(&name) { + return; + } + let was_already_present = !self.seen.insert(name); if was_already_present { return; } - if !self.value_may_affect_style(name, specified_value) { + if !self.value_may_affect_style(name, value) { return; } @@ -530,7 +546,7 @@ impl<'a> CustomPropertiesBuilder<'a> { } let map = self.custom_properties.as_mut().unwrap(); - match *specified_value { + match *value { CustomDeclarationValue::Value(ref unparsed_value) => { let has_references = !unparsed_value.references.is_empty(); self.may_have_cycles |= has_references; @@ -554,6 +570,12 @@ impl<'a> CustomPropertiesBuilder<'a> { map.insert(name.clone(), value); }, CustomDeclarationValue::CSSWideKeyword(keyword) => match keyword { + CSSWideKeyword::Revert => { + self.seen.remove(name); + for origin in origin.following_including() { + self.reverted.borrow_mut_for_origin(&origin).insert(name); + } + }, CSSWideKeyword::Initial => { map.remove(name); }, @@ -587,10 +609,10 @@ impl<'a> CustomPropertiesBuilder<'a> { // not existing in the map. return false; }, - (Some(existing_value), &CustomDeclarationValue::Value(ref specified_value)) => { + (Some(existing_value), &CustomDeclarationValue::Value(ref value)) => { // Don't bother overwriting an existing inherited value with // the same specified value. - if existing_value == specified_value { + if existing_value == value { return false; } }, diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 4a0e07fdcac..09b055b07cd 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -17,6 +17,7 @@ use crate::properties::CASCADE_PROPERTY; use crate::rule_cache::{RuleCache, RuleCacheConditions}; use crate::rule_tree::{CascadeLevel, StrongRuleNode}; use crate::selector_parser::PseudoElement; +use crate::stylesheets::{Origin, PerOrigin}; use servo_arc::Arc; use crate::shared_lock::StylesheetGuards; use smallbitvec::SmallBitVec; @@ -251,7 +252,7 @@ where for (declaration, cascade_level) in iter_declarations() { declarations.push((declaration, cascade_level)); if let PropertyDeclaration::Custom(ref declaration) = *declaration { - builder.cascade(&declaration.name, &declaration.value); + builder.cascade(declaration, cascade_level.origin()); } } @@ -339,14 +340,8 @@ fn should_ignore_declaration_when_ignoring_document_colors( return false; } - let is_ua_or_user_rule = matches!( - cascade_level, - CascadeLevel::UANormal | - CascadeLevel::UserNormal | - CascadeLevel::UserImportant | - CascadeLevel::UAImportant - ); - + let is_ua_or_user_rule = + matches!(cascade_level.origin(), Origin::User | Origin::UserAgent); if is_ua_or_user_rule { return false; } @@ -388,6 +383,7 @@ struct Cascade<'a, 'b: 'a> { context: &'a mut computed::Context<'b>, cascade_mode: CascadeMode<'a>, seen: LonghandIdSet, + reverted: PerOrigin, saved_font_size: Option, saved_font_family: Option, } @@ -398,6 +394,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { context, cascade_mode, seen: LonghandIdSet::default(), + reverted: Default::default(), saved_font_size: None, saved_font_family: None, } @@ -488,6 +485,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { for (declaration, cascade_level) in declarations { let declaration_id = declaration.id(); + let origin = cascade_level.origin(); let longhand_id = match declaration_id { PropertyDeclarationId::Longhand(id) => id, PropertyDeclarationId::Custom(..) => continue, @@ -513,6 +511,10 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { continue; } + if self.reverted.borrow_for_origin(&origin).contains(physical_longhand_id) { + continue; + } + // Only a few properties are allowed to depend on the visited state // of links. When cascading visited styles, we can save time by // only processing these properties. @@ -540,6 +542,15 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { } } + if declaration.is_revert() { + for origin in origin.following_including() { + self.reverted + .borrow_mut_for_origin(&origin) + .insert(physical_longhand_id); + } + continue; + } + self.seen.insert(physical_longhand_id); // FIXME(emilio): We should avoid generating code for logical diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index d7058d99635..e4396e764a4 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -849,7 +849,7 @@ impl PropertyDeclarationBlock { for declaration in self.normal_declaration_iter() { if let PropertyDeclaration::Custom(ref declaration) = *declaration { - builder.cascade(&declaration.name, &declaration.value); + builder.cascade(declaration, Origin::Author); } } diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 1129a36908d..334ea5de09e 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -349,6 +349,7 @@ context.builder.inherit_${property.ident}(); % endif } + CSSWideKeyword::Revert => unreachable!("Should never get here"), } return; } diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 5669101351d..aa2d0247e82 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -452,14 +452,23 @@ impl AnimationValue { % for prop in data.longhands: % if prop.animatable: LonghandId::${prop.camel_case} => { + // FIXME(emilio, bug 1533327): I think + // CSSWideKeyword::Revert handling is not fine here, but + // what to do instead? + // + // Seems we'd need the computed value as if it was + // revert, somehow. Treating it as `unset` seems fine + // for now... let style_struct = match declaration.keyword { % if not prop.style_struct.inherited: + CSSWideKeyword::Revert | CSSWideKeyword::Unset | % endif CSSWideKeyword::Initial => { initial.get_${prop.style_struct.name_lower}() }, % if prop.style_struct.inherited: + CSSWideKeyword::Revert | CSSWideKeyword::Unset | % endif CSSWideKeyword::Inherit => { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index ec859a42ea7..7bc10b785c6 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -900,6 +900,8 @@ pub enum CSSWideKeyword { Inherit, /// The `unset` keyword. Unset, + /// The `revert` keyword. + Revert, } impl CSSWideKeyword { @@ -908,6 +910,7 @@ impl CSSWideKeyword { CSSWideKeyword::Initial => "initial", CSSWideKeyword::Inherit => "inherit", CSSWideKeyword::Unset => "unset", + CSSWideKeyword::Revert => "revert", } } } @@ -921,6 +924,7 @@ impl CSSWideKeyword { "initial" => CSSWideKeyword::Initial, "inherit" => CSSWideKeyword::Inherit, "unset" => CSSWideKeyword::Unset, + "revert" => CSSWideKeyword::Revert, _ => return Err(()), } }; @@ -2103,6 +2107,7 @@ impl PropertyDeclaration { } /// Returns a CSS-wide keyword if the declaration's value is one. + #[inline] pub fn get_css_wide_keyword(&self) -> Option { match *self { PropertyDeclaration::CSSWideKeyword(ref declaration) => { @@ -2112,6 +2117,12 @@ impl PropertyDeclaration { } } + /// Whether this declaration is the `revert` keyword. + #[inline] + pub fn is_revert(&self) -> bool { + self.get_css_wide_keyword().map_or(false, |kw| kw == CSSWideKeyword::Revert) + } + /// Returns whether or not the property is set by a system font pub fn get_system(&self) -> Option { match *self { diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index a2caba6f9f7..029e3d068f9 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -11,7 +11,7 @@ use crate::applicable_declarations::ApplicableDeclarationList; use crate::gecko::selector_parser::PseudoElement; use crate::properties::{Importance, LonghandIdSet, PropertyDeclarationBlock}; use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; -use crate::stylesheets::StyleRule; +use crate::stylesheets::{StyleRule, Origin}; use crate::thread_state; #[cfg(feature = "gecko")] use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; @@ -659,6 +659,27 @@ impl CascadeLevel { } } + /// Returns the cascade origin of the rule. + #[inline] + pub fn origin(&self) -> Origin { + match *self { + CascadeLevel::UAImportant | + CascadeLevel::UANormal => Origin::UserAgent, + CascadeLevel::UserImportant | + CascadeLevel::UserNormal => Origin::User, + CascadeLevel::PresHints | + CascadeLevel::InnerShadowNormal | + CascadeLevel::SameTreeAuthorNormal | + CascadeLevel::StyleAttributeNormal | + CascadeLevel::SMILOverride | + CascadeLevel::Animations | + CascadeLevel::SameTreeAuthorImportant | + CascadeLevel::StyleAttributeImportant | + CascadeLevel::InnerShadowImportant | + CascadeLevel::Transitions => Origin::Author, + } + } + /// Returns whether this cascade level represents an animation rules. #[inline] pub fn is_animation(&self) -> bool { diff --git a/components/style/stylesheets/origin.rs b/components/style/stylesheets/origin.rs index 69b9b866c86..4e4c304baeb 100644 --- a/components/style/stylesheets/origin.rs +++ b/components/style/stylesheets/origin.rs @@ -36,6 +36,25 @@ impl Origin { _ => return None, }) } + + fn to_index(self) -> i8 { + match self { + Origin::Author => 0, + Origin::User => 1, + Origin::UserAgent => 2, + } + } + + /// Returns an iterator from this origin, towards all the less specific + /// origins. So for `UserAgent`, it'd iterate through all origins. + #[inline] + pub fn following_including(self) -> OriginSetIterator { + OriginSetIterator { + set: OriginSet::ORIGIN_USER | OriginSet::ORIGIN_AUTHOR | OriginSet::ORIGIN_USER_AGENT, + cur: self.to_index(), + rev: true, + } + } } bitflags! { @@ -57,7 +76,7 @@ impl OriginSet { /// See the `OriginSet` documentation for information about the order /// origins are iterated. pub fn iter(&self) -> OriginSetIterator { - OriginSetIterator { set: *self, cur: 0 } + OriginSetIterator { set: *self, cur: 0, rev: false } } } @@ -79,6 +98,7 @@ impl BitOrAssign for OriginSet { pub struct OriginSetIterator { set: OriginSet, cur: i8, + rev: bool, } impl Iterator for OriginSetIterator { @@ -88,7 +108,11 @@ impl Iterator for OriginSetIterator { loop { let origin = Origin::from_index(self.cur)?; - self.cur += 1; + if self.rev { + self.cur -= 1; + } else { + self.cur += 1; + } if self.set.contains(origin.into()) { return Some(origin); diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index ccda0f290c1..e29b8923af7 100644 --- a/components/style/values/mod.rs +++ b/components/style/values/mod.rs @@ -160,7 +160,7 @@ impl CustomIdent { excluding: &[&str], ) -> Result> { let valid = match_ignore_ascii_case! { ident, - "initial" | "inherit" | "unset" | "default" => false, + "initial" | "inherit" | "unset" | "default" | "revert" => false, _ => true }; if !valid { From 4a1f390788f647c4a96371a8a6608a677b600552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 7 Mar 2019 12:48:07 +0000 Subject: [PATCH 24/31] style: Optimize cascading of other wide keywords if possible. The way the copy-on-write stuff works, and the way that we have to apply properties from most specific to less specific guarantees that always that we're going to inherit an inherited property, or reset a reset property, we have already the right value on the style. Revert relies on that, so there doesn't seem to be a reason to not use that fact more often and skip useless work earlier. Font-size is still special of course... I think I have a way to move the specialness outside of the style, but piece by piece. Differential Revision: https://phabricator.services.mozilla.com/D21882 --- components/style/properties/cascade.rs | 31 +++++++++++++------ components/style/properties/helpers.mako.rs | 20 ++++++------ .../style/properties/properties.mako.rs | 24 +++----------- 3 files changed, 37 insertions(+), 38 deletions(-) diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 09b055b07cd..23970d64888 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -11,7 +11,7 @@ use crate::font_metrics::FontMetricsProvider; use crate::logical_geometry::WritingMode; use crate::media_queries::Device; use crate::properties::{ComputedValues, StyleBuilder}; -use crate::properties::{LonghandId, LonghandIdSet}; +use crate::properties::{LonghandId, LonghandIdSet, CSSWideKeyword}; use crate::properties::{PropertyDeclaration, PropertyDeclarationId, DeclarationImportanceIterator}; use crate::properties::CASCADE_PROPERTY; use crate::rule_cache::{RuleCache, RuleCacheConditions}; @@ -542,7 +542,11 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { } } - if declaration.is_revert() { + let css_wide_keyword = declaration.get_css_wide_keyword(); + if let Some(CSSWideKeyword::Revert) = css_wide_keyword { + // We intentionally don't want to insert it into `self.seen`, + // `reverted` takes care of rejecting other declarations as + // needed. for origin in origin.following_including() { self.reverted .borrow_mut_for_origin(&origin) @@ -553,6 +557,19 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { self.seen.insert(physical_longhand_id); + let unset = css_wide_keyword.map_or(false, |css_wide_keyword| { + match css_wide_keyword { + CSSWideKeyword::Unset => true, + CSSWideKeyword::Inherit => inherited, + CSSWideKeyword::Initial => !inherited, + CSSWideKeyword::Revert => unreachable!(), + } + }); + + if unset { + continue; + } + // FIXME(emilio): We should avoid generating code for logical // longhands and just use the physical ones, then rename // physical_longhand_id to just longhand_id. @@ -811,18 +828,14 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { self.seen.contains(LonghandId::MozMinFontSizeRatio) || self.seen.contains(LonghandId::FontFamily) { - use crate::properties::{CSSWideKeyword, WideKeywordDeclaration}; + use crate::values::computed::FontSize; // font-size must be explicitly inherited to handle lang // changes and scriptlevel changes. // // FIXME(emilio): That looks a bit bogus... - let inherit = PropertyDeclaration::CSSWideKeyword(WideKeywordDeclaration { - id: LonghandId::FontSize, - keyword: CSSWideKeyword::Inherit, - }); - - self.apply_declaration_ignoring_phase(LonghandId::FontSize, &inherit); + self.context.for_non_inherited_property = None; + FontSize::cascade_inherit_font_size(&mut self.context); } } } diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 334ea5de09e..81931bc713b 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -326,26 +326,28 @@ PropertyDeclaration::CSSWideKeyword(ref declaration) => { debug_assert_eq!(declaration.id, LonghandId::${property.camel_case}); match declaration.keyword { - % if not data.current_style_struct.inherited: + % if not property.style_struct.inherited: CSSWideKeyword::Unset | % endif CSSWideKeyword::Initial => { - % if property.ident == "font_size": - computed::FontSize::cascade_initial_font_size(context); + % if not property.style_struct.inherited: + debug_assert!(false, "Should be handled in apply_properties"); % else: + % if property.name == "font-size": + computed::FontSize::cascade_initial_font_size(context); + % else: context.builder.reset_${property.ident}(); + % endif % endif }, - % if data.current_style_struct.inherited: + % if property.style_struct.inherited: CSSWideKeyword::Unset | % endif CSSWideKeyword::Inherit => { - % if not property.style_struct.inherited: - context.rule_cache_conditions.borrow_mut().set_uncacheable(); - % endif - % if property.ident == "font_size": - computed::FontSize::cascade_inherit_font_size(context); + % if property.style_struct.inherited: + debug_assert!(false, "Should be handled in apply_properties"); % else: + context.rule_cache_conditions.borrow_mut().set_uncacheable(); context.builder.inherit_${property.ident}(); % endif } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 7bc10b785c6..73965f90a3c 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2117,12 +2117,6 @@ impl PropertyDeclaration { } } - /// Whether this declaration is the `revert` keyword. - #[inline] - pub fn is_revert(&self) -> bool { - self.get_css_wide_keyword().map_or(false, |kw| kw == CSSWideKeyword::Revert) - } - /// Returns whether or not the property is set by a system font pub fn get_system(&self) -> Option { match *self { @@ -3447,22 +3441,16 @@ impl<'a> StyleBuilder<'a> { } % for property in data.longhands: - % if property.ident != "font_size": + % if not property.style_struct.inherited: /// Inherit `${property.ident}` from our parent style. #[allow(non_snake_case)] pub fn inherit_${property.ident}(&mut self) { let inherited_struct = - % if property.style_struct.inherited: - self.inherited_style.get_${property.style_struct.name_lower}(); - % else: self.inherited_style_ignoring_first_line .get_${property.style_struct.name_lower}(); - % endif - % if not property.style_struct.inherited: - self.flags.insert(ComputedValueFlags::INHERITS_RESET_STYLE); self.modified_reset = true; - % endif + self.flags.insert(ComputedValueFlags::INHERITS_RESET_STYLE); % if property.ident == "content": self.flags.insert(ComputedValueFlags::INHERITS_CONTENT); @@ -3484,17 +3472,13 @@ impl<'a> StyleBuilder<'a> { % endif ); } - + % elif property.name != "font-size": /// Reset `${property.ident}` to the initial value. #[allow(non_snake_case)] pub fn reset_${property.ident}(&mut self) { let reset_struct = self.reset_style.get_${property.style_struct.name_lower}(); - % if not property.style_struct.inherited: - self.modified_reset = true; - % endif - if self.${property.style_struct.ident}.ptr_eq(reset_struct) { return; } @@ -3507,6 +3491,7 @@ impl<'a> StyleBuilder<'a> { % endif ); } + % endif % if not property.is_vector: /// Set the `${property.ident}` to the computed value `value`. @@ -3528,7 +3513,6 @@ impl<'a> StyleBuilder<'a> { ); } % endif - % endif % endfor <% del property %> From 9a8cee28005efef9ff327c7985828a3d783dfc44 Mon Sep 17 00:00:00 2001 From: violet <> Date: Sat, 9 Mar 2019 13:05:56 +0000 Subject: [PATCH 25/31] style: Change stroke-miterlimit restriction from GreaterThanOrEqualToOneNumber to NonNegativeNumber. --- components/style/properties/longhands/inherited_svg.mako.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/style/properties/longhands/inherited_svg.mako.rs b/components/style/properties/longhands/inherited_svg.mako.rs index 35d0a02aab2..2bbdc2794c2 100644 --- a/components/style/properties/longhands/inherited_svg.mako.rs +++ b/components/style/properties/longhands/inherited_svg.mako.rs @@ -110,11 +110,11 @@ ${helpers.single_keyword( ${helpers.predefined_type( "stroke-miterlimit", - "GreaterThanOrEqualToOneNumber", + "NonNegativeNumber", "From::from(4.0)", products="gecko", - animation_value_type="crate::values::computed::GreaterThanOrEqualToOneNumber", - spec="https://www.w3.org/TR/SVG11/painting.html#StrokeMiterlimitProperty", + animation_value_type="crate::values::computed::NonNegativeNumber", + spec="https://www.w3.org/TR/SVG2/painting.html#StrokeMiterlimitProperty", )} ${helpers.predefined_type( From 5aeab7adb1bbad63e926e6736df78121aaf91f23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 12 Mar 2019 17:02:19 +0100 Subject: [PATCH 26/31] style: Reformat recent changes. --- components/style/custom_properties.rs | 6 +----- components/style/gecko/media_queries.rs | 17 +++++++++-------- components/style/gecko/wrapper.rs | 3 +-- .../gecko_bindings/sugar/ns_style_coord.rs | 2 +- components/style/rule_tree/mod.rs | 8 +++----- components/style/stylesheets/origin.rs | 6 +++++- components/style/values/animated/svg.rs | 2 +- components/style/values/computed/mod.rs | 2 +- components/style/values/computed/svg.rs | 4 +--- components/style/values/computed/text.rs | 2 +- components/style/values/computed/transform.rs | 4 +--- components/style/values/generics/mod.rs | 14 +++++++++++++- components/style/values/generics/text.rs | 4 +--- components/style/values/generics/transform.rs | 19 ++++++++++++++++--- components/style/values/specified/mod.rs | 7 ++++++- components/style/values/specified/svg.rs | 17 ++++++----------- components/style_derive/cg.rs | 6 +++--- 17 files changed, 70 insertions(+), 53 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 93cd3488f53..0dbd611820d 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -515,11 +515,7 @@ impl<'a> CustomPropertiesBuilder<'a> { } /// Cascade a given custom property declaration. - pub fn cascade( - &mut self, - declaration: &'a CustomDeclaration, - origin: Origin, - ) { + pub fn cascade(&mut self, declaration: &'a CustomDeclaration, origin: Origin) { let CustomDeclaration { ref name, ref value, diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 3fef4974c33..4c04d9573e5 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -116,10 +116,7 @@ impl Device { }; unsafe { - bindings::Gecko_AnimationNameMayBeReferencedFromStyle( - pc, - name.as_atom().as_ptr(), - ) + bindings::Gecko_AnimationNameMayBeReferencedFromStyle(pc, name.as_atom().as_ptr()) } } @@ -169,7 +166,12 @@ impl Device { #[inline] pub fn pres_context(&self) -> Option<&structs::nsPresContext> { unsafe { - self.document().mPresShell.as_ref()?.mPresContext.mRawPtr.as_ref() + self.document() + .mPresShell + .as_ref()? + .mPresContext + .mRawPtr + .as_ref() } } @@ -268,9 +270,8 @@ impl Device { if doc.mIsBeingUsedAsImage() { return true; } - let document_color_use = unsafe { - structs::StaticPrefs_sVarCache_browser_display_document_color_use - }; + let document_color_use = + unsafe { structs::StaticPrefs_sVarCache_browser_display_document_color_use }; let prefs = self.pref_sheet_prefs(); match document_color_use { 1 => true, diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index f5bb7d8a94c..f5058fc63c4 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1246,8 +1246,7 @@ impl<'le> TElement for GeckoElement<'le> { } fn owner_doc_matches_for_testing(&self, device: &Device) -> bool { - self.as_node().owner_doc().0 as *const structs::Document == - device.document() as *const _ + self.as_node().owner_doc().0 as *const structs::Document == device.document() as *const _ } fn style_attribute(&self) -> Option>> { diff --git a/components/style/gecko_bindings/sugar/ns_style_coord.rs b/components/style/gecko_bindings/sugar/ns_style_coord.rs index 133ce4d9b7b..7c6612f35ef 100644 --- a/components/style/gecko_bindings/sugar/ns_style_coord.rs +++ b/components/style/gecko_bindings/sugar/ns_style_coord.rs @@ -5,8 +5,8 @@ //! Rust helpers for Gecko's `nsStyleCoord`. use crate::gecko_bindings::bindings; -use crate::gecko_bindings::structs::{nsStyleCoord, nsStyleCoord_Calc, nsStyleCoord_CalcValue}; use crate::gecko_bindings::structs::nsStyleSides; +use crate::gecko_bindings::structs::{nsStyleCoord, nsStyleCoord_Calc, nsStyleCoord_CalcValue}; use crate::gecko_bindings::structs::{nsStyleUnion, nsStyleUnit, nscoord}; use std::mem; diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 029e3d068f9..f165e1f455f 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -11,7 +11,7 @@ use crate::applicable_declarations::ApplicableDeclarationList; use crate::gecko::selector_parser::PseudoElement; use crate::properties::{Importance, LonghandIdSet, PropertyDeclarationBlock}; use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; -use crate::stylesheets::{StyleRule, Origin}; +use crate::stylesheets::{Origin, StyleRule}; use crate::thread_state; #[cfg(feature = "gecko")] use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; @@ -663,10 +663,8 @@ impl CascadeLevel { #[inline] pub fn origin(&self) -> Origin { match *self { - CascadeLevel::UAImportant | - CascadeLevel::UANormal => Origin::UserAgent, - CascadeLevel::UserImportant | - CascadeLevel::UserNormal => Origin::User, + CascadeLevel::UAImportant | CascadeLevel::UANormal => Origin::UserAgent, + CascadeLevel::UserImportant | CascadeLevel::UserNormal => Origin::User, CascadeLevel::PresHints | CascadeLevel::InnerShadowNormal | CascadeLevel::SameTreeAuthorNormal | diff --git a/components/style/stylesheets/origin.rs b/components/style/stylesheets/origin.rs index 4e4c304baeb..fc89260abd2 100644 --- a/components/style/stylesheets/origin.rs +++ b/components/style/stylesheets/origin.rs @@ -76,7 +76,11 @@ impl OriginSet { /// See the `OriginSet` documentation for information about the order /// origins are iterated. pub fn iter(&self) -> OriginSetIterator { - OriginSetIterator { set: *self, cur: 0, rev: false } + OriginSetIterator { + set: *self, + cur: 0, + rev: false, + } } } diff --git a/components/style/values/animated/svg.rs b/components/style/values/animated/svg.rs index 15c6580cd74..436bb73d658 100644 --- a/components/style/values/animated/svg.rs +++ b/components/style/values/animated/svg.rs @@ -8,7 +8,7 @@ use super::{Animate, Procedure, ToAnimatedZero}; use crate::properties::animated_properties::ListAnimation; use crate::values::animated::color::Color as AnimatedColor; use crate::values::computed::url::ComputedUrl; -use crate::values::distance::{SquaredDistance, ComputeSquaredDistance}; +use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; use crate::values::generics::svg::{SVGPaint, SVGStrokeDashArray}; /// Animated SVGPaint. diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 299b8bc1ead..1f7d1e3eddc 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -76,7 +76,7 @@ pub use self::svg::{SVGLength, SVGOpacity, SVGPaint, SVGPaintKind}; pub use self::svg::{SVGPaintOrder, SVGStrokeDashArray, SVGWidth}; pub use self::table::XSpan; pub use self::text::{InitialLetter, LetterSpacing, LineHeight}; -pub use self::text::{OverflowWrap, TextOverflow, WordSpacing, WordBreak}; +pub use self::text::{OverflowWrap, TextOverflow, WordBreak, WordSpacing}; pub use self::text::{TextAlign, TextEmphasisPosition, TextEmphasisStyle}; pub use self::time::Time; pub use self::transform::{Rotate, Scale, Transform, TransformOperation}; diff --git a/components/style/values/computed/svg.rs b/components/style/values/computed/svg.rs index 27d4dd6adc3..f05e9f836da 100644 --- a/components/style/values/computed/svg.rs +++ b/components/style/values/computed/svg.rs @@ -57,9 +57,7 @@ impl SVGWidth { /// `1px`. pub fn one() -> Self { use crate::values::generics::NonNegative; - generic::SVGLength::LengthPercentage( - NonNegative(LengthPercentage::one()), - ) + generic::SVGLength::LengthPercentage(NonNegative(LengthPercentage::one())) } } diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index 783e553d1ce..65b694c5057 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -18,9 +18,9 @@ use crate::Zero; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; -pub use crate::values::specified::{OverflowWrap, WordBreak}; pub use crate::values::specified::TextAlignKeyword as TextAlign; pub use crate::values::specified::TextEmphasisPosition; +pub use crate::values::specified::{OverflowWrap, WordBreak}; /// A computed value for the `initial-letter` property. pub type InitialLetter = GenericInitialLetter; diff --git a/components/style/values/computed/transform.rs b/components/style/values/computed/transform.rs index cdb7fb39b47..e23ec1524ff 100644 --- a/components/style/values/computed/transform.rs +++ b/components/style/values/computed/transform.rs @@ -592,9 +592,7 @@ impl Translate { /// Convert Translate to TransformOperation. pub fn from_transform_operation(operation: &TransformOperation) -> Translate { match *operation { - generic::TransformOperation::Translate(tx, ty) => { - generic::Translate::Translate(tx, ty) - }, + generic::TransformOperation::Translate(tx, ty) => generic::Translate::Translate(tx, ty), generic::TransformOperation::Translate3D(tx, ty, tz) => { generic::Translate::Translate3D(tx, ty, tz) }, diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index 425ddf62dcf..e054db7281f 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -215,7 +215,19 @@ pub struct GreaterThanOrEqualToOne(pub T); /// A clip rect for clip and image-region #[allow(missing_docs)] -#[derive(Clone, ComputeSquaredDistance, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToAnimatedValue, ToAnimatedZero, ToComputedValue, ToCss)] +#[derive( + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToAnimatedZero, + ToComputedValue, + ToCss, +)] #[css(function = "rect", comma)] pub struct ClipRect { pub top: LengthOrAuto, diff --git a/components/style/values/generics/text.rs b/components/style/values/generics/text.rs index f339bf83432..7dae932faac 100644 --- a/components/style/values/generics/text.rs +++ b/components/style/values/generics/text.rs @@ -29,9 +29,7 @@ impl InitialLetter { } /// A generic spacing value for the `letter-spacing` and `word-spacing` properties. -#[derive( - Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, -)] +#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)] pub enum Spacing { /// `normal` Normal, diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index 15e0ea2d274..3e091162b2e 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -137,7 +137,10 @@ where SkewY(Angle), /// translate(x, y) or translate(x) #[css(comma, function)] - Translate(LengthPercentage, #[css(skip_if = "Zero::is_zero")] LengthPercentage), + Translate( + LengthPercentage, + #[css(skip_if = "Zero::is_zero")] LengthPercentage, + ), /// translateX(x) #[css(function = "translateX")] TranslateX(LengthPercentage), @@ -637,7 +640,14 @@ impl ToCss for Scale { } #[derive( - Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToAnimatedZero, ToComputedValue, ToCss + Clone, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedZero, + ToComputedValue, + ToCss, )] /// A value of the `translate` property /// @@ -660,7 +670,10 @@ where /// 'none' None, /// '' or ' ' - Translate(LengthPercentage, #[css(skip_if = "Zero::is_zero")] LengthPercentage), + Translate( + LengthPercentage, + #[css(skip_if = "Zero::is_zero")] LengthPercentage, + ), /// ' ' Translate3D(LengthPercentage, LengthPercentage, Length), } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 48bba0a5a77..7295f060697 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -652,7 +652,12 @@ impl ClipRect { left = parse_argument(context, input, allow_quirks)?; } - Ok(ClipRect { top, right, bottom, left }) + Ok(ClipRect { + top, + right, + bottom, + left, + }) }) } } diff --git a/components/style/values/specified/svg.rs b/components/style/values/specified/svg.rs index fa54b5338b4..556e995c02f 100644 --- a/components/style/values/specified/svg.rs +++ b/components/style/values/specified/svg.rs @@ -6,9 +6,9 @@ use crate::parser::{Parse, ParserContext}; use crate::values::generics::svg as generic; -use crate::values::specified::AllowQuirks; use crate::values::specified::color::Color; use crate::values::specified::url::SpecifiedUrl; +use crate::values::specified::AllowQuirks; use crate::values::specified::LengthPercentage; use crate::values::specified::{NonNegativeLengthPercentage, Opacity}; use crate::values::CustomIdent; @@ -52,10 +52,9 @@ macro_rules! parse_svg_length { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - if let Ok(lp) = input.try(|i| { - <$lp>::parse_quirky(context, i, AllowQuirks::Always) - }) { - return Ok(generic::SVGLength::LengthPercentage(lp)) + if let Ok(lp) = input.try(|i| <$lp>::parse_quirky(context, i, AllowQuirks::Always)) + { + return Ok(generic::SVGLength::LengthPercentage(lp)); } try_match_ident_ignore_ascii_case! { input, @@ -65,7 +64,7 @@ macro_rules! parse_svg_length { } } } - } + }; } parse_svg_length!(SVGLength, LengthPercentage); @@ -78,11 +77,7 @@ impl Parse for SVGStrokeDashArray { ) -> Result> { if let Ok(values) = input.try(|i| { CommaWithSpace::parse(i, |i| { - NonNegativeLengthPercentage::parse_quirky( - context, - i, - AllowQuirks::Always, - ) + NonNegativeLengthPercentage::parse_quirky(context, i, AllowQuirks::Always) }) }) { return Ok(generic::SVGStrokeDashArray::Values(values)); diff --git a/components/style_derive/cg.rs b/components/style_derive/cg.rs index 880cb07378b..a5f15a0358e 100644 --- a/components/style_derive/cg.rs +++ b/components/style_derive/cg.rs @@ -70,9 +70,9 @@ pub fn propagate_clauses_to_output_type( for bound in extra_bounds { let ty = bound.bounded_ty; let bounds = bound.bounds; - where_clause.predicates.push( - parse_quote!(<#ty as #trait_path>::#trait_output: #bounds), - ) + where_clause + .predicates + .push(parse_quote!(<#ty as #trait_path>::#trait_output: #bounds)) } } From a65925cb855d8e8bd82c3b7dc4b8158c9a1a673d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 12 Mar 2019 17:50:55 +0100 Subject: [PATCH 27/31] style: Fix servo build. --- components/style/properties/properties.mako.rs | 8 +++----- components/style/values/computed/text.rs | 2 +- components/style/values/specified/text.rs | 1 + 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 73965f90a3c..0153d6fe336 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2591,6 +2591,7 @@ pub mod style_structs { /// Whether the border-${side} property has nonzero width. #[allow(non_snake_case)] pub fn border_${side}_has_nonzero_width(&self) -> bool { + use crate::Zero; !self.border_${side}_width.is_zero() } % endfor @@ -2630,6 +2631,7 @@ pub mod style_structs { /// Whether the outline-width property is non-zero. #[inline] pub fn outline_has_nonzero_width(&self) -> bool { + use crate::Zero; !self.outline_width.is_zero() } % elif style_struct.name == "Text": @@ -2724,11 +2726,7 @@ pub mod style_structs { /// Whether this is a multicol style. #[cfg(feature = "servo")] pub fn is_multicol(&self) -> bool { - use crate::values::Either; - match self.column_width { - Either::First(_width) => true, - Either::Second(_auto) => !self.column_count.is_auto(), - } + !self.column_width.is_auto() || !self.column_count.is_auto() } % endif } diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index 65b694c5057..911891bf33b 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -38,7 +38,7 @@ pub type InitialLetter = GenericInitialLetter; ToAnimatedValue, ToAnimatedZero, )] -pub struct LetterSpacing(Length); +pub struct LetterSpacing(pub Length); impl LetterSpacing { /// Return the `normal` computed value, which is just zero. diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 33394857f68..883084e649a 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -836,6 +836,7 @@ pub enum WordBreak { /// /// Specifying `word-break: break-word` makes `overflow-wrap` behave as /// `anywhere`, and `word-break` behave like `normal`. + #[cfg(feature = "gecko")] BreakWord, } From 0c01325c0523d3b04a0512bdb2ebb10328c11a32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 12 Mar 2019 18:33:00 +0100 Subject: [PATCH 28/31] layout: Fix servo build. --- components/layout/display_list/builder.rs | 21 ++++++++++++--------- components/layout/fragment.rs | 1 + components/layout/multicol.rs | 6 ++++-- components/layout/text.rs | 21 +++++++++------------ 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index d26cd6b9c32..73343c3a4f2 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -58,7 +58,7 @@ use style::properties::{style_structs, ComputedValues}; use style::servo::restyle_damage::ServoRestyleDamage; use style::values::computed::effects::SimpleShadow; use style::values::computed::image::Image as ComputedImage; -use style::values::computed::Gradient; +use style::values::computed::{Gradient, LengthOrAuto}; use style::values::generics::background::BackgroundSize; use style::values::generics::image::{GradientKind, Image, PaintWorklet}; use style::values::specified::ui::CursorKind; @@ -2627,19 +2627,22 @@ impl BlockFlow { _ => return, } + fn extract_clip_component(p: &LengthOrAuto) -> Option { + match *p { + LengthOrAuto::Auto => None, + LengthOrAuto::LengthPercentage(ref length) => Some(Au::from(*length)), + } + } + let clip_origin = Point2D::new( stacking_relative_border_box.origin.x + - style_clip_rect.left.map(Au::from).unwrap_or(Au(0)), + extract_clip_component(&style_clip_rect.left).unwrap_or_default(), stacking_relative_border_box.origin.y + - style_clip_rect.top.map(Au::from).unwrap_or(Au(0)), + extract_clip_component(&style_clip_rect.top).unwrap_or_default(), ); - let right = style_clip_rect - .right - .map(Au::from) + let right = extract_clip_component(&style_clip_rect.right) .unwrap_or(stacking_relative_border_box.size.width); - let bottom = style_clip_rect - .bottom - .map(Au::from) + let bottom = extract_clip_component(&style_clip_rect.bottom) .unwrap_or(stacking_relative_border_box.size.height); let clip_size = Size2D::new(right - clip_origin.x, bottom - clip_origin.y); diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index d8183eae1c9..ff62cc79638 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -64,6 +64,7 @@ use style::values::computed::counters::ContentItem; use style::values::computed::{LengthPercentage, LengthPercentageOrAuto, Size}; use style::values::generics::box_::{Perspective, VerticalAlign}; use style::values::generics::transform; +use style::Zero; use webrender_api::{self, LayoutTransform}; // From gfxFontConstants.h in Firefox. diff --git a/components/layout/multicol.rs b/components/layout/multicol.rs index ca92c200d40..f8ddfa8200c 100644 --- a/components/layout/multicol.rs +++ b/components/layout/multicol.rs @@ -19,7 +19,7 @@ use std::fmt; use std::sync::Arc; use style::logical_geometry::LogicalSize; use style::properties::ComputedValues; -use style::values::computed::{MaxSize, Size}; +use style::values::computed::length::{MaxSize, NonNegativeLengthOrAuto, Size}; use style::values::generics::column::ColumnCount; use style::values::Either; @@ -114,7 +114,9 @@ impl Flow for MulticolFlow { let column_style = style.get_column(); let mut column_count; - if let Either::First(column_width) = column_style.column_width { + if let NonNegativeLengthOrAuto::LengthPercentage(column_width) = + column_style.column_width + { let column_width = Au::from(column_width); column_count = max( 1, diff --git a/components/layout/text.rs b/components/layout/text.rs index aba5703a1c6..b26e531e670 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -14,7 +14,6 @@ use gfx::font::{FontMetrics, FontRef, RunMetrics, ShapingFlags, ShapingOptions}; use gfx::text::glyph::ByteIndex; use gfx::text::text_run::TextRun; use gfx::text::util::{self, CompressionMode}; -use ordered_float::NotNan; use range::Range; use servo_atoms::Atom; use std::borrow::ToOwned; @@ -196,11 +195,7 @@ impl TextRunScanner { }; text_transform = inherited_text_style.text_transform; letter_spacing = inherited_text_style.letter_spacing; - word_spacing = inherited_text_style - .word_spacing - .value() - .map(|lop| lop.to_hash_key()) - .unwrap_or((Au(0), NotNan::new(0.0).unwrap())); + word_spacing = inherited_text_style.word_spacing.to_hash_key(); text_rendering = inherited_text_style.text_rendering; word_break = inherited_text_style.word_break; } @@ -321,10 +316,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(); - if let Some(v) = letter_spacing.value() { - if v.px() != 0. { - flags.insert(ShapingFlags::IGNORE_LIGATURES_SHAPING_FLAG); - } + if letter_spacing.0.px() != 0. { + flags.insert(ShapingFlags::IGNORE_LIGATURES_SHAPING_FLAG); } if text_rendering == TextRendering::Optimizespeed { flags.insert(ShapingFlags::IGNORE_LIGATURES_SHAPING_FLAG); @@ -334,8 +327,12 @@ impl TextRunScanner { flags.insert(ShapingFlags::KEEP_ALL_FLAG); } let options = ShapingOptions { - letter_spacing: letter_spacing.value().cloned().map(Au::from), - word_spacing: word_spacing, + letter_spacing: if letter_spacing.0.px() == 0. { + None + } else { + Some(Au::from(letter_spacing.0)) + }, + word_spacing, script: Script::Common, flags: flags, }; From 8020faed9b5e5ce4714c0066c9bb7287c8d64682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 12 Mar 2019 18:40:56 +0100 Subject: [PATCH 29/31] style: Fix unit tests. --- tests/unit/style/animated_properties.rs | 50 ------------------------- tests/unit/style/custom_properties.rs | 16 ++++---- 2 files changed, 8 insertions(+), 58 deletions(-) diff --git a/tests/unit/style/animated_properties.rs b/tests/unit/style/animated_properties.rs index 8fa45a6305f..dd233f01b89 100644 --- a/tests/unit/style/animated_properties.rs +++ b/tests/unit/style/animated_properties.rs @@ -92,53 +92,3 @@ fn test_transform_interpolation_on_scale() { Transform(vec![TransformOperation::Scale3D(1.5, 3.0, 1.5)]) ); } - -#[test] -fn test_transform_interpolation_on_rotate() { - use style::values::computed::Angle; - - let from = Transform(vec![TransformOperation::Rotate3D( - 0.0, - 0.0, - 1.0, - Angle::from_radians(0.0), - )]); - let to = Transform(vec![TransformOperation::Rotate3D( - 0.0, - 0.0, - 1.0, - Angle::from_radians(100.0), - )]); - assert_eq!( - from.animate(&to, Procedure::Interpolate { progress: 0.5 }) - .unwrap(), - Transform(vec![TransformOperation::Rotate3D( - 0.0, - 0.0, - 1.0, - Angle::from_radians(50.0) - )]) - ); -} - -#[test] -fn test_transform_interpolation_on_skew() { - use style::values::computed::Angle; - - let from = Transform(vec![TransformOperation::Skew( - Angle::from_radians(0.0), - Some(Angle::from_radians(100.0)), - )]); - let to = Transform(vec![TransformOperation::Skew( - Angle::from_radians(100.0), - Some(Angle::from_radians(0.0)), - )]); - assert_eq!( - from.animate(&to, Procedure::Interpolate { progress: 0.5 }) - .unwrap(), - Transform(vec![TransformOperation::Skew( - Angle::from_radians(50.0), - Some(Angle::from_radians(50.0)), - )]) - ); -} diff --git a/tests/unit/style/custom_properties.rs b/tests/unit/style/custom_properties.rs index b24ca3b132a..5b5741d2690 100644 --- a/tests/unit/style/custom_properties.rs +++ b/tests/unit/style/custom_properties.rs @@ -7,30 +7,30 @@ use servo_arc::Arc; use style::custom_properties::{ CssEnvironment, CustomPropertiesBuilder, CustomPropertiesMap, Name, SpecifiedValue, }; -use style::properties::CustomDeclarationValue; +use style::properties::{CustomDeclaration, CustomDeclarationValue}; +use style::stylesheets::Origin; use test::{self, Bencher}; fn cascade( name_and_value: &[(&str, &str)], inherited: Option<&Arc>, ) -> Option> { - let values = name_and_value + let declarations = name_and_value .iter() .map(|&(name, value)| { let mut input = ParserInput::new(value); let mut parser = Parser::new(&mut input); - ( - Name::from(name), - SpecifiedValue::parse(&mut parser).unwrap(), - ) + let name = Name::from(name); + let value = CustomDeclarationValue::Value(SpecifiedValue::parse(&mut parser).unwrap()); + CustomDeclaration { name, value } }) .collect::>(); let env = CssEnvironment; let mut builder = CustomPropertiesBuilder::new(inherited, &env); - for &(ref name, ref val) in &values { - builder.cascade(name, &CustomDeclarationValue::Value(val.clone())); + for declaration in &declarations { + builder.cascade(declaration, Origin::Author); } builder.build() From c3f46d78a096e69f57a65ba3dd3596c160488733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 13 Mar 2019 12:14:16 +0100 Subject: [PATCH 30/31] style: Add two commas to appease tidy. --- components/style_derive/to_css.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/style_derive/to_css.rs b/components/style_derive/to_css.rs index 3a1bc9f4c6c..249ff2969cd 100644 --- a/components/style_derive/to_css.rs +++ b/components/style_derive/to_css.rs @@ -151,7 +151,7 @@ fn derive_variant_fields_expr( if let Some(condition) = attrs.contextual_skip_if { expr = quote! { - if !#condition(#(#bindings),*) { + if !#condition(#(#bindings), *) { #expr } } @@ -227,7 +227,7 @@ fn derive_single_field_expr( if let Some(condition) = attrs.contextual_skip_if { expr = quote! { - if !#condition(#(#bindings),*) { + if !#condition(#(#bindings), *) { #expr } } From b42342a27381d644f5817eb4530252c4c142d4e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 13 Mar 2019 12:14:54 +0100 Subject: [PATCH 31/31] Update test expectations. --- tests/wpt/metadata/css/css-text/inheritance.html.ini | 3 --- .../css-variables/variable-definition-keywords.html.ini | 7 ------- .../css-variables/variable-presentation-attribute.html.ini | 3 --- 3 files changed, 13 deletions(-) delete mode 100644 tests/wpt/metadata/css/css-variables/variable-definition-keywords.html.ini diff --git a/tests/wpt/metadata/css/css-text/inheritance.html.ini b/tests/wpt/metadata/css/css-text/inheritance.html.ini index ea3be987a40..a2715f4fe76 100644 --- a/tests/wpt/metadata/css/css-text/inheritance.html.ini +++ b/tests/wpt/metadata/css/css-text/inheritance.html.ini @@ -2,6 +2,3 @@ [Property text-justify inherits] expected: FAIL - [Property word-spacing has initial value 0px] - expected: FAIL - diff --git a/tests/wpt/metadata/css/css-variables/variable-definition-keywords.html.ini b/tests/wpt/metadata/css/css-variables/variable-definition-keywords.html.ini deleted file mode 100644 index 1759114e0b3..00000000000 --- a/tests/wpt/metadata/css/css-variables/variable-definition-keywords.html.ini +++ /dev/null @@ -1,7 +0,0 @@ -[variable-definition-keywords.html] - [specified style revert] - expected: FAIL - - [computed style revert] - expected: FAIL - diff --git a/tests/wpt/metadata/css/css-variables/variable-presentation-attribute.html.ini b/tests/wpt/metadata/css/css-variables/variable-presentation-attribute.html.ini index bbb64382d67..4e4418a7cce 100644 --- a/tests/wpt/metadata/css/css-variables/variable-presentation-attribute.html.ini +++ b/tests/wpt/metadata/css/css-variables/variable-presentation-attribute.html.ini @@ -110,9 +110,6 @@ [Testing 'visibility'.] expected: FAIL - [Testing 'word-spacing'.] - expected: FAIL - [Testing 'writing-mode'.] expected: FAIL