Bug 1429299 - Part 1: Unpack StyleMotion and use cbindgen for OffsetPath. r=emilio
authorBoris Chiou <boris.chiou@gmail.com>
Mon, 20 May 2019 23:42:50 +0000
changeset 474640 4d20aaa34b107d8abaeb11020862776858678a11
parent 474639 158156b7af1806b8667b1a4491147c7332da88ab
child 474641 9d417fb6fcc29efd42b2b3f8070fca6518ae0c89
push id36042
push userdvarga@mozilla.com
push dateTue, 21 May 2019 04:19:40 +0000
treeherdermozilla-central@ca560ff55451 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1429299
milestone69.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 1429299 - Part 1: Unpack StyleMotion and use cbindgen for OffsetPath. r=emilio Unpack StyleMotion and move its members into nsStyleDisplay, use cbindgen to generate StyleOffsetPath. Differential Revision: https://phabricator.services.mozilla.com/D31164
layout/base/nsLayoutUtils.cpp
layout/painting/ActiveLayerTracker.cpp
layout/style/GeckoBindings.cpp
layout/style/GeckoBindings.h
layout/style/ServoBindings.toml
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
servo/components/style/properties/gecko.mako.rs
servo/components/style/values/specified/motion.rs
servo/ports/geckolib/cbindgen.toml
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -9950,43 +9950,42 @@ ComputedStyle* nsLayoutUtils::StyleForSc
   return style.get();
 }
 
 /* static */
 Maybe<MotionPathData> nsLayoutUtils::ResolveMotionPath(const nsIFrame* aFrame) {
   MOZ_ASSERT(aFrame);
 
   const nsStyleDisplay* display = aFrame->StyleDisplay();
-  if (!display->mMotion || !display->mMotion->HasPath()) {
+  if (display->mOffsetPath.IsNone()) {
     return Nothing();
   }
 
-  const UniquePtr<StyleMotion>& motion = display->mMotion;
   // Bug 1429299 - Implement offset-distance for motion path. For now, we use
   // the default value, i.e. 0%.
   float distance = 0.0;
   float angle = 0.0;
   Point point;
-  if (motion->OffsetPath().GetType() == StyleShapeSourceType::Path) {
+  if (display->mOffsetPath.IsPath()) {
     // Build the path and compute the point and angle for creating the
     // equivalent translate and rotate.
     // Here we only need to build a valid path for motion path, so
     // using the default values of stroke-width, stoke-linecap, and fill-rule
     // is fine for now because what we want is get the point and its normal
     // vector along the path, instead of rendering it.
     // FIXME: Bug 1484780, we should cache the path to avoid rebuilding it here
     // at every restyle. (Caching the path avoids the cost of flattening it
     // again each time.)
     RefPtr<DrawTarget> drawTarget =
         gfxPlatform::GetPlatform()->ScreenReferenceDrawTarget();
     RefPtr<PathBuilder> builder =
         drawTarget->CreatePathBuilder(FillRule::FILL_WINDING);
     RefPtr<gfx::Path> gfxPath =
-        SVGPathData::BuildPath(motion->OffsetPath().Path().Path(), builder,
-                               NS_STYLE_STROKE_LINECAP_BUTT, 0.0);
+        SVGPathData::BuildPath(display->mOffsetPath.AsPath()._0.AsSpan(),
+                               builder, NS_STYLE_STROKE_LINECAP_BUTT, 0.0);
     if (!gfxPath) {
       return Nothing();
     }
     float pathLength = gfxPath->ComputeLength();
     float computedDistance = distance * pathLength;
     Point tangent;
     point = gfxPath->ComputePointAtLength(computedDistance, &tangent);
     // Bug 1429301 - Implement offset-rotate for motion path.
--- a/layout/painting/ActiveLayerTracker.cpp
+++ b/layout/painting/ActiveLayerTracker.cpp
@@ -260,17 +260,17 @@ void ActiveLayerTracker::TransferActivit
   aFrame->AddStateBits(NS_FRAME_HAS_LAYER_ACTIVITY_PROPERTY);
   aFrame->SetProperty(LayerActivityProperty(), layerActivity);
 }
 
 static void IncrementScaleRestyleCountIfNeeded(nsIFrame* aFrame,
                                                LayerActivity* aActivity) {
   const nsStyleDisplay* display = aFrame->StyleDisplay();
   if (!display->HasTransformProperty() && !display->HasIndividualTransform() &&
-      !(display->mMotion && display->mMotion->HasPath())) {
+      display->mOffsetPath.IsNone()) {
     // The transform was removed.
     aActivity->mPreviousTransformScale = Nothing();
     IncrementMutationCount(
         &aActivity->mRestyleCounts[LayerActivity::ACTIVITY_SCALE]);
     return;
   }
 
   // Compute the new scale due to the CSS transform property.
--- a/layout/style/GeckoBindings.cpp
+++ b/layout/style/GeckoBindings.cpp
@@ -1551,30 +1551,16 @@ void Gecko_NewShapeImage(StyleShapeSourc
 
 void Gecko_SetToSVGPath(StyleShapeSource* aShape,
                         StyleForgottenArcSlicePtr<StylePathCommand> aCommands,
                         StyleFillRule aFill) {
   MOZ_ASSERT(aShape);
   aShape->SetPath(MakeUnique<StyleSVGPath>(aCommands, aFill));
 }
 
-void Gecko_SetStyleMotion(UniquePtr<StyleMotion>* aMotion,
-                          StyleMotion* aValue) {
-  MOZ_ASSERT(aMotion);
-  aMotion->reset(aValue);
-}
-
-StyleMotion* Gecko_NewStyleMotion() { return new StyleMotion(); }
-
-void Gecko_CopyStyleMotions(UniquePtr<StyleMotion>* aMotion,
-                            const StyleMotion* aOther) {
-  MOZ_ASSERT(aMotion);
-  *aMotion = aOther ? MakeUnique<StyleMotion>(*aOther) : nullptr;
-}
-
 void Gecko_ResetFilters(nsStyleEffects* effects, size_t new_len) {
   effects->mFilters.Clear();
   effects->mFilters.SetLength(new_len);
 }
 
 void Gecko_CopyFiltersFrom(nsStyleEffects* aSrc, nsStyleEffects* aDest) {
   aDest->mFilters = aSrc->mFilters;
 }
--- a/layout/style/GeckoBindings.h
+++ b/layout/style/GeckoBindings.h
@@ -528,24 +528,16 @@ void Gecko_NewShapeImage(mozilla::StyleS
 void Gecko_StyleShapeSource_SetURLValue(mozilla::StyleShapeSource* shape,
                                         mozilla::css::URLValue* uri);
 
 void Gecko_SetToSVGPath(
     mozilla::StyleShapeSource* shape,
     mozilla::StyleForgottenArcSlicePtr<mozilla::StylePathCommand>,
     mozilla::StyleFillRule);
 
-void Gecko_SetStyleMotion(mozilla::UniquePtr<mozilla::StyleMotion>* aMotion,
-                          mozilla::StyleMotion* aValue);
-
-mozilla::StyleMotion* Gecko_NewStyleMotion();
-
-void Gecko_CopyStyleMotions(mozilla::UniquePtr<mozilla::StyleMotion>* motion,
-                            const mozilla::StyleMotion* other);
-
 void Gecko_ResetFilters(nsStyleEffects* effects, size_t new_len);
 
 void Gecko_CopyFiltersFrom(nsStyleEffects* aSrc, nsStyleEffects* aDest);
 
 void Gecko_nsStyleFilter_SetURLValue(nsStyleFilter* effects,
                                      mozilla::css::URLValue* uri);
 
 void Gecko_nsStyleSVGPaint_CopyFrom(nsStyleSVGPaint* dest,
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -403,16 +403,17 @@ cbindgen-types = [
     { gecko = "StyleDisplay", servo = "values::specified::Display" },
     { gecko = "StyleDisplayMode", servo = "gecko::media_features::DisplayMode" },
     { gecko = "StylePrefersColorScheme", servo = "gecko::media_features::PrefersColorScheme" },
     { gecko = "StyleExtremumLength", servo = "values::computed::length::ExtremumLength" },
     { gecko = "StyleFillRule", servo = "values::generics::basic_shape::FillRule" },
     { gecko = "StyleFontDisplay", servo = "font_face::FontDisplay" },
     { gecko = "StyleFontFaceSourceListComponent", servo = "font_face::FontFaceSourceListComponent" },
     { gecko = "StyleFontLanguageOverride", servo = "values::computed::font::FontLanguageOverride" },
+    { gecko = "StyleOffsetPath", servo = "values::computed::motion::OffsetPath" },
     { gecko = "StylePathCommand", servo = "values::specified::svg_path::PathCommand" },
     { gecko = "StyleUnicodeRange", servo = "cssparser::UnicodeRange" },
     { gecko = "StyleOverflowWrap", servo = "values::computed::OverflowWrap" },
     { gecko = "StyleWordBreak", servo = "values::computed::WordBreak" },
     { gecko = "StyleLineBreak", servo = "values::computed::LineBreak" },
     { gecko = "StyleUserSelect", servo = "values::computed::UserSelect" },
     { gecko = "StyleBreakBetween", servo = "values::computed::BreakBetween" },
     { gecko = "StyleBreakWithin", servo = "values::computed::BreakWithin" },
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -2979,16 +2979,17 @@ nsStyleDisplay::nsStyleDisplay(const Doc
       mLineClamp(0),
       mScrollSnapPointsX(eStyleUnit_None),
       mScrollSnapPointsY(eStyleUnit_None),
       mScrollSnapDestination(
           {LengthPercentage::Zero(), LengthPercentage::Zero()}),
       mBackfaceVisibility(NS_STYLE_BACKFACE_VISIBILITY_VISIBLE),
       mTransformStyle(NS_STYLE_TRANSFORM_STYLE_FLAT),
       mTransformBox(StyleGeometryBox::BorderBox),
+      mOffsetPath(StyleOffsetPath::None()),
       mTransformOrigin{LengthPercentage::FromPercentage(0.5),
                        LengthPercentage::FromPercentage(0.5),
                        {0.}},
       mChildPerspective(StylePerspective::None()),
       mPerspectiveOrigin(Position::FromPercentage(0.5f)),
       mVerticalAlign(
           StyleVerticalAlign::Keyword(StyleVerticalAlignKeyword::Baseline)),
       mShapeMargin(LengthPercentage::Zero()) {
@@ -3046,18 +3047,17 @@ nsStyleDisplay::nsStyleDisplay(const nsS
       mScrollSnapCoordinate(aSource.mScrollSnapCoordinate),
       mTransform(aSource.mTransform),
       mRotate(aSource.mRotate),
       mTranslate(aSource.mTranslate),
       mScale(aSource.mScale),
       mBackfaceVisibility(aSource.mBackfaceVisibility),
       mTransformStyle(aSource.mTransformStyle),
       mTransformBox(aSource.mTransformBox),
-      mMotion(aSource.mMotion ? MakeUnique<StyleMotion>(*aSource.mMotion)
-                              : nullptr),
+      mOffsetPath(aSource.mOffsetPath),
       mTransformOrigin(aSource.mTransformOrigin),
       mChildPerspective(aSource.mChildPerspective),
       mPerspectiveOrigin(aSource.mPerspectiveOrigin),
       mVerticalAlign(aSource.mVerticalAlign),
       mShapeImageThreshold(aSource.mShapeImageThreshold),
       mShapeMargin(aSource.mShapeMargin),
       mShapeOutside(aSource.mShapeOutside) {
   MOZ_COUNT_CTOR(nsStyleDisplay);
@@ -3089,32 +3089,33 @@ static inline nsChangeHint CompareTransf
     } else {
       result |= nsChangeHint_UpdateOverflow;
     }
   }
 
   return result;
 }
 
-static inline nsChangeHint CompareMotionValues(const StyleMotion* aMotion,
-                                               const StyleMotion* aNewMotion) {
+static inline nsChangeHint CompareMotionValues(
+    const StyleOffsetPath& aOffsetPath, const StyleOffsetPath& aNewOffsetPath) {
   nsChangeHint result = nsChangeHint(0);
 
+  if (aOffsetPath == aNewOffsetPath) {
+    return result;
+  }
+
   // TODO: Bug 1482737: This probably doesn't need to UpdateOverflow
   // (or UpdateTransformLayer) if there's already a transform.
-  if (!aMotion != !aNewMotion || (aMotion && *aMotion != *aNewMotion)) {
-    // Set the same hints as what we use for transform because motion path is
-    // a kind of transform and will be combined with other transforms.
-    result |= nsChangeHint_UpdateTransformLayer;
-    if ((aMotion && aMotion->HasPath()) &&
-        (aNewMotion && aNewMotion->HasPath())) {
-      result |= nsChangeHint_UpdatePostTransformOverflow;
-    } else {
-      result |= nsChangeHint_UpdateOverflow;
-    }
+  // Set the same hints as what we use for transform because motion path is
+  // a kind of transform and will be combined with other transforms.
+  result |= nsChangeHint_UpdateTransformLayer;
+  if (!aOffsetPath.IsNone() && !aNewOffsetPath.IsNone()) {
+    result |= nsChangeHint_UpdatePostTransformOverflow;
+  } else {
+    result |= nsChangeHint_UpdateOverflow;
   }
   return result;
 }
 
 nsChangeHint nsStyleDisplay::CalcDifference(
     const nsStyleDisplay& aNewData) const {
   nsChangeHint hint = nsChangeHint(0);
 
@@ -3232,17 +3233,17 @@ nsChangeHint nsStyleDisplay::CalcDiffere
      * nsChangeHint_NeutralChange.
      */
     nsChangeHint transformHint = nsChangeHint(0);
 
     transformHint |= CompareTransformValues(mTransform, aNewData.mTransform);
     transformHint |= CompareTransformValues(mRotate, aNewData.mRotate);
     transformHint |= CompareTransformValues(mTranslate, aNewData.mTranslate);
     transformHint |= CompareTransformValues(mScale, aNewData.mScale);
-    transformHint |= CompareMotionValues(mMotion.get(), aNewData.mMotion.get());
+    transformHint |= CompareMotionValues(mOffsetPath, aNewData.mOffsetPath);
 
     if (mTransformOrigin != aNewData.mTransformOrigin) {
       transformHint |= nsChangeHint_UpdateTransformLayer |
                        nsChangeHint_UpdatePostTransformOverflow;
     }
 
     if (mPerspectiveOrigin != aNewData.mPerspectiveOrigin ||
         mTransformStyle != aNewData.mTransformStyle ||
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -1660,37 +1660,16 @@ struct StyleShapeSource final {
     mozilla::UniquePtr<nsStyleImage> mShapeImage;
     mozilla::UniquePtr<StyleSVGPath> mSVGPath;
     // TODO: Bug 1480665, implement ray() function.
   };
   StyleShapeSourceType mType = StyleShapeSourceType::None;
   StyleGeometryBox mReferenceBox = StyleGeometryBox::NoBox;
 };
 
-struct StyleMotion final {
-  bool operator==(const StyleMotion& aOther) const {
-    return mOffsetPath == aOther.mOffsetPath;
-  }
-
-  bool operator!=(const StyleMotion& aOther) const {
-    return !(*this == aOther);
-  }
-
-  const StyleShapeSource& OffsetPath() const { return mOffsetPath; }
-
-  bool HasPath() const {
-    // Bug 1186329: We have to check other acceptable types after supporting
-    // different values of offset-path. e.g. basic-shapes, ray.
-    return mOffsetPath.GetType() == StyleShapeSourceType::Path;
-  }
-
- private:
-  StyleShapeSource mOffsetPath;
-};
-
 }  // namespace mozilla
 
 struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay {
   typedef mozilla::StyleGeometryBox StyleGeometryBox;
 
   explicit nsStyleDisplay(const mozilla::dom::Document&);
   nsStyleDisplay(const nsStyleDisplay& aOther);
   ~nsStyleDisplay();
@@ -1768,17 +1747,17 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
   mozilla::StyleRotate mRotate;
   mozilla::StyleTranslate mTranslate;
   mozilla::StyleScale mScale;
 
   uint8_t mBackfaceVisibility;
   uint8_t mTransformStyle;
   StyleGeometryBox mTransformBox;
 
-  mozilla::UniquePtr<mozilla::StyleMotion> mMotion;
+  mozilla::StyleOffsetPath mOffsetPath;
 
   mozilla::StyleTransformOrigin mTransformOrigin;
   mozilla::StylePerspective mChildPerspective;
   mozilla::Position mPerspectiveOrigin;
 
   mozilla::StyleVerticalAlign mVerticalAlign;
 
   nsCSSPropertyID GetTransitionProperty(uint32_t aIndex) const {
@@ -1992,17 +1971,17 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
   }
 
   /* Returns whether the element has the transform property or a related
    * property. */
   bool HasTransformStyle() const {
     return HasTransformProperty() || HasIndividualTransform() ||
            mTransformStyle == NS_STYLE_TRANSFORM_STYLE_PRESERVE_3D ||
            (mWillChange.bits & mozilla::StyleWillChangeBits_TRANSFORM) ||
-           (mMotion && mMotion->HasPath());
+           !mOffsetPath.IsNone();
   }
 
   bool HasTransformProperty() const { return !mTransform._0.IsEmpty(); }
 
   bool HasIndividualTransform() const {
     return !mRotate.IsNone() || !mTranslate.IsNone() || !mScale.IsNone();
   }
 
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -2200,18 +2200,17 @@ fn static_assert() {
 <% skip_box_longhands= """display
                           animation-name animation-delay animation-duration
                           animation-direction animation-fill-mode animation-play-state
                           animation-iteration-count animation-timing-function
                           clear transition-duration transition-delay
                           transition-timing-function transition-property
                           transform-style scroll-snap-points-x
                           scroll-snap-points-y scroll-snap-coordinate
-                          -moz-binding offset-path shape-outside
-                          -webkit-line-clamp""" %>
+                          -moz-binding shape-outside -webkit-line-clamp""" %>
 <%self:impl_trait style_struct_name="Box" skip_longhands="${skip_box_longhands}">
     #[inline]
     pub fn set_display(&mut self, v: longhands::display::computed_value::T) {
         self.gecko.mDisplay = v;
         self.gecko.mOriginalDisplay = v;
     }
 
     #[inline]
@@ -2508,49 +2507,16 @@ fn static_assert() {
 
     ${impl_animation_count('iteration_count', 'IterationCount')}
     ${impl_copy_animation_value('iteration_count', 'IterationCount')}
 
     ${impl_animation_timing_function()}
 
     <% impl_shape_source("shape_outside", "mShapeOutside") %>
 
-    pub fn set_offset_path(&mut self, v: longhands::offset_path::computed_value::T) {
-        use crate::gecko_bindings::bindings::{Gecko_NewStyleMotion, Gecko_SetStyleMotion};
-        use crate::gecko_bindings::structs::StyleShapeSourceType;
-        use crate::values::generics::basic_shape::FillRule;
-        use crate::values::specified::OffsetPath;
-
-        let motion = unsafe { Gecko_NewStyleMotion().as_mut().unwrap() };
-        match v {
-            OffsetPath::None => motion.mOffsetPath.mType = StyleShapeSourceType::None,
-            OffsetPath::Path(p) => {
-                set_style_svg_path(&mut motion.mOffsetPath, p, FillRule::Nonzero)
-            },
-        }
-        unsafe { Gecko_SetStyleMotion(&mut self.gecko.mMotion, motion) };
-    }
-
-    pub fn clone_offset_path(&self) -> longhands::offset_path::computed_value::T {
-        use crate::values::specified::OffsetPath;
-        match unsafe { self.gecko.mMotion.mPtr.as_ref() } {
-            None => OffsetPath::none(),
-            Some(v) => (&v.mOffsetPath).into()
-        }
-    }
-
-    pub fn copy_offset_path_from(&mut self, other: &Self) {
-        use crate::gecko_bindings::bindings::Gecko_CopyStyleMotions;
-        unsafe { Gecko_CopyStyleMotions(&mut self.gecko.mMotion, other.gecko.mMotion.mPtr) };
-    }
-
-    pub fn reset_offset_path(&mut self, other: &Self) {
-        self.copy_offset_path_from(other);
-    }
-
     #[allow(non_snake_case)]
     pub fn set__webkit_line_clamp(&mut self, v: longhands::_webkit_line_clamp::computed_value::T) {
         self.gecko.mLineClamp = match v {
             Either::First(n) => n.0 as u32,
             Either::Second(None_) => 0,
         };
     }
 
--- a/servo/components/style/values/specified/motion.rs
+++ b/servo/components/style/values/specified/motion.rs
@@ -7,30 +7,32 @@
 use crate::parser::{Parse, ParserContext};
 use crate::values::specified::SVGPathData;
 use cssparser::Parser;
 use style_traits::{ParseError, StyleParseErrorKind};
 
 /// The offset-path value.
 ///
 /// https://drafts.fxtf.org/motion-1/#offset-path-property
+/// cbindgen:derive-tagged-enum-copy-constructor=true
 #[derive(
     Animate,
     Clone,
     ComputeSquaredDistance,
     Debug,
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToComputedValue,
     ToCss,
     ToResolvedValue,
     ToShmem,
 )]
+#[repr(C, u8)]
 pub enum OffsetPath {
     // We could merge SVGPathData into ShapeSource, so we could reuse them. However,
     // we don't want to support other value for offset-path, so use SVGPathData only for now.
     /// Path value for path(<string>).
     #[css(function)]
     Path(SVGPathData),
     /// None value.
     #[animation(error)]
--- a/servo/ports/geckolib/cbindgen.toml
+++ b/servo/ports/geckolib/cbindgen.toml
@@ -64,17 +64,17 @@ include = [
   "FillRule",
   "FontDisplay",
   "FontFaceSourceListComponent",
   "FontLanguageOverride",
   "GenericFontFamily",
   "FontFamilyNameSyntax",
   "OverflowWrap",
   "TimingFunction",
-  "PathCommand",
+  "OffsetPath",
   "UnicodeRange",
   "UserSelect",
   "Float",
   "OverscrollBehavior",
   "ScrollSnapAlign",
   "ScrollSnapAxis",
   "ScrollSnapStrictness",
   "ScrollSnapType",
@@ -432,14 +432,22 @@ renaming_overrides_prefixing = true
  private:
   // Private default constructor without initialization so that the helper
   // constructor functions still work as expected. They take care of
   // initializing the fields properly.
   StyleGenericTransformOperation() {}
  public:
 """
 
-
 "Angle" = """
   inline static StyleAngle Zero();
   inline float ToDegrees() const;
   inline double ToRadians() const;
 """
+
+"OffsetPath" = """
+ private:
+  // Private default constructor without initialization so that the helper
+  // constructor functions still work as expected. They take care of
+  // initializing the fields properly.
+  StyleOffsetPath() {}
+ public:
+"""