Bug 1200729 - Recompute FrameLayerBuilder item visibility if dirty region changes. r=mwoodrow, a=sylvestre
authorJamie Nicol <jamie@thenicols.net>
Tue, 15 Sep 2015 14:41:42 +0100
changeset 295054 a064e4cbc639f50f09d55bd05f829b17c09afed6
parent 295053 f5099905567566b2c530a7e7fef760a659dd96d9
child 295055 52f2a71f9dd49bcffbdd802ab01de519893274fe
push id5628
push usermozilla@noorenberghe.ca
push dateMon, 21 Sep 2015 23:09:25 +0000
reviewersmwoodrow, sylvestre
bugs1200729
milestone42.0a2
Bug 1200729 - Recompute FrameLayerBuilder item visibility if dirty region changes. r=mwoodrow, a=sylvestre Make the FrameLayerBuilder remember for what region it has calculated display item visibility, then recompute the visibility whenever the dirty region it is passed to DrawPaintedLayer changes. This means that the caller does not have to know the entire dirty region that will be drawn for the transaction, but we can still optimise cases where it knows some of the dirty region in advance. This fixes a regression where MultiTiledContentClient's low-res display port would not be painted if a smaller region of its high-res buffer had already been painted that transaction, since the FrameLayerBuilder had decided that most of the larger low-res region was invisible.
embedding/browser/nsWebBrowser.cpp
gfx/layers/Layers.h
gfx/layers/basic/BasicPaintedLayer.cpp
gfx/layers/basic/BasicPaintedLayer.h
gfx/layers/client/ClientPaintedLayer.cpp
gfx/layers/client/SingleTiledContentClient.cpp
gfx/layers/client/TiledContentClient.cpp
layout/base/FrameLayerBuilder.cpp
layout/base/FrameLayerBuilder.h
--- a/embedding/browser/nsWebBrowser.cpp
+++ b/embedding/browser/nsWebBrowser.cpp
@@ -1683,17 +1683,17 @@ nsWebBrowser::EnsureDocShellTreeOwner()
 
   return NS_OK;
 }
 
 static void
 DrawPaintedLayer(PaintedLayer* aLayer,
                  gfxContext* aContext,
                  const nsIntRegion& aRegionToDraw,
-                 const nsIntRegion* aDirtyRegion,
+                 const nsIntRegion& aDirtyRegion,
                  DrawRegionClip aClip,
                  const nsIntRegion& aRegionToInvalidate,
                  void* aCallbackData)
 {
   DrawTarget& aDrawTarget = *aContext->GetDrawTarget();
 
   ColorPattern color(ToDeviceColor(*static_cast<nscolor*>(aCallbackData)));
   nsIntRect dirtyRect = aRegionToDraw.GetBounds();
--- a/gfx/layers/Layers.h
+++ b/gfx/layers/Layers.h
@@ -253,21 +253,22 @@ public:
    * Function called to draw the contents of each PaintedLayer.
    * aRegionToDraw contains the region that needs to be drawn.
    * This would normally be a subregion of the visible region.
    * The callee must draw all of aRegionToDraw. Drawing outside
    * aRegionToDraw will be clipped out or ignored.
    * The callee must draw all of aRegionToDraw.
    * This region is relative to 0,0 in the PaintedLayer.
    *
-   * aDirtyRegion, if non-null, contains the total region that is due to be
-   * painted during the transaction, even though only aRegionToDraw should
-   * be drawn during this call. The sum of every aRegionToDraw over the
-   * course of the transaction must equal aDirtyRegion. aDirtyRegion can be
-   * null if the total dirty region is unknown.
+   * aDirtyRegion should contain the total region that is be due to be painted
+   * during the transaction, even though only aRegionToDraw should be drawn
+   * during this call. aRegionToDraw must be entirely contained within
+   * aDirtyRegion. If the total dirty region is unknown it is okay to pass a
+   * subregion of the total dirty region, e.g. just aRegionToDraw, though it
+   * may not be as efficient.
    *
    * aRegionToInvalidate contains a region whose contents have been
    * changed by the layer manager and which must therefore be invalidated.
    * For example, this could be non-empty if a retained layer internally
    * switches from RGBA to RGB or back ... we might want to repaint it to
    * consistently use subpixel-AA or not.
    * This region is relative to 0,0 in the PaintedLayer.
    * aRegionToInvalidate may contain areas that are outside
@@ -279,17 +280,17 @@ public:
    * We guarantee that buffered contents in the visible
    * region are valid once drawing is complete.
    *
    * The origin of aContext is 0,0 in the PaintedLayer.
    */
   typedef void (* DrawPaintedLayerCallback)(PaintedLayer* aLayer,
                                            gfxContext* aContext,
                                            const nsIntRegion& aRegionToDraw,
-                                           const nsIntRegion* aDirtyRegion,
+                                           const nsIntRegion& aDirtyRegion,
                                            DrawRegionClip aClip,
                                            const nsIntRegion& aRegionToInvalidate,
                                            void* aCallbackData);
 
   /**
    * Finish the construction phase of the transaction, perform the
    * drawing phase, and end the transaction.
    * During the drawing phase, all PaintedLayers in the tree are
--- a/gfx/layers/basic/BasicPaintedLayer.cpp
+++ b/gfx/layers/basic/BasicPaintedLayer.cpp
@@ -86,17 +86,17 @@ BasicPaintedLayer::PaintThebes(gfxContex
                                             &needsClipToVisibleRegion);
         if (effectiveOperator != CompositionOp::OP_OVER) {
           needsClipToVisibleRegion = true;
         }
       } else {
         groupContext = aContext;
       }
       SetAntialiasingFlags(this, groupContext->GetDrawTarget());
-      aCallback(this, groupContext, toDraw, &toDraw,
+      aCallback(this, groupContext, toDraw, toDraw,
                 DrawRegionClip::NONE, nsIntRegion(), aCallbackData);
       if (needsGroup) {
         aContext->PopGroupToSource();
         if (needsClipToVisibleRegion) {
           gfxUtils::ClipToRegion(aContext, toDraw);
         }
         AutoSetOperator setOptimizedOperator(aContext, ThebesOp(effectiveOperator));
         PaintWithMask(aContext, opacity, aMaskLayer);
--- a/gfx/layers/basic/BasicPaintedLayer.h
+++ b/gfx/layers/basic/BasicPaintedLayer.h
@@ -107,17 +107,17 @@ protected:
               DrawRegionClip aClip,
               LayerManager::DrawPaintedLayerCallback aCallback,
               void* aCallbackData)
   {
     if (!aCallback) {
       BasicManager()->SetTransactionIncomplete();
       return;
     }
-    aCallback(this, aContext, aExtendedRegionToDraw, &aExtendedRegionToDraw,
+    aCallback(this, aContext, aExtendedRegionToDraw, aExtendedRegionToDraw,
               aClip, aRegionToInvalidate, aCallbackData);
     // Everything that's visible has been validated. Do this instead of just
     // OR-ing with aRegionToDraw, since that can lead to a very complex region
     // here (OR doesn't automatically simplify to the simplest possible
     // representation of a region.)
     nsIntRegion tmp;
     tmp.Or(mVisibleRegion, aExtendedRegionToDraw);
     mValidRegion.Or(mValidRegion, tmp);
--- a/gfx/layers/client/ClientPaintedLayer.cpp
+++ b/gfx/layers/client/ClientPaintedLayer.cpp
@@ -82,17 +82,17 @@ ClientPaintedLayer::PaintThebes()
   while (DrawTarget* target = mContentClient->BorrowDrawTargetForPainting(state, &iter)) {
     SetAntialiasingFlags(this, target);
 
     nsRefPtr<gfxContext> ctx = gfxContext::ContextForDrawTarget(target);
 
     ClientManager()->GetPaintedLayerCallback()(this,
                                               ctx,
                                               iter.mDrawRegion,
-                                              nullptr,
+                                              iter.mDrawRegion,
                                               state.mClip,
                                               state.mRegionToInvalidate,
                                               ClientManager()->GetPaintedLayerCallbackData());
 
     ctx = nullptr;
     mContentClient->ReturnDrawTargetToBuffer(target);
     didUpdate = true;
   }
--- a/gfx/layers/client/SingleTiledContentClient.cpp
+++ b/gfx/layers/client/SingleTiledContentClient.cpp
@@ -176,17 +176,17 @@ ClientSingleTiledLayerBuffer::PaintThebe
     dt = Factory::CreateDualDrawTarget(dt, dtOnWhite);
     dtOnWhite = nullptr;
   }
 
   {
     nsRefPtr<gfxContext> ctx = new gfxContext(dt);
     ctx->SetMatrix(ctx->CurrentMatrix().Translate(-mTilingOrigin.x, -mTilingOrigin.y));
 
-    aCallback(mPaintedLayer, ctx, paintRegion, &paintRegion, DrawRegionClip::DRAW, nsIntRegion(), aCallbackData);
+    aCallback(mPaintedLayer, ctx, paintRegion, paintRegion, DrawRegionClip::DRAW, nsIntRegion(), aCallbackData);
   }
 
   // Mark the area we just drew into the back buffer as invalid in the front buffer as they're
   // now out of sync.
   mTile.mInvalidFront.OrWith(tileDirtyRegion);
 
   // The new buffer is now validated, remove the dirty region from it.
   mTile.mInvalidBack.SubOut(tileDirtyRegion);
--- a/gfx/layers/client/TiledContentClient.cpp
+++ b/gfx/layers/client/TiledContentClient.cpp
@@ -959,17 +959,17 @@ ClientMultiTiledLayerBuffer::PaintThebes
     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,
+    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()) {
@@ -1166,17 +1166,17 @@ void ClientMultiTiledLayerBuffer::Update
     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,
+    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();
--- a/layout/base/FrameLayerBuilder.cpp
+++ b/layout/base/FrameLayerBuilder.cpp
@@ -1362,18 +1362,17 @@ class PaintedDisplayItemLayerUserData : 
 public:
   PaintedDisplayItemLayerUserData() :
     mMaskClipCount(0),
     mForcedBackgroundColor(NS_RGBA(0,0,0,0)),
     mFontSmoothingBackgroundColor(NS_RGBA(0,0,0,0)),
     mXScale(1.f), mYScale(1.f),
     mAppUnitsPerDevPixel(0),
     mTranslation(0, 0),
-    mAnimatedGeometryRootPosition(0, 0),
-    mNeedsRecomputeVisibility(false) {}
+    mAnimatedGeometryRootPosition(0, 0) {}
 
   /**
    * Record the number of clips in the PaintedLayer's mask layer.
    * Should not be reset when the layer is recycled since it is used to track
    * changes in the use of mask layers.
    */
   uint32_t mMaskClipCount;
 
@@ -1435,20 +1434,22 @@ public:
   // calls to layer->InvalidateRegion during DLBI. Instead, the parts outside
   // this rectangle will be invalidated in InvalidateVisibleBoundsChangesForScrolledLayer.
   // See the comment in ComputeAndSetIgnoreInvalidationRect for more information.
   Maybe<nsIntRect> mIgnoreInvalidationsOutsideRect;
 
   nsRefPtr<ColorLayer> mColorLayer;
   nsRefPtr<ImageLayer> mImageLayer;
 
-  // True if the display items for this layer have changed and we need a call
-  // to RecomputeVisibilityForItems before painting them. This can be false
-  // during the latter iterations of progressive painting.
-  bool mNeedsRecomputeVisibility;
+  // The region for which display item visibility for this layer has already
+  // been calculated. Used to reduce the number of calls to
+  // RecomputeVisibilityForItems if it is known in advance that a larger
+  // region will be painted during a transaction than in a single call to
+  // DrawPaintedLayer, for example when progressive paint is enabled.
+  nsIntRegion mVisibilityComputedRegion;
 };
 
 /*
  * User data for layers which will be used as masks.
  */
 struct MaskLayerUserData : public LayerUserData
 {
   MaskLayerUserData()
@@ -2280,17 +2281,17 @@ ContainerState::PreparePaintedLayerForUs
                        RoundToMatchResidual(scaledOffset.y, aData->mAnimatedGeometryRootPosition.y));
   aData->mTranslation = pixOffset;
   pixOffset += mParameters.mOffset;
   Matrix matrix = Matrix::Translation(pixOffset.x, pixOffset.y);
   aLayer->SetBaseTransform(Matrix4x4::From2D(matrix));
 
   ComputeAndSetIgnoreInvalidationRect(aLayer, aData, aAnimatedGeometryRoot, mBuilder, pixOffset);
 
