Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ec9d3051 authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Reland "Optimize magnifier viewport drawing" 

1. Reduce the number of drawIfNeeded calls in fullscreen mode by 99%.
Because updateMagnificationSpec -> setMagnifiedRegionBorderShown will
be called with motion events when changing zoom. Then even if the
border region is not changed, it still always forces redraw. So simply
skip if the border shown state is not changed.

2. Only apply surface transaction if related attributes are changed.

3. drawIfNeeded was always called inside WM lock, so the change [1]
didn't work. With this change, drawIfNeeded is still called on the
same animation thread but it is executed from message directly,
then the change [1] can take effect that avoids holding WM lock
when calling lockCanvas.

AccessibilityMagnificationTest was highly flaky by "mSurface.release()"
in releaseSurface is called on other thread. This reland fixes it
by also posting the cleanup to the same thread.

[1]: Id828744c8c5bcb4bdb3be9a11810338614b84b2e

Bug: 276845499
Bug: 316075123
Bug: 318327737
Test: atest WindowManagerServiceTests#testDrawMagnifiedViewport
Test: atest AccessibilityMagnificationTest --rerun-until-failure 10

Change-Id: I7b6559e10db3dcea0352f17b81e5537b711a18c5
parent 429020dd

core/java/android/window/flags/windowing_frontend.aconfig

+8 −0
Original line number Diff line number Diff line
@@ -37,6 +37,14 @@ flag { 
  bug: "291870756"

}



flag {

  name: "draw_magnifier_border_outside_wmlock"

  namespace: "windowing_frontend"

  description: "Avoid holding WM locks for a long time when executing lockCanvas"

  bug: "316075123"

  is_fixed_read_only: true

}



