Bug 1465619 - Part 5. Move actual drawing in imgFrame::Draw outside the monitor. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Sun, 03 Jun 2018 19:37:17 -0400
changeset 490707 ddaa812c31e4589b269dc622c6f6a1bd049f81ec
parent 490706 edeea0a49a3322a3af796fba2f582ed0f1f6bd5f
child 490708 7a2954981481f2f9f7bae58709dd50f67dab7cba
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerstnikkel
bugs1465619
milestone65.0a1
Bug 1465619 - Part 5. Move actual drawing in imgFrame::Draw outside the monitor. r=tnikkel Since imgFrame::Draw will limit the drawing to only look at pixels that we have written to and posted an invalidation for, there is no need to hold the monitor while doing so. By taking the most expensive operation outside the lock, we will minimize our chances of hitting contention with the decoder thread. A later part in this series will require that a surface be freed outside the lock because it may end up reacquiring it. In addition to the contention win, this change should facilitate that. Differential Revision: https://phabricator.services.mozilla.com/D7510
image/imgFrame.cpp
image/imgFrame.h
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -557,36 +557,43 @@ bool imgFrame::Draw(gfxContext* aContext
   MOZ_ASSERT(mFrameRect.IsEqualEdges(IntRect(IntPoint(), mImageSize)),
              "Directly drawing an image with a non-trivial frame rect!");
 
   if (mPalettedImageData) {
     MOZ_ASSERT_UNREACHABLE("Directly drawing a paletted image!");
     return false;
   }
 
-  MonitorAutoLock lock(mMonitor);
+  // Perform the draw and freeing of the surface outside the lock. We want to
+  // avoid contention with the decoder if we can.
+  RefPtr<SourceSurface> surf;
+  SurfaceWithFormat surfaceResult;
+  ImageRegion region(aRegion);
+  gfxRect imageRect(0, 0, mImageSize.width, mImageSize.height);
 
-  // Possibly convert this image into a GPU texture, this may also cause our
-  // mLockedSurface to be released and the OS to release the underlying memory.
-  Optimize(aContext->GetDrawTarget());
+  {
+    MonitorAutoLock lock(mMonitor);
 
-  bool doPartialDecode = !AreAllPixelsWritten();
+    // Possibly convert this image into a GPU texture, this may also cause our
+    // mLockedSurface to be released and the OS to release the underlying memory.
+    Optimize(aContext->GetDrawTarget());
 
-  RefPtr<SourceSurface> surf = GetSourceSurfaceInternal();
-  if (!surf) {
-    return false;
-  }
+    bool doPartialDecode = !AreAllPixelsWritten();
+
+    surf = GetSourceSurfaceInternal();
+    if (!surf) {
+      return false;
+    }
 
-  gfxRect imageRect(0, 0, mImageSize.width, mImageSize.height);
-  bool doTile = !imageRect.Contains(aRegion.Rect()) &&
-                !(aImageFlags & imgIContainer::FLAG_CLAMP);
+    bool doTile = !imageRect.Contains(aRegion.Rect()) &&
+                  !(aImageFlags & imgIContainer::FLAG_CLAMP);
 
-  ImageRegion region(aRegion);
-  SurfaceWithFormat surfaceResult =
-    SurfaceForDrawing(doPartialDecode, doTile, region, surf);
+    surfaceResult =
+      SurfaceForDrawing(doPartialDecode, doTile, region, surf);
+  }
 
   if (surfaceResult.IsValid()) {
     gfxUtils::DrawPixelSnapped(aContext, surfaceResult.mDrawable,
                                imageRect.Size(), region, surfaceResult.mFormat,
                                aSamplingFilter, aImageFlags, aOpacity);
   }
 
   return true;
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -260,16 +260,27 @@ private: // methods
     SurfaceFormat mFormat;
     SurfaceWithFormat()
       : mFormat(SurfaceFormat::UNKNOWN)
     {
     }
     SurfaceWithFormat(gfxDrawable* aDrawable, SurfaceFormat aFormat)
       : mDrawable(aDrawable), mFormat(aFormat)
     { }
+    SurfaceWithFormat(SurfaceWithFormat&& aOther)
+      : mDrawable(std::move(aOther.mDrawable)), mFormat(aOther.mFormat)
+    { }
+    SurfaceWithFormat& operator=(SurfaceWithFormat&& aOther)
+    {
+      mDrawable = std::move(aOther.mDrawable);
+      mFormat = aOther.mFormat;
+      return *this;
+    }
+    SurfaceWithFormat& operator=(const SurfaceWithFormat& aOther) = delete;
+    SurfaceWithFormat(const SurfaceWithFormat& aOther) = delete;
     bool IsValid() { return !!mDrawable; }
   };
 
   SurfaceWithFormat SurfaceForDrawing(bool               aDoPartialDecode,
                                       bool               aDoTile,
                                       ImageRegion&       aRegion,
                                       SourceSurface*     aSurface);