Bug 732576. make getViewTransform fast. r=kats

This removes the logging, locking and allocations from getViewTransform.

This reduces the time spent from an median of 6.3ms to 0.061ms

We use a new scheme where the view transform is immutable and the member
variable containing it is atomically overwritten. So we may get a slightly old
view transform but this won't be a problem.

mobile/android/base/GeckoApp.java

@@ -51,6 +51,7 @@ import org.mozilla.gecko.gfx.PlaceholderLayerClient;
 import org.mozilla.gecko.gfx.RectUtils;
 import org.mozilla.gecko.gfx.SurfaceTextureLayer;
 import org.mozilla.gecko.gfx.ViewportMetrics;
+import org.mozilla.gecko.gfx.ImmutableViewportMetrics;
 import org.mozilla.gecko.Tab.HistoryEntry;
 
 import java.io.*;
@@ -1375,12 +1376,12 @@ abstract public class GeckoApp
                 if (tab == null)
                     return;
 
-                ViewportMetrics targetViewport = mLayerController.getViewportMetrics();
+                ImmutableViewportMetrics targetViewport = mLayerController.getViewportMetrics();
-                ViewportMetrics pluginViewport;
+                ImmutableViewportMetrics pluginViewport;
                 
                 try {
                     JSONObject viewportObject = new JSONObject(metadata);
-                    pluginViewport = new ViewportMetrics(viewportObject);
+                    pluginViewport = new ImmutableViewportMetrics(new ViewportMetrics(viewportObject));
                 } catch (JSONException e) {
                     Log.e(LOGTAG, "Bad viewport metadata: ", e);
                     return;
@@ -1570,7 +1571,7 @@ abstract public class GeckoApp
     }
 
     public void repositionPluginViews(Tab tab, boolean setVisible) {
-        ViewportMetrics targetViewport = mLayerController.getViewportMetrics();
+        ImmutableViewportMetrics targetViewport = mLayerController.getViewportMetrics();
 
         if (targetViewport == null)
             return;
@@ -2729,20 +2730,20 @@ class PluginLayoutParams extends AbsoluteLayout.LayoutParams
     private int mOriginalY;
     private int mOriginalWidth;
     private int mOriginalHeight;
-    private ViewportMetrics mOriginalViewport;
+    private ImmutableViewportMetrics mOriginalViewport;
     private float mLastResolution;
 
-    public PluginLayoutParams(int aX, int aY, int aWidth, int aHeight, ViewportMetrics aViewport) {
+    public PluginLayoutParams(int aX, int aY, int aWidth, int aHeight, ImmutableViewportMetrics aViewport) {
         super(aWidth, aHeight, aX, aY);
 
-        Log.i(LOGTAG, "Creating plugin at " + aX + ", " + aY + ", " + aWidth + "x" + aHeight + ", (" + (aViewport.getZoomFactor() * 100) + "%)");
+        Log.i(LOGTAG, "Creating plugin at " + aX + ", " + aY + ", " + aWidth + "x" + aHeight + ", (" + (aViewport.zoomFactor * 100) + "%)");
 
         mOriginalX = aX;
         mOriginalY = aY;
         mOriginalWidth = aWidth;
         mOriginalHeight = aHeight;
         mOriginalViewport = aViewport;
-        mLastResolution = aViewport.getZoomFactor();
+        mLastResolution = aViewport.zoomFactor;
 
         clampToMaxSize();
     }
@@ -2759,7 +2760,7 @@ class PluginLayoutParams extends AbsoluteLayout.LayoutParams
         }
     }
 
