diff --git a/dom/svg/SVGContentUtils.cpp b/dom/svg/SVGContentUtils.cpp index 735bb90f8da8..a4d9286415ed 100644 --- a/dom/svg/SVGContentUtils.cpp +++ b/dom/svg/SVGContentUtils.cpp @@ -489,7 +489,9 @@ SVGViewportElement* SVGContentUtils::GetNearestViewportElement( return nullptr; } -static gfx::Matrix GetCTMInternal(SVGElement* aElement, bool aScreenCTM, +enum class CTMType { NearestViewport, OuterViewport, Screen }; + +static gfx::Matrix GetCTMInternal(SVGElement* aElement, CTMType aCTMType, bool aHaveRecursed) { auto getLocalTransformHelper = [](SVGElement const* e, bool shouldIncludeChildToUserSpace) -> gfxMatrix { @@ -530,7 +532,8 @@ static gfx::Matrix GetCTMInternal(SVGElement* aElement, bool aScreenCTM, !ancestor->IsSVGElement(nsGkAtoms::foreignObject)) { element = static_cast(ancestor); matrix *= getLocalTransformHelper(element, true); - if (!aScreenCTM && SVGContentUtils::EstablishesViewport(element)) { + if (aCTMType == CTMType::NearestViewport && + SVGContentUtils::EstablishesViewport(element)) { if (!element->IsAnyOfSVGElements(nsGkAtoms::svg, nsGkAtoms::symbol)) { NS_ERROR("New (SVG > 1.1) SVG viewport establishing element?"); return gfx::Matrix(0.0, 0.0, 0.0, 0.0, 0.0, 0.0); // singular @@ -540,7 +543,7 @@ static gfx::Matrix GetCTMInternal(SVGElement* aElement, bool aScreenCTM, } ancestor = ancestor->GetFlattenedTreeParent(); } - if (!aScreenCTM) { + if (aCTMType == CTMType::NearestViewport) { // didn't find a nearestViewportElement return gfx::Matrix(0.0, 0.0, 0.0, 0.0, 0.0, 0.0); // singular } @@ -568,13 +571,16 @@ static gfx::Matrix GetCTMInternal(SVGElement* aElement, bool aScreenCTM, int32_t appUnitsPerCSSPixel = AppUnitsPerCSSPixel(); tm.PostTranslate(NSAppUnitsToFloatPixels(bp.left, appUnitsPerCSSPixel), NSAppUnitsToFloatPixels(bp.top, appUnitsPerCSSPixel)); + if (aCTMType == CTMType::OuterViewport) { + return tm; + } } if (!ancestor || !ancestor->IsElement()) { return tm; } if (auto* ancestorSVG = SVGElement::FromNode(ancestor)) { - return tm * GetCTMInternal(ancestorSVG, true, true); + return tm * GetCTMInternal(ancestorSVG, aCTMType, true); } nsIFrame* parentFrame = frame->GetParent(); if (!parentFrame) { @@ -612,16 +618,20 @@ static gfx::Matrix GetCTMInternal(SVGElement* aElement, bool aScreenCTM, } return nearestSVGAncestor ? tm * GetCTMInternal(static_cast(nearestSVGAncestor), - true, true) + aCTMType, true) : tm; } gfx::Matrix SVGContentUtils::GetCTM(SVGElement* aElement) { - return GetCTMInternal(aElement, false, false); + return GetCTMInternal(aElement, CTMType::NearestViewport, false); +} + +gfx::Matrix SVGContentUtils::GetOuterViewportCTM(SVGElement* aElement) { + return GetCTMInternal(aElement, CTMType::OuterViewport, false); } gfx::Matrix SVGContentUtils::GetScreenCTM(SVGElement* aElement) { - return GetCTMInternal(aElement, true, false); + return GetCTMInternal(aElement, CTMType::Screen, false); } void SVGContentUtils::RectilinearGetStrokeBounds( diff --git a/dom/svg/SVGContentUtils.h b/dom/svg/SVGContentUtils.h index c62067ab64ca..ba394b404b62 100644 --- a/dom/svg/SVGContentUtils.h +++ b/dom/svg/SVGContentUtils.h @@ -197,6 +197,8 @@ class SVGContentUtils { static Matrix GetCTM(dom::SVGElement* aElement); + static Matrix GetOuterViewportCTM(dom::SVGElement* aElement); + static Matrix GetScreenCTM(dom::SVGElement* aElement); /** diff --git a/dom/svg/SVGGeometryElement.cpp b/dom/svg/SVGGeometryElement.cpp index 57b641f631e3..a550739c3243 100644 --- a/dom/svg/SVGGeometryElement.cpp +++ b/dom/svg/SVGGeometryElement.cpp @@ -201,9 +201,7 @@ bool SVGGeometryElement::IsPointInStroke(const DOMPointInit& aPoint) { SVGGeometryProperty::DoForComputedStyle(this, [&](const ComputedStyle* s) { // Per spec, we should take vector-effect into account. if (s->StyleSVGReset()->HasNonScalingStroke()) { - auto mat = s->StyleSVGReset()->mVectorEffect.IsScreen() - ? SVGContentUtils::GetScreenCTM(this) - : SVGContentUtils::GetCTM(this); + auto mat = SVGContentUtils::GetOuterViewportCTM(this); if (mat.HasNonTranslation()) { // We have non-scaling-stroke as well as a non-translation transform. // We should transform the path first then apply the stroke on the diff --git a/layout/style/ServoBindings.toml b/layout/style/ServoBindings.toml index 2d07183281ba..81cc093e29f7 100644 --- a/layout/style/ServoBindings.toml +++ b/layout/style/ServoBindings.toml @@ -587,8 +587,6 @@ cbindgen-types = [ { gecko = "StylePageOrientation", servo = "crate::values::generics::page::PageOrientation" }, { gecko = "StylePageSize", servo = "crate::values::computed::page::PageSize" }, { gecko = "StyleDProperty", servo = "crate::values::specified::svg::DProperty" }, - { gecko = "StyleVectorEffectType", servo = "crate::values::specified::svg::VectorEffectType" }, - { gecko = "StyleCoordinateSpace", servo = "crate::values::specified::svg::CoordinateSpace" }, { gecko = "StyleVectorEffect", servo = "crate::values::specified::svg::VectorEffect" }, { gecko = "StyleImageRendering", servo = "crate::values::computed::ImageRendering" }, { gecko = "StylePrintColorAdjust", servo = "crate::values::computed::PrintColorAdjust" }, diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index 639746b4edbc..1f22a501c090 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -909,6 +909,7 @@ nsStyleSVGReset::nsStyleSVGReset() mLightingColor(StyleColor::White()), mStopOpacity(1.0f), mFloodOpacity(1.0f), + mVectorEffect(StyleVectorEffect::NONE), mMaskType(StyleMaskType::Luminance), mD(StyleDProperty::None()) { MOZ_COUNT_CTOR(nsStyleSVGReset); diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js index ab3c8fdcbe1a..6036ca65590c 100644 --- a/layout/style/test/property_database.js +++ b/layout/style/test/property_database.js @@ -9787,18 +9787,8 @@ var gCSSProperties = { applies_to_first_letter: true, applies_to_first_line: true, initial_values: ["none"], - other_values: [ - "non-scaling-stroke", - "non-scaling-stroke viewport", - "non-scaling-stroke screen", - ], - invalid_values: [ - "none non-scaling-stroke", - "non-scaling-stroke viewport screen", - "none viewport", - "none screen", - "viewport non-scaling-stroke", - ], + other_values: ["non-scaling-stroke"], + invalid_values: ["none non-scaling-stroke"], }, "-moz-window-dragging": { domProp: "MozWindowDragging", diff --git a/layout/svg/SVGUtils.cpp b/layout/svg/SVGUtils.cpp index 8b13939c5455..f0bbd331f0ce 100644 --- a/layout/svg/SVGUtils.cpp +++ b/layout/svg/SVGUtils.cpp @@ -1083,9 +1083,7 @@ bool SVGUtils::GetNonScalingStrokeTransform(const nsIFrame* aFrame, SVGElement* content = static_cast(aFrame->GetContent()); *aUserToOuterSVG = - ThebesMatrix(aFrame->StyleSVGReset()->mVectorEffect.IsScreen() - ? SVGContentUtils::GetScreenCTM(content) - : SVGContentUtils::GetCTM(content)); + ThebesMatrix(SVGContentUtils::GetOuterViewportCTM(content)); return aUserToOuterSVG->HasNonTranslation() && !aUserToOuterSVG->IsSingular(); } diff --git a/servo/components/style/values/specified/svg.rs b/servo/components/style/values/specified/svg.rs index f60ece83ce28..a70b293caf91 100644 --- a/servo/components/style/values/specified/svg.rs +++ b/servo/components/style/values/specified/svg.rs @@ -420,11 +420,10 @@ impl Parse for DProperty { )] #[css(bitflags(single = "none", mixed = "non-scaling-stroke"))] #[repr(C)] -/// Values for vector-effect: /// https://svgwg.org/svg2-draft/coords.html#VectorEffects -pub struct VectorEffectType(u8); +pub struct VectorEffect(u8); bitflags! { - impl VectorEffectType: u8 { + impl VectorEffect: u8 { /// `none` const NONE = 0; /// `non-scaling-stroke` @@ -432,92 +431,10 @@ bitflags! { } } -#[allow(missing_docs)] -impl VectorEffectType { - pub fn is_none(&self) -> bool { - *self == VectorEffectType::NONE - } -} - -#[derive( - Clone, - Copy, - Debug, - Default, - Eq, - MallocSizeOf, - Parse, - PartialEq, - SpecifiedValueInfo, - ToComputedValue, - ToCss, - ToResolvedValue, - ToShmem, -)] -#[repr(C)] -/// co-ordinate space for vector-effect: -/// https://svgwg.org/svg2-draft/coords.html#VectorEffects -pub enum CoordinateSpace { - #[default] - /// `viewport` - Viewport, - /// `screen` - Screen, -} - -#[allow(missing_docs)] -impl CoordinateSpace { - pub fn is_viewport(&self) -> bool { - *self == Self::Viewport - } -} - -#[derive( - Clone, - Copy, - Debug, - MallocSizeOf, - PartialEq, - SpecifiedValueInfo, - ToCss, - ToComputedValue, - ToResolvedValue, - ToShmem, -)] -#[repr(C)] -/// Specified value for the vector-effect property. -/// (The spec grammar gives -/// `none | [ non-scaling-stroke | non-scaling-size | non-rotation | fixed-position ]+ [ viewport | screen ]?`.) -/// https://svgwg.org/svg2-draft/coords.html#VectorEffects -pub struct VectorEffect { - /// none or non-scaling-stroke - pub effect_type: VectorEffectType, - /// screen or viewport - #[css(skip_if = "CoordinateSpace::is_viewport")] - pub coordinate_space: CoordinateSpace, -} - -#[allow(missing_docs)] impl VectorEffect { + /// Returns the initial value of vector-effect #[inline] pub fn none() -> Self { - Self { - effect_type: VectorEffectType::NONE, - coordinate_space: CoordinateSpace::Viewport, - } - } -} - -impl Parse for VectorEffect { - fn parse<'i, 't>( - context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - let effect_type = VectorEffectType::parse(context, input)?; - if effect_type.is_none() { - return Ok(Self::none()); - } - let coordinate_space = input.try_parse(CoordinateSpace::parse).unwrap_or(CoordinateSpace::Viewport); - Ok(Self { effect_type, coordinate_space }) + Self::NONE } } diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml index 1ba036f65ded..d507293e195d 100644 --- a/servo/ports/geckolib/cbindgen.toml +++ b/servo/ports/geckolib/cbindgen.toml @@ -557,12 +557,7 @@ renaming_overrides_prefixing = true """ "VectorEffect" = """ - StyleVectorEffect() - : effect_type(StyleVectorEffectType::NONE), - coordinate_space(StyleCoordinateSpace::Viewport) {} - - bool HasNonScalingStroke() const { return bool(effect_type & StyleVectorEffectType::NON_SCALING_STROKE); } - bool IsScreen() const { return coordinate_space == StyleCoordinateSpace::Screen; } + bool HasNonScalingStroke() const { return bool(*this & StyleVectorEffect::NON_SCALING_STROKE); } """ # TODO(emilio): Add hooks to cbindgen to be able to generate [[nodiscard]] diff --git a/testing/web-platform/tests/svg/painting/reftests/non-scaling-stroke-003.html b/testing/web-platform/tests/svg/painting/reftests/non-scaling-stroke-003.html index 147bf814d30f..4fec5711d6c4 100644 --- a/testing/web-platform/tests/svg/painting/reftests/non-scaling-stroke-003.html +++ b/testing/web-platform/tests/svg/painting/reftests/non-scaling-stroke-003.html @@ -1,6 +1,6 @@ -non-scaling-stroke with screen transform +non-scaling-stroke with outer viewport transform @@ -10,24 +10,30 @@ margin: 0; width: 200px; height: 200px; - transform: scale(0.5); } svg { - width: 100px; - height: 100px; - transform: scale(2) translate(-25px, -25px); transform-origin: center; transform-bxox: fill-box; } + #outer { + width: 100px; + height: 100px; + transform: scale(2); + } + #inner { + transform: scale(0.5); + } rect { fill: red; stroke: green; stroke-width: 75px; - vector-effect: non-scaling-stroke screen; + vector-effect: non-scaling-stroke; } - - + + + + diff --git a/testing/web-platform/tests/svg/styling/vector-effect-invalid.html b/testing/web-platform/tests/svg/styling/vector-effect-invalid.html index ec49b88c5995..18f82502ddd3 100644 --- a/testing/web-platform/tests/svg/styling/vector-effect-invalid.html +++ b/testing/web-platform/tests/svg/styling/vector-effect-invalid.html @@ -18,6 +18,9 @@ test_invalid_value("vector-effect", "viewport"); test_invalid_value("vector-effect", "screen"); test_invalid_value("vector-effect", "screen non-scaling-stroke"); +// The following were removed by https://github.com/w3c/svgwg/issues/582 +test_invalid_value("vector-effect", "non-scaling-stroke viewport"); +test_invalid_value("vector-effect", "non-scaling-stroke screen"); diff --git a/testing/web-platform/tests/svg/styling/vector-effect-valid.html b/testing/web-platform/tests/svg/styling/vector-effect-valid.html deleted file mode 100644 index 9563db0523ba..000000000000 --- a/testing/web-platform/tests/svg/styling/vector-effect-valid.html +++ /dev/null @@ -1,19 +0,0 @@ - - - - -vector-effect test: parsing vector-effect with invalid values - - - - - - - - -