Bug 1598158 - Support offset-path:url() in style. r=devtools-reviewers,emilio

In layout, we build a default `path("m 0 0")` for now. We will implement
it later.

Besides, we don't support compositor animations for `url()`, so we don't
have to serialize it for IPC.

Note:
`<url>` includes `url()` and `src()`. For now we only support `url()`.
We should revisit `src()` in Bug 1845390.

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

devtools/shared/css/generated/properties-db.js

@@ -9362,6 +9362,7 @@ exports.CSS_PROPERTIES = {
       "stroke-box",
       "top",
       "unset",
+      "url",
       "view-box",
       "xywh"
     ]
@@ -9425,6 +9426,7 @@ exports.CSS_PROPERTIES = {
       "revert-layer",
       "stroke-box",
       "unset",
+      "url",
       "view-box",
       "xywh"
     ]

dom/animation/KeyframeEffect.cpp

@@ -1670,6 +1670,13 @@ bool KeyframeEffect::CanAnimateTransformOnCompositor(
 bool KeyframeEffect::ShouldBlockAsyncTransformAnimations(
     const nsIFrame* aFrame, const nsCSSPropertyIDSet& aPropertySet,
     AnimationPerformanceWarning::Type& aPerformanceWarning /* out */) const {
+  // If we depend on the SVG url (no matter whether there are any offset-path
+  // animations), we cannot run any transform-like animations in the compositor
+  // because we cannot resolve the url in the compositor if its style uses url.
+  if (aFrame->StyleDisplay()->mOffsetPath.IsUrl()) {
+    return true;
+  }
+
   EffectSet* effectSet = EffectSet::Get(mTarget.mElement, mTarget.mPseudoType);
   // The various transform properties ('transform', 'scale' etc.) get combined
   // on the compositor.
@@ -1727,6 +1734,17 @@ bool KeyframeEffect::ShouldBlockAsyncTransformAnimations(
         return true;
       }
     }
+
+    // If there are any offset-path animations whose animation values are url(),
+    // we have to sync with the main thread when resolving it.
+    if (property.mProperty == eCSSProperty_offset_path) {
+      for (const auto& seg : property.mSegments) {
+        if (seg.mFromValue.IsOffsetPathUrl() ||
+            seg.mToValue.IsOffsetPathUrl()) {
+          return true;
+        }
+      }
+    }
   }
 
   return false;

layout/base/MotionPathUtils.cpp

@@ -404,6 +404,19 @@ static already_AddRefed<gfx::Path> BuildSimpleInsetPath(
                                    aPathBuilder);
 }
 
