Bug 1187619 - Only optmimize FrameLayerBuilder visibility calculations if correct. r=mattwoodrow
authorJamie Nicol <jnicol@mozilla.com>
Mon, 03 Aug 2015 04:07:00 -0400
changeset 287532 1ddb3a3a725577047292f98a34b4272540729f57
parent 287531 ea2c3ec477c6e1cea5111975cae0f01c5080cc00
child 287533 5a52576d240161dc532596e77856a47cc35d3b60
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1187619, 1176077
milestone42.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 1187619 - Only optmimize FrameLayerBuilder visibility calculations if correct. r=mattwoodrow Bug 1176077 introduced the parameter aDirtyRegion to DrawPaintedLayerCallback, which allows the callback to recompute the visibility of all items to be painted in that transaction in a single go. However, this parameter can not always be determined correctly when using RotatedBuffer, and using an incorrect value was causing graphical glitches. Make the parameter optional, and on null values do not perform the optimisation. Pass null from ClientPaintedLayer, which uses RotatedBuffer and was causing problems, but continue to pass the correct value from other Layer implementations. This optimisation was most important for tiled layers using progressive paint, so this is okay.
embedding/browser/nsWebBrowser.cpp
gfx/layers/Layers.h
gfx/layers/RotatedBuffer.cpp
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
@@ -1681,17 +1681,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,21 @@ 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 contains the region that is due to be painted at some point,
-   * even though only aRegionToDraw should be drawn during this call.
-   * They will often be equal, but aDirtyRegion may be larger, for example
-   * when painting progressively. aRegionToDraw must be entirely contained
-   * within aDirtyRegion.
+   * 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.
    *
    * 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 +279,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/RotatedBuffer.cpp
+++ b/gfx/layers/RotatedBuffer.cpp
@@ -730,31 +730,29 @@ RotatedContentBuffer::BorrowDrawTargetFo
   }
 
   DrawTarget* result = BorrowDrawTargetForQuadrantUpdate(aPaintState.mRegionToDraw.GetBounds(),
                                                          BUFFER_BOTH, aIter);
   if (!result) {
     return nullptr;
   }
 
-  if (result->GetBackendType() == BackendType::DIRECT2D ||
-      result->GetBackendType() == BackendType::DIRECT2D1_1) {
-    // Simplify the draw region to avoid hitting expensive drawing paths for
-    // complex regions. Must be applied to the entire draw region so that it
-    // remains a superset of the iterator's draw region.
-    aPaintState.mRegionToDraw.SimplifyOutwardByArea(100 * 100);
-  }
-
   nsIntRegion* drawPtr = &aPaintState.mRegionToDraw;
   if (aIter) {
     // The iterators draw region currently only contains the bounds of the region,
     // this makes it the precise region.
     aIter->mDrawRegion.And(aIter->mDrawRegion, aPaintState.mRegionToDraw);
     drawPtr = &aIter->mDrawRegion;
   }
+  if (result->GetBackendType() == BackendType::DIRECT2D ||
+      result->GetBackendType() == BackendType::DIRECT2D1_1) {
+    // Simplify the draw region to avoid hitting expensive drawing paths
+    // for complex regions.
+    drawPtr->SimplifyOutwardByArea(100 * 100);
+  }
 
   if (aPaintState.mMode == SurfaceMode::SURFACE_COMPONENT_ALPHA) {
     if (!mDTBuffer || !mDTBufferOnWhite) {
       // This can happen in release builds if allocating one of the two buffers
       // failed. This in turn can happen if unreasonably large textures are
       // requested.
       return nullptr;
     }
--- 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,
-                                              state.mRegionToDraw,
+                                              nullptr,
                                               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
@@ -5542,17 +5542,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);
@@ -5600,21 +5600,26 @@ FrameLayerBuilder::DrawPaintedLayer(Pain
   nsIntPoint offset = GetTranslationForPaintedLayer(aLayer);
   nsPresContext* presContext = entry->mContainerLayerFrame->PresContext();
 
   if (userData->mNeedsRecomputeVisibility &&
       !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.
     int32_t appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel();
-    RecomputeVisibilityForItems(entry->mItems, builder, aDirtyRegion,
+    RecomputeVisibilityForItems(entry->mItems, builder,
+                                aDirtyRegion ? *aDirtyRegion : aRegionToDraw,
                                 offset, appUnitsPerDevPixel,
                                 userData->mXScale, userData->mYScale);
-    userData->mNeedsRecomputeVisibility = false;
+    if (aDirtyRegion) {
+      userData->mNeedsRecomputeVisibility = false;
+    }
   }
 
   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,22 +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 which case aRegionToDraw will be a subregion of aDirtyRegion.
+   * 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.
    */
   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.
    */