From 4aa1ad81ba71cccc93d0f1a65e17848dff391b22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 21 Aug 2024 15:00:48 +0000 Subject: [PATCH] Bug 878346 - Make transform a mapped attribute for SVG. r=longsonr,firefox-style-system-reviewers,zrhoffman The tricky bit is rotate() which in SVG means something different if there's an origin (you translate-then-untranslate it). But this seems to work off-hand, and fix the reminder of bug 1906261. Differential Revision: https://phabricator.services.mozilla.com/D215788 --- .../test_animation_performance_warning.html | 3 +- dom/base/AttrArray.h | 2 +- dom/svg/SVGElement.cpp | 154 +++++++++++++++--- dom/svg/SVGElement.h | 16 +- dom/svg/SVGFragmentIdentifier.cpp | 8 +- dom/svg/SVGGeometryElement.cpp | 22 ++- dom/svg/SVGGeometryElement.h | 2 + dom/svg/SVGSVGElement.cpp | 18 +- dom/svg/SVGSVGElement.h | 2 +- dom/svg/SVGTransformableElement.cpp | 34 +--- dom/svg/SVGTransformableElement.h | 12 +- dom/svg/SVGUseElement.cpp | 20 +-- dom/svg/SVGViewportElement.cpp | 18 +- dom/svg/SVGViewportElement.h | 3 - layout/painting/nsDisplayList.cpp | 5 +- layout/style/nsDOMCSSAttrDeclaration.cpp | 16 +- layout/style/nsDOMCSSAttrDeclaration.h | 4 + layout/svg/SVGGeometryFrame.cpp | 13 +- layout/svg/SVGOuterSVGFrame.cpp | 21 +-- layout/svg/SVGTextFrame.cpp | 80 ++++----- layout/svg/SVGUtils.cpp | 20 +-- servo/ports/geckolib/glue.rs | 18 ++ ...and-transform-css-property-in-svg.html.ini | 2 - .../presentation-attributes-relevant.html.ini | 4 - ...entation-attributes-special-cases.html.ini | 3 - .../css-viewport/zoom/svg-transform-ref.html | 4 + .../css/css-viewport/zoom/svg-transform.html | 7 + 27 files changed, 295 insertions(+), 216 deletions(-) delete mode 100644 testing/web-platform/meta/css/css-transforms/translate/translate-and-transform-css-property-in-svg.html.ini create mode 100644 testing/web-platform/tests/css/css-viewport/zoom/svg-transform-ref.html create mode 100644 testing/web-platform/tests/css/css-viewport/zoom/svg-transform.html diff --git a/dom/animation/test/chrome/test_animation_performance_warning.html b/dom/animation/test/chrome/test_animation_performance_warning.html index 13df5d7842a0..2471d2391b20 100644 --- a/dom/animation/test/chrome/test_animation_performance_warning.html +++ b/dom/animation/test/chrome/test_animation_performance_warning.html @@ -1560,8 +1560,7 @@ function testTransformSVG() { animation.effect.getProperties(), [ { property: subtest.property, - runningOnCompositor: false, - warning: 'CompositorAnimationWarningTransformSVG' + runningOnCompositor: true, } ]); svg.removeAttribute('transform'); await waitForFrame(); diff --git a/dom/base/AttrArray.h b/dom/base/AttrArray.h index 9331ac232626..b3e11a8f1ac9 100644 --- a/dom/base/AttrArray.h +++ b/dom/base/AttrArray.h @@ -98,7 +98,7 @@ class AttrArray { // called when a mapped attribute is changed (regardless of connectedness). bool MarkAsPendingPresAttributeEvaluation() { // It'd be great to be able to assert that mImpl is non-null or we're the - // element. + // or elements. if (MOZ_UNLIKELY(!mImpl) && !GrowBy(1)) { return false; } diff --git a/dom/svg/SVGElement.cpp b/dom/svg/SVGElement.cpp index e1a25f2642b1..6e92b82ec67d 100644 --- a/dom/svg/SVGElement.cpp +++ b/dom/svg/SVGElement.cpp @@ -248,8 +248,7 @@ nsresult SVGElement::CopyInnerTo(mozilla::dom::Element* aDest) { if (const auto* pathSegList = GetAnimPathSegList()) { *dest->GetAnimPathSegList() = *pathSegList; if (pathSegList->IsAnimating()) { - dest->SMILOverrideStyle()->SetSMILValue(nsCSSPropertyID::eCSSProperty_d, - *pathSegList); + dest->SMILOverrideStyle()->SetSMILValue(eCSSProperty_d, *pathSegList); } } if (const auto* transformList = GetAnimatedTransformList()) { @@ -1116,6 +1115,93 @@ bool SVGElement::UpdateDeclarationBlockFromPath( return true; } +template +static StyleTransformOperation MatrixToTransformOperation( + const gfx::BaseMatrix& aMatrix) { + return StyleTransformOperation::Matrix(StyleGenericMatrix{ + .a = float(aMatrix._11), + .b = float(aMatrix._12), + .c = float(aMatrix._21), + .d = float(aMatrix._22), + .e = float(aMatrix._31), + .f = float(aMatrix._32), + }); +} + +static void SVGTransformToCSS(const SVGTransform& aTransform, + nsTArray& aOut) { + switch (aTransform.Type()) { + case dom::SVGTransform_Binding::SVG_TRANSFORM_SCALE: { + const auto& m = aTransform.GetMatrix(); + aOut.AppendElement(StyleTransformOperation::Scale(m._11, m._22)); + return; + } + case dom::SVGTransform_Binding::SVG_TRANSFORM_TRANSLATE: { + auto p = aTransform.GetMatrix().GetTranslation(); + aOut.AppendElement(StyleTransformOperation::Translate( + LengthPercentage::FromPixels(CSSCoord(p.x)), + LengthPercentage::FromPixels(CSSCoord(p.y)))); + return; + } + case dom::SVGTransform_Binding::SVG_TRANSFORM_ROTATE: { + float cx, cy; + aTransform.GetRotationOrigin(cx, cy); + const StyleAngle angle{aTransform.Angle()}; + const bool hasOrigin = cx != 0.0f || cy != 0.0f; + if (hasOrigin) { + aOut.AppendElement(StyleTransformOperation::Translate( + LengthPercentage::FromPixels(cx), + LengthPercentage::FromPixels(cy))); + } + aOut.AppendElement(StyleTransformOperation::Rotate(angle)); + if (hasOrigin) { + aOut.AppendElement(StyleTransformOperation::Translate( + LengthPercentage::FromPixels(-cx), + LengthPercentage::FromPixels(-cy))); + } + return; + } + case dom::SVGTransform_Binding::SVG_TRANSFORM_SKEWX: + aOut.AppendElement(StyleTransformOperation::SkewX({aTransform.Angle()})); + return; + case dom::SVGTransform_Binding::SVG_TRANSFORM_SKEWY: + aOut.AppendElement(StyleTransformOperation::SkewY({aTransform.Angle()})); + return; + case dom::SVGTransform_Binding::SVG_TRANSFORM_MATRIX: { + aOut.AppendElement(MatrixToTransformOperation(aTransform.GetMatrix())); + return; + } + case dom::SVGTransform_Binding::SVG_TRANSFORM_UNKNOWN: + default: + MOZ_CRASH("Bad SVGTransform?"); + } +} + +/* static */ +bool SVGElement::UpdateDeclarationBlockFromTransform( + StyleLockedDeclarationBlock& aBlock, + const SVGAnimatedTransformList* aTransform, + const gfx::Matrix* aAnimateMotionTransform, ValToUse aValToUse) { + MOZ_ASSERT(aTransform || aAnimateMotionTransform); + AutoTArray operations; + if (aAnimateMotionTransform) { + operations.AppendElement( + MatrixToTransformOperation(*aAnimateMotionTransform)); + } + if (aTransform) { + const SVGTransformList& transforms = aValToUse == ValToUse::Anim + ? aTransform->GetAnimValue() + : aTransform->GetBaseValue(); + // TODO: Maybe make SVGTransform use StyleTransformOperation directly? + for (size_t i = 0, len = transforms.Length(); i < len; ++i) { + SVGTransformToCSS(transforms[i], operations); + } + } + Servo_DeclarationBlock_SetTransform(&aBlock, eCSSProperty_transform, + &operations); + return true; +} + //------------------------------------------------------------------------ // Helper class: MappedAttrParser, for parsing values of mapped attributes @@ -1142,7 +1228,8 @@ class MOZ_STACK_CLASS MappedAttrParser { void TellStyleAlreadyParsedResult(nsAtom const* aAtom, SVGAnimatedLength const& aLength); - void TellStyleAlreadyParsedResult(const SVGAnimatedPathSegList& aPath); + void TellStyleAlreadyParsedResult(const SVGAnimatedPathSegList&); + void TellStyleAlreadyParsedResult(const SVGAnimatedTransformList&); // If we've parsed any values for mapped attributes, this method returns the // already_AddRefed declaration block that incorporates the parsed values. @@ -1228,6 +1315,13 @@ void MappedAttrParser::TellStyleAlreadyParsedResult( SVGElement::ValToUse::Base); } +void MappedAttrParser::TellStyleAlreadyParsedResult( + const SVGAnimatedTransformList& aTransform) { + SVGElement::UpdateDeclarationBlockFromTransform(EnsureDeclarationBlock(), + &aTransform, nullptr, + SVGElement::ValToUse::Base); +} + } // namespace //---------------------------------------------------------------------- @@ -1239,6 +1333,7 @@ void SVGElement::UpdateMappedDeclarationBlock() { const bool lengthAffectsStyle = SVGGeometryProperty::ElementMapsLengthsToStyle(this); + bool sawTransform = false; uint32_t i = 0; while (BorrowedAttrInfo info = GetAttrInfoAt(i++)) { @@ -1267,7 +1362,18 @@ void SVGElement::UpdateMappedDeclarationBlock() { } } - if (attrName->Equals(nsGkAtoms::d, kNameSpaceID_None)) { + if (attrName->Atom() == nsGkAtoms::transform) { + sawTransform = true; + const auto* transform = GetAnimatedTransformList(); + MOZ_ASSERT(GetTransformListAttrName() == nsGkAtoms::transform); + MOZ_ASSERT(transform); + // We want to go through the optimized path to tell the style system the + // result directly, rather than let it parse the same thing again. + mappedAttrParser.TellStyleAlreadyParsedResult(*transform); + continue; + } + + if (attrName->Atom() == nsGkAtoms::d) { const auto* path = GetAnimPathSegList(); // Note: Only SVGPathElement has d attribute. MOZ_ASSERT( @@ -1295,6 +1401,14 @@ void SVGElement::UpdateMappedDeclarationBlock() { info.mValue->ToString(value); mappedAttrParser.ParseMappedAttrValue(attrName->Atom(), value); } + + // We need to map the SVG view's transform if we haven't mapped it already. + if (NodeInfo()->NameAtom() == nsGkAtoms::svg && !sawTransform) { + if (const auto* transform = GetAnimatedTransformList()) { + mappedAttrParser.TellStyleAlreadyParsedResult(*transform); + } + } + mAttrs.SetMappedDeclarationBlock(mappedAttrParser.TakeDeclarationBlock()); } @@ -1713,10 +1827,9 @@ void SVGElement::DidAnimatePathSegList() { if (name == nsGkAtoms::d) { auto* animPathSegList = GetAnimPathSegList(); if (animPathSegList->IsAnimating()) { - SMILOverrideStyle()->SetSMILValue(nsCSSPropertyID::eCSSProperty_d, - *animPathSegList); + SMILOverrideStyle()->SetSMILValue(eCSSProperty_d, *animPathSegList); } else { - SMILOverrideStyle()->ClearSMILValue(nsCSSPropertyID::eCSSProperty_d); + SMILOverrideStyle()->ClearSMILValue(eCSSProperty_d); } } @@ -1970,25 +2083,20 @@ void SVGElement::DidAnimateTransformList(int32_t aModType) { MOZ_ASSERT(GetTransformListAttrName(), "Animating non-existent transform data?"); - if (auto* frame = GetPrimaryFrame()) { - nsAtom* transformAttr = GetTransformListAttrName(); - frame->AttributeChanged(kNameSpaceID_None, transformAttr, aModType); - // When script changes the 'transform' attribute, Element::SetAttrAndNotify - // will call MutationObservers::NotifyAttributeChanged, under which - // SVGTransformableElement::GetAttributeChangeHint will be called and an - // appropriate change event posted to update our frame's overflow rects. - // The SetAttrAndNotify doesn't happen for transform changes caused by - // 'animateTransform' though (and sending out the mutation events that - // MutationObservers::NotifyAttributeChanged dispatches would be - // inappropriate anyway), so we need to post the change event ourself. - nsChangeHint changeHint = GetAttributeChangeHint(transformAttr, aModType); - if (changeHint) { - nsLayoutUtils::PostRestyleEvent(this, RestyleHint{0}, changeHint); + nsAtom* transformAttr = GetTransformListAttrName(); + if (transformAttr == nsGkAtoms::transform) { + const auto* animTransformList = GetAnimatedTransformList(); + const auto* animateMotion = GetAnimateMotionTransform(); + if (animateMotion || + (animTransformList && animTransformList->IsAnimating())) { + SMILOverrideStyle()->SetSMILValue(eCSSProperty_transform, + animTransformList, animateMotion); + } else { + SMILOverrideStyle()->ClearSMILValue(eCSSProperty_transform); } - SVGObserverUtils::InvalidateRenderingObservers(frame); return; } - SVGObserverUtils::InvalidateDirectRenderingObservers(this); + DidAnimateAttribute(kNameSpaceID_None, transformAttr); } SVGElement::StringAttributesInfo SVGElement::GetStringInfo() { diff --git a/dom/svg/SVGElement.h b/dom/svg/SVGElement.h index c6fab341830b..af0f5991bbb0 100644 --- a/dom/svg/SVGElement.h +++ b/dom/svg/SVGElement.h @@ -181,12 +181,16 @@ class SVGElement : public SVGElementBase // nsIContent void SetLength(nsAtom* aName, const SVGAnimatedLength& aLength); enum class ValToUse { Base, Anim }; - static bool UpdateDeclarationBlockFromLength( - StyleLockedDeclarationBlock& aBlock, nsCSSPropertyID aPropId, - const SVGAnimatedLength& aLength, ValToUse aValToUse); - static bool UpdateDeclarationBlockFromPath( - StyleLockedDeclarationBlock& aBlock, const SVGAnimatedPathSegList& aPath, - ValToUse aValToUse); + static bool UpdateDeclarationBlockFromLength(StyleLockedDeclarationBlock&, + nsCSSPropertyID, + const SVGAnimatedLength&, + ValToUse); + static bool UpdateDeclarationBlockFromPath(StyleLockedDeclarationBlock&, + const SVGAnimatedPathSegList&, + ValToUse); + static bool UpdateDeclarationBlockFromTransform( + StyleLockedDeclarationBlock&, const SVGAnimatedTransformList*, + const gfx::Matrix* aAnimateMotionTransform, ValToUse); nsAttrValue WillChangeLength(uint8_t aAttrEnum, const mozAutoDocUpdate& aProofOfUpdate); diff --git a/dom/svg/SVGFragmentIdentifier.cpp b/dom/svg/SVGFragmentIdentifier.cpp index 0932afd8cafd..6652ff9bc18d 100644 --- a/dom/svg/SVGFragmentIdentifier.cpp +++ b/dom/svg/SVGFragmentIdentifier.cpp @@ -43,11 +43,9 @@ class MOZ_RAII AutoSVGViewHandler { if (mValid) { mRoot->mSVGView = std::move(mSVGView); } - mRoot->InvalidateTransformNotifyFrame(); - if (nsIFrame* f = mRoot->GetPrimaryFrame()) { - if (SVGOuterSVGFrame* osf = do_QueryFrame(f)) { - osf->MaybeSendIntrinsicSizeAndRatioToEmbedder(); - } + mRoot->DidChangeSVGView(); + if (SVGOuterSVGFrame* osf = do_QueryFrame(mRoot->GetPrimaryFrame())) { + osf->MaybeSendIntrinsicSizeAndRatioToEmbedder(); } } diff --git a/dom/svg/SVGGeometryElement.cpp b/dom/svg/SVGGeometryElement.cpp index afda7c29bc6a..79986468db2f 100644 --- a/dom/svg/SVGGeometryElement.cpp +++ b/dom/svg/SVGGeometryElement.cpp @@ -19,7 +19,7 @@ #include "mozilla/dom/SVGLengthBinding.h" #include "mozilla/gfx/2D.h" #include "mozilla/RefPtr.h" -#include "mozilla/StaticPrefs_layout.h" +#include "nsLayoutUtils.h" #include "mozilla/SVGContentUtils.h" using namespace mozilla::gfx; @@ -242,6 +242,21 @@ already_AddRefed SVGGeometryElement::GetPointAtLength( clamped(distance, 0.f, path->ComputeLength())))); } +gfx::Matrix SVGGeometryElement::LocalTransform() const { + gfx::Matrix result; + nsIFrame* f = GetPrimaryFrame(); + if (!f || !f->IsTransformed()) { + return result; + } + Matrix4x4Flagged matrix = nsDisplayTransform::GetResultingTransformMatrix( + f, nsPoint(), f->PresContext()->AppUnitsPerDevPixel(), + nsDisplayTransform::INCLUDE_PERSPECTIVE); + if (!matrix.IsIdentity()) { + std::ignore = matrix.CanDraw2D(&result); + } + return result; +} + float SVGGeometryElement::GetPathLengthScale(PathLengthScaleForType aFor) { MOZ_ASSERT(aFor == eForTextPath || aFor == eForStroking, "Unknown enum"); if (mPathLength.IsExplicitlySet()) { @@ -257,10 +272,9 @@ float SVGGeometryElement::GetPathLengthScale(PathLengthScaleForType aFor) { // For textPath, a transform on the referenced path affects the // textPath layout, so when calculating the actual path length // we need to take that into account. - gfxMatrix matrix = PrependLocalTransformsTo(gfxMatrix()); + auto matrix = LocalTransform(); if (!matrix.IsIdentity()) { - RefPtr builder = - path->TransformedCopyToBuilder(ToMatrix(matrix)); + RefPtr builder = path->TransformedCopyToBuilder(matrix); path = builder->Finish(); } } diff --git a/dom/svg/SVGGeometryElement.h b/dom/svg/SVGGeometryElement.h index 12576730ee20..eebffeff4bd4 100644 --- a/dom/svg/SVGGeometryElement.h +++ b/dom/svg/SVGGeometryElement.h @@ -247,6 +247,8 @@ class SVGGeometryElement : public SVGGeometryElementBase { MOZ_CAN_RUN_SCRIPT already_AddRefed GetPointAtLength( float distance, ErrorResult& rv); + gfx::Matrix LocalTransform() const; + protected: // SVGElement method NumberAttributesInfo GetNumberInfo() override; diff --git a/dom/svg/SVGSVGElement.cpp b/dom/svg/SVGSVGElement.cpp index a44c974ccc76..072dfd411f7e 100644 --- a/dom/svg/SVGSVGElement.cpp +++ b/dom/svg/SVGSVGElement.cpp @@ -441,10 +441,19 @@ bool SVGSVGElement::WillBeOutermostSVG(nsINode& aParent) const { return true; } +void SVGSVGElement::DidChangeSVGView() { + InvalidateTransformNotifyFrame(); + // We map the SVGView transform as the transform css property, so need to + // schedule attribute mapping. + if (!IsPendingMappedAttributeEvaluation() && + mAttrs.MarkAsPendingPresAttributeEvaluation()) { + OwnerDoc()->ScheduleForPresAttrEvaluation(this); + } +} + void SVGSVGElement::InvalidateTransformNotifyFrame() { - ISVGSVGFrame* svgframe = do_QueryFrame(GetPrimaryFrame()); // might fail this check if we've failed conditional processing - if (svgframe) { + if (ISVGSVGFrame* svgframe = do_QueryFrame(GetPrimaryFrame())) { svgframe->NotifyViewportOrTransformChanged( ISVGDisplayableFrame::TRANSFORM_CHANGED); } @@ -584,9 +593,4 @@ const SVGAnimatedViewBox& SVGSVGElement::GetViewBoxInternal() const { return mViewBox; } -SVGAnimatedTransformList* SVGSVGElement::GetTransformInternal() const { - return (mSVGView && mSVGView->mTransforms) ? mSVGView->mTransforms.get() - : mTransforms.get(); -} - } // namespace mozilla::dom diff --git a/dom/svg/SVGSVGElement.h b/dom/svg/SVGSVGElement.h index 0e800e8bd161..155b222c1134 100644 --- a/dom/svg/SVGSVGElement.h +++ b/dom/svg/SVGSVGElement.h @@ -183,6 +183,7 @@ class SVGSVGElement final : public SVGSVGElementBase { // invalidate viewbox -> viewport xform & inform frames void InvalidateTransformNotifyFrame(); + void DidChangeSVGView(); // Methods for elements to override my "PreserveAspectRatio" value. // These are private so that only our friends @@ -197,7 +198,6 @@ class SVGSVGElement final : public SVGSVGElementBase { bool ClearPreserveAspectRatioProperty(); const SVGAnimatedViewBox& GetViewBoxInternal() const override; - SVGAnimatedTransformList* GetTransformInternal() const override; EnumAttributesInfo GetEnumInfo() override; diff --git a/dom/svg/SVGTransformableElement.cpp b/dom/svg/SVGTransformableElement.cpp index adbbe10719e0..33eb1aee0845 100644 --- a/dom/svg/SVGTransformableElement.cpp +++ b/dom/svg/SVGTransformableElement.cpp @@ -26,6 +26,12 @@ SVGTransformableElement::Transform() { //---------------------------------------------------------------------- // nsIContent methods +bool SVGTransformableElement::IsAttributeMapped( + const nsAtom* aAttribute) const { + return aAttribute == nsGkAtoms::transform || + SVGElement::IsAttributeMapped(aAttribute); +} + nsChangeHint SVGTransformableElement::GetAttributeChangeHint( const nsAtom* aAttribute, int32_t aModType) const { nsChangeHint retval = @@ -75,14 +81,9 @@ bool SVGTransformableElement::IsEventAttributeNameInternal(nsAtom* aName) { gfxMatrix SVGTransformableElement::PrependLocalTransformsTo( const gfxMatrix& aMatrix, SVGTransformTypes aWhich) const { - if (aWhich == eChildToUserSpace) { - // We don't have any eUserSpaceToParent transforms. (Sub-classes that do - // must override this function and handle that themselves.) - return aMatrix; - } - return GetUserToParentTransform(mAnimateMotionTransform.get(), - mTransforms.get()) * - aMatrix; + // We don't have any eUserSpaceToParent transforms. (Sub-classes that do + // must override this function and handle that themselves.) + return aMatrix; } const gfx::Matrix* SVGTransformableElement::GetAnimateMotionTransform() const { @@ -130,21 +131,4 @@ SVGAnimatedTransformList* SVGTransformableElement::GetAnimatedTransformList( return mTransforms.get(); } -/* static */ -gfxMatrix SVGTransformableElement::GetUserToParentTransform( - const gfx::Matrix* aAnimateMotionTransform, - const SVGAnimatedTransformList* aTransforms) { - gfxMatrix result; - - if (aAnimateMotionTransform) { - result.PreMultiply(ThebesMatrix(*aAnimateMotionTransform)); - } - - if (aTransforms) { - result.PreMultiply(aTransforms->GetAnimValue().GetConsolidationMatrix()); - } - - return result; -} - } // namespace mozilla::dom diff --git a/dom/svg/SVGTransformableElement.h b/dom/svg/SVGTransformableElement.h index 2f6a504ce2c2..72386a69eac8 100644 --- a/dom/svg/SVGTransformableElement.h +++ b/dom/svg/SVGTransformableElement.h @@ -45,6 +45,7 @@ class SVGTransformableElement : public SVGElement { SVGTransformTypes aWhich = eAllTransforms) const override; const gfx::Matrix* GetAnimateMotionTransform() const override; void SetAnimateMotionTransform(const gfx::Matrix* aMatrix) override; + NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override; SVGAnimatedTransformList* GetAnimatedTransformList( uint32_t aFlags = 0) override; @@ -55,17 +56,6 @@ class SVGTransformableElement : public SVGElement { bool IsTransformable() override { return true; } protected: - /** - * Helper for overrides of PrependLocalTransformsTo. If both arguments are - * provided they are multiplied in the order in which the arguments appear, - * and the result is returned. If neither argument is provided, the identity - * matrix is returned. If only one argument is provided its transform is - * returned. - */ - static gfxMatrix GetUserToParentTransform( - const gfx::Matrix* aAnimateMotionTransform, - const SVGAnimatedTransformList* aTransforms); - UniquePtr mTransforms; // XXX maybe move this to property table, to save space on un-animated elems? diff --git a/dom/svg/SVGUseElement.cpp b/dom/svg/SVGUseElement.cpp index 084f9c08b7e0..fbe8603c4542 100644 --- a/dom/svg/SVGUseElement.cpp +++ b/dom/svg/SVGUseElement.cpp @@ -606,15 +606,8 @@ void SVGUseElement::UnlinkSource() { /* virtual */ gfxMatrix SVGUseElement::PrependLocalTransformsTo( const gfxMatrix& aMatrix, SVGTransformTypes aWhich) const { - // 'transform' attribute: - gfxMatrix userToParent; - - if (aWhich == eUserSpaceToParent || aWhich == eAllTransforms) { - userToParent = GetUserToParentTransform(mAnimateMotionTransform.get(), - mTransforms.get()); - if (aWhich == eUserSpaceToParent) { - return userToParent * aMatrix; - } + if (aWhich == eUserSpaceToParent) { + return aMatrix; } // our 'x' and 'y' attributes: @@ -624,13 +617,8 @@ gfxMatrix SVGUseElement::PrependLocalTransformsTo( } gfxMatrix childToUser = gfxMatrix::Translation(x, y); - - if (aWhich == eAllTransforms) { - return childToUser * userToParent * aMatrix; - } - - MOZ_ASSERT(aWhich == eChildToUserSpace, "Unknown TransformTypes"); - + MOZ_ASSERT(aWhich == eChildToUserSpace || aWhich == eAllTransforms, + "Unknown TransformTypes"); // The following may look broken because pre-multiplying our eChildToUserSpace // transform with another matrix without including our eUserSpaceToParent // transform between the two wouldn't make sense. We don't expect that to diff --git a/dom/svg/SVGViewportElement.cpp b/dom/svg/SVGViewportElement.cpp index bbc8823dc3a1..a8df815c5f2b 100644 --- a/dom/svg/SVGViewportElement.cpp +++ b/dom/svg/SVGViewportElement.cpp @@ -241,14 +241,8 @@ float SVGViewportElement::GetLength(uint8_t aCtxType) const { gfxMatrix SVGViewportElement::PrependLocalTransformsTo( const gfxMatrix& aMatrix, SVGTransformTypes aWhich) const { // 'transform' attribute (or an override from a fragment identifier): - gfxMatrix userToParent; - - if (aWhich == eUserSpaceToParent || aWhich == eAllTransforms) { - userToParent = GetUserToParentTransform(mAnimateMotionTransform.get(), - GetTransformInternal()); - if (aWhich == eUserSpaceToParent) { - return userToParent * aMatrix; - } + if (aWhich == eUserSpaceToParent) { + return aMatrix; } gfxMatrix childToUser; @@ -271,12 +265,8 @@ gfxMatrix SVGViewportElement::PrependLocalTransformsTo( childToUser = ThebesMatrix(GetViewBoxTransform()); } - if (aWhich == eAllTransforms) { - return childToUser * userToParent * aMatrix; - } - - MOZ_ASSERT(aWhich == eChildToUserSpace, "Unknown TransformTypes"); - + MOZ_ASSERT(aWhich == eAllTransforms || aWhich == eChildToUserSpace, + "Unknown TransformTypes"); // The following may look broken because pre-multiplying our eChildToUserSpace // transform with another matrix without including our eUserSpaceToParent // transform between the two wouldn't make sense. We don't expect that to diff --git a/dom/svg/SVGViewportElement.h b/dom/svg/SVGViewportElement.h index 528ca7a1c81d..c19be0a9436a 100644 --- a/dom/svg/SVGViewportElement.h +++ b/dom/svg/SVGViewportElement.h @@ -160,9 +160,6 @@ class SVGViewportElement : public SVGGraphicsElement { virtual const SVGAnimatedViewBox& GetViewBoxInternal() const { return mViewBox; } - virtual SVGAnimatedTransformList* GetTransformInternal() const { - return mTransforms.get(); - } SVGAnimatedViewBox mViewBox; SVGAnimatedPreserveAspectRatio mPreserveAspectRatio; diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp index 831c8b9cfc26..8904d2a96e28 100644 --- a/layout/painting/nsDisplayList.cpp +++ b/layout/painting/nsDisplayList.cpp @@ -6247,12 +6247,13 @@ Matrix4x4 nsDisplayTransform::GetResultingTransformMatrixInternal( aProperties.mTranslate, aProperties.mRotate, aProperties.mScale, aProperties.mMotion.ptrOr(nullptr), aProperties.mTransform, aRefBox, aAppUnitsPerPixel); - } else if (hasSVGTransforms) { + } + if (hasSVGTransforms) { // Correct the translation components for zoom: float pixelsPerCSSPx = AppUnitsPerCSSPixel() / aAppUnitsPerPixel; svgTransform._31 *= pixelsPerCSSPx; svgTransform._32 *= pixelsPerCSSPx; - result = Matrix4x4::From2D(svgTransform); + result *= Matrix4x4::From2D(svgTransform); } // Apply any translation due to 'transform-origin' and/or 'transform-box': diff --git a/layout/style/nsDOMCSSAttrDeclaration.cpp b/layout/style/nsDOMCSSAttrDeclaration.cpp index 0422760936cb..c286432603ba 100644 --- a/layout/style/nsDOMCSSAttrDeclaration.cpp +++ b/layout/style/nsDOMCSSAttrDeclaration.cpp @@ -182,7 +182,8 @@ nsresult nsDOMCSSAttributeDeclaration::SetSMILValue( } nsresult nsDOMCSSAttributeDeclaration::SetSMILValue( - const nsCSSPropertyID /*aPropID*/, const SVGAnimatedPathSegList& aPath) { + const nsCSSPropertyID aPropID, const SVGAnimatedPathSegList& aPath) { + MOZ_ASSERT(aPropID == eCSSProperty_d); return SetSMILValueHelper([&aPath](DeclarationBlock& aDecl) { MOZ_ASSERT(aDecl.IsMutable()); return SVGElement::UpdateDeclarationBlockFromPath( @@ -190,6 +191,19 @@ nsresult nsDOMCSSAttributeDeclaration::SetSMILValue( }); } +nsresult nsDOMCSSAttributeDeclaration::SetSMILValue( + const nsCSSPropertyID aPropID, const SVGAnimatedTransformList* aTransform, + const gfx::Matrix* aAnimateMotionTransform) { + MOZ_ASSERT(aPropID == eCSSProperty_transform); + return SetSMILValueHelper( + [aTransform, aAnimateMotionTransform](DeclarationBlock& aDecl) { + MOZ_ASSERT(aDecl.IsMutable()); + return SVGElement::UpdateDeclarationBlockFromTransform( + *aDecl.Raw(), aTransform, aAnimateMotionTransform, + SVGElement::ValToUse::Anim); + }); +} + // Scripted modifications to style.opacity or style.transform (or other // transform-like properties, e.g. style.translate, style.rotate, style.scale) // could immediately force us into the animated state if heuristics suggest diff --git a/layout/style/nsDOMCSSAttrDeclaration.h b/layout/style/nsDOMCSSAttrDeclaration.h index b2e9154a2dac..39c66f275b84 100644 --- a/layout/style/nsDOMCSSAttrDeclaration.h +++ b/layout/style/nsDOMCSSAttrDeclaration.h @@ -21,6 +21,7 @@ namespace mozilla { class SMILValue; class SVGAnimatedLength; class SVGAnimatedPathSegList; +class SVGAnimatedTransformList; namespace dom { class DomGroup; @@ -55,6 +56,9 @@ class nsDOMCSSAttributeDeclaration final : public nsDOMCSSDeclaration { const SVGAnimatedLength& aLength); nsresult SetSMILValue(const nsCSSPropertyID, const mozilla::SVGAnimatedPathSegList& aPath); + nsresult SetSMILValue(const nsCSSPropertyID, + const mozilla::SVGAnimatedTransformList*, + const mozilla::gfx::Matrix* aAnimateMotion = nullptr); void ClearSMILValue(const nsCSSPropertyID aPropID) { // Put empty string in override style for our property SetPropertyValue(aPropID, ""_ns, nullptr, mozilla::IgnoreErrors()); diff --git a/layout/svg/SVGGeometryFrame.cpp b/layout/svg/SVGGeometryFrame.cpp index 2cfa43bba71e..19c8b8fd8f80 100644 --- a/layout/svg/SVGGeometryFrame.cpp +++ b/layout/svg/SVGGeometryFrame.cpp @@ -104,14 +104,17 @@ void SVGGeometryFrame::DidSetComputedStyle(ComputedStyle* aOldComputedStyle) { // For clipPath we use clip-rule as the path's fill-rule. element->ClearAnyCachedPath(); } - } else { - if (StyleSVG()->mFillRule != oldStyleSVG->mFillRule) { - // Moz2D Path objects are fill-rule specific. - element->ClearAnyCachedPath(); - } + } else if (StyleSVG()->mFillRule != oldStyleSVG->mFillRule) { + // Moz2D Path objects are fill-rule specific. + element->ClearAnyCachedPath(); } } + if (StyleDisplay()->CalcTransformPropertyDifference( + *aOldComputedStyle->StyleDisplay())) { + SVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED); + } + if (element->IsGeometryChangedViaCSS(*Style(), *aOldComputedStyle)) { element->ClearAnyCachedPath(); SVGObserverUtils::InvalidateRenderingObservers(this); diff --git a/layout/svg/SVGOuterSVGFrame.cpp b/layout/svg/SVGOuterSVGFrame.cpp index eb341bc5d8dc..271697b989be 100644 --- a/layout/svg/SVGOuterSVGFrame.cpp +++ b/layout/svg/SVGOuterSVGFrame.cpp @@ -44,7 +44,7 @@ SVGOuterSVGFrame::SVGOuterSVGFrame(ComputedStyle* aStyle, // Outer- has CSS layout, so remove this bit: RemoveStateBits(NS_FRAME_SVG_LAYOUT); AddStateBits(NS_FRAME_REFLOW_ROOT | NS_FRAME_FONT_INFLATION_CONTAINER | - NS_FRAME_FONT_INFLATION_FLOW_ROOT | NS_FRAME_MAY_BE_TRANSFORMED); + NS_FRAME_FONT_INFLATION_FLOW_ROOT); } // The CSS Containment spec says that size-contained replaced elements must be @@ -531,21 +531,7 @@ bool SVGOuterSVGFrame::IsSVGTransformed(Matrix* aOwnTransform, // Our anonymous child's HasChildrenOnlyTransform() implementation makes sure // our children-only transforms are applied to our children. We only care // about transforms that transform our own frame here. - - bool foundTransform = false; - - SVGSVGElement* content = static_cast(GetContent()); - SVGAnimatedTransformList* transformList = content->GetAnimatedTransformList(); - if ((transformList && transformList->HasTransform()) || - content->GetAnimateMotionTransform()) { - if (aOwnTransform) { - *aOwnTransform = gfx::ToMatrix( - content->PrependLocalTransformsTo(gfxMatrix(), eUserSpaceToParent)); - } - foundTransform = true; - } - - return foundTransform; + return false; } //---------------------------------------------------------------------- @@ -667,8 +653,7 @@ SVGBBox SVGOuterSVGFrame::GetBBoxContribution( gfxMatrix SVGOuterSVGFrame::GetCanvasTM() { if (!mCanvasTM) { - SVGSVGElement* content = static_cast(GetContent()); - + auto* content = static_cast(GetContent()); float devPxPerCSSPx = 1.0f / nsPresContext::AppUnitsToFloatCSSPixels( PresContext()->AppUnitsPerDevPixel()); diff --git a/layout/svg/SVGTextFrame.cpp b/layout/svg/SVGTextFrame.cpp index c824effb01a2..07fbc923891d 100644 --- a/layout/svg/SVGTextFrame.cpp +++ b/layout/svg/SVGTextFrame.cpp @@ -351,13 +351,26 @@ static SVGTextFrame* FrameIfAnonymousChildReflowed(SVGTextFrame* aFrame) { return aFrame; } -static double GetContextScale(const gfxMatrix& aMatrix) { - // The context scale is the ratio of the length of the transformed - // diagonal vector (1,1) to the length of the untransformed diagonal - // (which is sqrt(2)). - gfxPoint p = aMatrix.TransformPoint(gfxPoint(1, 1)) - - aMatrix.TransformPoint(gfxPoint(0, 0)); - return SVGContentUtils::ComputeNormalizedHypotenuse(p.x, p.y); +// FIXME(emilio): SVG is a special-case where transforms affect layout. We don't +// want that to go outside the SVG stuff (and really we should aim to remove +// that). +static float GetContextScale(SVGTextFrame* aFrame) { + if (aFrame->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) { + // When we are non-display, we could be painted in different coordinate + // spaces, and we don't want to have to reflow for each of these. We just + // assume that the context scale is 1.0 for them all, so we don't get stuck + // with a font size scale factor based on whichever referencing frame + // happens to reflow first. + return 1.0f; + } + auto matrix = nsLayoutUtils::GetTransformToAncestor( + RelativeTo{aFrame}, RelativeTo{SVGUtils::GetOuterSVGFrame(aFrame)}); + Matrix transform2D; + if (!matrix.CanDraw2D(&transform2D)) { + return {}; + } + auto scales = transform2D.ScaleFactors(); + return std::max(1.0f, std::max(scales.xScale, scales.yScale)); } // ============================================================================ @@ -2993,16 +3006,16 @@ void SVGTextFrame::NotifySVGChanged(uint32_t aFlags) { // do not get too far out of sync with the final font size on the screen. if (needNewCanvasTM && mLastContextScale != 0.0f) { mCanvasTM = nullptr; - // If we are a non-display frame, then we don't want to call - // GetCanvasTM(), since the context scale does not use it. - gfxMatrix newTM = - HasAnyStateBits(NS_FRAME_IS_NONDISPLAY) ? gfxMatrix() : GetCanvasTM(); - // Compare the old and new context scales. - float scale = GetContextScale(newTM); - float change = scale / mLastContextScale; - if (change >= 2.0f || change <= 0.5f) { - needNewBounds = true; - needGlyphMetricsUpdate = true; + // If we are a non-display frame, then we don't want to get the transform + // since the context scale does not use it. + if (!HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) { + // Compare the old and new context scales. + float scale = GetContextScale(this); + float change = scale / mLastContextScale; + if (change >= 2.0f || change <= 0.5f) { + needNewBounds = true; + needGlyphMetricsUpdate = true; + } } } @@ -4521,11 +4534,10 @@ already_AddRefed SVGTextFrame::GetTextPath(nsIFrame* aTextPathFrame) { return nullptr; } - gfxMatrix matrix = geomElement->PrependLocalTransformsTo(gfxMatrix()); + // Apply the geometry element's transform if appropriate. + auto matrix = geomElement->LocalTransform(); if (!matrix.IsIdentity()) { - // Apply the geometry element's transform - RefPtr builder = - path->TransformedCopyToBuilder(ToMatrix(matrix)); + RefPtr builder = path->TransformedCopyToBuilder(matrix); path = builder->Finish(); } @@ -5119,8 +5131,6 @@ void SVGTextFrame::DoReflow() { bool SVGTextFrame::UpdateFontSizeScaleFactor() { double oldFontSizeScaleFactor = mFontSizeScaleFactor; - nsPresContext* presContext = PresContext(); - bool geometricPrecision = false; CSSCoord min = std::numeric_limits::max(); CSSCoord max = std::numeric_limits::min(); @@ -5158,31 +5168,9 @@ bool SVGTextFrame::UpdateFontSizeScaleFactor() { return mFontSizeScaleFactor != oldFontSizeScaleFactor; } - // When we are non-display, we could be painted in different coordinate - // spaces, and we don't want to have to reflow for each of these. We - // just assume that the context scale is 1.0 for them all, so we don't - // get stuck with a font size scale factor based on whichever referencing - // frame happens to reflow first. - double contextScale = 1.0; - if (!HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) { - gfxMatrix m(GetCanvasTM()); - if (!m.IsSingular()) { - contextScale = GetContextScale(m); - if (!std::isfinite(contextScale)) { - contextScale = 1.0f; - } - } - } + float contextScale = GetContextScale(this); mLastContextScale = contextScale; - // But we want to ignore any scaling required due to HiDPI displays, since - // regular CSS text frames will still create text runs using the font size - // in CSS pixels, and we want SVG text to have the same rendering as HTML - // text for regular font sizes. - float cssPxPerDevPx = nsPresContext::AppUnitsToFloatCSSPixels( - presContext->AppUnitsPerDevPixel()); - contextScale *= cssPxPerDevPx; - double minTextRunSize = min * contextScale; double maxTextRunSize = max * contextScale; diff --git a/layout/svg/SVGUtils.cpp b/layout/svg/SVGUtils.cpp index 577c9c75071d..02f1eb325a55 100644 --- a/layout/svg/SVGUtils.cpp +++ b/layout/svg/SVGUtils.cpp @@ -357,23 +357,10 @@ bool SVGUtils::IsSVGTransformed(const nsIFrame* aFrame, NS_FRAME_MAY_BE_TRANSFORMED), "Expecting an SVG frame that can be transformed"); bool foundTransform = false; - // Check if our parent has children-only transforms: if (SVGContainerFrame* parent = do_QueryFrame(aFrame->GetParent())) { foundTransform = parent->HasChildrenOnlyTransform(aFromParentTransform); } - - if (auto* content = SVGElement::FromNode(aFrame->GetContent())) { - auto* transformList = content->GetAnimatedTransformList(); - if ((transformList && transformList->HasTransform()) || - content->GetAnimateMotionTransform()) { - if (aOwnTransform) { - *aOwnTransform = gfx::ToMatrix( - content->PrependLocalTransformsTo(gfxMatrix(), eUserSpaceToParent)); - } - foundTransform = true; - } - } return foundTransform; } @@ -1532,15 +1519,14 @@ gfxMatrix SVGUtils::GetTransformMatrixInUserSpace(const nsIFrame* aFrame) { Matrix svgTransform; Matrix4x4 trans; - (void)aFrame->IsSVGTransformed(&svgTransform); - if (properties.HasTransform()) { trans = nsStyleTransformMatrix::ReadTransforms( properties.mTranslate, properties.mRotate, properties.mScale, properties.mMotion.ptrOr(nullptr), properties.mTransform, refBox, AppUnitsPerCSSPixel()); - } else { - trans = Matrix4x4::From2D(svgTransform); + } + if (aFrame->IsSVGTransformed(&svgTransform)) { + trans *= Matrix4x4::From2D(svgTransform); } trans.ChangeBasis(svgTransformOrigin); diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 62109ffa6a01..27d1d5f41394 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -5690,6 +5690,24 @@ pub extern "C" fn Servo_DeclarationBlock_SetLengthValue( }) } +#[no_mangle] +pub extern "C" fn Servo_DeclarationBlock_SetTransform( + declarations: &LockedDeclarationBlock, + property: nsCSSPropertyID, + ops: &nsTArray, +) { + use style::values::generics::transform::GenericTransform; + use style::properties::PropertyDeclaration; + let long = get_longhand_from_id!(property); + let v = GenericTransform(ops.iter().map(ToComputedValue::from_computed_value).collect()); + let prop = match_wrap_declared! { long, + Transform => v, + }; + write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| { + decls.push(prop, Importance::Normal); + }) +} + #[no_mangle] pub extern "C" fn Servo_DeclarationBlock_SetPathValue( declarations: &LockedDeclarationBlock, diff --git a/testing/web-platform/meta/css/css-transforms/translate/translate-and-transform-css-property-in-svg.html.ini b/testing/web-platform/meta/css/css-transforms/translate/translate-and-transform-css-property-in-svg.html.ini deleted file mode 100644 index a35b8392ee2f..000000000000 --- a/testing/web-platform/meta/css/css-transforms/translate/translate-and-transform-css-property-in-svg.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[translate-and-transform-css-property-in-svg.html] - expected: FAIL diff --git a/testing/web-platform/meta/svg/styling/presentation-attributes-relevant.html.ini b/testing/web-platform/meta/svg/styling/presentation-attributes-relevant.html.ini index f62af3a8644d..a0a642b52466 100644 --- a/testing/web-platform/meta/svg/styling/presentation-attributes-relevant.html.ini +++ b/testing/web-platform/meta/svg/styling/presentation-attributes-relevant.html.ini @@ -1,7 +1,3 @@ [presentation-attributes-relevant.html] [text-overflow presentation attribute supported on a relevant element] expected: FAIL - - [transform presentation attribute supported on a relevant element] - expected: FAIL - diff --git a/testing/web-platform/meta/svg/styling/presentation-attributes-special-cases.html.ini b/testing/web-platform/meta/svg/styling/presentation-attributes-special-cases.html.ini index 8e1567cf00c2..d8559cd76dfa 100644 --- a/testing/web-platform/meta/svg/styling/presentation-attributes-special-cases.html.ini +++ b/testing/web-platform/meta/svg/styling/presentation-attributes-special-cases.html.ini @@ -11,9 +11,6 @@ [fill presentation attribute not supported on discard] expected: FAIL - [transform presentation attribute supported on g] - expected: FAIL - [patternTransform presentation attribute supported on pattern] expected: FAIL diff --git a/testing/web-platform/tests/css/css-viewport/zoom/svg-transform-ref.html b/testing/web-platform/tests/css/css-viewport/zoom/svg-transform-ref.html new file mode 100644 index 000000000000..abaed2accea0 --- /dev/null +++ b/testing/web-platform/tests/css/css-viewport/zoom/svg-transform-ref.html @@ -0,0 +1,4 @@ + + + + diff --git a/testing/web-platform/tests/css/css-viewport/zoom/svg-transform.html b/testing/web-platform/tests/css/css-viewport/zoom/svg-transform.html new file mode 100644 index 000000000000..e0adf0725e86 --- /dev/null +++ b/testing/web-platform/tests/css/css-viewport/zoom/svg-transform.html @@ -0,0 +1,7 @@ + + + + + + +