+// Create a path for `path("m 0 0")`, which is the default URL path.
+// https://drafts.fxtf.org/motion-1/#valdef-offset-path-url
+static already_AddRefed<gfx::Path> BuildDefaultPathForURL(
+    gfx::PathBuilder* aBuilder) {
+  if (!aBuilder) {
+    return nullptr;
+  }
+
+  Array<const StylePathCommand, 1> array(StylePathCommand::MoveTo(
+      StyleCoordPair(gfx::Point{0.0, 0.0}), StyleIsAbsolute::No));
+  return SVGPathData::BuildPath(array, aBuilder, StyleStrokeLinecap::Butt, 0.0);
+}
+
 // Generate data for motion path on the main thread.
 static OffsetPathData GenerateOffsetPathData(const nsIFrame* aFrame) {
   const StyleOffsetPath& offsetPath = aFrame->StyleDisplay()->mOffsetPath;
@@ -439,6 +452,19 @@ static OffsetPathData GenerateOffsetPathData(const nsIFrame* aFrame) {
     return OffsetPathData::Shape(gfxPath.forget(), {}, IsClosedPath(pathData));
   }
 
+  RefPtr<gfx::PathBuilder> builder = MotionPathUtils::GetPathBuilder();
+
+  if (offsetPath.IsUrl()) {
+    // TODO: Finish this in the following patches.
+
+    // Note: This behaves as path("m 0 0") (a <basic-shape>).
+    RefPtr<gfx::Path> path = BuildDefaultPathForURL(builder);
+    // FIXME: Bug 1836847. Once we support "at <position>" for path(), we have
+    // to give it the current box position.
+    return path ? OffsetPathData::Shape(path.forget(), {}, false)
+                : OffsetPathData::None();
+  }
+
   // The rest part is to handle "<basic-shape> || <coord-box>".
   MOZ_ASSERT(offsetPath.IsBasicShapeOrCoordBox());
 
@@ -448,10 +474,8 @@ static OffsetPathData GenerateOffsetPathData(const nsIFrame* aFrame) {
   if (!containingFrame || coordBox.IsEmpty()) {
     return OffsetPathData::None();
   }
-
-  const nsStyleDisplay* disp = aFrame->StyleDisplay();
   nsPoint currentPosition = aFrame->GetOffsetTo(containingFrame);
-  RefPtr<gfx::PathBuilder> builder = MotionPathUtils::GetPathBuilder();
+  const nsStyleDisplay* disp = aFrame->StyleDisplay();
   RefPtr<gfx::Path> path =
       disp->mOffsetPath.IsCoordBox()
           ? BuildSimpleInsetPath(containingFrame->StyleBorder()->mBorderRadius,

layout/style/StyleAnimationValue.cpp

@@ -116,6 +116,10 @@ const mozilla::StyleOffsetPosition& AnimationValue::GetOffsetPositionProperty()
   return *Servo_AnimationValue_GetOffsetPosition(mServo);
 }
 
+bool AnimationValue::IsOffsetPathUrl() const {
+  return mServo && Servo_AnimationValue_IsOffsetPathUrl(mServo);
+}
+
 MatrixScales AnimationValue::GetScaleValue(const nsIFrame* aFrame) const {
   using namespace nsStyleTransformMatrix;
 

layout/style/StyleAnimationValue.h

@@ -85,6 +85,7 @@ struct AnimationValue {
   const mozilla::StyleOffsetRotate& GetOffsetRotateProperty() const;
   const mozilla::StylePositionOrAuto& GetOffsetAnchorProperty() const;
   const mozilla::StyleOffsetPosition& GetOffsetPositionProperty() const;
+  bool IsOffsetPathUrl() const;
 
   // Return the scale for mServo, which is calculated with reference to aFrame.
   mozilla::gfx::MatrixScales GetScaleValue(const nsIFrame* aFrame) const;

layout/style/test/mochitest.ini

@@ -15,6 +15,7 @@ prefs =
   layout.css.motion-path-basic-shapes.enabled=true
   layout.css.motion-path-coord-box.enabled=true
   layout.css.motion-path-offset-position.enabled=true
+  layout.css.motion-path-url.enabled=true
   layout.css.overflow-logical.enabled=true
   layout.css.backdrop-filter.enabled=true
   layout.css.fit-content-function.enabled=true

layout/style/test/property_database.js

@@ -13557,6 +13557,10 @@ if (IsCSSPropertyPrefEnabled("layout.css.motion-path.enabled")) {
     );
   }
 
+  if (IsCSSPropertyPrefEnabled("layout.css.motion-path-url.enabled")) {
+    gCSSProperties["offset-path"]["other_values"].push("url(#svgPath)");
+  }
+
   gCSSProperties["offset-distance"] = {
     domProp: "offsetDistance",
     inherited: false,

modules/libpref/init/StaticPrefList.yaml

@@ -8611,6 +8611,13 @@
   mirror: always
   rust: true
 
+# Is support for motion-path url enabled?
+- name: layout.css.motion-path-url.enabled
+  type: RelaxedAtomicBool
+  value: @IS_NIGHTLY_BUILD@
+  mirror: always
+  rust: true
+
 # Pref to control whether the ::marker property restrictions defined in [1]
 # apply.
 #

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

@@ -5,6 +5,7 @@
 //! Computed types for CSS values that are related to motion path.
 
 use crate::values::computed::basic_shape::BasicShape;
+use crate::values::computed::url::ComputedUrl;
 use crate::values::computed::{Angle, LengthPercentage, Position};
 use crate::values::generics::motion::{
     GenericOffsetPath, GenericOffsetPathFunction, GenericOffsetPosition, GenericRayFunction,
@@ -15,7 +16,7 @@ use crate::Zero;
 pub type RayFunction = GenericRayFunction<Angle, Position>;
 
 /// The computed value of <offset-path>.
-pub type OffsetPathFunction = GenericOffsetPathFunction<BasicShape, RayFunction>;
+pub type OffsetPathFunction = GenericOffsetPathFunction<BasicShape, RayFunction, ComputedUrl>;
 
 /// The computed value of `offset-path`.
 pub type OffsetPath = GenericOffsetPath<OffsetPathFunction>;

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

@@ -7,6 +7,7 @@
 use crate::values::animated::ToAnimatedZero;
 use crate::values::generics::position::{GenericPosition, GenericPositionOrAuto};
 use crate::values::specified::motion::CoordBox;
+use serde::Deserializer;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
 
@@ -106,6 +107,22 @@ where
     }
 }
 
+/// Return error if we try to deserialize the url, for Gecko IPC purposes.
+// Note: we cannot use #[serde(skip_deserializing)] variant attribute, which may cause the fatal
+// error when trying to read the parameters because it cannot deserialize the input byte buffer,
+// even if the type of OffsetPathFunction is not an url(), in our tests. This may be an issue of
+// #[serde(skip_deserializing)] on enum, at least in the version (1.0) we are using. So we have to
+// manually implement this deseriailzing function, but return error.
+// FIXME: Bug 1847620, fiure out this is a serde issue or a gecko bug.
+fn deserialize_url<'de, D, T>(_deserializer: D) -> Result<T, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    use crate::serde::de::Error;
+    // Return Err() so the IPC will catch it and assert this as a fetal error.
+    Err(<D as Deserializer>::Error::custom("we don't support the deserializing for url"))
+}
+
 /// The <offset-path> value.
 /// <offset-path> = <ray()> | <url> | <basic-shape>
 ///
@@ -126,15 +143,21 @@ where
     ToResolvedValue,
     ToShmem,
 )]
