Backed out changeset 09ce12f88ca8 (bug 1420245) for test breakage on a CLOSED TREE.
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 01 Dec 2017 11:29:16 -0500
changeset 448844 958496eddbcc096a34d79b2faccfe6edff53449d
parent 448843 e0a21cc26e07b19383d2bf5038b88eed211de53e
child 448845 b89daf5c23b8d8e2b655ac201c68423ff0acfcce
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1420245
milestone59.0a1
backs out09ce12f88ca86d73e332a72a2f172ca438266758
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 09ce12f88ca8 (bug 1420245) for test breakage on a CLOSED TREE.
image/VectorImage.cpp
image/VectorImage.h
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -323,46 +323,16 @@ 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
@@ -810,42 +780,45 @@ 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>());
   }
 
-  // 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),
+  // 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 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);
 
-  bool didCache; // Was the surface put into the cache?
+  // 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();
-
-  AutoRestoreSVGState autoRestore(params, mSVGDocumentWrapper,
-                                  mIsDrawing, contextPaint);
-
-  RefPtr<gfxDrawable> svgDrawable = CreateSVGDrawable(params);
-  RefPtr<SourceSurface> surface =
-    CreateSurface(params, svgDrawable, didCache);
+  RefPtr<SourceSurface> surface = DrawInternal(params, contextPaint);
   if (!surface) {
-    MOZ_ASSERT(!didCache);
-    return MakeTuple(DrawResult::TEMPORARY_ERROR, aSize,
-                     RefPtr<SourceSurface>());
+    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)
@@ -1017,51 +990,52 @@ 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;
   }
 
-  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);
+  RefPtr<SourceSurface> surface = DrawInternal(params, contextPaint);
 
   // Image got put into a painted layer, it will not be shared with another
   // process.
-  MarkSurfaceShared(sourceSurface);
+  MarkSurfaceShared(surface);
   return DrawResult::SUCCESS;
 }
 
-already_AddRefed<gfxDrawable>
-VectorImage::CreateSVGDrawable(const SVGDrawingParameters& aParams)
+already_AddRefed<SourceSurface>
+VectorImage::DrawInternal(const SVGDrawingParameters& aParams,
+                          bool aContextPaint)
 {
-  RefPtr<gfxDrawingCallback> cb =
-    new SVGDrawingCallback(mSVGDocumentWrapper,
-                           aParams.viewportSize,
-                           aParams.size,
-                           aParams.flags);
+  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<gfxDrawable> svgDrawable =
-    new gfxCallbackDrawable(cb, aParams.size);
-  return svgDrawable.forget();
+  // 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);
 }
 
 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.
@@ -1091,121 +1065,104 @@ VectorImage::LookupCachedSurface(const I
     RecoverFromLossOfSurfaces();
     return nullptr;
   }
 
   return sourceSurface.forget();
 }
 
 already_AddRefed<SourceSurface>
-VectorImage::CreateSurface(const SVGDrawingParameters& aParams,
-                           gfxDrawable* aSVGDrawable,
-                           bool& aWillCache)
+VectorImage::CreateSurfaceAndShow(const SVGDrawingParameters& aParams, BackendType aBackend)
 {
-  MOZ_ASSERT(mIsDrawing);
-
   mSVGDocumentWrapper->UpdateViewportBounds(aParams.viewportSize);
   mSVGDocumentWrapper->FlushImageTransformInvalidation();
 
-  // 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);
+  RefPtr<gfxDrawingCallback> cb =
+    new SVGDrawingCallback(mSVGDocumentWrapper,
+                           aParams.viewportSize,
+                           aParams.size,
+                           aParams.flags);
+
+  RefPtr<gfxDrawable> svgDrawable =
+    new gfxCallbackDrawable(cb, aParams.size);
 
-  // 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) {
+  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);
     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.
-  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();
+  SurfaceCache::UnlockEntries(ImageKey(this));
 
   // 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(aSVGDrawable, aParams.size,
+    frame->InitWithDrawable(svgDrawable, aParams.size,
                             SurfaceFormat::B8G8R8A8,
                             SamplingFilter::POINT, aParams.flags,
-                            backend);
+                            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)) {
-    aWillCache = false;
+    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) {
-    aWillCache = false;
+    Show(svgDrawable, aParams);
     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();
-}
 
-void
-VectorImage::SendFrameComplete(bool aDidCache, uint32_t aFlags)
-{
-  // If the cache was not updated, we have nothing to do.
-  if (!aDidCache) {
-    return;
-  }
+  // Draw.
+  RefPtr<gfxDrawable> drawable =
+    new gfxSurfaceDrawable(surface, aParams.size);
+  Show(drawable, aParams);
 
   // Send out an invalidation so that surfaces that are still in use get
   // re-locked. See the discussion of the UnlockSurfaces call above.
-  if (!(aFlags & FLAG_ASYNC_NOTIFY)) {
+  if (!(aParams.flags & 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,30 +96,22 @@ private:
     LookupCachedSurface(const IntSize& aSize,
                         const Maybe<SVGImageContext>& aSVGContext,
                         uint32_t aFlags);
 
   bool MaybeRestrictSVGContext(Maybe<SVGImageContext>& aNewSVGContext,
                                const Maybe<SVGImageContext>& aSVGContext,
                                uint32_t aFlags);
 
-  /// 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>
-    CreateSurface(const SVGDrawingParameters& aParams,
-                  gfxDrawable* aSVGDrawable,
-                  bool& aWillCache);
+    DrawInternal(const SVGDrawingParameters& aParams, bool aContextPaint);
 
-  /// Send a frame complete notification if appropriate. Must be called only
-  /// after all drawing has been completed.
-  void SendFrameComplete(bool aDidCache, uint32_t aFlags);
+  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