Bug 1250517 - Differentiate between no critical display port and empty critical display port in ClientTiledPaintedLayer; r=kats
authorJamie Nicol <jnicol@mozilla.com>
Tue, 23 Feb 2016 15:38:29 +0000
changeset 285333 00cda66bd40083d8e2e14980a50090cc7cd8ab02
parent 285332 b18eacf1d5c935bf2084894a2f7f8fab35cc3730
child 285334 cfa41433ed01a8dba9572f1d86ab6213fd349324
push id72325
push usercbook@mozilla.com
push dateWed, 24 Feb 2016 14:28:49 +0000
treeherdermozilla-inbound@00cda66bd400 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1250517
milestone47.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 1250517 - Differentiate between no critical display port and empty critical display port in ClientTiledPaintedLayer; r=kats Currently the logic in ClientTiledPaintedLayer treats an empty critical display port to mean that there is no critical display port, i.e. that the entire visible region should be painted. However, the critical displayport should be, and is, empty if either the display port or composition bounds are entirely outwith the layer's bounds. We want to render none of the layer in this case, not all of it. Change BasicTiledLayerPaintData::mCriticalDisplayPort's type to a Maybe<LayerIntRect>, and differentiate between it being not set and it being an empty rect. MozReview-Commit-ID: Gi1iZOQcOVL
gfx/layers/client/ClientTiledPaintedLayer.cpp
gfx/layers/client/TiledContentClient.cpp
gfx/layers/client/TiledContentClient.h
--- a/gfx/layers/client/ClientTiledPaintedLayer.cpp
+++ b/gfx/layers/client/ClientTiledPaintedLayer.cpp
@@ -173,40 +173,42 @@ ClientTiledPaintedLayer::BeginPaint()
   // what parts of it might move into view in the compositor.
   if (!hasTransformAnimation &&
       mContentClient->GetLowPrecisionTiledBuffer()) {
     ParentLayerRect criticalDisplayPort =
       (displayportMetrics.GetCriticalDisplayPort() * displayportMetrics.GetZoom())
       + displayportMetrics.GetCompositionBounds().TopLeft();
     Maybe<LayerRect> criticalDisplayPortTransformed =
       ApplyParentLayerToLayerTransform(transformDisplayPortToLayer, criticalDisplayPort, layerBounds);
-    if (!criticalDisplayPortTransformed) {
-      mPaintData.ResetPaintData();
-      return;
+    if (criticalDisplayPortTransformed) {
+      mPaintData.mCriticalDisplayPort = Some(RoundedToInt(*criticalDisplayPortTransformed));
+    } else {
+      mPaintData.mCriticalDisplayPort = Some(LayerIntRect(0, 0, 0, 0));
     }
-    mPaintData.mCriticalDisplayPort = RoundedToInt(*criticalDisplayPortTransformed);
   }
-  TILING_LOG("TILING %p: Critical displayport %s\n", this, Stringify(mPaintData.mCriticalDisplayPort).c_str());
+  TILING_LOG("TILING %p: Critical displayport %s\n", this,
+             mPaintData.mCriticalDisplayPort ?
+             Stringify(*mPaintData.mCriticalDisplayPort).c_str() : "not set");
 
   // Store the resolution from the displayport ancestor layer. Because this is Gecko-side,
   // before any async transforms have occurred, we can use the zoom for this.
   mPaintData.mResolution = displayportMetrics.GetZoom();
   TILING_LOG("TILING %p: Resolution %s\n", this, Stringify(mPaintData.mResolution).c_str());
 
   // Store the applicable composition bounds in this layer's Layer units.
   mPaintData.mTransformToCompBounds =
     GetTransformToAncestorsParentLayer(this, scrollAncestor);
   ParentLayerToLayerMatrix4x4 transformToBounds = mPaintData.mTransformToCompBounds.Inverse();
   Maybe<LayerRect> compositionBoundsTransformed = ApplyParentLayerToLayerTransform(
     transformToBounds, scrollMetrics.GetCompositionBounds(), layerBounds);
-  if (!compositionBoundsTransformed) {
-    mPaintData.ResetPaintData();
-    return;
+  if (compositionBoundsTransformed) {
+    mPaintData.mCompositionBounds = *compositionBoundsTransformed;
+  } else {
+    mPaintData.mCompositionBounds.SetEmpty();
   }
-  mPaintData.mCompositionBounds = *compositionBoundsTransformed;
   TILING_LOG("TILING %p: Composition bounds %s\n", this, Stringify(mPaintData.mCompositionBounds).c_str());
 
   // Calculate the scroll offset since the last transaction
   mPaintData.mScrollOffset = displayportMetrics.GetScrollOffset() * displayportMetrics.GetZoom();
   TILING_LOG("TILING %p: Scroll offset %s\n", this, Stringify(mPaintData.mScrollOffset).c_str());
 }
 
 bool
@@ -252,17 +254,17 @@ ClientTiledPaintedLayer::UseProgressiveD
 
   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;
   }
 
