Backed out changeset 588c44c7a966 (bug 1058040)
authorIris Hsiao <ihsiao@mozilla.com>
Thu, 02 Mar 2017 11:24:33 +0800
changeset 394531 8c7dbec36fa77c5bb427c480f3620dd4a781260f
parent 394530 296647884376857ff7cce38795606184fb806fd0
child 394532 66535e831760421b270662aa8d0773b0fde7c9f3
child 394561 5e3c5f0f53568d7aefdc67e21d69ed7d11b141ce
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1058040
milestone54.0a1
backs out588c44c7a96616073364b7b10324f57604ef72f9
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
Backed out changeset 588c44c7a966 (bug 1058040) CLOSED TREE
dom/canvas/CanvasRenderingContext2D.cpp
image/ClippedImage.cpp
image/OrientedImage.cpp
image/VectorImage.cpp
layout/base/nsLayoutUtils.cpp
layout/svg/SVGImageContext.h
layout/svg/nsSVGImageFrame.cpp
--- a/dom/canvas/CanvasRenderingContext2D.cpp
+++ b/dom/canvas/CanvasRenderingContext2D.cpp
@@ -5101,17 +5101,17 @@ CanvasRenderingContext2D::DrawDirectlyTo
                        Scale(1.0 / contextScale.width,
                              1.0 / contextScale.height).
                        Translate(aDest.x - aSrc.x, aDest.y - aSrc.y));
 
   // FLAG_CLAMP is added for increased performance, since we never tile here.
   uint32_t modifiedFlags = aImage.mDrawingFlags | imgIContainer::FLAG_CLAMP;
 
   CSSIntSize sz(scaledImageSize.width, scaledImageSize.height); // XXX hmm is scaledImageSize really in CSS pixels?
-  SVGImageContext svgContext(Some(sz), Nothing(), CurrentState().globalAlpha);
+  SVGImageContext svgContext(sz, Nothing(), CurrentState().globalAlpha);
 
   auto result = aImage.mImgContainer->
     Draw(context, scaledImageSize,
          ImageRegion::Create(gfxRect(aSrc.x, aSrc.y, aSrc.width, aSrc.height)),
          aImage.mWhichFrame, SamplingFilter::GOOD, Some(svgContext), modifiedFlags, 1.0);
 
   if (result != DrawResult::SUCCESS) {
     NS_WARNING("imgIContainer::Draw failed");
--- a/image/ClippedImage.cpp
+++ b/image/ClippedImage.cpp
@@ -462,26 +462,23 @@ ClippedImage::DrawSingleTile(gfxContext*
   aContext->Multiply(gfxMatrix::Translation(-clip.x, -clip.y));
 
   auto unclipViewport = [&](const SVGImageContext& aOldContext) {
     // Map the viewport to the inner image. Note that we don't take the aSize
     // parameter of imgIContainer::Draw into account, just the clipping region.
     // 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);
-    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));
-    }
+    context.SetViewportSize(vSize);
     return context;
   };
 
   return InnerImage()->Draw(aContext, size, region,
                             aWhichFrame, aSamplingFilter,
                             aSVGContext.map(unclipViewport),
                             aFlags, aOpacity);
 }
