Don't create back buffer for front buffer until we know what type to create. (bug 1399692 part 4, r=bas)
☠☠ backed out by 28647c01f828 ☠ ☠
authorRyan Hunt <rhunt@eqrion.net>
Mon, 23 Oct 2017 15:33:40 -0400
changeset 443203 926af2eca400cf8a16777813ceb586b1d3be7d68
parent 443202 b57a3f0d08478f094328b5a50c57eed35798fdf0
child 443204 3d0da65640964e5a0565b5e7b7646cf719ce1449
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbas
bugs1399692
milestone58.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 create back buffer for front buffer until we know what type to create. (bug 1399692 part 4, r=bas) This commit is an optimization for double buffering that delays the creation of a back buffer until we know what kind of content type it needs to be. Before this commit, we would EnsureBackBufferIfFrontBuffer before BeginPaint, then in BeginPaint we could determine that we actually needed a different kind of buffer because the content changed type, and recreate it. This was needed because BeginPaint would copy the old front buffer to the buffer created by EnsureBackBufferIfFrontBuffer, and then if anything failed or we had determined we couldn't reuse the buffer, we would create a new one and copy that "temporary" back buffer over, and use the new one. This is unnecessary because we only need read access on that "temporary" back buffer, and so we can just use the current front buffer instead. This optimization only affects the double buffered case, and the single buffered or basic cases should remain the same. Note: Because we now need the front buffer for copying into the new back buffer, we cannot Clear() it away in some error cases. MozReview-Commit-ID: 2hyrrUhA4zO
gfx/layers/client/ContentClient.cpp
gfx/layers/client/ContentClient.h
--- a/gfx/layers/client/ContentClient.cpp
+++ b/gfx/layers/client/ContentClient.cpp
@@ -144,58 +144,53 @@ ContentClient::BeginPaint(PaintedLayer* 
                            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)) {
-      // 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.
-      result.mRegionToDraw = dest.mNeededRegion;
-      dest.mCanReuseBuffer = false;
-      Clear();
-    }
-  }
-
   // We need to disable rotation if we're going to be resampled when
   // drawing, because we might sample across the rotation boundary.
   // Also disable buffer rotation when using webrender.
   bool canHaveRotation = gfxPlatform::BufferRotationEnabled() &&
                          !(aFlags & (PAINT_WILL_RESAMPLE | PAINT_NO_ROTATION)) &&
                          !(aLayer->Manager()->AsWebRenderLayerManager());
   bool canDrawRotated = aFlags & PAINT_CAN_DRAW_ROTATED;
   IntRect drawBounds = result.mRegionToDraw.GetBounds();
 
   if (dest.mCanReuseBuffer) {
     MOZ_ASSERT(mBuffer);
 
-    if (!mBuffer->AdjustTo(dest.mBufferRect,
-                           drawBounds,
-                           canHaveRotation,
-                           canDrawRotated)) {
+    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);
+
+      if (!mBuffer->AdjustTo(dest.mBufferRect,
+                             drawBounds,
+                             canHaveRotation,
+                             canDrawRotated)) {
+        dest.mBufferRect = ComputeBufferRect(dest.mNeededRegion.GetBounds());
+        dest.mCanReuseBuffer = false;
+        mBuffer->Unlock();
+      }
+    } else {
       dest.mBufferRect = ComputeBufferRect(dest.mNeededRegion.GetBounds());
       dest.mCanReuseBuffer = false;
     }
   }
 
   NS_ASSERTION(!(aFlags & PAINT_WILL_RESAMPLE) || dest.mBufferRect == dest.mNeededRegion.GetBounds(),
                "If we're resampling, we need to validate the entire buffer");
 