-  aData->mNeedsRecomputeVisibility = true;
+  aData->mVisibilityComputedRegion.SetEmpty();
 
   // FIXME: Temporary workaround for bug 681192 and bug 724786.
 #ifndef MOZ_WIDGET_ANDROID
   // Calculate exact position of the top-left of the active scrolled root.
   // This might not be 0,0 due to the snapping in ScaleToNearestPixels.
   gfxPoint animatedGeometryRootTopLeft = scaledOffset - ThebesPoint(matrix.GetTranslation()) + mParameters.mOffset;
   // If it has changed, then we need to invalidate the entire layer since the
   // pixels in the layer buffer have the content at a (subpixel) offset
@@ -5542,17 +5543,17 @@ private:
  * and ceil(xmost) >= snap(xmost + r), so the rendering of snapped content
  * always falls within the visible region we computed.
  */
 
 /* static */ void
 FrameLayerBuilder::DrawPaintedLayer(PaintedLayer* aLayer,
                                    gfxContext* aContext,
                                    const nsIntRegion& aRegionToDraw,
-                                   const nsIntRegion* aDirtyRegion,
+                                   const nsIntRegion& aDirtyRegion,
                                    DrawRegionClip aClip,
                                    const nsIntRegion& aRegionToInvalidate,
                                    void* aCallbackData)
 {
   DrawTarget& aDrawTarget = *aContext->GetDrawTarget();
 
   PROFILER_LABEL("FrameLayerBuilder", "DrawPaintedLayer",
     js::ProfileEntry::Category::GRAPHICS);
@@ -5595,31 +5596,28 @@ FrameLayerBuilder::DrawPaintedLayer(Pain
   }
 
   // make the origin of the context coincide with the origin of the
   // PaintedLayer
   gfxContextMatrixAutoSaveRestore saveMatrix(aContext);
   nsIntPoint offset = GetTranslationForPaintedLayer(aLayer);
   nsPresContext* presContext = entry->mContainerLayerFrame->PresContext();
 
-  if (userData->mNeedsRecomputeVisibility &&
+  if (!userData->mVisibilityComputedRegion.Contains(aDirtyRegion) &&
       !layerBuilder->GetContainingPaintedLayerData()) {
-    // Recompute visibility of items in our PaintedLayer. Note that this
-    // recomputes visibility for all descendants of our display items too,
-    // so there's no need to do this for the items in inactive PaintedLayers.
-    // If aDirtyRegion is non-null then recompute the visibility of the entire
-    // aDirtyRegion at once, rather of aRegionToDraw separately on each call.
+    // Recompute visibility of items in our PaintedLayer, if required. Note
+    // that this recomputes visibility for all descendants of our display
+    // items too, so there's no need to do this for the items in inactive
+    // PaintedLayers. If aDirtyRegion has not changed since the previous call
+    // then we can skip this.
     int32_t appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel();
-    RecomputeVisibilityForItems(entry->mItems, builder,
-                                aDirtyRegion ? *aDirtyRegion : aRegionToDraw,
+    RecomputeVisibilityForItems(entry->mItems, builder, aDirtyRegion,
                                 offset, appUnitsPerDevPixel,
                                 userData->mXScale, userData->mYScale);
-    if (aDirtyRegion) {
-      userData->mNeedsRecomputeVisibility = false;
-    }
+    userData->mVisibilityComputedRegion = aDirtyRegion;
   }
 
   nsRenderingContext rc(aContext);
 
   if (shouldDrawRectsSeparately) {
     nsIntRegionRectIterator it(aRegionToDraw);
     while (const nsIntRect* iterRect = it.Next()) {
       gfxContextAutoSaveRestore save(aContext);
--- a/layout/base/FrameLayerBuilder.h
+++ b/layout/base/FrameLayerBuilder.h
@@ -277,24 +277,24 @@ public:
    */
   static Layer* GetDedicatedLayer(nsIFrame* aFrame, uint32_t aDisplayItemKey);
 
   /**
    * This callback must be provided to EndTransaction. The callback data
    * must be the nsDisplayListBuilder containing this FrameLayerBuilder.
    * This function can be called multiple times in a row to draw
    * different regions. This will occur when, for example, progressive paint is
-   * enabled. In these cases aDirtyRegion can optionally be used to specify the
-   * total region that will be drawn during the transaction, possibly allowing
-   * the callback to make optimizations.
+   * enabled. In these cases aDirtyRegion can be used to specify a larger region
+   * than aRegionToDraw that will be drawn during the transaction, possibly
+   * allowing the callback to make optimizations.
    */
   static void DrawPaintedLayer(PaintedLayer* aLayer,
                               gfxContext* aContext,
                               const nsIntRegion& aRegionToDraw,
-                              const nsIntRegion* aDirtyRegion,
+                              const nsIntRegion& aDirtyRegion,
                               mozilla::layers::DrawRegionClip aClip,
                               const nsIntRegion& aRegionToInvalidate,
                               void* aCallbackData);
 
   /**
    * Dumps this FrameLayerBuilder's retained layer manager's retained
    * layer tree. Defaults to dumping to stdout in non-HTML format.
    */