--- a/image/OrientedImage.cpp
+++ b/image/OrientedImage.cpp
@@ -288,23 +288,22 @@ OrientedImage::Draw(gfxContext* aContext
   // The region is already in the orientation expected by the caller, but we
   // need it to be in the image's coordinate system, so we transform it using
   // the inverse of the orientation matrix.
   gfxMatrix inverseMatrix(OrientationMatrix(size, /* aInvert = */ true));
   ImageRegion region(aRegion);
   region.TransformBoundsBy(inverseMatrix);
 
   auto orientViewport = [&](const SVGImageContext& aOldContext) {
+    CSSIntSize viewportSize(aOldContext.GetViewportSize());
+    if (mOrientation.SwapsWidthAndHeight()) {
+      swap(viewportSize.width, viewportSize.height);
+    }
     SVGImageContext context(aOldContext);
-    auto oldViewport = aOldContext.GetViewportSize();
-    if (oldViewport && mOrientation.SwapsWidthAndHeight()) {
-      // Swap width and height:
-      CSSIntSize newViewport(oldViewport->height, oldViewport->width);
-      context.SetViewportSize(Some(newViewport));
-    }
+    context.SetViewportSize(viewportSize);
     return context;
   };
 
   return InnerImage()->Draw(aContext, size, region, aWhichFrame,
                             aSamplingFilter,
                             aSVGContext.map(orientViewport), aFlags,
                             aOpacity);
 }
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -784,20 +784,18 @@ struct SVGDrawingParameters
     , samplingFilter(aSamplingFilter)
     , svgContext(aSVGContext)
     , viewportSize(aSize)
     , animationTime(aAnimationTime)
     , flags(aFlags)
     , opacity(aSVGContext ? aSVGContext->GetGlobalOpacity() : aOpacity)
   {
     if (aSVGContext) {
-      auto sz = aSVGContext->GetViewportSize();
-      if (sz) {
-        viewportSize = nsIntSize(sz->width, sz->height); // XXX losing unit
-      }
+      CSSIntSize sz = aSVGContext->GetViewportSize();
+      viewportSize = nsIntSize(sz.width, sz.height); // XXX losing unit
     }
   }
 
   gfxContext*                   context;
   IntSize                       size;
   ImageRegion                   region;
   SamplingFilter                samplingFilter;
   const Maybe<SVGImageContext>& svgContext;
