Bug 1592822 - Use Serde for SVGOffsetPath. r=emilio

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

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

gfx/layers/ipc/LayersMessageUtils.h

@@ -873,7 +873,7 @@ inline mozilla::StyleVecU8 ConvertToStyleVecU8(mozilla::ipc::ByteBuf&& aOther) {
   };
 
 IMPL_PARAMTRAITS_BY_SERDE(LengthPercentage)
-IMPL_PARAMTRAITS_BY_SERDE(RayFunction)
+IMPL_PARAMTRAITS_BY_SERDE(StyleOffsetPath)
 IMPL_PARAMTRAITS_BY_SERDE(StyleRotate)
 IMPL_PARAMTRAITS_BY_SERDE(StyleScale)
 IMPL_PARAMTRAITS_BY_SERDE(StyleTranslate)

gfx/layers/ipc/LayersMessages.ipdlh

@@ -59,8 +59,8 @@ using mozilla::layers::LayersId from "mozilla/layers/LayersTypes.h";
 using mozilla::layers::TransactionId from "mozilla/layers/LayersTypes.h";
 using mozilla::VsyncId from "mozilla/VsyncDispatcher.h";
 using mozilla::LengthPercentage from "mozilla/ServoStyleConsts.h";
-using mozilla::RayFunction from "mozilla/MotionPathUtils.h";
 using mozilla::RayReferenceData from "mozilla/MotionPathUtils.h";
+using mozilla::StyleOffsetPath from "mozilla/ServoStyleConsts.h";
 using mozilla::StyleRotate from "mozilla/ServoStyleConsts.h";
 using mozilla::StyleScale from "mozilla/ServoStyleConsts.h";
 using mozilla::StyleTranslate from "mozilla/ServoStyleConsts.h";