-    public void reset(int aX, int aY, int aWidth, int aHeight, ViewportMetrics aViewport) {
+    public void reset(int aX, int aY, int aWidth, int aHeight, ImmutableViewportMetrics aViewport) {
         PointF origin = aViewport.getOrigin();
 
         x = mOriginalX = aX + (int)origin.x;
@@ -2767,7 +2768,7 @@ class PluginLayoutParams extends AbsoluteLayout.LayoutParams
         width = mOriginalWidth = aWidth;
         height = mOriginalHeight = aHeight;
         mOriginalViewport = aViewport;
-        mLastResolution = aViewport.getZoomFactor();
+        mLastResolution = aViewport.zoomFactor;
 
         clampToMaxSize();
     }
@@ -2785,14 +2786,14 @@ class PluginLayoutParams extends AbsoluteLayout.LayoutParams
         }
     }
 
-    public void reposition(ViewportMetrics viewport) {
+    public void reposition(ImmutableViewportMetrics viewport) {
         PointF targetOrigin = viewport.getOrigin();
         PointF originalOrigin = mOriginalViewport.getOrigin();
 
         Point offset = new Point(Math.round(originalOrigin.x - targetOrigin.x),
                                  Math.round(originalOrigin.y - targetOrigin.y));
 
-        reposition(offset, viewport.getZoomFactor());
+        reposition(offset, viewport.zoomFactor);
     }
 
     public float getLastResolution() {

mobile/android/base/Makefile.in

@@ -116,6 +116,7 @@ FENNEC_JAVA_FILES = \
   gfx/GeckoLayerClient.java \
   gfx/GLController.java \
   gfx/GLThread.java \
+  gfx/ImmutableViewportMetrics.java \
   gfx/InputConnectionHandler.java \
   gfx/IntSize.java \
   gfx/Layer.java \

mobile/android/base/gfx/GeckoLayerClient.java

@@ -95,6 +95,9 @@ public class GeckoLayerClient implements GeckoEventResponder,
     /* Used by robocop for testing purposes */
     private DrawListener mDrawListener;
 
+    /* Used as a temporary ViewTransform by getViewTransform */
+    private ViewTransform mCurrentViewTransform;
+
     public GeckoLayerClient(Context context) {
         mScreenSize = new IntSize(0, 0);
         mBufferSize = new IntSize(0, 0);
@@ -102,6 +105,9 @@ public class GeckoLayerClient implements GeckoEventResponder,
                                        DEFAULT_DISPLAY_PORT_MARGIN,
                                        DEFAULT_DISPLAY_PORT_MARGIN,
                                        DEFAULT_DISPLAY_PORT_MARGIN);
+        // we can fill this in with dummy values because it is always written
+        // to before being read
+        mCurrentViewTransform = new ViewTransform(0, 0, 1);
     }
 
     /** Attaches the root layer to the layer controller so that Gecko appears. */
@@ -317,18 +323,19 @@ public class GeckoLayerClient implements GeckoEventResponder,
     }
 
     /** This function is invoked by Gecko via JNI; be careful when modifying signature. */
+    /* This functions needs to be fast because it is called by the compositor every frame.
+     * 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 could probably switch to returning a ImmutableViewportMetrics
+     * which would avoid the copy into mCurrentViewTransform. */
     public ViewTransform getViewTransform() {
-        // NB: We don't begin a transaction here because this can be called in a synchronous
+        // getViewportMetrics is thread safe so we don't need to synchronize
-        // manner between beginDrawing() and endDrawing(), and that will cause a deadlock.
+        // on myLayerController.
-
+        ImmutableViewportMetrics viewportMetrics = mLayerController.getViewportMetrics();
-        synchronized (mLayerController) {
+        mCurrentViewTransform.x = viewportMetrics.viewportRectLeft;
-            ViewportMetrics viewportMetrics = mLayerController.getViewportMetrics();
+        mCurrentViewTransform.y = viewportMetrics.viewportRectTop;
-            PointF viewportOrigin = viewportMetrics.getOrigin();
+        mCurrentViewTransform.scale = viewportMetrics.zoomFactor;
-            float scrollX = viewportOrigin.x; 
+        return mCurrentViewTransform;
-            float scrollY = viewportOrigin.y;
-            float zoomFactor = viewportMetrics.getZoomFactor();
-            return new ViewTransform(scrollX, scrollY, zoomFactor);
-        }
     }
 
     /** This function is invoked by Gecko via JNI; be careful when modifying signature. */

