Bug 1137203 - Cleanup to ditch the fast-path code entirely and just prevent progressive drawing in the equivalent scenarios. r=BenWa
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 26 Feb 2015 17:45:37 -0500
changeset 247009 da60b4e6c98bcd11e640e733242852296b64e66b
parent 247008 2054e4cb26ff054c88118d1d4817a568cf6a593a
child 247010 22fafa8806c8d547658ef94b4ee043d83b6172c3
push id884
push userdburns@mozilla.com
push dateTue, 03 Mar 2015 15:29:12 +0000
reviewersBenWa
bugs1137203
milestone39.0a1
Bug 1137203 - Cleanup to ditch the fast-path code entirely and just prevent progressive drawing in the equivalent scenarios. r=BenWa
gfx/layers/client/ClientTiledPaintedLayer.cpp
gfx/layers/client/ClientTiledPaintedLayer.h
--- a/gfx/layers/client/ClientTiledPaintedLayer.cpp
+++ b/gfx/layers/client/ClientTiledPaintedLayer.cpp
@@ -224,68 +224,58 @@ ClientTiledPaintedLayer::IsScrollingOnCo
                               aParentMetrics.GetScrollOffset().x,
                               COORDINATE_EPSILON) ||
          !FuzzyEqualsAdditive(compositorMetrics.GetScrollOffset().y,
                               aParentMetrics.GetScrollOffset().y,
                               COORDINATE_EPSILON);
 }
 
 bool
-ClientTiledPaintedLayer::UseFastPath()
-{
-  LayerMetricsWrapper scrollAncestor;
-  bool hasTransformAnimation;
-  GetAncestorLayers(&scrollAncestor, nullptr, &hasTransformAnimation);
-  // If there is no scroll ancestor or if this layer is subject to OMTA then
-  // we effectively don't have a displayport and will just be drawing the
-  // entire visible region of the layer, so we can use the fast-path and be
-  // done with it. In particular we don't want to be drawing OMTA layers
-  // progressively as they might already be animating before we're done drawing.
-  if (!scrollAncestor || hasTransformAnimation) {
-    return true;
+ClientTiledPaintedLayer::UseProgressiveDraw() {
+  if (!gfxPlatform::GetPlatform()->UseProgressivePaint()) {
+    // pref is disabled, so never do progressive
+    return false;
   }
 
-  // The fast path doesn't allow rendering at low resolution. It will draw the low-res
-  // area at full resolution and cause OOM.
-  if (gfxPrefs::UseLowPrecisionBuffer()) {
+  if (ClientManager()->HasShadowTarget()) {
+    // This condition is true when we are in a reftest scenario. We don't want
+    // to draw progressively here because it can cause intermittent reftest
+    // failures because the harness won't wait for all the tiles to be drawn.
     return false;
   }
 
-  const FrameMetrics& parentMetrics = scrollAncestor.Metrics();
-
-  bool multipleTransactionsNeeded = gfxPlatform::GetPlatform()->UseProgressivePaint()
-                                 || !parentMetrics.GetCriticalDisplayPort().IsEmpty();
-  bool isFixed = GetIsFixedPosition() || GetParent()->GetIsFixedPosition();
-  bool isScrollable = parentMetrics.IsScrollable();
+  if (mPaintData.mCriticalDisplayPort.IsEmpty()) {
+    // This catches three scenarios:
+    // 1) This layer doesn't have a scrolling ancestor
+    // 2) This layer is subject to OMTA transforms
+    // 3) Low-precision painting is disabled
+    // In all of these cases, we don't want to draw this layer progressively.
+    return false;
+  }
 
-  return !multipleTransactionsNeeded || isFixed || !isScrollable;
-}
-
-bool
-ClientTiledPaintedLayer::UseProgressiveDraw() {
-  // Don't draw progressively in a reftest scenario (that's what the HasShadowTarget() check is for).
-  if (!gfxPlatform::GetPlatform()->UseProgressivePaint() || ClientManager()->HasShadowTarget()) {
+  if (GetIsFixedPosition() || GetParent()->GetIsFixedPosition()) {
+    // This layer is fixed-position and so even if it does have a scrolling
+    // ancestor it will likely be entirely on-screen all the time, so we
+    // should draw it all at once
     return false;
   }
 
   // XXX We probably want to disable progressive drawing for non active APZ layers in the future
   //     but we should wait for a proper test case before making this change.
-
 #if 0 //!defined(MOZ_WIDGET_ANDROID) || defined(MOZ_ANDROID_APZ)
   LayerMetricsWrapper scrollAncestor;
-  GetAncestorLayers(&scrollAncestor, nullptr);
-  if (!scrollAncestor) {
-    return true;
-  }
+  GetAncestorLayers(&scrollAncestor, nullptr, nullptr);
+  MOZ_ASSERT(scrollAncestor); // because mPaintData.mCriticalDisplayPort is non-empty
   const FrameMetrics& parentMetrics = scrollAncestor.Metrics();
+  if (!IsScrollingOnCompositor(parentMetrics)) {
+    return false;
+  }
+#endif
 
-  return !IsScrollingOnCompositor(parentMetrics);
-#else
   return true;
-#endif
 }
 
 bool
 ClientTiledPaintedLayer::RenderHighPrecision(nsIntRegion& aInvalidRegion,
                                             const nsIntRegion& aVisibleRegion,
                                             LayerManager::DrawPaintedLayerCallback aCallback,
                                             void* aCallbackData)
 {
@@ -455,26 +445,16 @@ ClientTiledPaintedLayer::RenderLayer()
   }
 
   if (!ClientManager()->IsRepeatTransaction()) {
     // Only paint the mask layer on the first transaction.
     if (GetMaskLayer()) {
       ToClientLayer(GetMaskLayer())->RenderLayer();
     }
 
-    // In some cases we can take a fast path and just be done with it.
-    if (UseFastPath()) {
-      TILING_LOG("TILING %p: Taking fast-path\n", this);
-      mValidRegion = neededRegion;
-      mContentClient->mTiledBuffer.PaintThebes(mValidRegion, invalidRegion, callback, data);
-      ClientManager()->Hold(this);
-      mContentClient->UseTiledLayerBuffer(TiledContentClient::TILED_BUFFER);
-      return;
-    }
-
     // For more complex cases we need to calculate a bunch of metrics before we
     // can do the paint.
     BeginPaint();
     if (mPaintData.mPaintFinished) {
       return;
     }
 
     // Make sure that tiles that fall outside of the visible region or outside of the
--- a/gfx/layers/client/ClientTiledPaintedLayer.h
+++ b/gfx/layers/client/ClientTiledPaintedLayer.h
@@ -92,22 +92,16 @@ private:
 
   /**
    * For the initial PaintThebes of a transaction, calculates all the data
    * needed for that paint and any repeated transactions.
    */
   void BeginPaint();
 
   /**
-   * Determine if we can use a fast path to just do a single high-precision,
-   * non-progressive paint.
-   */
-  bool UseFastPath();
-
-  /**
    * Check if the layer is being scrolled by APZ on the compositor.
    */
   bool IsScrollingOnCompositor(const FrameMetrics& aParentMetrics);
 
   /**
    * Check if we should use progressive draw on this layer. We will
    * disable progressive draw based on a preference or if the layer
    * is not being scrolled.