diff --git a/mobile/android/base/gfx/GeckoLayerClient.java b/mobile/android/base/gfx/GeckoLayerClient.java index 2d3b4d3b782a..e58945626a50 100644 --- a/mobile/android/base/gfx/GeckoLayerClient.java +++ b/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 */ diff --git a/mobile/android/base/gfx/Layer.java b/mobile/android/base/gfx/Layer.java index 1bb9aeb68ee8..373d98e3613f 100644 --- a/mobile/android/base/gfx/Layer.java +++ b/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; diff --git a/mobile/android/base/gfx/LayerController.java b/mobile/android/base/gfx/LayerController.java index f0131fdeef5a..edf12eab5f8d 100644 --- a/mobile/android/base/gfx/LayerController.java +++ b/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; } diff --git a/mobile/android/base/gfx/LayerRenderer.java b/mobile/android/base/gfx/LayerRenderer.java index f322443ab252..ed63d55715c0 100644 --- a/mobile/android/base/gfx/LayerRenderer.java +++ b/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); diff --git a/mobile/android/base/gfx/VirtualLayer.java b/mobile/android/base/gfx/VirtualLayer.java index 32d7dd9ad424..4665fa54fbff 100644 --- a/mobile/android/base/gfx/VirtualLayer.java +++ b/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; - } } diff --git a/widget/android/AndroidJavaWrappers.cpp b/widget/android/AndroidJavaWrappers.cpp index 040764046dc1..0e98af3a994c 100644 --- a/widget/android/AndroidJavaWrappers.cpp +++ b/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) { diff --git a/widget/android/AndroidJavaWrappers.h b/widget/android/AndroidJavaWrappers.h index 9aa01d9b1199..ab3afa776cdf 100644 --- a/widget/android/AndroidJavaWrappers.h +++ b/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; diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index 3d1b4a86cfc9..8259c1e2fe13 100644 --- a/widget/android/nsWindow.cpp +++ b/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 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