Bug 732756 - Fix overdrawing of checkerboard. r=kats

Fix overdrawing of the checkerboard layer by letting layers have a concept of
a display-port, and keeping the root layer's display port in track with
Gecko's.

gfx/layers/ipc/CompositorParent.cpp

@@ -284,9 +284,9 @@ CompositorParent::TransformShadowTree()
                                                   mContentSize.height);
   }
 
-  // We request the view transform from Java after sending the above notifications,
-  // so that Java can take these into account in its response.
-  RequestViewTransform();
+  // We synchronise the viewport information with Java after sending the above
+  // notifications, so that Java can take these into account in its response.
+  SyncViewportInfo();
 
   // Handle transformations for asynchronous panning and zooming. We determine the
   // zoom used by Gecko from the transformation set on the root layer, and we
@@ -314,9 +314,20 @@ CompositorParent::TransformShadowTree()
 
 #ifdef MOZ_WIDGET_ANDROID
 void
-CompositorParent::RequestViewTransform()
+CompositorParent::SyncViewportInfo()
 {
-  mozilla::AndroidBridge::Bridge()->GetViewTransform(mScrollOffset, mXScale, mYScale);
+  ContainerLayer* container = GetPrimaryScrollableLayer()->AsContainerLayer();
+  const FrameMetrics* metrics = &container->GetFrameMetrics();
+
+  if (metrics) {
+    // Calculate the absolute display port to send to Java
+    nsIntRect displayPort = container->GetFrameMetrics().mDisplayPort;
+    nsIntPoint scrollOffset = metrics->mViewportScrollOffset;
+    displayPort.x += scrollOffset.x;
+    displayPort.y += scrollOffset.y;
+
+    mozilla::AndroidBridge::Bridge()->SyncViewportInfo(displayPort, mScrollOffset, mXScale, mYScale);
+  }
 }
 #endif
 

gfx/layers/ipc/CompositorParent.h

@@ -123,10 +123,11 @@ private:
   // Platform specific functions
 #ifdef MOZ_WIDGET_ANDROID
   /**
-   * Asks Java for the viewport position and updates the world transform
-   * accordingly.
+   * Informs Java of the current display port, and asks Java for its viewport
+   * position and zoom, to use in updating the world transform in
+   * TransformShadowTree.
    */
-  void RequestViewTransform();
+  void SyncViewportInfo();
 
   /**
    * Does a breadth-first search to find the first layer in the tree with a

mobile/android/base/gfx/GeckoLayerClient.java

@@ -76,12 +76,18 @@ public class GeckoLayerClient implements GeckoEventResponder,
     /* The viewport that Gecko is currently displaying. */
     private ViewportMetrics mGeckoViewport;
 
+    /*
+     * The viewport metrics being used to draw the current frame. This is only
+     * accessed by the compositor thread, and so needs no synchronisation.
+     */
+    private ImmutableViewportMetrics mFrameMetrics;
+
     private String mLastCheckerboardColor;
 
     /* Used by robocop for testing purposes */
     private DrawListener mDrawListener;
 
