Bug 731603 - Save the display port and resolution in the VirtualLayer's mPosition and mResolution fields. r=Cwiiis

mobile/android/base/gfx/GeckoLayerClient.java

@@ -73,15 +73,9 @@ public class GeckoLayerClient implements GeckoEventResponder,
 
     private VirtualLayer mRootLayer;
 
-    /* The viewport that Gecko is currently displaying. */
+    /* The Gecko viewport as per the UI thread. Must be touched only on the UI thread. */
     private ViewportMetrics mGeckoViewport;
 
-    /*
-     * The display port that Gecko is currently displaying. This is stored only
-     * to avoid having to create a Rect on every composition.
-     */
-    private Rect mGeckoDisplayPort;
-
     /*
      * The viewport metrics being used to draw the current frame. This is only
      * accessed by the compositor thread, and so needs no synchronisation.
@@ -102,7 +96,6 @@ public class GeckoLayerClient implements GeckoEventResponder,
         mScreenSize = new IntSize(0, 0);
         mWindowSize = new IntSize(0, 0);
         mDisplayPort = new RectF();
-        mGeckoDisplayPort = new Rect();
         mCurrentViewTransform = new ViewTransform(0, 0, 1);
     }
 
@@ -126,27 +119,6 @@ public class GeckoLayerClient implements GeckoEventResponder,
         sendResizeEventIfNecessary(true);
     }
 
-    /** This function is invoked by Gecko via JNI; be careful when modifying signature. */
-    public boolean beginDrawing(int width, int height, String metadata) {
-        try {
-            JSONObject viewportObject = new JSONObject(metadata);
-            mGeckoViewport = new ViewportMetrics(viewportObject);
-        } catch (JSONException e) {
-            Log.e(LOGTAG, "Aborting draw, bad viewport description: " + metadata);
-            return false;
-        }
-
-        return true;
-    }
-
-    /** 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());
-        }
-    }
-
     RectF getDisplayPort() {
         return mDisplayPort;
     }
@@ -267,17 +239,23 @@ public class GeckoLayerClient implements GeckoEventResponder,
 
         mDisplayPort = calculateDisplayPort(mLayerController.getViewportMetrics());
         GeckoAppShell.sendEventToGecko(GeckoEvent.createViewportEvent(viewportMetrics, mDisplayPort));
+        mGeckoViewport = viewportMetrics;
     }
 
     /** Implementation of GeckoEventResponder/GeckoEventListener. */
     public void handleMessage(String event, JSONObject message) {
         try {
             if ("Viewport:Update".equals(event)) {
-                ViewportMetrics newMetrics = new ViewportMetrics(message);
+                final ViewportMetrics newMetrics = new ViewportMetrics(message);
                 synchronized (mLayerController) {
                     // keep the old viewport size, but update everything else
                     ImmutableViewportMetrics oldMetrics = mLayerController.getViewportMetrics();
                     newMetrics.setSize(oldMetrics.getSize());
+                    mLayerController.post(new Runnable() {
+                        public void run() {
+                            mGeckoViewport = newMetrics;
+                        }
+                    });
                     mLayerController.setViewportMetrics(newMetrics);
                     mLayerController.abortPanZoomAnimation();
                     mDisplayPort = calculateDisplayPort(mLayerController.getViewportMetrics());
@@ -324,11 +302,16 @@ public class GeckoLayerClient implements GeckoEventResponder,
             adjustViewport();
     }
 
+    /*
+     * This function returns the last viewport that we sent to Gecko. If any additional events are
+     * being sent to Gecko that are relative on the Gecko viewport position, they must (a) be relative
+     * to this viewport, and (b) be sent on the UI thread to avoid races. As long as these two
+     * conditions are satisfied, and the events being sent to Gecko are processed in FIFO order, the
+     * events will properly be relative to the Gecko viewport position. Note that if Gecko updates
+     * its viewport independently, we get notified synchronously and also update this on the UI thread.
+     */
     public ViewportMetrics getGeckoViewportMetrics() {
-        // Return a copy, as we modify this inside the Gecko thread
-        if (mGeckoViewport != null)
-            return new ViewportMetrics(mGeckoViewport);
-        return null;
+        return mGeckoViewport;
     }
 
     /** This function is invoked by Gecko via JNI; be careful when modifying signature.
@@ -401,8 +384,7 @@ public class GeckoLayerClient implements GeckoEventResponder,
         mCurrentViewTransform.y = mFrameMetrics.viewportRectTop;
         mCurrentViewTransform.scale = mFrameMetrics.zoomFactor;
 
-        mGeckoDisplayPort.set(x, y, x + width, y + height);
-        mRootLayer.setDisplayPort(mGeckoDisplayPort);
+        mRootLayer.setPositionAndResolution(x, y, x + width, y + height, resolution);
 
         if (layersUpdated && mDrawListener != null) {
             /* Used by robocop for testing purposes */

mobile/android/base/gfx/Layer.java

@@ -52,11 +52,9 @@ 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);
@@ -101,23 +99,13 @@ 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 display-port bounds of the layer,
-     * but this may be overridden.
+     * implementation of this will return the bounds of the layer, but this
+     * may be overridden.
      */
     public Region getValidRegion(RenderContext context) {
-        return new Region(RectUtils.round(getDisplayPortBounds(context)));
+        return new Region(RectUtils.round(getBounds(context)));
     }
 
     /**
@@ -176,22 +164,6 @@ 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
@@ -203,10 +175,6 @@ 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/LayerController.java

@@ -352,28 +352,26 @@ public class LayerController implements Tabs.OnTabsChangedListener {
     /**
      * Converts a point from layer view coordinates to layer coordinates. In other words, given a
      * point measured in pixels from the top left corner of the layer view, returns the point in
-     * pixels measured from the top left corner of the root layer, in the coordinate system of the
-     * layer itself (CSS pixels). This method is used as part of the process of translating touch
-     * events to Gecko's coordinate system.
+     * pixels measured from the last scroll position we sent to Gecko, in CSS pixels. Assuming the
+     * events being sent to Gecko are processed in FIFO order, this calculation should always be
+     * correct.
      */
     public PointF convertViewPointToLayerPoint(PointF viewPoint) {
-        if (mRootLayer == null)
-            return null;
-
         ImmutableViewportMetrics viewportMetrics = mViewportMetrics;
         PointF origin = viewportMetrics.getOrigin();
         float zoom = viewportMetrics.zoomFactor;
-        Rect rootPosition = mRootLayer.getPosition();
-        float rootScale = mRootLayer.getResolution();
+        ViewportMetrics geckoViewport = mLayerClient.getGeckoViewportMetrics();
+        PointF geckoOrigin = geckoViewport.getOrigin();
+        float geckoZoom = geckoViewport.getZoomFactor();
 
         // viewPoint + origin gives the coordinate in device pixels from the top-left corner of the page.
         // Divided by zoom, this gives us the coordinate in CSS pixels from the top-left corner of the page.
-        // rootPosition / rootScale is where Gecko thinks it is (scrollTo position) in CSS pixels from
+        // geckoOrigin / geckoZoom is where Gecko thinks it is (scrollTo position) in CSS pixels from
         // the top-left corner of the page. Subtracting the two gives us the offset of the viewPoint from
         // the current Gecko coordinate in CSS pixels.
         PointF layerPoint = new PointF(
-                ((viewPoint.x + origin.x) / zoom) - (rootPosition.left / rootScale),
-                ((viewPoint.y + origin.y) / zoom) - (rootPosition.top / rootScale));
+                ((viewPoint.x + origin.x) / zoom) - (geckoOrigin.x / geckoZoom),
+                ((viewPoint.y + origin.y) / zoom) - (geckoOrigin.y / geckoZoom));
 
         return layerPoint;
     }

mobile/android/base/gfx/LayerRenderer.java

@@ -580,7 +580,7 @@ public class LayerRenderer implements GLSurfaceView.Renderer {
             Rect rootMask = null;
             Layer rootLayer = mView.getController().getRoot();
             if (rootLayer != null) {
-                RectF rootBounds = rootLayer.getDisplayPortBounds(mPageContext);
+                RectF rootBounds = rootLayer.getBounds(mPageContext);
                 rootBounds.offset(-mPageContext.viewport.left, -mPageContext.viewport.top);
                 rootMask = new Rect();
                 rootBounds.roundOut(rootMask);

mobile/android/base/gfx/VirtualLayer.java

@@ -50,11 +50,11 @@ public class VirtualLayer extends Layer {
         // No-op.
     }
 
-    void setPositionAndResolution(Rect newPosition, float newResolution) {
+    void setPositionAndResolution(int left, int top, int right, int bottom, float newResolution) {
         // This is an optimized version of the following code:
         // beginTransaction();
         // try {
-        //     setPosition(newPosition);
+        //     setPosition(new Rect(left, top, right, bottom));
         //     setResolution(newResolution);
         //     performUpdates(null);
         // } finally {
@@ -63,21 +63,9 @@ public class VirtualLayer extends Layer {
 
         // it is safe to drop the transaction lock in this instance (i.e. for the
         // VirtualLayer that is just a shadow of what gecko is painting) because
-        // the position and resolution of this layer are never used for anything
-        // meaningful.
-        // XXX The above is not true any more; the compositor uses these values
-        // in order to determine where to draw the checkerboard. The values are
-        // also used in LayerController's convertViewPointToLayerPoint function.
-        mPosition = newPosition;
+        // the position and resolution of this layer are always touched on the compositor
+        // thread, and therefore do not require synchronization.
+        mPosition.set(left, top, right, bottom);
         mResolution = newResolution;
     }
-
-    @Override
-    public void setDisplayPort(Rect displayPort) {
-        // Similar to the above, this removes the lock from setDisplayPort. As
-        // this is currently only called by the Compositor, and it is only
-        // accessed by the composition thread, it is safe to remove the locks
-        // from this call.
-        mDisplayPort = displayPort;
-    }
 }

widget/android/AndroidJavaWrappers.cpp

@@ -93,8 +93,6 @@ jmethodID AndroidLocation::jGetSpeedMethod = 0;
 jmethodID AndroidLocation::jGetTimeMethod = 0;
 
 jclass AndroidGeckoLayerClient::jGeckoLayerClientClass = 0;
-jmethodID AndroidGeckoLayerClient::jBeginDrawingMethod = 0;
-jmethodID AndroidGeckoLayerClient::jEndDrawingMethod = 0;
 jmethodID AndroidGeckoLayerClient::jSetFirstPaintViewport = 0;
 jmethodID AndroidGeckoLayerClient::jSetPageSize = 0;
 jmethodID AndroidGeckoLayerClient::jSyncViewportInfoMethod = 0;
@@ -270,8 +268,6 @@ AndroidGeckoLayerClient::InitGeckoLayerClientClass(JNIEnv *jEnv)
 
     jGeckoLayerClientClass = getClassGlobalRef("org/mozilla/gecko/gfx/GeckoLayerClient");
 
-    jBeginDrawingMethod = getMethod("beginDrawing", "(IILjava/lang/String;)Z");
-    jEndDrawingMethod = getMethod("endDrawing", "()V");
     jSetFirstPaintViewport = getMethod("setFirstPaintViewport", "(FFFFF)V");
     jSetPageSize = getMethod("setPageSize", "(FFF)V");
     jSyncViewportInfoMethod = getMethod("syncViewportInfo",
@@ -656,32 +652,6 @@ AndroidGeckoSurfaceView::Draw2D(jobject buffer, int stride)
     env->CallVoidMethod(wrapped_obj, jDraw2DBufferMethod, buffer, stride);
 }
 
-bool
-AndroidGeckoLayerClient::BeginDrawing(int aWidth, int aHeight, const nsAString &aMetadata)
-{
-    NS_ASSERTION(!isNull(), "BeginDrawing() called on null layer client!");
-    JNIEnv *env = AndroidBridge::GetJNIEnv();
-    if (!env)
-        return false;
-
-    AndroidBridge::AutoLocalJNIFrame jniFrame(env);
-    jstring jMetadata = env->NewString(nsPromiseFlatString(aMetadata).get(), aMetadata.Length());
-
-    return env->CallBooleanMethod(wrapped_obj, jBeginDrawingMethod, aWidth, aHeight, jMetadata);
-}
-
-void
-AndroidGeckoLayerClient::EndDrawing()
-{
-    NS_ASSERTION(!isNull(), "EndDrawing() called on null layer client!");
-    JNIEnv *env = AndroidBridge::GetJNIEnv();
-    if (!env)
-        return;
-
-    AndroidBridge::AutoLocalJNIFrame jniFrame(env);
-    return env->CallVoidMethod(wrapped_obj, jEndDrawingMethod);
-}
-
 void
 AndroidGeckoLayerClient::SetFirstPaintViewport(float aOffsetX, float aOffsetY, float aZoom, float aPageWidth, float aPageHeight)
 {

widget/android/AndroidJavaWrappers.h

@@ -202,8 +202,6 @@ public:
     AndroidGeckoLayerClient() {}
     AndroidGeckoLayerClient(jobject jobj) { Init(jobj); }
 
-    bool BeginDrawing(int aWidth, int aHeight, const nsAString &aMetadata);
-    void EndDrawing();
     void SetFirstPaintViewport(float aOffsetX, float aOffsetY, float aZoom, float aPageWidth, float aPageHeight);
     void SetPageSize(float aZoom, float aPageWidth, float aPageHeight);
     void SyncViewportInfo(const nsIntRect& aDisplayPort, float aDisplayResolution, bool aLayersUpdated,
@@ -214,8 +212,6 @@ public:
 
 protected:
     static jclass jGeckoLayerClientClass;
-    static jmethodID jBeginDrawingMethod;
-    static jmethodID jEndDrawingMethod;
     static jmethodID jSetFirstPaintViewport;
     static jmethodID jSetPageSize;
     static jmethodID jSyncViewportInfoMethod;

widget/android/nsWindow.cpp

@@ -1158,13 +1158,6 @@ nsWindow::OnDraw(AndroidGeckoEvent *ae)
     }
     layers::renderTraceEventEnd("Check supress", "424242");
 
-    layers::renderTraceEventStart("Get Drawable", "424343");
-    nsAutoString metadata;
-    if (metadataProvider) {
-        metadataProvider->GetDrawMetadata(metadata);
-    }
-    layers::renderTraceEventEnd("Get Drawable", "424343");
-
     layers::renderTraceEventStart("Get surface", "424545");
     static unsigned char bits2[32 * 32 * 2];
     nsRefPtr<gfxImageSurface> targetSurface =
@@ -1172,26 +1165,14 @@ nsWindow::OnDraw(AndroidGeckoEvent *ae)
                             gfxASurface::ImageFormatRGB16_565);
     layers::renderTraceEventEnd("Get surface", "424545");
 
-    layers::renderTraceEventStart("Check Bridge", "434444");
-    nsIntRect dirtyRect = ae->Rect().Intersect(nsIntRect(0, 0, gAndroidBounds.width, gAndroidBounds.height));
-
-    AndroidGeckoLayerClient &client = AndroidBridge::Bridge()->GetLayerClient();
-    if (!client.BeginDrawing(gAndroidBounds.width, gAndroidBounds.height, metadata)) {
-        return;
-    }
-    layers::renderTraceEventEnd("Check Bridge", "434444");
-
     layers::renderTraceEventStart("Widget draw to", "434646");
+    nsIntRect dirtyRect = ae->Rect().Intersect(nsIntRect(0, 0, gAndroidBounds.width, gAndroidBounds.height));
     if (targetSurface->CairoStatus()) {
         ALOG("### Failed to create a valid surface from the bitmap");
     } else {
         DrawTo(targetSurface, dirtyRect);
     }
     layers::renderTraceEventEnd("Widget draw to", "434646");
-
-    layers::renderTraceEventStart("Widget end draw", "434747");
-    client.EndDrawing();
-    layers::renderTraceEventEnd("Widget end draw", "434747");
     return;
 #endif