Bug 1413693 - Ensure data copied from discarded front buffer isn't also painted. r?mstange draft
authorJamie Nicol <jnicol@mozilla.com>
Thu, 02 Nov 2017 11:46:08 +0000
changeset 692197 3f305075f19387578dac788fa68bc6766f437cea
parent 692062 b5a3b8ef6902998507fc881b6d628b055457fe31
child 738696 810f87092d86676620c63244b8748bc06f4e7fac
push id87432
push userbmo:jnicol@mozilla.com
push dateThu, 02 Nov 2017 17:55:52 +0000
reviewersmstange
bugs1413693, 1092294
milestone58.0a1
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;
   }