Bug 1512883 - Part 1: Clamp to non-negative value after doing interpolation for circle(), ellipse(), and inset(). r=emilio,birtles
authorBoris Chiou <boris.chiou@gmail.com>
Wed, 19 Dec 2018 19:08:08 +0000
changeset 508496 70912a8087d7ddf36dec65b83e07a2e703f3dd44
parent 508495 58305b0665c963cb5a38a77da062ed83f5f40c15
child 508497 c1b46f5fc73bf34193b70d8aba86a1d6f3597357
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio, birtles
bugs1512883
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
servo/components/style/gecko/values.rs
servo/components/style/properties/longhands/box.mako.rs
servo/components/style/properties/longhands/svg.mako.rs
servo/components/style/values/animated/mod.rs
servo/components/style/values/computed/basic_shape.rs
servo/components/style/values/generics/basic_shape.rs
servo/components/style/values/generics/border.rs
servo/components/style/values/generics/position.rs
servo/components/style/values/generics/rect.rs
servo/components/style/values/specified/basic_shape.rs
--- a/servo/components/style/gecko/values.rs
+++ b/servo/components/style/gecko/values.rs
@@ -281,17 +281,19 @@ impl GeckoStyleCoordConvertible for Comp
                 if v == StyleShapeRadius::ClosestSide as u32 {
                     Some(ShapeRadius::ClosestSide)
                 } else if v == StyleShapeRadius::FarthestSide as u32 {
                     Some(ShapeRadius::FarthestSide)
                 } else {
                     None
                 }
             },
-            _ => LengthOrPercentage::from_gecko_style_coord(coord).map(ShapeRadius::Length),
+            _ => {
+                GeckoStyleCoordConvertible::from_gecko_style_coord(coord).map(ShapeRadius::Length)
+            },
         }
     }
 }
 
 impl<T: GeckoStyleCoordConvertible> GeckoStyleCoordConvertible for Option<T> {
     fn to_gecko_style_coord<U: CoordDataMut>(&self, coord: &mut U) {
         if let Some(ref me) = *self {
             me.to_gecko_style_coord(coord);
--- a/servo/components/style/properties/longhands/box.mako.rs
+++ b/servo/components/style/properties/longhands/box.mako.rs
@@ -622,17 +622,17 @@
 )}
 
 ${helpers.predefined_type(
     "shape-outside",
     "basic_shape::FloatAreaShape",
     "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",
 )}
 
 ${helpers.predefined_type(
     "touch-action",
     "TouchAction",
     "computed::TouchAction::auto()",
--- a/servo/components/style/properties/longhands/svg.mako.rs
+++ b/servo/components/style/properties/longhands/svg.mako.rs
@@ -83,17 +83,17 @@
 )}
 
 ${helpers.predefined_type(
     "clip-path",
     "basic_shape::ClippingShape",
     "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",
 )}
 
 ${helpers.single_keyword(
     "mask-mode",
     "match-source alpha luminance",
     vector=True,
--- a/servo/components/style/values/animated/mod.rs
+++ b/servo/components/style/values/animated/mod.rs
@@ -8,17 +8,19 @@
 //! computed values and need yet another intermediate representation. This
 //! module's raison d'ĂȘtre is to ultimately contain all these types.
 
 use app_units::Au;
 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;
 
 pub mod color;
 pub mod effects;
 mod font;
 mod length;
@@ -333,16 +335,29 @@ macro_rules! trivial_to_animated_value {
 }
 
 trivial_to_animated_value!(Au);
 trivial_to_animated_value!(CalcLengthOrPercentage);
 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]
     fn to_animated_zero(&self) -> Result<Self, ()> {
         Ok(Au(0))
     }
 }
 
--- a/servo/components/style/values/computed/basic_shape.rs
+++ b/servo/components/style/values/computed/basic_shape.rs
@@ -18,30 +18,36 @@ pub use crate::values::generics::basic_s
 
 /// A computed clipping shape.
 pub type ClippingShape = generic::ClippingShape<BasicShape, ComputedUrl>;
 
 /// A computed float area shape.
 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
     where
         W: Write,
     {
         dest.write_str("circle(")?;
         self.radius.to_css(dest)?;
--- a/servo/components/style/values/generics/basic_shape.rs
+++ b/servo/components/style/values/generics/basic_shape.rs
@@ -15,17 +15,26 @@ use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
 
 /// A clipping shape, for `clip-path`.
 pub type ClippingShape<BasicShape, Url> = ShapeSource<BasicShape, GeometryBox, Url>;
 
 /// <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,
     StrokeBox,
     ViewBox,
     ShapeBox(ShapeBox),
 }
 
@@ -40,31 +49,40 @@ pub type FloatAreaShape<BasicShape, Imag
     Clone,
     Copy,
     Debug,
     Eq,
     MallocSizeOf,
     Parse,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
     ToCss,
 )]
 pub enum ShapeBox {
     MarginBox,
     BorderBox,
     PaddingBox,
     ContentBox,
 }
 
 /// A shape source, for some reference box.
 #[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)]
     ImageOrUrl(ImageOrUrl),
     Shape(BasicShape, Option<ReferenceBox>),
     #[animation(error)]
     Box(ReferenceBox),
     #[css(function)]
