Bug 272288 Patch 1: Fix nsSVGImageFrame to handle SVG images. r=roc a=roc

content/svg/content/src/crashtests/crashtests.list

@@ -55,3 +55,4 @@ load 595608-1.svg
 load 601251-1.html
 load 601406-1.svg
 load 603145-1.svg
+load zero-size-image.svg

content/svg/content/src/crashtests/zero-size-image.svg

@@ -0,0 +1,6 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink">
+  <!-- Test that we don't fail assertions with zero-size <image> elements. -->
+  <image xlink:href="data:image/svg+xml,%3Csvg%3E%3C%2Fsvg%3E"/>
+  <image xlink:href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAAAXNSR0IArs4c6QAAAApJREFUCNdjYAAAAAIAAeIhvDMAAAAASUVORK5CYII%3D"/>
+</svg>

layout/reftests/svg/as-image/reftest.list

@@ -73,3 +73,13 @@ random-if(winWidget) == img-width-slice-1.html  img-width-slice-1-ref.html
 == img-width-slice-2.html  img-width-slice-2-ref.html
 
 == list-simple-1.html list-simple-1-ref.html
+
+== svg-image-simple-1.svg lime100x100.svg
+== svg-image-simple-2.svg lime100x100.svg
+== svg-image-simple-3.svg lime100x100.svg
+
+# tests for <svg> files that include themselves as an <image>
+== svg-image-recursive-1a.svg  svg-image-recursive-1-ref.svg
+== svg-image-recursive-1b.svg  svg-image-recursive-1-ref.svg
+== svg-image-recursive-2a.svg  svg-image-recursive-2-ref.svg
+== svg-image-recursive-2b.html svg-image-recursive-2-ref.svg

layout/reftests/svg/as-image/svg-image-recursive-1-ref.svg

@@ -0,0 +1,6 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="100" height="100">
+  <circle cx="10" cy="10" r="10" fill="blue"/>
+  <circle cx="30" cy="30" r="10" fill="blue"/>
+</svg>

layout/reftests/svg/as-image/svg-image-recursive-1a.svg

@@ -0,0 +1,13 @@
+<!-- This SVG file uses itself as an image.  Currently, we don't paint
+     recursively-referenced images beyond the first level.  When this testcase
+     is viewed directly, it gets treated as a document (not an image), so its
+     <image> element is painted.  However, the <image>'s own *internal* <image>
+     element does *not* get painted.  So we end up painting two blue circles:
+     one for the <circle> and one for the <image>'s <circle> (and no more). -->
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="100" height="100">
+  <circle cx="10" cy="10" r="10" fill="blue"/>
+  <image x="20" y="20" width="100" height="100"
+         xlink:href="svg-image-recursive-1a.svg"/> <!-- my own filename -->
+</svg>

layout/reftests/svg/as-image/svg-image-recursive-1b.svg

@@ -0,0 +1,15 @@
+<!-- This SVG file uses itself as an image.  Currently, we don't paint
+     recursively-referenced images beyond the first level.  When this testcase
+     is viewed directly, it gets treated as a document (not an image), so its
+     <image> element is painted.  However, the <image>'s own *internal* <image>
+     element does *not* get painted.  So we end up painting two blue circles:
+     one for the <circle> and one for the <image>'s <circle> (and no more). -->
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="100" height="100">
+  <circle cx="10" cy="10" r="10" fill="blue"/>
+  <image x="20" y="20" width="100" height="100"
+         xlink:href="#foo"/>
+  <!-- When used as a URL, #foo evaluates to ${my_url}#foo, which (when treated
+       as an image URL) just turns into ${my_url}. -->
+</svg>

layout/reftests/svg/as-image/svg-image-recursive-2-ref.svg

@@ -0,0 +1,5 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="100" height="100">
+  <circle cx="10" cy="10" r="10" fill="blue"/>
+</svg>

layout/reftests/svg/as-image/svg-image-recursive-2a.svg

@@ -0,0 +1,10 @@
+<!-- The SVG file referenced in our <image> tag uses itself as an <image>.
+     Currently, we don't paint recursively-referenced images beyond the first
+     level.  So, the <image> element inside our helper document doesn't get
+     painted, and we end up only showing one blue circle. -->
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="100" height="100">
+  <image width="100" height="100"
+         xlink:href="svg-image-recursive-1a.svg"/>
+</svg>

layout/reftests/svg/as-image/svg-image-recursive-2b.html

