Bug 1413693 - Ensure data copied from discarded front buffer isn't also painted. r=mstange
authorJamie Nicol <jnicol@mozilla.com>
Thu, 02 Nov 2017 11:46:08 +0000
changeset 443180 6f7cae36d92d36304c05df539135de6e913ab45c
parent 443179 6c869780c87ec220a3530de3ae9bb06e340b7151
child 443181 ef5b98d6c07d3562621b2abf11b375a9f180a34c
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)
reviewersmstange
bugs1413693, 1092294
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
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
gfx/layers/client/SingleTiledContentClient.cpp
--- 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;
   }