From a1909a88ffe34b3d552ef5d59fb4954c26529085 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Tue, 14 Aug 2018 18:58:18 -0700 Subject: [PATCH] Bug 1246764 - Part 2: Define path() for clip-path. r=emilio For now, |clip-path: path()| is chrome-only, and not for shape-outside, so we only implement the parser for clip-path. Besides, I didn't put path() in BasicShape because path() doesn't use the reference box to resolve the percentage or keywords (i.e. SVG path only accept floating point or integer number as the css pixel value). Therefore, I add it into ShapeSource, instead of BasicShape. Differential Revision: https://phabricator.services.mozilla.com/D3633 --- .../shared/css/generated/properties-db.js | 2 + layout/inspector/tests/test_bug877690.html | 2 +- layout/style/nsStyleStruct.h | 8 ++- layout/style/test/property_database.js | 13 ++++ servo/components/style/gecko/conversions.rs | 42 ++++++++---- .../components/style/properties/gecko.mako.rs | 49 +++++++++----- .../style/values/generics/basic_shape.rs | 17 +++++ .../style/values/specified/basic_shape.rs | 64 ++++++++++++++++++- 8 files changed, 166 insertions(+), 31 deletions(-) diff --git a/devtools/shared/css/generated/properties-db.js b/devtools/shared/css/generated/properties-db.js index 50499051695b..d4d08f10fe3c 100644 --- a/devtools/shared/css/generated/properties-db.js +++ b/devtools/shared/css/generated/properties-db.js @@ -4884,6 +4884,7 @@ exports.CSS_PROPERTIES = { "margin-box", "none", "padding-box", + "path", "polygon", "stroke-box", "unset", @@ -8269,6 +8270,7 @@ exports.CSS_PROPERTIES = { "margin-box", "none", "padding-box", + "path", "polygon", "radial-gradient", "repeating-linear-gradient", diff --git a/layout/inspector/tests/test_bug877690.html b/layout/inspector/tests/test_bug877690.html index 52da3d8aaa33..8fd8399657b1 100644 --- a/layout/inspector/tests/test_bug877690.html +++ b/layout/inspector/tests/test_bug877690.html @@ -190,7 +190,7 @@ function do_test() { // Regression test for bug 1255379. var expected = [ "inherit", "initial", "unset", "none", "url", - "polygon", "circle", "ellipse", "inset", + "polygon", "circle", "ellipse", "inset", "path", "fill-box", "stroke-box", "view-box", "margin-box", "border-box", "padding-box", "content-box" ]; var values = InspectorUtils.getCSSValuesForProperty("clip-path"); diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h index d1c7b8142cce..662de8297f39 100644 --- a/layout/style/nsStyleStruct.h +++ b/layout/style/nsStyleStruct.h @@ -1978,9 +1978,14 @@ struct StyleSVGPath final return mPath; } + StyleFillRule FillRule() const + { + return mFillRule; + } + bool operator==(const StyleSVGPath& aOther) const { - return mPath == aOther.mPath; + return mPath == aOther.mPath && mFillRule == aOther.mFillRule; } bool operator!=(const StyleSVGPath& aOther) const @@ -1990,6 +1995,7 @@ struct StyleSVGPath final private: nsTArray mPath; + StyleFillRule mFillRule = StyleFillRule::Nonzero; }; struct StyleShapeSource final diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js index e38f81cf8e9a..5d48a338b4ac 100644 --- a/layout/style/test/property_database.js +++ b/layout/style/test/property_database.js @@ -8129,6 +8129,19 @@ if (false) { other_values: [ "green", "#fc3" ], invalid_values: [ "000000", "ff00ff" ] }; + + // |clip-path: path()| is chrome-only. + gCSSProperties["clip-path"].other_values.push( + "path(nonzero, 'M 10 10 h 100 v 100 h-100 v-100 z')", + "path(evenodd, 'M 10 10 h 100 v 100 h-100 v-100 z')", + "path('M10,30A20,20 0,0,1 50,30A20,20 0,0,1 90,30Q90,60 50,90Q10,60 10,30z')", + ); + + gCSSProperties["clip-path"].invalid_values.push( + "path(nonzero)", + "path(evenodd, '')", + "path(abs, 'M 10 10 L 10 10 z')", + ); } if (IsCSSPropertyPrefEnabled("layout.css.filters.enabled")) { diff --git a/servo/components/style/gecko/conversions.rs b/servo/components/style/gecko/conversions.rs index a3109a1f2264..e85bc8ec3103 100644 --- a/servo/components/style/gecko/conversions.rs +++ b/servo/components/style/gecko/conversions.rs @@ -670,10 +670,11 @@ pub mod basic_shape { use values::computed::position; use values::computed::url::ComputedUrl; use values::generics::basic_shape::{BasicShape as GenericBasicShape, InsetRect, Polygon}; - use values::generics::basic_shape::{Circle, Ellipse, FillRule, PolygonCoord}; + use values::generics::basic_shape::{Circle, Ellipse, FillRule, Path, PolygonCoord}; use values::generics::basic_shape::{GeometryBox, ShapeBox, ShapeSource}; use values::generics::border::BorderRadius as GenericBorderRadius; use values::generics::rect::Rect; + use values::specified::SVGPathData; impl StyleShapeSource { /// Convert StyleShapeSource to ShapeSource except URL and Image @@ -698,7 +699,34 @@ pub mod basic_shape { Some(ShapeSource::Shape(shape, reference_box)) }, StyleShapeSourceType::URL | StyleShapeSourceType::Image => None, - StyleShapeSourceType::Path => None, + StyleShapeSourceType::Path => { + let path = self.to_svg_path().expect("expect an SVGPathData"); + let gecko_path = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }; + let fill = if gecko_path.mFillRule == StyleFillRule::Evenodd { + FillRule::Evenodd + } else { + FillRule::Nonzero + }; + Some(ShapeSource::Path(Path { fill, path })) + }, + } + } + + /// Generate a SVGPathData from StyleShapeSource if possible. + fn to_svg_path(&self) -> Option { + use gecko_bindings::structs::StylePathCommand; + use 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 = + gecko_path.mPath.iter().map(|gecko: &StylePathCommand| { + // unsafe: cbindgen ensures the representation is the same. + unsafe{ ::std::mem::transmute(*gecko) } + }).collect(); + Some(SVGPathData::new(result.into_boxed_slice())) + }, + _ => None, } } } @@ -742,17 +770,9 @@ pub mod basic_shape { impl<'a> From<&'a StyleShapeSource> for OffsetPath { fn from(other: &'a StyleShapeSource) -> Self { - use gecko_bindings::structs::StylePathCommand; - use values::specified::svg_path::{SVGPathData, PathCommand}; match other.mType { StyleShapeSourceType::Path => { - let gecko_path = unsafe { &*other.__bindgen_anon_1.mSVGPath.as_ref().mPtr }; - let result: Vec = - gecko_path.mPath.iter().map(|gecko: &StylePathCommand| { - // unsafe: cbindgen ensures the representation is the same. - unsafe{ ::std::mem::transmute(*gecko) } - }).collect(); - OffsetPath::Path(SVGPathData::new(result.into_boxed_slice())) + OffsetPath::Path(other.to_svg_path().expect("Cannot convert to SVGPathData")) }, StyleShapeSourceType::None => OffsetPath::none(), StyleShapeSourceType::Shape | diff --git a/servo/components/style/properties/gecko.mako.rs b/servo/components/style/properties/gecko.mako.rs index 76d61838708a..9e9a6835a426 100644 --- a/servo/components/style/properties/gecko.mako.rs +++ b/servo/components/style/properties/gecko.mako.rs @@ -3683,27 +3683,16 @@ fn static_assert() { ${impl_simple_type_with_conversion("touch_action")} pub fn set_offset_path(&mut self, v: longhands::offset_path::computed_value::T) { - use gecko_bindings::bindings::{Gecko_NewStyleMotion, Gecko_NewStyleSVGPath}; - use gecko_bindings::bindings::Gecko_SetStyleMotion; + use gecko_bindings::bindings::{Gecko_NewStyleMotion, Gecko_SetStyleMotion}; use gecko_bindings::structs::StyleShapeSourceType; + use values::generics::basic_shape::FillRule; use values::specified::OffsetPath; let motion = unsafe { Gecko_NewStyleMotion().as_mut().unwrap() }; match v { OffsetPath::None => motion.mOffsetPath.mType = StyleShapeSourceType::None, - OffsetPath::Path(servo_path) => { - motion.mOffsetPath.mType = StyleShapeSourceType::Path; - let gecko_path = unsafe { - let ref mut source = motion.mOffsetPath; - Gecko_NewStyleSVGPath(source); - &mut source.__bindgen_anon_1.mSVGPath.as_mut().mPtr.as_mut().unwrap().mPath - }; - unsafe { gecko_path.set_len(servo_path.commands().len() as u32) }; - debug_assert_eq!(gecko_path.len(), servo_path.commands().len()); - for (servo, gecko) in servo_path.commands().iter().zip(gecko_path.iter_mut()) { - // unsafe: cbindgen ensures the representation is the same. - *gecko = unsafe { transmute(*servo) }; - } + OffsetPath::Path(p) => { + set_style_svg_path(&mut motion.mOffsetPath, &p, FillRule::Nonzero) }, } unsafe { Gecko_SetStyleMotion(&mut self.gecko.mMotion, motion) }; @@ -4982,6 +4971,35 @@ 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, + fill: values::generics::basic_shape::FillRule, +) { + use gecko_bindings::bindings::Gecko_NewStyleSVGPath; + use 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() + }; + unsafe { gecko_path.mPath.set_len(servo_path.commands().len() as u32) }; + debug_assert_eq!(gecko_path.mPath.len(), servo_path.commands().len()); + for (servo, gecko) in servo_path.commands().iter().zip(gecko_path.mPath.iter_mut()) { + // unsafe: cbindgen ensures the representation is the same. + *gecko = unsafe { transmute(*servo) }; + } + + // Setup fill-rule. + // unsafe: cbindgen ensures the representation is the same. + gecko_path.mFillRule = unsafe { transmute(fill) }; +} + <%def name="impl_shape_source(ident, gecko_ffi_name)"> pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { use gecko_bindings::bindings::{Gecko_NewBasicShape, Gecko_DestroyShapeSource}; @@ -5021,6 +5039,7 @@ fn static_assert() { ${ident}.mReferenceBox = reference.into(); ${ident}.mType = StyleShapeSourceType::Box; } + ShapeSource::Path(p) => set_style_svg_path(${ident}, &p.path, p.fill), ShapeSource::Shape(servo_shape, maybe_box) => { fn init_shape(${ident}: &mut StyleShapeSource, basic_shape_type: StyleBasicShapeType) -> &mut StyleBasicShape { diff --git a/servo/components/style/values/generics/basic_shape.rs b/servo/components/style/values/generics/basic_shape.rs index ee7455e777de..0eccf011c716 100644 --- a/servo/components/style/values/generics/basic_shape.rs +++ b/servo/components/style/values/generics/basic_shape.rs @@ -12,6 +12,7 @@ use values::distance::{ComputeSquaredDistance, SquaredDistance}; use values::generics::border::BorderRadius; use values::generics::position::Position; use values::generics::rect::Rect; +use values::specified::SVGPathData; /// A clipping shape, for `clip-path`. pub type ClippingShape = ShapeSource; @@ -54,6 +55,9 @@ pub enum ShapeSource { #[animation(error)] Box(ReferenceBox), #[animation(error)] + #[css(function)] + Path(Path), + #[animation(error)] None, } @@ -144,6 +148,19 @@ pub enum FillRule { Evenodd, } +/// The path function defined in css-shape-2. +/// +/// https://drafts.csswg.org/css-shapes-2/#funcdef-path +#[css(comma)] +#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)] +pub struct Path { + /// The filling rule for the svg path. + #[css(skip_if = "fill_is_default")] + pub fill: FillRule, + /// The svg path data. + pub path: SVGPathData, +} + // FIXME(nox): Implement ComputeSquaredDistance for T types and stop // using PartialEq here, this will let us derive this impl. impl ComputeSquaredDistance for ShapeSource diff --git a/servo/components/style/values/specified/basic_shape.rs b/servo/components/style/values/specified/basic_shape.rs index 2fa693a1978a..824c13f1f014 100644 --- a/servo/components/style/values/specified/basic_shape.rs +++ b/servo/components/style/values/specified/basic_shape.rs @@ -14,9 +14,11 @@ use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; use values::computed::Percentage; use values::generics::basic_shape as generic; -use values::generics::basic_shape::{FillRule, GeometryBox, PolygonCoord, ShapeBox, ShapeSource}; +use values::generics::basic_shape::{FillRule, GeometryBox, Path, PolygonCoord}; +use values::generics::basic_shape::{ShapeBox, ShapeSource}; use values::generics::rect::Rect; use values::specified::LengthOrPercentage; +use values::specified::SVGPathData; use values::specified::border::BorderRadius; use values::specified::image::Image; use values::specified::position::{HorizontalPosition, Position, PositionComponent}; @@ -47,12 +49,42 @@ pub type ShapeRadius = generic::ShapeRadius; /// The specified value of `Polygon` pub type Polygon = generic::Polygon; -impl Parse for ShapeSource +impl Parse for ClippingShape { + #[inline] + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + // |clip-path:path()| is a chrome-only property value support for now. `path()` is + // defined in css-shape-2, but the spec is not stable enough, and we haven't decided + // to make it public yet. However, it has some benefits for the front-end, so we + // implement it. + if context.chrome_rules_enabled() { + if let Ok(p) = input.try(|i| Path::parse(context, i)) { + return Ok(ShapeSource::Path(p)); + } + } + Self::parse_internal(context, input) + } +} + +impl Parse for FloatAreaShape { + #[inline] + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + Self::parse_internal(context, input) + } +} + +impl ShapeSource where ReferenceBox: Parse, ImageOrUrl: Parse, { - fn parse<'i, 't>( + /// The internal parser for ShapeSource. + fn parse_internal<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { @@ -393,3 +425,29 @@ impl Polygon { }) } } + +impl Parse for Path { + fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + input.expect_function_matching("path")?; + input.parse_nested_block(|i| Self::parse_function_arguments(context, i)) + } +} + +impl Path { + /// Parse the inner arguments of a `path` function. + fn parse_function_arguments<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let fill = input.try(|i| -> Result<_, ParseError> { + let fill = FillRule::parse(i)?; + i.expect_comma()?; + Ok(fill) + }).unwrap_or_default(); + let path = SVGPathData::parse(context, input)?; + Ok(Path { fill, path }) + } +}