@@ -77,37 +95,39 @@ pub enum ShapeSource<BasicShape, Referen
 #[derive(
     Animate,
     Clone,
     ComputeSquaredDistance,
     Debug,
     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>),
 }
 
 /// <https://drafts.csswg.org/css-shapes/#funcdef-inset>
 #[allow(missing_docs)]
 #[css(function = "inset")]
 #[derive(
     Animate,
     Clone,
     ComputeSquaredDistance,
     Debug,
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
 )]
 pub struct InsetRect<LengthOrPercentage, NonNegativeLengthOrPercentage> {
     pub rect: Rect<LengthOrPercentage>,
     pub round: Option<BorderRadius<NonNegativeLengthOrPercentage>>,
 }
 
 /// <https://drafts.csswg.org/css-shapes/#funcdef-circle>
@@ -117,81 +137,102 @@ pub struct InsetRect<LengthOrPercentage,
     Animate,
     Clone,
     ComputeSquaredDistance,
     Copy,
     Debug,
     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>
 #[allow(missing_docs)]
 #[css(function)]
 #[derive(
     Animate,
     Clone,
     ComputeSquaredDistance,
     Copy,
     Debug,
     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>
 #[allow(missing_docs)]
 #[derive(
     Animate,
     Clone,
     ComputeSquaredDistance,
     Copy,
     Debug,
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
     ToCss,
 )]
-pub enum ShapeRadius<LengthOrPercentage> {
-    Length(LengthOrPercentage),
+pub enum ShapeRadius<NonNegativeLengthOrPercentage> {
+    Length(NonNegativeLengthOrPercentage),
     #[animation(error)]
     ClosestSide,
     #[animation(error)]
     FarthestSide,
 }
 
 /// A generic type for representing the `polygon()` function
 ///
 /// <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")]
     pub fill: FillRule,
     /// A collection of (x, y) coordinates to draw the polygon.
     #[css(iterable)]
     pub coordinates: Vec<PolygonCoord<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
 // NOTE: Basic shapes spec says that these are the only two values, however
 // https://www.w3.org/TR/SVG/painting.html#FillRuleProperty
 // says that it can also be `inherit`
 #[allow(missing_docs)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
@@ -199,31 +240,40 @@ pub struct PolygonCoord<LengthOrPercenta
     Clone,
     Copy,
     Debug,
     Eq,
     MallocSizeOf,
     Parse,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
     ToCss,
 )]
 #[repr(u8)]
 pub enum FillRule {
     Nonzero,
     Evenodd,
 }
 
 /// The path function defined in css-shape-2.
 ///
 /// 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.
     #[css(skip_if = "fill_is_default")]
     #[animation(constant)]
     pub fill: FillRule,
     /// The svg path data.
     pub path: SVGPathData,
--- a/servo/components/style/values/generics/border.rs
+++ b/servo/components/style/values/generics/border.rs
@@ -90,16 +90,17 @@ impl<L> BorderSpacing<L> {
     Animate,
     Clone,
     ComputeSquaredDistance,
     Copy,
     Debug,
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
 )]
 pub struct BorderRadius<LengthOrPercentage> {
     /// The top left radius.
     pub top_left: BorderCornerRadius<LengthOrPercentage>,
     /// The top right radius.
     pub top_right: BorderCornerRadius<LengthOrPercentage>,
     /// The bottom right radius.
--- a/servo/components/style/values/generics/position.rs
+++ b/servo/components/style/values/generics/position.rs
@@ -10,16 +10,17 @@
     Animate,
     Clone,
     ComputeSquaredDistance,
     Copy,
     Debug,
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToAnimatedZero,
     ToComputedValue,
 )]
 pub struct Position<H, V> {
     /// The horizontal component of position.
     pub horizontal: H,
     /// The vertical component of position.
     pub vertical: V,
--- a/servo/components/style/values/generics/rect.rs
+++ b/servo/components/style/values/generics/rect.rs
@@ -15,16 +15,17 @@ use style_traits::{CssWriter, ParseError
     Animate,
     Clone,
     ComputeSquaredDistance,
     Copy,
     Debug,
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
+    ToAnimatedValue,
     ToComputedValue,
 )]
 pub struct Rect<T>(pub T, pub T, pub T, pub T);
 
 impl<T> Rect<T> {
     /// Returns a new `Rect<T>` value.
     pub fn new(first: T, second: T, third: T, fourth: T) -> Self {
         Rect(first, second, third, fourth)
--- a/servo/components/style/values/specified/basic_shape.rs
+++ b/servo/components/style/values/specified/basic_shape.rs
@@ -27,29 +27,36 @@ pub use crate::values::generics::basic_s
 
 /// A specified clipping shape.
 pub type ClippingShape = generic::ClippingShape<BasicShape, SpecifiedUrl>;
 
 /// A specified float area shape.
 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>;
 
 #[cfg(feature = "gecko")]
 fn is_clip_path_path_enabled(context: &ParserContext) -> bool {
     use crate::gecko_bindings::structs::mozilla;
     context.chrome_rules_enabled() ||
@@ -304,17 +311,17 @@ impl ToCss for Ellipse {
     }
 }
 
 impl Parse for ShapeRadius {
     fn parse<'i, 't>(
         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));
         }
 
         try_match_ident_ignore_ascii_case! { input,
             "closest-side" => Ok(generic::ShapeRadius::ClosestSide),
             "farthest-side" => Ok(generic::ShapeRadius::FarthestSide),
         }
     }