Bug 272288 Patch 1: Fix nsSVGImageFrame to handle SVG images. r=roc a=roc
authorDaniel Holbert <dholbert@cs.stanford.edu>
Sun, 19 Dec 2010 16:45:29 -0800
changeset 59473 c347676c5cffa5e763bc9d9ea56e3d14c80500a9
parent 59472 be1f7ea06a5efbfb0165625c0759d25291e03c32
child 59474 16f87faaacb4107d7ac8b1089b3a6a476dc06197
push id17637
push userdholbert@mozilla.com
push dateMon, 20 Dec 2010 00:53:32 +0000
treeherdermozilla-central@150af817b65d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, roc
bugs272288
milestone2.0b9pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 272288 Patch 1: Fix nsSVGImageFrame to handle SVG images. r=roc a=roc
content/svg/content/src/crashtests/crashtests.list
content/svg/content/src/crashtests/zero-size-image.svg
layout/reftests/svg/as-image/reftest.list
layout/reftests/svg/as-image/svg-image-recursive-1-ref.svg
layout/reftests/svg/as-image/svg-image-recursive-1a.svg
layout/reftests/svg/as-image/svg-image-recursive-1b.svg
layout/reftests/svg/as-image/svg-image-recursive-2-ref.svg
layout/reftests/svg/as-image/svg-image-recursive-2a.svg
layout/reftests/svg/as-image/svg-image-recursive-2b.html
layout/reftests/svg/as-image/svg-image-simple-1.svg
layout/reftests/svg/as-image/svg-image-simple-2.svg
layout/reftests/svg/as-image/svg-image-simple-3.svg
layout/svg/base/src/nsSVGImageFrame.cpp
layout/svg/base/src/nsSVGUtils.cpp
layout/svg/base/src/nsSVGUtils.h
--- a/content/svg/content/src/crashtests/crashtests.list
+++ b/content/svg/content/src/crashtests/crashtests.list
@@ -50,8 +50,9 @@ load 535691-1.svg
 load 539167-1.svg
 load 573316-1.svg
 load 579356-1.svg
 load 579356-2.svg
 load 595608-1.svg
 load 601251-1.html
 load 601406-1.svg
 load 603145-1.svg
+load zero-size-image.svg
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/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>
--- a/layout/reftests/svg/as-image/reftest.list
+++ b/layout/reftests/svg/as-image/reftest.list
@@ -68,8 +68,18 @@ random-if(winWidget) == img-height-slice
 == img-height-slice-2.html  img-height-slice-2-ref.html
 
 random-if(winWidget) == img-width-meet-1.html   img-width-meet-1-ref.html
 == img-width-meet-2.html   img-width-meet-2-ref.html
 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
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
new file mode 100644
--- /dev/null
+++ b/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>
--- a/layout/svg/base/src/nsSVGImageFrame.cpp
+++ b/layout/svg/base/src/nsSVGImageFrame.cpp
@@ -110,17 +110,20 @@ public:
 #ifdef DEBUG
   NS_IMETHOD GetFrameName(nsAString& aResult) const
   {
     return MakeFrameName(NS_LITERAL_STRING("SVGImage"), aResult);
   }
 #endif
 
 private:
-  gfxMatrix GetImageTransform(PRInt32 aNativeWidth, PRInt32 aNativeHeight);
+  gfxMatrix GetRasterImageTransform(PRInt32 aNativeWidth,
+                                    PRInt32 aNativeHeight);
+  gfxMatrix GetVectorImageTransform();
+  PRBool    TransformContextForPainting(gfxContext* aGfxContext);
 
   nsCOMPtr<imgIDecoderObserver> mListener;
 
   nsCOMPtr<imgIContainer> mImageContainer;
 
   friend class nsSVGImageListener;
 };
 
@@ -188,31 +191,71 @@ nsSVGImageFrame::AttributeChanged(PRInt3
      return NS_OK;
    }
 
    return nsSVGPathGeometryFrame::AttributeChanged(aNameSpaceID,
                                                    aAttribute, aModType);
 }
 
 gfxMatrix