mobile/android/base/gfx/ImmutableViewportMetrics.java

@@ -0,0 +1,64 @@
+/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.gfx;
+
+import android.graphics.PointF;
+import android.graphics.RectF;
+
+/**
+ * ImmutableViewportMetrics are used to store the viewport metrics
+ * in way that we can access a version of them from multiple threads
+ * without having to take a lock
+ */
+public class ImmutableViewportMetrics {
+
+    // We need to flatten the RectF and FloatSize structures
+    // because Java doesn't have the concept of const classes
+    public final float pageSizeWidth;
+    public final float pageSizeHeight;
+    public final float viewportRectBottom;
+    public final float viewportRectLeft;
+    public final float viewportRectRight;
+    public final float viewportRectTop;
+    public final float zoomFactor;
+
+    public ImmutableViewportMetrics(ViewportMetrics m) {
+        RectF viewportRect = m.getViewport();
+        viewportRectBottom = viewportRect.bottom;
+        viewportRectLeft = viewportRect.left;
+        viewportRectRight = viewportRect.right;
+        viewportRectTop = viewportRect.top;
+
+        FloatSize pageSize = m.getPageSize();
+        pageSizeWidth = pageSize.width;
+        pageSizeHeight = pageSize.height;
+
+        zoomFactor = m.getZoomFactor();
+    }
+
+    // some helpers to make ImmutableViewportMetrics act more like ViewportMetrics
+
+    public PointF getOrigin() {
+        return new PointF(viewportRectLeft, viewportRectTop);
+    }
+
+    public FloatSize getSize() {
+        return new FloatSize(viewportRectRight - viewportRectLeft, viewportRectBottom - viewportRectTop);
+    }
+
+    public RectF getViewport() {
+        return new RectF(viewportRectLeft,
+                         viewportRectTop,
+                         viewportRectRight,
+                         viewportRectBottom);
+    }
+
+    public FloatSize getPageSize() {
+        return new FloatSize(pageSizeWidth, pageSizeHeight);
+    }
+
+
+}

mobile/android/base/gfx/LayerController.java

