From 8a4c400778b6751ae19dd16086a8ed9c74a1046f Mon Sep 17 00:00:00 2001 From: Ryan Hunt Date: Mon, 23 Oct 2017 15:33:40 -0400 Subject: [PATCH] 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. Note: The code in FinalizeFrame assumes that the back and front buffer have the same size. This was implicitly enforced before, and now needs to be explicitly enforced. This commit tries to preserve the exact same behavior, although the restriction should be removed as long as the back buffer is large enough for the visible region. MozReview-Commit-ID: 2hyrrUhA4zO --HG-- extra : rebase_source : 2ddaa7d7d250475cf68524ad2151d8bcc7101013 extra : source : 926af2eca400cf8a16777813ceb586b1d3be7d68 --- gfx/layers/client/ContentClient.cpp | 122 ++++++++++++++++------------ gfx/layers/client/ContentClient.h | 7 +- 2 files changed, 73 insertions(+), 56 deletions(-) diff --git a/gfx/layers/client/ContentClient.cpp b/gfx/layers/client/ContentClient.cpp index 850dc33a0a66..73c41210fb80 100644 --- a/gfx/layers/client/ContentClient.cpp +++ b/gfx/layers/client/ContentClient.cpp @@ -149,22 +149,6 @@ ContentClient::BeginPaint(PaintedLayer* aLayer, 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. @@ -177,20 +161,33 @@ ContentClient::BeginPaint(PaintedLayer* aLayer, 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; + dest.mMustRemoveFrontBuffer = true; + mBuffer->Unlock(); + } + } else { dest.mBufferRect = ComputeBufferRect(dest.mNeededRegion.GetBounds()); dest.mCanReuseBuffer = false; + dest.mMustRemoveFrontBuffer = true; } } 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) { @@ -209,29 +206,29 @@ ContentClient::BeginPaint(PaintedLayer* aLayer, << 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()); + // If we have an existing front buffer, copy it into the new back buffer + if (RefPtr 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; + } - // We are done with the old back buffer now and it is about to be - // destroyed, so unlock it - mBuffer->Unlock(); + if (dest.mMustRemoveFrontBuffer) { + Clear(); + } } - // Ensure our reference to the front buffer is released - // as well as the old back buffer - Clear(); - mBuffer = newBuffer; } @@ -368,6 +365,8 @@ ContentClient::CalculateBufferForPaint(PaintedLayer* aLayer, aLayer->CanUseOpaqueSurface() ? gfxContentType::COLOR : gfxContentType::COLOR_ALPHA; + RefPtr frontBuffer = GetFrontBuffer(); + SurfaceMode mode; gfxContentType contentType; IntRect destBufferRect; @@ -376,6 +375,7 @@ ContentClient::CalculateBufferForPaint(PaintedLayer* aLayer, bool canReuseBuffer = !!mBuffer; bool canKeepBufferContents = true; + bool mustRemoveFrontBuffer = false; while (true) { mode = aLayer->GetSurfaceMode(); @@ -397,8 +397,22 @@ ContentClient::CalculateBufferForPaint(PaintedLayer* aLayer, destBufferRect = neededRegion.GetBounds(); } } else { - // We won't be reusing the buffer. Compute a new rect. - destBufferRect = ComputeBufferRect(neededRegion.GetBounds()); + // We won't be reusing the buffer. Compute a new rect. + if (frontBuffer) { + // We must create a buffer that is the same size as the front buffer, + // or else we need to remove the front buffer + if (ValidBufferSize(mBufferSizePolicy, + frontBuffer->BufferRect().Size(), + neededRegion.GetBounds().Size())) { + destBufferRect = frontBuffer->BufferRect(); + destBufferRect.MoveTo(neededRegion.GetBounds().TopLeft()); + } else { + destBufferRect = ComputeBufferRect(neededRegion.GetBounds()); + mustRemoveFrontBuffer = true; + } + } else { + destBufferRect = ComputeBufferRect(neededRegion.GetBounds()); + } } if (mode == SurfaceMode::SURFACE_COMPONENT_ALPHA) { @@ -433,11 +447,11 @@ ContentClient::CalculateBufferForPaint(PaintedLayer* aLayer, // 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())) - { + 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; @@ -460,6 +474,7 @@ ContentClient::CalculateBufferForPaint(PaintedLayer* aLayer, dest.mBufferContentType = contentType; dest.mCanReuseBuffer = canReuseBuffer; dest.mCanKeepBufferContents = canKeepBufferContents; + dest.mMustRemoveFrontBuffer = mustRemoveFrontBuffer; return dest; } @@ -473,6 +488,12 @@ ContentClient::ValidBufferSize(BufferSizePolicy aPolicy, aVisibleBoundsSize < aBufferSize)); } +RefPtr +ContentClient::GetFrontBuffer() const +{ + return mBuffer; +} + void ContentClient::PrintInfo(std::stringstream& aStream, const char* aPrefix) { @@ -831,8 +852,6 @@ ContentClient::PaintState ContentClientDoubleBuffered::BeginPaint(PaintedLayer* aLayer, uint32_t aFlags) { - EnsureBackBufferIfFrontBuffer(); - mIsNewBuffer = false; if (!mFrontBuffer || !mBuffer) { @@ -859,6 +878,12 @@ ContentClientDoubleBuffered::BeginPaint(PaintedLayer* aLayer, return ContentClient::BeginPaint(aLayer, aFlags); } +RefPtr +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 @@ -907,16 +932,5 @@ ContentClientDoubleBuffered::FinalizeFrame(const nsIntRegion& aRegionToDraw) } } -void -ContentClientDoubleBuffered::EnsureBackBufferIfFrontBuffer() -{ - if (!mBuffer && mFrontBuffer) { - mBuffer = CreateBufferInternal(mFrontBuffer->BufferRect(), - mFrontBuffer->GetFormat(), - mTextureFlags); - MOZ_ASSERT(mBuffer); - } -} - } // namespace layers } // namespace mozilla diff --git a/gfx/layers/client/ContentClient.h b/gfx/layers/client/ContentClient.h index 4ba12e9d6a56..51c929138811 100644 --- a/gfx/layers/client/ContentClient.h +++ b/gfx/layers/client/ContentClient.h @@ -193,6 +193,7 @@ protected: gfxContentType mBufferContentType; bool mCanReuseBuffer; bool mCanKeepBufferContents; + bool mMustRemoveFrontBuffer; }; /** @@ -207,6 +208,8 @@ protected: const gfx::IntSize& aBufferSize, const gfx::IntSize& aVisibleBoundsSize); + virtual RefPtr 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, @@ -357,6 +360,8 @@ public: virtual PaintState BeginPaint(PaintedLayer* aLayer, uint32_t aFlags) override; + virtual RefPtr GetFrontBuffer() const override; + virtual void FinalizeFrame(const nsIntRegion& aRegionToDraw) override; virtual TextureInfo GetTextureInfo() const override @@ -365,8 +370,6 @@ public: } private: - void EnsureBackBufferIfFrontBuffer(); - RefPtr mFrontBuffer; nsIntRegion mFrontUpdatedRegion; bool mFrontAndBackBufferDiffer;