From 40f3cb3d657cffeb77e0386e6fafe1bfacd263fc Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 22 Jan 2020 18:40:11 +0000 Subject: [PATCH] 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 | 2 +- gfx/layers/ipc/LayersMessages.ipdlh | 48 +---------- layout/base/MotionPathUtils.cpp | 75 +---------------- layout/base/MotionPathUtils.h | 7 +- layout/painting/nsDisplayList.cpp | 26 +++--- layout/style/ServoBindings.h | 2 +- layout/style/StyleAnimationValue.cpp | 80 +------------------ .../style/values/generics/motion.rs | 3 + .../style/values/specified/svg_path.rs | 12 ++- servo/components/style_traits/arc_slice.rs | 21 +++++ servo/ports/geckolib/cbindgen.toml | 6 ++ servo/ports/geckolib/glue.rs | 41 +++------- 12 files changed, 72 insertions(+), 251 deletions(-) diff --git a/gfx/layers/ipc/LayersMessageUtils.h b/gfx/layers/ipc/LayersMessageUtils.h index c94373b3822f..429393702202 100644 --- a/gfx/layers/ipc/LayersMessageUtils.h +++ b/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) diff --git a/gfx/layers/ipc/LayersMessages.ipdlh b/gfx/layers/ipc/LayersMessages.ipdlh index a8b3764dd37e..05f14200717f 100644 --- a/gfx/layers/ipc/LayersMessages.ipdlh +++ b/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; diff --git a/layout/base/MotionPathUtils.cpp b/layout/base/MotionPathUtils.cpp index 56048a28d82c..300a65995541 100644 --- a/layout/base/MotionPathUtils.cpp +++ b/layout/base/MotionPathUtils.cpp @@ -543,82 +543,11 @@ Maybe MotionPathUtils::ResolveMotionPath( } /* static */ -nsTArray -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 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 */ diff --git a/layout/base/MotionPathUtils.h b/layout/base/MotionPathUtils.h index 10fb5d873be7..98358d7f7030 100644 --- a/layout/base/MotionPathUtils.h +++ b/layout/base/MotionPathUtils.h @@ -184,16 +184,15 @@ class MotionPathUtils final { const CSSSize& aFrameSize, gfx::Path* aCachedMotionPath); /** - * Normalize and convert StyleSVGPathData into nsTArray. + * 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 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 diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp index c30011006e09..209c8fce763b 100644 --- a/layout/painting/nsDisplayList.cpp +++ b/layout/painting/nsDisplayList.cpp @@ -332,21 +332,14 @@ static TimingFunction ToTimingFunction( aCTF->GetSteps().mSteps, static_cast(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: diff --git a/layout/style/ServoBindings.h b/layout/style/ServoBindings.h index 53e8fa20affc..de84fd50bdcf 100644 --- a/layout/style/ServoBindings.h +++ b/layout/style/ServoBindings.h @@ -65,11 +65,11 @@ BASIC_RULE_FUNCS(CounterStyle) using RayFunction = StyleRayFunction; 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 diff --git a/layout/style/StyleAnimationValue.cpp b/layout/style/StyleAnimationValue.cpp index 8263fa51c476..105a1f7b16fb 100644 --- a/layout/style/StyleAnimationValue.cpp +++ b/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 CreatePathCommandList( - const nsTArray& aCommands) { - nsTArray 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 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 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()) diff --git a/servo/components/style/values/generics/motion.rs b/servo/components/style/values/generics/motion.rs index 685eaad43e29..25ba5873adc9 100644 --- a/servo/components/style/values/generics/motion.rs +++ b/servo/components/style/values/generics/motion.rs @@ -73,13 +73,16 @@ pub struct RayFunction { /// 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, diff --git a/servo/components/style/values/specified/svg_path.rs b/servo/components/style/values/specified/svg_path.rs index 9a94af6a82f3..b5461bb37fc6 100644 --- a/servo/components/style/values/specified/svg_path.rs +++ b/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); diff --git a/servo/components/style_traits/arc_slice.rs b/servo/components/style_traits/arc_slice.rs index bbbac1a07574..f5d0c56e7fc4 100644 --- a/servo/components/style_traits/arc_slice.rs +++ b/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 Default for ArcSlice { } } +impl Serialize for ArcSlice { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.deref().serialize(serializer) + } +} + +impl<'de, T: Deserialize<'de>> Deserialize<'de> for ArcSlice { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let r = Vec::deserialize(deserializer)?; + Ok(ArcSlice::from_iter(r.into_iter())) + } +} + impl ArcSlice { /// Creates an Arc for a slice using the given iterator to generate the /// slice. diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml index 5ccfea6817c5..61d9aa53d46b 100644 --- a/servo/ports/geckolib/cbindgen.toml +++ b/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) {} +""" diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index ec12942d4882..a03274312cb8 100644 --- a/servo/ports/geckolib/glue.rs +++ b/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 { - 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, -) -> Strong { - 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 { - 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 -); - 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,