Bug 1420245 - When getting the current frame of a VectorImage, eliminate any unnecessary draws. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 01 Dec 2017 18:50:07 -0500
changeset 394585 3bfd43a13286f2a7b1e929f592ffd94f59c392ed
parent 394580 0a559782a53a0d27424d3ec0c7f8a942ead597d7
child 394586 ee0a6b975c1bd82e5dbb8d761c893c14164bcb8a
push id33011
push usernerli@mozilla.com
push dateSat, 02 Dec 2017 21:41:32 +0000
treeherdermozilla-central@de1f7a92e872 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1420245
milestone59.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 1420245 - When getting the current frame of a VectorImage, eliminate any unnecessary draws. r=tnikkel
image/VectorImage.cpp
image/VectorImage.h
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -323,16 +323,46 @@ SVGDrawingCallback::operator()(gfxContex
 
   presShell->RenderDocument(svgRect, renderDocFlags,
                             NS_RGBA(0, 0, 0, 0), // transparent
                             aContext);
 
   return true;
 }
 
+class MOZ_STACK_CLASS AutoRestoreSVGState final {
+public:
+  AutoRestoreSVGState(const SVGDrawingParameters& aParams,
+                      SVGDocumentWrapper* aSVGDocumentWrapper,
+                      bool& aIsDrawing,
+                      bool aContextPaint)
+    : mIsDrawing(aIsDrawing)
+    // Apply any 'preserveAspectRatio' override (if specified) to the root
+    // element:
+    , mPAR(aParams.svgContext, aSVGDocumentWrapper->GetRootSVGElem())
+    // Set the animation time:
+    , mTime(aSVGDocumentWrapper->GetRootSVGElem(), aParams.animationTime)
+  {
+    MOZ_ASSERT(!aIsDrawing);
+    aIsDrawing = true;
+
+    // Set context paint (if specified) on the document:
+    if (aContextPaint) {
+      mContextPaint.emplace(aParams.svgContext->GetContextPaint(),
+                            aSVGDocumentWrapper->GetDocument());
+    }
+  }
+
+private:
+  AutoRestore<bool> mIsDrawing;
+  AutoPreserveAspectRatioOverride mPAR;
+  AutoSVGTimeSetRestore mTime;
+  Maybe<AutoSetRestoreSVGContextPaint> mContextPaint;
+};
+
 // Implement VectorImage's nsISupports-inherited methods
 NS_IMPL_ISUPPORTS(VectorImage,
                   imgIContainer,
                   nsIStreamListener,
                   nsIRequestObserver)
 
 //------------------------------------------------------------------------------
 // Constructor / Destructor
@@ -736,16 +766,20 @@ VectorImage::GetFrame(uint32_t aWhichFra
   return GetFrameAtSize(imageIntSize, aWhichFrame, aFlags);
 }
 
 NS_IMETHODIMP_(already_AddRefed<SourceSurface>)
 VectorImage::GetFrameAtSize(const IntSize& aSize,
                             uint32_t aWhichFrame,
                             uint32_t aFlags)
 {
+#ifdef DEBUG
+  NotifyDrawingObservers();
+#endif
+
   auto result = GetFrameInternal(aSize, Nothing(), aWhichFrame, aFlags);
   RefPtr<SourceSurface> surf = Get<2>(result).forget();
 
   // If we are here, it suggests the image is embedded in a canvas or some
   // other path besides layers, and we won't need the file handle.
   MarkSurfaceShared(surf);
   return surf.forget();
 }
@@ -780,45 +814,42 @@ VectorImage::GetFrameInternal(const IntS
   }
 
   if (mIsDrawing) {
     NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw");
     return MakeTuple(DrawResult::TEMPORARY_ERROR, aSize,
                      RefPtr<SourceSurface>());
   }
 
