commit 1edc0ba8328ad9d280b2f35b5f3ea377049b5517 parent 18c2a1971158fcfe109304816eec390d63f2b8a4 Author: Swarup Ukil <sukil@mozilla.com> Date: Mon, 24 Nov 2025 03:18:24 +0000 Bug 1982757 - Implement direction-agnostic arc radii handling in shape(). r=emilio,firefox-style-system-reviewers,layout-reviewers Differential Revision: https://phabricator.services.mozilla.com/D272823 Diffstat:
27 files changed, 110 insertions(+), 119 deletions(-)
diff --git a/dom/svg/SVGAnimatedPathSegList.cpp b/dom/svg/SVGAnimatedPathSegList.cpp @@ -124,7 +124,8 @@ class MOZ_STACK_CLASS SVGPathSegmentInitWrapper final { return StylePathCommand::Arc( MakeEndPoint(PositionType::Absolute, mInit.mValues[5], mInit.mValues[6]), - {mInit.mValues[0], mInit.mValues[1]}, + StyleArcRadii<float>(mInit.mValues[0], + StyleOptional<float>::Some(mInit.mValues[1])), mInit.mValues[4] ? StyleArcSweep::Cw : StyleArcSweep::Ccw, mInit.mValues[3] ? StyleArcSize::Large : StyleArcSize::Small, mInit.mValues[2]); @@ -132,7 +133,8 @@ class MOZ_STACK_CLASS SVGPathSegmentInitWrapper final { return StylePathCommand::Arc( MakeEndPoint(PositionType::Relative, mInit.mValues[5], mInit.mValues[6]), - {mInit.mValues[0], mInit.mValues[1]}, + StyleArcRadii<float>(mInit.mValues[0], + StyleOptional<float>::Some(mInit.mValues[1])), mInit.mValues[4] ? StyleArcSweep::Cw : StyleArcSweep::Ccw, mInit.mValues[3] ? StyleArcSize::Large : StyleArcSize::Small, mInit.mValues[2]); diff --git a/dom/svg/SVGPathData.cpp b/dom/svg/SVGPathData.cpp @@ -598,8 +598,9 @@ void SVGPathData::GetMarkerPositioningData(Span<const StylePathCommand> aPath, } case StylePathCommand::Tag::Arc: { const auto& arc = cmd.arc; - float rx = arc.radii.x * aZoom; - float ry = arc.radii.y * aZoom; + auto radii = arc.radii.ToGfxPoint() * aZoom; + float rx = radii.x; + float ry = radii.y; float angle = arc.rotate; bool largeArcFlag = arc.arc_size == StyleArcSize::Large; bool sweepFlag = arc.arc_sweep == StyleArcSweep::Cw; diff --git a/dom/svg/SVGPathSegment.cpp b/dom/svg/SVGPathSegment.cpp @@ -73,15 +73,17 @@ SVGPathSegment::SVGPathSegment(SVGPathElement* aSVGPathElement, AppendControlPoint(aCommand.quad_curve.control1); AppendEndPoint(aCommand.quad_curve.point); break; - case StylePathCommand::Tag::Arc: + case StylePathCommand::Tag::Arc: { mCommand.AssignLiteral(aCommand.arc.point.IsToPosition() ? "A" : "a"); - mValues.AppendElement(aCommand.arc.radii.x); - mValues.AppendElement(aCommand.arc.radii.y); + const auto r = aCommand.arc.radii.ToGfxPoint(); + mValues.AppendElement(r.x); + mValues.AppendElement(r.y); mValues.AppendElement(aCommand.arc.rotate); mValues.AppendElement(aCommand.arc.arc_size == StyleArcSize::Large); mValues.AppendElement(aCommand.arc.arc_sweep == StyleArcSweep::Cw); AppendEndPoint(aCommand.arc.point); break; + } case StylePathCommand::Tag::HLine: mCommand.AssignLiteral(aCommand.h_line.by_to == StyleByTo::To ? "H" : "h"); diff --git a/layout/style/ServoStyleConstsInlines.h b/layout/style/ServoStyleConstsInlines.h @@ -16,6 +16,7 @@ #include "mozilla/AspectRatio.h" #include "mozilla/EndianUtils.h" #include "mozilla/IntegerRange.h" +#include "mozilla/SVGContentUtils.h" #include "mozilla/ServoStyleConsts.h" #include "mozilla/URLExtraData.h" #include "mozilla/dom/WorkerCommon.h" @@ -1372,6 +1373,30 @@ StyleControlPoint<StyleShapePosition<LengthPercentage>, } } +template <> +inline gfx::Point StyleArcRadii<StyleCSSFloat>::ToGfxPoint( + const CSSSize* aBasis) const { + return ry.IsSome() ? gfx::Point(rx, ry.AsSome()) : gfx::Point(rx, rx); +} + +template <> +inline gfx::Point StyleArcRadii<LengthPercentage>::ToGfxPoint( + const CSSSize* aBasis) const { + MOZ_ASSERT(aBasis); + if (ry.IsSome()) { + return gfx::Point(rx.ResolveToCSSPixels(aBasis->Width()), + ry.AsSome().ResolveToCSSPixels(aBasis->Height())); + } + + // Else percentages are resolved against the direction-agnostic size + // of the reference box for both radiuses. + // https://drafts.csswg.org/css-shapes-1/#typedef-shape-arc-command + const auto directionAgnostic = SVGContentUtils::ComputeNormalizedHypotenuse( + aBasis->Width(), aBasis->Height()); + const auto radius = rx.ResolveToCSSPixels(directionAgnostic); + return gfx::Point(radius, radius); +} + inline StylePhysicalSide ToStylePhysicalSide(mozilla::Side aSide) { // TODO(dshin): Should look into merging these two types... static_assert(static_cast<uint8_t>(mozilla::Side::eSideLeft) == diff --git a/servo/components/style/values/computed/basic_shape.rs b/servo/components/style/values/computed/basic_shape.rs @@ -248,3 +248,16 @@ impl From<&generic::ControlPoint<ShapePosition<CSSFloat>, CSSFloat>> for Control } } } + +impl From<&generic::ArcRadii<CSSFloat>> for generic::ArcRadii<LengthPercentage> { + #[inline] + fn from(p: &generic::ArcRadii<CSSFloat>) -> Self { + use crate::values::computed::CSSPixelLength; + Self { + rx: LengthPercentage::new_length(CSSPixelLength::new(p.rx)), + ry: p + .ry + .map(|v| LengthPercentage::new_length(CSSPixelLength::new(v))), + } + } +} diff --git a/servo/components/style/values/generics/basic_shape.rs b/servo/components/style/values/generics/basic_shape.rs @@ -11,7 +11,7 @@ use crate::values::generics::{ border::GenericBorderRadius, position::{GenericPosition, GenericPositionOrAuto}, rect::Rect, - NonNegative, + NonNegative, Optional, }; use crate::values::specified::svg_path::{PathCommand, SVGPathData}; use crate::Zero; @@ -794,7 +794,7 @@ pub enum GenericShapeCommand<Angle, Position, LengthPercentage> { /// The arc command. Arc { point: CommandEndPoint<Position, LengthPercentage>, - radii: CoordinatePair<LengthPercentage>, + radii: ArcRadii<LengthPercentage>, arc_sweep: ArcSweep, arc_size: ArcSize, rotate: Angle, @@ -883,11 +883,7 @@ where dest.write_str("arc ")?; point.to_css(dest)?; dest.write_str(" of ")?; - radii.x.to_css(dest)?; - if radii.x != radii.y { - dest.write_char(' ')?; - radii.y.to_css(dest)?; - } + radii.to_css(dest)?; if matches!(arc_sweep, ArcSweep::Cw) { dest.write_str(" cw")?; @@ -1091,7 +1087,7 @@ impl<Position, LengthPercentage> ControlPoint<Position, LengthPercentage> { } /// Defines a relative control point to a quadratic or cubic Bézier curve, dependent on the -/// reference value. The default "none" is to be relative to the command’s starting point. +/// reference value. The default `None` is to be relative to the command’s starting point. /// https://drafts.csswg.org/css-shapes/#typedef-shape-relative-control-point #[allow(missing_docs)] #[derive( @@ -1179,6 +1175,37 @@ pub enum ControlReference { Origin, } +/// Defines the radiuses for an <arc-command>. +/// +/// The first <length-percentage> is the ellipse's horizontal radius, and the second is +/// the vertical radius. If only one value is provided, it is used for both radii, and any +/// <percentage> is resolved against the direction-agnostic size of the reference box. +/// https://drafts.csswg.org/css-shapes-1/#typedef-shape-arc-command +#[allow(missing_docs)] +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + Deserialize, + MallocSizeOf, + PartialEq, + Serialize, + SpecifiedValueInfo, + ToAnimatedValue, + ToAnimatedZero, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(C)] +pub struct ArcRadii<LengthPercentage> { + pub rx: LengthPercentage, + pub ry: Optional<LengthPercentage>, +} + /// This indicates that the arc that is traced around the ellipse clockwise or counter-clockwise /// from the center. /// https://drafts.csswg.org/css-shapes-2/#typedef-shape-arc-sweep diff --git a/servo/components/style/values/specified/basic_shape.rs b/servo/components/style/values/specified/basic_shape.rs @@ -779,7 +779,7 @@ impl Parse for ShapeCommand { input: &mut Parser<'i, 't>, ) -> Result<Self, ParseError<'i>> { use crate::values::generics::basic_shape::{ - ArcSize, ArcSweep, ByTo, CommandEndPoint, ControlPoint, CoordinatePair, + ArcRadii, ArcSize, ArcSweep, ByTo, CommandEndPoint, ControlPoint, }; // <shape-command> = <move-command> | <line-command> | <hv-line-command> | @@ -856,10 +856,8 @@ impl Parse for ShapeCommand { let point = CommandEndPoint::parse(context, input, by_to)?; input.expect_ident_matching("of")?; let rx = LengthPercentage::parse(context, input)?; - let ry = input - .try_parse(|i| LengthPercentage::parse(context, i)) - .unwrap_or(rx.clone()); - let radii = CoordinatePair::new(rx, ry); + let ry = input.try_parse(|i| LengthPercentage::parse(context, i)).ok(); + let radii = ArcRadii { rx, ry: ry.into() }; // [<arc-sweep> || <arc-size> || rotate <angle>]? let mut arc_sweep = None; diff --git a/servo/components/style/values/specified/svg_path.rs b/servo/components/style/values/specified/svg_path.rs @@ -9,8 +9,8 @@ use crate::values::animated::{lists, Animate, Procedure}; use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; use crate::values::generics::basic_shape::GenericShapeCommand; use crate::values::generics::basic_shape::{ - ArcSize, ArcSweep, ByTo, CommandEndPoint, ControlPoint, ControlReference, CoordinatePair, - RelativeControlPoint, ShapePosition, + ArcRadii, ArcSize, ArcSweep, ByTo, CommandEndPoint, ControlPoint, ControlReference, + CoordinatePair, RelativeControlPoint, ShapePosition, }; use crate::values::generics::position::GenericPosition; use crate::values::CSSFloat; @@ -380,7 +380,7 @@ impl PathCommand { state.pos = point.into(); if reduce { state.last_command = *self; - if radii.x == 0. && radii.y == 0. { + if radii.rx == 0. && radii.ry.as_ref().is_none_or(|v| *v == 0.) { let end_point = CoordPair::from(point); CubicCurve { point: CommandEndPoint::ToPosition(state.pos.into()), @@ -860,7 +860,7 @@ impl<'a> PathParser<'a> { }; if by_to.is_abs() { parse_arguments!(self, Arc, [ - radii => parse_coord, + radii => parse_arc, rotate => parse_number, arc_size => parse_arc_size, arc_sweep => parse_arc_sweep, @@ -868,7 +868,7 @@ impl<'a> PathParser<'a> { ]) } else { parse_arguments!(self, Arc, [ - radii => parse_coord, + radii => parse_arc, rotate => parse_number, arc_size => parse_arc_size, arc_sweep => parse_arc_sweep, @@ -916,6 +916,14 @@ fn parse_control_point( })) } +fn parse_arc(iter: &mut Peekable<Cloned<slice::Iter<u8>>>) -> Result<ArcRadii<CSSFloat>, ()> { + let coord = parse_coord(iter)?; + Ok(ArcRadii { + rx: coord.x, + ry: Some(coord.y).into(), + }) +} + /// This is a special version which parses the number for SVG Path. e.g. "M 0.6.5" should be parsed /// as MoveTo with a coordinate of ("0.6", ".5"), instead of treating 0.6.5 as a non-valid floating /// point number. In other words, the logic here is similar with that of diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml @@ -879,6 +879,13 @@ renaming_overrides_prefixing = true } """ +"ArcRadii" = """ + inline gfx::Point ToGfxPoint(const CSSSize* aBasis = nullptr) const; + gfx::Point ToGfxPoint(const CSSSize& aBasis) const { + return ToGfxPoint(&aBasis); + }; +""" + "TextOverflow" = """ StyleTextOverflow() : first(StyleTextOverflowSide::Clip()), diff --git a/testing/web-platform/meta/css/css-masking/animations/clip-path-interpolation-shape-arc-direction-agnostic-radius.html.ini b/testing/web-platform/meta/css/css-masking/animations/clip-path-interpolation-shape-arc-direction-agnostic-radius.html.ini @@ -1,2 +0,0 @@ -[clip-path-interpolation-shape-arc-direction-agnostic-radius.html] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-masking/clip-path/animations/clip-path-shape-interpolation-003.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/animations/clip-path-shape-interpolation-003.html.ini @@ -1,5 +0,0 @@ -[clip-path-shape-interpolation-003.html] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - if (os == "android") and not debug: [PASS, FAIL] - if (os == "win") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-001.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-001.html.ini @@ -1,3 +0,0 @@ -[clip-path-shape-001.html] - expected: - if (os == "win") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-002-units.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-002-units.html.ini @@ -1,3 +0,0 @@ -[clip-path-shape-002-units.html] - expected: - if (os == "win") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-002.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-002.html.ini @@ -1,3 +0,0 @@ -[clip-path-shape-002.html] - expected: - if (os == "win") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-004.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-004.html.ini @@ -1,3 +0,0 @@ -[clip-path-shape-004.html] - expected: - if (os == "win") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-005.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-005.html.ini @@ -1,3 +0,0 @@ -[clip-path-shape-005.html] - expected: - if (os == "win") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-006.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-006.html.ini @@ -1,3 +0,0 @@ -[clip-path-shape-006.html] - expected: - if (os == "win") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-007.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-007.html.ini @@ -1,4 +0,0 @@ -[clip-path-shape-007.html] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - if (os == "android") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-008.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-008.html.ini @@ -1,4 +0,0 @@ -[clip-path-shape-008.html] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - if (os == "android") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-009.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-009.html.ini @@ -1,4 +0,0 @@ -[clip-path-shape-009.html] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - if (os == "android") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-010.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-010.html.ini @@ -1,4 +0,0 @@ -[clip-path-shape-010.html] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - if (os == "android") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-011.html.ini b/testing/web-platform/meta/css/css-masking/clip-path/clip-path-shape-011.html.ini @@ -1,2 +0,0 @@ -[clip-path-shape-011.html] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-masking/parsing/clip-path-shape-parsing.html.ini b/testing/web-platform/meta/css/css-masking/parsing/clip-path-shape-parsing.html.ini @@ -1,6 +0,0 @@ -[clip-path-shape-parsing.html] - [e.style['clip-path'\] = "shape(from 10px 10px, arc to 50px 1pt of 10px 10px)" should set the property value] - expected: FAIL - - [e.style['clip-path'\] = "shape(from 10px 10px, arc to 50px 1pt of 10px 10px small rotate 0deg)" should set the property value] - expected: FAIL diff --git a/testing/web-platform/meta/css/motion/parsing/offset-path-shape-computed.html.ini b/testing/web-platform/meta/css/motion/parsing/offset-path-shape-computed.html.ini @@ -1,33 +0,0 @@ -[offset-path-shape-computed.html] - [Property offset-path value 'shape(from 0px 0px, line to 10px 10px)'] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - - [Property offset-path value 'shape(from 1em 50px, line to 10rem 10%)'] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - - [Property offset-path value 'shape(from 10px 10px, move by 10px 5px, line by 20px 40%, close)'] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - - [Property offset-path value 'shape(from 10px 10px, hline by 10px, vline to 5rem)'] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - - [Property offset-path value 'shape(from 10px 10px, vline by 5%, hline to 1px)'] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - - [Property offset-path value 'shape(from 10px 10px, smooth to 50px 3pt)'] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - - [Property offset-path value 'shape(from 10px 10px, arc to 50px 3pt of 10px 10px)'] - expected: - if (os == "mac") and not debug: [FAIL, PASS] - FAIL - - [Property offset-path value 'shape(from 10% 1rem, arc to 50px 3pt of 20% cw large rotate 25deg)'] - expected: - if (os == "mac") and not debug: [PASS, FAIL] diff --git a/testing/web-platform/meta/css/motion/parsing/offset-path-shape-parsing.html.ini b/testing/web-platform/meta/css/motion/parsing/offset-path-shape-parsing.html.ini @@ -1,14 +1,4 @@ [offset-path-shape-parsing.html] - [e.style['offset-path'\] = "shape(from 10px 10px, arc to 50px 1pt of 10px 10px)" should set the property value] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - FAIL - - [e.style['offset-path'\] = "shape(from 10px 10px, arc to 50px 1pt of 10px 10px small rotate 0deg)" should set the property value] - expected: - if (os == "mac") and not debug: [PASS, FAIL] - FAIL - [e.style['offset-path'\] = "shape(evenodd from 0px 0px, line to 10px 10px)" should set the property value] expected: FAIL diff --git a/testing/web-platform/tests/css/css-masking/animations/clip-path-interpolation-shape-arc-direction-agnostic-radius.html b/testing/web-platform/tests/css/css-masking/animations/clip-path-interpolation-shape-arc-direction-agnostic-radius.html @@ -4,7 +4,7 @@ <title>CSS Masking: Test animating single-value arc radius of the shape() function</title> <link rel="help" href="https://drafts.csswg.org/css-shapes-2/#funcdef-shape"> <link rel="match" href="clip-path-interpolation-shape-arc-direction-agnostic-radius-ref.html"> - <meta name="fuzzy" content="maxDifference=0-10;totalPixels=0-360"> + <meta name="fuzzy" content="maxDifference=0-69;totalPixels=0-360"> </head> <style> @keyframes animate-shape { diff --git a/testing/web-platform/tests/css/css-masking/clip-path/clip-path-shape-011.html b/testing/web-platform/tests/css/css-masking/clip-path/clip-path-shape-011.html @@ -5,7 +5,7 @@ <link rel="help" href="https://drafts.csswg.org/css-shapes-2/#funcdef-shape"> <link rel="match" href="reference/clip-path-shape-arc-ref.html"> <meta name="assert" content="When providing one radius value with percentage, the percentage should be relative to the diagonal."> -<meta name="fuzzy" content="maxDifference=0-64;totalPixels=0-128"> +<meta name="fuzzy" content="maxDifference=0-67;totalPixels=0-128"> </head> <style> #shape {