Bug 1250517 - Differentiate between no critical display port and empty critical display port in ClientTiledPaintedLayer; r?kats draft
authorJamie Nicol <jnicol@mozilla.com>
Tue, 23 Feb 2016 15:38:29 +0000
changeset 333316 d54d3ad26c4e083d6e6879f392a603fb85e2d4ab
parent 332881 789a12291942763bc1e3a89f97e0b82dc1c9d00b
child 514727 8d5e1f695cf1267cd3b29c9c5beea9b4e330952f
push id11345
push userbmo:jnicol@mozilla.com
push dateTue, 23 Feb 2016 15:46:29 +0000
reviewerskats
bugs1250517
milestone47.0a1
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;
 
   /*