Bug 1383650 - Support geometry property for SVG image element r=longsonr

The only different part is to resolve intrinsic image size. This patch
implements explicit requirements of the spec, but an edge case is tricky.
It's not clear per spec what the intrinsic image size is for an SVG
without explicit width/height, something like:

<svg>
  <image href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg'><rect width='40' height='90' fill='red' /></svg>"/>
</svg>

Chrome treats the intrinsic size of the href svg as the default size of
a replaced element (300x150), our image/VectorImage.cpp doesn't resolve
size in this case.

We can handle this particular case in some seperate bug if necessary, I think.

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

--HG--
extra : moz-landing-system : lando

dom/svg/SVGGeometryProperty.cpp

@@ -8,6 +8,7 @@
 #include "SVGCircleElement.h"
 #include "SVGEllipseElement.h"
 #include "SVGForeignObjectElement.h"
+#include "SVGImageElement.h"
 #include "SVGRectElement.h"
 
 namespace mozilla {
@@ -63,6 +64,9 @@ nsCSSPropertyID AttrEnumToCSSPropId(const SVGElement* aElement,
   if (aElement->IsSVGElement(nsGkAtoms::ellipse)) {
     return SVGEllipseElement::GetCSSPropertyIdForAttrEnum(aAttrEnum);
   }
+  if (aElement->IsSVGElement(nsGkAtoms::image)) {
+    return SVGImageElement::GetCSSPropertyIdForAttrEnum(aAttrEnum);
+  }
   if (aElement->IsSVGElement(nsGkAtoms::foreignObject)) {
     return SVGForeignObjectElement::GetCSSPropertyIdForAttrEnum(aAttrEnum);
   }
@@ -79,6 +83,7 @@ bool ElementMapsLengthsToStyle(SVGElement const* aElement) {
   return aElement->IsSVGElement(nsGkAtoms::rect) ||
          aElement->IsSVGElement(nsGkAtoms::circle) ||
          aElement->IsSVGElement(nsGkAtoms::ellipse) ||
+         aElement->IsSVGElement(nsGkAtoms::image) ||
          aElement->IsSVGElement(nsGkAtoms::foreignObject);
 }
 

dom/svg/SVGGeometryProperty.h

@@ -8,9 +8,11 @@
 #define mozilla_dom_SVGGeometryProperty_SVGGeometryProperty_h
 
 #include "mozilla/dom/SVGElement.h"
-#include "SVGAnimatedLength.h"
 #include "ComputedStyle.h"
+#include "SVGAnimatedLength.h"
+#include "nsGkAtoms.h"
 #include "nsIFrame.h"
+#include "nsSVGImageFrame.h"
 #include <type_traits>
 
 namespace mozilla {
@@ -38,13 +40,25 @@ SVGGEOMETRYPROPERTY_GENERATETAG(Y, LengthPercentNoAuto, Y, nsStyleSVGReset);
 SVGGEOMETRYPROPERTY_GENERATETAG(Cx, LengthPercentNoAuto, X, nsStyleSVGReset);
 SVGGEOMETRYPROPERTY_GENERATETAG(Cy, LengthPercentNoAuto, Y, nsStyleSVGReset);
 SVGGEOMETRYPROPERTY_GENERATETAG(R, LengthPercentNoAuto, XY, nsStyleSVGReset);
-SVGGEOMETRYPROPERTY_GENERATETAG(Width, LengthPercentWidthHeight, X,
-                                nsStylePosition);
-SVGGEOMETRYPROPERTY_GENERATETAG(Height, LengthPercentWidthHeight, Y,
-                                nsStylePosition);
 
 #undef SVGGEOMETRYPROPERTY_GENERATETAG
 
+struct Height;
+struct Width {
+  using ResolverType = ResolverTypes::LengthPercentWidthHeight;
+  constexpr static auto CtxDirection = SVGContentUtils::X;
+  constexpr static auto Getter = &nsStylePosition::mWidth;
+  constexpr static auto SizeGetter = &gfx::Size::width;
+  using CounterPart = Height;
+};
+struct Height {
+  using ResolverType = ResolverTypes::LengthPercentWidthHeight;
+  constexpr static auto CtxDirection = SVGContentUtils::Y;
+  constexpr static auto Getter = &nsStylePosition::mHeight;
+  constexpr static auto SizeGetter = &gfx::Size::height;
+  using CounterPart = Width;
+};
+
 struct Ry;
 struct Rx {
   using ResolverType = ResolverTypes::LengthPercentRXY;
@@ -94,7 +108,46 @@ float ResolveImpl(ComputedStyle const& aStyle, SVGElement* aElement,
         aElement, value.AsLengthPercentage());
   }
 
-  // |auto| and |max-content| etc. are treated as 0.
+  if (aElement->IsSVGElement(nsGkAtoms::image)) {
+    // It's not clear per SVG2 spec what should be done for values other
+    // than |auto| (e.g. |max-content|). We treat them as nonsense, thus
+    // using the initial value behavior, i.e. |auto|.
+
+    auto* f = aElement->GetPrimaryFrame();
+    MOZ_ASSERT(f && f->IsSVGImageFrame());
+    auto* imgf = static_cast<nsSVGImageFrame const*>(f);
+
+    using Other = typename Tag::CounterPart;
+    auto const& valueOther = aStyle.StylePosition()->*Other::Getter;
+
+    gfx::Size intrinsicImageSize;
+    if (!imgf->GetIntrinsicImageSize(intrinsicImageSize)) {
+      // Cannot get intrinsic image size, just return 0.
+      return 0.f;
+    }
+
+    if (valueOther.IsLengthPercentage()) {
+      // We are |auto|, but the other side has specifed length. Then
+      // we need to preserve aspect ratio.
+
+      float intrinsicLengthOther = intrinsicImageSize.*Other::SizeGetter;
+      if (!intrinsicLengthOther) {
+        // Avoid dividing by 0.
+        return 0.f;
+      }
+
+      float intrinsicLength = intrinsicImageSize.*Tag::SizeGetter,
+            lengthOther = ResolvePureLengthPercentage<Other::CtxDirection>(
+                aElement, valueOther.AsLengthPercentage());
+
+      return intrinsicLength * lengthOther / intrinsicLengthOther;
+    }
+
+    // So |width| and |height| are both |auto|, just use intrinsic size.
+    return intrinsicImageSize.*Tag::SizeGetter;
+  }
+
+  // For other elements, |auto| and |max-content| etc. are treated as 0.
   return 0.f;
 }
 

dom/svg/SVGImageElement.cpp

@@ -17,6 +17,7 @@
 #include "mozilla/dom/SVGImageElementBinding.h"
 #include "mozilla/dom/SVGLengthBinding.h"
 #include "nsContentUtils.h"
+#include "SVGGeometryProperty.h"
 
 NS_IMPL_NS_NEW_SVG_ELEMENT(Image)
 
@@ -54,6 +55,8 @@ NS_IMPL_ISUPPORTS_INHERITED(SVGImageElement, SVGImageElementBase,
 //----------------------------------------------------------------------
 // Implementation
 
+namespace SVGT = SVGGeometryProperty::Tags;
+
 SVGImageElement::SVGImageElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
     : SVGImageElementBase(std::move(aNodeInfo)) {
@@ -63,6 +66,22 @@ SVGImageElement::SVGImageElement(
 
 SVGImageElement::~SVGImageElement() { DestroyImageLoadingContent(); }
 
+nsCSSPropertyID SVGImageElement::GetCSSPropertyIdForAttrEnum(
+    uint8_t aAttrEnum) {
+  switch (aAttrEnum) {
+    case ATTR_X:
+      return eCSSProperty_x;
+    case ATTR_Y:
+      return eCSSProperty_y;
+    case ATTR_WIDTH:
+      return eCSSProperty_width;
+    case ATTR_HEIGHT:
+      return eCSSProperty_height;
+    default:
+      MOZ_ASSERT_UNREACHABLE("Unknown attr enum");
+      return eCSSProperty_UNKNOWN;
+  }
+}
 //----------------------------------------------------------------------
 // nsINode methods
 
@@ -232,7 +251,10 @@ bool SVGImageElement::GetGeometryBounds(
     Rect* aBounds, const StrokeOptions& aStrokeOptions,
     const Matrix& aToBoundsSpace, const Matrix* aToNonScalingStrokeSpace) {
   Rect rect;
-  GetAnimatedLengthValues(&rect.x, &rect.y, &rect.width, &rect.height, nullptr);
+
+  MOZ_ASSERT(GetPrimaryFrame());
+  SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height>(
+      this, &rect.x, &rect.y, &rect.width, &rect.height);
 
   if (rect.IsEmpty()) {
     // Rendering of the element disabled
@@ -248,7 +270,9 @@ already_AddRefed<Path> SVGImageElement::BuildPath(PathBuilder* aBuilder) {
   // hit-testing against it. For that we just pretend to be a rectangle.
 
   float x, y, width, height;
-  GetAnimatedLengthValues(&x, &y, &width, &height, nullptr);
+  MOZ_ASSERT(GetPrimaryFrame());
+  SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height>(
+      this, &x, &y, &width, &height);
 
   if (width <= 0 || height <= 0) {
     return nullptr;
@@ -269,10 +293,13 @@ already_AddRefed<Path> SVGImageElement::BuildPath(PathBuilder* aBuilder) {
 
 /* virtual */
 bool SVGImageElement::HasValidDimensions() const {
-  return mLengthAttributes[ATTR_WIDTH].IsExplicitlySet() &&
-         mLengthAttributes[ATTR_WIDTH].GetAnimValInSpecifiedUnits() > 0 &&
-         mLengthAttributes[ATTR_HEIGHT].IsExplicitlySet() &&
-         mLengthAttributes[ATTR_HEIGHT].GetAnimValInSpecifiedUnits() > 0;
+  float width, height;
+
+  MOZ_ASSERT(GetPrimaryFrame());
+  SVGGeometryProperty::ResolveAll<SVGT::Width, SVGT::Height>(this, &width,
+                                                             &height);
+
+  return width > 0 && height > 0;
 }
 
 SVGElement::LengthAttributesInfo SVGImageElement::GetLengthInfo() {

dom/svg/SVGImageElement.h

@@ -95,6 +95,8 @@ class SVGImageElement : public SVGImageElementBase,
 
   already_AddRefed<Promise> Decode(ErrorResult& aRv);
 
+  static nsCSSPropertyID GetCSSPropertyIdForAttrEnum(uint8_t aAttrEnum);
+
  protected:
   nsresult LoadSVGImage(bool aForce, bool aNotify);
 

layout/reftests/svg/geometry-properties-in-css-ref.html

@@ -17,4 +17,5 @@
       <rect width="50" height="50" rx="4" ry="4" fill="brown" />
     </svg>
   </foreignObject>
+  <rect x="300" y="260" width="50" height="50" fill="red" />
 </svg>

layout/reftests/svg/geometry-properties-in-css.html

@@ -53,6 +53,11 @@
     width: 50px;
     height:50px;
   }
+  svg image {
+    x: 300px;
+    y: 260px;
+    height: 50px;
+  }
 </style>
 <svg>
   <g>
@@ -72,4 +77,5 @@
       <rect id="r2" style="x:0;y:0" fill="brown" />
     </svg>
   </foreignObject>
+  <image href="data:image/svg+xml,<svg width='10' height='10' xmlns='http://www.w3.org/2000/svg'><rect width='100%' height='100%' fill='red' /></svg>" />
 </svg>

layout/svg/nsSVGImageFrame.cpp

@@ -15,6 +15,7 @@
 #include "nsIImageLoadingContent.h"
 #include "nsLayoutUtils.h"
 #include "imgINotificationObserver.h"
+#include "SVGGeometryProperty.h"
 #include "SVGObserverUtils.h"
 #include "nsSVGUtils.h"
 #include "SVGContentUtils.h"
@@ -30,6 +31,7 @@ using namespace mozilla;
 using namespace mozilla::dom;
 using namespace mozilla::gfx;
 using namespace mozilla::image;
+namespace SVGT = SVGGeometryProperty::Tags;
 
 // ---------------------------------------------------------------------
 // nsQueryFrame methods
@@ -177,7 +179,8 @@ gfx::Matrix nsSVGImageFrame::GetRasterImageTransform(int32_t aNativeWidth,
                                                      int32_t aNativeHeight) {
   float x, y, width, height;
   SVGImageElement* element = static_cast<SVGImageElement*>(GetContent());
-  element->GetAnimatedLengthValues(&x, &y, &width, &height, nullptr);
+  SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height>(
+      element, &x, &y, &width, &height);
 
   Matrix viewBoxTM = SVGContentUtils::GetViewBoxTransform(
       width, height, 0, 0, aNativeWidth, aNativeHeight,
@@ -189,7 +192,8 @@ gfx::Matrix nsSVGImageFrame::GetRasterImageTransform(int32_t aNativeWidth,
 gfx::Matrix nsSVGImageFrame::GetVectorImageTransform() {
   float x, y, width, height;
   SVGImageElement* element = static_cast<SVGImageElement*>(GetContent());
-  element->GetAnimatedLengthValues(&x, &y, &width, &height, nullptr);
+  SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height>(
+      element, &x, &y, &width, &height);
 
   // No viewBoxTM needed here -- our height/width overrides any concept of
   // "native size" that the SVG image has, and it will handle viewBox and
@@ -198,6 +202,26 @@ gfx::Matrix nsSVGImageFrame::GetVectorImageTransform() {
   return gfx::Matrix::Translation(x, y);
 }
 
+bool nsSVGImageFrame::GetIntrinsicImageSize(
+    mozilla::gfx::Size& aIntrinsicSize) const {
+  if (!mImageContainer) {
+    return false;
+  }
+
+  int32_t width, height;
+  if (NS_FAILED(mImageContainer->GetWidth(&width))) {
+    return false;
+  }
+
+  if (NS_FAILED(mImageContainer->GetHeight(&height))) {
+    return false;
+  }
+
+  aIntrinsicSize = {float(width), float(height)};
+
+  return true;
+}
+
 bool nsSVGImageFrame::TransformContextForPainting(gfxContext* aGfxContext,
                                                   const gfxMatrix& aTransform) {
   gfx::Matrix imageTransform;
@@ -241,7 +265,8 @@ void nsSVGImageFrame::PaintSVG(gfxContext& aContext,
 
   float x, y, width, height;
   SVGImageElement* imgElem = static_cast<SVGImageElement*>(GetContent());
-  imgElem->GetAnimatedLengthValues(&x, &y, &width, &height, nullptr);
+  SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height>(
+      imgElem, &x, &y, &width, &height);
   NS_ASSERTION(width > 0 && height > 0,
                "Should only be painting things with valid width/height");
 
@@ -347,8 +372,8 @@ nsIFrame* nsSVGImageFrame::GetFrameForPoint(const gfxPoint& aPoint) {
 
   Rect rect;
   SVGImageElement* element = static_cast<SVGImageElement*>(GetContent());
-  element->GetAnimatedLengthValues(&rect.x, &rect.y, &rect.width, &rect.height,
-                                   nullptr);
+  SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height>(
+      element, &rect.x, &rect.y, &rect.width, &rect.height);
 
   if (!rect.Contains(ToPoint(aPoint))) {
     return nullptr;
@@ -402,7 +427,8 @@ void nsSVGImageFrame::ReflowSVG() {
 
   float x, y, width, height;
   SVGImageElement* element = static_cast<SVGImageElement*>(GetContent());
-  element->GetAnimatedLengthValues(&x, &y, &width, &height, nullptr);
+  SVGGeometryProperty::ResolveAll<SVGT::X, SVGT::Y, SVGT::Width, SVGT::Height>(
+      element, &x, &y, &width, &height);
 
   Rect extent(x, y, width, height);
 

layout/svg/nsSVGImageFrame.h

@@ -88,6 +88,8 @@ class nsSVGImageFrame final : public mozilla::SVGGeometryFrame,
   virtual void DestroyFrom(nsIFrame* aDestructRoot,
                            PostDestroyData& aPostDestroyData) override;
 
+  bool GetIntrinsicImageSize(mozilla::gfx::Size& aIntrinsicSize) const;
+
 #ifdef DEBUG_FRAME_DUMP
   virtual nsresult GetFrameName(nsAString& aResult) const override {
     return MakeFrameName(NS_LITERAL_STRING("SVGImage"), aResult);

testing/web-platform/meta/svg/geometry/svg-image-intrinsic-size-with-cssstyle-auto-dynamic-image-change.html.ini

@@ -1,4 +0,0 @@
-[svg-image-intrinsic-size-with-cssstyle-auto-dynamic-image-change.html]
-  [Test <svg:image>'s sizing with css size as auto, with dynamic image change]
-    expected: FAIL
-

testing/web-platform/meta/svg/geometry/svg-image-intrinsic-size-with-cssstyle-auto.html.ini

@@ -1,25 +1,13 @@
 [svg-image-intrinsic-size-with-cssstyle-auto.html]
-  [<svg:image> 'auto' sizing with 200x100 svg image viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'height:auto']
-    expected: FAIL
-
   [<svg:image> 'auto' sizing with default sized svg image viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'width:auto']
     expected: FAIL
 
   [<svg:image> 'auto' sizing with default sized svg image, attributes width='200' and CSS 'width:auto; height:auto']
     expected: FAIL
 
-  [<svg:image> 'auto' sizing with 200x100 svg image, attributes width='50' height='50' and CSS 'width:auto; height:auto']
-    expected: FAIL
-
-  [<svg:image> 'auto' sizing with 256x256 png image, attributes width='64' height='128' and CSS 'height:auto']
-    expected: FAIL
-
   [<svg:image> 'auto' sizing with default sized svg image, without attributes width and height, no css width/height specified]
     expected: FAIL
 
-  [<svg:image> 'auto' sizing with 200x100 svg image, without attributes width and height and CSS 'width:auto']
-    expected: FAIL
-
   [<svg:image> 'auto' sizing with svg image height='100' viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'height:auto']
     expected: FAIL
 
@@ -29,12 +17,6 @@
   [<svg:image> 'auto' sizing with svg image height='100' viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'width:auto; height:auto']
     expected: FAIL
 
-  [<svg:image> 'auto' sizing with 256x256 png image, attributes width='64' height='64' and CSS 'width:auto; height:auto']
-    expected: FAIL
-
-  [<svg:image> 'auto' sizing with 200x100 svg image viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'width:auto; height:auto']
-    expected: FAIL
-
   [<svg:image> 'auto' sizing with default sized svg image, attributes height='200' and CSS 'width:auto; height:auto']
     expected: FAIL
 
@@ -44,30 +26,15 @@
   [<svg:image> 'auto' sizing with svg image width='200' viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'height:auto']
     expected: FAIL
 
-  [<svg:image> 'auto' sizing with 200x100 svg image, attributes width='50' height='50' and CSS 'width:auto']
-    expected: FAIL
-
   [<svg:image> 'auto' sizing with svg image height='100' viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'width:auto']
     expected: FAIL
 
   [<svg:image> 'auto' sizing with default sized svg image, attributes width='200' height='200' and CSS 'width:auto; height:auto']
     expected: FAIL
 
-  [<svg:image> 'auto' sizing with 200x100 svg image viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'width:auto']
-    expected: FAIL
-
   [<svg:image> 'auto' sizing with default sized svg image viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'height:auto']
     expected: FAIL
 
-  [<svg:image> 'auto' sizing with 200x100 svg image, attributes width='50' and CSS height:auto]
-    expected: FAIL
-
-  [<svg:image> 'auto' sizing with 256x256 png image, attributes width='128' height='64' and CSS 'width:auto']
-    expected: FAIL
-
-  [<svg:image> 'auto' sizing with 200x100 svg image, attributes height='50' and 'width:auto']
-    expected: FAIL
-
   [<svg:image> 'auto' sizing with default sized svg image viewBox='0 0 400 100', attributes width='60' height='60' and CSS 'width:auto; height:auto']
     expected: FAIL