Remove mInAsyncPaint and move locking new back buffer into BeginPaint (bug 1409871 part 19, r=nical) draft
authorRyan Hunt <rhunt@eqrion.net>
Wed, 18 Oct 2017 15:31:15 -0400
changeset 684141 8904be3b78787e1ade97bbf7bec9b3a65d694ebc
parent 684140 8c58ba66dc913dfaf8f870c8330fe993f4ab7d85
child 684142 02e4a680898842f1a3fab90eac19408593da2ce1
push id85567
push userbmo:rhunt@eqrion.net
push dateFri, 20 Oct 2017 22:13:22 +0000
reviewersnical
bugs1409871
milestone58.0a1
Remove mInAsyncPaint and move locking new back buffer into BeginPaint (bug 1409871 part 19, r=nical) mInAsyncPaint is only needed because the code using CreateBuffer expects the buffer to be locked when it is returned. This isn't necessary because we can now lock in BeginPaint. MozReview-Commit-ID: 8WazvBKMbvb
gfx/layers/client/ContentClient.cpp
gfx/layers/client/ContentClient.h
--- a/gfx/layers/client/ContentClient.cpp
+++ b/gfx/layers/client/ContentClient.cpp
@@ -109,20 +109,16 @@ ContentClient::Clear()
 {
   mBuffer = nullptr;
 }
 
 ContentClient::PaintState
 ContentClient::BeginPaint(PaintedLayer* aLayer,
                           uint32_t aFlags)
 {
-  if (aFlags & PAINT_ASYNC) {
-    mInAsyncPaint = true;
-  }
-
   PaintState result;
 
   BufferDecision dest = CalculateBufferForPaint(aLayer, aFlags);
   result.mContentType = dest.mBufferContentType;
 
   if (!dest.mCanKeepBufferContents) {
     // We're effectively clearing the valid region, so we need to draw
     // the entire needed region now.
@@ -144,18 +140,21 @@ ContentClient::BeginPaint(PaintedLayer* 
   }
 
   result.mRegionToDraw.Sub(dest.mNeededRegion,
                            dest.mValidRegion);
 
   if (result.mRegionToDraw.IsEmpty())
     return result;
 
+  OpenMode lockMode = aFlags & PAINT_ASYNC ? OpenMode::OPEN_READ_ASYNC_WRITE
+                                           : OpenMode::OPEN_READ_WRITE;
+
   if (mBuffer) {
-    if (mBuffer->Lock(LockMode())) {
+    if (mBuffer->Lock(lockMode)) {
       // Do not modify result.mRegionToDraw or result.mContentType after this call.
       // Do not modify the back buffer's bufferRect, bufferRotation, or didSelfCopy.
       FinalizeFrame(result.mRegionToDraw);
     } else {
       // Abandon everything and redraw it all. Ideally we'd reallocate and copy
       // the old to the new and then call FinalizeFrame on the new buffer so that
       // we only need to draw the latest bits, but we need a big refactor to support
       // that ordering.
@@ -213,16 +212,22 @@ ContentClient::BeginPaint(PaintedLayer* 
     }
   }
 
   NS_ASSERTION(!(aFlags & PAINT_WILL_RESAMPLE) || dest.mBufferRect == dest.mNeededRegion.GetBounds(),
                "If we're resampling, we need to validate the entire buffer");
 
   // If needed, copy the old buffer over to the new one
   if (newBuffer) {
+    if (!newBuffer->Lock(lockMode)) {
+      gfxCriticalNote << "Failed to lock new back buffer.";
+      Clear();
+      return result;
+    }
+
     if (mBuffer) {
       newBuffer->UpdateDestinationFrom(*mBuffer, newBuffer->BufferRect());
     }
 
     // Ensure our reference to the front buffer is released
     // as well as the old back buffer.
     Clear();
 
@@ -468,29 +473,16 @@ bool
 ContentClient::BufferSizeOkFor(const IntSize& aSize)
 {
   MOZ_ASSERT(mBuffer);
   return (aSize == mBuffer->BufferRect().Size() ||
           (SizedToVisibleBounds != mBufferSizePolicy &&
            aSize < mBuffer->BufferRect().Size()));
 }
 
-OpenMode
-ContentClient::LockMode() const
-{
-  return mInAsyncPaint ? OpenMode::OPEN_READ_ASYNC_WRITE
-                       : OpenMode::OPEN_READ_WRITE;
-}
-
-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();
 }
 
 // We pass a null pointer for the ContentClient Forwarder argument, which means
@@ -675,19 +667,16 @@ ContentClientRemoteBuffer::CreateBuffer(
   }
 
   RefPtr<RotatedBuffer> buffer = CreateBufferInternal(aRect, format, textureFlags);
 
   if (!buffer) {
     return nullptr;
   }
 
-  DebugOnly<bool> locked = buffer->Lock(LockMode());
-  MOZ_ASSERT(locked, "Could not lock the RemoteRotatedBuffer");
-
   mIsNewBuffer = true;
   mTextureFlags = textureFlags;
 
   return buffer;
 }
 
 RefPtr<RotatedBuffer>
 ContentClientRemoteBuffer::CreateBufferInternal(const gfx::IntRect& aRect,
--- a/gfx/layers/client/ContentClient.h
+++ b/gfx/layers/client/ContentClient.h
@@ -78,17 +78,16 @@ public:
     SizedToVisibleBounds,
     ContainsVisibleBounds
   };
 
   explicit ContentClient(CompositableForwarder* aForwarder,
                          BufferSizePolicy aBufferSizePolicy)
   : CompositableClient(aForwarder)
   , mBufferSizePolicy(aBufferSizePolicy)
-  , mInAsyncPaint(false)
   {}
   virtual ~ContentClient()
   {}
 
   virtual void PrintInfo(std::stringstream& aStream, const char* aPrefix);
 
   virtual void Clear();
 
@@ -139,17 +138,17 @@ public:
    * invalid pixels outside the visible region, if the visible region doesn't
    * fill the buffer bounds).
    * PAINT_CAN_DRAW_ROTATED can be passed if the caller supports drawing
    * rotated content that crosses the physical buffer boundary. The caller
    * will need to call BorrowDrawTargetForPainting multiple times to achieve
    * this.
    */
   virtual PaintState BeginPaint(PaintedLayer* aLayer, uint32_t aFlags);
-  virtual void EndPaint(nsTArray<ReadbackProcessor::Update>* aReadbackUpdates = nullptr);
+  virtual void EndPaint(nsTArray<ReadbackProcessor::Update>* aReadbackUpdates = nullptr) {}
 
   /**
    * Fetch a DrawTarget for rendering. The DrawTarget remains owned by
    * this. See notes on BorrowDrawTargetForQuadrantUpdate.
    * May return null. If the return value is non-null, it must be
    * 'un-borrowed' using ReturnDrawTarget.
    *
    * If PAINT_CAN_DRAW_ROTATED was specified for BeginPaint, then the caller
@@ -209,22 +208,16 @@ protected:
   gfxContentType BufferContentType();
   /**
    * Returns whether the specified size is adequate for the current
    * buffer and buffer size policy.
    */
   bool BufferSizeOkFor(const gfx::IntSize& aSize);
 
   /**
-   * Returns what open mode to use on texture clients. Ignored when
-   * using basic content clients.
-   */
-  OpenMode LockMode() const;
-
-  /**
    * Any actions that should be performed at the last moment before we begin
    * rendering the next frame. I.e., after we calculate what we will draw,
    * but before we rotate the buffer and possibly create new buffers.
    * aRegionToDraw is the region which is guaranteed to be overwritten when
    * drawing the next frame.
    */
   virtual void FinalizeFrame(const nsIntRegion& aRegionToDraw) {}
 
@@ -233,17 +226,16 @@ protected:
    * and buffer flags.
    */
   virtual RefPtr<RotatedBuffer> CreateBuffer(gfxContentType aType,
                                              const gfx::IntRect& aRect,
                                              uint32_t aFlags) = 0;
 
   RefPtr<RotatedBuffer> mBuffer;
   BufferSizePolicy      mBufferSizePolicy;
-  bool mInAsyncPaint;
 };
 
 // Thin wrapper around DrawTargetRotatedBuffer, for on-mtc
 class ContentClientBasic final : public ContentClient
 {
 public:
   explicit ContentClientBasic(gfx::BackendType aBackend);