@@ -81,7 +81,20 @@ public class LayerController implements Tabs.OnTabsChangedListener {
     private Layer mRootLayer;                   /* The root layer. */
     private LayerView mView;                    /* The main rendering view. */
     private Context mContext;                   /* The current context. */
-    private ViewportMetrics mViewportMetrics;   /* The current viewport metrics. */
+
+    /* This is volatile so that we can read and write to it from different threads.
+     * We avoid synchronization to make getting the viewport metrics from
+     * the compositor as cheap as possible. The viewport is immutable so
+     * we don't need to worry about anyone mutating it while we're reading from it.
+     * Specifically:
+     * 1) reading mViewportMetrics from any thread is fine without synchronization
+     * 2) writing to mViewportMetrics requires synchronizing on the layer controller object
+     * 3) whenver reading multiple fields from mViewportMetrics without synchronization (i.e. in
+     *    case 1 above) you should always frist grab a local copy of the reference, and then use
+     *    that because mViewportMetrics might get reassigned in between reading the different
+     *    fields. */
+    private volatile ImmutableViewportMetrics mViewportMetrics;   /* The current viewport metrics. */
+
     private boolean mWaitForTouchListeners;
 
     private PanZoomController mPanZoomController;
@@ -124,7 +137,7 @@ public class LayerController implements Tabs.OnTabsChangedListener {
         mContext = context;
 
         mForceRedraw = true;
-        mViewportMetrics = new ViewportMetrics();
+        mViewportMetrics = new ImmutableViewportMetrics(new ViewportMetrics());
         mPanZoomController = new PanZoomController(this);
         mView = new LayerView(context, this);
 
@@ -152,7 +165,7 @@ public class LayerController implements Tabs.OnTabsChangedListener {
     public Layer getRoot()                        { return mRootLayer; }
     public LayerView getView()                    { return mView; }
     public Context getContext()                   { return mContext; }
-    public ViewportMetrics getViewportMetrics()   { return mViewportMetrics; }
+    public ImmutableViewportMetrics getViewportMetrics()   { return mViewportMetrics; }
 
     public RectF getViewport() {
         return mViewportMetrics.getViewport();
@@ -171,7 +184,7 @@ public class LayerController implements Tabs.OnTabsChangedListener {
     }
 
     public float getZoomFactor() {
-        return mViewportMetrics.getZoomFactor();
+        return mViewportMetrics.zoomFactor;
     }
 
     public Bitmap getBackgroundPattern()    { return getDrawable("background"); }
@@ -203,10 +216,11 @@ public class LayerController implements Tabs.OnTabsChangedListener {
     public void setViewportSize(FloatSize size) {
         // Resize the viewport, and modify its zoom factor so that the page retains proportionally
         // zoomed relative to the screen.
-        float oldHeight = mViewportMetrics.getSize().height;
+        ViewportMetrics viewportMetrics = new ViewportMetrics(mViewportMetrics);
-        float oldWidth = mViewportMetrics.getSize().width;
+        float oldHeight = viewportMetrics.getSize().height;
-        float oldZoomFactor = mViewportMetrics.getZoomFactor();
+        float oldWidth = viewportMetrics.getSize().width;
-        mViewportMetrics.setSize(size);
+        float oldZoomFactor = viewportMetrics.getZoomFactor();
+        viewportMetrics.setSize(size);
 
         // if the viewport got larger (presumably because the vkb went away), and the page
         // is smaller than the new viewport size, increase the page size so that the panzoomcontroller
@@ -214,9 +228,9 @@ public class LayerController implements Tabs.OnTabsChangedListener {
         // gecko increasing the page size to match the new viewport size, which will happen the next
         // time we get a draw update.
         if (size.width >= oldWidth && size.height >= oldHeight) {
-            FloatSize pageSize = mViewportMetrics.getPageSize();
+            FloatSize pageSize = viewportMetrics.getPageSize();
             if (pageSize.width < size.width || pageSize.height < size.height) {
-                mViewportMetrics.setPageSize(new FloatSize(Math.max(pageSize.width, size.width),
+                viewportMetrics.setPageSize(new FloatSize(Math.max(pageSize.width, size.width),
                                                            Math.max(pageSize.height, size.height)));
             }
         }
@@ -231,7 +245,8 @@ public class LayerController implements Tabs.OnTabsChangedListener {
             newFocus = new PointF(size.width / 2.0f, size.height / 2.0f);
         }
         float newZoomFactor = size.width * oldZoomFactor / oldWidth;
-        mViewportMetrics.scaleTo(newZoomFactor, newFocus);
+        viewportMetrics.scaleTo(newZoomFactor, newFocus);
+        mViewportMetrics = new ImmutableViewportMetrics(viewportMetrics);
 
         setForceRedraw();
 
@@ -246,9 +261,11 @@ public class LayerController implements Tabs.OnTabsChangedListener {
 
     /** Scrolls the viewport by the given offset. You must hold the monitor while calling this. */
     public void scrollBy(PointF point) {
-        PointF origin = mViewportMetrics.getOrigin();
+        ViewportMetrics viewportMetrics = new ViewportMetrics(mViewportMetrics);
+        PointF origin = viewportMetrics.getOrigin();
         origin.offset(point.x, point.y);
-        mViewportMetrics.setOrigin(origin);
+        viewportMetrics.setOrigin(origin);
+        mViewportMetrics = new ImmutableViewportMetrics(viewportMetrics);
 
         notifyLayerClientOfGeometryChange();
         GeckoApp.mAppContext.repositionPluginViews(false);
@@ -260,7 +277,9 @@ public class LayerController implements Tabs.OnTabsChangedListener {
         if (mViewportMetrics.getPageSize().fuzzyEquals(size))
             return;
 
-        mViewportMetrics.setPageSize(size);
+        ViewportMetrics viewportMetrics = new ViewportMetrics(mViewportMetrics);
+        viewportMetrics.setPageSize(size);
+        mViewportMetrics = new ImmutableViewportMetrics(viewportMetrics);
 
         // Page size is owned by the layer client, so no need to notify it of
         // this change.
@@ -280,7 +299,7 @@ public class LayerController implements Tabs.OnTabsChangedListener {
      * while calling this.
      */
     public void setViewportMetrics(ViewportMetrics viewport) {
-        mViewportMetrics = new ViewportMetrics(viewport);
+        mViewportMetrics = new ImmutableViewportMetrics(viewport);
         // this function may or may not be called on the UI thread,
         // but repositionPluginViews must only be called on the UI thread.
         GeckoApp.mAppContext.runOnUiThread(new Runnable() {
@@ -296,7 +315,9 @@ public class LayerController implements Tabs.OnTabsChangedListener {
      * scale operation. You must hold the monitor while calling this.
      */
     public void scaleWithFocus(float zoomFactor, PointF focus) {
-        mViewportMetrics.scaleTo(zoomFactor, focus);
+        ViewportMetrics viewportMetrics = new ViewportMetrics(mViewportMetrics);
+        viewportMetrics.scaleTo(zoomFactor, focus);
+        mViewportMetrics = new ImmutableViewportMetrics(viewportMetrics);
 
         // We assume the zoom level will only be modified by the
         // PanZoomController, so no need to notify it of this change.
@@ -375,10 +396,11 @@ public class LayerController implements Tabs.OnTabsChangedListener {
         if (mRootLayer == null)
             return null;
 
+        ImmutableViewportMetrics viewportMetrics = mViewportMetrics;
         // Undo the transforms.
-        PointF origin = mViewportMetrics.getOrigin();
+        PointF origin = viewportMetrics.getOrigin();
         PointF newPoint = new PointF(origin.x, origin.y);
-        float zoom = mViewportMetrics.getZoomFactor();
+        float zoom = viewportMetrics.zoomFactor;
         viewPoint.x /= zoom;
         viewPoint.y /= zoom;
         newPoint.offset(viewPoint.x, viewPoint.y);

mobile/android/base/gfx/ViewportMetrics.java

@@ -79,6 +79,16 @@ public class ViewportMetrics {
         mZoomFactor = viewport.getZoomFactor();
     }
 
+    public ViewportMetrics(ImmutableViewportMetrics viewport) {
+        mPageSize = new FloatSize(viewport.pageSizeWidth, viewport.pageSizeHeight);
+        mViewportRect = new RectF(viewport.viewportRectLeft,
+                                  viewport.viewportRectTop,
+                                  viewport.viewportRectRight,
+                                  viewport.viewportRectBottom);
+        mZoomFactor = viewport.zoomFactor;
+    }
+
+
     public ViewportMetrics(JSONObject json) throws JSONException {
         float x = (float)json.getDouble("x");
         float y = (float)json.getDouble("y");

mobile/android/base/ui/PanZoomController.java

@@ -228,7 +228,7 @@ public class PanZoomController
     public void pageSizeUpdated() {
         if (mState == PanZoomState.NOTHING) {
             ViewportMetrics validated = getValidViewportMetrics();
-            if (! mController.getViewportMetrics().fuzzyEquals(validated)) {
+            if (! (new ViewportMetrics(mController.getViewportMetrics())).fuzzyEquals(validated)) {
                 // page size changed such that we are now in overscroll. snap to the
                 // the nearest valid viewport
                 mController.setViewportMetrics(validated);