Bug 1366097 - Part 1. VectorImage::GetFrameAtSize should not create a DrawTarget if using cached surface. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 17 Nov 2017 14:08:51 -0500
changeset 436905 eb77f75f95d945e1a9f95d035f83337c4e10b0c6
parent 436902 1faa238d9faf94d3b763730351171b86c2e5bc9f
child 436906 12fc7f8b8930fac9b91e1d4232e404b7ac6db3d5
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewerstnikkel
bugs1366097
milestone59.0a1
Bug 1366097 - Part 1. VectorImage::GetFrameAtSize should not create a DrawTarget if using cached surface. r=tnikkel Creating a DrawTarget can be an expensive operation. This is especially true in this case because checking for a cached already decoded version of the VectorImage is expected to be fast. Currently VectorImage::Draw is the typical path to render these images, but in the future, getting the frames directly or indirectly (through an ImageContainer) will become more common.
image/VectorImage.cpp
image/VectorImage.h
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -746,33 +746,47 @@ VectorImage::GetFrameAtSize(const IntSiz
   if (aWhichFrame > FRAME_MAX_VALUE) {
     return nullptr;
   }
 
   if (mError || !mIsFullyLoaded) {
     return nullptr;
   }
 
+  RefPtr<SourceSurface> sourceSurface =
+    LookupCachedSurface(aSize, Nothing(), aFlags);
+  if (sourceSurface) {
+    return sourceSurface.forget();
+  }
+
+  if (mIsDrawing) {
+    NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw");
+    return nullptr;
+  }
+
   // Make our surface the size of what will ultimately be drawn to it.
   // (either the full image size, or the restricted region)
   RefPtr<DrawTarget> dt = gfxPlatform::GetPlatform()->
     CreateOffscreenContentDrawTarget(aSize, SurfaceFormat::B8G8R8A8);
   if (!dt || !dt->IsValid()) {
     NS_ERROR("Could not create a DrawTarget");
     return nullptr;
   }
 
   RefPtr<gfxContext> context = gfxContext::CreateOrNull(dt);
   MOZ_ASSERT(context); // already checked the draw target above
 
-  auto result = Draw(context, aSize, ImageRegion::Create(aSize),
-                     aWhichFrame, SamplingFilter::POINT, Nothing(), aFlags,
-                     1.0);
+  Maybe<SVGImageContext> svgContext;
+  SVGDrawingParameters params(context, aSize, ImageRegion::Create(aSize),
+                              SamplingFilter::POINT, svgContext,
+                              mSVGDocumentWrapper->GetCurrentTime(),
+                              aFlags, 1.0);
 
-  return result == DrawResult::SUCCESS ? dt->Snapshot() : nullptr;
+  DrawInternal(params, false);
+  return dt->Snapshot();
 }
 
 //******************************************************************************
 NS_IMETHODIMP_(bool)
 VectorImage::IsImageContainerAvailable(LayerManager* aManager, uint32_t aFlags)
 {
   return false;
 }
