From 589cb11de89d100a5b20765fe63d31b7fafc8a28 Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Fri, 2 Mar 2012 14:31:27 -0500 Subject: [PATCH] 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 | 25 ++++---- mobile/android/base/Makefile.in | 1 + mobile/android/base/gfx/GeckoLayerClient.java | 29 +++++---- .../base/gfx/ImmutableViewportMetrics.java | 64 +++++++++++++++++++ mobile/android/base/gfx/LayerController.java | 58 +++++++++++------ mobile/android/base/gfx/ViewportMetrics.java | 10 +++ mobile/android/base/ui/PanZoomController.java | 2 +- 7 files changed, 147 insertions(+), 42 deletions(-) create mode 100644 mobile/android/base/gfx/ImmutableViewportMetrics.java diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java index 9e31778b3bdf..b7f27b1ceb0c 100644 --- a/mobile/android/base/GeckoApp.java +++ b/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(); - ViewportMetrics pluginViewport; + ImmutableViewportMetrics targetViewport = mLayerController.getViewportMetrics(); + 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() { diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in index 41854a75e861..3a8c1509dc53 100644 --- a/mobile/android/base/Makefile.in +++ b/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 \ diff --git a/mobile/android/base/gfx/GeckoLayerClient.java b/mobile/android/base/gfx/GeckoLayerClient.java index a445c4e712e0..2bae0503230a 100644 --- a/mobile/android/base/gfx/GeckoLayerClient.java +++ b/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 - // manner between beginDrawing() and endDrawing(), and that will cause a deadlock. - - synchronized (mLayerController) { - ViewportMetrics viewportMetrics = mLayerController.getViewportMetrics(); - PointF viewportOrigin = viewportMetrics.getOrigin(); - float scrollX = viewportOrigin.x; - float scrollY = viewportOrigin.y; - float zoomFactor = viewportMetrics.getZoomFactor(); - return new ViewTransform(scrollX, scrollY, zoomFactor); - } + // 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; + return mCurrentViewTransform; } /** This function is invoked by Gecko via JNI; be careful when modifying signature. */ diff --git a/mobile/android/base/gfx/ImmutableViewportMetrics.java b/mobile/android/base/gfx/ImmutableViewportMetrics.java new file mode 100644 index 000000000000..d05a46cdf58b --- /dev/null +++ b/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); + } + + +} diff --git a/mobile/android/base/gfx/LayerController.java b/mobile/android/base/gfx/LayerController.java index 017057884a8b..30ade38b5efa 100644 --- a/mobile/android/base/gfx/LayerController.java +++ b/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; - float oldWidth = mViewportMetrics.getSize().width; - float oldZoomFactor = mViewportMetrics.getZoomFactor(); - mViewportMetrics.setSize(size); + ViewportMetrics viewportMetrics = new ViewportMetrics(mViewportMetrics); + float oldHeight = viewportMetrics.getSize().height; + float oldWidth = viewportMetrics.getSize().width; + 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); diff --git a/mobile/android/base/gfx/ViewportMetrics.java b/mobile/android/base/gfx/ViewportMetrics.java index 27ce82313942..ac5c57961ee1 100644 --- a/mobile/android/base/gfx/ViewportMetrics.java +++ b/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"); diff --git a/mobile/android/base/ui/PanZoomController.java b/mobile/android/base/ui/PanZoomController.java index 1b99154682e6..3eab15fea65a 100644 --- a/mobile/android/base/ui/PanZoomController.java +++ b/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);