Bug 1914221 - Map pattern/gradientTransform to the transform property. r=longsonr

And fix a typo in a test which got me confused.

Differential Revision: https://phabricator.services.mozilla.com/D220728

dom/svg/SVGElement.cpp

@@ -1334,38 +1334,43 @@ void SVGElement::UpdateMappedDeclarationBlock() {
   const bool lengthAffectsStyle =
       SVGGeometryProperty::ElementMapsLengthsToStyle(this);
   bool sawTransform = false;
-
   uint32_t i = 0;
   while (BorrowedAttrInfo info = GetAttrInfoAt(i++)) {
     const nsAttrName* attrName = info.mName;
-    if (!attrName->IsAtom() || !IsAttributeMapped(attrName->Atom())) {
+    if (!attrName->IsAtom()) {
       continue;
     }
 
-    if (attrName->Atom() == nsGkAtoms::lang &&
+    nsAtom* nameAtom = attrName->Atom();
+    if (!IsAttributeMapped(nameAtom)) {
+      continue;
+    }
+
+    if (nameAtom == nsGkAtoms::lang &&
         HasAttr(kNameSpaceID_XML, nsGkAtoms::lang)) {
       // xml:lang has precedence, and will get set via Gecko_GetXMLLangValue().
       continue;
     }
 
     if (lengthAffectsStyle) {
-      auto const* length = GetAnimatedLength(attrName->Atom());
+      auto const* length = GetAnimatedLength(nameAtom);
 
       if (length && length->HasBaseVal()) {
         // This is an element with geometry property set via SVG attribute,
         // and the attribute is already successfully parsed. 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(attrName->Atom(),
-                                                      *length);
+        mappedAttrParser.TellStyleAlreadyParsedResult(nameAtom, *length);
         continue;
       }
     }
 
-    if (attrName->Atom() == nsGkAtoms::transform) {
+    if (nameAtom == nsGkAtoms::transform ||
+        nameAtom == nsGkAtoms::patternTransform ||
+        nameAtom == nsGkAtoms::gradientTransform) {
       sawTransform = true;
       const auto* transform = GetAnimatedTransformList();
-      MOZ_ASSERT(GetTransformListAttrName() == nsGkAtoms::transform);
+      MOZ_ASSERT(GetTransformListAttrName() == nameAtom);
       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.
@@ -1373,7 +1378,7 @@ void SVGElement::UpdateMappedDeclarationBlock() {
       continue;
     }
 
-    if (attrName->Atom() == nsGkAtoms::d) {
+    if (nameAtom == nsGkAtoms::d) {
       const auto* path = GetAnimPathSegList();
       // Note: Only SVGPathElement has d attribute.
       MOZ_ASSERT(
@@ -1399,7 +1404,7 @@ void SVGElement::UpdateMappedDeclarationBlock() {
 
     nsAutoString value;
     info.mValue->ToString(value);
-    mappedAttrParser.ParseMappedAttrValue(attrName->Atom(), value);
+    mappedAttrParser.ParseMappedAttrValue(nameAtom, value);
   }
 
   // We need to map the SVG view's transform if we haven't mapped it already.
@@ -2082,21 +2087,15 @@ void SVGElement::DidChangeTransformList(
 void SVGElement::DidAnimateTransformList(int32_t aModType) {
   MOZ_ASSERT(GetTransformListAttrName(),
              "Animating non-existent transform data?");
-
-  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);
+    SMILOverrideStyle()->SetSMILValue(eCSSProperty_transform, animTransformList,
+                                      animateMotion);
   } else {
     SMILOverrideStyle()->ClearSMILValue(eCSSProperty_transform);
   }
-    return;
-  }
-  DidAnimateAttribute(kNameSpaceID_None, transformAttr);
 }
 
 SVGElement::StringAttributesInfo SVGElement::GetStringInfo() {

dom/svg/SVGGradientElement.cpp

@@ -208,6 +208,11 @@ already_AddRefed<DOMSVGAnimatedLength> SVGRadialGradientElement::Fr() {
   return mLengthAttributes[ATTR_FR].ToDOMAnimatedLength(this);
 }
 
+bool SVGGradientElement::IsAttributeMapped(const nsAtom* aAttribute) const {
+  return aAttribute == nsGkAtoms::gradientTransform ||
+         SVGElement::IsAttributeMapped(aAttribute);
+}
+
 //----------------------------------------------------------------------
 // SVGElement methods
 

dom/svg/SVGGradientElement.h

@@ -45,11 +45,12 @@ class SVGGradientElement : public SVGGradientElementBase {
   // nsIContent
   nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override = 0;
 
-  virtual SVGAnimatedTransformList* GetAnimatedTransformList(
+  SVGAnimatedTransformList* GetAnimatedTransformList(
       uint32_t aFlags = 0) override;
   nsStaticAtom* GetTransformListAttrName() const override {
     return nsGkAtoms::gradientTransform;
   }
+  NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
 
   // WebIDL
   already_AddRefed<DOMSVGAnimatedEnumeration> GradientUnits();

dom/svg/SVGPatternElement.cpp

@@ -66,6 +66,11 @@ already_AddRefed<SVGAnimatedRect> SVGPatternElement::ViewBox() {
   return mViewBox.ToSVGAnimatedRect(this);
 }
 
+bool SVGPatternElement::IsAttributeMapped(const nsAtom* aAttribute) const {
+  return aAttribute == nsGkAtoms::patternTransform ||
+         SVGPatternElementBase::IsAttributeMapped(aAttribute);
+}
+
 already_AddRefed<DOMSVGAnimatedPreserveAspectRatio>
 SVGPatternElement::PreserveAspectRatio() {
   return mPreserveAspectRatio.ToDOMAnimatedPreserveAspectRatio(this);

dom/svg/SVGPatternElement.h

@@ -41,6 +41,7 @@ class SVGPatternElement final : public SVGPatternElementBase {
  public:
   // nsIContent interface
   nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override;
+  NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
 
   // SVGSVGElement methods:
   bool HasValidDimensions() const override;

layout/reftests/svg/pattern-transform-presence-01-ref.svg

@@ -8,18 +8,12 @@
     <rect y="50" width="50" height="50" fill="red"/>
     <rect x="50" y="50" width="50" height="50" fill="blue"/>
   </pattern>
-  <pattern id="patternNotRotated" width="1" height="1">
-    <rect width="50" height="50" fill="blue"/>
-    <rect x="50" width="50" height="50" fill="red"/>
-    <rect y="50" width="50" height="50" fill="red"/>
-    <rect x="50" y="50" width="50" height="50" fill="blue"/>
-  </pattern>
 </defs>
 <rect width="100" height="100" stroke="black" fill="url(#patternRotated)"/>
 <g transform="translate(100)">
   <rect width="100" height="100" stroke="black" fill="url(#patternRotated)"/>
 </g>
 <g transform="translate(200)">
-  <rect width="100" height="100" stroke="black" fill="url(#patternNotRotated)"/>
+  <rect width="100" height="100" stroke="black" fill="url(#patternRotated)"/>
 </g>
 </svg>

layout/reftests/svg/pattern-transform-presence-01.svg

@@ -37,9 +37,8 @@ function addTransform()
        which are not defined on this element are inherited by this element.').
        Hence this pattern should look IDENTICAL to patternBase. -->
   <pattern xlink:href="#patternBase" id="patternRefWithoutTransform"/>
-  <!-- 3. References the base pattern but patternTransform is defined (although
-       empty) and hence the patternTransform should NOT be inherited and this
-       pattern should look DIFFERENT to patternBase. -->
+  <!-- There's no way to differentiate an explicitly specified (but empty)
+       transform from no transform, so this should look IDENTICAL to patternBase -->
   <pattern xlink:href="#patternBase" id="patternRefWithTransform"
     patternTransform=""/>
   <!-- The case of a patternTransform being supplied by animation is covered by

layout/svg/SVGGradientFrame.cpp

@@ -108,15 +108,11 @@ uint16_t SVGGradientFrame::GetSpreadMethod() {
   return GetEnumValue(dom::SVGGradientElement::SPREADMETHOD);
 }
 
-const SVGAnimatedTransformList* SVGGradientFrame::GetGradientTransformList(
-    nsIContent* aDefault) {
-  SVGAnimatedTransformList* thisTransformList =
-      static_cast<dom::SVGGradientElement*>(GetContent())
-          ->GetAnimatedTransformList();
-
-  if (thisTransformList && thisTransformList->IsExplicitlySet())
-    return thisTransformList;
-
+SVGGradientFrame* SVGGradientFrame::GetGradientTransformFrame(
+    SVGGradientFrame* aDefault) {
+  if (!StyleDisplay()->mTransform.IsNone()) {
+    return this;
+  }
   // Before we recurse, make sure we'll break reference loops and over long
   // reference chains:
   static int16_t sRefChainLengthCounter = AutoReferenceChainGuard::noChain;
@@ -124,21 +120,18 @@ const SVGAnimatedTransformList* SVGGradientFrame::GetGradientTransformList(
                                         &sRefChainLengthCounter);
   if (MOZ_UNLIKELY(!refChainGuard.Reference())) {
     // Break reference chain
-    return static_cast<const dom::SVGGradientElement*>(aDefault)
-        ->mGradientTransform.get();
+    return aDefault;
   }
 
-  SVGGradientFrame* next = GetReferencedGradient();
-
-  return next ? next->GetGradientTransformList(aDefault)
-              : static_cast<const dom::SVGGradientElement*>(aDefault)
-                    ->mGradientTransform.get();
+  if (SVGGradientFrame* next = GetReferencedGradient()) {
+    return next->GetGradientTransformFrame(aDefault);
+  }
+  return aDefault;
 }
 
 gfxMatrix SVGGradientFrame::GetGradientTransform(
     nsIFrame* aSource, const gfxRect* aOverrideBounds) {
   gfxMatrix bboxMatrix;
-
   uint16_t gradientUnits = GetGradientUnits();
   if (gradientUnits != SVG_UNIT_TYPE_USERSPACEONUSE) {
     NS_ASSERTION(gradientUnits == SVG_UNIT_TYPE_OBJECTBOUNDINGBOX,
@@ -154,16 +147,8 @@ gfxMatrix SVGGradientFrame::GetGradientTransform(
         gfxMatrix(bbox.Width(), 0, 0, bbox.Height(), bbox.X(), bbox.Y());
   }
 
-  const SVGAnimatedTransformList* animTransformList =
-      GetGradientTransformList(GetContent());
-  if (!animTransformList) {
   return bboxMatrix.PreMultiply(
-        SVGUtils::GetTransformMatrixInUserSpace(this));
-  }
-
-  gfxMatrix gradientTransform =
-      animTransformList->GetAnimValue().GetConsolidationMatrix();
-  return bboxMatrix.PreMultiply(gradientTransform);
+      SVGUtils::GetTransformMatrixInUserSpace(GetGradientTransformFrame(this)));
 }
 
 dom::SVGLinearGradientElement* SVGGradientFrame::GetLinearGradientWithLength(
@@ -294,7 +279,6 @@ already_AddRefed<gfxPattern> SVGGradientFrame::GetPaintServerPattern(
   // above since this call can be expensive when "gradientUnits" is set to
   // "objectBoundingBox" (since that requiring a GetBBox() call).
   gfxMatrix patternMatrix = GetGradientTransform(aSource, aOverrideBounds);
-
   if (patternMatrix.IsSingular()) {
     return nullptr;
   }

layout/svg/SVGGradientFrame.h

@@ -75,8 +75,7 @@ class SVGGradientFrame : public SVGPaintServerFrame {
 
   void GetStops(nsTArray<ColorStop>* aStops, float aGraphicOpacity);
 
-  const SVGAnimatedTransformList* GetGradientTransformList(
-      nsIContent* aDefault);
+  SVGGradientFrame* GetGradientTransformFrame(SVGGradientFrame* aDefault);
   // Will be singular for gradientUnits="objectBoundingBox" with an empty bbox.
   gfxMatrix GetGradientTransform(nsIFrame* aSource,
                                  const gfxRect* aOverrideBounds);

layout/svg/SVGPatternFrame.cpp

@@ -439,13 +439,11 @@ uint16_t SVGPatternFrame::GetEnumValue(uint32_t aIndex, nsIContent* aDefault) {
                     .GetAnimValue();
 }
 
-SVGAnimatedTransformList* SVGPatternFrame::GetPatternTransformList(
-    nsIContent* aDefault) {
-  SVGAnimatedTransformList* thisTransformList =
-      static_cast<SVGPatternElement*>(GetContent())->GetAnimatedTransformList();
-
-  if (thisTransformList && thisTransformList->IsExplicitlySet())
-    return thisTransformList;
+SVGPatternFrame* SVGPatternFrame::GetPatternTransformFrame(
+    SVGPatternFrame* aDefault) {
+  if (!StyleDisplay()->mTransform.IsNone()) {
+    return this;
+  }
 
   // Before we recurse, make sure we'll break reference loops and over long
   // reference chains:
@@ -454,23 +452,18 @@ SVGAnimatedTransformList* SVGPatternFrame::GetPatternTransformList(
                                         &sRefChainLengthCounter);
   if (MOZ_UNLIKELY(!refChainGuard.Reference())) {
     // Break reference chain
-    return static_cast<SVGPatternElement*>(aDefault)->mPatternTransform.get();
+    return aDefault;
   }
 
-  SVGPatternFrame* next = GetReferencedPattern();
-  return next ? next->GetPatternTransformList(aDefault)
-              : static_cast<SVGPatternElement*>(aDefault)
-                    ->mPatternTransform.get();
+  if (SVGPatternFrame* next = GetReferencedPattern()) {
+    return next->GetPatternTransformFrame(aDefault);
+  }
+  return aDefault;
 }
 
 gfxMatrix SVGPatternFrame::GetPatternTransform() {
-  SVGAnimatedTransformList* animTransformList =
-      GetPatternTransformList(GetContent());
-  if (!animTransformList) {
-    return SVGUtils::GetTransformMatrixInUserSpace(this);
-  }
-
-  return animTransformList->GetAnimValue().GetConsolidationMatrix();
+  return SVGUtils::GetTransformMatrixInUserSpace(
+      GetPatternTransformFrame(this));
 }
 
 const SVGAnimatedViewBox& SVGPatternFrame::GetViewBox(nsIContent* aDefault) {

layout/svg/SVGPatternFrame.h

@@ -80,7 +80,7 @@ class SVGPatternFrame final : public SVGPaintServerFrame {
   uint16_t GetEnumValue(uint32_t aIndex) {
     return GetEnumValue(aIndex, mContent);
   }
-  SVGAnimatedTransformList* GetPatternTransformList(nsIContent* aDefault);
+  SVGPatternFrame* GetPatternTransformFrame(SVGPatternFrame* aDefault);
   gfxMatrix GetPatternTransform();
   const SVGAnimatedViewBox& GetViewBox(nsIContent* aDefault);
   const SVGAnimatedViewBox& GetViewBox() { return GetViewBox(mContent); }

servo/components/style/sharing/mod.rs

@@ -677,6 +677,11 @@ impl<E: TElement> StyleSharingCache<E> {
             return;
         }
 
+        if element.smil_override().is_some() {
+            debug!("Failing to insert to the cache: SMIL");
+            return;
+        }
+
         debug!(
             "Inserting into cache: {:?} with parent {:?}",
             element, parent
@@ -814,6 +819,11 @@ impl<E: TElement> StyleSharingCache<E> {
             return None;
         }
 
+        if target.element.smil_override().is_some() {
+            trace!("Miss: SMIL");
+            return None;
+        }
+
         if target.matches_user_and_content_rules() !=
             candidate.element.matches_user_and_content_rules()
         {

testing/web-platform/meta/svg/styling/presentation-attributes-special-cases.html.ini

@@ -10,16 +10,3 @@
 
   [fill presentation attribute not supported on discard]
     expected: FAIL
-
-  [patternTransform presentation attribute supported on pattern]
-    expected: FAIL
-
-  [patternTransform presentation attribute supported on linearGradient]
-    expected: FAIL
-
-  [patternTransform presentation attribute supported on radialGradient]
-    expected: FAIL
-
-  [transform presentation attribute supported on g]
-    expected:
-      if (os == "mac") and not debug: [PASS, FAIL]

testing/web-platform/tests/svg/styling/presentation-attributes-special-cases.html

@@ -121,7 +121,7 @@ if (CSS.supports("transform", "initial")) {
   for (let e of ["linearGradient", "radialGradient"]) {
     test(function() {
       assertPresentationAttributeIsSupported(e, "gradientTransform", "scale(2)", "transform");
-    }, `patternTransform presentation attribute supported on ${e}`);
+    }, `gradientTransform presentation attribute supported on ${e}`);
   }