-  // 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");
+  // By using a null gfxContext, we ensure that we will always attempt to
+  // create a surface, even if we aren't capable of caching it (e.g. due to our
+  // flags, having an animation, etc). Otherwise CreateSurface will assume that
+  // the caller is capable of drawing directly to its own draw target if we
+  // cannot cache.
+  SVGDrawingParameters params(nullptr, aSize, ImageRegion::Create(aSize),
+                              SamplingFilter::POINT, aSVGContext,
+                              mSVGDocumentWrapper->GetCurrentTime(),
+                              aFlags, 1.0);
+
+  bool didCache; // Was the surface put into the cache?
+  bool contextPaint = aSVGContext && aSVGContext->GetContextPaint();
+
+  AutoRestoreSVGState autoRestore(params, mSVGDocumentWrapper,
+                                  mIsDrawing, contextPaint);
+
+  RefPtr<gfxDrawable> svgDrawable = CreateSVGDrawable(params);
+  RefPtr<SourceSurface> surface =
+    CreateSurface(params, svgDrawable, didCache);
+  if (!surface) {
+    MOZ_ASSERT(!didCache);
     return MakeTuple(DrawResult::TEMPORARY_ERROR, aSize,
                      RefPtr<SourceSurface>());
   }
 
-  RefPtr<gfxContext> context = gfxContext::CreateOrNull(dt);
-  MOZ_ASSERT(context); // already checked the draw target above
-
-  SVGDrawingParameters params(context, aSize, ImageRegion::Create(aSize),
-                              SamplingFilter::POINT, aSVGContext,
-                              mSVGDocumentWrapper->GetCurrentTime(),
-                              aFlags, 1.0);
-
-  // DrawInternal may return a surface which is stored in the cache. It is
-  // important to prefer this result over the snapshot because it may be a
-  // different surface type (e.g. SourceSurfaceSharedData for WebRender). If
-  // we did not put anything in the cache, we will need to fallback to the
-  // snapshot surface.
-  bool contextPaint = aSVGContext && aSVGContext->GetContextPaint();
-  RefPtr<SourceSurface> surface = DrawInternal(params, contextPaint);
-  if (!surface) {
-    surface = dt->Snapshot();
-  }
-
+  SendFrameComplete(didCache, params.flags);
   return MakeTuple(DrawResult::SUCCESS, aSize, Move(surface));
 }
 
 //******************************************************************************
 IntSize
 VectorImage::GetImageContainerSize(LayerManager* aManager,
                                    const IntSize& aSize,
                                    uint32_t aFlags)
@@ -990,52 +1021,51 @@ VectorImage::Draw(gfxContext* aContext,
 
   // else, we need to paint the image:
 
   if (mIsDrawing) {
     NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw");
     return DrawResult::TEMPORARY_ERROR;
   }
 
-  RefPtr<SourceSurface> surface = DrawInternal(params, contextPaint);
+  AutoRestoreSVGState autoRestore(params, mSVGDocumentWrapper,
+                                  mIsDrawing, contextPaint);
+
+  bool didCache; // Was the surface put into the cache?
+  RefPtr<gfxDrawable> svgDrawable = CreateSVGDrawable(params);
+  sourceSurface = CreateSurface(params, svgDrawable, didCache);
+  if (!sourceSurface) {
+    MOZ_ASSERT(!didCache);
+    Show(svgDrawable, params);
+    return DrawResult::SUCCESS;
+  }
+
+  RefPtr<gfxDrawable> drawable =
+    new gfxSurfaceDrawable(sourceSurface, params.size);
+  Show(drawable, params);
+  SendFrameComplete(didCache, params.flags);
 
   // Image got put into a painted layer, it will not be shared with another
   // process.
-  MarkSurfaceShared(surface);
+  MarkSurfaceShared(sourceSurface);
   return DrawResult::SUCCESS;
 }
 
-already_AddRefed<SourceSurface>
-VectorImage::DrawInternal(const SVGDrawingParameters& aParams,
-                          bool aContextPaint)
+already_AddRefed<gfxDrawable>
+VectorImage::CreateSVGDrawable(const SVGDrawingParameters& aParams)
 {
-  MOZ_ASSERT(!mIsDrawing);
-
-  AutoRestore<bool> autoRestoreIsDrawing(mIsDrawing);
-  mIsDrawing = true;
-
-  // Apply any 'preserveAspectRatio' override (if specified) to the root
-  // element:
-  AutoPreserveAspectRatioOverride autoPAR(aParams.svgContext,
-                                          mSVGDocumentWrapper->GetRootSVGElem());
+  RefPtr<gfxDrawingCallback> cb =
+    new SVGDrawingCallback(mSVGDocumentWrapper,
+                           aParams.viewportSize,
+                           aParams.size,
+                           aParams.flags);
 
-  // Set the animation time:
-  AutoSVGTimeSetRestore autoSVGTime(mSVGDocumentWrapper->GetRootSVGElem(),
-                                    aParams.animationTime);
-
-  // Set context paint (if specified) on the document:
-  Maybe<AutoSetRestoreSVGContextPaint> autoContextPaint;
-  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.
-  BackendType backend = aParams.context->GetDrawTarget()->GetBackendType();
-  return CreateSurfaceAndShow(aParams, backend);
+  RefPtr<gfxDrawable> svgDrawable =
+    new gfxCallbackDrawable(cb, aParams.size);
+  return svgDrawable.forget();
 }
 
 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.
