Bug 1413693 - Ensure data copied from discarded front buffer isn't also painted. r?mstange
Bug 1092294 introduced a regression in to the code to copy from
the discarded front buffers to the new backbuffers in
SingleTiledContentClient. The aim was to ensure that if locking the
the frontbuffers failed, meaning the region could not be copied, that it
would be painted instead. However due to incorrect logic the
region would both be copied and painted in cases where
there was no onWhite buffers.
To fix this we take both locks (frontLock, and frontOnWhiteLock if
required) up front, so that we either copy both buffers or neither.
MozReview-Commit-ID: 3iepOuweruk
--- a/gfx/layers/client/SingleTiledContentClient.cpp
+++ b/gfx/layers/client/SingleTiledContentClient.cpp
@@ -2,16 +2,17 @@
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "mozilla/layers/SingleTiledContentClient.h"
#include "ClientTiledPaintedLayer.h"
+#include "mozilla/Maybe.h"
namespace mozilla {
namespace layers {
SingleTiledContentClient::SingleTiledContentClient(ClientTiledPaintedLayer& aPaintedLayer,
ClientLayerManager* aManager)
: TiledContentClient(aManager, "Single")
@@ -196,41 +197,42 @@ ClientSingleTiledLayerBuffer::PaintThebe
if (discardedFrontBuffer) {
nsIntRegion copyableRegion;
copyableRegion.And(aNewValidRegion, discardedValidRegion);
copyableRegion.SubOut(aDirtyRegion);
if (!copyableRegion.IsEmpty()) {
TextureClientAutoLock frontLock(discardedFrontBuffer,
OpenMode::OPEN_READ);
- if (frontLock.Succeeded()) {
+ Maybe<TextureClientAutoLock> frontOnWhiteLock;
+ if (discardedFrontBufferOnWhite && backBufferOnWhite) {
+ frontOnWhiteLock.emplace(discardedFrontBufferOnWhite, OpenMode::OPEN_READ);
+ }
+
+ // Copy to both backBuffer and backBufferOnWhite if required, or copy to neither.
+ if (frontLock.Succeeded() && (!frontOnWhiteLock || frontOnWhiteLock->Succeeded())) {
for (auto iter = copyableRegion.RectIter(); !iter.Done(); iter.Next()) {
const gfx::IntRect rect = iter.Get() - discardedValidRegion.GetBounds().TopLeft();
const gfx::IntPoint dest = iter.Get().TopLeft() - mTilingOrigin;
discardedFrontBuffer->CopyToTextureClient(backBuffer, &rect, &dest);
}
- if (discardedFrontBufferOnWhite && backBufferOnWhite) {
- TextureClientAutoLock frontOnWhiteLock(discardedFrontBufferOnWhite,
- OpenMode::OPEN_READ);
- if (frontOnWhiteLock.Succeeded()) {
- for (auto iter = copyableRegion.RectIter(); !iter.Done(); iter.Next()) {
- const gfx::IntRect rect = iter.Get() - discardedValidRegion.GetBounds().TopLeft();
- const gfx::IntPoint dest = iter.Get().TopLeft() - mTilingOrigin;
-
- discardedFrontBufferOnWhite->CopyToTextureClient(backBufferOnWhite,
- &rect, &dest);
- }
-
- TILING_LOG("TILING %p: Region copied from discarded frontbuffer %s\n", &mPaintedLayer, Stringify(copyableRegion).c_str());
-
- // We don't need to repaint valid content that was just copied.
- paintRegion.SubOut(copyableRegion);
+ if (frontOnWhiteLock) {
+ for (auto iter = copyableRegion.RectIter(); !iter.Done(); iter.Next()) {
+ const gfx::IntRect rect = iter.Get() - discardedValidRegion.GetBounds().TopLeft();
+ const gfx::IntPoint dest = iter.Get().TopLeft() - mTilingOrigin;
+ discardedFrontBufferOnWhite->CopyToTextureClient(backBufferOnWhite,
+ &rect, &dest);
}
}
+
+ TILING_LOG("TILING %p: Region copied from discarded frontbuffer %s\n", &mPaintedLayer, Stringify(copyableRegion).c_str());
+
+ // We don't need to repaint valid content that was just copied.
+ paintRegion.SubOut(copyableRegion);
}
}
}
if (dtOnWhite) {
dt = gfx::Factory::CreateDualDrawTarget(dt, dtOnWhite);
dtOnWhite = nullptr;
}