+#[animation(no_bound(U))]
 #[repr(C, u8)]
-pub enum GenericOffsetPathFunction<Shapes, RayFunction> {
+pub enum GenericOffsetPathFunction<Shapes, RayFunction, U> {
     /// ray() function, which defines a path in the polar coordinate system.
     /// Use Box<> to make sure the size of offset-path is not too large.
     #[css(function)]
     Ray(RayFunction),
+    /// A URL reference to an SVG shape element. If the URL does not reference a shape element,
+    /// this behaves as path("m 0 0") instead.
+    #[animation(error)]
+    #[serde(deserialize_with = "deserialize_url")]
+    #[serde(skip_serializing)]
+    Url(U),
     /// The <basic-shape> value.
     Shape(Shapes),
-    // FIXME: Bug 1598158. Support <url>.
 }
 
 pub use self::GenericOffsetPathFunction as OffsetPathFunction;

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

@@ -10,6 +10,7 @@ use crate::values::computed::{Context, ToComputedValue};
 use crate::values::generics::motion as generics;
 use crate::values::specified::basic_shape::BasicShape;
 use crate::values::specified::position::{HorizontalPosition, VerticalPosition};
+use crate::values::specified::url::SpecifiedUrl;
 use crate::values::specified::{Angle, Position};
 use crate::Zero;
 use cssparser::Parser;
@@ -19,7 +20,8 @@ use style_traits::{ParseError, StyleParseErrorKind};
 pub type RayFunction = generics::GenericRayFunction<Angle, Position>;
 
 /// The specified value of <offset-path>.
-pub type OffsetPathFunction = generics::GenericOffsetPathFunction<BasicShape, RayFunction>;
+pub type OffsetPathFunction =
+    generics::GenericOffsetPathFunction<BasicShape, RayFunction, SpecifiedUrl>;
 
 /// The specified value of `offset-path`.
 pub type OffsetPath = generics::GenericOffsetPath<OffsetPathFunction>;
@@ -161,6 +163,12 @@ impl Parse for OffsetPathFunction {
             }
         }
 