@@ -1065,104 +1095,121 @@ VectorImage::LookupCachedSurface(const I
     RecoverFromLossOfSurfaces();
     return nullptr;
   }
 
   return sourceSurface.forget();
 }
 
 already_AddRefed<SourceSurface>
-VectorImage::CreateSurfaceAndShow(const SVGDrawingParameters& aParams, BackendType aBackend)
+VectorImage::CreateSurface(const SVGDrawingParameters& aParams,
+                           gfxDrawable* aSVGDrawable,
+                           bool& aWillCache)
 {
+  MOZ_ASSERT(mIsDrawing);
+
   mSVGDocumentWrapper->UpdateViewportBounds(aParams.viewportSize);
   mSVGDocumentWrapper->FlushImageTransformInvalidation();
 
-  RefPtr<gfxDrawingCallback> cb =
-    new SVGDrawingCallback(mSVGDocumentWrapper,
-                           aParams.viewportSize,
-                           aParams.size,
-                           aParams.flags);
-
-  RefPtr<gfxDrawable> svgDrawable =
-    new gfxCallbackDrawable(cb, aParams.size);
+  // Determine whether or not we should put the surface to be created into
+  // the cache. If we fail, we need to reset this to false to let the caller
+  // know nothing was put in the cache.
+  aWillCache = !(aParams.flags & FLAG_BYPASS_SURFACE_CACHE) &&
+               // Refuse to cache animated images:
+               // XXX(seth): We may remove this restriction in bug 922893.
+               !mHaveAnimations &&
+               // The image is too big to fit in the cache:
+               SurfaceCache::CanHold(aParams.size);
 
-  bool bypassCache = bool(aParams.flags & FLAG_BYPASS_SURFACE_CACHE) ||
-                     // Refuse to cache animated images:
-                     // XXX(seth): We may remove this restriction in bug 922893.
-                     mHaveAnimations ||
-                     // The image is too big to fit in the cache:
-                     !SurfaceCache::CanHold(aParams.size);
-  if (bypassCache) {
-    Show(svgDrawable, aParams);
+  // If we weren't given a context, then we know we just want the rasterized
+  // surface. We will create the frame below but only insert it into the cache
+  // if we actually need to.
+  if (!aWillCache && aParams.context) {
     return nullptr;
   }
 
   // We're about to rerasterize, which may mean that some of the previous
   // surfaces we've rasterized aren't useful anymore. We can allow them to
   // expire from the cache by unlocking them here, and then sending out an
   // invalidation. If this image is locked, any surfaces that are still useful
   // will become locked again when Draw touches them, and the remainder will
   // eventually expire.
-  SurfaceCache::UnlockEntries(ImageKey(this));
+  if (aWillCache) {
+    SurfaceCache::UnlockEntries(ImageKey(this));
+  }
+
+  // If there is no context, the default backend is fine.
+  BackendType backend =
+    aParams.context ? aParams.context->GetDrawTarget()->GetBackendType()
+                    : gfxPlatform::GetPlatform()->GetDefaultContentBackend();
 
   // Try to create an imgFrame, initializing the surface it contains by drawing
   // our gfxDrawable into it. (We use FILTER_NEAREST since we never scale here.)
   auto frame = MakeNotNull<RefPtr<imgFrame>>();
   nsresult rv =
-    frame->InitWithDrawable(svgDrawable, aParams.size,
+    frame->InitWithDrawable(aSVGDrawable, aParams.size,
                             SurfaceFormat::B8G8R8A8,
                             SamplingFilter::POINT, aParams.flags,
-                            aBackend);
+                            backend);
 
   // If we couldn't create the frame, it was probably because it would end
   // up way too big. Generally it also wouldn't fit in the cache, but the prefs
   // could be set such that the cache isn't the limiting factor.
   if (NS_FAILED(rv)) {
-    Show(svgDrawable, aParams);
+    aWillCache = false;
     return nullptr;
   }
 
   // Take a strong reference to the frame's surface and make sure it hasn't
   // already been purged by the operating system.
   RefPtr<SourceSurface> surface = frame->GetSourceSurface();
   if (!surface) {
-    Show(svgDrawable, aParams);
+    aWillCache = false;
     return nullptr;
   }
 
+  // We created the frame, but only because we had no context to draw to
+  // directly. All the caller wants is the surface in this case.
+  if (!aWillCache) {
+    return surface.forget();
+  }
+
   // Attempt to cache the frame.
   SurfaceKey surfaceKey = VectorSurfaceKey(aParams.size, aParams.svgContext);
   NotNull<RefPtr<ISurfaceProvider>> provider =
     MakeNotNull<SimpleSurfaceProvider*>(ImageKey(this), surfaceKey, frame);
   SurfaceCache::Insert(provider);
+  return surface.forget();
+}
 
-  // Draw.
-  RefPtr<gfxDrawable> drawable =
-    new gfxSurfaceDrawable(surface, aParams.size);
-  Show(drawable, aParams);
+void
+VectorImage::SendFrameComplete(bool aDidCache, uint32_t aFlags)
+{
+  // If the cache was not updated, we have nothing to do.
+  if (!aDidCache) {
+    return;
+  }
 
   // Send out an invalidation so that surfaces that are still in use get
   // re-locked. See the discussion of the UnlockSurfaces call above.
-  if (!(aParams.flags & FLAG_ASYNC_NOTIFY)) {
+  if (!(aFlags & FLAG_ASYNC_NOTIFY)) {
     mProgressTracker->SyncNotifyProgress(FLAG_FRAME_COMPLETE,
                                          GetMaxSizedIntRect());
   } else {
     NotNull<RefPtr<VectorImage>> image = WrapNotNull(this);
     NS_DispatchToMainThread(NS_NewRunnableFunction(
                               "ProgressTracker::SyncNotifyProgress",
                               [=]() -> void {
       RefPtr<ProgressTracker> tracker = image->GetProgressTracker();
       if (tracker) {
         tracker->SyncNotifyProgress(FLAG_FRAME_COMPLETE,
                                     GetMaxSizedIntRect());
       }
     }));
   }
-
-  return surface.forget();
 }
 
 
 void
 VectorImage::Show(gfxDrawable* aDrawable, const SVGDrawingParameters& aParams)
 {
   MOZ_ASSERT(aDrawable, "Should have a gfxDrawable by now");
   gfxUtils::DrawPixelSnapped(aParams.context, aDrawable,
--- a/image/VectorImage.h
+++ b/image/VectorImage.h
@@ -96,22 +96,30 @@ private:
     LookupCachedSurface(const IntSize& aSize,
                         const Maybe<SVGImageContext>& aSVGContext,
                         uint32_t aFlags);
 
   bool MaybeRestrictSVGContext(Maybe<SVGImageContext>& aNewSVGContext,
                                const Maybe<SVGImageContext>& aSVGContext,
                                uint32_t aFlags);
 
-  already_AddRefed<SourceSurface>
-    DrawInternal(const SVGDrawingParameters& aParams, bool aContextPaint);
+  /// Create a gfxDrawable which callbacks into the SVG document.
+  already_AddRefed<gfxDrawable>
+    CreateSVGDrawable(const SVGDrawingParameters& aParams);
 
+  /// Rasterize the SVG into a surface. aWillCache will be set to whether or
+  /// not the new surface was put into the cache.
   already_AddRefed<SourceSurface>
-    CreateSurfaceAndShow(const SVGDrawingParameters& aParams,
-                         gfx::BackendType aBackend);
+    CreateSurface(const SVGDrawingParameters& aParams,
+                  gfxDrawable* aSVGDrawable,
+                  bool& aWillCache);
+
+  /// Send a frame complete notification if appropriate. Must be called only
+  /// after all drawing has been completed.
+  void SendFrameComplete(bool aDidCache, uint32_t aFlags);
 
   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
    * surfaces even if they're locked. RecoverFromLossOfSurfaces discards all