@@ -126,50 +126,6 @@ struct CSSAngle {
 
 struct LayerColor { Color value; };
 
-struct MoveTo { Point point; };
-struct LineTo { Point point; };
-struct HorizontalLineTo { float x; };
-struct VerticalLineTo { float y; };
-struct CurveTo {
-  Point control1;
-  Point control2;
-  Point point;
-};
-struct SmoothCurveTo { Point control2; Point point; };
-struct QuadBezierCurveTo { Point control1; Point point; };
-struct SmoothQuadBezierCurveTo { Point point; };
-struct EllipticalArc {
-  float rx;
-  float ry;
-  float angle;
-  bool largeArcFlag;
-  bool sweepFlag;
-  Point point;
-};
-
-// PathCommand should be always absolute because we normalize it when passing
-// it through ipc.
-union PathCommand {
-  // Use null_t to represent ClosePath.
-  null_t;
-  MoveTo;
-  LineTo;
-  HorizontalLineTo;
-  VerticalLineTo;
-  CurveTo;
-  SmoothCurveTo;
-  QuadBezierCurveTo;
-  SmoothQuadBezierCurveTo;
-  EllipticalArc;
-};
-
-union OffsetPath {
-  // null_t represents None.
-  null_t;
-  PathCommand[];
-  RayFunction;
-};
-
 struct OffsetRotate {
   CSSAngle angle;
   bool isAuto;
@@ -194,7 +150,7 @@ union Animatable {
   StyleScale;
   StyleTranslate;
   StyleTransform;
-  OffsetPath;
+  StyleOffsetPath;
   LengthPercentage;
   OffsetRotate;
   OffsetAnchor;

layout/base/MotionPathUtils.cpp

@@ -543,82 +543,11 @@ Maybe<ResolvedMotionPathData> MotionPathUtils::ResolveMotionPath(
 }
 
 /* static */
-nsTArray<layers::PathCommand>
-MotionPathUtils::NormalizeAndConvertToPathCommands(
+StyleSVGPathData MotionPathUtils::NormalizeSVGPathData(
     const StyleSVGPathData& aPath) {
-  // Normalization
   StyleSVGPathData n;
   Servo_SVGPathData_Normalize(&aPath, &n);
-
-  auto asPoint = [](const StyleCoordPair& aPair) {
-    return gfx::Point(aPair._0, aPair._1);
-  };
-
-  // Converstion
-  nsTArray<layers::PathCommand> commands;
-  for (const StylePathCommand& command : n._0.AsSpan()) {
-    switch (command.tag) {
-      case StylePathCommand::Tag::ClosePath:
-        commands.AppendElement(mozilla::null_t());
-        break;
-      case StylePathCommand::Tag::MoveTo: {
-        const auto& p = command.AsMoveTo().point;
-        commands.AppendElement(layers::MoveTo(asPoint(p)));
-        break;
-      }
-      case StylePathCommand::Tag::LineTo: {
-        const auto& p = command.AsLineTo().point;
-        commands.AppendElement(layers::LineTo(asPoint(p)));
-        break;
-      }
-      case StylePathCommand::Tag::HorizontalLineTo: {
-        const auto& h = command.AsHorizontalLineTo();
-        commands.AppendElement(layers::HorizontalLineTo(h.x));
-        break;
-      }
-      case StylePathCommand::Tag::VerticalLineTo: {
-        const auto& v = command.AsVerticalLineTo();
-        commands.AppendElement(layers::VerticalLineTo(v.y));
-        break;
-      }
-      case StylePathCommand::Tag::CurveTo: {
-        const auto& curve = command.AsCurveTo();
-        commands.AppendElement(layers::CurveTo(asPoint(curve.control1),
-                                               asPoint(curve.control2),
-                                               asPoint(curve.point)));
-        break;
-      }
-      case StylePathCommand::Tag::SmoothCurveTo: {
-        const auto& curve = command.AsSmoothCurveTo();
-        commands.AppendElement(layers::SmoothCurveTo(asPoint(curve.control2),
-                                                     asPoint(curve.point)));
-        break;
-      }
-      case StylePathCommand::Tag::QuadBezierCurveTo: {
-        const auto& curve = command.AsQuadBezierCurveTo();
-        commands.AppendElement(layers::QuadBezierCurveTo(
-            asPoint(curve.control1), asPoint(curve.point)));
-        break;
-      }
-      case StylePathCommand::Tag::SmoothQuadBezierCurveTo: {
-        const auto& curve = command.AsSmoothCurveTo();
-        commands.AppendElement(
-            layers::SmoothQuadBezierCurveTo(asPoint(curve.point)));
-        break;
-      }
-      case StylePathCommand::Tag::EllipticalArc: {
-        const auto& arc = command.AsEllipticalArc();
-        gfx::Point point = asPoint(arc.point);
-        commands.AppendElement(layers::EllipticalArc(arc.rx, arc.ry, arc.angle,
-                                                     arc.large_arc_flag._0,
-                                                     arc.sweep_flag._0, point));
-        break;
-      }
-      case StylePathCommand::Tag::Unknown:
-        MOZ_ASSERT_UNREACHABLE("Unsupported path command");
-    }
-  }
-  return commands;
+  return n;
 }
 
 /* static */

layout/base/MotionPathUtils.h

@@ -184,16 +184,15 @@ class MotionPathUtils final {
       const CSSSize& aFrameSize, gfx::Path* aCachedMotionPath);
 
   /**
-   * Normalize and convert StyleSVGPathData into nsTArray<layers::PathCommand>.
+   * Normalize StyleSVGPathData.
    *
    * The algorithm of normalization is the same as normalize() in
    * servo/components/style/values/specified/svg_path.rs
    * FIXME: Bug 1489392: We don't have to normalize the path here if we accept
-   * the spec issue which would like to normalize svg paths ar computed time.
+   * the spec issue which would like to normalize svg paths at computed time.
    * https://github.com/w3c/svgwg/issues/321
    */
-  static nsTArray<layers::PathCommand> NormalizeAndConvertToPathCommands(
-      const StyleSVGPathData& aPath);
+  static StyleSVGPathData NormalizeSVGPathData(const StyleSVGPathData& aPath);
 
   /**
    * Build a gfx::Path from the computed svg path. We should give it a path

layout/painting/nsDisplayList.cpp

@@ -332,21 +332,14 @@ static TimingFunction ToTimingFunction(
       aCTF->GetSteps().mSteps, static_cast<uint8_t>(aCTF->GetSteps().mPos)));
 }
 
-static Animatable GetOffsetPath(const StyleOffsetPath& aOffsetPath) {
-  Animatable result;
-  switch (aOffsetPath.tag) {
-    case StyleOffsetPath::Tag::Path:
-      result = OffsetPath(MotionPathUtils::NormalizeAndConvertToPathCommands(
-          aOffsetPath.AsPath()));
-      break;
-    case StyleOffsetPath::Tag::Ray:
-      result = OffsetPath(aOffsetPath.AsRay());
-      break;
-    case StyleOffsetPath::Tag::None:
-    default:
-      result = OffsetPath(null_t());
+// FIXME: Bug 1489392: We don't have to normalize the path here if we accept
+// the spec issue which would like to normalize svg paths at computed time.
+static StyleOffsetPath NormalizeOffsetPath(const StyleOffsetPath& aOffsetPath) {
+  if (aOffsetPath.IsPath()) {
+    return StyleOffsetPath::Path(
+        MotionPathUtils::NormalizeSVGPathData(aOffsetPath.AsPath()));
   }
-  return result;
+  return StyleOffsetPath(aOffsetPath);
 }
 
 static void SetAnimatable(nsCSSPropertyID aProperty,
@@ -387,7 +380,8 @@ static void SetAnimatable(nsCSSPropertyID aProperty,
           aAnimationValue.GetTransformProperty(), aRefBox);
       break;
     case eCSSProperty_offset_path:
-      aAnimatable = GetOffsetPath(aAnimationValue.GetOffsetPathProperty());
+      aAnimatable =
+          NormalizeOffsetPath(aAnimationValue.GetOffsetPathProperty());
       break;
     case eCSSProperty_offset_distance:
       aAnimatable = aAnimationValue.GetOffsetDistanceProperty();
@@ -774,7 +768,7 @@ static void AddNonAnimatingTransformLikePropertiesStyles(
         break;
       case eCSSProperty_offset_path:
         if (!display->mOffsetPath.IsNone()) {
-          appendFakeAnimation(id, GetOffsetPath(display->mOffsetPath));
+          appendFakeAnimation(id, NormalizeOffsetPath(display->mOffsetPath));
         }
         break;
       case eCSSProperty_offset_distance:

layout/style/ServoBindings.h

@@ -65,11 +65,11 @@ BASIC_RULE_FUNCS(CounterStyle)
 
 using RayFunction = StyleRayFunction<StyleAngle>;
 BASIC_SERDE_FUNCS(LengthPercentage)
-BASIC_SERDE_FUNCS(RayFunction)
 BASIC_SERDE_FUNCS(StyleRotate)
 BASIC_SERDE_FUNCS(StyleScale)
 BASIC_SERDE_FUNCS(StyleTranslate)
 BASIC_SERDE_FUNCS(StyleTransform)
+BASIC_SERDE_FUNCS(StyleOffsetPath)
 
 #undef BASIC_SERDE_FUNCS
 

layout/style/StyleAnimationValue.cpp

@@ -51,63 +51,6 @@ static inline StyleAngle GetCSSAngle(const layers::CSSAngle& aAngle) {
   return StyleAngle{aAngle.value()};
 }
 
-static StylePathCommand CommandFromLayers(const layers::PathCommand& aCommand) {
-  switch (aCommand.type()) {
-    case layers::PathCommand::TMoveTo:
-      return StylePathCommand::MoveTo(
-          StyleCoordPair(aCommand.get_MoveTo().point()), StyleIsAbsolute::Yes);
-    case layers::PathCommand::TLineTo:
-      return StylePathCommand::LineTo(
-          StyleCoordPair(aCommand.get_LineTo().point()), StyleIsAbsolute::Yes);
-    case layers::PathCommand::THorizontalLineTo:
-      return StylePathCommand::HorizontalLineTo(
-          aCommand.get_HorizontalLineTo().x(), StyleIsAbsolute::Yes);
-    case layers::PathCommand::TVerticalLineTo:
-      return StylePathCommand::VerticalLineTo(aCommand.get_VerticalLineTo().y(),
-                                              StyleIsAbsolute::Yes);
-    case layers::PathCommand::TCurveTo:
-      return StylePathCommand::CurveTo(
-          StyleCoordPair(aCommand.get_CurveTo().control1()),
-          StyleCoordPair(aCommand.get_CurveTo().control2()),
-          StyleCoordPair(aCommand.get_CurveTo().point()), StyleIsAbsolute::Yes);
-    case layers::PathCommand::TSmoothCurveTo:
-      return StylePathCommand::SmoothCurveTo(
-          StyleCoordPair(aCommand.get_SmoothCurveTo().control2()),
-          StyleCoordPair(aCommand.get_SmoothCurveTo().point()),
-          StyleIsAbsolute::Yes);
-    case layers::PathCommand::TQuadBezierCurveTo:
-      return StylePathCommand::QuadBezierCurveTo(
-          StyleCoordPair(aCommand.get_QuadBezierCurveTo().control1()),
-          StyleCoordPair(aCommand.get_QuadBezierCurveTo().point()),
-          StyleIsAbsolute::Yes);
-    case layers::PathCommand::TSmoothQuadBezierCurveTo:
-      return StylePathCommand::SmoothQuadBezierCurveTo(
-          StyleCoordPair(aCommand.get_SmoothQuadBezierCurveTo().point()),
-          StyleIsAbsolute::Yes);
-    case layers::PathCommand::TEllipticalArc: {
-      const layers::EllipticalArc& arc = aCommand.get_EllipticalArc();
-      return StylePathCommand::EllipticalArc(
-          arc.rx(), arc.ry(), arc.angle(), StyleArcFlag{arc.largeArcFlag()},
-          StyleArcFlag{arc.sweepFlag()}, StyleCoordPair(arc.point()),
-          StyleIsAbsolute::Yes);
-    }
-    case layers::PathCommand::Tnull_t:
-      return StylePathCommand::ClosePath();
-    default:
-      MOZ_ASSERT_UNREACHABLE("Unsupported path command");
-  }
-  return StylePathCommand::Unknown();
-}
-
-static nsTArray<StylePathCommand> CreatePathCommandList(
-    const nsTArray<layers::PathCommand>& aCommands) {
-  nsTArray<StylePathCommand> result(aCommands.Length());
-  for (const layers::PathCommand& command : aCommands) {
-    result.AppendElement(CommandFromLayers(command));
-  }
-  return result;
-}
-
 // AnimationValue Implementation
 
 bool AnimationValue::operator==(const AnimationValue& aOther) const {
@@ -321,26 +264,9 @@ already_AddRefed<RawServoAnimationValue> AnimationValue::FromAnimatable(
           "Should have been resolved already");
       return Servo_AnimationValue_Translate(&aAnimatable.get_StyleTranslate())
           .Consume();
-    case layers::Animatable::TOffsetPath: {
-      const layers::OffsetPath& p = aAnimatable.get_OffsetPath();
-      switch (p.type()) {
-        case layers::OffsetPath::TArrayOfPathCommand: {
-          nsTArray<StylePathCommand> commands =
-              CreatePathCommandList(p.get_ArrayOfPathCommand());
-          return Servo_AnimationValue_SVGPath(commands.Elements(),
-                                              commands.Length())
-              .Consume();
-        }
-        case layers::OffsetPath::TRayFunction:
-          return Servo_AnimationValue_RayFunction(&p.get_RayFunction())
-              .Consume();
-        case layers::OffsetPath::Tnull_t:
-          return Servo_AnimationValue_NoneOffsetPath().Consume();
-        default:
-          MOZ_ASSERT_UNREACHABLE("Unsupported path");
-      }
-      return nullptr;
-    }
+    case layers::Animatable::TStyleOffsetPath:
+      return Servo_AnimationValue_OffsetPath(&aAnimatable.get_StyleOffsetPath())
+          .Consume();
     case layers::Animatable::TLengthPercentage:
       return Servo_AnimationValue_OffsetDistance(
                  &aAnimatable.get_LengthPercentage())

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

@@ -73,13 +73,16 @@ pub struct RayFunction<Angle> {
 /// The offset-path value.
 ///
 /// https://drafts.fxtf.org/motion-1/#offset-path-property
+/// cbindgen:private-default-tagged-enum-constructor=false
 #[derive(
     Animate,
     Clone,
     ComputeSquaredDistance,
     Debug,
+    Deserialize,
     MallocSizeOf,
     PartialEq,
+    Serialize,
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToComputedValue,

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

@@ -21,8 +21,10 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 #[derive(
     Clone,
     Debug,
+    Deserialize,
     MallocSizeOf,
     PartialEq,
+    Serialize,
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToComputedValue,
@@ -156,8 +158,10 @@ impl ComputeSquaredDistance for SVGPathData {
     ComputeSquaredDistance,
     Copy,
     Debug,
+    Deserialize,
     MallocSizeOf,
     PartialEq,
+    Serialize,
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToShmem,
@@ -483,8 +487,10 @@ impl ToCss for PathCommand {
     ComputeSquaredDistance,
     Copy,
     Debug,
+    Deserialize,
     MallocSizeOf,
     PartialEq,
+    Serialize,
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToShmem,
@@ -511,8 +517,10 @@ impl IsAbsolute {
     ComputeSquaredDistance,
     Copy,
     Debug,
+    Deserialize,
     MallocSizeOf,
     PartialEq,
+    Serialize,
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToCss,
@@ -530,7 +538,9 @@ impl CoordPair {
 }
 
 /// The EllipticalArc flag type.
-#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToShmem)]
+#[derive(
+    Clone, Copy, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize, SpecifiedValueInfo, ToShmem,
+)]
 #[repr(C)]
 pub struct ArcFlag(bool);
 

servo/components/style_traits/arc_slice.rs

@@ -4,6 +4,8 @@
 
 //! A thin atomically-reference-counted slice.
 
+use serde::de::{Deserialize, Deserializer};
+use serde::ser::{Serialize, Serializer};
 use servo_arc::ThinArc;
 use std::ops::Deref;
 use std::ptr::NonNull;
@@ -60,6 +62,25 @@ impl<T> Default for ArcSlice<T> {
     }
 }
 
+impl<T: Serialize> Serialize for ArcSlice<T> {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        self.deref().serialize(serializer)
+    }
+}
+
+impl<'de, T: Deserialize<'de>> Deserialize<'de> for ArcSlice<T> {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let r = Vec::deserialize(deserializer)?;
+        Ok(ArcSlice::from_iter(r.into_iter()))
+    }
+}
+
 impl<T> ArcSlice<T> {
     /// Creates an Arc for a slice using the given iterator to generate the
     /// slice.

servo/ports/geckolib/cbindgen.toml

@@ -715,3 +715,9 @@ public:
   // The implementation of IPC LayersMessages needs this to be public.
   StyleGenericTranslate(): tag(Tag::None) {}
 """
+
+"GenericOffsetPath" = """
+public:
+  // The implementation of IPC LayersMessages needs this to be public.
+  StyleGenericOffsetPath(): tag(Tag::None) {}
+"""

servo/ports/geckolib/glue.rs

@@ -133,7 +133,6 @@ use style::use_counters::UseCounters;
 use style::values::animated::{Animate, Procedure, ToAnimatedZero};
 use style::values::computed::{self, Context, ToComputedValue};
 use style::values::distance::ComputeSquaredDistance;
-use style::values::generics;
 use style::values::specified;
 use style::values::specified::gecko::IntersectionObserverRootMargin;
 use style::values::specified::source_size_list::SourceSizeList;
@@ -915,32 +914,10 @@ pub unsafe extern "C" fn Servo_AnimationValue_Transform(
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn Servo_AnimationValue_SVGPath(
-    list: *const specified::svg_path::PathCommand,
-    len: usize,
+pub unsafe extern "C" fn Servo_AnimationValue_OffsetPath(
+    p: &computed::motion::OffsetPath,
 ) -> Strong<RawServoAnimationValue> {
-    use style::values::generics::motion::OffsetPath;
-    use style::values::specified::SVGPathData;
-
-    let slice = std::slice::from_raw_parts(list, len);
-    Arc::new(AnimationValue::OffsetPath(OffsetPath::Path(SVGPathData(
-        style_traits::arc_slice::ArcSlice::from_iter(slice.iter().cloned()),
-    ))))
-    .into_strong()
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn Servo_AnimationValue_RayFunction(
-    r: &generics::motion::RayFunction<computed::Angle>,
-) -> Strong<RawServoAnimationValue> {
-    use style::values::generics::motion::OffsetPath;
-    Arc::new(AnimationValue::OffsetPath(OffsetPath::Ray(r.clone()))).into_strong()
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn Servo_AnimationValue_NoneOffsetPath() -> Strong<RawServoAnimationValue> {
-    use style::values::generics::motion::OffsetPath;
-    Arc::new(AnimationValue::OffsetPath(OffsetPath::None)).into_strong()
+    Arc::new(AnimationValue::OffsetPath(p.clone())).into_strong()
 }
 
 #[no_mangle]
@@ -1066,12 +1043,6 @@ impl_basic_serde_funcs!(
     computed::LengthPercentage
 );
 
-impl_basic_serde_funcs!(
-    Servo_RayFunction_Serialize,
-    Servo_RayFunction_Deserialize,
-    generics::motion::RayFunction<computed::Angle>
-);
-
 impl_basic_serde_funcs!(
     Servo_StyleRotate_Serialize,
     Servo_StyleRotate_Deserialize,
@@ -1096,6 +1067,12 @@ impl_basic_serde_funcs!(
     computed::transform::Transform
 );
 
+impl_basic_serde_funcs!(
+    Servo_StyleOffsetPath_Serialize,
+    Servo_StyleOffsetPath_Deserialize,
+    computed::motion::OffsetPath
+);
+
 #[no_mangle]
 pub extern "C" fn Servo_SVGPathData_Normalize(
     input: &specified::SVGPathData,