@@ -0,0 +1,9 @@
+<!-- The SVG file referenced in our <img> tag uses itself as an <image>.
+     Currently, we don't paint recursively-referenced images beyond the first
+     level.  So, the <image> element inside our helper document doesn't get
+     painted, and we end up only showing one blue circle. -->
+<html>
+<body style="margin: 0">
+  <img src="svg-image-recursive-1a.svg">
+</body>
+</html>

layout/reftests/svg/as-image/svg-image-simple-1.svg

@@ -0,0 +1,6 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="100" height="100">
+  <rect  width="100%" height="100%" fill="red"/>
+  <image width="100%" height="100%" xlink:href="lime100x100.svg"/>
+</svg>

layout/reftests/svg/as-image/svg-image-simple-2.svg

@@ -0,0 +1,7 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="100" height="100">
+  <rect  width="100%" height="100%" fill="red"/>
+  <image width="100%" height="100%"
+         xlink:href="lime100x100-noSVGDimensions.svg"/>
+</svg>

layout/reftests/svg/as-image/svg-image-simple-3.svg

@@ -0,0 +1,7 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     width="100" height="100">
+  <rect  width="100%" height="100%" fill="red"/>
+  <image width="100%" height="100%"
+         xlink:href="limeInRed-noSVGDimensions-viewBox.svg"/>
+</svg>

layout/svg/base/src/nsSVGImageFrame.cpp

@@ -115,7 +115,10 @@ public:
 #endif
 
 private:
-  gfxMatrix GetImageTransform(PRInt32 aNativeWidth, PRInt32 aNativeHeight);
+  gfxMatrix GetRasterImageTransform(PRInt32 aNativeWidth,
+                                    PRInt32 aNativeHeight);
+  gfxMatrix GetVectorImageTransform();
+  PRBool    TransformContextForPainting(gfxContext* aGfxContext);
 
   nsCOMPtr<imgIDecoderObserver> mListener;
 
@@ -193,7 +196,7 @@ nsSVGImageFrame::AttributeChanged(PRInt32         aNameSpaceID,
 }
 
 gfxMatrix
-nsSVGImageFrame::GetImageTransform(PRInt32 aNativeWidth, PRInt32 aNativeHeight)
+nsSVGImageFrame::GetRasterImageTransform(PRInt32 aNativeWidth, PRInt32 aNativeHeight)
 {
   float x, y, width, height;
   nsSVGImageElement *element = static_cast<nsSVGImageElement*>(mContent);
@@ -208,6 +211,46 @@ nsSVGImageFrame::GetImageTransform(PRInt32 aNativeWidth, PRInt32 aNativeHeight)
   return viewBoxTM * gfxMatrix().Translate(gfxPoint(x, y)) * GetCanvasTM();
 }
 
+gfxMatrix
+nsSVGImageFrame::GetVectorImageTransform()
+{
+  float x, y, width, height;
+  nsSVGImageElement *element = static_cast<nsSVGImageElement*>(mContent);
+  element->GetAnimatedLengthValues(&x, &y, &width, &height, nsnull);
+
+  // No viewBoxTM needed here -- our height/width overrides any concept of
+  // "native size" that the SVG image has, and it will handle viewBox and
+  // preserveAspectRatio on its own once we give it a region to draw into.
+
+  return gfxMatrix().Translate(gfxPoint(x, y)) * GetCanvasTM();
+}
+
+PRBool
+nsSVGImageFrame::TransformContextForPainting(gfxContext* aGfxContext)
+{
+  gfxMatrix imageTransform;
+  if (mImageContainer->GetType() == imgIContainer::TYPE_VECTOR) {
+    imageTransform = GetVectorImageTransform();
+  } else {
+    PRInt32 nativeWidth, nativeHeight;
+    if (NS_FAILED(mImageContainer->GetWidth(&nativeWidth)) ||
+        NS_FAILED(mImageContainer->GetHeight(&nativeHeight)) ||
+        nativeWidth == 0 || nativeHeight == 0) {
+      return PR_FALSE;
+    }
+    imageTransform = GetRasterImageTransform(nativeWidth, nativeHeight);
+  }
+
+  // NOTE: We need to cancel out the effects of Full-Page-Zoom, or else
+  // it'll get applied an extra time by DrawSingleUnscaledImage.
+  nscoord appUnitsPerDevPx = PresContext()->AppUnitsPerDevPixel();
+  gfxFloat pageZoomFactor =
+    nsPresContext::AppUnitsToFloatCSSPixels(appUnitsPerDevPx);
+  aGfxContext->Multiply(imageTransform.Scale(pageZoomFactor, pageZoomFactor));
+
+  return PR_TRUE;
+}
+
 //----------------------------------------------------------------------
 // nsISVGChildFrame methods:
 NS_IMETHODIMP
