Bug 794130 - Abort drawing if appropriate when using progressive tile updates. r=bgirard,blassey

Add a function to BasicLayerManager to check if it's appropriate to abort an
ongoing progressive update, and add an Android-specific implementation in
mobile/android/base/gfx/GeckoLayerClient.java.

--HG--
extra : rebase_source : edbd20acb53660340d52265591849aacafd179c4

gfx/layers/basic/BasicLayerManager.cpp

@@ -27,6 +27,10 @@
 #include "mozilla/Preferences.h"
 #include "nsIWidget.h"
 
+#ifdef MOZ_WIDGET_ANDROID
+#include "AndroidBridge.h"
+#endif
+
 using namespace mozilla::dom;
 using namespace mozilla::gfx;
 
@@ -1272,6 +1276,32 @@ BasicShadowLayerManager::SetIsFirstPaint()
   ShadowLayerForwarder::SetIsFirstPaint();
 }
 
+bool
+BasicShadowLayerManager::ShouldAbortProgressiveUpdate(bool aHasPendingNewThebesContent)
+{
+#ifdef MOZ_WIDGET_ANDROID
+  Layer* primaryScrollable = GetPrimaryScrollableLayer();
+  if (primaryScrollable) {
+    const FrameMetrics& metrics = primaryScrollable->AsContainerLayer()->GetFrameMetrics();
+
+    // This is derived from the code in
+    // gfx/layers/ipc/CompositorParent.cpp::TransformShadowTree.
+    const gfx3DMatrix& rootTransform = GetRoot()->GetTransform();
+    float devPixelRatioX = 1 / rootTransform.GetXScale();
+    float devPixelRatioY = 1 / rootTransform.GetYScale();
+    gfx::Rect displayPort((metrics.mDisplayPort.x + metrics.mScrollOffset.x) * devPixelRatioX,
+                          (metrics.mDisplayPort.y + metrics.mScrollOffset.y) * devPixelRatioY,
+                          metrics.mDisplayPort.width * devPixelRatioX,
+                          metrics.mDisplayPort.height * devPixelRatioY);
+
+    return AndroidBridge::Bridge()->ShouldAbortProgressiveUpdate(
+      aHasPendingNewThebesContent, displayPort, devPixelRatioX);
+  }
+#endif
+
+  return false;
+}
+
 already_AddRefed<ThebesLayer>
 BasicShadowLayerManager::CreateThebesLayer()
 {

gfx/layers/basic/BasicLayers.h

@@ -272,6 +272,14 @@ public:
 
   void SetRepeatTransaction() { mRepeatTransaction = true; }
 
+  /**
+   * Determines if a progressive update should be cancelled. This is only called
+   * if gfxPlatform::UseProgressiveTilePainting() returns true.
+   * aHasPendingNewThebesContent is true if there is a Thebes layer update
+   * that will cause its valid region to expand.
+   */
+  bool ShouldAbortProgressiveUpdate(bool aHasPendingNewThebesContent);
+
 private:
   /**
    * Forward transaction results to the parent context.

gfx/layers/basic/BasicTiledThebesLayer.cpp

@@ -280,10 +280,17 @@ BasicTiledThebesLayer::PaintThebes(gfxContext* aContext,
     // Paint tiles that have no content before tiles that only have stale content.
     nsIntRegion staleRegion = mTiledBuffer.GetValidRegion();
     staleRegion.And(staleRegion, regionToPaint);
-    if (!staleRegion.IsEmpty() && !staleRegion.Contains(regionToPaint)) {
+    bool hasNewContent = !staleRegion.Contains(regionToPaint);
+    if (!staleRegion.IsEmpty() && hasNewContent) {
       regionToPaint.Sub(regionToPaint, staleRegion);
     }
 
+    // Find out if we should just abort this paint, usually due to there being
+    // an incoming, more relevant paint.
+    if (BasicManager()->ShouldAbortProgressiveUpdate(hasNewContent)) {
+      return;
+    }
+
     // The following code decides what order to draw tiles in, based on the
     // current scroll direction of the primary scrollable layer.
     // XXX While this code is of a reasonable size currently, it is likely
@@ -347,7 +354,7 @@ BasicTiledThebesLayer::PaintThebes(gfxContext* aContext,
     mValidRegion = mVisibleRegion;
   }
 
-  mTiledBuffer.PaintThebes(this, mVisibleRegion, regionToPaint, aCallback, aCallbackData);
+  mTiledBuffer.PaintThebes(this, mValidRegion, regionToPaint, aCallback, aCallbackData);
 
   mTiledBuffer.ReadLock();
   if (aMaskLayer) {

mobile/android/base/gfx/DisplayPortMetrics.java

@@ -18,16 +18,32 @@ import android.graphics.RectF;
  * subsection of that with compositor scaling.
  */
 public final class DisplayPortMetrics {
+    public final float resolution;
     private final RectF mPosition;
-    private final float mResolution;
 
     public DisplayPortMetrics() {
         this(0, 0, 0, 0, 1);
     }
 
     public DisplayPortMetrics(float left, float top, float right, float bottom, float resolution) {
+        this.resolution = resolution;
         mPosition = new RectF(left, top, right, bottom);
-        mResolution = resolution;
+    }
+
+    public float getLeft() {
+        return mPosition.left;
+    }
+
+    public float getTop() {
+        return mPosition.top;
+    }
+
+    public float getRight() {
+        return mPosition.right;
+    }
+
+    public float getBottom() {
+        return mPosition.bottom;
     }
 
     public boolean contains(RectF rect) {
@@ -36,7 +52,7 @@ public final class DisplayPortMetrics {
 
     public boolean fuzzyEquals(DisplayPortMetrics metrics) {
         return RectUtils.fuzzyEquals(mPosition, metrics.mPosition)
-            && FloatUtils.fuzzyEquals(mResolution, metrics.mResolution);
+            && FloatUtils.fuzzyEquals(resolution, metrics.resolution);
     }
 
     public String toJSON() {
@@ -45,15 +61,14 @@ public final class DisplayPortMetrics {
           .append(", \"top\": ").append(mPosition.top)
           .append(", \"right\": ").append(mPosition.right)
           .append(", \"bottom\": ").append(mPosition.bottom)
-          .append(", \"resolution\": ").append(mResolution)
+          .append(", \"resolution\": ").append(resolution)
           .append('}');
         return sb.toString();
     }
 
     @Override
     public String toString() {
-        return "DisplayPortMetrics v=(" + mPosition.left + ","
-                + mPosition.top + "," + mPosition.right + ","
-                + mPosition.bottom + ") z=" + mResolution;
+        return "DisplayPortMetrics v=(" + mPosition.left + "," + mPosition.top + "," + mPosition.right + ","
+                + mPosition.bottom + ") z=" + resolution;
     }
 }

mobile/android/base/gfx/GeckoLayerClient.java

@@ -14,6 +14,7 @@ import org.mozilla.gecko.ZoomConstraints;
 import org.mozilla.gecko.ui.PanZoomController;
 import org.mozilla.gecko.ui.PanZoomTarget;
 import org.mozilla.gecko.util.EventDispatcher;
+import org.mozilla.gecko.util.FloatUtils;
 import org.mozilla.gecko.util.GeckoEventResponder;
 
 import org.json.JSONException;
@@ -356,6 +357,69 @@ public class GeckoLayerClient
         }
     }
 
+    // This is called on the Gecko thread to determine if we're still interested
+    // in the update of this display-port to continue. We can return true here
+    // to abort the current update and continue with any subsequent ones. This
+    // is useful for slow-to-render pages when the display-port starts lagging
+    // behind enough that continuing to draw it is wasted effort.
+    public boolean shouldAbortProgressiveUpdate(boolean aHasPendingNewThebesContent,
+                                                float x, float y, float width, float height, float resolution) {
+        // XXX Grab a local copy of the last display-port sent to Gecko to
+        //     avoid races when accessing it.
+        DisplayPortMetrics displayPort = mDisplayPort;
+
+        // Always abort updates if the resolution has changed. There's no use
+        // in drawing at the incorrect resolution.
+        if (!FloatUtils.fuzzyEquals(resolution, displayPort.resolution)) {
+            Log.d(LOGTAG, "Aborting draw due to resolution change");
+            return true;
+        }
+
+        // XXX All sorts of rounding happens inside Gecko that becomes hard to
+        //     account exactly for. Given we align the display-port to tile
+        //     boundaries (and so they rarely vary by sub-pixel amounts), just
+        //     check that values are within a pixel of the display-port bounds.
+
+        // Never abort drawing if we can't be sure we've sent a more recent
+        // display-port. If we abort updating when we shouldn't, we can end up
+        // with blank regions on the screen and we open up the risk of entering
+        // an endless updating cycle.
+        if (Math.abs(displayPort.getLeft() - x) <= 1 &&
+            Math.abs(displayPort.getTop() - y) <= 1 &&
+            Math.abs(displayPort.getBottom() - (y + height)) <= 1 &&
+            Math.abs(displayPort.getRight() - (x + width)) <= 1) {
+            return false;
+        }
+
+        // Abort updates when the display-port no longer contains the visible
+        // area of the page (that is, the viewport cropped by the page
+        // boundaries).
+        // XXX This makes the assumption that we never let the visible area of
+        //     the page fall outside of the display-port.
+        // Grab a local copy of the viewport metrics to avoid races.
+        ImmutableViewportMetrics viewportMetrics = mViewportMetrics;
+        if (Math.max(viewportMetrics.viewportRectLeft, viewportMetrics.pageRectLeft) + 1 < x ||
+            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");
+            return true;
+        }
+
+        // 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) {
+            Log.d(LOGTAG, "Aborting update due to more relevant display-port in event queue");
+            return true;
+        }
+
+        return false;
+    }
+
     /** Implementation of GeckoEventResponder/GeckoEventListener. */
     public void handleMessage(String event, JSONObject message) {
         try {

widget/android/AndroidBridge.cpp

@@ -2524,6 +2524,17 @@ AndroidBridge::GetDisplayPort(bool aPageSizeUpdate, bool aIsBrowserContentDispla
     return NS_OK;
 }
 
+bool
+AndroidBridge::ShouldAbortProgressiveUpdate(bool aHasPendingNewThebesContent, const gfx::Rect& aDisplayPort, float aDisplayResolution)
+{
+    JNIEnv* env = GetJNIEnv();
+    if (!env || !mLayerClient)
+        return false;
+
+    AutoLocalJNIFrame jniFrame(env, 0);
+    return mLayerClient->ShouldAbortProgressiveUpdate(&jniFrame, aHasPendingNewThebesContent, aDisplayPort, aDisplayResolution);
+}
+
 void
 AndroidBridge::NotifyPaintedRect(float top, float left, float bottom, float right)
 {

widget/android/AndroidBridge.h

@@ -156,6 +156,8 @@ public:
     nsresult TakeScreenshot(nsIDOMWindow *window, int32_t srcX, int32_t srcY, int32_t srcW, int32_t srcH, int32_t dstY, int32_t dstX, int32_t dstW, int32_t dstH, int32_t bufW, int32_t bufH, int32_t tabId, int32_t token, jobject buffer);
     nsresult GetDisplayPort(bool aPageSizeUpdate, bool aIsBrowserContentDisplayed, int32_t tabId, nsIAndroidViewport* metrics, nsIAndroidDisplayport** displayPort);
 
+    bool ShouldAbortProgressiveUpdate(bool aHasPendingNewThebesContent, const gfx::Rect& aDisplayPort, float aDisplayResolution);
+
     static void NotifyPaintedRect(float top, float left, float bottom, float right);
 
     void AcknowledgeEventSync();

widget/android/AndroidJavaWrappers.cpp

@@ -84,6 +84,7 @@ jmethodID AndroidGeckoLayerClient::jGetDisplayPort = 0;
 jmethodID AndroidGeckoLayerClient::jViewportCtor = 0;
 jfieldID AndroidGeckoLayerClient::jDisplayportPosition = 0;
 jfieldID AndroidGeckoLayerClient::jDisplayportResolution = 0;
+jmethodID AndroidGeckoLayerClient::jShouldAbortProgressiveUpdate = 0;
 
 jclass AndroidLayerRendererFrame::jLayerRendererFrameClass = 0;
 jmethodID AndroidLayerRendererFrame::jBeginDrawingMethod = 0;
@@ -344,13 +345,14 @@ AndroidGeckoLayerClient::InitGeckoLayerClientClass(JNIEnv *jEnv)
     jActivateProgramMethod = getMethod("activateProgram", "()V");
     jDeactivateProgramMethod = getMethod("deactivateProgram", "()V");
     jGetDisplayPort = getMethod("getDisplayPort", "(ZZILorg/mozilla/gecko/gfx/ViewportMetrics;)Lorg/mozilla/gecko/gfx/DisplayPortMetrics;");
+    jShouldAbortProgressiveUpdate = getMethod("shouldAbortProgressiveUpdate", "(ZFFFFF)Z");
 
     jViewportClass = GetClassGlobalRef(jEnv, "org/mozilla/gecko/gfx/ViewportMetrics");
     jViewportCtor = GetMethodID(jEnv, jViewportClass, "<init>", "(FFFFFFFFFFFFF)V");
 
     jDisplayportClass = GetClassGlobalRef(jEnv, "org/mozilla/gecko/gfx/DisplayPortMetrics");
     jDisplayportPosition = GetFieldID(jEnv, jDisplayportClass, "mPosition", "Landroid/graphics/RectF;");
-    jDisplayportResolution = GetFieldID(jEnv, jDisplayportClass, "mResolution", "F");
+    jDisplayportResolution = GetFieldID(jEnv, jDisplayportClass, "resolution", "F");
 
 #endif
 }
@@ -885,6 +887,14 @@ AndroidGeckoLayerClient::GetDisplayPort(AutoLocalJNIFrame *jniFrame, bool aPageS
     (*displayPort)->AddRef();
 }
 
+bool
+AndroidGeckoLayerClient::ShouldAbortProgressiveUpdate(AutoLocalJNIFrame *jniFrame, bool aHasPendingNewThebesContent, const gfx::Rect& aDisplayPort, float aDisplayResolution)
+{
+  bool ret = jniFrame->GetEnv()->CallObjectMethod(wrapped_obj, jShouldAbortProgressiveUpdate, aHasPendingNewThebesContent, (float)aDisplayPort.x, (float)aDisplayPort.y, (float)aDisplayPort.width, (float)aDisplayPort.height, aDisplayResolution);
+  if (jniFrame->CheckForException()) return false;
+  return ret;
+}
+
 jobject
 AndroidGeckoSurfaceView::GetSoftwareDrawBitmap(AutoLocalJNIFrame *jniFrame)
 {

widget/android/AndroidJavaWrappers.h

@@ -237,6 +237,7 @@ public:
     bool ActivateProgram(AutoLocalJNIFrame *jniFrame);
     bool DeactivateProgram(AutoLocalJNIFrame *jniFrame);
     void GetDisplayPort(AutoLocalJNIFrame *jniFrame, bool aPageSizeUpdate, bool aIsBrowserContentDisplayed, int32_t tabId, nsIAndroidViewport* metrics, nsIAndroidDisplayport** displayPort);
+    bool ShouldAbortProgressiveUpdate(AutoLocalJNIFrame *jniFrame, bool aHasPendingNewThebesContent, const gfx::Rect& aDisplayPort, float aDisplayResolution);
 
 protected:
     static jclass jGeckoLayerClientClass;
@@ -247,6 +248,7 @@ protected:
     static jmethodID jActivateProgramMethod;
     static jmethodID jDeactivateProgramMethod;
     static jmethodID jGetDisplayPort;
+    static jmethodID jShouldAbortProgressiveUpdate;
 
 public:
     static jclass jViewportClass;