From f0f3eb31949a98d3ceabac545b531d8c278dc31f Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 19 Dec 2018 19:08:08 +0000 Subject: [PATCH] style: Clamp to non-negative value after doing interpolation for circle(), ellipse(), and inset(). Replace LengthOrPercentage with NonNegativeLengthOrPercentage on ShapeRadius, Circle, Ellipse. And derive ToAnimatedValue for ShapeSource and its related types, so we clamp its interpolated results into non-negative values. (i.e. The radius of circle()/ellipse() and the border-radius of inset().) Note: We may get negative values when using a negative easing function, so the clamp is necessary to avoid the incorrect result or any undefined behavior. Differential Revision: https://phabricator.services.mozilla.com/D14654 --- components/style/gecko/values.rs | 4 +- .../style/properties/longhands/box.mako.rs | 2 +- .../style/properties/longhands/svg.mako.rs | 2 +- components/style/values/animated/mod.rs | 15 ++++ .../style/values/computed/basic_shape.rs | 16 ++-- .../style/values/generics/basic_shape.rs | 78 +++++++++++++++---- components/style/values/generics/border.rs | 1 + components/style/values/generics/position.rs | 1 + components/style/values/generics/rect.rs | 1 + .../style/values/specified/basic_shape.rs | 17 ++-- 10 files changed, 110 insertions(+), 27 deletions(-) diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index 8006d50925f..b22f68cd05d 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -286,7 +286,9 @@ impl GeckoStyleCoordConvertible for ComputedShapeRadius { None } }, - _ => LengthOrPercentage::from_gecko_style_coord(coord).map(ShapeRadius::Length), + _ => { + GeckoStyleCoordConvertible::from_gecko_style_coord(coord).map(ShapeRadius::Length) + }, } } } diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index e8946c3206e..7eb6ff0028d 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -627,7 +627,7 @@ ${helpers.predefined_type( "generics::basic_shape::ShapeSource::None", products="gecko", boxed=True, - animation_value_type="ComputedValue", + animation_value_type="basic_shape::FloatAreaShape", flags="APPLIES_TO_FIRST_LETTER", 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 d6e76f6f1f6..43a8952d86e 100644 --- a/components/style/properties/longhands/svg.mako.rs +++ b/components/style/properties/longhands/svg.mako.rs @@ -88,7 +88,7 @@ ${helpers.predefined_type( "generics::basic_shape::ShapeSource::None", products="gecko", boxed=True, - animation_value_type="ComputedValue", + animation_value_type="basic_shape::ClippingShape", flags="CREATES_STACKING_CONTEXT", spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path", )} diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index cf5ca94bae6..bd0a7af1f95 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -12,7 +12,9 @@ use crate::properties::PropertyId; use crate::values::computed::length::CalcLengthOrPercentage; use crate::values::computed::url::ComputedUrl; use crate::values::computed::Angle as ComputedAngle; +use crate::values::computed::Image; use crate::values::CSSFloat; +use crate::values::specified::SVGPathData; use app_units::Au; use euclid::{Point2D, Size2D}; use smallvec::SmallVec; @@ -338,6 +340,19 @@ trivial_to_animated_value!(ComputedAngle); trivial_to_animated_value!(ComputedUrl); trivial_to_animated_value!(bool); trivial_to_animated_value!(f32); +// Note: This implementation is for ToAnimatedValue of ShapeSource. +// +// SVGPathData uses Box<[T]>. If we want to derive ToAnimatedValue for all the +// types, we have to do "impl ToAnimatedValue for Box<[T]>" first. +// However, the general version of "impl ToAnimatedValue for Box<[T]>" needs to +// clone |T| and convert it into |T::AnimatedValue|. However, for SVGPathData +// that is unnecessary--moving |T| is sufficient. So here, we implement this +// trait manually. +trivial_to_animated_value!(SVGPathData); +// FIXME: Bug 1514342, Image is not animatable, but we still need to implement +// this to avoid adding this derive to generic::Image and all its arms. We can +// drop this after landing Bug 1514342. +trivial_to_animated_value!(Image); impl ToAnimatedZero for Au { #[inline] diff --git a/components/style/values/computed/basic_shape.rs b/components/style/values/computed/basic_shape.rs index 383c7b39bfc..f808e0bf9f5 100644 --- a/components/style/values/computed/basic_shape.rs +++ b/components/style/values/computed/basic_shape.rs @@ -23,20 +23,26 @@ pub type ClippingShape = generic::ClippingShape; pub type FloatAreaShape = generic::FloatAreaShape; /// A computed basic shape. -pub type BasicShape = - generic::BasicShape; +pub type BasicShape = generic::BasicShape< + LengthOrPercentage, + LengthOrPercentage, + LengthOrPercentage, + NonNegativeLengthOrPercentage, +>; /// The computed value of `inset()` pub type InsetRect = generic::InsetRect; /// A computed circle. -pub type Circle = generic::Circle; +pub type Circle = + generic::Circle; /// A computed ellipse. -pub type Ellipse = generic::Ellipse; +pub type Ellipse = + generic::Ellipse; /// The computed value of `ShapeRadius` -pub type ShapeRadius = generic::ShapeRadius; +pub type ShapeRadius = generic::ShapeRadius; impl ToCss for Circle { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index 9390de653d9..bfe709ceb4b 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -20,7 +20,16 @@ pub type ClippingShape = ShapeSource #[allow(missing_docs)] #[derive( - Animate, Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss, + Animate, + Clone, + Copy, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToComputedValue, + ToCss, )] pub enum GeometryBox { FillBox, @@ -45,6 +54,7 @@ pub type FloatAreaShape = ShapeSource { #[animation(error)] @@ -82,13 +100,14 @@ pub enum ShapeSource { MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToComputedValue, ToCss, )] pub enum BasicShape { Inset(#[css(field_bound)] InsetRect), - Circle(#[css(field_bound)] Circle), - Ellipse(#[css(field_bound)] Ellipse), + Circle(#[css(field_bound)] Circle), + Ellipse(#[css(field_bound)] Ellipse), Polygon(Polygon), } @@ -103,6 +122,7 @@ pub enum BasicShape { MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToComputedValue, )] pub struct InsetRect { @@ -122,11 +142,12 @@ pub struct InsetRect { MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToComputedValue, )] -pub struct Circle { +pub struct Circle { pub position: Position, - pub radius: ShapeRadius, + pub radius: ShapeRadius, } /// @@ -141,12 +162,13 @@ pub struct Circle { MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToComputedValue, )] -pub struct Ellipse { +pub struct Ellipse { pub position: Position, - pub semiaxis_x: ShapeRadius, - pub semiaxis_y: ShapeRadius, + pub semiaxis_x: ShapeRadius, + pub semiaxis_y: ShapeRadius, } /// @@ -160,11 +182,12 @@ pub struct Ellipse { MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToComputedValue, ToCss, )] -pub enum ShapeRadius { - Length(LengthOrPercentage), +pub enum ShapeRadius { + Length(NonNegativeLengthOrPercentage), #[animation(error)] ClosestSide, #[animation(error)] @@ -175,7 +198,16 @@ pub enum ShapeRadius { /// /// #[css(comma, function)] -#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)] +#[derive( + Clone, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToComputedValue, + ToCss, +)] pub struct Polygon { /// The filling rule for a polygon. #[css(skip_if = "fill_is_default")] @@ -186,7 +218,16 @@ pub struct Polygon { } /// Coordinates for Polygon. -#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)] +#[derive( + Clone, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToComputedValue, + ToCss, +)] pub struct PolygonCoord(pub LengthOrPercentage, pub LengthOrPercentage); // https://drafts.csswg.org/css-shapes/#typedef-fill-rule @@ -204,6 +245,7 @@ pub struct PolygonCoord(pub LengthOrPercentage, pub LengthOr Parse, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToComputedValue, ToCss, )] @@ -218,7 +260,15 @@ pub enum FillRule { /// https://drafts.csswg.org/css-shapes-2/#funcdef-path #[css(comma)] #[derive( - Animate, Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss, + Animate, + Clone, + Debug, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToComputedValue, + ToCss, )] pub struct Path { /// The filling rule for the svg path. diff --git a/components/style/values/generics/border.rs b/components/style/values/generics/border.rs index eb651377e0d..20274816615 100644 --- a/components/style/values/generics/border.rs +++ b/components/style/values/generics/border.rs @@ -95,6 +95,7 @@ impl BorderSpacing { MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToComputedValue, )] pub struct BorderRadius { diff --git a/components/style/values/generics/position.rs b/components/style/values/generics/position.rs index 0bde0067def..07612913c0a 100644 --- a/components/style/values/generics/position.rs +++ b/components/style/values/generics/position.rs @@ -15,6 +15,7 @@ MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToAnimatedZero, ToComputedValue, )] diff --git a/components/style/values/generics/rect.rs b/components/style/values/generics/rect.rs index dfffb3a4834..985a9c4a320 100644 --- a/components/style/values/generics/rect.rs +++ b/components/style/values/generics/rect.rs @@ -20,6 +20,7 @@ use style_traits::{CssWriter, ParseError, ToCss}; MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToAnimatedValue, ToComputedValue, )] pub struct Rect(pub T, pub T, pub T, pub T); diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index 2f1f510372c..20c7ad6eeb4 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -32,19 +32,26 @@ pub type ClippingShape = generic::ClippingShape; pub type FloatAreaShape = generic::FloatAreaShape; /// A specified basic shape. -pub type BasicShape = generic::BasicShape; +pub type BasicShape = generic::BasicShape< + HorizontalPosition, + VerticalPosition, + LengthOrPercentage, + NonNegativeLengthOrPercentage, +>; /// The specified value of `inset()` pub type InsetRect = generic::InsetRect; /// A specified circle. -pub type Circle = generic::Circle; +pub type Circle = + generic::Circle; /// A specified ellipse. -pub type Ellipse = generic::Ellipse; +pub type Ellipse = + generic::Ellipse; /// The specified value of `ShapeRadius` -pub type ShapeRadius = generic::ShapeRadius; +pub type ShapeRadius = generic::ShapeRadius; /// The specified value of `Polygon` pub type Polygon = generic::Polygon; @@ -309,7 +316,7 @@ impl Parse for ShapeRadius { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - if let Ok(lop) = input.try(|i| LengthOrPercentage::parse_non_negative(context, i)) { + if let Ok(lop) = input.try(|i| NonNegativeLengthOrPercentage::parse(context, i)) { return Ok(generic::ShapeRadius::Length(lop)); }