@@ -878,84 +892,99 @@ VectorImage::Draw(gfxContext* aContext,
                      ? 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) {
+  RefPtr<SourceSurface> sourceSurface =
+    LookupCachedSurface(aSize, params.svgContext, aFlags);
+
+  if (sourceSurface) {
+    RefPtr<gfxDrawable> svgDrawable =
+      new gfxSurfaceDrawable(sourceSurface, sourceSurface->GetSize());
     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;
   }
+
+  DrawInternal(params, haveContextPaint && !blockContextPaint);
+  return DrawResult::SUCCESS;
+}
+
+void
+VectorImage::DrawInternal(const SVGDrawingParameters& aParams,
+                          bool aContextPaint)
+{
+  MOZ_ASSERT(!mIsDrawing);
+
   AutoRestore<bool> autoRestoreIsDrawing(mIsDrawing);
   mIsDrawing = true;
 
   // Apply any 'preserveAspectRatio' override (if specified) to the root
   // element:
-  AutoPreserveAspectRatioOverride autoPAR(newSVGContext ? newSVGContext : aSVGContext,
+  AutoPreserveAspectRatioOverride autoPAR(aParams.svgContext,
                                           mSVGDocumentWrapper->GetRootSVGElem());
 
   // Set the animation time:
   AutoSVGTimeSetRestore autoSVGTime(mSVGDocumentWrapper->GetRootSVGElem(),
-                                    animTime);
+                                    aParams.animationTime);
 
   // Set context paint (if specified) on the document:
   Maybe<AutoSetRestoreSVGContextPaint> autoContextPaint;
-  if (haveContextPaint && !blockContextPaint) {
-    autoContextPaint.emplace(aSVGContext->GetContextPaint(),
+  if (aContextPaint) {
+    autoContextPaint.emplace(aParams.svgContext->GetContextPaint(),
                              mSVGDocumentWrapper->GetDocument());
   }
 
   // We didn't get a hit in the surface cache, so we'll need to rerasterize.
-  CreateSurfaceAndShow(params, aContext->GetDrawTarget()->GetBackendType());
-  return DrawResult::SUCCESS;
+  BackendType backend = aParams.context->GetDrawTarget()->GetBackendType();
+  CreateSurfaceAndShow(aParams, backend);
 }
 
-already_AddRefed<gfxDrawable>
-VectorImage::LookupCachedSurface(const SVGDrawingParameters& aParams)
+already_AddRefed<SourceSurface>
+VectorImage::LookupCachedSurface(const IntSize& aSize,
+                                 const Maybe<SVGImageContext>& aSVGContext,
+                                 uint32_t aFlags)
 {
   // If we're not allowed to use a cached surface, don't attempt a lookup.
-  if (aParams.flags & FLAG_BYPASS_SURFACE_CACHE) {
+  if (aFlags & FLAG_BYPASS_SURFACE_CACHE) {
     return nullptr;
   }
 
   // We don't do any caching if we have animation, so don't bother with a lookup
   // in this case either.
   if (mHaveAnimations) {
     return nullptr;
   }
 
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(this),
-                         VectorSurfaceKey(aParams.size, aParams.svgContext));
+                         VectorSurfaceKey(aSize, aSVGContext));
   if (!result) {
     return nullptr;  // No matching surface, or the OS freed the volatile buffer.
   }
 
   RefPtr<SourceSurface> sourceSurface = result.Surface()->GetSourceSurface();
   if (!sourceSurface) {
     // Something went wrong. (Probably a GPU driver crash or device reset.)
     // Attempt to recover.
     RecoverFromLossOfSurfaces();
     return nullptr;
   }
 
-  RefPtr<gfxDrawable> svgDrawable =
-    new gfxSurfaceDrawable(sourceSurface, result.Surface()->GetSize());
-  return svgDrawable.forget();
+  return sourceSurface.forget();
 }
 
 void
 VectorImage::CreateSurfaceAndShow(const SVGDrawingParameters& aParams, BackendType aBackend)
 {
   mSVGDocumentWrapper->UpdateViewportBounds(aParams.viewportSize);
   mSVGDocumentWrapper->FlushImageTransformInvalidation();
 
--- a/image/VectorImage.h
+++ b/image/VectorImage.h
@@ -76,20 +76,23 @@ protected:
   explicit VectorImage(ImageURL* aURI = nullptr);
   virtual ~VectorImage();
 
   virtual nsresult StartAnimation() override;
   virtual nsresult StopAnimation() override;
   virtual bool     ShouldAnimate() override;
 
 private:
-  /// Attempt to find a cached surface matching @aParams in the SurfaceCache.
-  already_AddRefed<gfxDrawable>
-    LookupCachedSurface(const SVGDrawingParameters& aParams);
+  /// Attempt to find a matching cached surface in the SurfaceCache.
+  already_AddRefed<SourceSurface>
+    LookupCachedSurface(const IntSize& aSize,
+                        const Maybe<SVGImageContext>& aSVGContext,
+                        uint32_t aFlags);
 
+  void DrawInternal(const SVGDrawingParameters& aParams, bool aContextPaint);
   void CreateSurfaceAndShow(const SVGDrawingParameters& aParams,
                             gfx::BackendType aBackend);
   void Show(gfxDrawable* aDrawable, const SVGDrawingParameters& aParams);
 
   nsresult Init(const char* aMimeType, uint32_t aFlags);
 
   /**
    * In catastrophic circumstances like a GPU driver crash, we may lose our