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

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

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

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<BasicShape, ComputedUrl>;
+/// A computed `clip-path` value.
+pub type ClipPath = generic::GenericClipPath<BasicShape, ComputedUrl>;
 
-/// A computed float area shape.
-pub type FloatAreaShape = generic::GenericFloatAreaShape<BasicShape, Image>;
+/// A computed `shape-outside` value.
+pub type ShapeOutside = generic::GenericShapeOutside<BasicShape, Image>;
 
 /// A computed basic shape.
 pub type BasicShape = generic::GenericBasicShape<

components/style/values/distance.rs

@@ -81,6 +81,16 @@ impl ComputeSquaredDistance for Au {
     }
 }
 
+impl<T> ComputeSquaredDistance for Box<T>
+where
+    T: ComputeSquaredDistance,
+{
+    #[inline]
+    fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
+        (**self).compute_squared_distance(&**other)
+    }
+}
+
 impl<T> ComputeSquaredDistance for Option<T>
 where
     T: ComputeSquaredDistance,

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<BasicShape, Url> = GenericShapeSource<BasicShape, ShapeGeometryBox, Url>;
-
 /// <https://drafts.fxtf.org/css-masking-1/#typedef-geometry-box>
 #[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<BasicShape, Image> = GenericShapeSource<BasicShape, ShapeBox, Image>;
-
 /// 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<BasicShape, Image> = GenericShapeSource<BasicShap
     Animate,
     Clone,
     Copy,
+    ComputeSquaredDistance,
     Debug,
     Eq,
     MallocSizeOf,
@@ -93,12 +89,13 @@ impl Default for ShapeBox {
     }
 }
 
-/// A shape source, for some reference box.
+/// A value for the `clip-path` property.
 #[allow(missing_docs)]
-#[animation(no_bound(I))]
+#[animation(no_bound(U))]
 #[derive(
     Animate,
     Clone,
+    ComputeSquaredDistance,
     Debug,
     MallocSizeOf,
     PartialEq,
@@ -110,22 +107,49 @@ impl Default for ShapeBox {
     ToShmem,
 )]
 #[repr(u8)]
-pub enum GenericShapeSource<BasicShape, ReferenceBox, I>
-where
-    ReferenceBox: Default + PartialEq,
-{
-    #[animation(error)]
-    ImageOrUrl(I),
-    Shape(Box<BasicShape>, #[css(skip_if = "is_default")] ReferenceBox),
-    #[animation(error)]
-    Box(ReferenceBox),
-    #[css(function)]
-    Path(Path),
+pub enum GenericClipPath<BasicShape, U> {
     #[animation(error)]
     None,
+    #[animation(error)]
+    Url(U),
+    #[css(function)]
+    Path(Path),
+    Shape(Box<BasicShape>, #[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<BasicShape, I> {
+    #[animation(error)]
+    None,
+    #[animation(error)]
+    Image(I),
+    Shape(Box<BasicShape>, #[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<B, T, U> ComputeSquaredDistance for ShapeSource<B, T, U>
-where
-    B: ComputeSquaredDistance,
-    T: Default + PartialEq,
-{
-    fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
-        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<B, U> ToAnimatedZero for ClipPath<B, U> {
+    fn to_animated_zero(&self) -> Result<Self, ()> {
+        Err(())
     }
 }
 
-impl<B, T, U> ToAnimatedZero for ShapeSource<B, T, U>
-where
-    T: Default + PartialEq,
-{
+impl<B, U> ToAnimatedZero for ShapeOutside<B, U> {
     fn to_animated_zero(&self) -> Result<Self, ()> {
         Err(())
     }

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<BasicShape, SpecifiedUrl>;
+/// A specified `clip-path` value.
+pub type ClipPath = generic::GenericClipPath<BasicShape, SpecifiedUrl>;
 
-/// A specified float area shape.
-pub type FloatAreaShape = generic::GenericFloatAreaShape<BasicShape, Image>;
+/// A specified `shape-outside` value.
+pub type ShapeOutside = generic::GenericShapeOutside<BasicShape, Image>;
 
 /// 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<BasicShape>, ReferenceBox) -> R,
+    to_reference_box: impl FnOnce(ReferenceBox) -> R,
+) -> Result<R, ParseError<'i>>
+where
+    ReferenceBox: Default + Parse,
+{
+    fn parse_component<U: Parse>(
+        context: &ParserContext,
+        input: &mut Parser,
+        component: &mut Option<U>,
+    ) -> 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<Self, ParseError<'i>> {
         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<ReferenceBox, ImageOrUrl> ShapeSource<BasicShape, ReferenceBox, ImageOrUrl>
-where
-    ReferenceBox: Parse + Default + PartialEq,
-{
-    /// The internal parser for ShapeSource.
-    fn parse_common<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        fn parse_component<U: Parse>(
-            context: &ParserContext,
-            input: &mut Parser,
-            component: &mut Option<U>,
-        ) -> 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,
+        )
     }
 }