Bug 783368 - Fix progressive tile update coherency issues. r=bgirard
authorChris Lord <chrislord.net@gmail.com>
Wed, 21 Nov 2012 22:34:19 +0000
changeset 113950 d014526b795e76b33e9bc4db372e0a40a7c5652c
parent 113949 808c7d0aedecf74eab53ffcce3207359653e3c05
child 113951 e93e08cd1d4ce96dc2ea2ead65cb40687df0050c
push id18466
push userchrislord.net@gmail.com
push dateWed, 21 Nov 2012 22:34:39 +0000
treeherdermozilla-inbound@c4d013240eac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgirard
bugs783368
milestone20.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 783368 - Fix progressive tile update coherency issues. r=bgirard Fix some progressive tile updating coherency issues caused by aborting at inopportune times and tile draw ordering.
gfx/layers/basic/BasicTiledThebesLayer.cpp
gfx/layers/basic/BasicTiledThebesLayer.h
mobile/android/base/gfx/GeckoLayerClient.java
--- a/gfx/layers/basic/BasicTiledThebesLayer.cpp
+++ b/gfx/layers/basic/BasicTiledThebesLayer.cpp
@@ -264,64 +264,73 @@ RoundedTransformViewportBounds(const gfx
 }
 
 bool
 BasicTiledThebesLayer::ComputeProgressiveUpdateRegion(BasicTiledLayerBuffer& aTiledBuffer,
                                                       const nsIntRegion& aInvalidRegion,
                                                       const nsIntRegion& aOldValidRegion,
                                                       nsIntRegion& aRegionToPaint,
                                                       const gfx3DMatrix& aTransform,
+                                                      const nsIntRect& aCompositionBounds,
                                                       const gfx::Point& aScrollOffset,
                                                       const gfxSize& aResolution,
                                                       bool aIsRepeated)
 {
   aRegionToPaint = aInvalidRegion;
 
   // If this is a low precision buffer, we force progressive updates. The
   // assumption is that the contents is less important, so visual coherency
   // is lower priority than speed.
   bool drawingLowPrecision = aTiledBuffer.IsLowPrecision();
 
   // Find out if we have any non-stale content to update.
-  nsIntRegion freshRegion;
-  if (!mFirstPaint) {
-    freshRegion.And(aInvalidRegion, aOldValidRegion);
-    freshRegion.Sub(aInvalidRegion, freshRegion);
-  }
+  nsIntRegion staleRegion;
+  staleRegion.And(aInvalidRegion, aOldValidRegion);
 
   // Find out the current view transform to determine which tiles to draw
   // first, and see if we should just abort this paint. Aborting is usually
   // caused by there being an incoming, more relevant paint.
   gfx::Rect viewport;
   float scaleX, scaleY;
-  if (BasicManager()->ProgressiveUpdateCallback(!freshRegion.IsEmpty(), viewport,
+  if (BasicManager()->ProgressiveUpdateCallback(!staleRegion.Contains(aInvalidRegion),
+                                                viewport,
                                                 scaleX, scaleY, !drawingLowPrecision)) {
     SAMPLE_MARKER("Abort painting");
     aRegionToPaint.SetEmpty();
     return aIsRepeated;
   }
 
   // Transform the screen coordinates into local layer coordinates.
   nsIntRect roundedTransformedViewport =
     RoundedTransformViewportBounds(viewport, aScrollOffset, aResolution,
                                    scaleX, scaleY, aTransform);
 
-  // Paint tiles that have no content before tiles that only have stale content.
-  bool drawingStale = freshRegion.IsEmpty();
+  // Paint tiles that have stale content or that intersected with the screen
+  // at the time of issuing the draw command in a single transaction first.
+  // This is to avoid rendering glitches on animated page content, and when
+  // layers change size/shape.
+  nsIntRect criticalViewportRect = roundedTransformedViewport.Intersect(aCompositionBounds);
+  aRegionToPaint.And(aInvalidRegion, criticalViewportRect);
+  aRegionToPaint.Or(aRegionToPaint, staleRegion);
+  bool drawingStale = !aRegionToPaint.IsEmpty();
   if (!drawingStale) {
-    aRegionToPaint = freshRegion;
+    aRegionToPaint = aInvalidRegion;
   }
 
   // Prioritise tiles that are currently visible on the screen.
   bool paintVisible = false;
   if (aRegionToPaint.Intersects(roundedTransformedViewport)) {
     aRegionToPaint.And(aRegionToPaint, roundedTransformedViewport);
     paintVisible = true;
   }
 
+  // Paint area that's visible and overlaps previously valid content to avoid
+  // visible glitches in animated elements, such as gifs.
+  bool paintInSingleTransaction = paintVisible && (drawingStale || mFirstPaint);
+
   // The following code decides what order to draw tiles in, based on the
   // current scroll direction of the primary scrollable layer.
   NS_ASSERTION(!aRegionToPaint.IsEmpty(), "Unexpectedly empty paint region!");
   nsIntRect paintBounds = aRegionToPaint.GetBounds();
 
   int startX, incX, startY, incY;
   int tileLength = aTiledBuffer.GetScaledTileLength();
   if (aScrollOffset.x >= mLastScrollOffset.x) {
@@ -359,51 +368,54 @@ BasicTiledThebesLayer::ComputeProgressiv
     }
   }
 
   bool repeatImmediately = false;
   if (!aRegionToPaint.Contains(aInvalidRegion)) {
     // The region needed to paint is larger then our progressive chunk size
     // therefore update what we want to paint and ask for a new paint transaction.
 
-    // If we're drawing stale, visible content, make sure that it happens
-    // in one go by repeating this work without calling the painted
-    // callback. The remaining content is then drawn tile-by-tile in
-    // multiple transactions.
-    if (!drawingLowPrecision && paintVisible && drawingStale) {
+    // If we need to draw more than one tile to maintain coherency, make
+    // sure it happens in the same transaction by requesting this work be
+    // repeated immediately.
+    // If this is unnecessary, the remaining work will be done tile-by-tile in
+    // subsequent transactions.
+    if (!drawingLowPrecision && paintInSingleTransaction) {
       repeatImmediately = true;
     } else {
       BasicManager()->SetRepeatTransaction();
     }
   }
 
   return repeatImmediately;
 }
 
 bool
 BasicTiledThebesLayer::ProgressiveUpdate(BasicTiledLayerBuffer& aTiledBuffer,
                                          nsIntRegion& aValidRegion,
                                          nsIntRegion& aInvalidRegion,
                                          const nsIntRegion& aOldValidRegion,
                                          const gfx3DMatrix& aTransform,
+                                         const nsIntRect& aCompositionBounds,
                                          const gfx::Point& aScrollOffset,
                                          const gfxSize& aResolution,
                                          LayerManager::DrawThebesLayerCallback aCallback,
                                          void* aCallbackData)
 {
   bool repeat = false;
   do {
     // Compute the region that should be updated. Repeat as many times as
     // is required.
     nsIntRegion regionToPaint;
     repeat = ComputeProgressiveUpdateRegion(aTiledBuffer,
                                             aInvalidRegion,
                                             aOldValidRegion,
                                             regionToPaint,
                                             aTransform,
+                                            aCompositionBounds,
                                             aScrollOffset,
                                             aResolution,
                                             repeat);
 
     // There's no further work to be done, return if nothing has been
     // drawn, or give what has been drawn to the shadow layer to upload.
     if (regionToPaint.IsEmpty()) {
       if (repeat) {
@@ -492,22 +504,30 @@ BasicTiledThebesLayer::PaintThebes(gfxCo
 
   gfxSize resolution(1, 1);
   for (ContainerLayer* parent = GetParent(); parent; parent = parent->GetParent()) {
     const FrameMetrics& metrics = parent->GetFrameMetrics();
     resolution.width *= metrics.mResolution.width;
     resolution.height *= metrics.mResolution.height;
   }
 
-  // Calculate the scroll offset since the last transaction.
+  // Calculate the scroll offset since the last transaction, and the
+  // composition bounds.
+  nsIntRect compositionBounds;
   gfx::Point scrollOffset(0, 0);
   Layer* primaryScrollable = BasicManager()->GetPrimaryScrollableLayer();
   if (primaryScrollable) {
     const FrameMetrics& metrics = primaryScrollable->AsContainerLayer()->GetFrameMetrics();
     scrollOffset = metrics.mScrollOffset;
+    gfxRect transformedViewport = transform.TransformBounds(
+      gfxRect(metrics.mCompositionBounds.x, metrics.mCompositionBounds.y,
+              metrics.mCompositionBounds.width, metrics.mCompositionBounds.height));
+    transformedViewport.RoundOut();
+    compositionBounds = nsIntRect(transformedViewport.x, transformedViewport.y,
+                                  transformedViewport.width, transformedViewport.height);
   }
 
   // Only draw progressively when the resolution is unchanged.
   if (gfxPlatform::UseProgressiveTilePainting() &&
       !BasicManager()->HasShadowTarget() &&
       mTiledBuffer.GetFrameResolution() == resolution) {
     // Store the old valid region, then clear it before painting.
     // We clip the old valid region to the visible region, as it only gets
@@ -523,18 +543,18 @@ BasicTiledThebesLayer::PaintThebes(gfxCo
     if (!BasicManager()->IsRepeatTransaction()) {
       mValidRegion.And(mValidRegion, mVisibleRegion);
       if (!layerDisplayPort.IsEmpty()) {
         mValidRegion.And(mValidRegion, layerDisplayPort);
       }
     }
 
     if (!ProgressiveUpdate(mTiledBuffer, mValidRegion, invalidRegion,
-                           oldValidRegion, transform, scrollOffset, resolution,
-                           aCallback, aCallbackData))
+                           oldValidRegion, transform, compositionBounds,
+                           scrollOffset, resolution, aCallback, aCallbackData))
       return;
   } else {
     mTiledBuffer.SetFrameResolution(resolution);
     mValidRegion = mVisibleRegion;
     if (!layerDisplayPort.IsEmpty()) {
       mValidRegion.And(mValidRegion, layerDisplayPort);
     }
     mTiledBuffer.PaintThebes(this, mValidRegion, invalidRegion, aCallback, aCallbackData);
@@ -589,17 +609,18 @@ BasicTiledThebesLayer::PaintThebes(gfxCo
     nsIntRegion invalidHighPrecisionIntersect;
     invalidHighPrecisionIntersect.And(lowPrecisionInvalidRegion, mValidRegion);
     lowPrecisionInvalidRegion.Sub(lowPrecisionInvalidRegion, invalidHighPrecisionIntersect);
 
     if (!lowPrecisionInvalidRegion.IsEmpty()) {
       updatedLowPrecision =
         ProgressiveUpdate(mLowPrecisionTiledBuffer, mLowPrecisionValidRegion,
                           lowPrecisionInvalidRegion, oldValidRegion, transform,
-                          scrollOffset, resolution, aCallback, aCallbackData);
+                          compositionBounds, scrollOffset, resolution, aCallback,
+                          aCallbackData);
     }
 
     // Re-add the high-precision valid region intersection so that we can
     // maintain coherency when the valid region changes.
     lowPrecisionInvalidRegion.Or(lowPrecisionInvalidRegion, invalidHighPrecisionIntersect);
   } else if (!mLowPrecisionValidRegion.IsEmpty()) {
     // Clear the low precision tiled buffer
     clearedLowPrecision = true;
--- a/gfx/layers/basic/BasicTiledThebesLayer.h
+++ b/gfx/layers/basic/BasicTiledThebesLayer.h
@@ -225,41 +225,46 @@ private:
    *
    * aInvalidRegion is the current invalid region.
    * aOldValidRegion is the valid region of aTiledBuffer at the beginning of the
    * current transaction.
    * aRegionToPaint will be filled with the region to update. This may be empty,
    * which indicates that there is no more work to do.
    * aTransform is the transform required to convert from screen-space to
    * layer-space.
+   * aCompositionBounds is the composition bounds from the primary scrollable
+   * layer, transformed into layer coordinates.
    * aScrollOffset is the current scroll offset of the primary scrollable layer.
    * aResolution is the render resolution of the layer.
    * aIsRepeated should be true if this function has already been called during
    * this transaction.
    *
    * Returns true if it should be called again, false otherwise. In the case
    * that aRegionToPaint is empty, this will return aIsRepeated for convenience.
    */
   bool ComputeProgressiveUpdateRegion(BasicTiledLayerBuffer& aTiledBuffer,
                                       const nsIntRegion& aInvalidRegion,
                                       const nsIntRegion& aOldValidRegion,
                                       nsIntRegion& aRegionToPaint,
                                       const gfx3DMatrix& aTransform,
+                                      const nsIntRect& aCompositionBounds,
                                       const gfx::Point& aScrollOffset,
                                       const gfxSize& aResolution,
                                       bool aIsRepeated);
 
   /**
    * Performs a progressive update of a given tiled buffer.
+   * See ComputeProgressiveUpdateRegion above for parameter documentation.
    */
   bool ProgressiveUpdate(BasicTiledLayerBuffer& aTiledBuffer,
                          nsIntRegion& aValidRegion,
                          nsIntRegion& aInvalidRegion,
                          const nsIntRegion& aOldValidRegion,
                          const gfx3DMatrix& aTransform,
+                         const nsIntRect& aCompositionBounds,
                          const gfx::Point& aScrollOffset,
                          const gfxSize& aResolution,
                          LayerManager::DrawThebesLayerCallback aCallback,
                          void* aCallbackData);
 
   // Members
   BasicTiledLayerBuffer mTiledBuffer;
   BasicTiledLayerBuffer mLowPrecisionTiledBuffer;
--- a/mobile/android/base/gfx/GeckoLayerClient.java
+++ b/mobile/android/base/gfx/GeckoLayerClient.java
@@ -407,27 +407,16 @@ public class GeckoLayerClient
             Math.max(viewportMetrics.viewportRectTop, viewportMetrics.pageRectTop) + 1 < y ||
             Math.min(viewportMetrics.viewportRectRight, viewportMetrics.pageRectRight) - 1 > x + width ||
             Math.min(viewportMetrics.viewportRectBottom, viewportMetrics.pageRectBottom) - 1 > y + height) {
             Log.d(LOGTAG, "Aborting update due to viewport not in display-port");
             mProgressiveUpdateData.abort = true;
             return mProgressiveUpdateData;
         }
 
-        // There's no new content (where new content is considered to be an
-        // update in a region that wasn't previously visible), and we've sent a
-        // more recent display-port.
-        // Aborting in this situation helps us recover more quickly when the
-        // user starts scrolling on a page that contains animated content that
-        // is slow to draw.
-        if (!aHasPendingNewThebesContent) {
-            mProgressiveUpdateData.abort = true;
-            return mProgressiveUpdateData;
-        }
-
         return mProgressiveUpdateData;
     }
 
     /** Implementation of GeckoEventResponder/GeckoEventListener. */
     public void handleMessage(String event, JSONObject message) {
         try {
             if ("Checkerboard:Toggle".equals(event)) {
                 mView.setCheckerboardShouldShowChecks(message.getBoolean("value"));