+        if static_prefs::pref!("layout.css.motion-path-url.enabled") {
+            if let Ok(url) = input.try_parse(|i| SpecifiedUrl::parse(context, i)) {
+                return Ok(OffsetPathFunction::Url(url));
+            }
+        }
+
         let allowed_shapes = if static_prefs::pref!("layout.css.motion-path-basic-shapes.enabled") {
             AllowedBasicShapes::ALL
         } else {

servo/ports/geckolib/cbindgen.toml

@@ -994,6 +994,15 @@ renaming_overrides_prefixing = true
     return AsOffsetPath().path->AsRay();
   }
 
+  // Return true if the <offset-path> is url().
+  bool IsUrl() const {
+    return IsOffsetPath() && AsOffsetPath().path->IsUrl();
+  }
+
+  const StyleComputedUrl& AsUrl() const {
+    return AsOffsetPath().path->AsUrl();
+  }
+
   // Return true if the <basic-shape> is path().
   bool IsPath() const {
     return IsOffsetPath() && AsOffsetPath().path->IsShape() &&

servo/ports/geckolib/glue.rs

@@ -867,6 +867,19 @@ pub unsafe extern "C" fn Servo_AnimationValue_GetOffsetPosition(
     }
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn Servo_AnimationValue_IsOffsetPathUrl(
+    value: &AnimationValue,
+) -> bool {
+    use style::values::generics::motion::{GenericOffsetPath, GenericOffsetPathFunction};
+    if let AnimationValue::OffsetPath(ref op) = value {
+        if let GenericOffsetPath::OffsetPath { path, coord_box: _ } = op {
+            return matches!(**path, GenericOffsetPathFunction::Url(_));
+        }
+    }
+    false
+}
+
 #[no_mangle]
 pub unsafe extern "C" fn Servo_AnimationValue_Rotate(
     r: &computed::Rotate,

testing/web-platform/meta/css/motion/__dir__.ini

@@ -1 +1 @@
-prefs: [layout.css.motion-path.enabled:true, layout.css.individual-transform.enabled:true, dom.animations-api.core.enabled:true, layout.css.motion-path-ray.enabled:true, layout.css.motion-path-offset-position.enabled:true, layout.css.motion-path-basic-shapes.enabled:true, layout.css.motion-path-coord-box.enabled:true, layout.css.basic-shape-rect.enabled:true, layout.css.basic-shape-xywh.enabled:true]
+prefs: [layout.css.motion-path.enabled:true, layout.css.individual-transform.enabled:true, dom.animations-api.core.enabled:true, layout.css.motion-path-ray.enabled:true, layout.css.motion-path-offset-position.enabled:true, layout.css.motion-path-basic-shapes.enabled:true, layout.css.motion-path-coord-box.enabled:true, layout.css.basic-shape-rect.enabled:true, layout.css.basic-shape-xywh.enabled:true, layout.css.motion-path-url.enabled:true]

testing/web-platform/meta/css/motion/parsing/offset-path-parsing-valid.html.ini

@@ -1,3 +0,0 @@
-[offset-path-parsing-valid.html]
-  [e.style['offset-path'\] = "url(\\"http://www.example.com/index.html#polyline1\\")" should set the property value]
-    expected: FAIL