Bug 1512883 - Part 1: Clamp to non-negative value after doing interpolation for circle(), ellipse(), and inset(). r=emilio,birtles

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

--HG--
extra : moz-landing-system : lando

servo/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)
+            },
         }
     }
 }

servo/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",
 )}

servo/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",
 )}

servo/components/style/values/animated/mod.rs

@@ -13,7 +13,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 euclid::{Point2D, Size2D};
 use smallvec::SmallVec;
 use std::cmp;
@@ -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]

servo/components/style/values/computed/basic_shape.rs

@@ -23,20 +23,26 @@ pub type ClippingShape = generic::ClippingShape<BasicShape, ComputedUrl>;
 pub type FloatAreaShape = generic::FloatAreaShape<BasicShape, Image>;
 
 /// A computed basic shape.
-pub type BasicShape =
-    generic::BasicShape<LengthOrPercentage, LengthOrPercentage, LengthOrPercentage, NonNegativeLengthOrPercentage>;
+pub type BasicShape = generic::BasicShape<
+    LengthOrPercentage,
+    LengthOrPercentage,
+    LengthOrPercentage,
+    NonNegativeLengthOrPercentage,
+>;
 
 /// The computed value of `inset()`
 pub type InsetRect = generic::InsetRect<LengthOrPercentage, NonNegativeLengthOrPercentage>;
 
 /// A computed circle.
-pub type Circle = generic::Circle<LengthOrPercentage, LengthOrPercentage, LengthOrPercentage>;
+pub type Circle =
+    generic::Circle<LengthOrPercentage, LengthOrPercentage, NonNegativeLengthOrPercentage>;
 
 /// A computed ellipse.
-pub type Ellipse = generic::Ellipse<LengthOrPercentage, LengthOrPercentage, LengthOrPercentage>;
+pub type Ellipse =
+    generic::Ellipse<LengthOrPercentage, LengthOrPercentage, NonNegativeLengthOrPercentage>;
 
 /// The computed value of `ShapeRadius`
-pub type ShapeRadius = generic::ShapeRadius<LengthOrPercentage>;
+pub type ShapeRadius = generic::ShapeRadius<NonNegativeLengthOrPercentage>;
 
 impl ToCss for Circle {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result

servo/components/style/values/generics/basic_shape.rs

@@ -20,7 +20,16 @@ pub type ClippingShape<BasicShape, Url> = ShapeSource<BasicShape, GeometryBox, U
 /// <https://drafts.fxtf.org/css-masking-1/#typedef-geometry-box>
 #[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<BasicShape, Image> = ShapeSource<BasicShape, ShapeBox, I
     Parse,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
     ToCss,
 )]
