Bug 1366097 - Part 4. Mark VectorImage as surface as shared when used outside an image layer. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 17 Nov 2017 14:08:52 -0500
changeset 392503 79b16521982847de443b83ec0f6cf9a4ce653bad
parent 392502 a4b811e0b860206b0aa4a66a14b8ebaf8655940b
child 392504 54d6ad659f11103861cf2d53259d77764dfd675e
push id32921
push usernerli@mozilla.com
push dateFri, 17 Nov 2017 22:02:18 +0000
treeherdermozilla-central@daa0dcd1616c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1366097
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 1366097 - Part 4. Mark VectorImage as surface as shared when used outside an image layer. r=tnikkel This is important because it ensures we release the shared memory handle (although not the data itself) for the underlying surface buffer when it turns out we will probably never need to share it. If we do need to share the surface data with the GPU process, it will reallocate a handle if necessary, and close it when it is finished. On some platforms we only have a finite number of handles, so if we don't need them, we should close them.
image/VectorImage.cpp
image/VectorImage.h
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -737,17 +737,22 @@ VectorImage::GetFrame(uint32_t aWhichFra
 }
 
 NS_IMETHODIMP_(already_AddRefed<SourceSurface>)
 VectorImage::GetFrameAtSize(const IntSize& aSize,
                             uint32_t aWhichFrame,
                             uint32_t aFlags)
 {
   auto result = GetFrameInternal(aSize, aWhichFrame, aFlags);
-  return Get<2>(result).forget();
+  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();
 }
 
 Tuple<DrawResult, IntSize, RefPtr<SourceSurface>>
 VectorImage::GetFrameInternal(const IntSize& aSize,
                               uint32_t aWhichFrame,
                               uint32_t aFlags)
 {
   MOZ_ASSERT(aWhichFrame <= FRAME_MAX_VALUE);
@@ -793,19 +798,27 @@ VectorImage::GetFrameInternal(const IntS
   MOZ_ASSERT(context); // already checked the draw target above
 
   Maybe<SVGImageContext> svgContext;
   SVGDrawingParameters params(context, aSize, ImageRegion::Create(aSize),
                               SamplingFilter::POINT, svgContext,
                               mSVGDocumentWrapper->GetCurrentTime(),
                               aFlags, 1.0);
 
-  DrawInternal(params, false);
-  RefPtr<SourceSurface> sourceSurface = dt->Snapshot();
-  return MakeTuple(DrawResult::SUCCESS, aSize, Move(sourceSurface));
+  // 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.
+  RefPtr<SourceSurface> surface = DrawInternal(params, false);
+  if (!surface) {
+    surface = dt->Snapshot();
+  }
+
+  return MakeTuple(DrawResult::SUCCESS, aSize, Move(surface));
 }
 
 //******************************************************************************
 IntSize
 VectorImage::GetImageContainerSize(LayerManager* aManager,
                                    const IntSize& aSize,
                                    uint32_t aFlags)
 {
@@ -949,21 +962,26 @@ 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;
   }
 
-  DrawInternal(params, haveContextPaint && !blockContextPaint);
+  RefPtr<SourceSurface> surface =
+    DrawInternal(params, haveContextPaint && !blockContextPaint);
+
+  // Image got put into a painted layer, it will not be shared with another
+  // process.
+  MarkSurfaceShared(surface);
   return DrawResult::SUCCESS;
 }
 
-void
+already_AddRefed<SourceSurface>
 VectorImage::DrawInternal(const SVGDrawingParameters& aParams,
                           bool aContextPaint)
 {
   MOZ_ASSERT(!mIsDrawing);
 
   AutoRestore<bool> autoRestoreIsDrawing(mIsDrawing);
   mIsDrawing = true;
 
@@ -980,17 +998,17 @@ VectorImage::DrawInternal(const SVGDrawi
   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();
-  CreateSurfaceAndShow(aParams, backend);
+  return CreateSurfaceAndShow(aParams, backend);
 }
 
 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.
@@ -1019,17 +1037,17 @@ VectorImage::LookupCachedSurface(const I
     // Attempt to recover.
     RecoverFromLossOfSurfaces();
     return nullptr;
   }
 
   return sourceSurface.forget();
 }
 
-void
+already_AddRefed<SourceSurface>
 VectorImage::CreateSurfaceAndShow(const SVGDrawingParameters& aParams, BackendType aBackend)
 {
   mSVGDocumentWrapper->UpdateViewportBounds(aParams.viewportSize);
   mSVGDocumentWrapper->FlushImageTransformInvalidation();
 
   RefPtr<gfxDrawingCallback> cb =
     new SVGDrawingCallback(mSVGDocumentWrapper,
                            aParams.viewportSize,
@@ -1041,17 +1059,18 @@ VectorImage::CreateSurfaceAndShow(const 
 
   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) {
-    return Show(svgDrawable, aParams);
+    Show(svgDrawable, aParams);
+    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.
@@ -1065,24 +1084,26 @@ VectorImage::CreateSurfaceAndShow(const 
                             SurfaceFormat::B8G8R8A8,
                             SamplingFilter::POINT, aParams.flags,
                             aBackend);
 
   // 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)) {
-    return Show(svgDrawable, aParams);
+    Show(svgDrawable, aParams);
+    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) {
-    return Show(svgDrawable, aParams);
+    Show(svgDrawable, aParams);
+    return nullptr;
   }
 
   // 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);
 
@@ -1103,16 +1124,18 @@ VectorImage::CreateSurfaceAndShow(const 
                               [=]() -> 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
@@ -91,19 +91,23 @@ private:
                                 uint32_t aFlags) override;
 
   /// 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);
+  already_AddRefed<SourceSurface>
+    DrawInternal(const SVGDrawingParameters& aParams, bool aContextPaint);
+
+  already_AddRefed<SourceSurface>
+    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
    * surfaces even if they're locked. RecoverFromLossOfSurfaces discards all
    * existing surfaces, allowing us to recover.