style: Add bindings for ArcSlice and ThinArc, and use them to reduce copies of SVG path data.

As I said over bug 1549593, the eventual goal is to use ArcSlice in all
inherited properties. But this seemed like a good first candidate that doesn't
require me to move around a lot more code, since we were already using cbindgen
for the path commands.

Differential Revision: https://phabricator.services.mozilla.com/D30134

5 files changed, 39 insertions, 41 deletions

diff --git a/components/servo_arc/lib.rs b/components/servo_arc/lib.rs
index 09b81920164..b7c179cd13f 100644
--- a/components/servo_arc/lib.rs
+++ b/components/servo_arc/lib.rs
@@ -308,6 +308,9 @@ impl<T: ?Sized> Arc<T> {
 impl<T: ?Sized> Clone for Arc<T> {
     #[inline]
     fn clone(&self) -> Self {
+        // NOTE(emilio): If you change anything here, make sure that the
+        // implementation in layout/style/ServoStyleConstsInlines.h matches!
+        //
         // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since
         // `count` never changes between STATIC_REFCOUNT and other values.
         if self.inner().count.load(Relaxed) != STATIC_REFCOUNT {
@@ -416,6 +419,9 @@ impl<T: ?Sized> Arc<T> {
 impl<T: ?Sized> Drop for Arc<T> {
     #[inline]
     fn drop(&mut self) {
+        // NOTE(emilio): If you change anything here, make sure that the
+        // implementation in layout/style/ServoStyleConstsInlines.h matches!
+        //
         // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since
         // `count` never changes between STATIC_REFCOUNT and other values.
         if self.inner().count.load(Relaxed) == STATIC_REFCOUNT {
@@ -571,6 +577,7 @@ impl<T: Serialize> Serialize for Arc<T> {
 /// Structure to allow Arc-managing some fixed-sized data and a variably-sized
 /// slice in a single allocation.
 #[derive(Debug, Eq, PartialEq, PartialOrd)]
+#[repr(C)]
 pub struct HeaderSlice<H, T: ?Sized> {
     /// The fixed-sized data.
     pub header: H,
@@ -743,6 +750,7 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
 /// Header data with an inline length. Consumers that use HeaderWithLength as the
 /// Header type in HeaderSlice can take advantage of ThinArc.
 #[derive(Debug, Eq, PartialEq, PartialOrd)]
+#[repr(C)]
 pub struct HeaderWithLength<H> {
     /// The fixed-sized data.
     pub header: H,
diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs
index 748396630fb..607c7388c3d 100644
--- a/components/style/gecko/conversions.rs
+++ b/components/style/gecko/conversions.rs
@@ -577,12 +577,10 @@ pub mod basic_shape {
 
         /// Generate a SVGPathData from StyleShapeSource if possible.
         fn to_svg_path(&self) -> Option<SVGPathData> {
-            use crate::values::specified::svg_path::PathCommand;
             match self.mType {
                 StyleShapeSourceType::Path => {
                     let gecko_path = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr };
-                    let result: Vec<PathCommand> = gecko_path.mPath.iter().cloned().collect();
-                    Some(SVGPathData::new(result.into_boxed_slice()))
+                    Some(SVGPathData(gecko_path.mPath.clone()))
                 },
                 _ => None,
             }
diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index 1f7603c923d..703cbed2c0b 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -2901,7 +2901,7 @@ fn static_assert() {
         match v {
             OffsetPath::None => motion.mOffsetPath.mType = StyleShapeSourceType::None,
             OffsetPath::Path(p) => {
-                set_style_svg_path(&mut motion.mOffsetPath, &p, FillRule::Nonzero)
+                set_style_svg_path(&mut motion.mOffsetPath, p, FillRule::Nonzero)
             },
         }
         unsafe { Gecko_SetStyleMotion(&mut self.gecko.mMotion, motion) };
@@ -4023,25 +4023,17 @@ fn static_assert() {
 // Set SVGPathData to StyleShapeSource.
 fn set_style_svg_path(
     shape_source: &mut structs::mozilla::StyleShapeSource,
-    servo_path: &values::specified::svg_path::SVGPathData,
+    servo_path: values::specified::svg_path::SVGPathData,
     fill: values::generics::basic_shape::FillRule,
 ) {
-    use crate::gecko_bindings::bindings::Gecko_NewStyleSVGPath;
-    use crate::gecko_bindings::structs::StyleShapeSourceType;
-
-    // Setup type.
-    shape_source.mType = StyleShapeSourceType::Path;
-
     // Setup path.
-    let gecko_path = unsafe {
-        Gecko_NewStyleSVGPath(shape_source);
-        &mut shape_source.__bindgen_anon_1.mSVGPath.as_mut().mPtr.as_mut().unwrap()
-    };
-
-    gecko_path.mPath.assign_from_iter_pod(servo_path.commands().iter().cloned());
-
-    // Setup fill-rule.
-    gecko_path.mFillRule = fill;
+    unsafe {
+        bindings::Gecko_SetToSVGPath(
+            shape_source,
+            servo_path.0.forget(),
+            fill,
+        );
+    }
 }
 
 <%def name="impl_shape_source(ident, gecko_ffi_name)">
@@ -4081,7 +4073,7 @@ fn set_style_svg_path(
                 ${ident}.mReferenceBox = reference.into();
                 ${ident}.mType = StyleShapeSourceType::Box;
             }
-            ShapeSource::Path(p) => set_style_svg_path(${ident}, &p.path, p.fill),
+            ShapeSource::Path(p) => set_style_svg_path(${ident}, p.path, p.fill),
             ShapeSource::Shape(servo_shape, maybe_box) => {
                 unsafe {
                     ${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr =
diff --git a/components/style/properties/longhands/svg.mako.rs b/components/style/properties/longhands/svg.mako.rs
index 19699aa1c96..3e4d207397a 100644
--- a/components/style/properties/longhands/svg.mako.rs
+++ b/components/style/properties/longhands/svg.mako.rs
@@ -88,7 +88,6 @@ ${helpers.predefined_type(
     "generics::basic_shape::ShapeSource::None",
     products="gecko",
     animation_value_type="basic_shape::ClippingShape",
-    boxed=True,
     flags="CREATES_STACKING_CONTEXT",
     spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path",
 )}
diff --git a/components/style/values/specified/svg_path.rs b/components/style/values/specified/svg_path.rs
index 5509312256a..e5276e5cce8 100644
--- a/components/style/values/specified/svg_path.rs
+++ b/components/style/values/specified/svg_path.rs
@@ -29,16 +29,15 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
     ToResolvedValue,
     ToShmem,
 )]
-pub struct SVGPathData(Box<[PathCommand]>);
+#[repr(C)]
+pub struct SVGPathData(
+    // TODO(emilio): Should probably measure this somehow only from the
+    // specified values.
+    #[ignore_malloc_size_of = "Arc"]
+    pub crate::ArcSlice<PathCommand>
+);
 
 impl SVGPathData {
-    /// Return SVGPathData by a slice of PathCommand.
-    #[inline]
-    pub fn new(cmd: Box<[PathCommand]>) -> Self {
-        debug_assert!(!cmd.is_empty());
-        SVGPathData(cmd)
-    }
-
     /// Get the array of PathCommand.
     #[inline]
     pub fn commands(&self) -> &[PathCommand] {
@@ -46,9 +45,9 @@ impl SVGPathData {
         &self.0
     }
 
-    /// Create a normalized copy of this path by converting each relative command to an absolute
-    /// command.
-    fn normalize(&self) -> Self {
+    /// Create a normalized copy of this path by converting each relative
+    /// command to an absolute command.
+    fn normalize(&self) -> Box<[PathCommand]> {
         let mut state = PathTraversalState {
             subpath_start: CoordPair::new(0.0, 0.0),
             pos: CoordPair::new(0.0, 0.0),
@@ -58,7 +57,7 @@ impl SVGPathData {
             .iter()
             .map(|seg| seg.normalize(&mut state))
             .collect::<Vec<_>>();
-        SVGPathData(result.into_boxed_slice())
+        result.into_boxed_slice()
     }
 }
 
@@ -71,7 +70,7 @@ impl ToCss for SVGPathData {
         dest.write_char('"')?;
         {
             let mut writer = SequenceWriter::new(dest, " ");
-            for command in self.0.iter() {
+            for command in self.commands() {
                 writer.item(command)?;
             }
         }
@@ -104,7 +103,7 @@ impl Parse for SVGPathData {
             }
         }
 
-        Ok(SVGPathData::new(path_parser.path.into_boxed_slice()))
+        Ok(SVGPathData(crate::ArcSlice::from_iter(path_parser.path.into_iter())))
     }
 }
 
@@ -114,14 +113,17 @@ impl Animate for SVGPathData {
             return Err(());
         }
 
+        // FIXME(emilio): This allocates three copies of the path, that's not
+        // great! Specially, once we're normalized once, we don't need to
+        // re-normalize again.
         let result = self
             .normalize()
-            .0
             .iter()
-            .zip(other.normalize().0.iter())
+            .zip(other.normalize().iter())
             .map(|(a, b)| a.animate(&b, procedure))
             .collect::<Result<Vec<_>, _>>()?;
-        Ok(SVGPathData::new(result.into_boxed_slice()))
+
+        Ok(SVGPathData(crate::ArcSlice::from_iter(result.into_iter())))
     }
 }
 
@@ -131,9 +133,8 @@ impl ComputeSquaredDistance for SVGPathData {
             return Err(());
         }
         self.normalize()
-            .0
             .iter()
-            .zip(other.normalize().0.iter())
+            .zip(other.normalize().iter())
             .map(|(this, other)| this.compute_squared_distance(&other))
             .sum()
     }