Don't assert about borrowed DT refcounts when async painting. (bug 1381393, r=mattwoodrow)
authorDavid Anderson <danderson@mozilla.com>
Tue, 25 Jul 2017 22:38:33 -0700
changeset 422378 4cd3ff1e99a289b90edf84fad83e30e51cafec29
parent 422377 d5cee2b7e3a4e2c758a96ceff3158faf4975aa67
child 422379 2a8ceea666e9d74fc162666ad42796ffaf480df1
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1381393
milestone56.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
Don't assert about borrowed DT refcounts when async painting. (bug 1381393, r=mattwoodrow)
gfx/layers/CompositorTypes.h
gfx/layers/client/ClientPaintedLayer.cpp
gfx/layers/client/ContentClient.cpp
gfx/layers/client/ContentClient.h
gfx/layers/client/TextureClient.cpp
--- a/gfx/layers/CompositorTypes.h
+++ b/gfx/layers/CompositorTypes.h
@@ -247,17 +247,22 @@ struct TextureInfo
  * See ShadowLayerForwarder::OpenDescriptor for example.
  */
 enum class OpenMode : uint8_t {
   OPEN_NONE        = 0,
   OPEN_READ        = 0x1,
   OPEN_WRITE       = 0x2,
   OPEN_READ_WRITE  = OPEN_READ|OPEN_WRITE,
   OPEN_READ_ONLY   = OPEN_READ,
-  OPEN_WRITE_ONLY  = OPEN_WRITE
+  OPEN_WRITE_ONLY  = OPEN_WRITE,
+
+  // This is only used in conjunction with OMTP to indicate that the DrawTarget
+  // that is being borrowed will be painted asynchronously, and so will outlive
+  // the write lock.
+  OPEN_ASYNC_WRITE = 0x04
 };
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(OpenMode)
 
 // The kinds of mask texture a shader can support
 // We rely on the items in this enum being sequential
 enum class MaskType : uint8_t {
   MaskNone = 0,   // no mask layer
   Mask,           // mask layer
--- a/gfx/layers/client/ClientPaintedLayer.cpp
+++ b/gfx/layers/client/ClientPaintedLayer.cpp
@@ -180,17 +180,17 @@ ClientPaintedLayer::PaintThebes(nsTArray
   if (didUpdate) {
     UpdateContentClient(state);
   }
 }
 
 bool
 ClientPaintedLayer::PaintOffMainThread()
 {
-  mContentClient->BeginPaint();
+  mContentClient->BeginAsyncPaint();
 
   uint32_t flags = GetPaintFlags();
 
   PaintState state = mContentClient->BeginPaintBuffer(this, flags);
   if (!UpdatePaintRegion(state)) {
     return false;
   }
 
--- a/gfx/layers/client/ContentClient.cpp
+++ b/gfx/layers/client/ContentClient.cpp
@@ -87,18 +87,25 @@ ContentClient::CreateContentClient(Compo
 
   if (useDoubleBuffering || gfxEnv::ForceDoubleBuffering()) {
     return MakeAndAddRef<ContentClientDoubleBuffered>(aForwarder);
   }
   return MakeAndAddRef<ContentClientSingleBuffered>(aForwarder);
 }
 
 void
+ContentClient::BeginAsyncPaint()
+{
+  mInAsyncPaint = true;
+}
+
+void
 ContentClient::EndPaint(nsTArray<ReadbackProcessor::Update>* aReadbackUpdates)
 {
+  mInAsyncPaint = false;
 }
 
 void
 ContentClient::PrintInfo(std::stringstream& aStream, const char* aPrefix)
 {
   aStream << aPrefix;
   aStream << nsPrintfCString("ContentClient (0x%p)", this).get();
 
@@ -234,16 +241,23 @@ ContentClientRemoteBuffer::BeginPaint()
     SetBufferProvider(mTextureClient);
   }
   if (mTextureClientOnWhite) {
     SetBufferProviderOnWhite(mTextureClientOnWhite);
   }
 }
 
 void
+ContentClientRemoteBuffer::BeginAsyncPaint()
+{
+  BeginPaint();
+  mInAsyncPaint = true;
+}
+
+void
 ContentClientRemoteBuffer::EndPaint(nsTArray<ReadbackProcessor::Update>* aReadbackUpdates)
 {
   MOZ_ASSERT(!mTextureClientOnWhite || !aReadbackUpdates || aReadbackUpdates->Length() == 0);
 
   // XXX: We might still not have a texture client if PaintThebes
   // decided we didn't need one yet because the region to draw was empty.
   SetBufferProvider(nullptr);
   SetBufferProviderOnWhite(nullptr);
@@ -344,24 +358,29 @@ ContentClientRemoteBuffer::CreateBuffer(
                                         RefPtr<gfx::DrawTarget>* aBlackDT,
                                         RefPtr<gfx::DrawTarget>* aWhiteDT)
 {
   BuildTextureClients(gfxPlatform::GetPlatform()->Optimal2DFormatForContent(aType), aRect, aFlags);
   if (!mTextureClient) {
     return;
   }
 
+  OpenMode mode = OpenMode::OPEN_READ_WRITE;
+  if (mInAsyncPaint) {
+    mode |= OpenMode::OPEN_ASYNC_WRITE;
+  }
+
   // We just created the textures and we are about to get their draw targets
   // so we have to lock them here.
-  DebugOnly<bool> locked = mTextureClient->Lock(OpenMode::OPEN_READ_WRITE);
+  DebugOnly<bool> locked = mTextureClient->Lock(mode);
   MOZ_ASSERT(locked, "Could not lock the TextureClient");
 
   *aBlackDT = mTextureClient->BorrowDrawTarget();
   if (aFlags & BUFFER_COMPONENT_ALPHA) {
-    locked = mTextureClientOnWhite->Lock(OpenMode::OPEN_READ_WRITE);
+    locked = mTextureClientOnWhite->Lock(mode);
     MOZ_ASSERT(locked, "Could not lock the second TextureClient for component alpha");
 
     *aWhiteDT = mTextureClientOnWhite->BorrowDrawTarget();
   }
 }
 
 nsIntRegion
 ContentClientRemoteBuffer::GetUpdatedRegion(const nsIntRegion& aRegionToDraw,
@@ -425,24 +444,28 @@ void
 ContentClientRemoteBuffer::SwapBuffers(const nsIntRegion& aFrontUpdatedRegion)
 {
   mFrontAndBackBufferDiffer = true;
 }
 
 bool
 ContentClientRemoteBuffer::LockBuffers()
 {
+  OpenMode mode = OpenMode::OPEN_READ_WRITE;
+  if (mInAsyncPaint) {
+    mode |= OpenMode::OPEN_ASYNC_WRITE;
+  }
   if (mTextureClient) {
-    bool locked = mTextureClient->Lock(OpenMode::OPEN_READ_WRITE);
+    bool locked = mTextureClient->Lock(mode);
     if (!locked) {
       return false;
     }
   }
   if (mTextureClientOnWhite) {
-    bool locked = mTextureClientOnWhite->Lock(OpenMode::OPEN_READ_WRITE);
+    bool locked = mTextureClientOnWhite->Lock(mode);
     if (!locked) {
       UnlockBuffers();
       return false;
     }
   }
   return true;
 }
 
@@ -548,16 +571,23 @@ ContentClientDoubleBuffered::BeginPaint(
     mBufferRect.MoveTo(mFrontBufferRect.TopLeft());
     mBufferRotation = nsIntPoint();
     return;
   }
   mBufferRect = mFrontBufferRect;
   mBufferRotation = mFrontBufferRotation;
 }
 
+void
+ContentClientDoubleBuffered::BeginAsyncPaint()
+{
+  BeginPaint();
+  mInAsyncPaint = true;
+}
+
 // Sync front/back buffers content
 // After executing, the new back buffer has the same (interesting) pixels as
 // the new front buffer, and mValidRegion et al. are correct wrt the new
 // back buffer (i.e. as they were for the old back buffer)
 void
 ContentClientDoubleBuffered::FinalizeFrame(const nsIntRegion& aRegionToDraw)
 {
   if (!mFrontAndBackBufferDiffer) {
--- a/gfx/layers/client/ContentClient.h
+++ b/gfx/layers/client/ContentClient.h
@@ -77,17 +77,18 @@ public:
   /**
    * Creates, configures, and returns a new content client. If necessary, a
    * message will be sent to the compositor to create a corresponding content
    * host.
    */
   static already_AddRefed<ContentClient> CreateContentClient(CompositableForwarder* aFwd);
 
   explicit ContentClient(CompositableForwarder* aForwarder)
-  : CompositableClient(aForwarder)
+  : CompositableClient(aForwarder),
+    mInAsyncPaint(false)
   {}
   virtual ~ContentClient()
   {}
 
   virtual void PrintInfo(std::stringstream& aStream, const char* aPrefix);
 
   virtual void Clear() = 0;
   virtual RotatedContentBuffer::PaintState BeginPaintBuffer(PaintedLayer* aLayer,
@@ -98,17 +99,21 @@ public:
 
   // Called as part of the layers transation reply. Conveys data about our
   // buffer(s) from the compositor. If appropriate we should swap references
   // to our buffers.
   virtual void SwapBuffers(const nsIntRegion& aFrontUpdatedRegion) {}
 
   // call before and after painting into this content client
   virtual void BeginPaint() {}
+  virtual void BeginAsyncPaint();
   virtual void EndPaint(nsTArray<ReadbackProcessor::Update>* aReadbackUpdates = nullptr);
+
+protected:
+  bool mInAsyncPaint;
 };
 
 /**
  * A ContentClient for use with OMTC.
  */
 class ContentClientRemote : public ContentClient
 {
 public:
@@ -238,16 +243,17 @@ public:
    * Begin/End Paint map a gfxASurface from the texture client
    * into the buffer of RotatedBuffer. The surface is only
    * valid when the texture client is locked, so is mapped out
    * of RotatedContentBuffer when we are done painting.
    * None of the underlying buffer attributes (rect, rotation)
    * are affected by mapping/unmapping.
    */
   virtual void BeginPaint() override;
+  virtual void BeginAsyncPaint() override;
   virtual void EndPaint(nsTArray<ReadbackProcessor::Update>* aReadbackUpdates = nullptr) override;
 
   virtual void Updated(const nsIntRegion& aRegionToDraw,
                        const nsIntRegion& aVisibleRegion,
                        bool aDidSelfCopy) override;
 
   virtual void SwapBuffers(const nsIntRegion& aFrontUpdatedRegion) override;
 
@@ -342,16 +348,17 @@ public:
 
   virtual void Updated(const nsIntRegion& aRegionToDraw,
                        const nsIntRegion& aVisibleRegion,
                        bool aDidSelfCopy) override;
 
   virtual void SwapBuffers(const nsIntRegion& aFrontUpdatedRegion) override;
 
   virtual void BeginPaint() override;
+  virtual void BeginAsyncPaint() override;
 
   virtual void FinalizeFrame(const nsIntRegion& aRegionToDraw) override;
 
   virtual void EnsureBackBufferIfFrontBuffer() override;
 
   virtual TextureInfo GetTextureInfo() const override
   {
     return TextureInfo(CompositableType::CONTENT_DOUBLE, mTextureFlags);
--- a/gfx/layers/client/TextureClient.cpp
+++ b/gfx/layers/client/TextureClient.cpp
@@ -504,17 +504,17 @@ TextureClient::Lock(OpenMode aMode)
 
   LockActor();
 
   mIsLocked = mData->Lock(aMode);
   mOpenMode = aMode;
 
   auto format = GetFormat();
   if (mIsLocked && CanExposeDrawTarget() &&
-      aMode == OpenMode::OPEN_READ_WRITE &&
+      (aMode & OpenMode::OPEN_READ_WRITE) == OpenMode::OPEN_READ_WRITE &&
       NS_IsMainThread() &&
       // the formats that we apparently expect, in the cairo backend. Any other
       // format will trigger an assertion in GfxFormatToCairoFormat.
       (format == SurfaceFormat::A8R8G8B8_UINT32 ||
       format == SurfaceFormat::X8R8G8B8_UINT32 ||
       format == SurfaceFormat::A8 ||
       format == SurfaceFormat::R5G6B5_UINT16)) {
     if (!BorrowDrawTarget()) {
@@ -552,17 +552,18 @@ TextureClient::Unlock()
         RefPtr<DataSourceSurface> dataSurf = snapshot->GetDataSurface();
         mReadbackSink->ProcessReadback(dataSurf);
       }
     }
 
     mBorrowedDrawTarget->DetachAllSnapshots();
     // If this assertion is hit, it means something is holding a strong reference
     // to our DrawTarget externally, which is not allowed.
-    MOZ_ASSERT(mBorrowedDrawTarget->refCount() <= mExpectedDtRefs);
+    MOZ_ASSERT_IF(!(mOpenMode & OpenMode::OPEN_ASYNC_WRITE),
+                  mBorrowedDrawTarget->refCount() <= mExpectedDtRefs);
 
     mBorrowedDrawTarget = nullptr;
   }
 
   if (mOpenMode & OpenMode::OPEN_WRITE) {
     mUpdated = true;
   }