@@ -237,19 +280,6 @@ nsSVGImageFrame::PaintSVG(nsSVGRenderState *aContext,
   }
 
   if (mImageContainer) {
-    PRInt32 nativeWidth, nativeHeight;
-    mImageContainer->GetWidth(&nativeWidth);
-    mImageContainer->GetHeight(&nativeHeight);
-
-    if (nativeWidth == 0 || nativeHeight == 0) {
-      return NS_ERROR_FAILURE;
-    }
-
-    if (mImageContainer->GetType() == imgIContainer::TYPE_VECTOR) {
-      // <svg:image> not supported for SVG images yet.
-      return NS_ERROR_FAILURE;
-    }
-
     gfxContext* ctx = aContext->GetGfxContext();
     gfxContextAutoSaveRestore autoRestorer(ctx);
 
@@ -259,14 +289,9 @@ nsSVGImageFrame::PaintSVG(nsSVGRenderState *aContext,
       nsSVGUtils::SetClipRect(ctx, GetCanvasTM(), clipRect);
     }
 
-    nscoord appUnitsPerDevPx = PresContext()->AppUnitsPerDevPixel();
-    gfxFloat pageZoomFactor =
-      nsPresContext::AppUnitsToFloatCSSPixels(appUnitsPerDevPx);
-
-    // NOTE: We need to cancel out the effects of Full-Page-Zoom, or else
-    // it'll get applied an extra time by DrawSingleUnscaledImage.
-    ctx->Multiply(GetImageTransform(nativeWidth, nativeHeight).
-      Scale(pageZoomFactor, pageZoomFactor));
+    if (!TransformContextForPainting(ctx)) {
+      return NS_ERROR_FAILURE;
+    }
 
     // fill-opacity doesn't affect <image>, so if we're allowed to
     // optimize group opacity, the opacity used for compositing the
@@ -280,6 +305,7 @@ nsSVGImageFrame::PaintSVG(nsSVGRenderState *aContext,
       ctx->PushGroup(gfxASurface::CONTENT_COLOR_ALPHA);
     }
 
+    nscoord appUnitsPerDevPx = PresContext()->AppUnitsPerDevPixel();
     nsRect dirtyRect; // only used if aDirtyRect is non-null
     if (aDirtyRect) {
       dirtyRect = aDirtyRect->ToAppUnits(appUnitsPerDevPx);
@@ -291,13 +317,32 @@ nsSVGImageFrame::PaintSVG(nsSVGRenderState *aContext,
     // warrant worrying about the responsiveness impact of doing synchronous
     // decodes. The extra code complexity of determinining when we want to
     // force sync probably just isn't worth it, so always pass FLAG_SYNC_DECODE
-    nsLayoutUtils::DrawSingleUnscaledImage(
-      aContext->GetRenderingContext(this),
-      mImageContainer,
-      nsLayoutUtils::GetGraphicsFilterForFrame(this),
-      nsPoint(0, 0),
-      aDirtyRect ? &dirtyRect : nsnull,
-      imgIContainer::FLAG_SYNC_DECODE);
+    PRUint32 drawFlags = imgIContainer::FLAG_SYNC_DECODE;
+
+    if (mImageContainer->GetType() == imgIContainer::TYPE_VECTOR) {
+      nsRect destRect(0, 0,
+                      appUnitsPerDevPx * width,
+                      appUnitsPerDevPx * height);
+
+      // Note: Can't use DrawSingleUnscaledImage for the TYPE_VECTOR case.
+      // That method needs our image to have a fixed native width & height,
+      // and that's not always true for TYPE_VECTOR images.
+      nsLayoutUtils::DrawSingleImage(
+        aContext->GetRenderingContext(this),
+        mImageContainer,
+        nsLayoutUtils::GetGraphicsFilterForFrame(this),
+        destRect,
+        aDirtyRect ? dirtyRect : destRect,
+        drawFlags);
+    } else { // mImageContainer->GetType() == TYPE_RASTER
+      nsLayoutUtils::DrawSingleUnscaledImage(
+        aContext->GetRenderingContext(this),
+        mImageContainer,
+        nsLayoutUtils::GetGraphicsFilterForFrame(this),
+        nsPoint(0, 0),
+        aDirtyRect ? &dirtyRect : nsnull,
+        drawFlags);
+    }
 
     if (opacity != 1.0f) {
       ctx->PopGroupToSource();
@@ -313,21 +358,32 @@ nsSVGImageFrame::PaintSVG(nsSVGRenderState *aContext,
 NS_IMETHODIMP_(nsIFrame*)
 nsSVGImageFrame::GetFrameForPoint(const nsPoint &aPoint)
 {
+  // Special case for raster images -- we only want to accept points that fall
+  // in the underlying image's (transformed) native bounds.  That region
+  // doesn't necessarily map to our <image> element's [x,y,width,height].  So,
+  // we have to look up the native image size & our image transform in order
+  // to filter out points that fall outside that area.
   if (GetStyleDisplay()->IsScrollableOverflow() && mImageContainer) {
-    PRInt32 nativeWidth, nativeHeight;
-    mImageContainer->GetWidth(&nativeWidth);
-    mImageContainer->GetHeight(&nativeHeight);
+    if (mImageContainer->GetType() == imgIContainer::TYPE_RASTER) {
+      PRInt32 nativeWidth, nativeHeight;
+      if (NS_FAILED(mImageContainer->GetWidth(&nativeWidth)) ||
+          NS_FAILED(mImageContainer->GetHeight(&nativeHeight)) ||
+          nativeWidth == 0 || nativeHeight == 0) {
+        return nsnull;
+      }
 
-    if (nativeWidth == 0 || nativeHeight == 0) {
-      return nsnull;
-    }
-
-    if (!nsSVGUtils::HitTestRect(GetImageTransform(nativeWidth, nativeHeight),
-                                 0, 0, nativeWidth, nativeHeight,
-                                 PresContext()->AppUnitsToDevPixels(aPoint.x),
-                                 PresContext()->AppUnitsToDevPixels(aPoint.y))) {
-      return nsnull;
+      if (!nsSVGUtils::HitTestRect(
+               GetRasterImageTransform(nativeWidth, nativeHeight),
+               0, 0, nativeWidth, nativeHeight,
+               PresContext()->AppUnitsToDevPixels(aPoint.x),
+               PresContext()->AppUnitsToDevPixels(aPoint.y))) {
+        return nsnull;
+      }
     }
+    // The special case above doesn't apply to vector images, because they
+    // don't limit their drawing to explicit "native bounds" -- they have
+    // an infinite canvas on which to place content.  So it's reasonable to
+    // just fall back on our <image> element's own bounds here.
   }
 
   return nsSVGPathGeometryFrame::GetFrameForPoint(aPoint);

layout/svg/base/src/nsSVGUtils.cpp

@@ -94,8 +94,6 @@
 using namespace mozilla;
 using namespace mozilla::dom;
 
-gfxASurface *nsSVGUtils::gThebesComputationalSurface = nsnull;
-
 // c = n / 255
 // (c <= 0.0031308 ? c * 12.92 : 1.055 * pow(c, 1 / 2.4) - 0.055) * 255 + 0.5
 static const PRUint8 glinearRGBTosRGBMap[256] = {
@@ -796,7 +794,9 @@ nsSVGUtils::GetViewBoxTransform(nsSVGElement* aElement,
                                 float aViewboxWidth, float aViewboxHeight,
                                 const SVGPreserveAspectRatio &aPreserveAspectRatio)
 {
-  NS_ASSERTION(aViewboxWidth > 0, "viewBox width must be greater than zero!");
+  NS_ASSERTION(aViewportWidth  >= 0, "viewport width must be nonnegative!");
+  NS_ASSERTION(aViewportHeight >= 0, "viewport height must be nonnegative!");
+  NS_ASSERTION(aViewboxWidth  > 0, "viewBox width must be greater than zero!");
   NS_ASSERTION(aViewboxHeight > 0, "viewBox height must be greater than zero!");
 
   PRUint16 align = aPreserveAspectRatio.GetAlign();

layout/svg/base/src/nsSVGUtils.h

@@ -611,10 +611,6 @@ public:
    * builds, it will trigger a PR_FALSE return-value as a safe fallback.)
    */
   static PRBool RootSVGElementHasViewbox(const nsIContent *aRootSVGElem);
-
-private:
-  /* Computational (nil) surfaces */
-  static gfxASurface *gThebesComputationalSurface;
 };
 
 #endif