From 4bcd4d40ff31a5261c9a27e1d7d9c25e0c438294 Mon Sep 17 00:00:00 2001 From: Mihai Popa Date: Wed, 7 Feb 2018 17:13:51 +0000 Subject: [PATCH 1/3] [Magnifier-20] Raw Surface instead of PopupWindow Since the movement of PopupWindows is handled by the global window manager, using one does not provide synchronization guarantees between the window movement and its view tree rendering. In the case of the magnifier this was producing a shaky-text effect, when the window was moved in one frame, but the content was only updated in the following one (although the movement and content update were issued during the same #show operation on the UI thread). This CL abandons the usage of a PopupWindow for the magnifier, in favour of a raw Surface whose movement can be controlled using the deferred transactions API. This way, we can defer the surface movement until the frame when the updated content is rendered. The allocated magnifier surface is slightly bigger than the magnifier content to have space for drawing the elevation. We obtain a native elevation by using a RenderNode to draw the content of the magnifier. Bug: 72041926 Test: atest CtsWidgetTestCases:android.widget.cts.MagnifierTest Change-Id: Icb8ecf48a2f528e06790db95d54575f2ae119747 --- core/java/android/view/ThreadedRenderer.java | 102 +++++++ core/java/android/widget/Magnifier.java | 299 +++++++++++++++---- 2 files changed, 350 insertions(+), 51 deletions(-) diff --git a/core/java/android/view/ThreadedRenderer.java b/core/java/android/view/ThreadedRenderer.java index e50d40ef098fb..6d42ac382a5d3 100644 --- a/core/java/android/view/ThreadedRenderer.java +++ b/core/java/android/view/ThreadedRenderer.java @@ -34,6 +34,7 @@ import android.os.Trace; import android.util.Log; import android.view.Surface.OutOfResourcesException; import android.view.View.AttachInfo; +import android.view.animation.AnimationUtils; import com.android.internal.R; import com.android.internal.util.VirtualRefBasePtr; @@ -943,6 +944,107 @@ public final class ThreadedRenderer { } } + /** + * Basic synchronous renderer. Currently only used to render the Magnifier, so use with care. + * TODO: deduplicate against ThreadedRenderer. + * + * @hide + */ + public static class SimpleRenderer { + private final RenderNode mRootNode; + private long mNativeProxy; + private final float mLightY, mLightZ; + private Surface mSurface; + private final FrameInfo mFrameInfo = new FrameInfo(); + + public SimpleRenderer(final Context context, final String name, final Surface surface) { + final TypedArray a = context.obtainStyledAttributes(null, R.styleable.Lighting, 0, 0); + mLightY = a.getDimension(R.styleable.Lighting_lightY, 0); + mLightZ = a.getDimension(R.styleable.Lighting_lightZ, 0); + final float lightRadius = a.getDimension(R.styleable.Lighting_lightRadius, 0); + final int ambientShadowAlpha = + (int) (255 * a.getFloat(R.styleable.Lighting_ambientShadowAlpha, 0) + 0.5f); + final int spotShadowAlpha = + (int) (255 * a.getFloat(R.styleable.Lighting_spotShadowAlpha, 0) + 0.5f); + a.recycle(); + + final long rootNodePtr = nCreateRootRenderNode(); + mRootNode = RenderNode.adopt(rootNodePtr); + mRootNode.setClipToBounds(false); + mNativeProxy = nCreateProxy(true /* translucent */, rootNodePtr); + nSetName(mNativeProxy, name); + + ProcessInitializer.sInstance.init(context, mNativeProxy); + nLoadSystemProperties(mNativeProxy); + + nSetup(mNativeProxy, lightRadius, ambientShadowAlpha, spotShadowAlpha); + + mSurface = surface; + nUpdateSurface(mNativeProxy, surface); + } + + /** + * Set the light center. + */ + public void setLightCenter(final Display display, + final int windowLeft, final int windowTop) { + // Adjust light position for window offsets. + final Point displaySize = new Point(); + display.getRealSize(displaySize); + final float lightX = displaySize.x / 2f - windowLeft; + final float lightY = mLightY - windowTop; + + nSetLightCenter(mNativeProxy, lightX, lightY, mLightZ); + } + + public RenderNode getRootNode() { + return mRootNode; + } + + /** + * Draw the surface. + */ + public void draw(final FrameDrawingCallback callback) { + final long vsync = AnimationUtils.currentAnimationTimeMillis() * 1000000L; + mFrameInfo.setVsync(vsync, vsync); + mFrameInfo.addFlags(1 << 2 /* VSYNC */); + if (callback != null) { + callback.onFrameDraw(mSurface.getNextFrameNumber()); + } + nSyncAndDrawFrame(mNativeProxy, mFrameInfo.mFrameInfo, mFrameInfo.mFrameInfo.length); + } + + /** + * Destroy the renderer. + */ + public void destroy() { + mSurface = null; + nDestroy(mNativeProxy, mRootNode.mNativeRenderNode); + } + + @Override + protected void finalize() throws Throwable { + try { + nDeleteProxy(mNativeProxy); + mNativeProxy = 0; + } finally { + super.finalize(); + } + } + } + + /** + * Interface used to receive callbacks when a frame is being drawn. + */ + public interface FrameDrawingCallback { + /** + * Invoked during a frame drawing. + * + * @param frame The id of the frame being drawn. + */ + void onFrameDraw(long frame); + } + private static class ProcessInitializer { static ProcessInitializer sInstance = new ProcessInitializer(); diff --git a/core/java/android/widget/Magnifier.java b/core/java/android/widget/Magnifier.java index 88365617cd6ef..f6f9b2eeed06c 100644 --- a/core/java/android/widget/Magnifier.java +++ b/core/java/android/widget/Magnifier.java @@ -18,21 +18,30 @@ package android.widget; import android.annotation.FloatRange; import android.annotation.NonNull; +import android.annotation.Nullable; import android.annotation.TestApi; import android.annotation.UiThread; import android.content.Context; import android.content.res.Resources; import android.graphics.Bitmap; +import android.graphics.Outline; +import android.graphics.Paint; +import android.graphics.PixelFormat; import android.graphics.Point; import android.graphics.PointF; import android.graphics.Rect; import android.os.Handler; -import android.view.Gravity; +import android.view.Display; +import android.view.DisplayListCanvas; import android.view.LayoutInflater; import android.view.PixelCopy; +import android.view.RenderNode; import android.view.Surface; +import android.view.SurfaceControl; import android.view.SurfaceHolder; +import android.view.SurfaceSession; import android.view.SurfaceView; +import android.view.ThreadedRenderer; import android.view.View; import android.view.ViewParent; import android.view.ViewRootImpl; @@ -51,7 +60,7 @@ public final class Magnifier { // The coordinates of the view in the surface. private final int[] mViewCoordinatesInSurface; // The window containing the magnifier. - private final PopupWindow mWindow; + private InternalPopupWindow mWindow; // The center coordinates of the window containing the magnifier. private final Point mWindowCoords = new Point(); // The width of the window containing the magnifier. @@ -60,6 +69,8 @@ public final class Magnifier { private final int mWindowHeight; // The bitmap used to display the contents of the magnifier. private final Bitmap mBitmap; + // The elevation of the window containing the magnifier. + private final float mWindowElevation; // The center coordinates of the content that is to be magnified. private final Point mCenterZoomCoords = new Point(); // The callback of the pixel copy request will be invoked on this Handler when @@ -72,6 +83,7 @@ public final class Magnifier { NONEXISTENT_PREVIOUS_CONFIG_VALUE, NONEXISTENT_PREVIOUS_CONFIG_VALUE); private final PointF mPrevPosInView = new PointF( NONEXISTENT_PREVIOUS_CONFIG_VALUE, NONEXISTENT_PREVIOUS_CONFIG_VALUE); + // Rectangle defining the view surface area we pixel copy content from. private final Rect mPixelCopyRequestRect = new Rect(); /** @@ -82,8 +94,6 @@ public final class Magnifier { public Magnifier(@NonNull View view) { mView = Preconditions.checkNotNull(view); final Context context = mView.getContext(); - final float elevation = context.getResources().getDimension( - com.android.internal.R.dimen.magnifier_elevation); final View content = LayoutInflater.from(context).inflate( com.android.internal.R.layout.magnifier, null); content.findViewById(com.android.internal.R.id.magnifier_inner).setClipToOutline(true); @@ -91,23 +101,16 @@ public final class Magnifier { com.android.internal.R.dimen.magnifier_width); mWindowHeight = context.getResources().getDimensionPixelSize( com.android.internal.R.dimen.magnifier_height); + mWindowElevation = context.getResources().getDimension( + com.android.internal.R.dimen.magnifier_elevation); mZoomScale = context.getResources().getFloat( com.android.internal.R.dimen.magnifier_zoom_scale); // The view's surface coordinates will not be updated until the magnifier is first shown. mViewCoordinatesInSurface = new int[2]; - mWindow = new PopupWindow(context); - mWindow.setContentView(content); - mWindow.setWidth(mWindowWidth); - mWindow.setHeight(mWindowHeight); - mWindow.setElevation(elevation); - mWindow.setTouchable(false); - mWindow.setBackgroundDrawable(null); - final int bitmapWidth = Math.round(mWindowWidth / mZoomScale); final int bitmapHeight = Math.round(mWindowHeight / mZoomScale); mBitmap = Bitmap.createBitmap(bitmapWidth, bitmapHeight, Bitmap.Config.ARGB_8888); - getImageView().setImageBitmap(mBitmap); } /** @@ -160,25 +163,39 @@ public final class Magnifier { final int startY = mCenterZoomCoords.y - mBitmap.getHeight() / 2; if (xPosInView != mPrevPosInView.x || yPosInView != mPrevPosInView.y) { - performPixelCopy(startX, startY); - + if (mWindow == null) { + mWindow = new InternalPopupWindow(mView.getContext(), mView.getDisplay(), + getValidViewSurface(), mWindowWidth, mWindowHeight, mWindowElevation); + } + performPixelCopy(startX, startY, true /* update window position */); mPrevPosInView.x = xPosInView; mPrevPosInView.y = yPosInView; - - if (mWindow.isShowing()) { - mWindow.update(mWindowCoords.x, mWindowCoords.y, mWindow.getWidth(), - mWindow.getHeight()); - } else { - mWindow.showAtLocation(mView, Gravity.NO_GRAVITY, mWindowCoords.x, mWindowCoords.y); - } } } + @Nullable + private Surface getValidViewSurface() { + // TODO: deduplicate this against the first part of #performPixelCopy + final Surface surface; + if (mView instanceof SurfaceView) { + surface = ((SurfaceView) mView).getHolder().getSurface(); + } else if (mView.getViewRootImpl() != null) { + surface = mView.getViewRootImpl().mSurface; + } else { + surface = null; + } + + return (surface != null && surface.isValid()) ? surface : null; + } + /** * Dismisses the magnifier from the screen. Calling this on a dismissed magnifier is a no-op. */ public void dismiss() { - mWindow.dismiss(); + if (mWindow != null) { + mWindow.destroy(); + mWindow = null; + } } /** @@ -186,43 +203,40 @@ public final class Magnifier { * {@link #show(float, float)}. This only happens if the magnifier is currently showing. */ public void update() { - if (mWindow.isShowing()) { - // Update the contents shown in the magnifier. - performPixelCopy(mPrevStartCoordsInSurface.x, mPrevStartCoordsInSurface.y); + if (mWindow != null) { + // Update the content shown in the magnifier. + performPixelCopy(mPrevStartCoordsInSurface.x, mPrevStartCoordsInSurface.y, + false /* update window position */); } } private void configureCoordinates(final float xPosInView, final float yPosInView) { // Compute the coordinates of the center of the content going to be displayed in the // magnifier. These are relative to the surface the content is copied from. - final float contentPosX; - final float contentPosY; + final float posX; + final float posY; if (mView instanceof SurfaceView) { // No offset required if the backing Surface matches the size of the SurfaceView. - contentPosX = xPosInView; - contentPosY = yPosInView; + posX = xPosInView; + posY = yPosInView; } else { mView.getLocationInSurface(mViewCoordinatesInSurface); - contentPosX = xPosInView + mViewCoordinatesInSurface[0]; - contentPosY = yPosInView + mViewCoordinatesInSurface[1]; + posX = xPosInView + mViewCoordinatesInSurface[0]; + posY = yPosInView + mViewCoordinatesInSurface[1]; } - mCenterZoomCoords.x = Math.round(contentPosX); - mCenterZoomCoords.y = Math.round(contentPosY); + mCenterZoomCoords.x = Math.round(posX); + mCenterZoomCoords.y = Math.round(posY); - // Compute the position of the magnifier window. These have to be relative to the window - // of the view the magnifier is attached to, as the magnifier popup is a panel window - // attached to that window. - final int[] viewCoordinatesInWindow = new int[2]; - mView.getLocationInWindow(viewCoordinatesInWindow); + // Compute the position of the magnifier window. Again, this has to be relative to the + // surface of the magnified view, as this surface is the parent of the magnifier surface. final int verticalOffset = mView.getContext().getResources().getDimensionPixelSize( com.android.internal.R.dimen.magnifier_offset); - final float magnifierPosX = xPosInView + viewCoordinatesInWindow[0]; - final float magnifierPosY = yPosInView + viewCoordinatesInWindow[1] - verticalOffset; - mWindowCoords.x = Math.round(magnifierPosX - mWindowWidth / 2); - mWindowCoords.y = Math.round(magnifierPosY - mWindowHeight / 2); + mWindowCoords.x = mCenterZoomCoords.x - mWindowWidth / 2; + mWindowCoords.y = mCenterZoomCoords.y - mWindowHeight / 2 - verticalOffset; } - private void performPixelCopy(final int startXInSurface, final int startYInSurface) { + private void performPixelCopy(final int startXInSurface, final int startYInSurface, + final boolean updateWindowPosition) { // Get the view surface where the content will be copied from. final Surface surface; final int surfaceWidth; @@ -258,18 +272,201 @@ public final class Magnifier { clampedStartYInSurface, clampedStartXInSurface + mBitmap.getWidth(), clampedStartYInSurface + mBitmap.getHeight()); + final int windowCoordsX = mWindowCoords.x; + final int windowCoordsY = mWindowCoords.y; PixelCopy.request(surface, mPixelCopyRequestRect, mBitmap, result -> { - getImageView().invalidate(); - mPrevStartCoordsInSurface.x = startXInSurface; - mPrevStartCoordsInSurface.y = startYInSurface; + if (mWindow != null) { + if (updateWindowPosition) { + // TODO: pull the magnifier position update outside #performPixelCopy + mWindow.setContentPositionForNextDraw(windowCoordsX, windowCoordsY); + } + mWindow.updateContent(mBitmap); + } }, mPixelCopyHandler); + mPrevStartCoordsInSurface.x = startXInSurface; + mPrevStartCoordsInSurface.y = startYInSurface; } - private ImageView getImageView() { - return mWindow.getContentView().findViewById( - com.android.internal.R.id.magnifier_image); + /** + * Magnifier's own implementation of PopupWindow-similar floating window. + * This exists to ensure frame-synchronization between window position updates and window + * content updates. By using a PopupWindow, these events would happen in different frames, + * producing a shakiness effect for the magnifier content. + */ + private static class InternalPopupWindow { + // Display associated to the view the magnifier is attached to. + private final Display mDisplay; + // The size of the content of the magnifier. + private final int mContentWidth; + private final int mContentHeight; + // The size of the allocated surface. + private final int mSurfaceWidth; + private final int mSurfaceHeight; + // The insets of the content inside the allocated surface. + private final int mOffsetX; + private final int mOffsetY; + // Whether the next draw will be the first one for the current instance. + private boolean mFirstDraw = true; + // The window position in the parent surface. Might be applied during the next draw, + // when mPendingWindowPositionUpdate is true. + private int mWindowPositionX; + private int mWindowPositionY; + private boolean mPendingWindowPositionUpdate; + // The surface we allocate for the magnifier content + shadow. + private final SurfaceSession mSurfaceSession; + private final SurfaceControl mSurfaceControl; + private final Surface mSurface; + // The renderer used for the allocated surface. + private final ThreadedRenderer.SimpleRenderer mRenderer; + // The RenderNode used to draw the magnifier content in the surface. + private final RenderNode mBitmapRenderNode; + + InternalPopupWindow(final Context context, final Display display, + final Surface parentSurface, + final int width, final int height, final float elevation) { + mDisplay = display; + + mContentWidth = width; + mContentHeight = height; + mOffsetX = (int) (0.1f * width); + mOffsetY = (int) (0.1f * height); + // Setup the surface we will use for drawing the content and shadow. + mSurfaceWidth = mContentWidth + 2 * mOffsetX; + mSurfaceHeight = mContentHeight + 2 * mOffsetY; + mSurfaceSession = new SurfaceSession(parentSurface); + mSurfaceControl = new SurfaceControl.Builder(mSurfaceSession) + .setFormat(PixelFormat.TRANSLUCENT) + .setSize(mSurfaceWidth, mSurfaceHeight) + .setName("magnifier surface") + .setFlags(SurfaceControl.HIDDEN) + .build(); + mSurface = new Surface(); + mSurface.copyFrom(mSurfaceControl); + + // Setup the RenderNode tree. The root has only one child, which contains the bitmap. + mRenderer = new ThreadedRenderer.SimpleRenderer( + context, + "magnifier renderer", + mSurface + ); + mBitmapRenderNode = createRenderNodeForBitmap( + "magnifier content", + elevation + ); + final DisplayListCanvas canvas = mRenderer.getRootNode().start(width, height); + try { + canvas.insertReorderBarrier(); + canvas.drawRenderNode(mBitmapRenderNode); + canvas.insertInorderBarrier(); + } finally { + mRenderer.getRootNode().end(canvas); + } + } + + private RenderNode createRenderNodeForBitmap(final String name, final float elevation) { + final RenderNode bitmapRenderNode = RenderNode.create(name, null); + + // Define the position of the bitmap in the parent render node. The surface regions + // outside the bitmap are used to draw elevation. + bitmapRenderNode.setLeftTopRightBottom(mOffsetX, mOffsetY, + mOffsetX + mContentWidth, mOffsetY + mContentHeight); + bitmapRenderNode.setElevation(elevation); + + final Outline outline = new Outline(); + outline.setRoundRect(0, 0, mContentWidth, mContentHeight, 3); + outline.setAlpha(1.0f); + bitmapRenderNode.setOutline(outline); + bitmapRenderNode.setClipToOutline(true); + + // Create a dummy draw, which will be replaced later with real drawing. + final DisplayListCanvas canvas = bitmapRenderNode.start(mContentWidth, mContentHeight); + try { + canvas.drawColor(0xFF00FF00); + } finally { + bitmapRenderNode.end(canvas); + } + + return bitmapRenderNode; + } + + /** + * Sets the position of the magnifier content relative to the parent surface. + * The position update will happen in the same frame with the next draw. + * + * @param contentX the x coordinate of the content + * @param contentY the y coordinate of the content + */ + public void setContentPositionForNextDraw(final int contentX, final int contentY) { + mWindowPositionX = contentX - mOffsetX; + mWindowPositionY = contentY - mOffsetY; + mPendingWindowPositionUpdate = true; + } + + /** + * Sets the content that should be displayed in the magnifier. + * The update happens immediately, and possibly triggers a pending window movement set + * by {@link #setContentPositionForNextDraw(int, int)}. + * + * @param bitmap the content bitmap + */ + public void updateContent(final Bitmap bitmap) { + final DisplayListCanvas canvas = mBitmapRenderNode.start(mContentWidth, mContentHeight); + try { + final Rect srcRect = new Rect(0, 0, bitmap.getWidth(), bitmap.getHeight()); + final Rect dstRect = new Rect(0, 0, mContentWidth, mContentHeight); + final Paint paint = new Paint(); + paint.setFilterBitmap(true); + canvas.drawBitmap(bitmap, srcRect, dstRect, paint); + } finally { + mBitmapRenderNode.end(canvas); + } + + doDraw(); + } + + /** + * Destroys this instance. + */ + public void destroy() { + mRenderer.destroy(); + mSurface.destroy(); + mSurfaceControl.destroy(); + mSurfaceSession.kill(); + mBitmapRenderNode.destroy(); + } + + private void doDraw() { + final ThreadedRenderer.FrameDrawingCallback callback; + if (mPendingWindowPositionUpdate || mFirstDraw) { + // If the window has to be shown or moved, defer this until the next draw. + final boolean firstDraw = mFirstDraw; + mFirstDraw = false; + final boolean updateWindowPosition = mPendingWindowPositionUpdate; + mPendingWindowPositionUpdate = false; + final int pendingX = mWindowPositionX; + final int pendingY = mWindowPositionY; + + callback = frame -> { + mRenderer.setLightCenter(mDisplay, pendingX, pendingY); + // Show or move the window at the content draw frame. + SurfaceControl.openTransaction(); + mSurfaceControl.deferTransactionUntil(mSurface, frame); + if (updateWindowPosition) { + mSurfaceControl.setPosition(pendingX, pendingY); + } + if (firstDraw) { + mSurfaceControl.show(); + } + SurfaceControl.closeTransaction(); + }; + } else { + callback = null; + } + + mRenderer.draw(callback); + } } /** @@ -296,7 +493,7 @@ public final class Magnifier { final int left = mWindowCoords.x + viewLocationOnScreen[0] - viewLocationInSurface[0]; final int top = mWindowCoords.y + viewLocationOnScreen[1] - viewLocationInSurface[1]; - return new Rect(left, top, left + mWindow.getWidth(), top + mWindow.getHeight()); + return new Rect(left, top, left + mWindowWidth, top + mWindowHeight); } /** From 8b78910635b2728e4473cb75d9403d83d001ac29 Mon Sep 17 00:00:00 2001 From: Mihai Popa Date: Thu, 15 Feb 2018 12:06:59 +0000 Subject: [PATCH 2/3] [Magnifier-21] Rate-limit drawings to renderer Previously, we would make a renderer draw whenever a pixel copy completes. Because of this happening more than once per view frame, magnifier frames were being dropped or queued up and displayed later, which was consequently leading to the magnifier movement getting out of sync with the updates to its content. This CL changes the magnifier to be rendered from a draw job post'd to the UI thread queue. This way, multiple magnifier updates (the ones already pending in the UI thread queue when the draw job is added) are batched together in a single draw, naturally rate-limiting the draws we send to the magnifier renderer by the number of frames of the magnified view - this only holds when the user of the magnifier sends updates as a result of user interaction or when the magnified view is drawn. Also, previously the pixel copy finished events were post'd to the UI thread. Since they were not post'd as async messages, they would have to wait at frame barriers before being executed. This CL creates a dedicated thread for them to be post'd, to avoid this from happening. Bug: 72041926 Test: atest CtsWidgetTestCases:android.widget.cts.MagnifierTest Change-Id: I89563a341a74e958f903eff2d470c6c33fb203ef --- core/java/android/view/ThreadedRenderer.java | 2 + core/java/android/widget/Magnifier.java | 210 +++++++++++++------ 2 files changed, 144 insertions(+), 68 deletions(-) diff --git a/core/java/android/view/ThreadedRenderer.java b/core/java/android/view/ThreadedRenderer.java index 6d42ac382a5d3..f9ad603e9d1cb 100644 --- a/core/java/android/view/ThreadedRenderer.java +++ b/core/java/android/view/ThreadedRenderer.java @@ -1008,6 +1008,8 @@ public final class ThreadedRenderer { final long vsync = AnimationUtils.currentAnimationTimeMillis() * 1000000L; mFrameInfo.setVsync(vsync, vsync); mFrameInfo.addFlags(1 << 2 /* VSYNC */); + // TODO: remove this fence + nFence(mNativeProxy); if (callback != null) { callback.onFrameDraw(mSurface.getNextFrameNumber()); } diff --git a/core/java/android/widget/Magnifier.java b/core/java/android/widget/Magnifier.java index f6f9b2eeed06c..53db2c2aebb2f 100644 --- a/core/java/android/widget/Magnifier.java +++ b/core/java/android/widget/Magnifier.java @@ -31,6 +31,8 @@ import android.graphics.Point; import android.graphics.PointF; import android.graphics.Rect; import android.os.Handler; +import android.os.HandlerThread; +import android.os.Message; import android.view.Display; import android.view.DisplayListCanvas; import android.view.LayoutInflater; @@ -55,6 +57,11 @@ import com.android.internal.util.Preconditions; public final class Magnifier { // Use this to specify that a previous configuration value does not exist. private static final int NONEXISTENT_PREVIOUS_CONFIG_VALUE = -1; + // The callbacks of the pixel copy requests will be invoked on + // the Handler of this Thread when the copy is finished. + private static final HandlerThread sPixelCopyHandlerThread = + new HandlerThread("magnifier pixel copy result handler"); + // The view to which this magnifier is attached. private final View mView; // The coordinates of the view in the surface. @@ -67,17 +74,14 @@ public final class Magnifier { private final int mWindowWidth; // The height of the window containing the magnifier. private final int mWindowHeight; - // The bitmap used to display the contents of the magnifier. - private final Bitmap mBitmap; + // The width of the bitmaps where the magnifier content is copied. + private final int mBitmapWidth; + // The height of the bitmaps where the magnifier content is copied. + private final int mBitmapHeight; // The elevation of the window containing the magnifier. private final float mWindowElevation; // The center coordinates of the content that is to be magnified. private final Point mCenterZoomCoords = new Point(); - // The callback of the pixel copy request will be invoked on this Handler when - // the copy is finished. - private final Handler mPixelCopyHandler = Handler.getMain(); - // Current magnification scale. - private final float mZoomScale; // Variables holding previous states, used for detecting redundant calls and invalidation. private final Point mPrevStartCoordsInSurface = new Point( NONEXISTENT_PREVIOUS_CONFIG_VALUE, NONEXISTENT_PREVIOUS_CONFIG_VALUE); @@ -103,14 +107,16 @@ public final class Magnifier { com.android.internal.R.dimen.magnifier_height); mWindowElevation = context.getResources().getDimension( com.android.internal.R.dimen.magnifier_elevation); - mZoomScale = context.getResources().getFloat( + final float zoomScale = context.getResources().getFloat( com.android.internal.R.dimen.magnifier_zoom_scale); + mBitmapWidth = Math.round(mWindowWidth / zoomScale); + mBitmapHeight = Math.round(mWindowHeight / zoomScale); // The view's surface coordinates will not be updated until the magnifier is first shown. mViewCoordinatesInSurface = new int[2]; + } - final int bitmapWidth = Math.round(mWindowWidth / mZoomScale); - final int bitmapHeight = Math.round(mWindowHeight / mZoomScale); - mBitmap = Bitmap.createBitmap(bitmapWidth, bitmapHeight, Bitmap.Config.ARGB_8888); + static { + sPixelCopyHandlerThread.start(); } /** @@ -158,14 +164,15 @@ public final class Magnifier { } final int startX = Math.max(zeroScrollXInSurface, Math.min( - mCenterZoomCoords.x - mBitmap.getWidth() / 2, - zeroScrollXInSurface + actualWidth - mBitmap.getWidth())); - final int startY = mCenterZoomCoords.y - mBitmap.getHeight() / 2; + mCenterZoomCoords.x - mBitmapWidth / 2, + zeroScrollXInSurface + actualWidth - mBitmapWidth)); + final int startY = mCenterZoomCoords.y - mBitmapHeight / 2; if (xPosInView != mPrevPosInView.x || yPosInView != mPrevPosInView.y) { if (mWindow == null) { mWindow = new InternalPopupWindow(mView.getContext(), mView.getDisplay(), - getValidViewSurface(), mWindowWidth, mWindowHeight, mWindowElevation); + getValidViewSurface(), mWindowWidth, mWindowHeight, mWindowElevation, + Handler.getMain() /* draw the magnifier on the UI thread */); } performPixelCopy(startX, startY, true /* update window position */); mPrevPosInView.x = xPosInView; @@ -270,21 +277,26 @@ public final class Magnifier { // Perform the pixel copy. mPixelCopyRequestRect.set(clampedStartXInSurface, clampedStartYInSurface, - clampedStartXInSurface + mBitmap.getWidth(), - clampedStartYInSurface + mBitmap.getHeight()); + clampedStartXInSurface + mBitmapWidth, + clampedStartYInSurface + mBitmapHeight); final int windowCoordsX = mWindowCoords.x; final int windowCoordsY = mWindowCoords.y; - PixelCopy.request(surface, mPixelCopyRequestRect, mBitmap, + + final Bitmap bitmap = + Bitmap.createBitmap(mBitmapWidth, mBitmapHeight, Bitmap.Config.ARGB_8888); + PixelCopy.request(surface, mPixelCopyRequestRect, bitmap, result -> { - if (mWindow != null) { - if (updateWindowPosition) { - // TODO: pull the magnifier position update outside #performPixelCopy - mWindow.setContentPositionForNextDraw(windowCoordsX, windowCoordsY); + synchronized (mWindow.mLock) { + if (mWindow != null) { + if (updateWindowPosition) { + // TODO: pull the position update outside #performPixelCopy + mWindow.setContentPositionForNextDraw(windowCoordsX, windowCoordsY); + } + mWindow.updateContent(bitmap); } - mWindow.updateContent(mBitmap); } }, - mPixelCopyHandler); + sPixelCopyHandlerThread.getThreadHandler()); mPrevStartCoordsInSurface.x = startXInSurface; mPrevStartCoordsInSurface.y = startYInSurface; } @@ -307,13 +319,6 @@ public final class Magnifier { // The insets of the content inside the allocated surface. private final int mOffsetX; private final int mOffsetY; - // Whether the next draw will be the first one for the current instance. - private boolean mFirstDraw = true; - // The window position in the parent surface. Might be applied during the next draw, - // when mPendingWindowPositionUpdate is true. - private int mWindowPositionX; - private int mWindowPositionY; - private boolean mPendingWindowPositionUpdate; // The surface we allocate for the magnifier content + shadow. private final SurfaceSession mSurfaceSession; private final SurfaceControl mSurfaceControl; @@ -322,10 +327,31 @@ public final class Magnifier { private final ThreadedRenderer.SimpleRenderer mRenderer; // The RenderNode used to draw the magnifier content in the surface. private final RenderNode mBitmapRenderNode; + // The job that will be post'd to apply the pending magnifier updates to the surface. + private final Runnable mMagnifierUpdater; + // The handler where the magnifier updater jobs will be post'd. + private final Handler mHandler; + + // Members below describe the state of the magnifier. Reads/writes to them + // have to be synchronized between the UI thread and the thread that handles + // the pixel copy results. This is the purpose of mLock. + private final Object mLock = new Object(); + // Whether a magnifier frame draw is currently pending in the UI thread queue. + private boolean mFrameDrawScheduled; + // The content bitmap. + private Bitmap mBitmap; + // Whether the next draw will be the first one for the current instance. + private boolean mFirstDraw = true; + // The window position in the parent surface. Might be applied during the next draw, + // when mPendingWindowPositionUpdate is true. + private int mWindowPositionX; + private int mWindowPositionY; + private boolean mPendingWindowPositionUpdate; InternalPopupWindow(final Context context, final Display display, final Surface parentSurface, - final int width, final int height, final float elevation) { + final int width, final int height, final float elevation, + final Handler handler) { mDisplay = display; mContentWidth = width; @@ -355,6 +381,7 @@ public final class Magnifier { "magnifier content", elevation ); + final DisplayListCanvas canvas = mRenderer.getRootNode().start(width, height); try { canvas.insertReorderBarrier(); @@ -363,6 +390,11 @@ public final class Magnifier { } finally { mRenderer.getRootNode().end(canvas); } + + // Initialize the update job and the handler where this will be post'd. + mHandler = handler; + mMagnifierUpdater = this::doDraw; + mFrameDrawScheduled = false; } private RenderNode createRenderNodeForBitmap(final String name, final float elevation) { @@ -394,6 +426,7 @@ public final class Magnifier { /** * Sets the position of the magnifier content relative to the parent surface. * The position update will happen in the same frame with the next draw. + * The method has to be called in a context that holds {@link #mLock}. * * @param contentX the x coordinate of the content * @param contentY the y coordinate of the content @@ -402,28 +435,33 @@ public final class Magnifier { mWindowPositionX = contentX - mOffsetX; mWindowPositionY = contentY - mOffsetY; mPendingWindowPositionUpdate = true; + requestUpdate(); } /** * Sets the content that should be displayed in the magnifier. * The update happens immediately, and possibly triggers a pending window movement set * by {@link #setContentPositionForNextDraw(int, int)}. + * The method has to be called in a context that holds {@link #mLock}. * * @param bitmap the content bitmap */ - public void updateContent(final Bitmap bitmap) { - final DisplayListCanvas canvas = mBitmapRenderNode.start(mContentWidth, mContentHeight); - try { - final Rect srcRect = new Rect(0, 0, bitmap.getWidth(), bitmap.getHeight()); - final Rect dstRect = new Rect(0, 0, mContentWidth, mContentHeight); - final Paint paint = new Paint(); - paint.setFilterBitmap(true); - canvas.drawBitmap(bitmap, srcRect, dstRect, paint); - } finally { - mBitmapRenderNode.end(canvas); + public void updateContent(final @NonNull Bitmap bitmap) { + if (mBitmap != null) { + mBitmap.recycle(); } + mBitmap = bitmap; + requestUpdate(); + } - doDraw(); + private void requestUpdate() { + if (mFrameDrawScheduled) { + return; + } + final Message request = Message.obtain(mHandler, mMagnifierUpdater); + request.setAsynchronous(true); + request.sendToTarget(); + mFrameDrawScheduled = true; } /** @@ -435,34 +473,65 @@ public final class Magnifier { mSurfaceControl.destroy(); mSurfaceSession.kill(); mBitmapRenderNode.destroy(); + synchronized (mLock) { + mHandler.removeCallbacks(mMagnifierUpdater); + if (mBitmap != null) { + mBitmap.recycle(); + } + } } private void doDraw() { final ThreadedRenderer.FrameDrawingCallback callback; - if (mPendingWindowPositionUpdate || mFirstDraw) { - // If the window has to be shown or moved, defer this until the next draw. - final boolean firstDraw = mFirstDraw; - mFirstDraw = false; - final boolean updateWindowPosition = mPendingWindowPositionUpdate; - mPendingWindowPositionUpdate = false; - final int pendingX = mWindowPositionX; - final int pendingY = mWindowPositionY; - callback = frame -> { - mRenderer.setLightCenter(mDisplay, pendingX, pendingY); - // Show or move the window at the content draw frame. - SurfaceControl.openTransaction(); - mSurfaceControl.deferTransactionUntil(mSurface, frame); - if (updateWindowPosition) { - mSurfaceControl.setPosition(pendingX, pendingY); - } - if (firstDraw) { - mSurfaceControl.show(); - } - SurfaceControl.closeTransaction(); - }; - } else { - callback = null; + // Draw the current bitmap to the surface, and prepare the callback which updates the + // surface position. These have to be in the same synchronized block, in order to + // guarantee the consistency between the bitmap content and the surface position. + synchronized (mLock) { + if (!mSurface.isValid()) { + // Probably #destroy() was called for the current instance, so we skip the draw. + return; + } + + final DisplayListCanvas canvas = + mBitmapRenderNode.start(mContentWidth, mContentHeight); + try { + final Rect srcRect = new Rect(0, 0, mBitmap.getWidth(), mBitmap.getHeight()); + final Rect dstRect = new Rect(0, 0, mContentWidth, mContentHeight); + final Paint paint = new Paint(); + paint.setFilterBitmap(true); + canvas.drawBitmap(mBitmap, srcRect, dstRect, paint); + } finally { + mBitmapRenderNode.end(canvas); + } + + if (mPendingWindowPositionUpdate || mFirstDraw) { + // If the window has to be shown or moved, defer this until the next draw. + final boolean firstDraw = mFirstDraw; + mFirstDraw = false; + final boolean updateWindowPosition = mPendingWindowPositionUpdate; + mPendingWindowPositionUpdate = false; + final int pendingX = mWindowPositionX; + final int pendingY = mWindowPositionY; + + callback = frame -> { + mRenderer.setLightCenter(mDisplay, pendingX, pendingY); + // Show or move the window at the content draw frame. + SurfaceControl.openTransaction(); + mSurfaceControl.deferTransactionUntil(mSurface, frame); + if (updateWindowPosition) { + mSurfaceControl.setPosition(pendingX, pendingY); + } + if (firstDraw) { + mSurfaceControl.show(); + } + SurfaceControl.closeTransaction(); + }; + } else { + callback = null; + } + + mFrameDrawScheduled = false; } mRenderer.draw(callback); @@ -475,8 +544,13 @@ public final class Magnifier { * @hide */ @TestApi - public Bitmap getContent() { - return mBitmap; + public @Nullable Bitmap getContent() { + if (mWindow == null) { + return null; + } + synchronized (mWindow.mLock) { + return mWindow.mBitmap; + } } /** From 2ba5d8e8e8d55e7bbf7727ab6cc0065692a14496 Mon Sep 17 00:00:00 2001 From: Mihai Popa Date: Tue, 20 Feb 2018 18:50:20 +0000 Subject: [PATCH 3/3] [Magnifier-24] Add completion callback TestApi The CL adds a TestApi callback mechanism which enables CTS tests to know when magnifier operations have completed. This is required since the magnifier is using an auxiliary thread (different from the main thread) to do work. Bug: 72041926 Test: atest CtsWidgetTestCases:android.widget.cts.MagnifierTest Change-Id: I44f324297b63910d19d60bd7462e9aee8d8a4f3e --- api/test-current.txt | 5 +++ core/java/android/widget/Magnifier.java | 43 +++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/api/test-current.txt b/api/test-current.txt index 92251f74a2944..0bfc3aa55934f 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -1096,6 +1096,11 @@ package android.widget { method public android.graphics.Bitmap getContent(); method public static android.graphics.PointF getMagnifierDefaultSize(); method public android.graphics.Rect getWindowPositionOnScreen(); + method public void setOnOperationCompleteCallback(android.widget.Magnifier.Callback); + } + + public static abstract interface Magnifier.Callback { + method public abstract void onOperationComplete(); } public class NumberPicker extends android.widget.LinearLayout { diff --git a/core/java/android/widget/Magnifier.java b/core/java/android/widget/Magnifier.java index 53db2c2aebb2f..64279655f799b 100644 --- a/core/java/android/widget/Magnifier.java +++ b/core/java/android/widget/Magnifier.java @@ -172,7 +172,7 @@ public final class Magnifier { if (mWindow == null) { mWindow = new InternalPopupWindow(mView.getContext(), mView.getDisplay(), getValidViewSurface(), mWindowWidth, mWindowHeight, mWindowElevation, - Handler.getMain() /* draw the magnifier on the UI thread */); + Handler.getMain() /* draw the magnifier on the UI thread */, mCallback); } performPixelCopy(startX, startY, true /* update window position */); mPrevPosInView.x = xPosInView; @@ -331,6 +331,8 @@ public final class Magnifier { private final Runnable mMagnifierUpdater; // The handler where the magnifier updater jobs will be post'd. private final Handler mHandler; + // The callback to be run after the next draw. Only used for testing. + private Callback mCallback; // Members below describe the state of the magnifier. Reads/writes to them // have to be synchronized between the UI thread and the thread that handles @@ -351,8 +353,9 @@ public final class Magnifier { InternalPopupWindow(final Context context, final Display display, final Surface parentSurface, final int width, final int height, final float elevation, - final Handler handler) { + final Handler handler, final Callback callback) { mDisplay = display; + mCallback = callback; mContentWidth = width; mContentHeight = height; @@ -535,6 +538,31 @@ public final class Magnifier { } mRenderer.draw(callback); + if (mCallback != null) { + mCallback.onOperationComplete(); + } + } + } + + // The rest of the file consists of test APIs. + + /** + * See {@link #setOnOperationCompleteCallback(Callback)}. + */ + @TestApi + private Callback mCallback; + + /** + * Sets a callback which will be invoked at the end of the next + * {@link #show(float, float)} or {@link #update()} operation. + * + * @hide + */ + @TestApi + public void setOnOperationCompleteCallback(final Callback callback) { + mCallback = callback; + if (mWindow != null) { + mWindow.mCallback = callback; } } @@ -584,4 +612,15 @@ public final class Magnifier { size.y = resources.getDimension(com.android.internal.R.dimen.magnifier_height) / density; return size; } + + /** + * @hide + */ + @TestApi + public interface Callback { + /** + * Callback called after the drawing for a magnifier update has happened. + */ + void onOperationComplete(); + } }