Bug 1358828, part 2 - Avoid setting state on SVG images if we use an image from the surface cache. r=longsonr
authorJonathan Watt <jwatt@jwatt.org>
Mon, 27 Mar 2017 12:49:44 +0100
changeset 354455 97af1ab68078271b2817d5686fbd6f344b47fc14
parent 354454 a06749566178c2018473e7b863e48e0eee31be96
child 354456 8375b7a4be084a1be30a90c7a6df089f8b1682e0
push id89470
push userjwatt@jwatt.org
push dateSun, 23 Apr 2017 22:05:53 +0000
treeherdermozilla-inbound@97af1ab68078 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr
bugs1358828
milestone55.0a1
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 1358828, part 2 - Avoid setting state on SVG images if we use an image from the surface cache. r=longsonr MozReview-Commit-ID: zvdStzP5Zx
image/VectorImage.cpp
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -836,62 +836,70 @@ VectorImage::Draw(gfxContext* aContext,
   if (mError) {
     return DrawResult::BAD_IMAGE;
   }
 
   if (!mIsFullyLoaded) {
     return DrawResult::NOT_READY;
   }
 
-  if (mIsDrawing) {
-    NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw");
-    return DrawResult::TEMPORARY_ERROR;
-  }
-
   if (mAnimationConsumers == 0) {
     SendOnUnlockedDraw(aFlags);
   }
 
-  AutoRestore<bool> autoRestoreIsDrawing(mIsDrawing);
-  mIsDrawing = true;
+  MOZ_ASSERT(!(aFlags & FLAG_FORCE_PRESERVEASPECTRATIO_NONE) ||
+             (aSVGContext && aSVGContext->GetViewportSize()),
+             "Viewport size is required when using "
+             "FLAG_FORCE_PRESERVEASPECTRATIO_NONE");
 
-  // If FLAG_FORCE_PRESERVEASPECTRATIO_NONE bit is set, that means we should
-  // 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.
   Maybe<SVGImageContext> newSVGContext;
   if ((aFlags & FLAG_FORCE_PRESERVEASPECTRATIO_NONE) && aSVGContext) {
+    // Create an SVGImageContext with the appropriate 'preserveAspectRatio'
+    // value so that LookupCachedSurface() below uses the appropriate key:
+    MOZ_ASSERT(!aSVGContext->GetPreserveAspectRatio(),
+               "FLAG_FORCE_PRESERVEASPECTRATIO_NONE is not expected if a "
+               "preserveAspectRatio override is supplied");
     Maybe<SVGPreserveAspectRatio> aspectRatio =
       Some(SVGPreserveAspectRatio(SVG_PRESERVEASPECTRATIO_NONE,
                                   SVG_MEETORSLICE_UNKNOWN));
     newSVGContext = aSVGContext; // copy
     newSVGContext->SetPreserveAspectRatio(aspectRatio);
   }
 
-  float animTime =
-    (aWhichFrame == FRAME_FIRST) ? 0.0f
-                                 : mSVGDocumentWrapper->GetCurrentTime();
-  AutoSVGRenderingState autoSVGState(newSVGContext ? newSVGContext : aSVGContext,
-                                     animTime,
-                                     mSVGDocumentWrapper->GetRootSVGElem());
-
+  float animTime = (aWhichFrame == FRAME_FIRST)
+                     ? 0.0f : mSVGDocumentWrapper->GetCurrentTime();
 
   SVGDrawingParameters params(aContext, aSize, aRegion, aSamplingFilter,
                               newSVGContext ? newSVGContext : aSVGContext,
                               animTime, aFlags, aOpacity);
 
   // If we have an prerasterized version of this image that matches the
   // drawing parameters, use that.
   RefPtr<gfxDrawable> svgDrawable = LookupCachedSurface(params);
   if (svgDrawable) {
     Show(svgDrawable, params);
     return DrawResult::SUCCESS;
   }
 
+  // else, we need to paint the image:
+
+  if (mIsDrawing) {
+    NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw");
+    return DrawResult::TEMPORARY_ERROR;
+  }
+  AutoRestore<bool> autoRestoreIsDrawing(mIsDrawing);
+  mIsDrawing = true;
+
+  // Apply any 'preserveAspectRatio' override (if specified) to the root
+  // element, and set the animation time:
+  AutoSVGRenderingState autoSVGState(newSVGContext ? newSVGContext : aSVGContext,
+                                     animTime,
+                                     mSVGDocumentWrapper->GetRootSVGElem());
+
+  // Set context paint (if specified) on the document:
   Maybe<AutoSetRestoreSVGContextPaint> autoContextPaint;
   if (aSVGContext &&
       aSVGContext->GetContextPaint()) {
     autoContextPaint.emplace(aSVGContext->GetContextPaint(),
                              mSVGDocumentWrapper->GetDocument());
   }
 
   // We didn't get a hit in the surface cache, so we'll need to rerasterize.