From ca8fae91dac401bc56d590a876ae86281baf7921 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 24 May 2017 11:38:18 +0200 Subject: [PATCH 1/3] Use generics for background-size property --- components/layout/display_list_builder.rs | 27 +-- components/style/properties/gecko.mako.rs | 35 ++-- .../properties/longhand/background.mako.rs | 189 +----------------- .../style/values/computed/background.rs | 52 +++++ components/style/values/computed/mod.rs | 2 + .../style/values/generics/background.rs | 52 +++++ components/style/values/generics/mod.rs | 1 + .../style/values/specified/background.rs | 29 +++ components/style/values/specified/mod.rs | 4 +- tests/unit/style/properties/serialization.rs | 27 ++- 10 files changed, 187 insertions(+), 231 deletions(-) create mode 100644 components/style/values/computed/background.rs create mode 100644 components/style/values/generics/background.rs create mode 100644 components/style/values/specified/background.rs diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 13829e8830d..b0549cde28a 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -48,7 +48,7 @@ use std::default::Default; use std::mem; use std::sync::Arc; use style::computed_values::{background_attachment, background_clip, background_origin}; -use style::computed_values::{background_repeat, background_size, border_style, cursor}; +use style::computed_values::{background_repeat, border_style, cursor}; use style::computed_values::{image_rendering, overflow_x, pointer_events, position, visibility}; use style::computed_values::filter::Filter; use style::computed_values::text_shadow::TextShadow; @@ -61,6 +61,7 @@ use style::values::{Either, RGBA}; use style::values::computed::{Gradient, GradientItem, LengthOrPercentage}; use style::values::computed::{LengthOrPercentageOrAuto, NumberOrPercentage, Position}; use style::values::computed::image::{EndingShape, LineDirection}; +use style::values::generics::background::BackgroundSize; use style::values::generics::image::{Circle, Ellipse, EndingShape as GenericEndingShape}; use style::values::generics::image::{GradientItem as GenericGradientItem, GradientKind}; use style::values::generics::image::{Image, ShapeExtent}; @@ -917,43 +918,29 @@ impl FragmentDisplayListBuilding for Fragment { Au::from_px(image.height as i32)); let background_size = get_cyclic(&style.get_background().background_size.0, index).clone(); match (background_size, image_aspect_ratio < bounds_aspect_ratio) { - (background_size::single_value::T::Contain, false) | - (background_size::single_value::T::Cover, true) => { + (BackgroundSize::Contain, false) | (BackgroundSize::Cover, true) => { Size2D::new(bounds.size.width, Au::from_f64_px(bounds.size.width.to_f64_px() / image_aspect_ratio)) } - (background_size::single_value::T::Contain, true) | - (background_size::single_value::T::Cover, false) => { + (BackgroundSize::Contain, true) | (BackgroundSize::Cover, false) => { Size2D::new(Au::from_f64_px(bounds.size.height.to_f64_px() * image_aspect_ratio), bounds.size.height) } - (background_size::single_value::T::Explicit(background_size::single_value - ::ExplicitSize { - width, - height: LengthOrPercentageOrAuto::Auto, - }), _) => { + (BackgroundSize::Explicit { width, height: LengthOrPercentageOrAuto::Auto }, _) => { let width = MaybeAuto::from_style(width, bounds.size.width) .specified_or_default(intrinsic_size.width); Size2D::new(width, Au::from_f64_px(width.to_f64_px() / image_aspect_ratio)) } - (background_size::single_value::T::Explicit(background_size::single_value - ::ExplicitSize { - width: LengthOrPercentageOrAuto::Auto, - height - }), _) => { + (BackgroundSize::Explicit { width: LengthOrPercentageOrAuto::Auto, height }, _) => { let height = MaybeAuto::from_style(height, bounds.size.height) .specified_or_default(intrinsic_size.height); Size2D::new(Au::from_f64_px(height.to_f64_px() * image_aspect_ratio), height) } - (background_size::single_value::T::Explicit(background_size::single_value - ::ExplicitSize { - width, - height - }), _) => { + (BackgroundSize::Explicit { width, height }, _) => { Size2D::new(MaybeAuto::from_style(width, bounds.size.width) .specified_or_default(intrinsic_size.width), MaybeAuto::from_style(height, bounds.size.height) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index fd4975826d7..0d5d33c8dcc 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2951,29 +2951,37 @@ fn static_assert() { use gecko_bindings::structs::nsStyleImageLayers_Size_Dimension; use gecko_bindings::structs::nsStyleImageLayers_Size_DimensionType; use gecko_bindings::structs::{nsStyleCoord_CalcValue, nsStyleImageLayers_Size}; - use properties::longhands::background_size::single_value::computed_value::T; + use values::generics::background::BackgroundSize; let mut width = nsStyleCoord_CalcValue::new(); let mut height = nsStyleCoord_CalcValue::new(); let (w_type, h_type) = match servo { - T::Explicit(size) => { + BackgroundSize::Explicit { width: explicit_width, height: explicit_height } => { let mut w_type = nsStyleImageLayers_Size_DimensionType::eAuto; let mut h_type = nsStyleImageLayers_Size_DimensionType::eAuto; - if let Some(w) = size.width.to_calc_value() { + if let Some(w) = explicit_width.to_calc_value() { width = w; w_type = nsStyleImageLayers_Size_DimensionType::eLengthPercentage; } - if let Some(h) = size.height.to_calc_value() { + if let Some(h) = explicit_height.to_calc_value() { height = h; h_type = nsStyleImageLayers_Size_DimensionType::eLengthPercentage; } (w_type, h_type) } - T::Cover => (nsStyleImageLayers_Size_DimensionType::eCover, - nsStyleImageLayers_Size_DimensionType::eCover), - T::Contain => (nsStyleImageLayers_Size_DimensionType::eContain, - nsStyleImageLayers_Size_DimensionType::eContain), + BackgroundSize::Cover => { + ( + nsStyleImageLayers_Size_DimensionType::eCover, + nsStyleImageLayers_Size_DimensionType::eCover, + ) + }, + BackgroundSize::Contain => { + ( + nsStyleImageLayers_Size_DimensionType::eContain, + nsStyleImageLayers_Size_DimensionType::eContain, + ) + }, }; nsStyleImageLayers_Size { @@ -2987,8 +2995,8 @@ fn static_assert() { pub fn clone_${shorthand}_size(&self) -> longhands::background_size::computed_value::T { use gecko_bindings::structs::nsStyleCoord_CalcValue as CalcValue; use gecko_bindings::structs::nsStyleImageLayers_Size_DimensionType as DimensionType; - use properties::longhands::background_size::single_value::computed_value::{ExplicitSize, T}; use values::computed::LengthOrPercentageOrAuto; + use values::generics::background::BackgroundSize; fn to_servo(value: CalcValue, ty: u8) -> LengthOrPercentageOrAuto { if ty == DimensionType::eAuto as u8 { @@ -3003,17 +3011,16 @@ fn static_assert() { self.gecko.${image_layers_field}.mLayers.iter().map(|ref layer| { if DimensionType::eCover as u8 == layer.mSize.mWidthType { debug_assert!(layer.mSize.mHeightType == DimensionType::eCover as u8); - return T::Cover + return BackgroundSize::Cover } if DimensionType::eContain as u8 == layer.mSize.mWidthType { debug_assert!(layer.mSize.mHeightType == DimensionType::eContain as u8); - return T::Contain + return BackgroundSize::Contain } - - T::Explicit(ExplicitSize { + BackgroundSize::Explicit { width: to_servo(layer.mSize.mWidth._base, layer.mSize.mWidthType), height: to_servo(layer.mSize.mHeight._base, layer.mSize.mHeightType), - }) + } }).collect() ) } diff --git a/components/style/properties/longhand/background.mako.rs b/components/style/properties/longhand/background.mako.rs index 699395dd9f2..60dc1e39e35 100644 --- a/components/style/properties/longhand/background.mako.rs +++ b/components/style/properties/longhand/background.mako.rs @@ -160,188 +160,13 @@ ${helpers.single_keyword("background-origin", spec="https://drafts.csswg.org/css-backgrounds/#the-background-origin", animation_value_type="none")} -<%helpers:vector_longhand name="background-size" animation_value_type="ComputedValue" extra_prefixes="webkit" - spec="https://drafts.csswg.org/css-backgrounds/#the-background-size"> - use std::fmt; - use style_traits::ToCss; - - #[allow(missing_docs)] - pub mod computed_value { - use values::computed::LengthOrPercentageOrAuto; - use properties::animated_properties::{Animatable, RepeatableListAnimatable}; - - #[derive(PartialEq, Clone, Debug)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct ExplicitSize { - pub width: LengthOrPercentageOrAuto, - pub height: LengthOrPercentageOrAuto, - } - - #[derive(PartialEq, Clone, Debug)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub enum T { - Explicit(ExplicitSize), - Cover, - Contain, - } - - impl RepeatableListAnimatable for T {} - - impl Animatable for T { - fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64) - -> Result { - use properties::longhands::background_size::single_value::computed_value::ExplicitSize; - match (self, other) { - (&T::Explicit(ref me), &T::Explicit(ref other)) => { - Ok(T::Explicit(ExplicitSize { - width: try!(me.width.add_weighted(&other.width, - self_portion, other_portion)), - height: try!(me.height.add_weighted(&other.height, - self_portion, other_portion)), - })) - } - _ => Err(()), - } - } - - #[inline] - fn compute_distance(&self, other: &Self) -> Result { - self.compute_squared_distance(other).map(|sd| sd.sqrt()) - } - - #[inline] - fn compute_squared_distance(&self, other: &Self) -> Result { - match (self, other) { - (&T::Explicit(ref me), &T::Explicit(ref other)) => { - Ok(try!(me.width.compute_squared_distance(&other.width)) + - try!(me.height.compute_squared_distance(&other.height))) - }, - _ => Err(()) - } - } - } - } - - impl ToCss for computed_value::T { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match *self { - computed_value::T::Explicit(ref size) => size.to_css(dest), - computed_value::T::Cover => dest.write_str("cover"), - computed_value::T::Contain => dest.write_str("contain"), - } - } - } - - #[derive(Clone, Debug, HasViewportPercentage, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - #[allow(missing_docs)] - pub struct ExplicitSize { - pub width: specified::LengthOrPercentageOrAuto, - pub height: specified::LengthOrPercentageOrAuto, - } - - impl ToCss for ExplicitSize { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - try!(self.width.to_css(dest)); - try!(dest.write_str(" ")); - self.height.to_css(dest) - } - } - - impl ToCss for computed_value::ExplicitSize { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - try!(self.width.to_css(dest)); - try!(dest.write_str(" ")); - self.height.to_css(dest) - } - } - - #[derive(Clone, Debug, HasViewportPercentage, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub enum SpecifiedValue { - Explicit(ExplicitSize), - Cover, - Contain, - } - - impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - match *self { - SpecifiedValue::Explicit(ref size) => size.to_css(dest), - SpecifiedValue::Cover => dest.write_str("cover"), - SpecifiedValue::Contain => dest.write_str("contain"), - } - } - } - - impl ToComputedValue for SpecifiedValue { - type ComputedValue = computed_value::T; - - #[inline] - fn to_computed_value(&self, context: &Context) -> computed_value::T { - match *self { - SpecifiedValue::Explicit(ref size) => { - computed_value::T::Explicit(computed_value::ExplicitSize { - width: size.width.to_computed_value(context), - height: size.height.to_computed_value(context), - }) - } - SpecifiedValue::Cover => computed_value::T::Cover, - SpecifiedValue::Contain => computed_value::T::Contain, - } - } - #[inline] - fn from_computed_value(computed: &computed_value::T) -> Self { - match *computed { - computed_value::T::Explicit(ref size) => { - SpecifiedValue::Explicit(ExplicitSize { - width: ToComputedValue::from_computed_value(&size.width), - height: ToComputedValue::from_computed_value(&size.height), - }) - } - computed_value::T::Cover => SpecifiedValue::Cover, - computed_value::T::Contain => SpecifiedValue::Contain, - } - } - } - - #[inline] - pub fn get_initial_value() -> computed_value::T { - computed_value::T::Explicit(computed_value::ExplicitSize { - width: computed::LengthOrPercentageOrAuto::Auto, - height: computed::LengthOrPercentageOrAuto::Auto, - }) - } - #[inline] - pub fn get_initial_specified_value() -> SpecifiedValue { - SpecifiedValue::Explicit(ExplicitSize { - width: specified::LengthOrPercentageOrAuto::Auto, - height: specified::LengthOrPercentageOrAuto::Auto, - }) - } - - pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { - if input.try(|input| input.expect_ident_matching("cover")).is_ok() { - return Ok(SpecifiedValue::Cover); - } - - if input.try(|input| input.expect_ident_matching("contain")).is_ok() { - return Ok(SpecifiedValue::Contain); - } - - let width = - try!(specified::LengthOrPercentageOrAuto::parse_non_negative(context, input)); - - let height = input.try(|input| { - specified::LengthOrPercentageOrAuto::parse_non_negative(context, input) - }).unwrap_or(specified::LengthOrPercentageOrAuto::Auto); - - Ok(SpecifiedValue::Explicit(ExplicitSize { - width: width, - height: height, - })) - } - +${helpers.predefined_type("background-size", "BackgroundSize", + initial_value="computed::LengthOrPercentageOrAuto::Auto.into()", + initial_specified_value="specified::LengthOrPercentageOrAuto::Auto.into()", + spec="https://drafts.csswg.org/css-backgrounds/#the-background-size", + vector=True, + animation_value_type="ComputedValue", + extra_prefixes="webkit")} // https://drafts.fxtf.org/compositing/#background-blend-mode ${helpers.single_keyword("background-blend-mode", diff --git a/components/style/values/computed/background.rs b/components/style/values/computed/background.rs new file mode 100644 index 00000000000..cdcab051405 --- /dev/null +++ b/components/style/values/computed/background.rs @@ -0,0 +1,52 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Computed types for CSS values related to backgrounds. + +use properties::animated_properties::{Animatable, RepeatableListAnimatable}; +use values::computed::length::LengthOrPercentageOrAuto; +use values::generics::background::BackgroundSize as GenericBackgroundSize; + +/// A computed value for the `background-size` property. +pub type BackgroundSize = GenericBackgroundSize; + +impl RepeatableListAnimatable for BackgroundSize {} + +impl Animatable for BackgroundSize { + fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64) -> Result { + match (self, other) { + ( + &GenericBackgroundSize::Explicit { width: self_width, height: self_height }, + &GenericBackgroundSize::Explicit { width: other_width, height: other_height }, + ) => { + Ok(GenericBackgroundSize::Explicit { + width: self_width.add_weighted(&other_width, self_portion, other_portion)?, + height: self_height.add_weighted(&other_height, self_portion, other_portion)?, + }) + } + _ => Err(()), + } + } + + #[inline] + fn compute_distance(&self, other: &Self) -> Result { + self.compute_squared_distance(other).map(|sd| sd.sqrt()) + } + + #[inline] + fn compute_squared_distance(&self, other: &Self) -> Result { + match (self, other) { + ( + &GenericBackgroundSize::Explicit { width: self_width, height: self_height }, + &GenericBackgroundSize::Explicit { width: other_width, height: other_height }, + ) => { + Ok( + self_width.compute_squared_distance(&other_width)? + + self_height.compute_squared_distance(&other_height)? + ) + } + _ => Err(()), + } + } +} diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 10b5014483b..64cd2f59353 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -24,6 +24,7 @@ use super::specified; pub use app_units::Au; pub use cssparser::Color as CSSColor; +pub use self::background::BackgroundSize; pub use self::border::{BorderImageSlice, BorderImageWidth, BorderImageWidthSide}; pub use self::image::{Gradient, GradientItem, ImageLayer, LineDirection, Image, ImageRect}; pub use self::rect::LengthOrNumberRect; @@ -38,6 +39,7 @@ pub use self::length::{LengthOrPercentageOrAutoOrContent, LengthOrPercentageOrNo pub use self::length::{MaxLength, MozLength}; pub use self::position::Position; +pub mod background; pub mod basic_shape; pub mod border; pub mod image; diff --git a/components/style/values/generics/background.rs b/components/style/values/generics/background.rs new file mode 100644 index 00000000000..af8cef23a38 --- /dev/null +++ b/components/style/values/generics/background.rs @@ -0,0 +1,52 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Generic types for CSS values related to backgrounds. + +use std::fmt; +use style_traits::ToCss; + +/// A generic value for the `background-size` property. +#[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq, ToComputedValue)] +#[cfg_attr(feature = "servo", derive(HeapSizeOf))] +pub enum BackgroundSize { + /// ` ` + Explicit { + /// Explicit width. + width: LengthOrPercentageOrAuto, + /// Explicit height. + height: LengthOrPercentageOrAuto + }, + /// `cover` + Cover, + /// `contain` + Contain, +} + +impl From for BackgroundSize + where L: Clone, +{ + #[inline] + fn from(value: L) -> Self { + BackgroundSize::Explicit { width: value.clone(), height: value } + } +} + +impl ToCss for BackgroundSize + where L: ToCss +{ + fn to_css(&self, dest: &mut W) -> fmt::Result + where W: fmt::Write + { + match *self { + BackgroundSize::Explicit { ref width, ref height } => { + width.to_css(dest)?; + dest.write_str(" ")?; + height.to_css(dest) + }, + BackgroundSize::Cover => dest.write_str("cover"), + BackgroundSize::Contain => dest.write_str("contain"), + } + } +} diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index 1efc617b0b0..0f36b2c3f1a 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -15,6 +15,7 @@ use super::CustomIdent; pub use self::basic_shape::serialize_radius_values; +pub mod background; pub mod basic_shape; pub mod border; pub mod grid; diff --git a/components/style/values/specified/background.rs b/components/style/values/specified/background.rs new file mode 100644 index 00000000000..3bafe5af22f --- /dev/null +++ b/components/style/values/specified/background.rs @@ -0,0 +1,29 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Specified types for CSS values related to backgrounds. + +use cssparser::Parser; +use parser::{Parse, ParserContext}; +use values::generics::background::BackgroundSize as GenericBackgroundSize; +use values::specified::length::LengthOrPercentageOrAuto; + +/// A specified value for the `background-size` property. +pub type BackgroundSize = GenericBackgroundSize; + +impl Parse for BackgroundSize { + fn parse(context: &ParserContext, input: &mut Parser) -> Result { + if let Ok(width) = input.try(|i| LengthOrPercentageOrAuto::parse_non_negative(context, i)) { + let height = input + .try(|i| LengthOrPercentageOrAuto::parse_non_negative(context, i)) + .unwrap_or(LengthOrPercentageOrAuto::Auto); + return Ok(GenericBackgroundSize::Explicit { width: width, height: height }); + } + match_ignore_ascii_case! { &input.expect_ident()?, + "cover" => Ok(GenericBackgroundSize::Cover), + "contain" => Ok(GenericBackgroundSize::Contain), + _ => Err(()), + } + } +} diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index ec0ac648d87..084486b4dd7 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -30,9 +30,10 @@ use values::specified::calc::CalcNode; #[cfg(feature = "gecko")] pub use self::align::{AlignItems, AlignJustifyContent, AlignJustifySelf, JustifyItems}; -pub use self::rect::LengthOrNumberRect; +pub use self::background::BackgroundSize; pub use self::border::{BorderImageSlice, BorderImageWidth, BorderImageWidthSide}; pub use self::color::Color; +pub use self::rect::LengthOrNumberRect; pub use super::generics::grid::GridLine; pub use self::image::{ColorStop, EndingShape as GradientEndingShape, Gradient}; pub use self::image::{GradientItem, GradientKind, Image, ImageRect, ImageLayer}; @@ -45,6 +46,7 @@ pub use self::position::{Position, PositionComponent}; #[cfg(feature = "gecko")] pub mod align; +pub mod background; pub mod basic_shape; pub mod border; pub mod calc; diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index 1f04847771d..46570f870a0 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -806,6 +806,7 @@ mod shorthand_serialization { use style::properties::longhands::mask_repeat as repeat; use style::properties::longhands::mask_size as size; use style::values::Either; + use style::values::generics::background::BackgroundSize; use style::values::generics::image::Image; use super::*; @@ -852,13 +853,12 @@ mod shorthand_serialization { ) ); - let size = single_vec_variant_value!(size, - size::single_value::SpecifiedValue::Explicit( - size::single_value::ExplicitSize { - width: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(70f32)), - height: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(50f32)) - } - ) + let size = single_vec_variant_value!( + size, + BackgroundSize::Explicit { + width: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(70f32)), + height: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(50f32)), + } ); let repeat = single_vec_keyword_value!(repeat, RepeatX); @@ -903,13 +903,12 @@ mod shorthand_serialization { PositionComponent::Length(LengthOrPercentage::Length(NoCalcLength::from_px(4f32))) ); - let size = single_vec_variant_value!(size, - size::single_value::SpecifiedValue::Explicit( - size::single_value::ExplicitSize { - width: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(70f32)), - height: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(50f32)) - } - ) + let size = single_vec_variant_value!( + size, + BackgroundSize::Explicit { + width: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(70f32)), + height: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(50f32)), + } ); let repeat = single_vec_keyword_value!(repeat, RepeatX); From 6f3c46ca61b0af8b3cccb4770e1ee1827e4b07b6 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 25 May 2017 12:22:49 +0200 Subject: [PATCH 2/3] Remove uses of parse_four_sides and serialize_four_sides --- components/style/properties/helpers.mako.rs | 27 +++++----- .../style/properties/properties.mako.rs | 49 ------------------ .../style/properties/shorthand/border.mako.rs | 19 +++---- .../properties/shorthand/serialize.mako.rs | 51 ------------------- .../style/values/generics/basic_shape.rs | 20 ++++---- .../style/values/specified/basic_shape.rs | 16 +++--- 6 files changed, 41 insertions(+), 141 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index b93f7b89413..0c629df19f5 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -887,35 +887,34 @@ <%call expr="self.shorthand(name, sub_properties=sub_properties, **kwargs)"> #[allow(unused_imports)] use parser::Parse; - use super::parse_four_sides; + use values::generics::rect::Rect; use values::specified; pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result { - let (top, right, bottom, left) = + let rect = Rect::parse_with(context, input, |_c, i| { % if allow_quirks: - try!(parse_four_sides(input, |i| ${parser_function}_quirky(context, i, specified::AllowQuirks::Yes))); + ${parser_function}_quirky(_c, i, specified::AllowQuirks::Yes) % elif needs_context: - try!(parse_four_sides(input, |i| ${parser_function}(context, i))); + ${parser_function}(_c, i) % else: - try!(parse_four_sides(input, ${parser_function})); - let _unused = context; + ${parser_function}(i) % endif + })?; Ok(expanded! { % for side in ["top", "right", "bottom", "left"]: - ${to_rust_ident(sub_property_pattern % side)}: ${side}, + ${to_rust_ident(sub_property_pattern % side)}: rect.${side}, % endfor }) } impl<'a> ToCss for LonghandsToSerialize<'a> { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - super::serialize_four_sides( - dest, - self.${to_rust_ident(sub_property_pattern % 'top')}, - self.${to_rust_ident(sub_property_pattern % 'right')}, - self.${to_rust_ident(sub_property_pattern % 'bottom')}, - self.${to_rust_ident(sub_property_pattern % 'left')} - ) + let rect = Rect::new( + % for side in ["top", "right", "bottom", "left"]: + &self.${to_rust_ident(sub_property_pattern % side)}, + % endfor + ); + rect.to_css(dest) } } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 92429d554dd..118965673a3 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -134,55 +134,6 @@ pub mod shorthands { use parser::{Parse, ParserContext}; use values::specified; - /// Parses a property for four different sides per CSS syntax. - /// - /// * Zero or more than four values is invalid. - /// * One value sets them all - /// * Two values set (top, bottom) and (left, right) - /// * Three values set top, (left, right) and bottom - /// * Four values set them in order - /// - /// returns the values in (top, right, bottom, left) order. - pub fn parse_four_sides(input: &mut Parser, parse_one: F) -> Result<(T, T, T, T), ()> - where F: Fn(&mut Parser) -> Result, - T: Clone, - { - let top = try!(parse_one(input)); - let right; - let bottom; - let left; - match input.try(|i| parse_one(i)) { - Err(()) => { - right = top.clone(); - bottom = top.clone(); - left = top.clone(); - } - Ok(value) => { - right = value; - match input.try(|i| parse_one(i)) { - Err(()) => { - bottom = top.clone(); - left = right.clone(); - } - Ok(value) => { - bottom = value; - match input.try(|i| parse_one(i)) { - Err(()) => { - left = right.clone(); - } - Ok(value) => { - left = value; - } - } - - } - } - - } - } - Ok((top, right, bottom, left)) - } - <%include file="/shorthand/serialize.mako.rs" /> <%include file="/shorthand/background.mako.rs" /> <%include file="/shorthand/border.mako.rs" /> diff --git a/components/style/properties/shorthand/border.mako.rs b/components/style/properties/shorthand/border.mako.rs index 772105fae25..8e9aff17fc9 100644 --- a/components/style/properties/shorthand/border.mako.rs +++ b/components/style/properties/shorthand/border.mako.rs @@ -17,27 +17,28 @@ ${helpers.four_sides_shorthand("border-style", "border-%s-style", ' '.join('border-%s-width' % side for side in PHYSICAL_SIDES)}" spec="https://drafts.csswg.org/css-backgrounds/#border-width"> - use super::parse_four_sides; + use values::generics::rect::Rect; use values::specified::{AllowQuirks, BorderWidth}; pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result { - let (top, right, bottom, left) = try!(parse_four_sides(input, |i| { + let rect = Rect::parse_with(context, input, |_, i| { BorderWidth::parse_quirky(context, i, AllowQuirks::Yes) - })); + })?; Ok(expanded! { % for side in PHYSICAL_SIDES: - ${to_rust_ident('border-%s-width' % side)}: ${side}, + ${to_rust_ident('border-%s-width' % side)}: rect.${side}, % endfor }) } impl<'a> ToCss for LonghandsToSerialize<'a> { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - % for side in PHYSICAL_SIDES: - let ${side} = self.border_${side}_width.clone(); - % endfor - - super::serialize_four_sides(dest, &top, &right, &bottom, &left) + let rect = Rect::new( + % for side in PHYSICAL_SIDES: + &self.border_${side}_width, + % endfor + ); + rect.to_css(dest) } } diff --git a/components/style/properties/shorthand/serialize.mako.rs b/components/style/properties/shorthand/serialize.mako.rs index f990ca81df3..f8c1ef9bc3e 100644 --- a/components/style/properties/shorthand/serialize.mako.rs +++ b/components/style/properties/shorthand/serialize.mako.rs @@ -6,57 +6,6 @@ use style_traits::ToCss; use values::specified::{BorderStyle, Color, CSSColor}; use std::fmt; -#[allow(missing_docs)] -pub fn serialize_four_sides(dest: &mut W, - top: &I, - right: &I, - bottom: &I, - left: &I) - -> fmt::Result - where W: fmt::Write, - I: ToCss + PartialEq, -{ - - if left == right { - let horizontal_value = left; - - if top == bottom { - let vertical_value = top; - - if horizontal_value == vertical_value { - let single_value = horizontal_value; - try!(single_value.to_css(dest)); - } else { - try!(vertical_value.to_css(dest)); - try!(write!(dest, " ")); - - try!(horizontal_value.to_css(dest)); - } - } else { - try!(top.to_css(dest)); - try!(write!(dest, " ")); - - try!(horizontal_value.to_css(dest)); - try!(write!(dest, " ")); - - try!(bottom.to_css(dest)); - } - } else { - try!(top.to_css(dest)); - try!(write!(dest, " ")); - - try!(right.to_css(dest)); - try!(write!(dest, " ")); - - try!(bottom.to_css(dest)); - try!(write!(dest, " ")); - - try!(left.to_css(dest)); - } - - Ok(()) -} - fn serialize_directional_border(dest: &mut W, width: &I, style: &BorderStyle, diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index f5b87ec1dd1..40cebf945ff 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -6,12 +6,12 @@ //! types that are generic over their `ToCss` implementations. use euclid::size::Size2D; -use properties::shorthands::serialize_four_sides; use std::fmt; use style_traits::{HasViewportPercentage, ToCss}; use values::computed::ComputedValueAsSpecified; use values::generics::BorderRadiusSize; use values::generics::position::Position; +use values::generics::rect::Rect; use values::specified::url::SpecifiedUrl; /// A clipping shape, for `clip-path`. @@ -224,17 +224,17 @@ pub fn serialize_radius_values(dest: &mut W, top_left: &Size2D, bottom_left: &Size2D) -> fmt::Result where L: ToCss + PartialEq, W: fmt::Write { - if top_left.width == top_left.height && top_right.width == top_right.height && - bottom_right.width == bottom_right.height && bottom_left.width == bottom_left.height { - serialize_four_sides(dest, &top_left.width, &top_right.width, - &bottom_right.width, &bottom_left.width) - } else { - serialize_four_sides(dest, &top_left.width, &top_right.width, - &bottom_right.width, &bottom_left.width)?; + Rect::new(&top_left.width, &top_right.width, &bottom_right.width, &bottom_left.width).to_css(dest)?; + if + top_left.width != top_left.height || + top_right.width != top_right.height || + bottom_right.width != bottom_right.height || + bottom_left.width != bottom_left.height + { dest.write_str(" / ")?; - serialize_four_sides(dest, &top_left.height, &top_right.height, - &bottom_right.height, &bottom_left.height) + Rect::new(&top_left.height, &top_right.height, &bottom_right.height, &bottom_left.height).to_css(dest)?; } + Ok(()) } impl Default for ShapeRadius { diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index 02729e0e635..a33d9eac57e 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -9,7 +9,6 @@ use cssparser::Parser; use parser::{Parse, ParserContext}; -use properties::shorthands::parse_four_sides; use std::borrow::Cow; use std::fmt; use style_traits::ToCss; @@ -20,6 +19,7 @@ use values::generics::basic_shape::{FillRule, BasicShape as GenericBasicShape}; use values::generics::basic_shape::{FloatAreaShape as GenericFloatAreaShape, InsetRect as GenericInsetRect}; use values::generics::basic_shape::{GeometryBox, ShapeBox, ShapeSource}; use values::generics::basic_shape::{Polygon as GenericPolygon, ShapeRadius as GenericShapeRadius}; +use values::generics::rect::Rect; use values::specified::{LengthOrPercentage, Percentage}; use values::specified::position::{HorizontalPosition, Position, PositionComponent, Side, VerticalPosition}; use values::specified::url::SpecifiedUrl; @@ -127,18 +127,18 @@ impl Parse for InsetRect { impl InsetRect { /// Parse the inner function arguments of `inset()` pub fn parse_function_arguments(context: &ParserContext, input: &mut Parser) -> Result { - let (t, r, b, l) = parse_four_sides(input, |i| LengthOrPercentage::parse(context, i))?; - let rect = if input.try(|i| i.expect_ident_matching("round")).is_ok() { + let rect = Rect::parse_with(context, input, LengthOrPercentage::parse)?; + let round_rect = if input.try(|i| i.expect_ident_matching("round")).is_ok() { Some(BorderRadius::parse(context, input)?) } else { None }; Ok(GenericInsetRect { - top: t, - right: r, - bottom: b, - left: l, - round: rect, + top: rect.top, + right: rect.right, + bottom: rect.bottom, + left: rect.left, + round: round_rect, }) } } From 150c9df2463ffaea3b13835b0a9a17e21dbf3eff Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 25 May 2017 13:01:06 +0200 Subject: [PATCH 3/3] Use Rect in InsetRect --- components/style/gecko/conversions.rs | 12 ++++++---- components/style/properties/gecko.mako.rs | 12 +++++----- .../style/properties/shorthand/border.mako.rs | 6 ++--- .../style/values/generics/basic_shape.rs | 19 ++++----------- .../style/values/specified/basic_shape.rs | 9 +++----- tests/unit/style/parsing/basic_shape.rs | 23 ++++++++----------- 6 files changed, 34 insertions(+), 47 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 6584d4cbdc8..392ec9a1012 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -369,6 +369,7 @@ pub mod basic_shape { use values::generics::basic_shape::{BasicShape as GenericBasicShape, InsetRect, Polygon}; use values::generics::basic_shape::{Circle, Ellipse, FillRule}; use values::generics::basic_shape::{GeometryBox, ShapeBox}; + use values::generics::rect::Rect; // using Borrow so that we can have a non-moving .into() impl> From for BasicShape { @@ -381,11 +382,14 @@ pub mod basic_shape { let b = LengthOrPercentage::from_gecko_style_coord(&other.mCoordinates[2]); let l = LengthOrPercentage::from_gecko_style_coord(&other.mCoordinates[3]); let round = (&other.mRadius).into(); + let rect = Rect::new( + t.expect("inset() offset should be a length, percentage, or calc value"), + r.expect("inset() offset should be a length, percentage, or calc value"), + b.expect("inset() offset should be a length, percentage, or calc value"), + l.expect("inset() offset should be a length, percentage, or calc value"), + ); GenericBasicShape::Inset(InsetRect { - top: t.expect("inset() offset should be a length, percentage, or calc value"), - right: r.expect("inset() offset should be a length, percentage, or calc value"), - bottom: b.expect("inset() offset should be a length, percentage, or calc value"), - left: l.expect("inset() offset should be a length, percentage, or calc value"), + rect: rect, round: Some(round), }) } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 0d5d33c8dcc..39b9bd54cc4 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3914,7 +3914,7 @@ fn static_assert() { } } match servo_shape { - BasicShape::Inset(rect) => { + BasicShape::Inset(inset) => { let mut shape = init_shape(${ident}, StyleBasicShapeType::Inset); unsafe { shape.mCoordinates.set_len(4) }; @@ -3926,15 +3926,15 @@ fn static_assert() { // the garbage data without // attempting to clean up. shape.mCoordinates[0].leaky_set_null(); - rect.top.to_gecko_style_coord(&mut shape.mCoordinates[0]); + inset.rect.top.to_gecko_style_coord(&mut shape.mCoordinates[0]); shape.mCoordinates[1].leaky_set_null(); - rect.right.to_gecko_style_coord(&mut shape.mCoordinates[1]); + inset.rect.right.to_gecko_style_coord(&mut shape.mCoordinates[1]); shape.mCoordinates[2].leaky_set_null(); - rect.bottom.to_gecko_style_coord(&mut shape.mCoordinates[2]); + inset.rect.bottom.to_gecko_style_coord(&mut shape.mCoordinates[2]); shape.mCoordinates[3].leaky_set_null(); - rect.left.to_gecko_style_coord(&mut shape.mCoordinates[3]); + inset.rect.left.to_gecko_style_coord(&mut shape.mCoordinates[3]); - set_corners_from_radius(rect.round, &mut shape.mRadius); + set_corners_from_radius(inset.round, &mut shape.mRadius); } BasicShape::Circle(circ) => { let mut shape = init_shape(${ident}, StyleBasicShapeType::Circle); diff --git a/components/style/properties/shorthand/border.mako.rs b/components/style/properties/shorthand/border.mako.rs index 8e9aff17fc9..67e807b824b 100644 --- a/components/style/properties/shorthand/border.mako.rs +++ b/components/style/properties/shorthand/border.mako.rs @@ -33,11 +33,11 @@ ${helpers.four_sides_shorthand("border-style", "border-%s-style", impl<'a> ToCss for LonghandsToSerialize<'a> { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - let rect = Rect::new( + let rect = Rect { % for side in PHYSICAL_SIDES: - &self.border_${side}_width, + ${side}: &self.border_${side}_width, % endfor - ); + }; rect.to_css(dest) } } diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index 40cebf945ff..688c9616d01 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -67,10 +67,7 @@ pub enum BasicShape { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[derive(Clone, Debug, PartialEq, ToComputedValue)] pub struct InsetRect { - pub top: LengthOrPercentage, - pub right: LengthOrPercentage, - pub bottom: LengthOrPercentage, - pub left: LengthOrPercentage, + pub rect: Rect, pub round: Option>, } @@ -190,22 +187,16 @@ impl ToCss for BasicShape } } -impl ToCss for InsetRect { - // XXXManishearth We should try to reduce the number of values printed here +impl ToCss for InsetRect + where L: ToCss + PartialEq +{ fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { dest.write_str("inset(")?; - self.top.to_css(dest)?; - dest.write_str(" ")?; - self.right.to_css(dest)?; - dest.write_str(" ")?; - self.bottom.to_css(dest)?; - dest.write_str(" ")?; - self.left.to_css(dest)?; + self.rect.to_css(dest)?; if let Some(ref radius) = self.round { dest.write_str(" round ")?; radius.to_css(dest)?; } - dest.write_str(")") } } diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index a33d9eac57e..5125ff9febd 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -128,17 +128,14 @@ impl InsetRect { /// Parse the inner function arguments of `inset()` pub fn parse_function_arguments(context: &ParserContext, input: &mut Parser) -> Result { let rect = Rect::parse_with(context, input, LengthOrPercentage::parse)?; - let round_rect = if input.try(|i| i.expect_ident_matching("round")).is_ok() { + let round = if input.try(|i| i.expect_ident_matching("round")).is_ok() { Some(BorderRadius::parse(context, input)?) } else { None }; Ok(GenericInsetRect { - top: rect.top, - right: rect.right, - bottom: rect.bottom, - left: rect.left, - round: round_rect, + rect: rect, + round: round, }) } } diff --git a/tests/unit/style/parsing/basic_shape.rs b/tests/unit/style/parsing/basic_shape.rs index 3cef9ca1803..7364a24e923 100644 --- a/tests/unit/style/parsing/basic_shape.rs +++ b/tests/unit/style/parsing/basic_shape.rs @@ -13,7 +13,10 @@ macro_rules! assert_roundtrip_basicshape { ($fun:expr, $input:expr, $output:expr) => { assert_roundtrip_with_context!($fun, $input, $output); assert_roundtrip_with_context!(BasicShape::parse, $input, $output); - } + }; + ($fun:expr, $input:expr) => { + assert_roundtrip_basicshape!($fun, $input, $input); + }; } macro_rules! assert_border_radius_values { @@ -35,20 +38,12 @@ macro_rules! assert_border_radius_values { #[test] fn test_inset() { - // these are actually wrong, we should be serializing to the minimum possible result - // the advantage of being wrong is that the roundtrip test actually suffices - // for testing the intermediate state - assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px)", "inset(10px 10px 10px 10px)"); - assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px 20%)", "inset(10px 20% 10px 20%)"); + assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px)"); + assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px 20%)"); - assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px)", - "inset(10px 10px 10px 10px round 10px)"); - assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px 20px 30px 40px)", - "inset(10px 10px 10px 10px round 10px 20px 30px 40px)"); - assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px 10px 10px 10px round 10px 20px 30px 40px \ - / 1px 2px 3px 4px)", - "inset(10px 10px 10px 10px round 10px 20px 30px 40px \ - / 1px 2px 3px 4px)"); + assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px)"); + assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px 20px 30px 40px)"); + assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px 20px 30px 40px / 1px 2px 3px 4px)"); } #[test]