From 149cb5f5f1ae7a36f6ae0cc79613419cf8b0f66c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 10 Feb 2020 20:02:31 +0000 Subject: [PATCH 01/74] style: Merge ImageLayer and Image. ImageLayer is almost the only usage of Image, so keeping them in the same enum makes the resulting C++ struct smaller, and makes it map more cleanly to nsStyleImage. Differential Revision: https://phabricator.services.mozilla.com/D62161 --- components/style/gecko/conversions.rs | 27 +++++----- components/style/properties/gecko.mako.rs | 27 ++-------- .../properties/longhands/background.mako.rs | 6 +-- .../style/properties/longhands/border.mako.rs | 6 +-- .../style/properties/longhands/svg.mako.rs | 6 +-- components/style/traversal.rs | 4 +- components/style/values/computed/image.rs | 5 +- components/style/values/computed/mod.rs | 2 +- components/style/values/generics/image.rs | 49 +++++-------------- .../style/values/specified/basic_shape.rs | 15 ++++-- components/style/values/specified/image.rs | 25 ++-------- components/style/values/specified/mod.rs | 2 +- 12 files changed, 62 insertions(+), 112 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 3deaeacadc8..5390d904d02 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -23,6 +23,9 @@ impl nsStyleImage { /// Set a given Servo `Image` value into this `nsStyleImage`. pub fn set(&mut self, image: Image) { match image { + GenericImage::None => unsafe { + bindings::Gecko_SetNullImageValue(self); + }, GenericImage::Gradient(boxed_gradient) => self.set_gradient(boxed_gradient), GenericImage::Url(ref url) => unsafe { bindings::Gecko_SetLayerImageImageValue(self, url); @@ -55,35 +58,35 @@ impl nsStyleImage { } /// Converts into Image. - pub unsafe fn into_image(self: &nsStyleImage) -> Option { + pub unsafe fn to_image(&self) -> Image { use crate::gecko_bindings::structs::nsStyleImageType; use crate::values::computed::MozImageRect; match self.mType { - nsStyleImageType::eStyleImageType_Null => None, + nsStyleImageType::eStyleImageType_Null => GenericImage::None, nsStyleImageType::eStyleImageType_Image => { let url = self.__bindgen_anon_1.mImage.as_ref().clone(); if self.mCropRect.mPtr.is_null() { - Some(GenericImage::Url(url)) + GenericImage::Url(url) } else { let rect = &*self.mCropRect.mPtr; - Some(GenericImage::Rect(Box::new(MozImageRect { + GenericImage::Rect(Box::new(MozImageRect { url, top: rect.0, right: rect.1, bottom: rect.2, left: rect.3, - }))) + })) } }, nsStyleImageType::eStyleImageType_Gradient => { let gradient: &Gradient = &**self.__bindgen_anon_1.mGradient.as_ref(); - Some(GenericImage::Gradient(Box::new(gradient.clone()))) + GenericImage::Gradient(Box::new(gradient.clone())) }, nsStyleImageType::eStyleImageType_Element => { use crate::gecko_string_cache::Atom; let atom = bindings::Gecko_GetImageElement(self); - Some(GenericImage::Element(Atom::from_raw(atom))) + GenericImage::Element(Atom::from_raw(atom)) }, } } @@ -102,7 +105,7 @@ pub mod basic_shape { impl StyleShapeSource { /// Convert StyleShapeSource to ShapeSource except URL and Image /// types. - fn into_shape_source( + fn to_shape_source( &self, ) -> Option> where @@ -149,14 +152,14 @@ pub mod basic_shape { use crate::values::generics::image::Image as GenericImage; let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr; - let image = shape_image.into_image().expect("Cannot convert to Image"); + let image = shape_image.to_image(); match image { GenericImage::Url(url) => ShapeSource::ImageOrUrl(url.0), _ => panic!("ClippingShape doesn't support non-url images"), } }, _ => other - .into_shape_source() + .to_shape_source() .expect("Couldn't convert to StyleSource!"), } } @@ -167,11 +170,11 @@ pub mod basic_shape { match other.mType { StyleShapeSourceType::Image => unsafe { let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr; - let image = shape_image.into_image().expect("Cannot convert to Image"); + let image = shape_image.to_image(); ShapeSource::ImageOrUrl(image) }, _ => other - .into_shape_source() + .to_shape_source() .expect("Couldn't convert to StyleSource!"), } } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 6cfb4c859b4..e6cd75e2ee0 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -47,7 +47,6 @@ use crate::values::computed::url::ComputedImageUrl; use crate::values::computed::BorderStyle; use crate::values::computed::font::FontSize; use crate::values::generics::column::ColumnCount; -use crate::values::generics::image::ImageLayer; pub mod style_structs { @@ -849,14 +848,7 @@ fn static_assert() { % endfor pub fn set_border_image_source(&mut self, image: longhands::border_image_source::computed_value::T) { - unsafe { - // Prevent leaking of the last elements we did set - Gecko_SetNullImageValue(&mut self.gecko.mBorderImageSource); - } - - if let ImageLayer::Image(image) = image { - self.gecko.mBorderImageSource.set(image); - } + self.gecko.mBorderImageSource.set(image); } pub fn copy_border_image_source_from(&mut self, other: &Self) { @@ -871,10 +863,7 @@ fn static_assert() { } pub fn clone_border_image_source(&self) -> longhands::border_image_source::computed_value::T { - match unsafe { self.gecko.mBorderImageSource.into_image() } { - Some(image) => ImageLayer::Image(image), - None => ImageLayer::None, - } + unsafe { self.gecko.mBorderImageSource.to_image() } } <% @@ -2042,9 +2031,7 @@ fn static_assert() { for (image, geckoimage) in images.zip(self.gecko.${image_layers_field} .mLayers.iter_mut()) { - if let ImageLayer::Image(image) = image { - geckoimage.mImage.set(image) - } + geckoimage.mImage.set(image) } } @@ -2052,12 +2039,8 @@ fn static_assert() { longhands::${shorthand}_image::computed_value::List( self.gecko.${image_layers_field}.mLayers.iter() .take(self.gecko.${image_layers_field}.mImageCount as usize) - .map(|ref layer| { - match unsafe { layer.mImage.into_image() } { - Some(image) => ImageLayer::Image(image), - None => ImageLayer::None, - } - }).collect() + .map(|layer| unsafe { layer.mImage.to_image() }) + .collect() ) } diff --git a/components/style/properties/longhands/background.mako.rs b/components/style/properties/longhands/background.mako.rs index e0ef021f32d..a0b026c592e 100644 --- a/components/style/properties/longhands/background.mako.rs +++ b/components/style/properties/longhands/background.mako.rs @@ -21,10 +21,10 @@ ${helpers.predefined_type( ${helpers.predefined_type( "background-image", - "ImageLayer", + "Image", engines="gecko servo-2013 servo-2020", - initial_value="computed::ImageLayer::none()", - initial_specified_value="specified::ImageLayer::none()", + initial_value="computed::Image::None", + initial_specified_value="specified::Image::None", spec="https://drafts.csswg.org/css-backgrounds/#the-background-image", vector="True", animation_value_type="discrete", diff --git a/components/style/properties/longhands/border.mako.rs b/components/style/properties/longhands/border.mako.rs index f281fa1a30d..1c4eae16939 100644 --- a/components/style/properties/longhands/border.mako.rs +++ b/components/style/properties/longhands/border.mako.rs @@ -106,10 +106,10 @@ ${helpers.single_keyword( ${helpers.predefined_type( "border-image-source", - "ImageLayer", + "Image", engines="gecko servo-2013 servo-2020", - initial_value="computed::ImageLayer::none()", - initial_specified_value="specified::ImageLayer::none()", + initial_value="computed::Image::None", + initial_specified_value="specified::Image::None", spec="https://drafts.csswg.org/css-backgrounds/#the-background-image", vector=False, animation_value_type="discrete", diff --git a/components/style/properties/longhands/svg.mako.rs b/components/style/properties/longhands/svg.mako.rs index 724c38b2d1b..3b992ba3f5a 100644 --- a/components/style/properties/longhands/svg.mako.rs +++ b/components/style/properties/longhands/svg.mako.rs @@ -172,10 +172,10 @@ ${helpers.single_keyword( ${helpers.predefined_type( "mask-image", - "ImageLayer", + "Image", engines="gecko", - initial_value="computed::ImageLayer::none()", - initial_specified_value="specified::ImageLayer::none()", + initial_value="computed::Image::None", + initial_specified_value="specified::Image::None", parse_method="parse_with_cors_anonymous", spec="https://drafts.fxtf.org/css-masking/#propdef-mask-image", vector=True, diff --git a/components/style/traversal.rs b/components/style/traversal.rs index a6aa220ccfa..3811597084b 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -684,7 +684,7 @@ fn notify_paint_worklet(context: &StyleContext, data: &ElementData) where E: TElement, { - use crate::values::generics::image::{GenericImageLayer, Image}; + use crate::values::generics::image::Image; use style_traits::ToCss; // We speculatively evaluate any paint worklets during styling. @@ -694,7 +694,7 @@ where if let Some(ref values) = data.styles.primary { for image in &values.get_background().background_image.0 { let (name, arguments) = match *image { - GenericImageLayer::Image(Image::PaintWorklet(ref worklet)) => { + Image::PaintWorklet(ref worklet) => { (&worklet.name, &worklet.arguments) }, _ => continue, diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs index bd59acac547..8610888bef7 100644 --- a/components/style/values/computed/image.rs +++ b/components/style/values/computed/image.rs @@ -22,9 +22,6 @@ use std::f32::consts::PI; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; -/// A computed image layer. -pub type ImageLayer = generic::GenericImageLayer; - /// Computed values for an image according to CSS-IMAGES. /// pub type Image = generic::GenericImage; @@ -65,7 +62,7 @@ pub type ColorStop = generic::ColorStop; /// Computed values for `-moz-image-rect(...)`. #[cfg(feature = "gecko")] -pub type MozImageRect = generic::MozImageRect; +pub type MozImageRect = generic::GenericMozImageRect; /// Empty enum on non-gecko #[cfg(not(feature = "gecko"))] diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 7cfd1e20abe..c1c83f829a8 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -57,7 +57,7 @@ pub use self::font::{FontSize, FontSizeAdjust, FontStretch, FontSynthesis}; pub use self::font::{FontVariantAlternates, FontWeight}; pub use self::font::{FontVariantEastAsian, FontVariationSettings}; pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XLang, XTextZoom}; -pub use self::image::{Gradient, GradientItem, Image, ImageLayer, LineDirection, MozImageRect}; +pub use self::image::{Gradient, GradientItem, Image, LineDirection, MozImageRect}; pub use self::length::{CSSPixelLength, ExtremumLength, NonNegativeLength}; pub use self::length::{Length, LengthOrNumber, LengthPercentage, NonNegativeLengthOrNumber}; pub use self::length::{LengthOrAuto, LengthPercentageOrAuto, MaxSize, Size}; diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index a6b45bc82ad..4b79dcb9cb8 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -13,51 +13,22 @@ use servo_arc::Arc; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; -/// An | (for background-image, for example). -#[derive( - Clone, - Debug, - MallocSizeOf, - Parse, - PartialEq, - SpecifiedValueInfo, - ToComputedValue, - ToCss, - ToResolvedValue, - ToShmem, -)] -pub enum GenericImageLayer { - /// The `none` value. - None, - /// The `` value. - Image(Image), -} - -pub use self::GenericImageLayer as ImageLayer; - -impl ImageLayer { - /// Returns `none`. - #[inline] - pub fn none() -> Self { - ImageLayer::None - } -} - -/// An [image]. +/// An ` | none` value. /// -/// [image]: https://drafts.csswg.org/css-images/#image-values +/// https://drafts.csswg.org/css-images/#image-values #[derive( Clone, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem, )] #[repr(C, u8)] -pub enum GenericImage { +pub enum GenericImage { + /// `none` variant. + None, /// A `` image. Url(ImageUrl), /// A `` image. Gradients are rather large, and not nearly as /// common as urls, so we box them here to keep the size of this enum sane. - Gradient(Box), - + Gradient(Box), /// A `-moz-image-rect` image. Also fairly large and rare. // not cfg’ed out on non-Gecko to avoid `error[E0392]: parameter `MozImageRect` is never used` // Instead we make MozImageRect an empty enum @@ -285,7 +256,7 @@ impl ToCss for PaintWorklet { /// /// `-moz-image-rect(, top, right, bottom, left);` #[allow(missing_docs)] -#[css(comma, function)] +#[css(comma, function = "-moz-image-rect")] #[derive( Clone, Debug, @@ -297,7 +268,8 @@ impl ToCss for PaintWorklet { ToResolvedValue, ToShmem, )] -pub struct MozImageRect { +#[repr(C)] +pub struct GenericMozImageRect { pub url: MozImageRectUrl, pub top: NumberOrPercentage, pub right: NumberOrPercentage, @@ -305,6 +277,8 @@ pub struct MozImageRect { pub left: NumberOrPercentage, } +pub use self::GenericMozImageRect as MozImageRect; + impl fmt::Debug for Image where G: ToCss, @@ -327,6 +301,7 @@ where W: Write, { match *self { + Image::None => dest.write_str("none"), Image::Url(ref url) => url.to_css(dest), Image::Gradient(ref gradient) => gradient.to_css(dest), Image::Rect(ref rect) => rect.to_css(dest), diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index 3c32144e71c..68938b4e6dd 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -71,6 +71,10 @@ impl Parse for ClippingShape { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { + if input.try(|i| i.expect_ident_matching("none")).is_ok() { + return Ok(ShapeSource::None); + } + if is_clip_path_path_enabled(context) { if let Ok(p) = input.try(|i| Path::parse(context, i)) { return Ok(ShapeSource::Path(p)); @@ -91,7 +95,14 @@ impl Parse for FloatAreaShape { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { + // Need to parse this here so that `Image::parse_with_cors_anonymous` + // doesn't parse it. + if input.try(|i| i.expect_ident_matching("none")).is_ok() { + return Ok(ShapeSource::None); + } + if let Ok(image) = input.try(|i| Image::parse_with_cors_anonymous(context, i)) { + debug_assert_ne!(image, Image::None); return Ok(ShapeSource::ImageOrUrl(image)); } @@ -108,10 +119,6 @@ where context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - if input.try(|i| i.expect_ident_matching("none")).is_ok() { - return Ok(ShapeSource::None); - } - fn parse_component( context: &ParserContext, input: &mut Parser, diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index 14a2a9ad9c8..369b1936f87 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -32,24 +32,6 @@ use std::fmt::{self, Write}; use style_traits::{CssType, CssWriter, KeywordsCollectFn, ParseError}; use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss}; -/// A specified image layer. -pub type ImageLayer = generic::GenericImageLayer; - -impl ImageLayer { - /// This is a specialization of Either with an alternative parse - /// method to provide anonymous CORS headers for the Image url fetch. - pub fn parse_with_cors_anonymous<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(v) = input.try(|i| Image::parse_with_cors_anonymous(context, i)) { - return Ok(generic::GenericImageLayer::Image(v)); - } - input.expect_ident_matching("none")?; - Ok(generic::GenericImageLayer::None) - } -} - /// Specified values for an image according to CSS-IMAGES. /// pub type Image = generic::Image; @@ -118,8 +100,8 @@ pub type ColorStop = generic::ColorStop; /// Specified values for `moz-image-rect` /// -moz-image-rect(, top, right, bottom, left); -#[cfg(feature = "gecko")] -pub type MozImageRect = generic::MozImageRect; +#[cfg(all(feature = "gecko", not(feature = "cbindgen")))] +pub type MozImageRect = generic::GenericMozImageRect; #[cfg(not(feature = "gecko"))] #[derive( @@ -141,6 +123,9 @@ impl Parse for Image { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { + if input.try(|i| i.expect_ident_matching("none")).is_ok() { + return Ok(generic::Image::None); + } if let Ok(url) = input.try(|input| SpecifiedImageUrl::parse(context, input)) { return Ok(generic::Image::Url(url)); } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 54401045809..d45643d6604 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -56,7 +56,7 @@ pub use self::font::{FontVariantAlternates, FontWeight}; pub use self::font::{FontVariantEastAsian, FontVariationSettings}; pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XLang, XTextZoom}; pub use self::image::{ColorStop, EndingShape as GradientEndingShape, Gradient}; -pub use self::image::{GradientItem, GradientKind, Image, ImageLayer, MozImageRect}; +pub use self::image::{GradientItem, GradientKind, Image, MozImageRect}; pub use self::length::{AbsoluteLength, CalcLengthPercentage, CharacterWidth}; pub use self::length::{FontRelativeLength, Length, LengthOrNumber, NonNegativeLengthOrNumber}; pub use self::length::{LengthOrAuto, LengthPercentage, LengthPercentageOrAuto}; From 08175a494e392b26033a51bf095474a416fbd46e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 11 Feb 2020 01:23:37 +0000 Subject: [PATCH 02/74] style: Remove some include dependencies in nsStyleConsts.h. We include it everywhere because it's included from gfxTypes.h. This should avoid including all the generated bindings _everywhere_. Differential Revision: https://phabricator.services.mozilla.com/D62174 --- components/style/values/computed/font.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 6867f582d7d..642dbc37eeb 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -308,8 +308,10 @@ pub enum SingleFontFamily { #[allow(missing_docs)] pub enum GenericFontFamily { /// No generic family specified, only for internal usage. + /// + /// NOTE(emilio): Gecko code relies on this variant being zero. #[css(skip)] - None, + None = 0, Serif, SansSerif, #[parse(aliases = "-moz-fixed")] From 4dadcf71a5c315953dc78806c7c3f40141884fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 11 Feb 2020 20:56:13 +0000 Subject: [PATCH 03/74] style: Some preparation to start using cbindgen for shape-outside and clip-path. Tweak the ShapeSourceRepresentation so that it doesn't store Option<>s. Some renames so that GeometryBox doesn't conflict with the Gecko type, and some other usual bits / re-exports to deal with cbindgen and generics. Also, drive-by derive parsing of GeometryBox as it's trivial. Doing this unfortunately is not possible without removing nsStyleImage first, so let's do that before. This makes us serialize in the shortest form for shape-outside, but that's what we should do anyway. (aside: the shapes code is a bit too generic, maybe we should unify ClippingShape and FloatAreaShape...) Differential Revision: https://phabricator.services.mozilla.com/D62163 --- components/style/gecko/conversions.rs | 45 +++++++------- components/style/properties/gecko.mako.rs | 6 +- .../style/values/computed/basic_shape.rs | 4 +- .../style/values/generics/basic_shape.rs | 59 +++++++++++++++---- .../style/values/specified/basic_shape.rs | 30 ++-------- 5 files changed, 76 insertions(+), 68 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 5390d904d02..52007c55622 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -99,7 +99,7 @@ pub mod basic_shape { }; use crate::values::computed::basic_shape::{BasicShape, ClippingShape, FloatAreaShape}; use crate::values::computed::motion::OffsetPath; - use crate::values::generics::basic_shape::{GeometryBox, Path, ShapeBox, ShapeSource}; + use crate::values::generics::basic_shape::{ShapeGeometryBox, Path, ShapeBox, ShapeSource}; use crate::values::specified::SVGPathData; impl StyleShapeSource { @@ -109,7 +109,7 @@ pub mod basic_shape { &self, ) -> Option> where - ReferenceBox: From, + ReferenceBox: From + Default + PartialEq, { match self.mType { StyleShapeSourceType::None => Some(ShapeSource::None), @@ -117,11 +117,7 @@ pub mod basic_shape { StyleShapeSourceType::Shape => { let other_shape = unsafe { &*self.__bindgen_anon_1.mBasicShape.as_ref().mPtr }; let shape = Box::new(other_shape.clone()); - let reference_box = if self.mReferenceBox == StyleGeometryBox::NoBox { - None - } else { - Some(self.mReferenceBox.into()) - }; + let reference_box = self.mReferenceBox.into(); Some(ShapeSource::Shape(shape, reference_box)) }, StyleShapeSourceType::Image => None, @@ -207,33 +203,32 @@ pub mod basic_shape { } } - impl From for StyleGeometryBox { - fn from(reference: GeometryBox) -> Self { + impl From for StyleGeometryBox { + fn from(reference: ShapeGeometryBox) -> Self { use crate::gecko_bindings::structs::StyleGeometryBox::*; match reference { - GeometryBox::ShapeBox(shape_box) => From::from(shape_box), - GeometryBox::FillBox => FillBox, - GeometryBox::StrokeBox => StrokeBox, - GeometryBox::ViewBox => ViewBox, + ShapeGeometryBox::ShapeBox(shape_box) => From::from(shape_box), + ShapeGeometryBox::FillBox => FillBox, + ShapeGeometryBox::StrokeBox => StrokeBox, + ShapeGeometryBox::ViewBox => ViewBox, + ShapeGeometryBox::ElementDependent => NoBox, } } } - // Will panic on NoBox - // Ideally these would be implemented on Option, - // but coherence doesn't like that and TryFrom isn't stable - impl From for GeometryBox { + impl From for ShapeGeometryBox { fn from(reference: StyleGeometryBox) -> Self { use crate::gecko_bindings::structs::StyleGeometryBox::*; match reference { - ContentBox => GeometryBox::ShapeBox(ShapeBox::ContentBox), - PaddingBox => GeometryBox::ShapeBox(ShapeBox::PaddingBox), - BorderBox => GeometryBox::ShapeBox(ShapeBox::BorderBox), - MarginBox => GeometryBox::ShapeBox(ShapeBox::MarginBox), - FillBox => GeometryBox::FillBox, - StrokeBox => GeometryBox::StrokeBox, - ViewBox => GeometryBox::ViewBox, - _ => panic!("Unexpected StyleGeometryBox while converting to GeometryBox"), + ContentBox => ShapeGeometryBox::ShapeBox(ShapeBox::ContentBox), + PaddingBox => ShapeGeometryBox::ShapeBox(ShapeBox::PaddingBox), + BorderBox => ShapeGeometryBox::ShapeBox(ShapeBox::BorderBox), + MarginBox => ShapeGeometryBox::ShapeBox(ShapeBox::MarginBox), + FillBox => ShapeGeometryBox::FillBox, + StrokeBox => ShapeGeometryBox::StrokeBox, + ViewBox => ShapeGeometryBox::ViewBox, + NoBox => ShapeGeometryBox::ElementDependent, + NoClip | Text | MozAlmostPadding => unreachable!(), } } } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index e6cd75e2ee0..1a9eba8eff8 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2247,7 +2247,6 @@ fn set_style_svg_path( pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { use crate::values::generics::basic_shape::ShapeSource; use crate::gecko_bindings::structs::StyleShapeSourceType; - use crate::gecko_bindings::structs::StyleGeometryBox; let ref mut ${ident} = self.gecko.${gecko_ffi_name}; @@ -2275,13 +2274,12 @@ fn set_style_svg_path( ${ident}.mType = StyleShapeSourceType::Box; } ShapeSource::Path(p) => set_style_svg_path(${ident}, p.path, p.fill), - ShapeSource::Shape(servo_shape, maybe_box) => { + ShapeSource::Shape(servo_shape, reference_box) => { unsafe { ${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr = Box::into_raw(servo_shape); } - ${ident}.mReferenceBox = - maybe_box.map(Into::into).unwrap_or(StyleGeometryBox::NoBox); + ${ident}.mReferenceBox = reference_box.into(); ${ident}.mType = StyleShapeSourceType::Shape; } } diff --git a/components/style/values/computed/basic_shape.rs b/components/style/values/computed/basic_shape.rs index 27a091a1115..2b36809fd7e 100644 --- a/components/style/values/computed/basic_shape.rs +++ b/components/style/values/computed/basic_shape.rs @@ -15,10 +15,10 @@ use crate::values::generics::basic_shape as generic; pub use crate::values::generics::basic_shape::FillRule; /// A computed clipping shape. -pub type ClippingShape = generic::ClippingShape; +pub type ClippingShape = generic::GenericClippingShape; /// A computed float area shape. -pub type FloatAreaShape = generic::FloatAreaShape; +pub type FloatAreaShape = generic::GenericFloatAreaShape; /// A computed basic shape. pub type BasicShape = generic::GenericBasicShape< diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index d91fa58eb0d..92a0a6d7839 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -16,7 +16,7 @@ use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; /// A clipping shape, for `clip-path`. -pub type ClippingShape = ShapeSource; +pub type GenericClippingShape = GenericShapeSource; /// #[allow(missing_docs)] @@ -27,6 +27,7 @@ pub type ClippingShape = ShapeSource = ShapeSource Self { + Self::ElementDependent + } +} + /// A float area shape, for `shape-outside`. -pub type FloatAreaShape = ShapeSource; +pub type GenericFloatAreaShape = GenericShapeSource; /// https://drafts.csswg.org/css-shapes-1/#typedef-shape-box #[allow(missing_docs)] @@ -63,6 +79,7 @@ pub type FloatAreaShape = ShapeSource Self { + ShapeBox::MarginBox + } +} + /// A shape source, for some reference box. #[allow(missing_docs)] -#[animation(no_bound(ImageOrUrl))] +#[animation(no_bound(I))] #[derive( Animate, Clone, @@ -86,10 +109,14 @@ pub enum ShapeBox { ToResolvedValue, ToShmem, )] -pub enum ShapeSource { +#[repr(u8)] +pub enum GenericShapeSource +where + ReferenceBox: Default + PartialEq, +{ #[animation(error)] - ImageOrUrl(ImageOrUrl), - Shape(Box, Option), + ImageOrUrl(I), + Shape(Box, #[css(skip_if = "is_default")] ReferenceBox), #[animation(error)] Box(ReferenceBox), #[css(function)] @@ -98,6 +125,8 @@ pub enum ShapeSource { None, } +pub use self::GenericShapeSource as ShapeSource; + #[allow(missing_docs)] #[derive( Animate, @@ -252,7 +281,7 @@ pub use self::GenericShapeRadius as ShapeRadius; #[repr(C)] pub struct GenericPolygon { /// The filling rule for a polygon. - #[css(skip_if = "fill_is_default")] + #[css(skip_if = "is_default")] pub fill: FillRule, /// A collection of (x, y) coordinates to draw the polygon. #[css(iterable)] @@ -321,9 +350,10 @@ pub enum FillRule { ToResolvedValue, ToShmem, )] +#[repr(C)] pub struct Path { /// The filling rule for the svg path. - #[css(skip_if = "fill_is_default")] + #[css(skip_if = "is_default")] #[animation(constant)] pub fill: FillRule, /// The svg path data. @@ -335,7 +365,7 @@ pub struct Path { impl ComputeSquaredDistance for ShapeSource where B: ComputeSquaredDistance, - T: PartialEq, + T: Default + PartialEq, { fn compute_squared_distance(&self, other: &Self) -> Result { match (self, other) { @@ -353,7 +383,10 @@ where } } -impl ToAnimatedZero for ShapeSource { +impl ToAnimatedZero for ShapeSource +where + T: Default + PartialEq, +{ fn to_animated_zero(&self) -> Result { Err(()) } @@ -488,6 +521,6 @@ impl Default for FillRule { } #[inline] -fn fill_is_default(fill: &FillRule) -> bool { - *fill == FillRule::default() +fn is_default(fill: &T) -> bool { + *fill == Default::default() } diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index 68938b4e6dd..188ec19279f 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -9,8 +9,7 @@ use crate::parser::{Parse, ParserContext}; use crate::values::generics::basic_shape as generic; -use crate::values::generics::basic_shape::{GeometryBox, Path, PolygonCoord}; -use crate::values::generics::basic_shape::{ShapeBox, ShapeSource}; +use crate::values::generics::basic_shape::{Path, PolygonCoord, ShapeSource}; use crate::values::generics::rect::Rect; use crate::values::specified::border::BorderRadius; use crate::values::specified::image::Image; @@ -26,13 +25,13 @@ use style_traits::{ParseError, StyleParseErrorKind}; pub use crate::values::generics::basic_shape::FillRule; /// A specified clipping shape. -pub type ClippingShape = generic::ClippingShape; +pub type ClippingShape = generic::GenericClippingShape; /// A specified float area shape. -pub type FloatAreaShape = generic::FloatAreaShape; +pub type FloatAreaShape = generic::GenericFloatAreaShape; /// A specified basic shape. -pub type BasicShape = generic::BasicShape< +pub type BasicShape = generic::GenericBasicShape< HorizontalPosition, VerticalPosition, LengthPercentage, @@ -112,7 +111,7 @@ impl Parse for FloatAreaShape { impl ShapeSource where - ReferenceBox: Parse, + ReferenceBox: Parse + Default + PartialEq, { /// The internal parser for ShapeSource. fn parse_common<'i, 't>( @@ -142,7 +141,7 @@ where } if let Some(shp) = shape { - return Ok(ShapeSource::Shape(Box::new(shp), ref_box)); + return Ok(ShapeSource::Shape(Box::new(shp), ref_box.unwrap_or_default())); } ref_box @@ -151,23 +150,6 @@ where } } -impl Parse for GeometryBox { - fn parse<'i, 't>( - _context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - if let Ok(shape_box) = input.try(ShapeBox::parse) { - return Ok(GeometryBox::ShapeBox(shape_box)); - } - - try_match_ident_ignore_ascii_case! { input, - "fill-box" => Ok(GeometryBox::FillBox), - "stroke-box" => Ok(GeometryBox::StrokeBox), - "view-box" => Ok(GeometryBox::ViewBox), - } - } -} - impl Parse for BasicShape { fn parse<'i, 't>( context: &ParserContext, From 558cc59288a6c5371ea06aa2a973a74be4e51d8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 11 Feb 2020 20:56:20 +0000 Subject: [PATCH 04/74] style: Use cbindgen instead of nsStyleImage. The trickier part is that we represent -moz-image-rect as a Rect() type instead of image with non-null clip-rect. So we need to add a bit of code to distinguish "image request types" from other types of images. But it's not too annoying, and we need to do the same for fancier images like image-set and such whenever we implement it, so seems nice to get rid of most explicit usages of nsStyleImage::GetType(). Differential Revision: https://phabricator.services.mozilla.com/D62164 --- components/style/gecko/conversions.rs | 89 ++--------------------- components/style/properties/gecko.mako.rs | 45 +++--------- 2 files changed, 15 insertions(+), 119 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 52007c55622..3a061c97c53 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -10,87 +10,10 @@ #![allow(unsafe_code)] -use crate::gecko_bindings::bindings; -use crate::gecko_bindings::structs::{self, Matrix4x4Components}; -use crate::gecko_bindings::structs::{nsStyleImage, nsresult}; +use crate::gecko_bindings::structs::{self, Matrix4x4Components, nsresult}; use crate::stylesheets::RulesMutateError; use crate::values::computed::transform::Matrix3D; -use crate::values::computed::{Gradient, Image, TextAlign}; -use crate::values::generics::image::GenericImage; -use crate::values::generics::rect::Rect; - -impl nsStyleImage { - /// Set a given Servo `Image` value into this `nsStyleImage`. - pub fn set(&mut self, image: Image) { - match image { - GenericImage::None => unsafe { - bindings::Gecko_SetNullImageValue(self); - }, - GenericImage::Gradient(boxed_gradient) => self.set_gradient(boxed_gradient), - GenericImage::Url(ref url) => unsafe { - bindings::Gecko_SetLayerImageImageValue(self, url); - }, - GenericImage::Rect(ref image_rect) => { - unsafe { - bindings::Gecko_SetLayerImageImageValue(self, &image_rect.url); - bindings::Gecko_InitializeImageCropRect(self); - - // Set CropRect - let ref mut rect = *self.mCropRect.mPtr; - *rect = Rect( - image_rect.top, - image_rect.right, - image_rect.bottom, - image_rect.left, - ); - } - }, - GenericImage::Element(ref element) => unsafe { - bindings::Gecko_SetImageElement(self, element.as_ptr()); - }, - } - } - - fn set_gradient(&mut self, gradient: Box) { - unsafe { - bindings::Gecko_SetGradientImageValue(self, Box::into_raw(gradient)); - } - } - - /// Converts into Image. - pub unsafe fn to_image(&self) -> Image { - use crate::gecko_bindings::structs::nsStyleImageType; - use crate::values::computed::MozImageRect; - - match self.mType { - nsStyleImageType::eStyleImageType_Null => GenericImage::None, - nsStyleImageType::eStyleImageType_Image => { - let url = self.__bindgen_anon_1.mImage.as_ref().clone(); - if self.mCropRect.mPtr.is_null() { - GenericImage::Url(url) - } else { - let rect = &*self.mCropRect.mPtr; - GenericImage::Rect(Box::new(MozImageRect { - url, - top: rect.0, - right: rect.1, - bottom: rect.2, - left: rect.3, - })) - } - }, - nsStyleImageType::eStyleImageType_Gradient => { - let gradient: &Gradient = &**self.__bindgen_anon_1.mGradient.as_ref(); - GenericImage::Gradient(Box::new(gradient.clone())) - }, - nsStyleImageType::eStyleImageType_Element => { - use crate::gecko_string_cache::Atom; - let atom = bindings::Gecko_GetImageElement(self); - GenericImage::Element(Atom::from_raw(atom)) - }, - } - } -} +use crate::values::computed::TextAlign; pub mod basic_shape { //! Conversions from and to CSS shape representations. @@ -148,9 +71,8 @@ pub mod basic_shape { use crate::values::generics::image::Image as GenericImage; let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr; - let image = shape_image.to_image(); - match image { - GenericImage::Url(url) => ShapeSource::ImageOrUrl(url.0), + match *shape_image { + GenericImage::Url(ref url) => ShapeSource::ImageOrUrl(url.0.clone()), _ => panic!("ClippingShape doesn't support non-url images"), } }, @@ -166,8 +88,7 @@ pub mod basic_shape { match other.mType { StyleShapeSourceType::Image => unsafe { let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr; - let image = shape_image.to_image(); - ShapeSource::ImageOrUrl(image) + ShapeSource::ImageOrUrl(shape_image.clone()) }, _ => other .to_shape_source() diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 1a9eba8eff8..ba959c65f15 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -24,11 +24,9 @@ use crate::gecko_bindings::bindings::Gecko_Destroy_${style_struct.gecko_ffi_name use crate::gecko_bindings::bindings::Gecko_CopyCounterStyle; use crate::gecko_bindings::bindings::Gecko_CopyCursorArrayFrom; use crate::gecko_bindings::bindings::Gecko_CopyFontFamilyFrom; -use crate::gecko_bindings::bindings::Gecko_CopyImageValueFrom; use crate::gecko_bindings::bindings::Gecko_EnsureImageLayersLength; use crate::gecko_bindings::bindings::Gecko_nsStyleFont_SetLang; use crate::gecko_bindings::bindings::Gecko_nsStyleFont_CopyLangFrom; -use crate::gecko_bindings::bindings::Gecko_SetNullImageValue; use crate::gecko_bindings::structs; use crate::gecko_bindings::structs::nsCSSPropertyID; use crate::gecko_bindings::structs::mozilla::PseudoStyleType; @@ -777,8 +775,7 @@ fn static_assert() { for x in CORNERS]) %> <%self:impl_trait style_struct_name="Border" - skip_longhands="${skip_border_longhands} border-image-source - border-image-repeat"> + skip_longhands="${skip_border_longhands} border-image-repeat"> % for side in SIDES: pub fn set_border_${side.ident}_style(&mut self, v: BorderStyle) { self.gecko.mBorderStyle[${side.index}] = v; @@ -847,25 +844,6 @@ fn static_assert() { corner) %> % endfor - pub fn set_border_image_source(&mut self, image: longhands::border_image_source::computed_value::T) { - self.gecko.mBorderImageSource.set(image); - } - - pub fn copy_border_image_source_from(&mut self, other: &Self) { - unsafe { - Gecko_CopyImageValueFrom(&mut self.gecko.mBorderImageSource, - &other.gecko.mBorderImageSource); - } - } - - pub fn reset_border_image_source(&mut self, other: &Self) { - self.copy_border_image_source_from(other) - } - - pub fn clone_border_image_source(&self) -> longhands::border_image_source::computed_value::T { - unsafe { self.gecko.mBorderImageSource.to_image() } - } - <% border_image_repeat_keywords = ["Stretch", "Repeat", "Round", "Space"] %> @@ -1998,7 +1976,7 @@ fn static_assert() { for (layer, other) in self.gecko.${image_layers_field}.mLayers.iter_mut() .zip(other.gecko.${image_layers_field}.mLayers.iter()) .take(count as usize) { - Gecko_CopyImageValueFrom(&mut layer.mImage, &other.mImage); + layer.mImage = other.mImage.clone(); } self.gecko.${image_layers_field}.mImageCount = count; } @@ -2018,20 +1996,17 @@ fn static_assert() { let images = images.into_iter(); unsafe { - // Prevent leaking of the last elements we did set - for image in &mut self.gecko.${image_layers_field}.mLayers { - Gecko_SetNullImageValue(&mut image.mImage) - } - // XXXManishearth clear mSourceURI for masks - Gecko_EnsureImageLayersLength(&mut self.gecko.${image_layers_field}, images.len(), - LayerType::${shorthand.title()}); + Gecko_EnsureImageLayersLength( + &mut self.gecko.${image_layers_field}, + images.len(), + LayerType::${shorthand.title()}, + ); } self.gecko.${image_layers_field}.mImageCount = images.len() as u32; - for (image, geckoimage) in images.zip(self.gecko.${image_layers_field} .mLayers.iter_mut()) { - geckoimage.mImage.set(image) + geckoimage.mImage = image; } } @@ -2039,7 +2014,7 @@ fn static_assert() { longhands::${shorthand}_image::computed_value::List( self.gecko.${image_layers_field}.mLayers.iter() .take(self.gecko.${image_layers_field}.mImageCount as usize) - .map(|layer| unsafe { layer.mImage.to_image() }) + .map(|layer| layer.mImage.clone()) .collect() ) } @@ -2266,7 +2241,7 @@ fn set_style_svg_path( unsafe { bindings::Gecko_NewShapeImage(${ident}); let style_image = &mut *${ident}.__bindgen_anon_1.mShapeImage.as_mut().mPtr; - style_image.set(image); + *style_image = image; } } ShapeSource::Box(reference) => { From ab03688994515b81312c2472f2a2c65bbccaf366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 11 Feb 2020 22:03:53 +0000 Subject: [PATCH 05/74] style: Use cbindgen for shape-outside and clip-path. Differential Revision: https://phabricator.services.mozilla.com/D62372 --- components/style/gecko/conversions.rs | 153 ---------------------- components/style/properties/gecko.mako.rs | 82 +----------- 2 files changed, 1 insertion(+), 234 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 3a061c97c53..8fbe3f47540 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -15,159 +15,6 @@ use crate::stylesheets::RulesMutateError; use crate::values::computed::transform::Matrix3D; use crate::values::computed::TextAlign; -pub mod basic_shape { - //! Conversions from and to CSS shape representations. - use crate::gecko_bindings::structs::{ - StyleGeometryBox, StyleShapeSource, StyleShapeSourceType, - }; - use crate::values::computed::basic_shape::{BasicShape, ClippingShape, FloatAreaShape}; - use crate::values::computed::motion::OffsetPath; - use crate::values::generics::basic_shape::{ShapeGeometryBox, Path, ShapeBox, ShapeSource}; - use crate::values::specified::SVGPathData; - - impl StyleShapeSource { - /// Convert StyleShapeSource to ShapeSource except URL and Image - /// types. - fn to_shape_source( - &self, - ) -> Option> - where - ReferenceBox: From + Default + PartialEq, - { - match self.mType { - StyleShapeSourceType::None => Some(ShapeSource::None), - StyleShapeSourceType::Box => Some(ShapeSource::Box(self.mReferenceBox.into())), - StyleShapeSourceType::Shape => { - let other_shape = unsafe { &*self.__bindgen_anon_1.mBasicShape.as_ref().mPtr }; - let shape = Box::new(other_shape.clone()); - let reference_box = self.mReferenceBox.into(); - Some(ShapeSource::Shape(shape, reference_box)) - }, - StyleShapeSourceType::Image => None, - StyleShapeSourceType::Path => { - let path = self.to_svg_path().expect("expect an SVGPathData"); - let fill = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }.mFillRule; - Some(ShapeSource::Path(Path { fill, path })) - }, - } - } - - /// Generate a SVGPathData from StyleShapeSource if possible. - fn to_svg_path(&self) -> Option { - match self.mType { - StyleShapeSourceType::Path => { - let gecko_path = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }; - Some(SVGPathData(gecko_path.mPath.clone())) - }, - _ => None, - } - } - } - - impl<'a> From<&'a StyleShapeSource> for ClippingShape { - fn from(other: &'a StyleShapeSource) -> Self { - match other.mType { - StyleShapeSourceType::Image => unsafe { - use crate::values::generics::image::Image as GenericImage; - - let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr; - match *shape_image { - GenericImage::Url(ref url) => ShapeSource::ImageOrUrl(url.0.clone()), - _ => panic!("ClippingShape doesn't support non-url images"), - } - }, - _ => other - .to_shape_source() - .expect("Couldn't convert to StyleSource!"), - } - } - } - - impl<'a> From<&'a StyleShapeSource> for FloatAreaShape { - fn from(other: &'a StyleShapeSource) -> Self { - match other.mType { - StyleShapeSourceType::Image => unsafe { - let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr; - ShapeSource::ImageOrUrl(shape_image.clone()) - }, - _ => other - .to_shape_source() - .expect("Couldn't convert to StyleSource!"), - } - } - } - - impl<'a> From<&'a StyleShapeSource> for OffsetPath { - fn from(other: &'a StyleShapeSource) -> Self { - use crate::values::generics::motion::GenericOffsetPath; - match other.mType { - StyleShapeSourceType::Path => GenericOffsetPath::Path( - other.to_svg_path().expect("Cannot convert to SVGPathData"), - ), - StyleShapeSourceType::None => OffsetPath::none(), - StyleShapeSourceType::Shape | - StyleShapeSourceType::Box | - StyleShapeSourceType::Image => unreachable!("Unsupported offset-path type"), - } - } - } - - impl From for StyleGeometryBox { - fn from(reference: ShapeBox) -> Self { - use crate::gecko_bindings::structs::StyleGeometryBox::*; - match reference { - ShapeBox::ContentBox => ContentBox, - ShapeBox::PaddingBox => PaddingBox, - ShapeBox::BorderBox => BorderBox, - ShapeBox::MarginBox => MarginBox, - } - } - } - - impl From for StyleGeometryBox { - fn from(reference: ShapeGeometryBox) -> Self { - use crate::gecko_bindings::structs::StyleGeometryBox::*; - match reference { - ShapeGeometryBox::ShapeBox(shape_box) => From::from(shape_box), - ShapeGeometryBox::FillBox => FillBox, - ShapeGeometryBox::StrokeBox => StrokeBox, - ShapeGeometryBox::ViewBox => ViewBox, - ShapeGeometryBox::ElementDependent => NoBox, - } - } - } - - impl From for ShapeGeometryBox { - fn from(reference: StyleGeometryBox) -> Self { - use crate::gecko_bindings::structs::StyleGeometryBox::*; - match reference { - ContentBox => ShapeGeometryBox::ShapeBox(ShapeBox::ContentBox), - PaddingBox => ShapeGeometryBox::ShapeBox(ShapeBox::PaddingBox), - BorderBox => ShapeGeometryBox::ShapeBox(ShapeBox::BorderBox), - MarginBox => ShapeGeometryBox::ShapeBox(ShapeBox::MarginBox), - FillBox => ShapeGeometryBox::FillBox, - StrokeBox => ShapeGeometryBox::StrokeBox, - ViewBox => ShapeGeometryBox::ViewBox, - NoBox => ShapeGeometryBox::ElementDependent, - NoClip | Text | MozAlmostPadding => unreachable!(), - } - } - } - - impl From for ShapeBox { - fn from(reference: StyleGeometryBox) -> Self { - use crate::gecko_bindings::structs::StyleGeometryBox::*; - match reference { - ContentBox => ShapeBox::ContentBox, - PaddingBox => ShapeBox::PaddingBox, - BorderBox => ShapeBox::BorderBox, - MarginBox => ShapeBox::MarginBox, - _ => panic!("Unexpected StyleGeometryBox while converting to ShapeBox"), - } - } - } -} - impl From for nsresult { fn from(other: RulesMutateError) -> Self { match other { diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index ba959c65f15..400f27fa44b 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -41,7 +41,6 @@ use std::mem::{forget, MaybeUninit}; use std::{cmp, ops, ptr}; use crate::values::{self, CustomIdent, Either, KeyframesName, None_}; use crate::values::computed::{Percentage, TransitionProperty}; -use crate::values::computed::url::ComputedImageUrl; use crate::values::computed::BorderStyle; use crate::values::computed::font::FontSize; use crate::values::generics::column::ColumnCount; @@ -1433,7 +1432,7 @@ fn static_assert() { animation-iteration-count animation-timing-function clear transition-duration transition-delay transition-timing-function transition-property - shape-outside -webkit-line-clamp""" %> + -webkit-line-clamp""" %> <%self:impl_trait style_struct_name="Box" skip_longhands="${skip_box_longhands}"> #[inline] pub fn set_display(&mut self, v: longhands::display::computed_value::T) { @@ -1693,8 +1692,6 @@ fn static_assert() { ${impl_animation_timing_function()} - <% impl_shape_source("shape_outside", "mShapeOutside") %> - #[allow(non_snake_case)] pub fn set__webkit_line_clamp(&mut self, v: longhands::_webkit_line_clamp::computed_value::T) { self.gecko.mLineClamp = match v { @@ -2202,84 +2199,8 @@ fn static_assert() { } -// Set SVGPathData to StyleShapeSource. -fn set_style_svg_path( - shape_source: &mut structs::mozilla::StyleShapeSource, - servo_path: values::specified::svg_path::SVGPathData, - fill: values::generics::basic_shape::FillRule, -) { - // Setup path. - unsafe { - bindings::Gecko_SetToSVGPath( - shape_source, - servo_path.0.forget(), - fill, - ); - } -} - -<%def name="impl_shape_source(ident, gecko_ffi_name)"> - pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { - use crate::values::generics::basic_shape::ShapeSource; - use crate::gecko_bindings::structs::StyleShapeSourceType; - - let ref mut ${ident} = self.gecko.${gecko_ffi_name}; - - // clean up existing struct. - unsafe { bindings::Gecko_DestroyShapeSource(${ident}) }; - - ${ident}.mType = StyleShapeSourceType::None; - - match v { - ShapeSource::None => {} // don't change the type - ShapeSource::ImageOrUrl(image) => { - % if ident == "clip_path": - use crate::values::generics::image::Image; - - let image = Image::Url(ComputedImageUrl(image)); - % endif - unsafe { - bindings::Gecko_NewShapeImage(${ident}); - let style_image = &mut *${ident}.__bindgen_anon_1.mShapeImage.as_mut().mPtr; - *style_image = image; - } - } - ShapeSource::Box(reference) => { - ${ident}.mReferenceBox = reference.into(); - ${ident}.mType = StyleShapeSourceType::Box; - } - ShapeSource::Path(p) => set_style_svg_path(${ident}, p.path, p.fill), - ShapeSource::Shape(servo_shape, reference_box) => { - unsafe { - ${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr = - Box::into_raw(servo_shape); - } - ${ident}.mReferenceBox = reference_box.into(); - ${ident}.mType = StyleShapeSourceType::Shape; - } - } - - } - - pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T { - (&self.gecko.${gecko_ffi_name}).into() - } - - pub fn copy_${ident}_from(&mut self, other: &Self) { - use crate::gecko_bindings::bindings::Gecko_CopyShapeSourceFrom; - unsafe { - Gecko_CopyShapeSourceFrom(&mut self.gecko.${gecko_ffi_name}, &other.gecko.${gecko_ffi_name}); - } - } - - pub fn reset_${ident}(&mut self, other: &Self) { - self.copy_${ident}_from(other) - } - - <% skip_svg_longhands = """ mask-mode mask-repeat mask-clip mask-origin mask-composite mask-position-x mask-position-y mask-size mask-image -clip-path """ %> <%self:impl_trait style_struct_name="SVG" @@ -2288,7 +2209,6 @@ clip-path <% impl_common_image_layer_properties("mask") %> <% impl_simple_image_array_property("mode", "mask", "mMask", "mMaskMode", "SVG") %> <% impl_simple_image_array_property("composite", "mask", "mMask", "mComposite", "SVG") %> - <% impl_shape_source("clip_path", "mClipPath") %> <%self:impl_trait style_struct_name="InheritedSVG" From 239302b1ed68b57762a745caabcf2d0266c2294c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 11 Feb 2020 23:34:53 +0000 Subject: [PATCH 06/74] style: Split clip-path and shape-outside values. We don't actually share _that_ much code across them. This makes callers clearer and code less confusing, IMHO. This also has the benefit of not autocompleting path from devtools for shape-outside. Differential Revision: https://phabricator.services.mozilla.com/D62373 --- .../style/properties/longhands/box.mako.rs | 6 +- .../style/properties/longhands/svg.mako.rs | 6 +- .../style/values/computed/basic_shape.rs | 8 +- components/style/values/distance.rs | 10 ++ .../style/values/generics/basic_shape.rs | 93 +++++++------- .../style/values/specified/basic_shape.rs | 121 ++++++++++-------- 6 files changed, 135 insertions(+), 109 deletions(-) diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index 8946290918e..98d5ad296c9 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -679,10 +679,10 @@ ${helpers.predefined_type( ${helpers.predefined_type( "shape-outside", - "basic_shape::FloatAreaShape", - "generics::basic_shape::ShapeSource::None", + "basic_shape::ShapeOutside", + "generics::basic_shape::ShapeOutside::None", engines="gecko", - animation_value_type="basic_shape::FloatAreaShape", + animation_value_type="basic_shape::ShapeOutside", spec="https://drafts.csswg.org/css-shapes/#shape-outside-property", )} diff --git a/components/style/properties/longhands/svg.mako.rs b/components/style/properties/longhands/svg.mako.rs index 3b992ba3f5a..fbf240db205 100644 --- a/components/style/properties/longhands/svg.mako.rs +++ b/components/style/properties/longhands/svg.mako.rs @@ -76,10 +76,10 @@ ${helpers.single_keyword( ${helpers.predefined_type( "clip-path", - "basic_shape::ClippingShape", - "generics::basic_shape::ShapeSource::None", + "basic_shape::ClipPath", + "generics::basic_shape::ClipPath::None", engines="gecko", - animation_value_type="basic_shape::ClippingShape", + animation_value_type="basic_shape::ClipPath", flags="CREATES_STACKING_CONTEXT", spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path", )} diff --git a/components/style/values/computed/basic_shape.rs b/components/style/values/computed/basic_shape.rs index 2b36809fd7e..fa30220157b 100644 --- a/components/style/values/computed/basic_shape.rs +++ b/components/style/values/computed/basic_shape.rs @@ -14,11 +14,11 @@ use crate::values::generics::basic_shape as generic; /// A computed alias for FillRule. pub use crate::values::generics::basic_shape::FillRule; -/// A computed clipping shape. -pub type ClippingShape = generic::GenericClippingShape; +/// A computed `clip-path` value. +pub type ClipPath = generic::GenericClipPath; -/// A computed float area shape. -pub type FloatAreaShape = generic::GenericFloatAreaShape; +/// A computed `shape-outside` value. +pub type ShapeOutside = generic::GenericShapeOutside; /// A computed basic shape. pub type BasicShape = generic::GenericBasicShape< diff --git a/components/style/values/distance.rs b/components/style/values/distance.rs index 67c735676b5..efaa2314e01 100644 --- a/components/style/values/distance.rs +++ b/components/style/values/distance.rs @@ -81,6 +81,16 @@ impl ComputeSquaredDistance for Au { } } +impl ComputeSquaredDistance for Box +where + T: ComputeSquaredDistance, +{ + #[inline] + fn compute_squared_distance(&self, other: &Self) -> Result { + (**self).compute_squared_distance(&**other) + } +} + impl ComputeSquaredDistance for Option where T: ComputeSquaredDistance, diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index 92a0a6d7839..111af31bfeb 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -15,14 +15,12 @@ use crate::Zero; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; -/// A clipping shape, for `clip-path`. -pub type GenericClippingShape = GenericShapeSource; - /// #[allow(missing_docs)] #[derive( Animate, Clone, + ComputeSquaredDistance, Copy, Debug, MallocSizeOf, @@ -57,9 +55,6 @@ impl Default for ShapeGeometryBox { } } -/// A float area shape, for `shape-outside`. -pub type GenericFloatAreaShape = GenericShapeSource; - /// https://drafts.csswg.org/css-shapes-1/#typedef-shape-box #[allow(missing_docs)] #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] @@ -67,6 +62,7 @@ pub type GenericFloatAreaShape = GenericShapeSource -where - ReferenceBox: Default + PartialEq, -{ - #[animation(error)] - ImageOrUrl(I), - Shape(Box, #[css(skip_if = "is_default")] ReferenceBox), - #[animation(error)] - Box(ReferenceBox), - #[css(function)] - Path(Path), +pub enum GenericClipPath { #[animation(error)] None, + #[animation(error)] + Url(U), + #[css(function)] + Path(Path), + Shape(Box, #[css(skip_if = "is_default")] ShapeGeometryBox), + #[animation(error)] + Box(ShapeGeometryBox), } -pub use self::GenericShapeSource as ShapeSource; +pub use self::GenericClipPath as ClipPath; + +/// A value for the `shape-outside` property. +#[allow(missing_docs)] +#[animation(no_bound(I))] +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(u8)] +pub enum GenericShapeOutside { + #[animation(error)] + None, + #[animation(error)] + Image(I), + Shape(Box, #[css(skip_if = "is_default")] ShapeBox), + #[animation(error)] + Box(ShapeBox), +} + +pub use self::GenericShapeOutside as ShapeOutside; #[allow(missing_docs)] #[derive( @@ -340,6 +364,7 @@ pub enum FillRule { #[derive( Animate, Clone, + ComputeSquaredDistance, Debug, MallocSizeOf, PartialEq, @@ -360,33 +385,13 @@ pub struct Path { pub path: SVGPathData, } -// FIXME(nox): Implement ComputeSquaredDistance for T types and stop -// using PartialEq here, this will let us derive this impl. -impl ComputeSquaredDistance for ShapeSource -where - B: ComputeSquaredDistance, - T: Default + PartialEq, -{ - fn compute_squared_distance(&self, other: &Self) -> Result { - match (self, other) { - ( - &ShapeSource::Shape(ref this, ref this_box), - &ShapeSource::Shape(ref other, ref other_box), - ) if this_box == other_box => this.compute_squared_distance(other), - (&ShapeSource::Path(ref this), &ShapeSource::Path(ref other)) - if this.fill == other.fill => - { - this.path.compute_squared_distance(&other.path) - }, - _ => Err(()), - } +impl ToAnimatedZero for ClipPath { + fn to_animated_zero(&self) -> Result { + Err(()) } } -impl ToAnimatedZero for ShapeSource -where - T: Default + PartialEq, -{ +impl ToAnimatedZero for ShapeOutside { fn to_animated_zero(&self) -> Result { Err(()) } diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index 188ec19279f..0811fb627a7 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -9,7 +9,7 @@ use crate::parser::{Parse, ParserContext}; use crate::values::generics::basic_shape as generic; -use crate::values::generics::basic_shape::{Path, PolygonCoord, ShapeSource}; +use crate::values::generics::basic_shape::{Path, PolygonCoord}; use crate::values::generics::rect::Rect; use crate::values::specified::border::BorderRadius; use crate::values::specified::image::Image; @@ -24,11 +24,11 @@ use style_traits::{ParseError, StyleParseErrorKind}; /// A specified alias for FillRule. pub use crate::values::generics::basic_shape::FillRule; -/// A specified clipping shape. -pub type ClippingShape = generic::GenericClippingShape; +/// A specified `clip-path` value. +pub type ClipPath = generic::GenericClipPath; -/// A specified float area shape. -pub type FloatAreaShape = generic::GenericFloatAreaShape; +/// A specified `shape-outside` value. +pub type ShapeOutside = generic::GenericShapeOutside; /// A specified basic shape. pub type BasicShape = generic::GenericBasicShape< @@ -64,31 +64,78 @@ fn is_clip_path_path_enabled(_: &ParserContext) -> bool { false } -impl Parse for ClippingShape { +/// A helper for both clip-path and shape-outside parsing of shapes. +fn parse_shape_or_box<'i, 't, R, ReferenceBox>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + to_shape: impl FnOnce(Box, ReferenceBox) -> R, + to_reference_box: impl FnOnce(ReferenceBox) -> R, +) -> Result> +where + ReferenceBox: Default + Parse, +{ + fn parse_component( + context: &ParserContext, + input: &mut Parser, + component: &mut Option, + ) -> bool { + if component.is_some() { + return false; // already parsed this component + } + + *component = input.try(|i| U::parse(context, i)).ok(); + component.is_some() + } + + let mut shape = None; + let mut ref_box = None; + + while parse_component(context, input, &mut shape) || + parse_component(context, input, &mut ref_box) + { + // + } + + if let Some(shp) = shape { + return Ok(to_shape(Box::new(shp), ref_box.unwrap_or_default())); + } + + match ref_box { + Some(r) => Ok(to_reference_box(r)), + None => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), + } +} + +impl Parse for ClipPath { #[inline] fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { if input.try(|i| i.expect_ident_matching("none")).is_ok() { - return Ok(ShapeSource::None); + return Ok(ClipPath::None); } if is_clip_path_path_enabled(context) { if let Ok(p) = input.try(|i| Path::parse(context, i)) { - return Ok(ShapeSource::Path(p)); + return Ok(ClipPath::Path(p)); } } if let Ok(url) = input.try(|i| SpecifiedUrl::parse(context, i)) { - return Ok(ShapeSource::ImageOrUrl(url)); + return Ok(ClipPath::Url(url)); } - Self::parse_common(context, input) + parse_shape_or_box( + context, + input, + ClipPath::Shape, + ClipPath::Box, + ) } } -impl Parse for FloatAreaShape { +impl Parse for ShapeOutside { #[inline] fn parse<'i, 't>( context: &ParserContext, @@ -97,56 +144,20 @@ impl Parse for FloatAreaShape { // Need to parse this here so that `Image::parse_with_cors_anonymous` // doesn't parse it. if input.try(|i| i.expect_ident_matching("none")).is_ok() { - return Ok(ShapeSource::None); + return Ok(ShapeOutside::None); } if let Ok(image) = input.try(|i| Image::parse_with_cors_anonymous(context, i)) { debug_assert_ne!(image, Image::None); - return Ok(ShapeSource::ImageOrUrl(image)); + return Ok(ShapeOutside::Image(image)); } - Self::parse_common(context, input) - } -} - -impl ShapeSource -where - ReferenceBox: Parse + Default + PartialEq, -{ - /// The internal parser for ShapeSource. - fn parse_common<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - fn parse_component( - context: &ParserContext, - input: &mut Parser, - component: &mut Option, - ) -> bool { - if component.is_some() { - return false; // already parsed this component - } - - *component = input.try(|i| U::parse(context, i)).ok(); - component.is_some() - } - - let mut shape = None; - let mut ref_box = None; - - while parse_component(context, input, &mut shape) || - parse_component(context, input, &mut ref_box) - { - // - } - - if let Some(shp) = shape { - return Ok(ShapeSource::Shape(Box::new(shp), ref_box.unwrap_or_default())); - } - - ref_box - .map(ShapeSource::Box) - .ok_or(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + parse_shape_or_box( + context, + input, + ShapeOutside::Shape, + ShapeOutside::Box, + ) } } From ef2d934dace14e2c7cde43943efb4128c3e23f7c Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Tue, 11 Feb 2020 01:12:00 +0000 Subject: [PATCH 07/74] style: Drop fallback attribute from animate and distance. Still keep the discriminant checks to avoid generating terrible code. Differential Revision: https://phabricator.services.mozilla.com/D62329 --- components/style/values/animated/mod.rs | 3 --- components/style/values/distance.rs | 3 --- components/style_derive/animate.rs | 16 ++-------------- .../style_derive/compute_squared_distance.rs | 16 ++-------------- 4 files changed, 4 insertions(+), 34 deletions(-) diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index 7e699542fd4..46a13410801 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -109,9 +109,6 @@ pub fn animate_multiplicative_factor( /// If a variant is annotated with `#[animation(error)]`, the corresponding /// `match` arm returns an error. /// -/// If the two values are not similar, an error is returned unless a fallback -/// function has been specified through `#[animate(fallback)]`. -/// /// Trait bounds for type parameter `Foo` can be opted out of with /// `#[animation(no_bound(Foo))]` on the type definition, trait bounds for /// fields can be opted into with `#[animation(field_bound)]` on the field. diff --git a/components/style/values/distance.rs b/components/style/values/distance.rs index efaa2314e01..a4259ce8c6b 100644 --- a/components/style/values/distance.rs +++ b/components/style/values/distance.rs @@ -19,9 +19,6 @@ use std::ops::Add; /// If a variant is annotated with `#[animation(error)]`, the corresponding /// `match` arm returns an error. /// -/// If the two values are not similar, an error is returned unless a fallback -/// function has been specified through `#[distance(fallback)]`. -/// /// Trait bounds for type parameter `Foo` can be opted out of with /// `#[animation(no_bound(Foo))]` on the type definition, trait bounds for /// fields can be opted into with `#[distance(field_bound)]` on the field. diff --git a/components/style_derive/animate.rs b/components/style_derive/animate.rs index 190568514fc..59b19214971 100644 --- a/components/style_derive/animate.rs +++ b/components/style_derive/animate.rs @@ -6,12 +6,11 @@ use darling::util::PathList; use derive_common::cg; use proc_macro2::TokenStream; use quote::TokenStreamExt; -use syn::{DeriveInput, Path, WhereClause}; +use syn::{DeriveInput, WhereClause}; use synstructure::{Structure, VariantInfo}; pub fn derive(mut input: DeriveInput) -> TokenStream { let animation_input_attrs = cg::parse_input_attrs::(&input); - let input_attrs = cg::parse_input_attrs::(&input); let no_bound = animation_input_attrs.no_bound.unwrap_or_default(); let mut where_clause = input.generics.where_clause.take(); @@ -44,11 +43,6 @@ pub fn derive(mut input: DeriveInput) -> TokenStream { let name = &input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - let fallback = match input_attrs.fallback { - Some(fallback) => quote! { #fallback(self, other, procedure) }, - None => quote! { Err(()) }, - }; - quote! { impl #impl_generics crate::values::animated::Animate for #name #ty_generics #where_clause { #[allow(unused_variables, unused_imports)] @@ -59,7 +53,7 @@ pub fn derive(mut input: DeriveInput) -> TokenStream { procedure: crate::values::animated::Procedure, ) -> Result { if std::mem::discriminant(self) != std::mem::discriminant(other) { - return #fallback; + return Err(()); } match (self, other) { #match_body @@ -118,12 +112,6 @@ fn derive_variant_arm( } } -#[darling(attributes(animate), default)] -#[derive(Default, FromDeriveInput)] -struct AnimateInputAttrs { - fallback: Option, -} - #[darling(attributes(animation), default)] #[derive(Default, FromDeriveInput)] pub struct AnimationInputAttrs { diff --git a/components/style_derive/compute_squared_distance.rs b/components/style_derive/compute_squared_distance.rs index 5e130e75b06..b4e67f6c984 100644 --- a/components/style_derive/compute_squared_distance.rs +++ b/components/style_derive/compute_squared_distance.rs @@ -6,12 +6,11 @@ use crate::animate::{AnimationFieldAttrs, AnimationInputAttrs, AnimationVariantA use derive_common::cg; use proc_macro2::TokenStream; use quote::TokenStreamExt; -use syn::{DeriveInput, Path, WhereClause}; +use syn::{DeriveInput, WhereClause}; use synstructure; pub fn derive(mut input: DeriveInput) -> TokenStream { let animation_input_attrs = cg::parse_input_attrs::(&input); - let input_attrs = cg::parse_input_attrs::(&input); let no_bound = animation_input_attrs.no_bound.unwrap_or_default(); let mut where_clause = input.generics.where_clause.take(); for param in input.generics.type_params() { @@ -43,11 +42,6 @@ pub fn derive(mut input: DeriveInput) -> TokenStream { match_body.append_all(quote! { _ => unsafe { debug_unreachable!() } }); } - let fallback = match input_attrs.fallback { - Some(fallback) => quote! { #fallback(self, other) }, - None => quote! { Err(()) }, - }; - let name = &input.ident; let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); @@ -60,7 +54,7 @@ pub fn derive(mut input: DeriveInput) -> TokenStream { other: &Self, ) -> Result { if std::mem::discriminant(self) != std::mem::discriminant(other) { - return #fallback; + return Err(()); } match (self, other) { #match_body @@ -124,12 +118,6 @@ fn derive_variant_arm( }; } -#[darling(attributes(distance), default)] -#[derive(Default, FromDeriveInput)] -struct DistanceInputAttrs { - fallback: Option, -} - #[darling(attributes(distance), default)] #[derive(Default, FromField)] struct DistanceFieldAttrs { From 5cedc45858786c90e38507b1459073b5d92b6c50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 12 Feb 2020 11:32:47 +0000 Subject: [PATCH 08/74] style: Make font-family serialization algorithm a bit more conservative. So as to avoid serializing as identifiers font-families with spaces as part of the identifier. This avoids serializing confusing escaped sequences if the beginning of the stuff after the space happens to not be a valid ident start. This is an slightly more restrictive version of the existing logic, which happens to also match other browsers in my testing. Differential Revision: https://phabricator.services.mozilla.com/D62376 --- components/style/values/computed/font.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 642dbc37eeb..6d17f9fa699 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -352,19 +352,22 @@ impl SingleFontFamily { }; let mut value = first_ident.as_ref().to_owned(); + let mut serialize_quoted = value.contains(' '); // These keywords are not allowed by themselves. // The only way this value can be valid with with another keyword. if reserved { let ident = input.expect_ident()?; + serialize_quoted = serialize_quoted || ident.contains(' '); value.push(' '); value.push_str(&ident); } while let Ok(ident) = input.try(|i| i.expect_ident_cloned()) { + serialize_quoted = serialize_quoted || ident.contains(' '); value.push(' '); value.push_str(&ident); } - let syntax = if value.starts_with(' ') || value.ends_with(' ') || value.contains(" ") { + let syntax = if serialize_quoted { // For font family names which contains special white spaces, e.g. // `font-family: \ a\ \ b\ \ c\ ;`, it is tricky to serialize them // as identifiers correctly. Just mark them quoted so we don't need From 0e3c1228904b4e7a463371b1c9c52862af5c5be4 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Wed, 12 Feb 2020 18:22:51 +0000 Subject: [PATCH 09/74] style: Make GradientItem and ColorStop support angular color stops. Differential Revision: https://phabricator.services.mozilla.com/D62544 --- components/style/values/computed/image.rs | 6 ----- components/style/values/computed/mod.rs | 2 +- components/style/values/generics/image.rs | 14 +++++----- components/style/values/specified/image.rs | 30 ++++++++++----------- components/style/values/specified/length.rs | 2 +- components/style/values/specified/mod.rs | 4 +-- 6 files changed, 26 insertions(+), 32 deletions(-) diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs index 8610888bef7..dc4bbe464a7 100644 --- a/components/style/values/computed/image.rs +++ b/components/style/values/computed/image.rs @@ -54,12 +54,6 @@ pub enum LineDirection { Corner(HorizontalPositionKeyword, VerticalPositionKeyword), } -/// A computed gradient item. -pub type GradientItem = generic::GenericGradientItem; - -/// A computed color stop. -pub type ColorStop = generic::ColorStop; - /// Computed values for `-moz-image-rect(...)`. #[cfg(feature = "gecko")] pub type MozImageRect = generic::GenericMozImageRect; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index c1c83f829a8..1566ab0f391 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -57,7 +57,7 @@ pub use self::font::{FontSize, FontSizeAdjust, FontStretch, FontSynthesis}; pub use self::font::{FontVariantAlternates, FontWeight}; pub use self::font::{FontVariantEastAsian, FontVariationSettings}; pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XLang, XTextZoom}; -pub use self::image::{Gradient, GradientItem, Image, LineDirection, MozImageRect}; +pub use self::image::{Gradient, Image, LineDirection, MozImageRect}; pub use self::length::{CSSPixelLength, ExtremumLength, NonNegativeLength}; pub use self::length::{Length, LengthOrNumber, LengthPercentage, NonNegativeLengthOrNumber}; pub use self::length::{LengthOrAuto, LengthPercentageOrAuto, MaxSize, Size}; diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index 4b79dcb9cb8..f6206a9fb25 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -180,7 +180,7 @@ pub enum ShapeExtent { Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem, )] #[repr(C, u8)] -pub enum GenericGradientItem { +pub enum GenericGradientItem { /// A simple color stop, without position. SimpleColorStop(Color), /// A complex color stop, with a position. @@ -188,10 +188,10 @@ pub enum GenericGradientItem { /// The color for the stop. color: Color, /// The position for the stop. - position: LengthPercentage, + position: T, }, /// An interpolation hint. - InterpolationHint(LengthPercentage), + InterpolationHint(T), } pub use self::GenericGradientItem as GradientItem; @@ -201,17 +201,17 @@ pub use self::GenericGradientItem as GradientItem; #[derive( Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem, )] -pub struct ColorStop { +pub struct ColorStop { /// The color of this stop. pub color: Color, /// The position of this stop. - pub position: Option, + pub position: Option, } -impl ColorStop { +impl ColorStop { /// Convert the color stop into an appropriate `GradientItem`. #[inline] - pub fn into_item(self) -> GradientItem { + pub fn into_item(self) -> GradientItem { match self.position { Some(position) => GradientItem::ComplexColorStop { color: self.color, diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index 369b1936f87..13e06d44c57 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -92,12 +92,6 @@ pub enum LineDirection { /// A specified ending shape. pub type EndingShape = generic::EndingShape; -/// A specified gradient item. -pub type GradientItem = generic::GradientItem; - -/// A computed color stop. -pub type ColorStop = generic::ColorStop; - /// Specified values for `moz-image-rect` /// -moz-image-rect(, top, right, bottom, left); #[cfg(all(feature = "gecko", not(feature = "cbindgen")))] @@ -256,7 +250,7 @@ impl Parse for Gradient { Shape::Linear => GradientKind::parse_linear(context, i, &mut compat_mode)?, Shape::Radial => GradientKind::parse_radial(context, i, &mut compat_mode)?, }; - let items = GradientItem::parse_comma_separated(context, i)?; + let items = generic::GradientItem::parse_comma_separated(context, i)?; Ok((shape, items)) })?; @@ -778,7 +772,10 @@ impl ShapeExtent { } } -impl GradientItem { +impl generic::GradientItem +where + T: Parse, +{ fn parse_comma_separated<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, @@ -789,20 +786,20 @@ impl GradientItem { loop { input.parse_until_before(Delimiter::Comma, |input| { if seen_stop { - if let Ok(hint) = input.try(|i| LengthPercentage::parse(context, i)) { + if let Ok(hint) = input.try(|i| T::parse(context, i)) { seen_stop = false; items.push(generic::GradientItem::InterpolationHint(hint)); return Ok(()); } } - let stop = ColorStop::parse(context, input)?; + let stop = generic::ColorStop::parse(context, input)?; - if let Ok(multi_position) = input.try(|i| LengthPercentage::parse(context, i)) { + if let Ok(multi_position) = input.try(|i| T::parse(context, i)) { let stop_color = stop.color.clone(); items.push(stop.into_item()); items.push( - ColorStop { + generic::ColorStop { color: stop_color, position: Some(multi_position), } @@ -830,14 +827,17 @@ impl GradientItem { } } -impl Parse for ColorStop { +impl Parse for generic::ColorStop +where + T: Parse, +{ fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - Ok(ColorStop { + Ok(generic::ColorStop { color: Color::parse(context, input)?, - position: input.try(|i| LengthPercentage::parse(context, i)).ok(), + position: input.try(|i| T::parse(context, i)).ok(), }) } } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index ff24404c09a..f7d7023a51a 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -28,7 +28,7 @@ use std::ops::{Add, Mul}; use style_traits::values::specified::AllowedNumericType; use style_traits::{ParseError, SpecifiedValueInfo, StyleParseErrorKind}; -pub use super::image::{ColorStop, EndingShape as GradientEndingShape, Gradient}; +pub use super::image::{EndingShape as GradientEndingShape, Gradient}; pub use super::image::{GradientKind, Image}; pub use crate::values::specified::calc::CalcLengthPercentage; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index d45643d6604..aa8bda5c227 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -55,8 +55,8 @@ pub use self::font::{FontSize, FontSizeAdjust, FontStretch, FontSynthesis}; pub use self::font::{FontVariantAlternates, FontWeight}; pub use self::font::{FontVariantEastAsian, FontVariationSettings}; pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XLang, XTextZoom}; -pub use self::image::{ColorStop, EndingShape as GradientEndingShape, Gradient}; -pub use self::image::{GradientItem, GradientKind, Image, MozImageRect}; +pub use self::image::{EndingShape as GradientEndingShape, Gradient}; +pub use self::image::{GradientKind, Image, MozImageRect}; pub use self::length::{AbsoluteLength, CalcLengthPercentage, CharacterWidth}; pub use self::length::{FontRelativeLength, Length, LengthOrNumber, NonNegativeLengthOrNumber}; pub use self::length::{LengthOrAuto, LengthPercentage, LengthPercentageOrAuto}; From cf20c627b5d5b5c51520991663fa071889c71abe Mon Sep 17 00:00:00 2001 From: Martin McNickle Date: Fri, 14 Feb 2020 15:48:01 +0000 Subject: [PATCH 10/74] style: Replace existing GridAutoFlow struct with a `bitflags!` backed one. Differential Revision: https://phabricator.services.mozilla.com/D62787 --- .../properties/longhands/position.mako.rs | 2 +- .../properties/shorthands/position.mako.rs | 32 ++-- components/style/values/specified/position.rs | 153 +++++++----------- 3 files changed, 75 insertions(+), 112 deletions(-) diff --git a/components/style/properties/longhands/position.mako.rs b/components/style/properties/longhands/position.mako.rs index 660da1a5a37..f43c2c56aa3 100644 --- a/components/style/properties/longhands/position.mako.rs +++ b/components/style/properties/longhands/position.mako.rs @@ -375,7 +375,7 @@ ${helpers.predefined_type( ${helpers.predefined_type( "grid-auto-flow", "GridAutoFlow", - "computed::GridAutoFlow::row()", + "computed::GridAutoFlow::ROW", engines="gecko", animation_value_type="discrete", spec="https://drafts.csswg.org/css-grid/#propdef-grid-auto-flow", diff --git a/components/style/properties/shorthands/position.mako.rs b/components/style/properties/shorthands/position.mako.rs index d9cb9da3342..48e56e6ef3c 100644 --- a/components/style/properties/shorthands/position.mako.rs +++ b/components/style/properties/shorthands/position.mako.rs @@ -549,7 +549,7 @@ use crate::properties::longhands::{grid_auto_columns, grid_auto_rows, grid_auto_flow}; use crate::values::generics::grid::GridTemplateComponent; use crate::values::specified::{GenericGridTemplateComponent, ImplicitGridTracks}; - use crate::values::specified::position::{AutoFlow, GridAutoFlow, GridTemplateAreas}; + use crate::values::specified::position::{GridAutoFlow, GridTemplateAreas}; pub fn parse_value<'i, 't>( context: &ParserContext, @@ -566,28 +566,28 @@ input: &mut Parser<'i, 't>, is_row: bool, ) -> Result> { - let mut auto_flow = None; - let mut dense = false; + let mut track = None; + let mut dense = GridAutoFlow::empty(); + for _ in 0..2 { if input.try(|i| i.expect_ident_matching("auto-flow")).is_ok() { - auto_flow = if is_row { - Some(AutoFlow::Row) + track = if is_row { + Some(GridAutoFlow::ROW) } else { - Some(AutoFlow::Column) + Some(GridAutoFlow::COLUMN) }; } else if input.try(|i| i.expect_ident_matching("dense")).is_ok() { - dense = true; + dense = GridAutoFlow::DENSE } else { break } } - auto_flow.map(|flow| { - GridAutoFlow { - autoflow: flow, - dense: dense, - } - }).ok_or(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + if track.is_some() { + Ok(track.unwrap() | dense) + } else { + Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + } } if let Ok((rows, cols, areas)) = input.try(|i| super::grid_template::parse_grid_template(context, i)) { @@ -637,7 +637,7 @@ self.grid_template_areas, dest); } - if self.grid_auto_flow.autoflow == AutoFlow::Column { + if self.grid_auto_flow.contains(GridAutoFlow::COLUMN) { // It should fail to serialize if other branch of the if condition's values are set. if !self.grid_auto_rows.is_initial() || !self.grid_template_columns.is_initial() { @@ -653,7 +653,7 @@ self.grid_template_rows.to_css(dest)?; dest.write_str(" / auto-flow")?; - if self.grid_auto_flow.dense { + if self.grid_auto_flow.contains(GridAutoFlow::DENSE) { dest.write_str(" dense")?; } @@ -676,7 +676,7 @@ } dest.write_str("auto-flow")?; - if self.grid_auto_flow.dense { + if self.grid_auto_flow.contains(GridAutoFlow::DENSE) { dest.write_str(" dense")?; } diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index 3f01988e49d..ff8cfade968 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -7,6 +7,7 @@ //! //! [position]: https://drafts.csswg.org/css-backgrounds-3/#position +use crate::gecko_bindings::structs; use crate::parser::{Parse, ParserContext}; use crate::selector_map::PrecomputedHashMap; use crate::str::HTML_SPACE_CHARACTERS; @@ -350,66 +351,24 @@ impl Side for VerticalPositionKeyword { } } -#[derive( - Clone, - Copy, - Debug, - Eq, - MallocSizeOf, - PartialEq, - SpecifiedValueInfo, - ToComputedValue, - ToCss, - ToResolvedValue, - ToShmem, -)] -/// Auto-placement algorithm Option -pub enum AutoFlow { - /// The auto-placement algorithm places items by filling each row in turn, - /// adding new rows as necessary. - Row, - /// The auto-placement algorithm places items by filling each column in turn, - /// adding new columns as necessary. - Column, -} - -/// If `dense` is specified, `row` is implied. -fn is_row_dense(autoflow: &AutoFlow, dense: &bool) -> bool { - *autoflow == AutoFlow::Row && *dense -} - -#[derive( - Clone, - Copy, - Debug, - Eq, - MallocSizeOf, - PartialEq, - SpecifiedValueInfo, - ToComputedValue, - ToCss, - ToResolvedValue, - ToShmem, -)] -/// Controls how the auto-placement algorithm works -/// specifying exactly how auto-placed items get flowed into the grid -pub struct GridAutoFlow { - /// Specifiy how auto-placement algorithm fills each `row` or `column` in turn - #[css(contextual_skip_if = "is_row_dense")] - pub autoflow: AutoFlow, - /// Specify use `dense` packing algorithm or not - #[css(represents_keyword)] - pub dense: bool, -} - -impl GridAutoFlow { - #[inline] - /// Get default `grid-auto-flow` as `row` - pub fn row() -> GridAutoFlow { - GridAutoFlow { - autoflow: AutoFlow::Row, - dense: false, - } +bitflags! { + /// Controls how the auto-placement algorithm works + /// specifying exactly how auto-placed items get flowed into the grid + #[derive( + MallocSizeOf, + SpecifiedValueInfo, + ToComputedValue, + ToResolvedValue, + ToShmem + )] + #[value_info(other_values = "row,column,dense")] + pub struct GridAutoFlow: u8 { + /// 'row' - mutually exclusive with 'column' + const ROW = structs::NS_STYLE_GRID_AUTO_FLOW_ROW as u8; + /// 'column' - mutually exclusive with 'row' + const COLUMN = structs::NS_STYLE_GRID_AUTO_FLOW_COLUMN as u8; + /// 'dense' + const DENSE = structs::NS_STYLE_GRID_AUTO_FLOW_DENSE as u8; } } @@ -419,26 +378,26 @@ impl Parse for GridAutoFlow { _context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let mut value = None; - let mut dense = false; + let mut track = None; + let mut dense = GridAutoFlow::empty(); while !input.is_exhausted() { let location = input.current_source_location(); let ident = input.expect_ident()?; let success = match_ignore_ascii_case! { &ident, - "row" if value.is_none() => { - value = Some(AutoFlow::Row); + "row" if track.is_none() => { + track = Some(GridAutoFlow::ROW); true }, - "column" if value.is_none() => { - value = Some(AutoFlow::Column); + "column" if track.is_none() => { + track = Some(GridAutoFlow::COLUMN); true }, - "dense" if !dense => { - dense = true; + "dense" if dense.is_empty() => { + dense = GridAutoFlow::DENSE; true }, - _ => false + _ => false, }; if !success { return Err(location @@ -446,47 +405,51 @@ impl Parse for GridAutoFlow { } } - if value.is_some() || dense { - Ok(GridAutoFlow { - autoflow: value.unwrap_or(AutoFlow::Row), - dense: dense, - }) + if track.is_some() || !dense.is_empty() { + Ok(track.unwrap_or(GridAutoFlow::ROW) | dense) } else { Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } } } +impl ToCss for GridAutoFlow { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + if *self == GridAutoFlow::ROW { + return dest.write_str("row"); + } + + if *self == GridAutoFlow::COLUMN { + return dest.write_str("column"); + } + + if *self == GridAutoFlow::ROW | GridAutoFlow::DENSE { + return dest.write_str("dense"); + } + + if *self == GridAutoFlow::COLUMN | GridAutoFlow::DENSE { + return dest.write_str("column dense"); + } + + debug_assert!(false, "Unknown or invalid grid-autoflow value"); + Ok(()) + } +} + #[cfg(feature = "gecko")] impl From for GridAutoFlow { fn from(bits: u8) -> GridAutoFlow { - use crate::gecko_bindings::structs; - - GridAutoFlow { - autoflow: if bits & structs::NS_STYLE_GRID_AUTO_FLOW_ROW as u8 != 0 { - AutoFlow::Row - } else { - AutoFlow::Column - }, - dense: bits & structs::NS_STYLE_GRID_AUTO_FLOW_DENSE as u8 != 0, - } + GridAutoFlow::from_bits(bits).unwrap() } } #[cfg(feature = "gecko")] impl From for u8 { fn from(v: GridAutoFlow) -> u8 { - use crate::gecko_bindings::structs; - - let mut result: u8 = match v.autoflow { - AutoFlow::Row => structs::NS_STYLE_GRID_AUTO_FLOW_ROW as u8, - AutoFlow::Column => structs::NS_STYLE_GRID_AUTO_FLOW_COLUMN as u8, - }; - - if v.dense { - result |= structs::NS_STYLE_GRID_AUTO_FLOW_DENSE as u8; - } - result + v.bits } } From 28d8565f385c29df84f5162a507c9498b750b44f Mon Sep 17 00:00:00 2001 From: Martin McNickle Date: Fri, 14 Feb 2020 15:48:10 +0000 Subject: [PATCH 11/74] style: Use the cbindgen output for GridAutoFlow directly in gecko. Depends on D62787 Differential Revision: https://phabricator.services.mozilla.com/D62910 --- components/style/values/specified/position.rs | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index ff8cfade968..a3463315ef7 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -7,7 +7,6 @@ //! //! [position]: https://drafts.csswg.org/css-backgrounds-3/#position -use crate::gecko_bindings::structs; use crate::parser::{Parse, ParserContext}; use crate::selector_map::PrecomputedHashMap; use crate::str::HTML_SPACE_CHARACTERS; @@ -362,13 +361,14 @@ bitflags! { ToShmem )] #[value_info(other_values = "row,column,dense")] + #[repr(C)] pub struct GridAutoFlow: u8 { /// 'row' - mutually exclusive with 'column' - const ROW = structs::NS_STYLE_GRID_AUTO_FLOW_ROW as u8; + const ROW = 1 << 0; /// 'column' - mutually exclusive with 'row' - const COLUMN = structs::NS_STYLE_GRID_AUTO_FLOW_COLUMN as u8; + const COLUMN = 1 << 1; /// 'dense' - const DENSE = structs::NS_STYLE_GRID_AUTO_FLOW_DENSE as u8; + const DENSE = 1 << 2; } } @@ -439,20 +439,6 @@ impl ToCss for GridAutoFlow { } } -#[cfg(feature = "gecko")] -impl From for GridAutoFlow { - fn from(bits: u8) -> GridAutoFlow { - GridAutoFlow::from_bits(bits).unwrap() - } -} - -#[cfg(feature = "gecko")] -impl From for u8 { - fn from(v: GridAutoFlow) -> u8 { - v.bits - } -} - #[derive( Clone, Debug, From fc88e908d5374e8507ab5492aae798590737fd04 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Fri, 14 Feb 2020 16:32:38 +0000 Subject: [PATCH 12/74] style: Re-introduce display: -moz-stack; as . Differential Revision: https://phabricator.services.mozilla.com/D62758 --- components/style/values/specified/box.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index fa5d5a2d43a..dbdaf2048a2 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -104,6 +104,8 @@ pub enum DisplayInside { #[cfg(feature = "gecko")] MozGridLine, #[cfg(feature = "gecko")] + MozStack, + #[cfg(feature = "gecko")] MozDeck, #[cfg(feature = "gecko")] MozPopup, @@ -227,6 +229,8 @@ impl Display { #[cfg(feature = "gecko")] pub const MozGridLine: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozGridLine); #[cfg(feature = "gecko")] + pub const MozStack: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozStack); + #[cfg(feature = "gecko")] pub const MozDeck: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozDeck); #[cfg(feature = "gecko")] pub const MozPopup: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozPopup); @@ -616,6 +620,8 @@ impl Parse for Display { #[cfg(feature = "gecko")] "-moz-grid-line" if moz_display_values_enabled(context) => Display::MozGridLine, #[cfg(feature = "gecko")] + "-moz-stack" if moz_display_values_enabled(context) => Display::MozStack, + #[cfg(feature = "gecko")] "-moz-deck" if moz_display_values_enabled(context) => Display::MozDeck, #[cfg(feature = "gecko")] "-moz-popup" if moz_display_values_enabled(context) => Display::MozPopup, From 25b265a10f3490aa3c4da29679654e533bc25a0a Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Sat, 15 Feb 2020 22:04:35 +0000 Subject: [PATCH 13/74] style: Refactor GenericGradient for conic-gradient support. Differential Revision: https://phabricator.services.mozilla.com/D62923 --- components/style/values/generics/image.rs | 105 ++++++++---------- components/style/values/specified/image.rs | 116 ++++++++++++-------- components/style/values/specified/length.rs | 2 +- components/style/values/specified/mod.rs | 2 +- 4 files changed, 120 insertions(+), 105 deletions(-) diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index f6206a9fb25..56e3d183d83 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -51,7 +51,7 @@ pub use self::GenericImage as Image; /// #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)] #[repr(C)] -pub struct GenericGradient< +pub enum GenericGradient< LineDirection, LengthPercentage, NonNegativeLength, @@ -59,19 +59,30 @@ pub struct GenericGradient< Position, Color, > { - /// Gradients can be linear or radial. - pub kind: GenericGradientKind< - LineDirection, - NonNegativeLength, - NonNegativeLengthPercentage, - Position, - >, - /// The color stops and interpolation hints. - pub items: crate::OwnedSlice>, - /// True if this is a repeating gradient. - pub repeating: bool, - /// Compatibility mode. - pub compat_mode: GradientCompatMode, + /// A linear gradient. + Linear { + /// Line direction + direction: LineDirection, + /// The color stops and interpolation hints. + items: crate::OwnedSlice>, + /// True if this is a repeating gradient. + repeating: bool, + /// Compatibility mode. + compat_mode: GradientCompatMode, + }, + /// A radial gradient. + Radial { + /// Shape of gradient + shape: GenericEndingShape, + /// Center of gradient + position: Position, + /// The color stops and interpolation hints. + items: crate::OwnedSlice>, + /// True if this is a repeating gradient. + repeating: bool, + /// Compatibility mode. + compat_mode: GradientCompatMode, + }, } pub use self::GenericGradient as Gradient; @@ -88,26 +99,6 @@ pub enum GradientCompatMode { Moz, } -/// A gradient kind. -#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)] -#[repr(C, u8)] -pub enum GenericGradientKind< - LineDirection, - NonNegativeLength, - NonNegativeLengthPercentage, - Position, -> { - /// A linear gradient. - Linear(LineDirection), - /// A radial gradient. - Radial( - GenericEndingShape, - Position, - ), -} - -pub use self::GenericGradientKind as GradientKind; - /// A radial gradient's ending shape. #[derive( Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem, @@ -330,32 +321,39 @@ where where W: Write, { - match self.compat_mode { + let (compat_mode, repeating) = match *self { + Gradient::Linear { compat_mode, repeating, .. } => (compat_mode, repeating), + Gradient::Radial { compat_mode, repeating, .. } => (compat_mode, repeating), + }; + + match compat_mode { GradientCompatMode::WebKit => dest.write_str("-webkit-")?, GradientCompatMode::Moz => dest.write_str("-moz-")?, _ => {}, } - if self.repeating { + if repeating { dest.write_str("repeating-")?; } - dest.write_str(self.kind.label())?; - dest.write_str("-gradient(")?; - let mut skip_comma = match self.kind { - GradientKind::Linear(ref direction) if direction.points_downwards(self.compat_mode) => { - true + + let (items, mut skip_comma) = match *self { + Gradient::Linear { ref direction, compat_mode, ref items, .. } => { + dest.write_str("linear-gradient(")?; + if !direction.points_downwards(compat_mode) { + direction.to_css(dest, compat_mode)?; + (items, false) + } else { + (items, true) + } }, - GradientKind::Linear(ref direction) => { - direction.to_css(dest, self.compat_mode)?; - false - }, - GradientKind::Radial(ref shape, ref position) => { + Gradient::Radial { ref shape, ref position, compat_mode, ref items, .. } => { + dest.write_str("radial-gradient(")?; let omit_shape = match *shape { EndingShape::Ellipse(Ellipse::Extent(ShapeExtent::Cover)) | EndingShape::Ellipse(Ellipse::Extent(ShapeExtent::FarthestCorner)) => true, _ => false, }; - if self.compat_mode == GradientCompatMode::Modern { + if compat_mode == GradientCompatMode::Modern { if !omit_shape { shape.to_css(dest)?; dest.write_str(" ")?; @@ -369,10 +367,10 @@ where shape.to_css(dest)?; } } - false + (items, false) }, }; - for item in &*self.items { + for item in &**items { if !skip_comma { dest.write_str(", ")?; } @@ -383,15 +381,6 @@ where } } -impl GradientKind { - fn label(&self) -> &str { - match *self { - GradientKind::Linear(..) => "linear", - GradientKind::Radial(..) => "radial", - } - } -} - /// The direction of a linear gradient. pub trait LineDirection { /// Whether this direction points towards, and thus can be omitted. diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index 13e06d44c57..fdc3a795db0 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -70,10 +70,6 @@ impl SpecifiedValueInfo for Gradient { } } -/// A specified gradient kind. -pub type GradientKind = - generic::GradientKind; - /// A specified gradient line direction. /// /// FIXME(emilio): This should be generic over Angle. @@ -245,25 +241,12 @@ impl Parse for Gradient { } }; - let (kind, items) = input.parse_nested_block(|i| { - let shape = match shape { - Shape::Linear => GradientKind::parse_linear(context, i, &mut compat_mode)?, - Shape::Radial => GradientKind::parse_radial(context, i, &mut compat_mode)?, - }; - let items = generic::GradientItem::parse_comma_separated(context, i)?; - Ok((shape, items)) - })?; - - if items.len() < 2 { - return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); - } - - Ok(Gradient { - items, - repeating, - kind, - compat_mode, - }) + Ok(input.parse_nested_block(|i| { + Ok(match shape { + Shape::Linear => Self::parse_linear(context, i, repeating, &mut compat_mode)?, + Shape::Radial => Self::parse_radial(context, i, repeating, compat_mode)?, + }) + })?) } } @@ -383,16 +366,21 @@ impl Gradient { let ident = input.expect_ident_cloned()?; input.expect_comma()?; - let (kind, reverse_stops) = match_ignore_ascii_case! { &ident, + Ok(match_ignore_ascii_case! { &ident, "linear" => { let first = Point::parse(context, input)?; input.expect_comma()?; let second = Point::parse(context, input)?; let direction = LineDirection::from_points(first, second); - let kind = generic::GradientKind::Linear(direction); + let items = Gradient::parse_webkit_gradient_stops(context, input, false)?; - (kind, false) + generic::Gradient::Linear { + direction, + items, + repeating: false, + compat_mode: GradientCompatMode::Modern, + } }, "radial" => { let first_point = Point::parse(context, input)?; @@ -412,16 +400,28 @@ impl Gradient { let rad = Circle::Radius(NonNegative(Length::from_px(radius.value))); let shape = generic::EndingShape::Circle(rad); let position: Position = point.into(); + let items = Gradient::parse_webkit_gradient_stops(context, input, reverse_stops)?; - let kind = generic::GradientKind::Radial(shape, position); - (kind, reverse_stops) + generic::Gradient::Radial { + shape, + position, + items, + repeating: false, + compat_mode: GradientCompatMode::Modern, + } }, _ => { let e = SelectorParseErrorKind::UnexpectedIdent(ident.clone()); return Err(input.new_custom_error(e)); }, - }; + }) + } + fn parse_webkit_gradient_stops<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + reverse_stops: bool, + ) -> Result>, ParseError<'i>> { let mut items = input .try(|i| { i.expect_comma()?; @@ -500,22 +500,29 @@ impl Gradient { } }) } - - Ok(generic::Gradient { - kind, - items: items.into(), - repeating: false, - compat_mode: GradientCompatMode::Modern, - }) + Ok(items.into()) + } + + /// Not used for -webkit-gradient syntax. + fn parse_stops<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result>, ParseError<'i>> { + let items = generic::GradientItem::parse_comma_separated(context, input)?; + + if items.len() < 2 { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); + } + + Ok(items) } -} -impl GradientKind { /// Parses a linear gradient. /// GradientCompatMode can change during `-moz-` prefixed gradient parsing if it come across a `to` keyword. fn parse_linear<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + repeating: bool, compat_mode: &mut GradientCompatMode, ) -> Result> { let direction = if let Ok(d) = input.try(|i| LineDirection::parse(context, i, compat_mode)) @@ -523,23 +530,33 @@ impl GradientKind { input.expect_comma()?; d } else { - match *compat_mode { + match compat_mode { GradientCompatMode::Modern => { LineDirection::Vertical(VerticalPositionKeyword::Bottom) }, _ => LineDirection::Vertical(VerticalPositionKeyword::Top), } }; - Ok(generic::GradientKind::Linear(direction)) + let items = Gradient::parse_stops(context, input)?; + + Ok(Gradient::Linear { + direction, + items, + repeating, + compat_mode: *compat_mode, + }) } + + /// Parses a radial gradient. fn parse_radial<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, - compat_mode: &mut GradientCompatMode, + repeating: bool, + compat_mode: GradientCompatMode, ) -> Result> { - let (shape, position) = match *compat_mode { + let (shape, position) = match compat_mode { GradientCompatMode::Modern => { - let shape = input.try(|i| EndingShape::parse(context, i, *compat_mode)); + let shape = input.try(|i| EndingShape::parse(context, i, compat_mode)); let position = input.try(|i| { i.expect_ident_matching("at")?; Position::parse(context, i) @@ -552,7 +569,7 @@ impl GradientKind { if position.is_ok() { i.expect_comma()?; } - EndingShape::parse(context, i, *compat_mode) + EndingShape::parse(context, i, compat_mode) }); (shape, position.ok()) }, @@ -567,7 +584,16 @@ impl GradientKind { }); let position = position.unwrap_or(Position::center()); - Ok(generic::GradientKind::Radial(shape, position)) + + let items = Gradient::parse_stops(context, input)?; + + Ok(Gradient::Radial { + shape, + position, + items, + repeating, + compat_mode, + }) } } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index f7d7023a51a..16b9bf003e6 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -29,7 +29,7 @@ use style_traits::values::specified::AllowedNumericType; use style_traits::{ParseError, SpecifiedValueInfo, StyleParseErrorKind}; pub use super::image::{EndingShape as GradientEndingShape, Gradient}; -pub use super::image::{GradientKind, Image}; +pub use super::image::Image; pub use crate::values::specified::calc::CalcLengthPercentage; /// Number of app units per pixel diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index aa8bda5c227..84e2b747234 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -56,7 +56,7 @@ pub use self::font::{FontVariantAlternates, FontWeight}; pub use self::font::{FontVariantEastAsian, FontVariationSettings}; pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XLang, XTextZoom}; pub use self::image::{EndingShape as GradientEndingShape, Gradient}; -pub use self::image::{GradientKind, Image, MozImageRect}; +pub use self::image::{Image, MozImageRect}; pub use self::length::{AbsoluteLength, CalcLengthPercentage, CharacterWidth}; pub use self::length::{FontRelativeLength, Length, LengthOrNumber, NonNegativeLengthOrNumber}; pub use self::length::{LengthOrAuto, LengthPercentage, LengthPercentageOrAuto}; From f25e0b77e9f2a4ee61bf07b9f92c356279eff1c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 16 Feb 2020 11:00:45 +0000 Subject: [PATCH 14/74] style: Minor gradient parsing cleanup. I suggested the compat_mode bit in D62923 but it was somehow only applied to one of the branches. Also rustfmt the code for consistency, and add a local alias. Differential Revision: https://phabricator.services.mozilla.com/D63015 --- components/style/values/specified/image.rs | 40 +++++++++++----------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index fdc3a795db0..fcd6210c6b9 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -47,6 +47,8 @@ pub type Gradient = generic::Gradient< Color, >; +type LengthPercentageItemList = crate::OwnedSlice>; + impl SpecifiedValueInfo for Gradient { const SUPPORTED_TYPES: u8 = CssType::GRADIENT; @@ -189,7 +191,7 @@ impl Parse for Gradient { } let func = input.expect_function()?; - let (shape, repeating, mut compat_mode) = match_ignore_ascii_case! { &func, + let (shape, repeating, compat_mode) = match_ignore_ascii_case! { &func, "linear-gradient" => { (Shape::Linear, false, GradientCompatMode::Modern) }, @@ -243,7 +245,7 @@ impl Parse for Gradient { Ok(input.parse_nested_block(|i| { Ok(match shape { - Shape::Linear => Self::parse_linear(context, i, repeating, &mut compat_mode)?, + Shape::Linear => Self::parse_linear(context, i, repeating, compat_mode)?, Shape::Radial => Self::parse_radial(context, i, repeating, compat_mode)?, }) })?) @@ -421,7 +423,7 @@ impl Gradient { context: &ParserContext, input: &mut Parser<'i, 't>, reverse_stops: bool, - ) -> Result>, ParseError<'i>> { + ) -> Result> { let mut items = input .try(|i| { i.expect_comma()?; @@ -507,13 +509,11 @@ impl Gradient { fn parse_stops<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, - ) -> Result>, ParseError<'i>> { + ) -> Result> { let items = generic::GradientItem::parse_comma_separated(context, input)?; - if items.len() < 2 { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } - Ok(items) } @@ -523,27 +523,27 @@ impl Gradient { context: &ParserContext, input: &mut Parser<'i, 't>, repeating: bool, - compat_mode: &mut GradientCompatMode, + mut compat_mode: GradientCompatMode, ) -> Result> { - let direction = if let Ok(d) = input.try(|i| LineDirection::parse(context, i, compat_mode)) - { - input.expect_comma()?; - d - } else { - match compat_mode { - GradientCompatMode::Modern => { - LineDirection::Vertical(VerticalPositionKeyword::Bottom) - }, - _ => LineDirection::Vertical(VerticalPositionKeyword::Top), - } - }; + let direction = + if let Ok(d) = input.try(|i| LineDirection::parse(context, i, &mut compat_mode)) { + input.expect_comma()?; + d + } else { + match compat_mode { + GradientCompatMode::Modern => { + LineDirection::Vertical(VerticalPositionKeyword::Bottom) + }, + _ => LineDirection::Vertical(VerticalPositionKeyword::Top), + } + }; let items = Gradient::parse_stops(context, input)?; Ok(Gradient::Linear { direction, items, repeating, - compat_mode: *compat_mode, + compat_mode, }) } From 47db01f7937f3334c152e64e45452d7c7c04a050 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Fri, 14 Feb 2020 11:11:27 +0000 Subject: [PATCH 15/74] style: Change initial value of image-orientation to from-image. Differential Revision: https://phabricator.services.mozilla.com/D62826 --- components/style/properties/longhands/inherited_box.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/longhands/inherited_box.mako.rs b/components/style/properties/longhands/inherited_box.mako.rs index e8f3a0a45d9..d2b66dc6a1f 100644 --- a/components/style/properties/longhands/inherited_box.mako.rs +++ b/components/style/properties/longhands/inherited_box.mako.rs @@ -85,7 +85,7 @@ ${helpers.single_keyword( ${helpers.single_keyword( "image-orientation", - "none from-image", + "from-image none", engines="gecko", gecko_enum_prefix="StyleImageOrientation", animation_value_type="discrete", From 31187b0180b3816a000c567f1ce3a12c4c9be222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 19 Feb 2020 14:48:43 +0000 Subject: [PATCH 16/74] style: Remove layout.css.webkit-appearance.enabled. I don't think we want to keep the ugly widget hacks forever. Let me know if you'd rather keep the property behind a pref but I don't think there's a point in doing that. Differential Revision: https://phabricator.services.mozilla.com/D62649 --- components/style/properties/longhands/box.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index 98d5ad296c9..3a8690e8604 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -633,7 +633,7 @@ ${helpers.predefined_type( "Appearance", "computed::Appearance::None", engines="gecko", - alias="-webkit-appearance:layout.css.webkit-appearance.enabled", + alias="-webkit-appearance", spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-appearance)", animation_value_type="discrete", gecko_ffi_name="mAppearance", From 61712d1a039d481ed073909c5785c9d7aec56809 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Wed, 19 Feb 2020 17:43:04 +0000 Subject: [PATCH 17/74] style: Implement parsing for CSS conic-gradient syntax. Differential Revision: https://phabricator.services.mozilla.com/D62148 --- components/style/values/computed/image.rs | 4 +- components/style/values/generics/image.rs | 64 +++++++++++++----- components/style/values/specified/image.rs | 77 +++++++++++++++++----- 3 files changed, 114 insertions(+), 31 deletions(-) diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs index dc4bbe464a7..4e16c2311d8 100644 --- a/components/style/values/computed/image.rs +++ b/components/style/values/computed/image.rs @@ -13,7 +13,7 @@ use crate::values::computed::url::ComputedImageUrl; use crate::values::computed::NumberOrPercentage; use crate::values::computed::{Angle, Color, Context}; use crate::values::computed::{ - LengthPercentage, NonNegativeLength, NonNegativeLengthPercentage, ToComputedValue, + AngleOrPercentage, LengthPercentage, NonNegativeLength, NonNegativeLengthPercentage, ToComputedValue, }; use crate::values::generics::image::{self as generic, GradientCompatMode}; use crate::values::specified::image::LineDirection as SpecifiedLineDirection; @@ -34,6 +34,8 @@ pub type Gradient = generic::GenericGradient< NonNegativeLength, NonNegativeLengthPercentage, Position, + Angle, + AngleOrPercentage, Color, >; diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index 56e3d183d83..dc64063ba08 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -9,6 +9,7 @@ use crate::custom_properties; use crate::values::serialize_atom_identifier; use crate::Atom; +use crate::Zero; use servo_arc::Arc; use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; @@ -57,6 +58,8 @@ pub enum GenericGradient< NonNegativeLength, NonNegativeLengthPercentage, Position, + Angle, + AngleOrPercentage, Color, > { /// A linear gradient. @@ -83,6 +86,17 @@ pub enum GenericGradient< /// Compatibility mode. compat_mode: GradientCompatMode, }, + /// A conic gradient. + Conic { + /// Start angle of gradient + angle: Angle, + /// Center of gradient + position: Position, + /// The color stops and interpolation hints. + items: crate::OwnedSlice>, + /// True if this is a repeating gradient. + repeating: bool, + }, } pub use self::GenericGradient as Gradient; @@ -308,13 +322,15 @@ where } } -impl ToCss for Gradient +impl ToCss for Gradient where D: LineDirection, LP: ToCss, NL: ToCss, NLP: ToCss, P: ToCss, + A: ToCss, + AoP: ToCss, C: ToCss, { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result @@ -324,6 +340,7 @@ where let (compat_mode, repeating) = match *self { Gradient::Linear { compat_mode, repeating, .. } => (compat_mode, repeating), Gradient::Radial { compat_mode, repeating, .. } => (compat_mode, repeating), + Gradient::Conic { repeating, .. } => (GradientCompatMode::Modern, repeating), }; match compat_mode { @@ -336,17 +353,24 @@ where dest.write_str("repeating-")?; } - let (items, mut skip_comma) = match *self { - Gradient::Linear { ref direction, compat_mode, ref items, .. } => { + match *self { + Gradient::Linear { ref direction, ref items, compat_mode, .. } => { dest.write_str("linear-gradient(")?; - if !direction.points_downwards(compat_mode) { + let mut skip_comma = if !direction.points_downwards(compat_mode) { direction.to_css(dest, compat_mode)?; - (items, false) + false } else { - (items, true) + true + }; + for item in &**items { + if !skip_comma { + dest.write_str(", ")?; + } + skip_comma = false; + item.to_css(dest)?; } }, - Gradient::Radial { ref shape, ref position, compat_mode, ref items, .. } => { + Gradient::Radial { ref shape, ref position, ref items, compat_mode, .. } => { dest.write_str("radial-gradient(")?; let omit_shape = match *shape { EndingShape::Ellipse(Ellipse::Extent(ShapeExtent::Cover)) | @@ -367,15 +391,25 @@ where shape.to_css(dest)?; } } - (items, false) + for item in &**items { + dest.write_str(", ")?; + item.to_css(dest)?; + } + }, + Gradient::Conic { ref angle, ref position, ref items, .. } => { + dest.write_str("conic-gradient(")?; + if !angle.is_zero() { + dest.write_str("from ")?; + angle.to_css(dest)?; + dest.write_str(" ")?; + } + dest.write_str("at ")?; + position.to_css(dest)?; + for item in &**items { + dest.write_str(", ")?; + item.to_css(dest)?; + } }, - }; - for item in &**items { - if !skip_comma { - dest.write_str(", ")?; - } - skip_comma = false; - item.to_css(dest)?; } dest.write_str(")") } diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index fcd6210c6b9..7536fc19fdd 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -19,7 +19,7 @@ use crate::values::specified::position::{HorizontalPositionKeyword, VerticalPosi use crate::values::specified::position::{Position, PositionComponent, Side}; use crate::values::specified::url::SpecifiedImageUrl; use crate::values::specified::{ - Angle, Color, Length, LengthPercentage, NonNegativeLength, NonNegativeLengthPercentage, + Angle, AngleOrPercentage, Color, Length, LengthPercentage, NonNegativeLength, NonNegativeLengthPercentage, }; use crate::values::specified::{Number, NumberOrPercentage, Percentage}; use crate::Atom; @@ -44,6 +44,8 @@ pub type Gradient = generic::Gradient< NonNegativeLength, NonNegativeLengthPercentage, Position, + Angle, + AngleOrPercentage, Color, >; @@ -69,6 +71,13 @@ impl SpecifiedValueInfo for Gradient { "-moz-repeating-radial-gradient", "-webkit-gradient", ]); + + if static_prefs::pref!("layout.css.conic-gradient.enabled") { + f(&[ + "conic-gradient", + "repeating-conic-gradient", + ]); + } } } @@ -188,6 +197,7 @@ impl Parse for Gradient { enum Shape { Linear, Radial, + Conic, } let func = input.expect_function()?; @@ -232,6 +242,12 @@ impl Parse for Gradient { "-moz-repeating-radial-gradient" => { (Shape::Radial, true, GradientCompatMode::Moz) }, + "conic-gradient" if static_prefs::pref!("layout.css.conic-gradient.enabled") => { + (Shape::Conic, false, GradientCompatMode::Modern) + }, + "repeating-conic-gradient" if static_prefs::pref!("layout.css.conic-gradient.enabled") => { + (Shape::Conic, true, GradientCompatMode::Modern) + }, "-webkit-gradient" => { return input.parse_nested_block(|i| { Self::parse_webkit_gradient_argument(context, i) @@ -247,6 +263,7 @@ impl Parse for Gradient { Ok(match shape { Shape::Linear => Self::parse_linear(context, i, repeating, compat_mode)?, Shape::Radial => Self::parse_radial(context, i, repeating, compat_mode)?, + Shape::Conic => Self::parse_conic(context, i, repeating)?, }) })?) } @@ -505,12 +522,12 @@ impl Gradient { Ok(items.into()) } - /// Not used for -webkit-gradient syntax. + /// Not used for -webkit-gradient syntax and conic-gradient fn parse_stops<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let items = generic::GradientItem::parse_comma_separated(context, input)?; + let items = generic::GradientItem::parse_comma_separated(context, input, LengthPercentage::parse)?; if items.len() < 2 { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } @@ -595,6 +612,40 @@ impl Gradient { compat_mode, }) } + fn parse_conic<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + repeating: bool, + ) -> Result> { + let angle = input.try(|i| { + i.expect_ident_matching("from")?; + // Spec allows unitless zero start angles + // https://drafts.csswg.org/css-images-4/#valdef-conic-gradient-angle + Angle::parse_with_unitless(context, i) + }); + let position = input.try(|i| { + i.expect_ident_matching("at")?; + Position::parse(context, i) + }); + if angle.is_ok() || position.is_ok() { + input.expect_comma()?; + } + + let angle = angle.unwrap_or(Angle::zero()); + let position = position.unwrap_or(Position::center()); + let items = generic::GradientItem::parse_comma_separated(context, input, AngleOrPercentage::parse_with_unitless)?; + + if items.len() < 2 { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); + } + + Ok(Gradient::Conic { + angle, + position, + items, + repeating, + }) + } } impl generic::LineDirection for LineDirection { @@ -798,13 +849,11 @@ impl ShapeExtent { } } -impl generic::GradientItem -where - T: Parse, -{ +impl generic::GradientItem { fn parse_comma_separated<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + parse_position: impl for<'i1, 't1> Fn(&ParserContext, &mut Parser<'i1, 't1>) -> Result> + Copy, ) -> Result, ParseError<'i>> { let mut items = Vec::new(); let mut seen_stop = false; @@ -812,16 +861,16 @@ where loop { input.parse_until_before(Delimiter::Comma, |input| { if seen_stop { - if let Ok(hint) = input.try(|i| T::parse(context, i)) { + if let Ok(hint) = input.try(|i| parse_position(context, i)) { seen_stop = false; items.push(generic::GradientItem::InterpolationHint(hint)); return Ok(()); } } - let stop = generic::ColorStop::parse(context, input)?; + let stop = generic::ColorStop::parse(context, input, parse_position)?; - if let Ok(multi_position) = input.try(|i| T::parse(context, i)) { + if let Ok(multi_position) = input.try(|i| parse_position(context, i)) { let stop_color = stop.color.clone(); items.push(stop.into_item()); items.push( @@ -853,17 +902,15 @@ where } } -impl Parse for generic::ColorStop -where - T: Parse, -{ +impl generic::ColorStop { fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + parse_position: impl for<'i1, 't1> Fn(&ParserContext, &mut Parser<'i1, 't1>) -> Result>, ) -> Result> { Ok(generic::ColorStop { color: Color::parse(context, input)?, - position: input.try(|i| T::parse(context, i)).ok(), + position: input.try(|i| parse_position(context, i)).ok(), }) } } From 471e491a8980d7ccd876b272041a53e1e8ff243e Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Wed, 19 Feb 2020 20:04:36 +0000 Subject: [PATCH 18/74] style: Add support for the 'all' value of text-decoration-skip-ink. Differential Revision: https://phabricator.services.mozilla.com/D61184 --- components/style/values/specified/text.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 0228d5123ce..1c231b0a9cc 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -1010,7 +1010,7 @@ pub enum OverflowWrap { Anywhere, } -/// Implements text-decoration-skip-ink which takes the keywords auto | none +/// Implements text-decoration-skip-ink which takes the keywords auto | none | all /// /// https://drafts.csswg.org/css-text-decor-4/#text-decoration-skip-ink-property #[repr(u8)] @@ -1033,6 +1033,7 @@ pub enum OverflowWrap { pub enum TextDecorationSkipInk { Auto, None, + All, } /// Implements type for `text-decoration-thickness` property From c52bae192347e2516010f103614e266b2d70c663 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 20 Feb 2020 08:43:08 +0000 Subject: [PATCH 19/74] style: Remove an invalid assert. This assert was wrong. The assert may fire if we resurrect the node from a different thread and insert a kid fast enough. We allow resurrecting nodes (bumping the nodes from zero to one) to avoid allocation churn. In particular, while the thread dropping the node gets to read the children (so after the fetch_sub from the refcount, but before the read() of the children), another thread could plausibly bumped the refcount back, and added a children. This is a very big edge case of course, but I'm kinda sad I hadn't realized before. Differential Revision: https://phabricator.services.mozilla.com/D63286 --- components/style/rule_tree/mod.rs | 64 ------------------------------- 1 file changed, 64 deletions(-) diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 113fa9cb9bb..ddac85a8075 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -136,29 +136,6 @@ impl StyleSource { let _ = write!(writer, " -> {:?}", self.read(guard).declarations()); } - // This is totally unsafe, should be removed when we figure out the cause of - // bug 1607553. - #[cfg(feature = "gecko")] - unsafe fn dump_unchecked(&self, writer: &mut W) { - if let Some(ref rule) = self.0.as_first() { - let rule = rule.read_unchecked(); - let _ = write!(writer, "{:?}", rule.selectors); - } - let _ = write!(writer, " -> {:?}", self.read_unchecked().declarations()); - } - - // This is totally unsafe, should be removed when we figure out the cause of - // bug 1607553. - #[inline] - #[cfg(feature = "gecko")] - unsafe fn read_unchecked(&self) -> &PropertyDeclarationBlock { - let block: &Locked = match self.0.borrow() { - ArcUnionBorrow::First(ref rule) => &rule.get().read_unchecked().block, - ArcUnionBorrow::Second(ref block) => block.get(), - }; - block.read_unchecked() - } - /// Read the style source guard, and obtain thus read access to the /// underlying property declaration block. #[inline] @@ -1742,47 +1719,6 @@ impl Drop for StrongRuleNode { return; } - #[cfg(feature = "gecko")] - #[inline(always)] - fn assert_on_release() -> bool { - crate::gecko_bindings::structs::GECKO_IS_NIGHTLY - } - - #[cfg(feature = "servo")] - fn assert_on_release() -> bool { - false - } - - if cfg!(debug_assertions) || assert_on_release() { - let children = node.children.read(); - if !children.is_empty() { - let mut crash_str = vec![]; - - #[cfg(feature = "gecko")] - unsafe { - // Try to unsafely collect some information of this before - // crashing the process. - if let Some(ref s) = node.source { - s.dump_unchecked(&mut crash_str); - crash_str.push(b'\n'); - } - children.each(|child| { - (*child.ptr()) - .source - .as_ref() - .unwrap() - .dump_unchecked(&mut crash_str); - crash_str.push(b'\n'); - }); - } - - panic!( - "Children left in the rule tree on drop: {}", - String::from_utf8_lossy(&crash_str).trim() - ); - } - } - if node.parent.is_none() { debug!("Dropping root node!"); // The free list should be null by this point From 869553357d895c4b0abae34d5bc32a9e82a19a21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 21 Feb 2020 00:46:26 +0000 Subject: [PATCH 20/74] style: Implement ToCss for CalcNode. We'll use `CalcNode` as the specified value representation for and values, so they'll have to implement ToCss. There's one minor issue (two calls to to_css() instead of to_css_impl() which are addressed later in the series). Differential Revision: https://phabricator.services.mozilla.com/D63395 --- components/style/values/specified/calc.rs | 102 ++++++++++++++++++ components/style/values/specified/length.rs | 44 +++++++- .../style/values/specified/percentage.rs | 3 - 3 files changed, 144 insertions(+), 5 deletions(-) diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index 9736491adce..f20230b2f76 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -287,6 +287,19 @@ impl PartialOrd for CalcNode { } impl CalcNode { + fn is_simple_negative(&self) -> bool { + match *self { + Self::Length(ref l) => l.is_negative(), + Self::Percentage(n) | + Self::Number(n) => n < 0., + Self::Angle(ref a) => a.degrees() < 0., + Self::Time(ref t) => t.seconds() < 0., + Self::MinMax(..) | + Self::Sum(..) | + Self::Clamp { .. } => false, + } + } + fn negate(&mut self) { self.mul_by(-1.); } @@ -1022,4 +1035,93 @@ impl CalcNode { Err(()) => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), } } + + fn to_css_impl(&self, dest: &mut CssWriter, is_outermost: bool) -> fmt::Result + where + W: Write, + { + let write_closing_paren = match *self { + Self::MinMax(_, op) => { + dest.write_str(match op { + MinMaxOp::Max => "max(", + MinMaxOp::Min => "min(", + })?; + true + }, + Self::Clamp { .. } => { + dest.write_str("clamp(")?; + true + }, + _ => { + if is_outermost { + dest.write_str("calc(")?; + } + is_outermost + }, + }; + + match *self { + Self::MinMax(ref children, _) => { + let mut first = true; + for child in &**children { + if !first { + dest.write_str(", ")?; + } + first = false; + child.to_css_impl(dest, false)?; + } + }, + Self::Sum(ref children) => { + let mut first = true; + for child in &**children { + if !first { + if child.is_simple_negative() { + dest.write_str(" - ")?; + let mut c = child.clone(); + c.negate(); + c.to_css(dest)?; + } else { + dest.write_str(" + ")?; + child.to_css(dest)?; + } + } else { + first = false; + child.to_css_impl(dest, false)?; + } + } + }, + Self::Clamp { + ref min, + ref center, + ref max, + } => { + min.to_css_impl(dest, false)?; + dest.write_str(", ")?; + center.to_css_impl(dest, false)?; + dest.write_str(", ")?; + max.to_css_impl(dest, false)?; + }, + Self::Length(ref l) => l.to_css(dest)?, + Self::Number(ref n) => n.to_css(dest)?, + Self::Percentage(p) => crate::values::serialize_percentage(p, dest)?, + Self::Angle(ref a) => a.to_css(dest)?, + Self::Time(ref t) => t.to_css(dest)?, + + } + + if write_closing_paren { + dest.write_str(")")?; + } + Ok(()) + } +} + +impl ToCss for CalcNode { + /// + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + self.to_css_impl(dest, /* is_outermost = */ true) + } } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 16b9bf003e6..7bc6ed80587 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -92,7 +92,16 @@ impl FontRelativeLength { FontRelativeLength::Em(v) | FontRelativeLength::Ex(v) | FontRelativeLength::Ch(v) | - FontRelativeLength::Rem(v) => v == 0.0, + FontRelativeLength::Rem(v) => v == 0., + } + } + + fn is_negative(&self) -> bool { + match *self { + FontRelativeLength::Em(v) | + FontRelativeLength::Ex(v) | + FontRelativeLength::Ch(v) | + FontRelativeLength::Rem(v) => v < 0., } } @@ -266,7 +275,16 @@ impl ViewportPercentageLength { ViewportPercentageLength::Vw(v) | ViewportPercentageLength::Vh(v) | ViewportPercentageLength::Vmin(v) | - ViewportPercentageLength::Vmax(v) => v == 0.0, + ViewportPercentageLength::Vmax(v) => v == 0., + } + } + + fn is_negative(&self) -> bool { + match *self { + ViewportPercentageLength::Vw(v) | + ViewportPercentageLength::Vh(v) | + ViewportPercentageLength::Vmin(v) | + ViewportPercentageLength::Vmax(v) => v < 0., } } @@ -370,6 +388,18 @@ impl AbsoluteLength { } } + fn is_negative(&self) -> bool { + match *self { + AbsoluteLength::Px(v) | + AbsoluteLength::In(v) | + AbsoluteLength::Cm(v) | + AbsoluteLength::Mm(v) | + AbsoluteLength::Q(v) | + AbsoluteLength::Pt(v) | + AbsoluteLength::Pc(v) => v < 0., + } + } + /// Convert this into a pixel value. #[inline] pub fn to_px(&self) -> CSSFloat { @@ -484,6 +514,16 @@ impl Mul for NoCalcLength { } impl NoCalcLength { + /// Returns whether the value of this length without unit is less than zero. + pub fn is_negative(&self) -> bool { + match *self { + NoCalcLength::Absolute(v) => v.is_negative(), + NoCalcLength::FontRelative(v) => v.is_negative(), + NoCalcLength::ViewportPercentage(v) => v.is_negative(), + NoCalcLength::ServoCharacterWidth(c) => c.0 < 0, + } + } + /// Parse a given absolute or relative dimension. pub fn parse_dimension( context: &ParserContext, diff --git a/components/style/values/specified/percentage.rs b/components/style/values/specified/percentage.rs index 75549dca3be..eac4d91cf8f 100644 --- a/components/style/values/specified/percentage.rs +++ b/components/style/values/specified/percentage.rs @@ -120,9 +120,6 @@ impl Percentage { Token::Function(ref name) => { let function = CalcNode::math_function(name, location)?; let value = CalcNode::parse_percentage(context, input, function)?; - - // TODO(emilio): -moz-image-rect is the only thing that uses - // the clamping mode... I guess we could disallow it... Ok(Percentage { value, calc_clamping_mode: Some(num_context), From 426edbd991df7ad9b4452a0a02f91259fe2a92d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 21 Feb 2020 00:46:33 +0000 Subject: [PATCH 21/74] style: Move the guts of calc nodes into a generic enum. We'll have different leaf nodes as we progress in the value computation stage. Differential Revision: https://phabricator.services.mozilla.com/D63396 --- components/style/values/generics/calc.rs | 408 ++++++++++++++++++++ components/style/values/generics/mod.rs | 1 + components/style/values/specified/calc.rs | 444 ++++------------------ 3 files changed, 488 insertions(+), 365 deletions(-) create mode 100644 components/style/values/generics/calc.rs diff --git a/components/style/values/generics/calc.rs b/components/style/values/generics/calc.rs new file mode 100644 index 00000000000..3a973f99f9d --- /dev/null +++ b/components/style/values/generics/calc.rs @@ -0,0 +1,408 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +//! [Calc expressions][calc]. +//! +//! [calc]: https://drafts.csswg.org/css-values/#calc-notation + +use style_traits::{CssWriter, ToCss}; +use std::fmt::{self, Write}; +use std::{cmp, mem}; +use smallvec::SmallVec; + +/// Whether we're a `min` or `max` function. +#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToShmem)] +#[repr(u8)] +pub enum MinMaxOp { + /// `min()` + Min, + /// `max()` + Max, +} + +/// This determines the order in which we serialize members of a calc() sum. +/// +/// See https://drafts.csswg.org/css-values-4/#sort-a-calculations-children +#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] +#[allow(missing_docs)] +pub enum SortKey { + Number, + Percentage, + Ch, + Deg, + Em, + Ex, + Px, + Rem, + Sec, + Vh, + Vmax, + Vmin, + Vw, + Other, +} + +/// A generic node in a calc expression. +#[repr(u8)] +#[derive(Clone, Debug, PartialEq)] +pub enum GenericCalcNode { + /// A leaf node. + Leaf(L), + /// A sum node, representing `a + b + c` where a, b, and c are the + /// arguments. + Sum(Box<[Self]>), + /// A `min` or `max` function. + MinMax(Box<[Self]>, MinMaxOp), + /// A `clamp()` function. + Clamp { + /// The minimum value. + min: Box, + /// The central value. + center: Box, + /// The maximum value. + max: Box, + }, +} + +pub use self::GenericCalcNode as CalcNode; + +/// A trait that represents all the stuff a valid leaf of a calc expression. +pub trait CalcNodeLeaf : Clone + Sized + PartialOrd + PartialEq + ToCss { + /// Whether this value is known-negative. + fn is_negative(&self) -> bool; + + /// Tries to merge one sum to another, that is, perform `x` + `y`. + fn try_sum_in_place(&mut self, other: &Self) -> Result<(), ()>; + + /// Multiplies the leaf by a given scalar number. + fn mul_by(&mut self, scalar: f32); + + /// Negates the leaf. + fn negate(&mut self) { + self.mul_by(-1.); + } + + /// Canonicalizes the expression if necessary. + fn simplify(&mut self); + + /// Returns the sort key for simplification. + fn sort_key(&self) -> SortKey; +} + +impl CalcNode { + /// Negates the node. + pub fn negate(&mut self) { + self.mul_by(-1.); + } + + fn sort_key(&self) -> SortKey { + match *self { + Self::Leaf(ref l) => l.sort_key(), + _ => SortKey::Other, + } + } + + /// Tries to merge one sum to another, that is, perform `x` + `y`. + fn try_sum_in_place(&mut self, other: &Self) -> Result<(), ()> { + match (self, other) { + (&mut CalcNode::Leaf(ref mut one), &CalcNode::Leaf(ref other)) => one.try_sum_in_place(other), + _ => Err(()), + } + } + + fn is_negative_leaf(&self) -> bool { + match *self { + Self::Leaf(ref l) => l.is_negative(), + _ => false, + } + } + + /// Multiplies the node by a scalar. + pub fn mul_by(&mut self, scalar: f32) { + match *self { + Self::Leaf(ref mut l) => l.mul_by(scalar), + // Multiplication is distributive across this. + Self::Sum(ref mut children) => { + for node in &mut **children { + node.mul_by(scalar); + } + }, + // This one is a bit trickier. + Self::MinMax(ref mut children, ref mut op) => { + for node in &mut **children { + node.mul_by(scalar); + } + + // For negatives we need to invert the operation. + if scalar < 0. { + *op = match *op { + MinMaxOp::Min => MinMaxOp::Max, + MinMaxOp::Max => MinMaxOp::Min, + } + } + }, + // This one is slightly tricky too. + Self::Clamp { + ref mut min, + ref mut center, + ref mut max, + } => { + min.mul_by(scalar); + center.mul_by(scalar); + max.mul_by(scalar); + // For negatives we need to swap min / max. + if scalar < 0. { + mem::swap(min, max); + } + }, + } + } + + /// Simplifies and sorts the calculation. This is only needed if it's going + /// to be preserved after parsing (so, for ``). Otherwise + /// we can just evaluate it and we'll come up with a simplified value + /// anyways. + pub fn simplify_and_sort_children(&mut self) { + macro_rules! replace_self_with { + ($slot:expr) => {{ + let dummy = Self::MinMax(Box::new([]), MinMaxOp::Max); + let result = mem::replace($slot, dummy); + mem::replace(self, result); + }}; + } + match *self { + Self::Clamp { + ref mut min, + ref mut center, + ref mut max, + } => { + min.simplify_and_sort_children(); + center.simplify_and_sort_children(); + max.simplify_and_sort_children(); + + // NOTE: clamp() is max(min, min(center, max)) + let min_cmp_center = match min.partial_cmp(¢er) { + Some(o) => o, + None => return, + }; + + // So if we can prove that min is more than center, then we won, + // as that's what we should always return. + if matches!(min_cmp_center, cmp::Ordering::Greater) { + return replace_self_with!(&mut **min); + } + + // Otherwise try with max. + let max_cmp_center = match max.partial_cmp(¢er) { + Some(o) => o, + None => return, + }; + + if matches!(max_cmp_center, cmp::Ordering::Less) { + // max is less than center, so we need to return effectively + // `max(min, max)`. + let max_cmp_min = match max.partial_cmp(&min) { + Some(o) => o, + None => { + debug_assert!( + false, + "We compared center with min and max, how are \ + min / max not comparable with each other?" + ); + return; + }, + }; + + if matches!(max_cmp_min, cmp::Ordering::Less) { + return replace_self_with!(&mut **min); + } + + return replace_self_with!(&mut **max); + } + + // Otherwise we're the center node. + return replace_self_with!(&mut **center); + }, + Self::MinMax(ref mut children, op) => { + for child in &mut **children { + child.simplify_and_sort_children(); + } + + let winning_order = match op { + MinMaxOp::Min => cmp::Ordering::Less, + MinMaxOp::Max => cmp::Ordering::Greater, + }; + + let mut result = 0; + for i in 1..children.len() { + let o = match children[i].partial_cmp(&children[result]) { + // We can't compare all the children, so we can't + // know which one will actually win. Bail out and + // keep ourselves as a min / max function. + // + // TODO: Maybe we could simplify compatible children, + // see https://github.com/w3c/csswg-drafts/issues/4756 + None => return, + Some(o) => o, + }; + + if o == winning_order { + result = i; + } + } + + replace_self_with!(&mut children[result]); + }, + Self::Sum(ref mut children_slot) => { + let mut sums_to_merge = SmallVec::<[_; 3]>::new(); + let mut extra_kids = 0; + for (i, child) in children_slot.iter_mut().enumerate() { + child.simplify_and_sort_children(); + if let Self::Sum(ref mut children) = *child { + extra_kids += children.len(); + sums_to_merge.push(i); + } + } + + // If we only have one kid, we've already simplified it, and it + // doesn't really matter whether it's a sum already or not, so + // lift it up and continue. + if children_slot.len() == 1 { + return replace_self_with!(&mut children_slot[0]); + } + + let mut children = mem::replace(children_slot, Box::new([])).into_vec(); + + if !sums_to_merge.is_empty() { + children.reserve(extra_kids - sums_to_merge.len()); + // Merge all our nested sums, in reverse order so that the + // list indices are not invalidated. + for i in sums_to_merge.drain(..).rev() { + let kid_children = match children.swap_remove(i) { + Self::Sum(c) => c, + _ => unreachable!(), + }; + + // This would be nicer with + // https://github.com/rust-lang/rust/issues/59878 fixed. + children.extend(kid_children.into_vec()); + } + } + + debug_assert!(children.len() >= 2, "Should still have multiple kids!"); + + // Sort by spec order. + children.sort_unstable_by_key(|c| c.sort_key()); + + // NOTE: if the function returns true, by the docs of dedup_by, + // a is removed. + children.dedup_by(|a, b| b.try_sum_in_place(a).is_ok()); + + if children.len() == 1 { + // If only one children remains, lift it up, and carry on. + replace_self_with!(&mut children[0]); + } else { + // Else put our simplified children back. + mem::replace(children_slot, children.into_boxed_slice()); + } + }, + Self::Leaf(ref mut l) => { + l.simplify(); + } + } + } + + fn to_css_impl(&self, dest: &mut CssWriter, is_outermost: bool) -> fmt::Result + where + W: Write, + { + let write_closing_paren = match *self { + Self::MinMax(_, op) => { + dest.write_str(match op { + MinMaxOp::Max => "max(", + MinMaxOp::Min => "min(", + })?; + true + }, + Self::Clamp { .. } => { + dest.write_str("clamp(")?; + true + }, + _ => { + if is_outermost { + dest.write_str("calc(")?; + } + is_outermost + }, + }; + + match *self { + Self::MinMax(ref children, _) => { + let mut first = true; + for child in &**children { + if !first { + dest.write_str(", ")?; + } + first = false; + child.to_css_impl(dest, false)?; + } + }, + Self::Sum(ref children) => { + let mut first = true; + for child in &**children { + if !first { + if child.is_negative_leaf() { + dest.write_str(" - ")?; + let mut c = child.clone(); + c.negate(); + c.to_css(dest)?; + } else { + dest.write_str(" + ")?; + child.to_css(dest)?; + } + } else { + first = false; + child.to_css_impl(dest, false)?; + } + } + }, + Self::Clamp { + ref min, + ref center, + ref max, + } => { + min.to_css_impl(dest, false)?; + dest.write_str(", ")?; + center.to_css_impl(dest, false)?; + dest.write_str(", ")?; + max.to_css_impl(dest, false)?; + }, + Self::Leaf(ref l) => l.to_css(dest)?, + } + + if write_closing_paren { + dest.write_str(")")?; + } + Ok(()) + } +} + +impl PartialOrd for CalcNode { + fn partial_cmp(&self, other: &Self) -> Option { + match (self, other) { + (&CalcNode::Leaf(ref one), &CalcNode::Leaf(ref other)) => one.partial_cmp(other), + _ => None, + } + } +} + +impl ToCss for CalcNode { + /// + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + self.to_css_impl(dest, /* is_outermost = */ true) + } +} diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index a97d9e1018a..6689405b2b1 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -19,6 +19,7 @@ pub mod basic_shape; pub mod border; #[path = "box.rs"] pub mod box_; +pub mod calc; pub mod color; pub mod column; pub mod counters; diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index f20230b2f76..61f9103a750 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -8,6 +8,8 @@ use crate::parser::ParserContext; use crate::values::computed; +use crate::values::generics::calc as generic; +use crate::values::generics::calc::{MinMaxOp, SortKey}; use crate::values::specified::length::ViewportPercentageLength; use crate::values::specified::length::{AbsoluteLength, FontRelativeLength, NoCalcLength}; use crate::values::specified::{self, Angle, Time}; @@ -15,7 +17,7 @@ use crate::values::{CSSFloat, CSSInteger}; use cssparser::{AngleOrNumber, CowRcStr, NumberOrPercentage, Parser, Token}; use smallvec::SmallVec; use std::fmt::{self, Write}; -use std::{cmp, mem}; +use std::cmp; use style_traits::values::specified::AllowedNumericType; use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, StyleParseErrorKind, ToCss}; @@ -32,40 +34,9 @@ pub enum MathFunction { Clamp, } -/// This determines the order in which we serialize members of a calc() -/// sum. -/// -/// See https://drafts.csswg.org/css-values-4/#sort-a-calculations-children -#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] -enum SortKey { - Number, - Percentage, - Ch, - Deg, - Em, - Ex, - Px, - Rem, - Sec, - Vh, - Vmax, - Vmin, - Vw, - Other, -} - -/// Whether we're a `min` or `max` function. -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum MinMaxOp { - /// `min()` - Min, - /// `max()` - Max, -} - -/// A node inside a `Calc` expression's AST. +/// A leaf node inside a `Calc` expression's AST. #[derive(Clone, Debug, PartialEq)] -pub enum CalcNode { +pub enum Leaf { /// `` Length(NoCalcLength), /// `` @@ -76,27 +47,28 @@ pub enum CalcNode { Percentage(CSSFloat), /// `` Number(CSSFloat), - /// An expression of the form `x + y + ...`. Subtraction is represented by - /// the negated expression of the right hand side. - Sum(Box<[CalcNode]>), - /// A `min()` / `max()` function. - MinMax(Box<[CalcNode]>, MinMaxOp), - /// A `clamp()` function. - Clamp { - /// The minimum value. - min: Box, - /// The central value. - center: Box, - /// The maximum value. - max: Box, - }, +} + +impl ToCss for Leaf { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + match *self { + Self::Length(ref l) => l.to_css(dest), + Self::Number(ref n) => n.to_css(dest), + Self::Percentage(p) => crate::values::serialize_percentage(p, dest), + Self::Angle(ref a) => a.to_css(dest), + Self::Time(ref t) => t.to_css(dest), + } + } } /// An expected unit we intend to parse within a `calc()` expression. /// /// This is used as a hint for the parser to fast-reject invalid expressions. #[derive(Clone, Copy, PartialEq)] -pub enum CalcUnit { +enum CalcUnit { /// `` Number, /// `` @@ -204,7 +176,7 @@ impl SpecifiedValueInfo for CalcLengthPercentage {} macro_rules! impl_generic_to_type { ($self:ident, $self_variant:ident, $to_self:ident, $to_float:ident, $from_float:path) => {{ - if let Self::$self_variant(ref v) = *$self { + if let Self::Leaf(Leaf::$self_variant(ref v)) = *$self { return Ok(v.clone()); } @@ -263,47 +235,46 @@ macro_rules! impl_generic_to_type { } result }, - Self::Length(..) | - Self::Angle(..) | - Self::Time(..) | - Self::Percentage(..) | - Self::Number(..) => return Err(()), + Self::Leaf(..) => return Err(()), }) }}; } -impl PartialOrd for CalcNode { +impl PartialOrd for Leaf { fn partial_cmp(&self, other: &Self) -> Option { - use self::CalcNode::*; + use self::Leaf::*; + + if std::mem::discriminant(self) != std::mem::discriminant(other) { + return None; + } + match (self, other) { (&Length(ref one), &Length(ref other)) => one.partial_cmp(other), (&Percentage(ref one), &Percentage(ref other)) => one.partial_cmp(other), (&Angle(ref one), &Angle(ref other)) => one.degrees().partial_cmp(&other.degrees()), (&Time(ref one), &Time(ref other)) => one.seconds().partial_cmp(&other.seconds()), (&Number(ref one), &Number(ref other)) => one.partial_cmp(other), - _ => None, + _ => { + match *self { + Length(..) | Percentage(..) | Angle(..) | Time(..) | Number(..) => {}, + } + unsafe { debug_unreachable!("Forgot a branch?"); } + } } } } -impl CalcNode { - fn is_simple_negative(&self) -> bool { +impl generic::CalcNodeLeaf for Leaf { + fn is_negative(&self) -> bool { match *self { Self::Length(ref l) => l.is_negative(), Self::Percentage(n) | Self::Number(n) => n < 0., Self::Angle(ref a) => a.degrees() < 0., Self::Time(ref t) => t.seconds() < 0., - Self::MinMax(..) | - Self::Sum(..) | - Self::Clamp { .. } => false, } } - fn negate(&mut self) { - self.mul_by(-1.); - } - fn mul_by(&mut self, scalar: f32) { match *self { Self::Length(ref mut l) => { @@ -323,44 +294,10 @@ impl CalcNode { Self::Percentage(ref mut p) => { *p *= scalar; }, - // Multiplication is distributive across this. - Self::Sum(ref mut children) => { - for node in &mut **children { - node.mul_by(scalar); - } - }, - // This one is a bit trickier. - Self::MinMax(ref mut children, ref mut op) => { - for node in &mut **children { - node.mul_by(scalar); - } - - // For negatives we need to invert the operation. - if scalar < 0. { - *op = match *op { - MinMaxOp::Min => MinMaxOp::Max, - MinMaxOp::Max => MinMaxOp::Min, - } - } - }, - // Multiplication is distributive across these. - Self::Clamp { - ref mut min, - ref mut center, - ref mut max, - } => { - min.mul_by(scalar); - center.mul_by(scalar); - max.mul_by(scalar); - // For negatives we need to swap min / max. - if scalar < 0. { - mem::swap(min, max); - } - }, } } - fn calc_node_sort_key(&self) -> SortKey { + fn sort_key(&self) -> SortKey { match *self { Self::Number(..) => SortKey::Number, Self::Percentage(..) => SortKey::Percentage, @@ -382,7 +319,12 @@ impl CalcNode { }, NoCalcLength::ServoCharacterWidth(..) => unreachable!(), }, - Self::Sum(..) | Self::MinMax(..) | Self::Clamp { .. } => SortKey::Other, + } + } + + fn simplify(&mut self) { + if let Self::Length(NoCalcLength::Absolute(ref mut abs)) = *self { + *abs = AbsoluteLength::Px(abs.to_px()); } } @@ -391,7 +333,11 @@ impl CalcNode { /// Only handles leaf nodes, it's the caller's responsibility to simplify /// them before calling this if needed. fn try_sum_in_place(&mut self, other: &Self) -> Result<(), ()> { - use self::CalcNode::*; + use self::Leaf::*; + + if std::mem::discriminant(self) != std::mem::discriminant(other) { + return Err(()); + } match (self, other) { (&mut Number(ref mut one), &Number(ref other)) | @@ -407,170 +353,22 @@ impl CalcNode { (&mut Length(ref mut one), &Length(ref other)) => { *one = one.try_sum(other)?; }, - _ => return Err(()), + _ => { + match *other { + Number(..) | Percentage(..) | Angle(..) | Time(..) | Length(..) => {}, + } + unsafe { debug_unreachable!(); } + } } Ok(()) } +} - /// Simplifies and sorts the calculation. This is only needed if it's going - /// to be preserved after parsing (so, for ``). Otherwise - /// we can just evaluate it and we'll come up with a simplified value - /// anyways. - fn simplify_and_sort_children(&mut self) { - macro_rules! replace_self_with { - ($slot:expr) => {{ - let result = mem::replace($slot, Self::Number(0.)); - mem::replace(self, result); - }}; - } - match *self { - Self::Clamp { - ref mut min, - ref mut center, - ref mut max, - } => { - min.simplify_and_sort_children(); - center.simplify_and_sort_children(); - max.simplify_and_sort_children(); - - // NOTE: clamp() is max(min, min(center, max)) - let min_cmp_center = match min.partial_cmp(¢er) { - Some(o) => o, - None => return, - }; - - // So if we can prove that min is more than center, then we won, - // as that's what we should always return. - if matches!(min_cmp_center, cmp::Ordering::Greater) { - return replace_self_with!(&mut **min); - } - - // Otherwise try with max. - let max_cmp_center = match max.partial_cmp(¢er) { - Some(o) => o, - None => return, - }; - - if matches!(max_cmp_center, cmp::Ordering::Less) { - // max is less than center, so we need to return effectively - // `max(min, max)`. - let max_cmp_min = match max.partial_cmp(&min) { - Some(o) => o, - None => { - debug_assert!( - false, - "We compared center with min and max, how are \ - min / max not comparable with each other?" - ); - return; - }, - }; - - if matches!(max_cmp_min, cmp::Ordering::Less) { - return replace_self_with!(&mut **min); - } - - return replace_self_with!(&mut **max); - } - - // Otherwise we're the center node. - return replace_self_with!(&mut **center); - }, - Self::MinMax(ref mut children, op) => { - for child in &mut **children { - child.simplify_and_sort_children(); - } - - let winning_order = match op { - MinMaxOp::Min => cmp::Ordering::Less, - MinMaxOp::Max => cmp::Ordering::Greater, - }; - - let mut result = 0; - for i in 1..children.len() { - let o = match children[i].partial_cmp(&children[result]) { - // We can't compare all the children, so we can't - // know which one will actually win. Bail out and - // keep ourselves as a min / max function. - // - // TODO: Maybe we could simplify compatible children, - // see https://github.com/w3c/csswg-drafts/issues/4756 - None => return, - Some(o) => o, - }; - - if o == winning_order { - result = i; - } - } - - replace_self_with!(&mut children[result]); - }, - Self::Sum(ref mut children_slot) => { - let mut sums_to_merge = SmallVec::<[_; 3]>::new(); - let mut extra_kids = 0; - for (i, child) in children_slot.iter_mut().enumerate() { - child.simplify_and_sort_children(); - if let Self::Sum(ref mut children) = *child { - extra_kids += children.len(); - sums_to_merge.push(i); - } - } - - // If we only have one kid, we've already simplified it, and it - // doesn't really matter whether it's a sum already or not, so - // lift it up and continue. - if children_slot.len() == 1 { - return replace_self_with!(&mut children_slot[0]); - } - - let mut children = mem::replace(children_slot, Box::new([])).into_vec(); - - if !sums_to_merge.is_empty() { - children.reserve(extra_kids - sums_to_merge.len()); - // Merge all our nested sums, in reverse order so that the - // list indices are not invalidated. - for i in sums_to_merge.drain(..).rev() { - let kid_children = match children.swap_remove(i) { - Self::Sum(c) => c, - _ => unreachable!(), - }; - - // This would be nicer with - // https://github.com/rust-lang/rust/issues/59878 fixed. - children.extend(kid_children.into_vec()); - } - } - - debug_assert!(children.len() >= 2, "Should still have multiple kids!"); - - // Sort by spec order. - children.sort_unstable_by_key(|c| c.calc_node_sort_key()); - - // NOTE: if the function returns true, by the docs of dedup_by, - // a is removed. - children.dedup_by(|a, b| b.try_sum_in_place(a).is_ok()); - - if children.len() == 1 { - // If only one children remains, lift it up, and carry on. - replace_self_with!(&mut children[0]); - } else { - // Else put our simplified children back. - mem::replace(children_slot, children.into_boxed_slice()); - } - }, - Self::Length(ref mut len) => { - if let NoCalcLength::Absolute(ref mut absolute_length) = *len { - *absolute_length = AbsoluteLength::Px(absolute_length.to_px()); - } - }, - Self::Percentage(..) | Self::Angle(..) | Self::Time(..) | Self::Number(..) => { - // These are leaves already, nothing to do. - }, - } - } +/// A calc node representation for specified values. +pub type CalcNode = generic::GenericCalcNode; +impl CalcNode { /// Tries to parse a single element in the expression, that is, a /// ``, ``, `