From 3a40cbe27bda40e18b95be43f45e07930ca02fda Mon Sep 17 00:00:00 2001 From: Chris Lord Date: Sat, 17 Mar 2012 15:08:22 +0000 Subject: [PATCH] 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 | 21 +++++++-- gfx/layers/ipc/CompositorParent.h | 7 +-- mobile/android/base/gfx/GeckoLayerClient.java | 46 ++++++++++++++----- mobile/android/base/gfx/Layer.java | 38 +++++++++++++-- mobile/android/base/gfx/LayerRenderer.java | 17 +++---- mobile/android/base/gfx/VirtualLayer.java | 9 ++++ widget/android/AndroidBridge.cpp | 4 +- widget/android/AndroidBridge.h | 2 +- widget/android/AndroidJavaWrappers.cpp | 14 +++--- widget/android/AndroidJavaWrappers.h | 4 +- 10 files changed, 119 insertions(+), 43 deletions(-) diff --git a/gfx/layers/ipc/CompositorParent.cpp b/gfx/layers/ipc/CompositorParent.cpp index 108b0493f49..7926e9b6556 100644 --- a/gfx/layers/ipc/CompositorParent.cpp +++ b/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 diff --git a/gfx/layers/ipc/CompositorParent.h b/gfx/layers/ipc/CompositorParent.h index 6aa65fae7f8..6af9e534d8f 100644 --- a/gfx/layers/ipc/CompositorParent.h +++ b/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 diff --git a/mobile/android/base/gfx/GeckoLayerClient.java b/mobile/android/base/gfx/GeckoLayerClient.java index 536b06e4a67..05db880491a 100644 --- a/mobile/android/base/gfx/GeckoLayerClient.java +++ b/mobile/android/base/gfx/GeckoLayerClient.java @@ -76,12 +76,24 @@ public class GeckoLayerClient implements GeckoEventResponder, /* The viewport that Gecko is currently displaying. */ 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. + */ + 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) { @@ -90,6 +102,7 @@ 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); } @@ -319,7 +332,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 +356,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 +376,30 @@ 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(); + + mCurrentViewTransform.x = mFrameMetrics.viewportRectLeft; + mCurrentViewTransform.y = mFrameMetrics.viewportRectTop; + mCurrentViewTransform.scale = mFrameMetrics.zoomFactor; + + mGeckoDisplayPort.set(x, y, x + width, y + height); + mRootLayer.setDisplayPort(mGeckoDisplayPort); + return mCurrentViewTransform; } @@ -389,7 +413,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); } diff --git a/mobile/android/base/gfx/Layer.java b/mobile/android/base/gfx/Layer.java index 373d98e3613..1bb9aeb68ee 100644 --- a/mobile/android/base/gfx/Layer.java +++ b/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; diff --git a/mobile/android/base/gfx/LayerRenderer.java b/mobile/android/base/gfx/LayerRenderer.java index 918aea14ac6..f322443ab25 100644 --- a/mobile/android/base/gfx/LayerRenderer.java +++ b/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); diff --git a/mobile/android/base/gfx/VirtualLayer.java b/mobile/android/base/gfx/VirtualLayer.java index 71971a91a13..32d7dd9ad42 100644 --- a/mobile/android/base/gfx/VirtualLayer.java +++ b/mobile/android/base/gfx/VirtualLayer.java @@ -71,4 +71,13 @@ public class VirtualLayer extends Layer { mPosition = newPosition; 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/AndroidBridge.cpp b/widget/android/AndroidBridge.cpp index 2d20b7d6dea..9e895fa1ff6 100644 --- a/widget/android/AndroidBridge.cpp +++ b/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() diff --git a/widget/android/AndroidBridge.h b/widget/android/AndroidBridge.h index 40a90a4c138..36a0ec1e6d2 100644 --- a/widget/android/AndroidBridge.h +++ b/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); diff --git a/widget/android/AndroidJavaWrappers.cpp b/widget/android/AndroidJavaWrappers.cpp index 775a1b72b76..3b7f341c026 100644 --- a/widget/android/AndroidJavaWrappers.cpp +++ b/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); diff --git a/widget/android/AndroidJavaWrappers.h b/widget/android/AndroidJavaWrappers.h index 127f49cf982..ea61ece6e7e 100644 --- a/widget/android/AndroidJavaWrappers.h +++ b/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;