Bug 1058040, part 12 - Make SVGImageContext's ctor's aViewportSize parameter optional. r=dholbert

dom/canvas/CanvasRenderingContext2D.cpp

@@ -5106,7 +5106,7 @@ CanvasRenderingContext2D::DrawDirectlyToCanvas(
   uint32_t modifiedFlags = aImage.mDrawingFlags | imgIContainer::FLAG_CLAMP;
 
   CSSIntSize sz(scaledImageSize.width, scaledImageSize.height); // XXX hmm is scaledImageSize really in CSS pixels?
-  SVGImageContext svgContext(sz, Nothing(), CurrentState().globalAlpha);
+  SVGImageContext svgContext(Some(sz), Nothing(), CurrentState().globalAlpha);
 
   auto result = aImage.mImgContainer->
     Draw(context, scaledImageSize,

image/ClippedImage.cpp

@@ -467,13 +467,16 @@ ClippedImage::DrawSingleTile(gfxContext* aContext,
     // The size in pixels at which the output will ultimately be drawn is
     // irrelevant here since the purpose of the SVG viewport size is to
     // determine what *region* of the SVG document will be drawn.
-    CSSIntSize vSize(aOldContext.GetViewportSize());
-    vSize.width = ceil(vSize.width * double(innerSize.width) / mClip.width);
-    vSize.height =
-      ceil(vSize.height * double(innerSize.height) / mClip.height);
-
     SVGImageContext context(aOldContext);
-    context.SetViewportSize(vSize);
+    auto oldViewport = aOldContext.GetViewportSize();
+    if (oldViewport) {
+      CSSIntSize newViewport;
+      newViewport.width =
+        ceil(oldViewport->width * double(innerSize.width) / mClip.width);
+      newViewport.height =
+        ceil(oldViewport->height * double(innerSize.height) / mClip.height);
+      context.SetViewportSize(Some(newViewport));
+    }
     return context;
   };
 

image/OrientedImage.cpp

@@ -293,12 +293,13 @@ OrientedImage::Draw(gfxContext* aContext,
   region.TransformBoundsBy(inverseMatrix);
 
   auto orientViewport = [&](const SVGImageContext& aOldContext) {
-    CSSIntSize viewportSize(aOldContext.GetViewportSize());
-    if (mOrientation.SwapsWidthAndHeight()) {
-      swap(viewportSize.width, viewportSize.height);
-    }
     SVGImageContext context(aOldContext);
-    context.SetViewportSize(viewportSize);
+    auto oldViewport = aOldContext.GetViewportSize();
+    if (oldViewport && mOrientation.SwapsWidthAndHeight()) {
+      // Swap width and height:
+      CSSIntSize newViewport(oldViewport->height, oldViewport->width);
+      context.SetViewportSize(Some(newViewport));
+    }
     return context;
   };
 

image/VectorImage.cpp

@@ -789,8 +789,10 @@ struct SVGDrawingParameters
     , opacity(aSVGContext ? aSVGContext->GetGlobalOpacity() : aOpacity)
   {
     if (aSVGContext) {
-      CSSIntSize sz = aSVGContext->GetViewportSize();
-      viewportSize = nsIntSize(sz.width, sz.height); // XXX losing unit
+      auto sz = aSVGContext->GetViewportSize();
+      if (sz) {
+        viewportSize = nsIntSize(sz->width, sz->height); // XXX losing unit
+      }
     }
   }
 
@@ -854,7 +856,7 @@ VectorImage::Draw(gfxContext* aContext,
     Maybe<SVGPreserveAspectRatio> aspectRatio =
       Some(SVGPreserveAspectRatio(SVG_PRESERVEASPECTRATIO_NONE,
                                   SVG_MEETORSLICE_UNKNOWN));
-    svgContext = Some(SVGImageContext(*aSVGContext)); // copy
+    svgContext = aSVGContext; // copy
     svgContext->SetPreserveAspectRatio(aspectRatio);
   } else {
     svgContext = aSVGContext;

layout/base/nsLayoutUtils.cpp

@@ -6541,7 +6541,7 @@ DrawImageInternal(gfxContext&            aContext,
     Maybe<const SVGImageContext> fallbackContext;
     if (!aSVGContext) {
       // Use the default viewport.
-      fallbackContext.emplace(params.svgViewportSize);
+      fallbackContext.emplace(Some(params.svgViewportSize));
     }
 
     result = aImage->Draw(destCtx, params.size, params.region,
@@ -6726,7 +6726,7 @@ nsLayoutUtils::DrawBackgroundImage(gfxContext&         aContext,
   PROFILER_LABEL("layout", "nsLayoutUtils::DrawBackgroundImage",
                  js::ProfileEntry::Category::GRAPHICS);
 
-  const Maybe<const SVGImageContext> svgContext(Some(SVGImageContext(aImageSize)));
+  const Maybe<SVGImageContext> svgContext(Some(SVGImageContext(Some(aImageSize))));
 
   /* Fast path when there is no need for image spacing */
   if (aRepeatSize.width == aDest.width && aRepeatSize.height == aDest.height) {

layout/svg/SVGImageContext.h

@@ -27,9 +27,26 @@ public:
     : mGlobalOpacity(1.0)
   { }
 
-  // Note: 'aIsPaintingSVGImageElement' should be used to indicate whether
-  // the SVG image in question is being painted for an SVG <image> element.
-  explicit SVGImageContext(const CSSIntSize& aViewportSize,
+  /**
+   * Currently it seems that the aViewportSize parameter ends up being used
+   * for different things by different pieces of code, and probably in some
+   * cases being used incorrectly (specifically in the case of pixel snapping
+   * under the nsLayoutUtils::Draw*Image() methods).  An unfortunate result of
+   * the messy code is that aViewportSize is currently a Maybe<T> since it
+   * is difficult to create a utility function that consumers can use up
+   * front to get the "correct" viewport size (i.e. which for compatibility
+   * with the current code (bugs and all) would mean the size including all
+   * the snapping and resizing magic that happens in various places under the
+   * nsLayoutUtils::Draw*Image() methods on the way to DrawImageInternal
+   * creating |fallbackContext|).  Using Maybe<T> allows code to pass Nothing()
+   * in order to get the size that's created for |fallbackContext|.  At some
+   * point we need to clean this code up, make our abstractions clear, create
+   * that utility and stop using Maybe for this parameter.
+   *
+   * Note: 'aIsPaintingSVGImageElement' should be used to indicate whether
+   * the SVG image in question is being painted for an SVG <image> element.
+   */
+  explicit SVGImageContext(const Maybe<CSSIntSize>& aViewportSize,
                            const Maybe<SVGPreserveAspectRatio>& aPreserveAspectRatio = Nothing(),
                            gfxFloat aOpacity = 1.0,
                            bool aIsPaintingSVGImageElement = false)
@@ -41,11 +58,11 @@ public:
 
   bool MaybeStoreContextPaint(nsIFrame* aFromFrame);
 
-  const CSSIntSize& GetViewportSize() const {
+  const Maybe<CSSIntSize>& GetViewportSize() const {
     return mViewportSize;
   }
 
-  void SetViewportSize(const CSSIntSize& aSize) {
+  void SetViewportSize(const Maybe<CSSIntSize>& aSize) {
     mViewportSize = aSize;
   }
 
@@ -86,21 +103,23 @@ public:
       hash = HashGeneric(hash, mContextPaint->Hash());
     }
     return HashGeneric(hash,
-                       mViewportSize.width,
-                       mViewportSize.height,
+                       mViewportSize.map(HashSize).valueOr(0),
                        mPreserveAspectRatio.map(HashPAR).valueOr(0),
                        HashBytes(&mGlobalOpacity, sizeof(mGlobalOpacity)),
                        mIsPaintingSVGImageElement);
   }
 
 private:
+  static uint32_t HashSize(const CSSIntSize& aSize) {
+    return HashGeneric(aSize.width, aSize.height);
+  }
   static uint32_t HashPAR(const SVGPreserveAspectRatio& aPAR) {
     return aPAR.Hash();
   }
 
   // NOTE: When adding new member-vars, remember to update Hash() & operator==.
   RefPtr<SVGContextPaint>       mContextPaint;
-  CSSIntSize                    mViewportSize;
+  Maybe<CSSIntSize>             mViewportSize;
   Maybe<SVGPreserveAspectRatio> mPreserveAspectRatio;
   gfxFloat                      mGlobalOpacity;
   bool                          mIsPaintingSVGImageElement;

layout/svg/nsSVGImageFrame.cpp

@@ -399,7 +399,7 @@ nsSVGImageFrame::PaintSVG(gfxContext& aContext,
       // of the SVG image's internal document that is visible, in combination
       // with preserveAspectRatio and viewBox.
       const Maybe<const SVGImageContext> context(
-        Some(SVGImageContext(CSSIntSize::Truncate(width, height),
+        Some(SVGImageContext(Some(CSSIntSize::Truncate(width, height)),
                              Some(imgElem->mPreserveAspectRatio.GetAnimValue()),
                              1.0, /* aIsPaintingSVGImageElement */ true)));