-nsSVGImageFrame::GetImageTransform(PRInt32 aNativeWidth, PRInt32 aNativeHeight)
+nsSVGImageFrame::GetRasterImageTransform(PRInt32 aNativeWidth, PRInt32 aNativeHeight)
 {
   float x, y, width, height;
   nsSVGImageElement *element = static_cast<nsSVGImageElement*>(mContent);
   element->GetAnimatedLengthValues(&x, &y, &width, &height, nsnull);
 
   gfxMatrix viewBoxTM =
     nsSVGUtils::GetViewBoxTransform(element,
                                     width, height,
                                     0, 0, aNativeWidth, aNativeHeight,
                                     element->mPreserveAspectRatio);
 
   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
 nsSVGImageFrame::PaintSVG(nsSVGRenderState *aContext,
                           const nsIntRect *aDirtyRect)
 {
   nsresult rv = NS_OK;
 
@@ -232,107 +275,120 @@ nsSVGImageFrame::PaintSVG(nsSVGRenderSta
       imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
                               getter_AddRefs(currentRequest));
 
     if (currentRequest)
       currentRequest->GetImage(getter_AddRefs(mImageContainer));
   }
 
   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);
 
     if (GetStyleDisplay()->IsScrollableOverflow()) {
       gfxRect clipRect = nsSVGUtils::GetClipRectForFrame(this, x, y,
                                                          width, height);
       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
     // image into the current canvas is just the group opacity.
     float opacity = 1.0f;
     if (nsSVGUtils::CanOptimizeOpacity(this)) {
       opacity = GetStyleDisplay()->mOpacity;
     }
 
     if (opacity != 1.0f) {
       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);
       // Adjust dirtyRect to match our local coordinate system.
       dirtyRect.MoveBy(-mRect.TopLeft());
     }
 
     // XXXbholley - I don't think huge images in SVGs are common enough to
     // 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();
       ctx->SetOperator(gfxContext::OPERATOR_OVER);
       ctx->Paint(opacity);
     }
     // gfxContextAutoSaveRestore goes out of scope & cleans up our gfxContext
   }
 
   return rv;
 }
 
 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(
+               GetRasterImageTransform(nativeWidth, nativeHeight),
+               0, 0, nativeWidth, nativeHeight,
+               PresContext()->AppUnitsToDevPixels(aPoint.x),
+               PresContext()->AppUnitsToDevPixels(aPoint.y))) {
+        return nsnull;
+      }
     }
-
-    if (!nsSVGUtils::HitTestRect(GetImageTransform(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);
 }
 
 nsIAtom *
 nsSVGImageFrame::GetType() const
 {
--- a/layout/svg/base/src/nsSVGUtils.cpp
+++ b/layout/svg/base/src/nsSVGUtils.cpp
@@ -89,18 +89,16 @@
 #include "nsSVGPathGeometryFrame.h"
 #include "prdtoa.h"
 #include "mozilla/dom/Element.h"
 #include "gfxUtils.h"
 
 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] = {
   0,  13,  22,  28,  34,  38,  42,  46,
  50,  53,  56,  59,  61,  64,  66,  69,
  71,  73,  75,  77,  79,  81,  83,  85,
  86,  88,  90,  92,  93,  95,  96,  98,
  99, 101, 102, 104, 105, 106, 108, 109,
@@ -791,17 +789,19 @@ nsSVGUtils::GetViewBoxTransform(nsSVGEle
 
 gfxMatrix
 nsSVGUtils::GetViewBoxTransform(nsSVGElement* aElement,
                                 float aViewportWidth, float aViewportHeight,
                                 float aViewboxX, float aViewboxY,
                                 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();
   PRUint16 meetOrSlice = aPreserveAspectRatio.GetMeetOrSlice();
 
   // default to the defaults
   if (align == nsIDOMSVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_UNKNOWN)
     align = nsIDOMSVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_XMIDYMID;
--- a/layout/svg/base/src/nsSVGUtils.h
+++ b/layout/svg/base/src/nsSVGUtils.h
@@ -606,15 +606,11 @@ public:
    * Given a nsIContent* that is actually an nsSVGSVGElement*, this method
    * checks whether it currently has a valid viewBox, and returns true if so.
    *
    * No other type of element should be passed to this method.
    * (In debug builds, anything non-<svg> will trigger an abort; in non-debug
    * 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