flag {

  name: "introduce_smoother_dimmer"

  namespace: "windowing_frontend"

services/core/java/com/android/server/wm/AccessibilityController.java

+93 −14
Original line number Diff line number Diff line
@@ -98,6 +98,8 @@ import android.view.animation.DecelerateInterpolator; 
import android.view.animation.Interpolator;



import com.android.internal.R;

import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.os.SomeArgs;

import com.android.internal.util.TraceBuffer;

import com.android.internal.util.function.pooled.PooledLambda;
@@ -297,6 +299,18 @@ final class AccessibilityController { 
        }

    }



    /** It is only used by unit test. */

    @VisibleForTesting

    Surface forceShowMagnifierSurface(int displayId) {

        final DisplayMagnifier displayMagnifier = mDisplayMagnifiers.get(displayId);

        if (displayMagnifier != null) {

            displayMagnifier.mMagnifedViewport.mWindow.setAlpha(DisplayMagnifier.MagnifiedViewport

                    .ViewportWindow.AnimationController.MAX_ALPHA);

            return displayMagnifier.mMagnifedViewport.mWindow.mSurface;

        }

        return null;

    }



    void onWindowLayersChanged(int displayId) {

        if (mAccessibilityTracing.isTracingEnabled(FLAGS_MAGNIFICATION_CALLBACK

                | FLAGS_WINDOWS_FOR_ACCESSIBILITY_CALLBACK)) {
@@ -448,6 +462,7 @@ final class AccessibilityController { 
        }

    }



    // TODO(b/318327737): Remove parameter 't' when removing flag DRAW_IN_WM_LOCK.

    void drawMagnifiedRegionBorderIfNeeded(int displayId, SurfaceControl.Transaction t) {

        if (mAccessibilityTracing.isTracingEnabled(FLAGS_MAGNIFICATION_CALLBACK)) {

            mAccessibilityTracing.logTrace(
@@ -1106,11 +1121,19 @@ final class AccessibilityController { 
            }



            void setMagnifiedRegionBorderShown(boolean shown, boolean animate) {

                if (ViewportWindow.DRAW_IN_WM_LOCK) {

                    if (shown) {

                        mFullRedrawNeeded = true;

                        mOldMagnificationRegion.set(0, 0, 0, 0);

                    }

                    mWindow.setShown(shown, animate);

                    return;

                }

                if (mWindow.setShown(shown, animate)) {

                    mFullRedrawNeeded = true;

                    // Clear the old region, so recomputeBounds will refresh the current region.

                    mOldMagnificationRegion.set(0, 0, 0, 0);

                }

            }



            void getMagnifiedFrameInContentCoords(Rect rect) {
@@ -1130,7 +1153,11 @@ final class AccessibilityController { 


            void drawWindowIfNeeded(SurfaceControl.Transaction t) {

                recomputeBounds();

                mWindow.drawIfNeeded(t);

                if (ViewportWindow.DRAW_IN_WM_LOCK) {

                    mWindow.drawOrRemoveIfNeeded(t);

                    return;

                }

                mWindow.postDrawIfNeeded();

            }



            void destroyWindow() {
@@ -1158,23 +1185,28 @@ final class AccessibilityController { 
                mWindow.dump(pw, prefix);

            }



            private final class ViewportWindow {

            private final class ViewportWindow implements Runnable {

                private static final String SURFACE_TITLE = "Magnification Overlay";

                // TODO(b/318327737): Remove if it is stable.

                static final boolean DRAW_IN_WM_LOCK = !Flags.drawMagnifierBorderOutsideWmlock();



                private final Region mBounds = new Region();

                private final Rect mDirtyRect = new Rect();

                private final Paint mPaint = new Paint();



                private final SurfaceControl mSurfaceControl;

                /** After initialization, it should only be accessed from animation thread. */

                private final SurfaceControl.Transaction mTransaction;

                private final BLASTBufferQueue mBlastBufferQueue;

                private final Surface mSurface;



                private final AnimationController mAnimationController;



                private boolean mShown;

                private boolean mLastSurfaceShown;

                private int mAlpha;



                private boolean mInvalidated;

                private volatile boolean mInvalidated;



                ViewportWindow(Context context) {

                    SurfaceControl surfaceControl = null;
@@ -1202,6 +1234,7 @@ final class AccessibilityController { 
                    InputMonitor.setTrustedOverlayInputInfo(mSurfaceControl, t,

                            mDisplayContent.getDisplayId(), "Magnification Overlay");

                    t.apply();

                    mTransaction = t;

                    mSurface = mBlastBufferQueue.createSurface();



                    mAnimationController = new AnimationController(context,
@@ -1219,10 +1252,11 @@ final class AccessibilityController { 
                    mInvalidated = true;

                }



                void setShown(boolean shown, boolean animate) {

                /** Returns {@code true} if the state is changed to shown. */

                boolean setShown(boolean shown, boolean animate) {

                    synchronized (mService.mGlobalLock) {

                        if (mShown == shown) {

                            return;

                            return false;

                        }

                        mShown = shown;

                        mAnimationController.onFrameShownStateChanged(shown, animate);
@@ -1230,6 +1264,7 @@ final class AccessibilityController { 
                            Slog.i(LOG_TAG, "ViewportWindow shown: " + mShown);

                        }

                    }

                    return shown;

                }



                @SuppressWarnings("unused")
@@ -1285,7 +1320,22 @@ final class AccessibilityController { 
                    mService.scheduleAnimationLocked();

                }



                void drawIfNeeded(SurfaceControl.Transaction t) {

                void postDrawIfNeeded() {

                    if (mInvalidated) {

                        mService.mAnimationHandler.post(this);

                    }

                }



                @Override

                public void run() {

                    drawOrRemoveIfNeeded(mTransaction);

                }



                /**

                 * This method must only be called by animation handler directly to make sure

                 * thread safe and there is no lock held outside.

                 */

                private void drawOrRemoveIfNeeded(SurfaceControl.Transaction t) {

                    // Drawing variables (alpha, dirty rect, and bounds) access is synchronized

                    // using WindowManagerGlobalLock. Grab copies of these values before

                    // drawing on the canvas so that drawing can be performed outside of the lock.
@@ -1293,6 +1343,14 @@ final class AccessibilityController { 
                    Rect drawingRect = null;

                    Region drawingBounds = null;

                    synchronized (mService.mGlobalLock) {

                        if (!DRAW_IN_WM_LOCK && mBlastBufferQueue.mNativeObject == 0) {

                            // Complete removal since releaseSurface has been called.

                            if (mSurface.isValid()) {

                                mTransaction.remove(mSurfaceControl).apply();

                                mSurface.release();

                            }

                            return;

                        }

                        if (!mInvalidated) {

                            return;

                        }
@@ -1314,6 +1372,7 @@ final class AccessibilityController { 
                        }

                    }



                    final boolean showSurface;

                    // Draw without holding WindowManagerGlobalLock.

                    if (alpha > 0) {

                        Canvas canvas = null;
@@ -1329,18 +1388,38 @@ final class AccessibilityController { 
                        mPaint.setAlpha(alpha);

                        canvas.drawPath(drawingBounds.getBoundaryPath(), mPaint);

                        mSurface.unlockCanvasAndPost(canvas);

                        if (DRAW_IN_WM_LOCK) {

                            t.show(mSurfaceControl);

                            return;

                        }

                        showSurface = true;

                    } else {

                        if (DRAW_IN_WM_LOCK) {

                            t.hide(mSurfaceControl);

                            return;

                        }

                        showSurface = false;

                    }



                    if (showSurface && !mLastSurfaceShown) {

                        mTransaction.show(mSurfaceControl).apply();

                        mLastSurfaceShown = true;

                    } else if (!showSurface && mLastSurfaceShown) {

                        mTransaction.hide(mSurfaceControl).apply();

                        mLastSurfaceShown = false;

                    }

                }



                @GuardedBy("mService.mGlobalLock")

                void releaseSurface() {

                    if (mBlastBufferQueue != null) {

                    mBlastBufferQueue.destroy();

                    }

                    if (DRAW_IN_WM_LOCK) {

                        mService.mTransactionFactory.get().remove(mSurfaceControl).apply();

                        mSurface.release();

                        return;

                    }

                    // Post to perform cleanup on the thread which handles mSurface.

                    mService.mAnimationHandler.post(this);

                }



                void dump(PrintWriter pw, String prefix) {

services/tests/wmtests/src/com/android/server/wm/WindowManagerServiceTests.java

+53 −0
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ import static android.view.WindowManager.LayoutParams.TYPE_INPUT_METHOD_DIALOG; 
import static android.view.WindowManager.LayoutParams.TYPE_TOAST;

import static android.window.DisplayAreaOrganizer.FEATURE_VENDOR_FIRST;



import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.never;
@@ -85,6 +86,7 @@ import android.view.IWindow; 
import android.view.InputChannel;

import android.view.InsetsSourceControl;

import android.view.InsetsState;

import android.view.Surface;

import android.view.SurfaceControl;

import android.view.View;

import android.view.WindowInsets;
@@ -951,6 +953,57 @@ public class WindowManagerServiceTests extends WindowTestsBase { 
                argThat(h -> (h.inputConfig & InputConfig.SPY) == InputConfig.SPY));

    }



    @Test

    public void testDrawMagnifiedViewport() {

        final int displayId = mDisplayContent.mDisplayId;

        // Use real surface, so ViewportWindow's BlastBufferQueue can be created.

        final ArrayList<SurfaceControl> surfaceControls = new ArrayList<>();

        mWm.mSurfaceControlFactory = s -> new SurfaceControl.Builder() {

            @Override

            public SurfaceControl build() {

                final SurfaceControl sc = super.build();

                surfaceControls.add(sc);

                return sc;

            }

        };

        mWm.mAccessibilityController.setMagnificationCallbacks(displayId,

                mock(WindowManagerInternal.MagnificationCallbacks.class));

        final boolean[] lockCanvasInWmLock = { false };

        final Surface surface = mWm.mAccessibilityController.forceShowMagnifierSurface(displayId);

        spyOn(surface);

        doAnswer(invocationOnMock -> {

            lockCanvasInWmLock[0] |= Thread.holdsLock(mWm.mGlobalLock);

            invocationOnMock.callRealMethod();

            return null;

        }).when(surface).lockCanvas(any());

        mWm.mAccessibilityController.drawMagnifiedRegionBorderIfNeeded(displayId, mTransaction);

        waitUntilHandlersIdle();

        try {

            verify(surface).lockCanvas(any());



            clearInvocations(surface);

            // Invalidate and redraw.

            mWm.mAccessibilityController.onDisplaySizeChanged(mDisplayContent);

            mWm.mAccessibilityController.drawMagnifiedRegionBorderIfNeeded(displayId, mTransaction);

            // Turn off magnification to release surface.

            mWm.mAccessibilityController.setMagnificationCallbacks(displayId, null);

            if (!com.android.window.flags.Flags.drawMagnifierBorderOutsideWmlock()) {

                verify(surface).release();

                assertTrue(lockCanvasInWmLock[0]);

                return;

            }

            waitUntilHandlersIdle();

            // lockCanvas must not be called after releasing.

            verify(surface, never()).lockCanvas(any());

            verify(surface).release();

            assertFalse(lockCanvasInWmLock[0]);

        } finally {

            for (int i = surfaceControls.size() - 1; i >= 0; --i) {

                surfaceControls.get(i).release();

            }

        }

    }



    @Test

    public void testRequestKeyboardShortcuts_noWindow() {

        doNothing().when(mWm.mContext).enforceCallingOrSelfPermission(anyString(), anyString());