@@ -851,17 +849,17 @@ VectorImage::Draw(gfxContext* aContext,
   // overwrite SVG preserveAspectRatio attibute of this image with none, and
   // always stretch this image to viewport non-uniformly.
   // And we can do this only if the caller pass in the the SVG viewport, via
   // aSVGContext.
   if ((aFlags & FLAG_FORCE_PRESERVEASPECTRATIO_NONE) && aSVGContext.isSome()) {
     Maybe<SVGPreserveAspectRatio> aspectRatio =
       Some(SVGPreserveAspectRatio(SVG_PRESERVEASPECTRATIO_NONE,
                                   SVG_MEETORSLICE_UNKNOWN));
-    svgContext = aSVGContext; // copy
+    svgContext = Some(SVGImageContext(*aSVGContext)); // copy
     svgContext->SetPreserveAspectRatio(aspectRatio);
   } else {
     svgContext = aSVGContext;
   }
 
   float animTime =
     (aWhichFrame == FRAME_FIRST) ? 0.0f
                                  : mSVGDocumentWrapper->GetCurrentTime();
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -6535,17 +6535,17 @@ DrawImageInternal(gfxContext&           
 
     RefPtr<gfxContext> destCtx = &aContext;
 
     destCtx->SetMatrix(params.imageSpaceToDeviceSpace);
 
     Maybe<const SVGImageContext> fallbackContext;
     if (!aSVGContext) {
       // Use the default viewport.
-      fallbackContext.emplace(Some(params.svgViewportSize));
+      fallbackContext.emplace(params.svgViewportSize);
     }
 
     result = aImage->Draw(destCtx, params.size, params.region,
                           imgIContainer::FRAME_CURRENT, aSamplingFilter,
                           aSVGContext ? aSVGContext : fallbackContext,
                           aImageFlags, aOpacity);
 
   }
@@ -6720,17 +6720,17 @@ nsLayoutUtils::DrawBackgroundImage(gfxCo
                                    const nsRect&       aDirty,
                                    uint32_t            aImageFlags,
                                    ExtendMode          aExtendMode,
                                    float               aOpacity)
 {
   PROFILER_LABEL("layout", "nsLayoutUtils::DrawBackgroundImage",
                  js::ProfileEntry::Category::GRAPHICS);
 
-  const Maybe<SVGImageContext> svgContext(Some(SVGImageContext(Some(aImageSize))));
+  const Maybe<const SVGImageContext> svgContext(Some(SVGImageContext(aImageSize)));
 
   /* Fast path when there is no need for image spacing */
   if (aRepeatSize.width == aDest.width && aRepeatSize.height == aDest.height) {
     return DrawImageInternal(aContext, aPresContext, aImage,
                              aSamplingFilter, aDest, aFill, aAnchor,
                              aDirty, svgContext, aImageFlags, aExtendMode,
                              aOpacity);
   }
--- a/layout/svg/SVGImageContext.h
+++ b/layout/svg/SVGImageContext.h
@@ -22,52 +22,35 @@ namespace mozilla {
 // to the image's internal SVG document when it's drawn.
 class SVGImageContext
 {
 public:
   SVGImageContext()
     : mGlobalOpacity(1.0)
   { }
 
-  /**
-   * 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,
+  // 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,
                            const Maybe<SVGPreserveAspectRatio>& aPreserveAspectRatio = Nothing(),
                            gfxFloat aOpacity = 1.0,
                            bool aIsPaintingSVGImageElement = false)
     : mViewportSize(aViewportSize)
     , mPreserveAspectRatio(aPreserveAspectRatio)
     , mGlobalOpacity(aOpacity)
     , mIsPaintingSVGImageElement(aIsPaintingSVGImageElement)
   { }
 
   bool MaybeStoreContextPaint(nsIFrame* aFromFrame);
 
-  const Maybe<CSSIntSize>& GetViewportSize() const {
+  const CSSIntSize& GetViewportSize() const {
     return mViewportSize;
   }
 
-  void SetViewportSize(const Maybe<CSSIntSize>& aSize) {
+  void SetViewportSize(const CSSIntSize& aSize) {
     mViewportSize = aSize;
   }
 
   const Maybe<SVGPreserveAspectRatio>& GetPreserveAspectRatio() const {
     return mPreserveAspectRatio;
   }
 
   void SetPreserveAspectRatio(const Maybe<SVGPreserveAspectRatio>& aPAR) {
@@ -98,33 +81,31 @@ public:
   }
 
   uint32_t Hash() const {
     uint32_t hash = 0;
     if (mContextPaint) {
       hash = HashGeneric(hash, mContextPaint->Hash());
     }
     return HashGeneric(hash,
-                       mViewportSize.map(HashSize).valueOr(0),
+                       mViewportSize.width,
+                       mViewportSize.height,
                        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;
-  Maybe<CSSIntSize>             mViewportSize;
+  CSSIntSize                    mViewportSize;
   Maybe<SVGPreserveAspectRatio> mPreserveAspectRatio;
   gfxFloat                      mGlobalOpacity;
   bool                          mIsPaintingSVGImageElement;
 };
 
 } // namespace mozilla
 
 #endif // MOZILLA_SVGCONTEXT_H_
--- a/layout/svg/nsSVGImageFrame.cpp
+++ b/layout/svg/nsSVGImageFrame.cpp
@@ -394,17 +394,17 @@ nsSVGImageFrame::PaintSVG(gfxContext& aC
     if (mImageContainer->GetType() == imgIContainer::TYPE_VECTOR) {
       // Package up the attributes of this image element which can override the
       // attributes of mImageContainer's internal SVG document.  The 'width' &
       // 'height' values we're passing in here are in CSS units (though they
       // come from width/height *attributes* in SVG). They influence the region
       // of the SVG image's internal document that is visible, in combination
       // with preserveAspectRatio and viewBox.
       const Maybe<const SVGImageContext> context(
-        Some(SVGImageContext(Some(CSSIntSize::Truncate(width, height)),
+        Some(SVGImageContext(CSSIntSize::Truncate(width, height),
                              Some(imgElem->mPreserveAspectRatio.GetAnimValue()),
                              1.0, /* aIsPaintingSVGImageElement */ true)));
 
       // For the actual draw operation to draw crisply (and at the right size),
       // our destination rect needs to be |width|x|height|, *in dev pixels*.
       LayoutDeviceSize devPxSize(width, height);
       nsRect destRect(nsPoint(),
                       LayoutDevicePixel::ToAppUnits(devPxSize,