From 7a5addd2b735844902ac5f89003b3afad299b3ad Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Thu, 26 Apr 2018 23:23:29 +0200 Subject: [PATCH] Apply surface parameters in sync with RenderThread Otherwise it could lead to parameters applied in the wrong frame, leading to jank. Test: Open notification Bug: 78611607 Change-Id: Ia7900e753b29187a7a7b81f393666687e8b8e04b --- core/java/android/view/ThreadedRenderer.java | 6 +- core/java/android/view/ViewRootImpl.java | 17 ++- libs/hwui/renderthread/DrawFrameTask.cpp | 1 + .../system/SyncRtSurfaceTransactionApplier.java | 119 +++++++++++++++++++++ .../notification/ActivityLaunchAnimator.java | 31 ++---- 5 files changed, 151 insertions(+), 23 deletions(-) create mode 100644 packages/SystemUI/shared/src/com/android/systemui/shared/system/SyncRtSurfaceTransactionApplier.java diff --git a/core/java/android/view/ThreadedRenderer.java b/core/java/android/view/ThreadedRenderer.java index ca181883a2d2..0252807a76aa 100644 --- a/core/java/android/view/ThreadedRenderer.java +++ b/core/java/android/view/ThreadedRenderer.java @@ -790,7 +790,8 @@ public final class ThreadedRenderer { * @param attachInfo AttachInfo tied to the specified view. * @param callbacks Callbacks invoked when drawing happens. */ - void draw(View view, AttachInfo attachInfo, DrawCallbacks callbacks) { + void draw(View view, AttachInfo attachInfo, DrawCallbacks callbacks, + FrameDrawingCallback frameDrawingCallback) { final Choreographer choreographer = attachInfo.mViewRootImpl.mChoreographer; choreographer.mFrameInfo.markDrawStart(); @@ -811,6 +812,9 @@ public final class ThreadedRenderer { } final long[] frameInfo = choreographer.mFrameInfo.mFrameInfo; + if (frameDrawingCallback != null) { + nSetFrameCallback(mNativeProxy, frameDrawingCallback); + } int syncResult = nSyncAndDrawFrame(mNativeProxy, frameInfo, frameInfo.length); if ((syncResult & SYNC_LOST_SURFACE_REWARD_IF_FOUND) != 0) { setEnabled(false); diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index 7ab005b1afe3..364a4a572a8b 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -80,6 +80,7 @@ import android.util.SparseBooleanArray; import android.util.TimeUtils; import android.util.TypedValue; import android.view.Surface.OutOfResourcesException; +import android.view.ThreadedRenderer.FrameDrawingCallback; import android.view.View.AttachInfo; import android.view.View.FocusDirection; import android.view.View.MeasureSpec; @@ -175,6 +176,8 @@ public final class ViewRootImpl implements ViewParent, static final ArrayList sFirstDrawHandlers = new ArrayList(); static boolean sFirstDrawComplete = false; + private FrameDrawingCallback mNextRtFrameCallback; + /** * Callback for notifying about global configuration changes. */ @@ -967,6 +970,17 @@ public final class ViewRootImpl implements ViewParent, } } + /** + * Registers a callback to be executed when the next frame is being drawn on RenderThread. This + * callback will be executed on a RenderThread worker thread, and only used for the next frame + * and thus it will only fire once. + * + * @param callback The callback to register. + */ + public void registerRtFrameCallback(FrameDrawingCallback callback) { + mNextRtFrameCallback = callback; + } + private void enableHardwareAcceleration(WindowManager.LayoutParams attrs) { mAttachInfo.mHardwareAccelerated = false; mAttachInfo.mHardwareAccelerationRequested = false; @@ -3252,7 +3266,8 @@ public final class ViewRootImpl implements ViewParent, requestDrawWindow(); } - mAttachInfo.mThreadedRenderer.draw(mView, mAttachInfo, this); + mAttachInfo.mThreadedRenderer.draw(mView, mAttachInfo, this, mNextRtFrameCallback); + mNextRtFrameCallback = null; } else { // If we get here with a disabled & requested hardware renderer, something went // wrong (an invalidate posted right before we destroyed the hardware surface diff --git a/libs/hwui/renderthread/DrawFrameTask.cpp b/libs/hwui/renderthread/DrawFrameTask.cpp index 778e7689d0f9..60df514ecc2b 100644 --- a/libs/hwui/renderthread/DrawFrameTask.cpp +++ b/libs/hwui/renderthread/DrawFrameTask.cpp @@ -95,6 +95,7 @@ void DrawFrameTask::run() { // Grab a copy of everything we need CanvasContext* context = mContext; std::function callback = std::move(mFrameCallback); + mFrameCallback = nullptr; // From this point on anything in "this" is *UNSAFE TO ACCESS* if (canUnblockUiThread) { diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/system/SyncRtSurfaceTransactionApplier.java b/packages/SystemUI/shared/src/com/android/systemui/shared/system/SyncRtSurfaceTransactionApplier.java new file mode 100644 index 000000000000..b08d4d417ff7 --- /dev/null +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/system/SyncRtSurfaceTransactionApplier.java @@ -0,0 +1,119 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +package com.android.systemui.shared.system; + +import android.graphics.Matrix; +import android.graphics.Rect; +import android.view.Surface; +import android.view.SurfaceControl; +import android.view.View; +import android.view.ViewRootImpl; + +import java.util.ArrayList; + +/** + * Helper class to apply surface transactions in sync with RenderThread. + */ +public class SyncRtSurfaceTransactionApplier { + + private final Object mLock = new Object(); + private final Surface mTargetSurface; + private final ViewRootImpl mTargetViewRootImpl; + private final float[] mTmpFloat9 = new float[9]; + + /** + * @param targetView The view in the surface that acts as synchronization anchor. + */ + public SyncRtSurfaceTransactionApplier(View targetView) { + mTargetViewRootImpl = targetView != null ? targetView.getViewRootImpl() : null; + mTargetSurface = mTargetViewRootImpl != null ? mTargetViewRootImpl.mSurface : null; + } + + /** + * Schedules applying surface parameters on the next frame. + * + * @param params The parameters for the surface to apply. + */ + public void scheduleApply(SurfaceParams params) { + ArrayList list = new ArrayList<>(1); + list.add(params); + scheduleApply(list); + } + + /** + * Schedules applying surface parameters on the next frame. + * + * @param params The surface parameters to apply. DO NOT MODIFY the list after passing into + * this method to avoid synchronization issues. + */ + public void scheduleApply(ArrayList params) { + if (mTargetViewRootImpl != null) { + + // Acquire mLock to establish a happens-before relationship to ensure the other thread + // sees the surface parameters. + synchronized (mLock) { + mTargetViewRootImpl.registerRtFrameCallback(frame -> { + synchronized (mLock) { + if (mTargetSurface == null || !mTargetSurface.isValid()) { + return; + } + SurfaceControl.Transaction t = new SurfaceControl.Transaction(); + for (int i = params.size() - 1; i >= 0; i--) { + SurfaceParams surfaceParams = params.get(i); + SurfaceControl surface = surfaceParams.surface; + t.deferTransactionUntilSurface(surface, mTargetSurface, frame); + t.setMatrix(surface, surfaceParams.matrix, mTmpFloat9); + t.setWindowCrop(surface, surfaceParams.windowCrop); + t.setAlpha(surface, surfaceParams.alpha); + t.setLayer(surface, surfaceParams.layer); + t.show(surface); + } + t.setEarlyWakeup(); + t.apply(); + } + }); + } + } + } + + public static class SurfaceParams { + + /** + * Constructs surface parameters to be applied when the current view state gets pushed to + * RenderThread. + * + * @param surface The surface to modify. + * @param alpha Alpha to apply. + * @param matrix Matrix to apply. + * @param windowCrop Crop to apply. + */ + public SurfaceParams(SurfaceControl surface, float alpha, Matrix matrix, Rect windowCrop, + int layer) { + this.surface = surface; + this.alpha = alpha; + this.matrix = new Matrix(matrix); + this.windowCrop = new Rect(windowCrop); + this.layer = layer; + } + + final SurfaceControl surface; + final float alpha; + final Matrix matrix; + final Rect windowCrop; + final int layer; + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/ActivityLaunchAnimator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/ActivityLaunchAnimator.java index 20ab64cfdb17..44b3a4dc08f0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/ActivityLaunchAnimator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/ActivityLaunchAnimator.java @@ -20,7 +20,6 @@ import android.animation.Animator; import android.animation.AnimatorListenerAdapter; import android.animation.ValueAnimator; import android.app.ActivityManager; -import android.app.ActivityOptions; import android.graphics.Matrix; import android.graphics.Rect; import android.os.RemoteException; @@ -29,11 +28,10 @@ import android.view.IRemoteAnimationFinishedCallback; import android.view.IRemoteAnimationRunner; import android.view.RemoteAnimationAdapter; import android.view.RemoteAnimationTarget; -import android.view.Surface; -import android.view.SurfaceControl; -import android.view.ViewRootImpl; import com.android.systemui.Interpolators; +import com.android.systemui.shared.system.SyncRtSurfaceTransactionApplier; +import com.android.systemui.shared.system.SyncRtSurfaceTransactionApplier.SurfaceParams; import com.android.systemui.statusbar.ExpandableNotificationRow; import com.android.systemui.statusbar.NotificationListContainer; import com.android.systemui.statusbar.StatusBarState; @@ -42,6 +40,8 @@ import com.android.systemui.statusbar.phone.NotificationPanelView; import com.android.systemui.statusbar.phone.StatusBar; import com.android.systemui.statusbar.phone.StatusBarWindowView; +import java.util.ArrayList; + /** * A class that allows activities to be launched in a seamless way where the notification * transforms nicely into the starting window. @@ -82,7 +82,7 @@ public class ActivityLaunchAnimator { } AnimationRunner animationRunner = new AnimationRunner(sourceNotification); return new RemoteAnimationAdapter(animationRunner, ANIMATION_DURATION, - 0 /* statusBarTransitionDelay */); + ANIMATION_DURATION - 150 /* statusBarTransitionDelay */); } public boolean isAnimationPending() { @@ -110,12 +110,13 @@ public class ActivityLaunchAnimator { private final ExpandableNotificationRow mSourceNotification; private final ExpandAnimationParameters mParams; private final Rect mWindowCrop = new Rect(); - private boolean mLeashShown; private boolean mInstantCollapsePanel = true; + private final SyncRtSurfaceTransactionApplier mSyncRtTransactionApplier; public AnimationRunner(ExpandableNotificationRow sourceNofitication) { mSourceNotification = sourceNofitication; mParams = new ExpandAnimationParameters(); + mSyncRtTransactionApplier = new SyncRtSurfaceTransactionApplier(mSourceNotification); } @Override @@ -241,24 +242,12 @@ public class ActivityLaunchAnimator { } private void applyParamsToWindow(RemoteAnimationTarget app) { - SurfaceControl.Transaction t = new SurfaceControl.Transaction(); - if (!mLeashShown) { - t.show(app.leash); - mLeashShown = true; - } Matrix m = new Matrix(); m.postTranslate(0, (float) (mParams.top - app.position.y)); - t.setMatrix(app.leash, m, new float[9]); mWindowCrop.set(mParams.left, 0, mParams.right, mParams.getHeight()); - t.setWindowCrop(app.leash, mWindowCrop); - ViewRootImpl viewRootImpl = mSourceNotification.getViewRootImpl(); - if (viewRootImpl != null) { - Surface systemUiSurface = viewRootImpl.mSurface; - t.deferTransactionUntilSurface(app.leash, systemUiSurface, - systemUiSurface.getNextFrameNumber()); - } - t.setEarlyWakeup(); - t.apply(); + SurfaceParams params = new SurfaceParams(app.leash, 1f /* alpha */, m, mWindowCrop, + app.prefixOrderIndex); + mSyncRtTransactionApplier.scheduleApply(params); } @Override -- 2.11.0