@@ -59,7 +69,15 @@ pub enum ShapeBox {
 #[allow(missing_docs)]
 #[animation(no_bound(ImageOrUrl))]
 #[derive(
-    Animate, Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss,
+    Animate,
+    Clone,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToAnimatedValue,
+    ToComputedValue,
+    ToCss,
 )]
 pub enum ShapeSource<BasicShape, ReferenceBox, ImageOrUrl> {
     #[animation(error)]
@@ -82,13 +100,14 @@ pub enum ShapeSource<BasicShape, ReferenceBox, ImageOrUrl> {
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
     ToCss,
 )]
 pub enum BasicShape<H, V, LengthOrPercentage, NonNegativeLengthOrPercentage> {
     Inset(#[css(field_bound)] InsetRect<LengthOrPercentage, NonNegativeLengthOrPercentage>),
-    Circle(#[css(field_bound)] Circle<H, V, LengthOrPercentage>),
-    Ellipse(#[css(field_bound)] Ellipse<H, V, LengthOrPercentage>),
+    Circle(#[css(field_bound)] Circle<H, V, NonNegativeLengthOrPercentage>),
+    Ellipse(#[css(field_bound)] Ellipse<H, V, NonNegativeLengthOrPercentage>),
     Polygon(Polygon<LengthOrPercentage>),
 }
 
@@ -103,6 +122,7 @@ pub enum BasicShape<H, V, LengthOrPercentage, NonNegativeLengthOrPercentage> {
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
 )]
 pub struct InsetRect<LengthOrPercentage, NonNegativeLengthOrPercentage> {
@@ -122,11 +142,12 @@ pub struct InsetRect<LengthOrPercentage, NonNegativeLengthOrPercentage> {
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
 )]
-pub struct Circle<H, V, LengthOrPercentage> {
+pub struct Circle<H, V, NonNegativeLengthOrPercentage> {
     pub position: Position<H, V>,
-    pub radius: ShapeRadius<LengthOrPercentage>,
+    pub radius: ShapeRadius<NonNegativeLengthOrPercentage>,
 }
 
 /// <https://drafts.csswg.org/css-shapes/#funcdef-ellipse>
@@ -141,12 +162,13 @@ pub struct Circle<H, V, LengthOrPercentage> {
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
 )]
-pub struct Ellipse<H, V, LengthOrPercentage> {
+pub struct Ellipse<H, V, NonNegativeLengthOrPercentage> {
     pub position: Position<H, V>,
-    pub semiaxis_x: ShapeRadius<LengthOrPercentage>,
-    pub semiaxis_y: ShapeRadius<LengthOrPercentage>,
+    pub semiaxis_x: ShapeRadius<NonNegativeLengthOrPercentage>,
+    pub semiaxis_y: ShapeRadius<NonNegativeLengthOrPercentage>,
 }
 
 /// <https://drafts.csswg.org/css-shapes/#typedef-shape-radius>
@@ -160,11 +182,12 @@ pub struct Ellipse<H, V, LengthOrPercentage> {
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
     ToCss,
 )]
-pub enum ShapeRadius<LengthOrPercentage> {
-    Length(LengthOrPercentage),
+pub enum ShapeRadius<NonNegativeLengthOrPercentage> {
+    Length(NonNegativeLengthOrPercentage),
     #[animation(error)]
     ClosestSide,
     #[animation(error)]
@@ -175,7 +198,16 @@ pub enum ShapeRadius<LengthOrPercentage> {
 ///
 /// <https://drafts.csswg.org/css-shapes/#funcdef-polygon>
 #[css(comma, function)]
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)]
+#[derive(
+    Clone,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToAnimatedValue,
+    ToComputedValue,
+    ToCss,
+)]
 pub struct Polygon<LengthOrPercentage> {
     /// The filling rule for a polygon.
     #[css(skip_if = "fill_is_default")]
@@ -186,7 +218,16 @@ pub struct Polygon<LengthOrPercentage> {
 }
 
 /// Coordinates for Polygon.
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)]
+#[derive(
+    Clone,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    SpecifiedValueInfo,
+    ToAnimatedValue,
+    ToComputedValue,
+    ToCss,
+)]
 pub struct PolygonCoord<LengthOrPercentage>(pub LengthOrPercentage, pub LengthOrPercentage);
 
 // https://drafts.csswg.org/css-shapes/#typedef-fill-rule
@@ -204,6 +245,7 @@ pub struct PolygonCoord<LengthOrPercentage>(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.

servo/components/style/values/generics/border.rs

@@ -95,6 +95,7 @@ impl<L> BorderSpacing<L> {
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
 )]
 pub struct BorderRadius<LengthOrPercentage> {

servo/components/style/values/generics/position.rs

@@ -15,6 +15,7 @@
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToAnimatedZero,
     ToComputedValue,
 )]

servo/components/style/values/generics/rect.rs

@@ -20,6 +20,7 @@ use style_traits::{CssWriter, ParseError, ToCss};
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
 )]
 pub struct Rect<T>(pub T, pub T, pub T, pub T);

servo/components/style/values/specified/basic_shape.rs

@@ -32,19 +32,26 @@ pub type ClippingShape = generic::ClippingShape<BasicShape, SpecifiedUrl>;
 pub type FloatAreaShape = generic::FloatAreaShape<BasicShape, Image>;
 
 /// A specified basic shape.
-pub type BasicShape = generic::BasicShape<HorizontalPosition, VerticalPosition, LengthOrPercentage, NonNegativeLengthOrPercentage>;
+pub type BasicShape = generic::BasicShape<
+    HorizontalPosition,
+    VerticalPosition,
+    LengthOrPercentage,
+    NonNegativeLengthOrPercentage,
+>;
 
 /// The specified value of `inset()`
 pub type InsetRect = generic::InsetRect<LengthOrPercentage, NonNegativeLengthOrPercentage>;
 
 /// A specified circle.
-pub type Circle = generic::Circle<HorizontalPosition, VerticalPosition, LengthOrPercentage>;
+pub type Circle =
+    generic::Circle<HorizontalPosition, VerticalPosition, NonNegativeLengthOrPercentage>;
 
 /// A specified ellipse.
-pub type Ellipse = generic::Ellipse<HorizontalPosition, VerticalPosition, LengthOrPercentage>;
+pub type Ellipse =
+    generic::Ellipse<HorizontalPosition, VerticalPosition, NonNegativeLengthOrPercentage>;
 
 /// The specified value of `ShapeRadius`
-pub type ShapeRadius = generic::ShapeRadius<LengthOrPercentage>;
+pub type ShapeRadius = generic::ShapeRadius<NonNegativeLengthOrPercentage>;
 
 /// The specified value of `Polygon`
 pub type Polygon = generic::Polygon<LengthOrPercentage>;
@@ -309,7 +316,7 @@ impl Parse for ShapeRadius {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        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));
         }