-  if (mPaintData.mCriticalDisplayPort.IsEmpty()) {
+  if (!mPaintData.mCriticalDisplayPort) {
     // 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;
   }
 
@@ -271,17 +273,17 @@ ClientTiledPaintedLayer::UseProgressiveD
     // ancestor it will likely be entirely on-screen all the time, so we
     // should draw it all at once
     return false;
   }
 
   if (ClientManager()->AsyncPanZoomEnabled()) {
     LayerMetricsWrapper scrollAncestor;
     GetAncestorLayers(&scrollAncestor, nullptr, nullptr);
-    MOZ_ASSERT(scrollAncestor); // because mPaintData.mCriticalDisplayPort is non-empty
+    MOZ_ASSERT(scrollAncestor); // because mPaintData.mCriticalDisplayPort is set
     const FrameMetrics& parentMetrics = scrollAncestor.Metrics();
     if (!IsScrollingOnCompositor(parentMetrics)) {
       return false;
     }
   }
 
   return true;
 }
@@ -301,31 +303,31 @@ ClientTiledPaintedLayer::RenderHighPreci
   // Only draw progressively when the resolution is unchanged
   if (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.IsEmpty()) {
-      oldValidRegion.And(oldValidRegion, mPaintData.mCriticalDisplayPort.ToUnknownRect());
+    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
 
   mValidRegion = aVisibleRegion;
-  if (!mPaintData.mCriticalDisplayPort.IsEmpty()) {
-    mValidRegion.And(mValidRegion, mPaintData.mCriticalDisplayPort.ToUnknownRect());
+  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());
 
   mContentClient->GetTiledBuffer()->SetFrameResolution(mPaintData.mResolution);
   mContentClient->GetTiledBuffer()->PaintThebes(mValidRegion, aInvalidRegion, aInvalidRegion,
                                                 aCallback, aCallbackData);
@@ -336,17 +338,18 @@ ClientTiledPaintedLayer::RenderHighPreci
 bool
 ClientTiledPaintedLayer::RenderLowPrecision(nsIntRegion& aInvalidRegion,
                                            const nsIntRegion& aVisibleRegion,
                                            LayerManager::DrawPaintedLayerCallback aCallback,
                                            void* aCallbackData)
 {
   // Render the low precision buffer, if the visible region is larger than the
   // critical display port.
-  if (!nsIntRegion(mPaintData.mCriticalDisplayPort.ToUnknownRect()).Contains(aVisibleRegion)) {
+  if (!mPaintData.mCriticalDisplayPort ||
+      !nsIntRegion(mPaintData.mCriticalDisplayPort->ToUnknownRect()).Contains(aVisibleRegion)) {
     nsIntRegion oldValidRegion = mContentClient->GetLowPrecisionTiledBuffer()->GetValidRegion();
     oldValidRegion.And(oldValidRegion, aVisibleRegion);
 
     bool updatedBuffer = false;
 
     // If the frame resolution or format have changed, invalidate the buffer
     if (mContentClient->GetLowPrecisionTiledBuffer()->GetFrameResolution() != mPaintData.mResolution ||
         mContentClient->GetLowPrecisionTiledBuffer()->HasFormatChanged()) {
@@ -490,26 +493,26 @@ ClientTiledPaintedLayer::RenderLayer()
     if (mPaintData.mPaintFinished) {
       return;
     }
 
     // Make sure that tiles that fall outside of the visible region or outside of the
     // critical displayport are discarded on the first update. Also make sure that we
     // only draw stuff inside the critical displayport on the first update.
     mValidRegion.And(mValidRegion, neededRegion);
-    if (!mPaintData.mCriticalDisplayPort.IsEmpty()) {
-      mValidRegion.And(mValidRegion, mPaintData.mCriticalDisplayPort.ToUnknownRect());
-      invalidRegion.And(invalidRegion, mPaintData.mCriticalDisplayPort.ToUnknownRect());
+    if (mPaintData.mCriticalDisplayPort) {
+      mValidRegion.And(mValidRegion, mPaintData.mCriticalDisplayPort->ToUnknownRect());
+      invalidRegion.And(invalidRegion, mPaintData.mCriticalDisplayPort->ToUnknownRect());
     }
 
     TILING_LOG("TILING %p: First-transaction valid region %s\n", this, Stringify(mValidRegion).c_str());
     TILING_LOG("TILING %p: First-transaction invalid region %s\n", this, Stringify(invalidRegion).c_str());
   } else {
-    if (!mPaintData.mCriticalDisplayPort.IsEmpty()) {
-      invalidRegion.And(invalidRegion, mPaintData.mCriticalDisplayPort.ToUnknownRect());
+    if (mPaintData.mCriticalDisplayPort) {
+      invalidRegion.And(invalidRegion, mPaintData.mCriticalDisplayPort->ToUnknownRect());
     }
     TILING_LOG("TILING %p: Repeat-transaction invalid region %s\n", this, Stringify(invalidRegion).c_str());
   }
 
   nsIntRegion lowPrecisionInvalidRegion;
   if (mContentClient->GetLowPrecisionTiledBuffer()) {
     // Calculate the invalid region for the low precision buffer. Make sure
     // to remove the valid high-precision area so we don't double-paint it.
--- a/gfx/layers/client/TiledContentClient.cpp
+++ b/gfx/layers/client/TiledContentClient.cpp
@@ -1691,13 +1691,13 @@ TiledContentClient::Dump(std::stringstre
 }
 
 void
 BasicTiledLayerPaintData::ResetPaintData()
 {
   mLowPrecisionPaintCount = 0;
   mPaintFinished = false;
   mCompositionBounds.SetEmpty();
-  mCriticalDisplayPort.SetEmpty();
+  mCriticalDisplayPort = Nothing();
 }
 
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/client/TiledContentClient.h
+++ b/gfx/layers/client/TiledContentClient.h
@@ -313,20 +313,20 @@ struct BasicTiledLayerPaintData {
    * the scroll ancestor's ParentLayer units. The "scroll ancestor" is
    * the closest ancestor layer which scrolls, and is used to obtain
    * the composition bounds that are relevant for this layer.
    */
   LayerToParentLayerMatrix4x4 mTransformToCompBounds;
 
   /*
    * The critical displayport of the content from the nearest ancestor layer
-   * that represents scrollable content with a display port set. Empty if a
-   * critical displayport is not set.
+   * that represents scrollable content with a display port set. isNothing()
+   * if a critical displayport is not set.
    */
-  LayerIntRect mCriticalDisplayPort;
+  Maybe<LayerIntRect> mCriticalDisplayPort;
 
   /*
    * The render resolution of the document that the content this layer
    * represents is in.
    */
   CSSToParentLayerScale2D mResolution;
 
   /*