Bug 1289816 - Simplify CopyableCanvasLayer::UpdateTarget and remove unnecessary copies. r=jnicol
☠☠ backed out by fc2138a46d6c ☠ ☠
authorNicolas Silva <nsilva@mozilla.com>
Thu, 28 Jul 2016 17:58:42 +0200
changeset 332206 d51327ec371ec44324d42734978802f0b1eff9b9
parent 332205 67cb195b1b45964b3d7a4d391c06d17228d47fe7
child 332207 d32a26786b30902820e02a9153bc466b8193b436
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjnicol
bugs1289816
milestone50.0a1
Bug 1289816 - Simplify CopyableCanvasLayer::UpdateTarget and remove unnecessary copies. r=jnicol
gfx/layers/CopyableCanvasLayer.cpp
gfx/layers/CopyableCanvasLayer.h
gfx/layers/basic/BasicCanvasLayer.cpp
gfx/layers/basic/BasicCanvasLayer.h
--- a/gfx/layers/CopyableCanvasLayer.cpp
+++ b/gfx/layers/CopyableCanvasLayer.cpp
@@ -45,18 +45,16 @@ CopyableCanvasLayer::CopyableCanvasLayer
 CopyableCanvasLayer::~CopyableCanvasLayer()
 {
   MOZ_COUNT_DTOR(CopyableCanvasLayer);
 }
 
 void
 CopyableCanvasLayer::Initialize(const Data& aData)
 {
-  NS_ASSERTION(mSurface == nullptr, "BasicCanvasLayer::Initialize called twice!");
-
   if (aData.mGLContext) {
     mGLContext = aData.mGLContext;
     mIsAlphaPremultiplied = aData.mIsGLAlphaPremult;
     mOriginPos = gl::OriginPos::BottomLeft;
     mIsMirror = aData.mIsMirror;
 
     MOZ_ASSERT(mGLContext->IsOffscreen(), "canvas gl context isn't offscreen");
 
@@ -67,65 +65,61 @@ CopyableCanvasLayer::Initialize(const Da
       mBufferProvider = aData.mBufferProvider;
     }
   } else if (aData.mBufferProvider) {
     mBufferProvider = aData.mBufferProvider;
   } else if (aData.mRenderer) {
     mAsyncRenderer = aData.mRenderer;
     mOriginPos = gl::OriginPos::BottomLeft;
   } else {
-    MOZ_CRASH("GFX: CanvasLayer created without mSurface, mDrawTarget or mGLContext?");
+    MOZ_CRASH("GFX: CanvasLayer created without BufferProvider, DrawTarget or GLContext?");
   }
 
   mBounds.SetRect(0, 0, aData.mSize.width, aData.mSize.height);
 }
 
 bool
 CopyableCanvasLayer::IsDataValid(const Data& aData)
 {
   return mGLContext == aData.mGLContext;
 }
 
 void
 CopyableCanvasLayer::UpdateTarget(DrawTarget* aDestTarget)
 {
-  AutoReturnSnapshot autoReturn;
-
-  if (mAsyncRenderer) {
-    mSurface = mAsyncRenderer->GetSurface();
-  } else if (!mGLFrontbuffer && mBufferProvider) {
-    mSurface = mBufferProvider->BorrowSnapshot();
-    if (aDestTarget) {
-      // If !aDestTarget we'll end up painting using mSurface later,
-      // so we can't return it to the provider (note that this will trigger a
-      // copy of the snapshot behind the scenes when the provider is unlocked).
-      autoReturn.mSnapshot = &mSurface;
-    }
-    // Either way we need to call ReturnSnapshot because ther may be an
-    // underlying TextureClient that has to be unlocked.
-    autoReturn.mBufferProvider = mBufferProvider;
+  MOZ_ASSERT(aDestTarget);
+  if (!aDestTarget) {
+    return;
   }
 
-  if (!mGLContext && aDestTarget) {
-    NS_ASSERTION(mSurface, "Must have surface to draw!");
-    if (mSurface) {
-      aDestTarget->CopySurface(mSurface,
+  RefPtr<SourceSurface> surface;
+
+  if (!mGLContext) {
+    AutoReturnSnapshot autoReturn;
+
+    if (mAsyncRenderer) {
+      surface = mAsyncRenderer->GetSurface();
+    } else if (mBufferProvider && !mGLContext) {
+      surface = mBufferProvider->BorrowSnapshot();
+      autoReturn.mSnapshot = &surface;
+      autoReturn.mBufferProvider = mBufferProvider;
+    }
+
+    NS_ASSERTION(surface, "Must have surface to draw!");
+    if (surface) {
+      aDestTarget->CopySurface(surface,
                                IntRect(0, 0, mBounds.width, mBounds.height),
                                IntPoint(0, 0));
-      mSurface = nullptr;
     }
 
     return;
   }
 
-  if ((!mGLFrontbuffer && mBufferProvider) || mAsyncRenderer) {
-    return;
-  }
-
-  MOZ_ASSERT(mGLContext);
+  MOZ_ASSERT(!mBufferProvider);
+  MOZ_ASSERT(!mAsyncRenderer);
 
   SharedSurface* frontbuffer = nullptr;
   if (mGLFrontbuffer) {
     frontbuffer = mGLFrontbuffer.get();
   } else {
     GLScreenBuffer* screen = mGLContext->Screen();
     const auto& front = screen->Front();
     if (front) {
@@ -140,59 +134,50 @@ CopyableCanvasLayer::UpdateTarget(DrawTa
 
   IntSize readSize(frontbuffer->mSize);
   SurfaceFormat format = (GetContentFlags() & CONTENT_OPAQUE)
                           ? SurfaceFormat::B8G8R8X8
                           : SurfaceFormat::B8G8R8A8;
   bool needsPremult = frontbuffer->mHasAlpha && !mIsAlphaPremultiplied;
 
   // Try to read back directly into aDestTarget's output buffer
-  if (aDestTarget) {
-    uint8_t* destData;
-    IntSize destSize;
-    int32_t destStride;
-    SurfaceFormat destFormat;
-    if (aDestTarget->LockBits(&destData, &destSize, &destStride, &destFormat)) {
-      if (destSize == readSize && destFormat == format) {
-        RefPtr<DataSourceSurface> data =
-          Factory::CreateWrappingDataSourceSurface(destData, destStride, destSize, destFormat);
-        mGLContext->Readback(frontbuffer, data);
-        if (needsPremult) {
-          gfxUtils::PremultiplyDataSurface(data, data);
-        }
-        aDestTarget->ReleaseBits(destData);
-        return;
+  uint8_t* destData;
+  IntSize destSize;
+  int32_t destStride;
+  SurfaceFormat destFormat;
+  if (aDestTarget->LockBits(&destData, &destSize, &destStride, &destFormat)) {
+    if (destSize == readSize && destFormat == format) {
+      RefPtr<DataSourceSurface> data =
+        Factory::CreateWrappingDataSourceSurface(destData, destStride, destSize, destFormat);
+      mGLContext->Readback(frontbuffer, data);
+      if (needsPremult) {
+        gfxUtils::PremultiplyDataSurface(data, data);
       }
       aDestTarget->ReleaseBits(destData);
+      return;
     }
+    aDestTarget->ReleaseBits(destData);
   }
 
   RefPtr<DataSourceSurface> resultSurf = GetTempSurface(readSize, format);
   // There will already be a warning from inside of GetTempSurface, but
   // it doesn't hurt to complain:
   if (NS_WARN_IF(!resultSurf)) {
     return;
   }
 
   // Readback handles Flush/MarkDirty.
   mGLContext->Readback(frontbuffer, resultSurf);
   if (needsPremult) {
     gfxUtils::PremultiplyDataSurface(resultSurf, resultSurf);
   }
-  MOZ_ASSERT(resultSurf);
 
-  if (aDestTarget) {
-    aDestTarget->CopySurface(resultSurf,
-                             IntRect(0, 0, readSize.width, readSize.height),
-                             IntPoint(0, 0));
-  } else {
-    // If !aDestSurface then we will end up painting using mSurface, so
-    // stick our surface into mSurface, so that the Paint() path is the same.
-    mSurface = resultSurf;
-  }
+  aDestTarget->CopySurface(resultSurf,
+                           IntRect(0, 0, readSize.width, readSize.height),
+                           IntPoint(0, 0));
 }
 
 DataSourceSurface*
 CopyableCanvasLayer::GetTempSurface(const IntSize& aSize,
                                     const SurfaceFormat aFormat)
 {
   if (!mCachedTempSurface ||
       aSize != mCachedTempSurface->GetSize() ||
@@ -201,16 +186,10 @@ CopyableCanvasLayer::GetTempSurface(cons
     // Create a surface aligned to 8 bytes since that's the highest alignment WebGL can handle.
     uint32_t stride = GetAlignedStride<8>(aSize.width * BytesPerPixel(aFormat));
     mCachedTempSurface = Factory::CreateDataSourceSurfaceWithStride(aSize, aFormat, stride);
   }
 
   return mCachedTempSurface;
 }
 
-void
-CopyableCanvasLayer::DiscardTempSurface()
-{
-  mCachedTempSurface = nullptr;
-}
-
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/CopyableCanvasLayer.h
+++ b/gfx/layers/CopyableCanvasLayer.h
@@ -44,31 +44,26 @@ public:
 
   virtual bool IsDataValid(const Data& aData) override;
 
   bool IsGLLayer() { return !!mGLContext; }
 
 protected:
   void UpdateTarget(gfx::DrawTarget* aDestTarget = nullptr);
 
-  RefPtr<gfx::SourceSurface> mSurface;
   RefPtr<gl::GLContext> mGLContext;
-  GLuint mCanvasFrontbufferTexID;
   RefPtr<PersistentBufferProvider> mBufferProvider;
-
   UniquePtr<gl::SharedSurface> mGLFrontbuffer;
 
   bool mIsAlphaPremultiplied;
   gl::OriginPos mOriginPos;
   bool mIsMirror;
 
   RefPtr<gfx::DataSourceSurface> mCachedTempSurface;
 
   gfx::DataSourceSurface* GetTempSurface(const gfx::IntSize& aSize,
                                          const gfx::SurfaceFormat aFormat);
-
-  void DiscardTempSurface();
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif
--- a/gfx/layers/basic/BasicCanvasLayer.cpp
+++ b/gfx/layers/basic/BasicCanvasLayer.cpp
@@ -6,58 +6,127 @@
 #include "BasicCanvasLayer.h"
 #include "AsyncCanvasRenderer.h"
 #include "basic/BasicLayers.h"          // for BasicLayerManager
 #include "basic/BasicLayersImpl.h"      // for GetEffectiveOperator
 #include "mozilla/mozalloc.h"           // for operator new
 #include "nsCOMPtr.h"                   // for already_AddRefed
 #include "nsISupportsImpl.h"            // for Layer::AddRef, etc
 #include "gfx2DGlue.h"
+#include "GLScreenBuffer.h"
+#include "GLContext.h"
+#include "gfxUtils.h"
+#include "mozilla/layers/PersistentBufferProvider.h"
+#include "client/TextureClientSharedSurface.h"
 
 class gfxContext;
 
 using namespace mozilla::gfx;
 using namespace mozilla::gl;
 
 namespace mozilla {
 namespace layers {
 
+already_AddRefed<SourceSurface>
+BasicCanvasLayer::UpdateSurface()
+{
+  if (mAsyncRenderer) {
+    return mAsyncRenderer->GetSurface();
+  }
+
+  if (mBufferProvider) {
+    // This is handled separately in Paint.
+    return nullptr;
+  }
+
+  if (!mGLContext) {
+    return nullptr;
+  }
+
+  SharedSurface* frontbuffer = nullptr;
+  if (mGLFrontbuffer) {
+    frontbuffer = mGLFrontbuffer.get();
+  } else {
+    GLScreenBuffer* screen = mGLContext->Screen();
+    const auto& front = screen->Front();
+    if (front) {
+      frontbuffer = front->Surf();
+    }
+  }
+
+  if (!frontbuffer) {
+    NS_WARNING("Null frame received.");
+    return nullptr;
+  }
+
+  IntSize readSize(frontbuffer->mSize);
+  SurfaceFormat format = (GetContentFlags() & CONTENT_OPAQUE)
+                          ? SurfaceFormat::B8G8R8X8
+                          : SurfaceFormat::B8G8R8A8;
+  bool needsPremult = frontbuffer->mHasAlpha && !mIsAlphaPremultiplied;
+
+  RefPtr<DataSourceSurface> resultSurf = GetTempSurface(readSize, format);
+  // There will already be a warning from inside of GetTempSurface, but
+  // it doesn't hurt to complain:
+  if (NS_WARN_IF(!resultSurf)) {
+    return nullptr;
+  }
+
+  // Readback handles Flush/MarkDirty.
+  mGLContext->Readback(frontbuffer, resultSurf);
+  if (needsPremult) {
+    gfxUtils::PremultiplyDataSurface(resultSurf, resultSurf);
+  }
+  MOZ_ASSERT(resultSurf);
+
+  return resultSurf.forget();
+}
+
 void
 BasicCanvasLayer::Paint(DrawTarget* aDT,
                         const Point& aDeviceOffset,
                         Layer* aMaskLayer)
 {
   if (IsHidden())
     return;
 
+  RefPtr<SourceSurface> surface;
   if (IsDirty()) {
     Painted();
 
     FirePreTransactionCallback();
-    UpdateTarget();
+    surface = UpdateSurface();
     FireDidTransactionCallback();
   }
 
-  if (!mSurface) {
+  AutoReturnSnapshot autoReturn(mBufferProvider);
+
+  if (mBufferProvider) {
+    MOZ_ASSERT(!surface);
+    surface = mBufferProvider->BorrowSnapshot();
+    autoReturn.mSnapshot = &surface;
+  }
+
+  if (!surface) {
     return;
   }
 
   const bool needsYFlip = (mOriginPos == gl::OriginPos::BottomLeft);
 
   Matrix oldTM;
   if (needsYFlip) {
     oldTM = aDT->GetTransform();
     aDT->SetTransform(Matrix(oldTM).
                         PreTranslate(0.0f, mBounds.height).
                         PreScale(1.0f, -1.0f));
   }
 
   FillRectWithMask(aDT, aDeviceOffset,
                    Rect(0, 0, mBounds.width, mBounds.height),
-                   mSurface, mSamplingFilter,
+                   surface, mSamplingFilter,
                    DrawOptions(GetEffectiveOpacity(), GetEffectiveOperator(this)),
                    aMaskLayer);
 
   if (needsYFlip) {
     aDT->SetTransform(oldTM);
   }
 }
 
--- a/gfx/layers/basic/BasicCanvasLayer.h
+++ b/gfx/layers/basic/BasicCanvasLayer.h
@@ -31,16 +31,19 @@ public:
     CanvasLayer::SetVisibleRegion(aRegion);
   }
 
   virtual void Paint(gfx::DrawTarget* aDT,
                      const gfx::Point& aDeviceOffset,
                      Layer* aMaskLayer) override;
 
 protected:
+
+  already_AddRefed<gfx::SourceSurface> UpdateSurface();
+
   BasicLayerManager* BasicManager()
   {
     return static_cast<BasicLayerManager*>(mManager);
   }
 };
 
 } // namespace layers
 } // namespace mozilla