Bug 1255448 - Call ClientMultiTiledLayerBuffer::PaintThebes even when region to paint is empty. r?mattwoodrow draft
authorJamie Nicol <jnicol@mozilla.com>
Thu, 10 Mar 2016 15:01:15 +0000
changeset 339242 f09648da6e34fea63e5066b7af3395d6f96fd92f
parent 339110 dd1abe874252e507b825a0a4e1063b0e13578288
child 515949 be0eae3fffa2056de611b9e56918c9a6e2f1badc
push id12682
push userbmo:jnicol@mozilla.com
push dateThu, 10 Mar 2016 22:07:40 +0000
reviewersmattwoodrow
bugs1255448
milestone48.0a1
Bug 1255448 - Call ClientMultiTiledLayerBuffer::PaintThebes even when region to paint is empty. r?mattwoodrow Remove early return for empty invalid regions from ClientTiledPaintedLayer::RenderHighPrecision so that ClientMultiTiledLayerBuffer::PaintThebes is called even when the region to paint is empty. This ensures that the tile buffer will free unused tiles in cases where no new painting is required but the valid region has shrunk. Add replacement early returns into ClientMultiTiledLayerBuffer to avoid as much needless work as possible while still recalculating which tiles are valid. MozReview-Commit-ID: C86Pi7lRjjs
gfx/layers/client/ClientTiledPaintedLayer.cpp
gfx/layers/client/TiledContentClient.cpp
--- a/gfx/layers/client/ClientTiledPaintedLayer.cpp
+++ b/gfx/layers/client/ClientTiledPaintedLayer.cpp
@@ -289,41 +289,44 @@ ClientTiledPaintedLayer::UseProgressiveD
 }
 
 bool
 ClientTiledPaintedLayer::RenderHighPrecision(nsIntRegion& aInvalidRegion,
                                             const nsIntRegion& aVisibleRegion,
                                             LayerManager::DrawPaintedLayerCallback aCallback,
                                             void* aCallbackData)
 {
-  // If we have no high-precision stuff to draw, or we have started drawing low-precision
-  // already, then we shouldn't do anything there.
-  if (aInvalidRegion.IsEmpty() || mPaintData.mLowPrecisionPaintCount != 0) {
+  // If we have started drawing low-precision already, then we
+  // shouldn't do anything there.
+  if (mPaintData.mLowPrecisionPaintCount != 0) {
     return false;
   }
 
-  // Only draw progressively when the resolution is unchanged
-  if (UseProgressiveDraw() &&
+  // Only draw progressively when there is something to paint and the
+  // resolution is unchanged
+  if (!aInvalidRegion.IsEmpty() &&
+      UseProgressiveDraw() &&
       mContentClient->GetTiledBuffer()->GetFrameResolution() == mPaintData.mResolution) {
     // Store the old valid region, then clear it before painting.
     // We clip the old valid region to the visible region, as it only gets
     // used to decide stale content (currently valid and previously visible)
     nsIntRegion oldValidRegion = mContentClient->GetTiledBuffer()->GetValidRegion();
     oldValidRegion.And(oldValidRegion, aVisibleRegion);
     if (mPaintData.mCriticalDisplayPort) {
       oldValidRegion.And(oldValidRegion, mPaintData.mCriticalDisplayPort->ToUnknownRect());
     }
 
     TILING_LOG("TILING %p: Progressive update with old valid region %s\n", this, Stringify(oldValidRegion).c_str());
 
     return mContentClient->GetTiledBuffer()->ProgressiveUpdate(mValidRegion, aInvalidRegion,
                       oldValidRegion, &mPaintData, aCallback, aCallbackData);
   }
 
-  // Otherwise do a non-progressive paint
+  // Otherwise do a non-progressive paint. We must do this even when
+  // the region to paint is empty as the valid region may have shrunk.
 
   mValidRegion = aVisibleRegion;
   if (mPaintData.mCriticalDisplayPort) {
     mValidRegion.And(mValidRegion, mPaintData.mCriticalDisplayPort->ToUnknownRect());
   }
 
   TILING_LOG("TILING %p: Non-progressive paint invalid region %s\n", this, Stringify(aInvalidRegion).c_str());
   TILING_LOG("TILING %p: Non-progressive paint new valid region %s\n", this, Stringify(mValidRegion).c_str());
--- a/gfx/layers/client/TiledContentClient.cpp
+++ b/gfx/layers/client/TiledContentClient.cpp
@@ -904,57 +904,57 @@ ClientMultiTiledLayerBuffer::PaintThebes
 
   mCallback = aCallback;
   mCallbackData = aCallbackData;
 
 #ifdef GFX_TILEDLAYER_PREF_WARNINGS
   long start = PR_IntervalNow();
 #endif
 
-  // If this region is empty XMost() - 1 will give us a negative value.
-  NS_ASSERTION(!aPaintRegion.GetBounds().IsEmpty(), "Empty paint region\n");
+  if (!gfxPrefs::TiledDrawTargetEnabled()) {
+    if (!aPaintRegion.IsEmpty()) {
+
+      RefPtr<gfxContext> ctxt;
+
+      const IntRect bounds = aPaintRegion.GetBounds();
+      {
+        PROFILER_LABEL("ClientMultiTiledLayerBuffer", "PaintThebesSingleBufferAlloc",
+          js::ProfileEntry::Category::GRAPHICS);
+
+        mSinglePaintDrawTarget =
+          gfxPlatform::GetPlatform()->CreateOffscreenContentDrawTarget(
+            gfx::IntSize(ceilf(bounds.width * mResolution),
+                         ceilf(bounds.height * mResolution)),
+            gfxPlatform::GetPlatform()->Optimal2DFormatForContent(
+              GetContentType()));
 
-  if (!gfxPrefs::TiledDrawTargetEnabled()) {
-    RefPtr<gfxContext> ctxt;
+        if (!mSinglePaintDrawTarget) {
+          return;
+        }
+
+        ctxt = new gfxContext(mSinglePaintDrawTarget);
 
-    const IntRect bounds = aPaintRegion.GetBounds();
-    {
-      PROFILER_LABEL("ClientMultiTiledLayerBuffer", "PaintThebesSingleBufferAlloc",
+        mSinglePaintBufferOffset = nsIntPoint(bounds.x, bounds.y);
+      }
+      ctxt->NewPath();
+      ctxt->SetMatrix(
+        ctxt->CurrentMatrix().Scale(mResolution, mResolution).
+                              Translate(-bounds.x, -bounds.y));
+#ifdef GFX_TILEDLAYER_PREF_WARNINGS
+      if (PR_IntervalNow() - start > 3) {
+        printf_stderr("Slow alloc %i\n", PR_IntervalNow() - start);
+      }
+      start = PR_IntervalNow();
+#endif
+      PROFILER_LABEL("ClientMultiTiledLayerBuffer", "PaintThebesSingleBufferDraw",
         js::ProfileEntry::Category::GRAPHICS);
 
-      mSinglePaintDrawTarget =
-        gfxPlatform::GetPlatform()->CreateOffscreenContentDrawTarget(
-          gfx::IntSize(ceilf(bounds.width * mResolution),
-                       ceilf(bounds.height * mResolution)),
-          gfxPlatform::GetPlatform()->Optimal2DFormatForContent(
-            GetContentType()));
-
-      if (!mSinglePaintDrawTarget) {
-        return;
-      }
-
-      ctxt = new gfxContext(mSinglePaintDrawTarget);
-
-      mSinglePaintBufferOffset = nsIntPoint(bounds.x, bounds.y);
+      mCallback(mPaintedLayer, ctxt, aPaintRegion, aDirtyRegion,
+                DrawRegionClip::NONE, nsIntRegion(), mCallbackData);
     }
-    ctxt->NewPath();
-    ctxt->SetMatrix(
-      ctxt->CurrentMatrix().Scale(mResolution, mResolution).
-                            Translate(-bounds.x, -bounds.y));
-#ifdef GFX_TILEDLAYER_PREF_WARNINGS
-    if (PR_IntervalNow() - start > 3) {
-      printf_stderr("Slow alloc %i\n", PR_IntervalNow() - start);
-    }
-    start = PR_IntervalNow();
-#endif
-    PROFILER_LABEL("ClientMultiTiledLayerBuffer", "PaintThebesSingleBufferDraw",
-      js::ProfileEntry::Category::GRAPHICS);
-
-    mCallback(mPaintedLayer, ctxt, aPaintRegion, aDirtyRegion,
-              DrawRegionClip::NONE, nsIntRegion(), mCallbackData);
   }
 
 #ifdef GFX_TILEDLAYER_PREF_WARNINGS
   if (PR_IntervalNow() - start > 30) {
     const IntRect bounds = aPaintRegion.GetBounds();
     printf_stderr("Time to draw %i: %i, %i, %i, %i\n", PR_IntervalNow() - start, bounds.x, bounds.y, bounds.width, bounds.height);
     if (aPaintRegion.IsComplex()) {
       printf_stderr("Complex region\n");
@@ -1120,89 +1120,91 @@ void ClientMultiTiledLayerBuffer::Update
       // release tiles that we are not going to reuse before allocating new ones
       // to avoid allocating unnecessarily.
       oldRetainedTiles[oldIndex].DiscardBuffers();
     }
   }
 
   oldRetainedTiles.Clear();
 
-  for (size_t i = 0; i < newTileCount; ++i) {
-    const TileIntPoint tilePosition = newTiles.TilePosition(i);
-
-    IntPoint tileOffset = GetTileOffset(tilePosition);
-    nsIntRegion tileDrawRegion = IntRect(tileOffset, scaledTileSize);
-    tileDrawRegion.AndWith(aPaintRegion);
-
-    if (tileDrawRegion.IsEmpty()) {
-      continue;
-    }
-
-    TileClient& tile = mRetainedTiles[i];
-    if (!ValidateTile(tile, GetTileOffset(tilePosition), tileDrawRegion)) {
-      gfxCriticalError() << "ValidateTile failed";
-    }
-  }
-
-  if (gfxPrefs::TiledDrawTargetEnabled() && mMoz2DTiles.size() > 0) {
-    gfx::TileSet tileset;
-    for (size_t i = 0; i < mMoz2DTiles.size(); ++i) {
-      mMoz2DTiles[i].mTileOrigin -= mTilingOrigin;
-    }
-    tileset.mTiles = &mMoz2DTiles[0];
-    tileset.mTileCount = mMoz2DTiles.size();
-    RefPtr<DrawTarget> drawTarget = gfx::Factory::CreateTiledDrawTarget(tileset);
-    drawTarget->SetTransform(Matrix());
+  if (!aPaintRegion.IsEmpty()) {
+    for (size_t i = 0; i < newTileCount; ++i) {
+      const TileIntPoint tilePosition = newTiles.TilePosition(i);
 
-    RefPtr<gfxContext> ctx = new gfxContext(drawTarget);
-    ctx->SetMatrix(
-      ctx->CurrentMatrix().Scale(mResolution, mResolution).Translate(ThebesPoint(-mTilingOrigin)));
-
-    mCallback(mPaintedLayer, ctx, aPaintRegion, aDirtyRegion,
-              DrawRegionClip::DRAW, nsIntRegion(), mCallbackData);
-    mMoz2DTiles.clear();
-    // Reset:
-    mTilingOrigin = IntPoint(std::numeric_limits<int32_t>::max(),
-                             std::numeric_limits<int32_t>::max());
-  }
-
-  bool edgePaddingEnabled = gfxPrefs::TileEdgePaddingEnabled();
-
-  for (uint32_t i = 0; i < mRetainedTiles.Length(); ++i) {
-    TileClient& tile = mRetainedTiles[i];
-
-    // Only worry about padding when not doing low-res because it simplifies
-    // the math and the artifacts won't be noticable
-    // Edge padding prevents sampling artifacts when compositing.
-    if (edgePaddingEnabled && mResolution == 1 &&
-        tile.mFrontBuffer && tile.mFrontBuffer->IsLocked()) {
-
-      const TileIntPoint tilePosition = newTiles.TilePosition(i);
       IntPoint tileOffset = GetTileOffset(tilePosition);
-      // Strictly speakig we want the unscaled rect here, but it doesn't matter
-      // because we only run this code when the resolution is equal to 1.
-      IntRect tileRect = IntRect(tileOffset.x, tileOffset.y,
-                                 GetTileSize().width, GetTileSize().height);
-
       nsIntRegion tileDrawRegion = IntRect(tileOffset, scaledTileSize);
       tileDrawRegion.AndWith(aPaintRegion);
 
-      nsIntRegion tileValidRegion = mValidRegion;
-      tileValidRegion.OrWith(tileDrawRegion);
+      if (tileDrawRegion.IsEmpty()) {
+        continue;
+      }
 
-      // We only need to pad out if the tile has area that's not valid
-      if (!tileValidRegion.Contains(tileRect)) {
-        tileValidRegion = tileValidRegion.Intersect(tileRect);
-        // translate the region into tile space and pad
-        tileValidRegion.MoveBy(-IntPoint(tileOffset.x, tileOffset.y));
-        RefPtr<DrawTarget> drawTarget = tile.mFrontBuffer->BorrowDrawTarget();
-        PadDrawTargetOutFromRegion(drawTarget, tileValidRegion);
+      TileClient& tile = mRetainedTiles[i];
+      if (!ValidateTile(tile, GetTileOffset(tilePosition), tileDrawRegion)) {
+        gfxCriticalError() << "ValidateTile failed";
       }
     }
-    UnlockTile(tile);
+
+    if (gfxPrefs::TiledDrawTargetEnabled() && mMoz2DTiles.size() > 0) {
+      gfx::TileSet tileset;
+      for (size_t i = 0; i < mMoz2DTiles.size(); ++i) {
+        mMoz2DTiles[i].mTileOrigin -= mTilingOrigin;
+      }
+      tileset.mTiles = &mMoz2DTiles[0];
+      tileset.mTileCount = mMoz2DTiles.size();
+      RefPtr<DrawTarget> drawTarget = gfx::Factory::CreateTiledDrawTarget(tileset);
+      drawTarget->SetTransform(Matrix());
+
+      RefPtr<gfxContext> ctx = new gfxContext(drawTarget);
+      ctx->SetMatrix(
+        ctx->CurrentMatrix().Scale(mResolution, mResolution).Translate(ThebesPoint(-mTilingOrigin)));
+
+      mCallback(mPaintedLayer, ctx, aPaintRegion, aDirtyRegion,
+                DrawRegionClip::DRAW, nsIntRegion(), mCallbackData);
+      mMoz2DTiles.clear();
+      // Reset:
+      mTilingOrigin = IntPoint(std::numeric_limits<int32_t>::max(),
+                               std::numeric_limits<int32_t>::max());
+    }
+
+    bool edgePaddingEnabled = gfxPrefs::TileEdgePaddingEnabled();
+
+    for (uint32_t i = 0; i < mRetainedTiles.Length(); ++i) {
+      TileClient& tile = mRetainedTiles[i];
+
+      // Only worry about padding when not doing low-res because it simplifies
+      // the math and the artifacts won't be noticable
+      // Edge padding prevents sampling artifacts when compositing.
+      if (edgePaddingEnabled && mResolution == 1 &&
+          tile.mFrontBuffer && tile.mFrontBuffer->IsLocked()) {
+
+        const TileIntPoint tilePosition = newTiles.TilePosition(i);
+        IntPoint tileOffset = GetTileOffset(tilePosition);
+        // Strictly speakig we want the unscaled rect here, but it doesn't matter
+        // because we only run this code when the resolution is equal to 1.
+        IntRect tileRect = IntRect(tileOffset.x, tileOffset.y,
+                                   GetTileSize().width, GetTileSize().height);
+
+        nsIntRegion tileDrawRegion = IntRect(tileOffset, scaledTileSize);
+        tileDrawRegion.AndWith(aPaintRegion);
+
+        nsIntRegion tileValidRegion = mValidRegion;
+        tileValidRegion.OrWith(tileDrawRegion);
+
+        // We only need to pad out if the tile has area that's not valid
+        if (!tileValidRegion.Contains(tileRect)) {
+          tileValidRegion = tileValidRegion.Intersect(tileRect);
+          // translate the region into tile space and pad
+          tileValidRegion.MoveBy(-IntPoint(tileOffset.x, tileOffset.y));
+          RefPtr<DrawTarget> drawTarget = tile.mFrontBuffer->BorrowDrawTarget();
+          PadDrawTargetOutFromRegion(drawTarget, tileValidRegion);
+        }
+      }
+      UnlockTile(tile);
+    }
   }
 
   mTiles = newTiles;
   mValidRegion = newValidRegion;
   mPaintedRegion.OrWith(aPaintRegion);
 }
 
 bool