-  // The buffer's not big enough, changed content, or we failed to unrotate it,
-  // so we need to allocate a new one and prepare it for drawing
+  // We never had a buffer, the buffer wasn't big enough, the content changed
+  // types, or we failed to unrotate the buffer when requested. In any case,
+  // we need to allocate a new one and prepare it for drawing.
   if (!dest.mCanReuseBuffer) {
     uint32_t bufferFlags = 0;
     if (dest.mBufferMode == SurfaceMode::SURFACE_COMPONENT_ALPHA) {
       bufferFlags |= BUFFER_COMPONENT_ALPHA;
     }
 
     RefPtr<RotatedBuffer> newBuffer = CreateBuffer(result.mContentType,
                                                    dest.mBufferRect,
@@ -204,33 +199,33 @@ ContentClient::BeginPaint(PaintedLayer* 
     if (!newBuffer) {
       if (Factory::ReasonableSurfaceSize(IntSize(dest.mBufferRect.Width(), dest.mBufferRect.Height()))) {
         gfxCriticalNote << "Failed buffer for "
                         << dest.mBufferRect.x << ", "
                         << dest.mBufferRect.y << ", "
                         << dest.mBufferRect.Width() << ", "
                         << dest.mBufferRect.Height();
       }
-      Clear();
       return result;
     }
 
     if (!newBuffer->Lock(lockMode)) {
       gfxCriticalNote << "Failed to lock new back buffer.";
-      Clear();
       return result;
     }
 
-    // If we have an existing back buffer, copy it into the new back buffer
-    if (mBuffer) {
-      newBuffer->UpdateDestinationFrom(*mBuffer, newBuffer->BufferRect());
-
-      // We are done with the old back buffer now and it is about to be
-      // destroyed, so unlock it
-      mBuffer->Unlock();
+    // If we have an existing front buffer, copy it into the new back buffer
+    if (RefPtr<RotatedBuffer> frontBuffer = GetFrontBuffer()) {
+      if (frontBuffer->Lock(OpenMode::OPEN_READ_ONLY)) {
+        newBuffer->UpdateDestinationFrom(*frontBuffer, newBuffer->BufferRect());
+        frontBuffer->Unlock();
+      } else {
+        gfxCriticalNote << "Failed to copy front buffer to back buffer.";
+        return result;
+      }
     }
 
     // Ensure our reference to the front buffer is released
     // as well as the old back buffer
     Clear();
 
     mBuffer = newBuffer;
   }
@@ -428,21 +423,22 @@ ContentClient::CalculateBufferForPaint(P
 
       // We need to validate the entire buffer, to make sure that only valid
       // pixels are sampled.
       neededRegion = destBufferRect;
     }
 
     // If we have an existing buffer, but the content type has changed or we
     // have transitioned into/out of component alpha, then we need to recreate it.
-    if (canKeepBufferContents &&
-        mBuffer &&
-        (contentType != mBuffer->GetContentType() ||
-        (mode == SurfaceMode::SURFACE_COMPONENT_ALPHA) != mBuffer->HaveBufferOnWhite()))
-    {
+    RefPtr<RotatedBuffer> frontBuffer = GetFrontBuffer();
+    bool needsComponentAlpha = (mode == SurfaceMode::SURFACE_COMPONENT_ALPHA);
+    bool changedSurfaceOrContent = frontBuffer &&
+                                   (contentType != frontBuffer->GetContentType() ||
+                                    needsComponentAlpha != frontBuffer->HaveBufferOnWhite());
+    if (canKeepBufferContents && changedSurfaceOrContent) {
       // Restart the decision process; we won't re-enter since we guard on
       // being able to keep the buffer contents.
       canReuseBuffer = false;
       canKeepBufferContents = false;
       validRegion.SetEmpty();
       continue;
     }
 
@@ -468,16 +464,22 @@ ContentClient::ValidBufferSize(BufferSiz
                                const gfx::IntSize& aBufferSize,
                                const gfx::IntSize& aVisibleBoundsSize)
 {
   return (aVisibleBoundsSize == aBufferSize ||
           (SizedToVisibleBounds != aPolicy &&
            aVisibleBoundsSize < aBufferSize));
 }
 
+RefPtr<RotatedBuffer>
+ContentClient::GetFrontBuffer() const
+{
+  return mBuffer;
+}
+
 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
@@ -826,18 +828,16 @@ ContentClientDoubleBuffered::SwapBuffers
 
   mFrontAndBackBufferDiffer = true;
 }
 
 ContentClient::PaintState
 ContentClientDoubleBuffered::BeginPaint(PaintedLayer* aLayer,
                                         uint32_t aFlags)
 {
-  EnsureBackBufferIfFrontBuffer();
-
   mIsNewBuffer = false;
 
   if (!mFrontBuffer || !mBuffer) {
     mFrontAndBackBufferDiffer = false;
   }
 
   if (mFrontAndBackBufferDiffer) {
     if (mFrontBuffer->DidSelfCopy()) {
@@ -854,16 +854,22 @@ ContentClientDoubleBuffered::BeginPaint(
       mBuffer->SetBufferRect(mFrontBuffer->BufferRect());
       mBuffer->SetBufferRotation(mFrontBuffer->BufferRotation());
     }
   }
 
   return ContentClient::BeginPaint(aLayer, aFlags);
 }
 
+RefPtr<RotatedBuffer>
+ContentClientDoubleBuffered::GetFrontBuffer() const
+{
+  return mFrontBuffer;
+}
+
 // 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) {
@@ -902,21 +908,10 @@ ContentClientDoubleBuffered::FinalizeFra
   }
 
   if (mFrontBuffer->Lock(OpenMode::OPEN_READ_ONLY)) {
     mBuffer->UpdateDestinationFrom(*mFrontBuffer, updateRegion.GetBounds());
     mFrontBuffer->Unlock();
   }
 }
 
-void
-ContentClientDoubleBuffered::EnsureBackBufferIfFrontBuffer()
-{
-  if (!mBuffer && mFrontBuffer) {
-    mBuffer = CreateBufferInternal(mFrontBuffer->BufferRect(),
-                                   mFrontBuffer->GetFormat(),
-                                   mTextureFlags);
-    MOZ_ASSERT(mBuffer);
-  }
-}
-
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/client/ContentClient.h
+++ b/gfx/layers/client/ContentClient.h
@@ -202,16 +202,18 @@ protected:
    */
   BufferDecision CalculateBufferForPaint(PaintedLayer* aLayer,
                                          uint32_t aFlags);
 
   static bool ValidBufferSize(BufferSizePolicy aPolicy,
                               const gfx::IntSize& aBufferSize,
                               const gfx::IntSize& aVisibleBoundsSize);
 
+  virtual RefPtr<RotatedBuffer> GetFrontBuffer() 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) {}
@@ -352,26 +354,26 @@ public:
                     TextureDumpMode aCompress=TextureDumpMode::Compress) override;
 
   virtual void Clear() override;
 
   virtual void SwapBuffers(const nsIntRegion& aFrontUpdatedRegion) override;
 
   virtual PaintState BeginPaint(PaintedLayer* aLayer, uint32_t aFlags) override;
 
+  virtual RefPtr<RotatedBuffer> GetFrontBuffer() const override;
+
   virtual void FinalizeFrame(const nsIntRegion& aRegionToDraw) override;
 
   virtual TextureInfo GetTextureInfo() const override
   {
     return TextureInfo(CompositableType::CONTENT_DOUBLE, mTextureFlags);
   }
 
 private:
-  void EnsureBackBufferIfFrontBuffer();
-
   RefPtr<RemoteRotatedBuffer> mFrontBuffer;
   nsIntRegion mFrontUpdatedRegion;
   bool mFrontAndBackBufferDiffer;
 };
 
 /**
  * A single buffered ContentClientRemoteBuffer. We have a single
  * TextureClient/Host which we update and then send a message to the