-    /* Used as a temporary ViewTransform by getViewTransform */
+    /* Used as a temporary ViewTransform by syncViewportInfo */
     private ViewTransform mCurrentViewTransform;
 
     public GeckoLayerClient(Context context) {
@@ -127,11 +133,6 @@ public class GeckoLayerClient implements GeckoEventResponder,
 
     /** This function is invoked by Gecko via JNI; be careful when modifying signature. */
     public void endDrawing() {
-        synchronized (mLayerController) {
-            RectF position = mGeckoViewport.getViewport();
-            mRootLayer.setPositionAndResolution(RectUtils.round(position), mGeckoViewport.getZoomFactor());
-        }
-
         /* Used by robocop for testing purposes */
         if (mDrawListener != null) {
             mDrawListener.drawFinished();
@@ -319,7 +320,7 @@ public class GeckoLayerClient implements GeckoEventResponder,
       * is different from the document composited on the last frame. In these cases, the viewport
       * information we have in Java is no longer valid and needs to be replaced with the new
       * viewport information provided. setPageSize will never be invoked on the same frame that
-      * this function is invoked on; and this function will always be called prior to getViewTransform.
+      * this function is invoked on; and this function will always be called prior to syncViewportInfo.
       */
     public void setFirstPaintViewport(float offsetX, float offsetY, float zoom, float pageWidth, float pageHeight) {
         synchronized (mLayerController) {
@@ -343,7 +344,7 @@ public class GeckoLayerClient implements GeckoEventResponder,
       * The compositor invokes this function whenever it determines that the page size
       * has changed (based on the information it gets from layout). If setFirstPaintViewport
       * is invoked on a frame, then this function will not be. For any given frame, this
-      * function will be invoked before getViewTransform.
+      * function will be invoked before syncViewportInfo.
       */
     public void setPageSize(float zoom, float pageWidth, float pageHeight) {
         synchronized (mLayerController) {
@@ -363,19 +364,32 @@ public class GeckoLayerClient implements GeckoEventResponder,
 
     /** This function is invoked by Gecko via JNI; be careful when modifying signature.
       * The compositor invokes this function on every frame to figure out what part of the
-      * page to display. Since it is called on every frame, it needs to be ultra-fast.
+      * page to display, and to inform Java of the current display port. Since it is called
+      * on every frame, it needs to be ultra-fast.
       * It avoids taking any locks or allocating any objects. We keep around a
       * mCurrentViewTransform so we don't need to allocate a new ViewTransform
       * everytime we're called. NOTE: we might be able to return a ImmutableViewportMetrics
       * which would avoid the copy into mCurrentViewTransform.
       */
-    public ViewTransform getViewTransform() {
+    public ViewTransform syncViewportInfo(int x, int y, int width, int height) {
         // getViewportMetrics is thread safe so we don't need to synchronize
-        // on myLayerController.
-        ImmutableViewportMetrics viewportMetrics = mLayerController.getViewportMetrics();
-        mCurrentViewTransform.x = viewportMetrics.viewportRectLeft;
-        mCurrentViewTransform.y = viewportMetrics.viewportRectTop;
-        mCurrentViewTransform.scale = viewportMetrics.zoomFactor;
+        // on mLayerController.
+        // We save the viewport metrics here, so we later use it later in
+        // createFrame (which will be called by nsWindow::DrawWindowUnderlay on
+        // the native side, by the compositor). The LayerController's viewport
+        // metrics can change between here and there, as it's accessed outside
+        // of the compositor thread.
+        mFrameMetrics = mLayerController.getViewportMetrics();
+
+        // Update the root layer metrics
+        mRootLayer.setMetrics(RectUtils.round(mGeckoViewport.getViewport()),
+                              new Rect(x, y, x + width, y + height),
+                              mGeckoViewport.getZoomFactor());
+
+        mCurrentViewTransform.x = mFrameMetrics.viewportRectLeft;
+        mCurrentViewTransform.y = mFrameMetrics.viewportRectTop;
+        mCurrentViewTransform.scale = mFrameMetrics.zoomFactor;
+
         return mCurrentViewTransform;
     }
 
@@ -389,7 +403,7 @@ public class GeckoLayerClient implements GeckoEventResponder,
         }
 
         // Build the contexts and create the frame.
-        Layer.RenderContext pageContext = mLayerRenderer.createPageContext();
+        Layer.RenderContext pageContext = mLayerRenderer.createPageContext(mFrameMetrics);
         Layer.RenderContext screenContext = mLayerRenderer.createScreenContext();
         return mLayerRenderer.createFrame(pageContext, screenContext);
     }

mobile/android/base/gfx/Layer.java

@@ -52,9 +52,11 @@ public abstract class Layer {
     private boolean mInTransaction;
     private Rect mNewPosition;
     private float mNewResolution;
+    private Rect mNewDisplayPort;
 
     protected Rect mPosition;
     protected float mResolution;
+    protected Rect mDisplayPort;
 
     public Layer() {
         this(null);
@@ -99,13 +101,23 @@ public abstract class Layer {
         return RectUtils.scale(new RectF(mPosition), context.zoomFactor / mResolution);
     }
 
+    /**
+     * Returns the pixel boundaries of the layer's display-port rect. If no display port
+     * is set, returns the bounds of the layer.
+     */
+    protected RectF getDisplayPortBounds(RenderContext context) {
+        if (mDisplayPort != null)
+            return RectUtils.scale(new RectF(mDisplayPort), context.zoomFactor / mResolution);
+        return getBounds(context);
+    }
+
     /**
      * Returns the region of the layer that is considered valid. The default
-     * implementation of this will return the bounds of the layer, but this
-     * may be overridden.
+     * implementation of this will return the display-port bounds of the layer,
+     * but this may be overridden.
      */
     public Region getValidRegion(RenderContext context) {
-        return new Region(RectUtils.round(getBounds(context)));
+        return new Region(RectUtils.round(getDisplayPortBounds(context)));
     }
 
     /**
@@ -164,6 +176,22 @@ public abstract class Layer {
         mNewResolution = newResolution;
     }
 
+    /**
+     * Returns the layer's display port, or null if none is set. This is the
+     * rectangle that represents the area the layer will render, which may be
+     * different to its position.
+     */
+    public Rect getDisplayPort() {
+        return mDisplayPort;
+    }
+
+    /** Sets the layer's display port. */
+    public void setDisplayPort(Rect newDisplayPort) {
+        if (!mInTransaction)
+            throw new RuntimeException("setDisplayPort() is only valid inside a transaction");
+        mNewDisplayPort = newDisplayPort;
+    }
+
     /**
      * Subclasses may override this method to perform custom layer updates. This will be called
      * with the transaction lock held. Subclass implementations of this method must call the
@@ -175,6 +203,10 @@ public abstract class Layer {
             mPosition = mNewPosition;
             mNewPosition = null;
         }
+        if (mNewDisplayPort != null) {
+            mDisplayPort = mNewDisplayPort;
+            mNewDisplayPort = null;
+        }
         if (mNewResolution != 0.0f) {
             mResolution = mNewResolution;
             mNewResolution = 0.0f;

mobile/android/base/gfx/LayerRenderer.java

@@ -266,7 +266,8 @@ public class LayerRenderer implements GLSurfaceView.Renderer {
      * Called whenever a new frame is about to be drawn.
      */
     public void onDrawFrame(GL10 gl) {
-        RenderContext pageContext = createPageContext(), screenContext = createScreenContext();
+        RenderContext pageContext = createPageContext(mView.getController().getViewportMetrics());
+        RenderContext screenContext = createScreenContext();
         Frame frame = createFrame(pageContext, screenContext);
         synchronized (mView.getController()) {
             frame.beginDrawing();
@@ -306,14 +307,10 @@ public class LayerRenderer implements GLSurfaceView.Renderer {
         return createContext(viewport, pageSize, 1.0f);
     }
 
-    public RenderContext createPageContext() {
-        LayerController layerController = mView.getController();
-
-        Rect viewport = new Rect();
-        layerController.getViewport().round(viewport);
-
-        FloatSize pageSize = new FloatSize(layerController.getPageSize());
-        float zoomFactor = layerController.getZoomFactor();
+    public RenderContext createPageContext(ImmutableViewportMetrics metrics) {
+        Rect viewport = RectUtils.round(metrics.getViewport());
+        FloatSize pageSize = metrics.getPageSize();
+        float zoomFactor = metrics.zoomFactor;
         return createContext(new RectF(viewport), pageSize, zoomFactor);
     }
 
@@ -583,7 +580,7 @@ public class LayerRenderer implements GLSurfaceView.Renderer {
             Rect rootMask = null;
             Layer rootLayer = mView.getController().getRoot();
             if (rootLayer != null) {
-                RectF rootBounds = rootLayer.getBounds(mPageContext);
+                RectF rootBounds = rootLayer.getDisplayPortBounds(mPageContext);
                 rootBounds.offset(-mPageContext.viewport.left, -mPageContext.viewport.top);
                 rootMask = new Rect();
                 rootBounds.roundOut(rootMask);

mobile/android/base/gfx/VirtualLayer.java

@@ -50,11 +50,12 @@ public class VirtualLayer extends Layer {
         // No-op.
     }
 
-    void setPositionAndResolution(Rect newPosition, float newResolution) {
+    void setMetrics(Rect newPosition, Rect newDisplayPort, float newResolution) {
         // This is an optimized version of the following code:
         // beginTransaction();
         // try {
         //     setPosition(newPosition);
+        //     setDisplayPort(newDisplayPort);
         //     setResolution(newResolution);
         //     performUpdates(null);
         // } finally {
@@ -69,6 +70,7 @@ public class VirtualLayer extends Layer {
         // in order to determine where to draw the checkerboard. The values are
         // also used in LayerController's convertViewPointToLayerPoint function.
         mPosition = newPosition;
+        mDisplayPort = newDisplayPort;
         mResolution = newResolution;
     }
 }

widget/android/AndroidBridge.cpp

@@ -1891,13 +1891,13 @@ AndroidBridge::SetPageSize(float aZoom, float aPageWidth, float aPageHeight)
 }
 
 void
-AndroidBridge::GetViewTransform(nsIntPoint& aScrollOffset, float& aScaleX, float& aScaleY)
+AndroidBridge::SyncViewportInfo(const nsIntRect& aDisplayPort, nsIntPoint& aScrollOffset, float& aScaleX, float& aScaleY)
 {
     AndroidGeckoLayerClient *client = mLayerClient;
     if (!client)
         return;
 
-    client->GetViewTransform(aScrollOffset, aScaleX, aScaleY);
+    client->SyncViewportInfo(aDisplayPort, aScrollOffset, aScaleX, aScaleY);
 }
 
 AndroidBridge::AndroidBridge()

widget/android/AndroidBridge.h

@@ -408,7 +408,7 @@ public:
                              base::Thread* aCompositorThread);
     void SetFirstPaintViewport(float aOffsetX, float aOffsetY, float aZoom, float aPageWidth, float aPageHeight);
     void SetPageSize(float aZoom, float aPageWidth, float aPageHeight);
-    void GetViewTransform(nsIntPoint& aScrollOffset, float& aScaleX, float& aScaleY);
+    void SyncViewportInfo(const nsIntRect& aDisplayPort, nsIntPoint& aScrollOffset, float& aScaleX, float& aScaleY);
 
     jobject CreateSurface();
     void DestroySurface(jobject surface);

widget/android/AndroidJavaWrappers.cpp

@@ -97,7 +97,7 @@ jmethodID AndroidGeckoLayerClient::jBeginDrawingMethod = 0;
 jmethodID AndroidGeckoLayerClient::jEndDrawingMethod = 0;
 jmethodID AndroidGeckoLayerClient::jSetFirstPaintViewport = 0;
 jmethodID AndroidGeckoLayerClient::jSetPageSize = 0;
-jmethodID AndroidGeckoLayerClient::jGetViewTransformMethod = 0;
+jmethodID AndroidGeckoLayerClient::jSyncViewportInfoMethod = 0;
 jmethodID AndroidGeckoLayerClient::jCreateFrameMethod = 0;
 jmethodID AndroidGeckoLayerClient::jActivateProgramMethod = 0;
 jmethodID AndroidGeckoLayerClient::jDeactivateProgramMethod = 0;
@@ -274,8 +274,8 @@ AndroidGeckoLayerClient::InitGeckoLayerClientClass(JNIEnv *jEnv)
     jEndDrawingMethod = getMethod("endDrawing", "()V");
     jSetFirstPaintViewport = getMethod("setFirstPaintViewport", "(FFFFF)V");
     jSetPageSize = getMethod("setPageSize", "(FFF)V");
-    jGetViewTransformMethod = getMethod("getViewTransform",
-                                        "()Lorg/mozilla/gecko/gfx/ViewTransform;");
+    jSyncViewportInfoMethod = getMethod("syncViewportInfo",
+                                        "(IIII)Lorg/mozilla/gecko/gfx/ViewTransform;");
     jCreateFrameMethod = getMethod("createFrame", "()Lorg/mozilla/gecko/gfx/LayerRenderer$Frame;");
     jActivateProgramMethod = getMethod("activateProgram", "()V");
     jDeactivateProgramMethod = getMethod("deactivateProgram", "()V");
@@ -707,9 +707,9 @@ AndroidGeckoLayerClient::SetPageSize(float aZoom, float aPageWidth, float aPageH
 }
 
 void
-AndroidGeckoLayerClient::GetViewTransform(nsIntPoint& aScrollOffset, float& aScaleX, float& aScaleY)
+AndroidGeckoLayerClient::SyncViewportInfo(const nsIntRect& aDisplayPort, nsIntPoint& aScrollOffset, float& aScaleX, float& aScaleY)
 {
-    NS_ASSERTION(!isNull(), "GetViewTransform called on null layer client!");
+    NS_ASSERTION(!isNull(), "SyncViewportInfo called on null layer client!");
     JNIEnv *env = GetJNIForThread();    // this is called on the compositor thread
     if (!env)
         return;
@@ -717,7 +717,9 @@ AndroidGeckoLayerClient::GetViewTransform(nsIntPoint& aScrollOffset, float& aSca
     AndroidViewTransform viewTransform;
     AndroidBridge::AutoLocalJNIFrame jniFrame(env);
 
-    jobject viewTransformJObj = env->CallObjectMethod(wrapped_obj, jGetViewTransformMethod);
+    jobject viewTransformJObj = env->CallObjectMethod(wrapped_obj, jSyncViewportInfoMethod,
+                                                      aDisplayPort.x, aDisplayPort.y,
+                                                      aDisplayPort.width, aDisplayPort.height);
     NS_ABORT_IF_FALSE(viewTransformJObj, "No view transform object!");
     viewTransform.Init(viewTransformJObj);
 

widget/android/AndroidJavaWrappers.h

@@ -206,7 +206,7 @@ public:
     void EndDrawing();
     void SetFirstPaintViewport(float aOffsetX, float aOffsetY, float aZoom, float aPageWidth, float aPageHeight);
     void SetPageSize(float aZoom, float aPageWidth, float aPageHeight);
-    void GetViewTransform(nsIntPoint& aScrollOffset, float& aScaleX, float& aScaleY);
+    void SyncViewportInfo(const nsIntRect& aDisplayPort, nsIntPoint& aScrollOffset, float& aScaleX, float& aScaleY);
     void CreateFrame(AndroidLayerRendererFrame& aFrame);
     void ActivateProgram();
     void DeactivateProgram();
@@ -217,7 +217,7 @@ protected:
     static jmethodID jEndDrawingMethod;
     static jmethodID jSetFirstPaintViewport;
     static jmethodID jSetPageSize;
-    static jmethodID jGetViewTransformMethod;
+    static jmethodID jSyncViewportInfoMethod;
     static jmethodID jCreateFrameMethod;
     static jmethodID jActivateProgramMethod;
     static jmethodID jDeactivateProgramMethod;