ScreenDecorations: Fix stuck / delayed rotations
Fixes the issue of stuck or delayed decor overlay rotations. The current logic
cannot handle the case where the overlay has not finished rotating when we start
another rotation.
To work around that, we make sure that the rotation finishes fast enough for that
not to happen, by decoupling it from the rest of SystemUI's traversals on a dedicated
thread.
Also fixes an issue in ScreenDecorations where we restarted drawing even though that
was not needed, which further delayed finishing the rotation of the overlay.
Also fixes an issue in ScreenDecorations where a spurious traversal after rotation
started but before SysUI was notified could draw in the wrong orientation. To prevent
that, we validate that our rotation matches the display rotation and restart the draw
otherwise.
In the future, we should investigate whether we can make this more robust by allowing
stacked rotations, or forbidding rotations while the overlay is not done yet. We should
also make the synchronization between WM and SysUI on what rotation is being drawn more
explict and thus more robust.
Change-Id: I5303a6a8e6392d309c0fe672ff6d1386ae18f235
Merged-In: Id8fe1c7f6b38de8cd3ce5f4170ea37adf8cb9f3d
Bug: 111761727
Test: Ensure you have a lot of notifications, and enable cutout emulation, open camera, quickly rotate phone between landscape and portrait repeatedly, ensure there are no artifacts on screen.
Test: atest ScreenDecorationsTest
diff --git a/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java b/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java
index 194679c..71cf9b5 100644
--- a/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java
+++ b/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java
@@ -39,10 +39,13 @@
import android.graphics.Rect;
import android.graphics.Region;
import android.hardware.display.DisplayManager;
+import android.os.Handler;
+import android.os.HandlerThread;
import android.os.SystemProperties;
import android.provider.Settings.Secure;
import android.support.annotation.VisibleForTesting;
import android.util.DisplayMetrics;
+import android.util.Log;
import android.view.DisplayCutout;
import android.view.DisplayInfo;
import android.view.Gravity;
@@ -57,6 +60,7 @@
import android.widget.FrameLayout;
import android.widget.ImageView;
+import com.android.internal.util.Preconditions;
import com.android.systemui.RegionInterceptingFrameLayout.RegionInterceptableView;
import com.android.systemui.fragments.FragmentHostManager;
import com.android.systemui.fragments.FragmentHostManager.FragmentListener;
@@ -74,6 +78,9 @@
* for antialiasing and emulation purposes.
*/
public class ScreenDecorations extends SystemUI implements Tunable {
+ private static final boolean DEBUG = false;
+ private static final String TAG = "ScreenDecorations";
+
public static final String SIZE = "sysui_rounded_size";
public static final String PADDING = "sysui_rounded_content_padding";
private static final boolean DEBUG_SCREENSHOT_ROUNDED_CORNERS =
@@ -93,9 +100,24 @@
private DisplayCutoutView mCutoutTop;
private DisplayCutoutView mCutoutBottom;
private boolean mPendingRotationChange;
+ private Handler mHandler;
@Override
public void start() {
+ mHandler = startHandlerThread();
+ mHandler.post(this::startOnScreenDecorationsThread);
+ setupStatusBarPaddingIfNeeded();
+ }
+
+ @VisibleForTesting
+ Handler startHandlerThread() {
+ HandlerThread thread = new HandlerThread("ScreenDecorations");
+ thread.start();
+ return thread.getThreadHandler();
+ }
+
+ private void startOnScreenDecorationsThread() {
+ mRotation = RotationUtils.getExactRotation(mContext);
mWindowManager = mContext.getSystemService(WindowManager.class);
mRoundedDefault = mContext.getResources().getDimensionPixelSize(
R.dimen.rounded_corner_radius);
@@ -107,12 +129,6 @@
setupDecorations();
}
- int padding = mContext.getResources().getDimensionPixelSize(
- R.dimen.rounded_corner_content_padding);
- if (padding != 0) {
- setupPadding(padding);
- }
-
mDisplayListener = new DisplayManager.DisplayListener() {
@Override
public void onDisplayAdded(int displayId) {
@@ -126,8 +142,8 @@
@Override
public void onDisplayChanged(int displayId) {
- if (mOverlay != null && mBottomOverlay != null
- && mRotation != RotationUtils.getExactRotation(mContext)) {
+ final int newRotation = RotationUtils.getExactRotation(mContext);
+ if (mOverlay != null && mBottomOverlay != null && mRotation != newRotation) {
// We cannot immediately update the orientation. Otherwise
// WindowManager is still deferring layout until it has finished dispatching
// the config changes, which may cause divergence between what we draw
@@ -136,20 +152,24 @@
// - we are trying to redraw. This because WM resized our window and told us to.
// - the config change has been dispatched, so WM is no longer deferring layout.
mPendingRotationChange = true;
- mOverlay.getViewTreeObserver().addOnPreDrawListener(
- new RestartingPreDrawListener(mOverlay));
- mBottomOverlay.getViewTreeObserver().addOnPreDrawListener(
- new RestartingPreDrawListener(mBottomOverlay));
+ if (DEBUG) {
+ Log.i(TAG, "Rotation changed, deferring " + newRotation + ", staying at "
+ + mRotation);
+ }
+ mOverlay.getViewTreeObserver().addOnPreDrawListener(
+ new RestartingPreDrawListener(mOverlay, newRotation));
+ mBottomOverlay.getViewTreeObserver().addOnPreDrawListener(
+ new RestartingPreDrawListener(mBottomOverlay, newRotation));
}
updateOrientation();
}
};
- mRotation = -1;
mDisplayManager = (DisplayManager) mContext.getSystemService(
Context.DISPLAY_SERVICE);
- mDisplayManager.registerDisplayListener(mDisplayListener, null);
+ mDisplayManager.registerDisplayListener(mDisplayListener, mHandler);
+ updateOrientation();
}
private void setupDecorations() {
@@ -179,10 +199,11 @@
mWindowManager.getDefaultDisplay().getMetrics(metrics);
mDensity = metrics.density;
- Dependency.get(TunerService.class).addTunable(this, SIZE);
+ Dependency.get(Dependency.MAIN_HANDLER).post(
+ () -> Dependency.get(TunerService.class).addTunable(this, SIZE));
// Watch color inversion and invert the overlay as needed.
- SecureSetting setting = new SecureSetting(mContext, Dependency.get(Dependency.MAIN_HANDLER),
+ SecureSetting setting = new SecureSetting(mContext, mHandler,
Secure.ACCESSIBILITY_DISPLAY_INVERSION_ENABLED) {
@Override
protected void handleValueChanged(int value, boolean observedChange) {
@@ -215,18 +236,37 @@
.start();
}
});
+
+ mOverlay.getViewTreeObserver().addOnPreDrawListener(
+ new ValidatingPreDrawListener(mOverlay));
+ mBottomOverlay.getViewTreeObserver().addOnPreDrawListener(
+ new ValidatingPreDrawListener(mBottomOverlay));
}
@Override
protected void onConfigurationChanged(Configuration newConfig) {
- mPendingRotationChange = false;
- updateOrientation();
- if (shouldDrawCutout() && mOverlay == null) {
- setupDecorations();
- }
+ mHandler.post(() -> {
+ int oldRotation = mRotation;
+ mPendingRotationChange = false;
+ updateOrientation();
+ if (DEBUG) Log.i(TAG, "onConfigChanged from rot " + oldRotation + " to " + mRotation);
+ if (shouldDrawCutout() && mOverlay == null) {
+ setupDecorations();
+ }
+ if (mOverlay != null) {
+ // Updating the layout params ensures that ViewRootImpl will call relayoutWindow(),
+ // which ensures that the forced seamless rotation will end, even if we updated
+ // the rotation before window manager was ready (and was still waiting for sending
+ // the updated rotation).
+ updateLayoutParams();
+ }
+ });
}
- protected void updateOrientation() {
+ private void updateOrientation() {
+ Preconditions.checkState(mHandler.getLooper().getThread() == Thread.currentThread(),
+ "must call on " + mHandler.getLooper().getThread()
+ + ", but was " + Thread.currentThread());
if (mPendingRotationChange) {
return;
}
@@ -306,7 +346,19 @@
com.android.internal.R.bool.config_fillMainBuiltInDisplayCutout);
}
- private void setupPadding(int padding) {
+
+ private void setupStatusBarPaddingIfNeeded() {
+ // TODO: This should be moved to a more appropriate place, as it is not related to the
+ // screen decorations overlay.
+ int padding = mContext.getResources().getDimensionPixelSize(
+ R.dimen.rounded_corner_content_padding);
+ if (padding != 0) {
+ setupStatusBarPadding(padding);
+ }
+
+ }
+
+ private void setupStatusBarPadding(int padding) {
// Add some padding to all the content near the edge of the screen.
StatusBar sb = getComponent(StatusBar.class);
View statusBar = (sb != null ? sb.getStatusBarWindow() : null);
@@ -375,30 +427,32 @@
@Override
public void onTuningChanged(String key, String newValue) {
- if (mOverlay == null) return;
- if (SIZE.equals(key)) {
- int size = mRoundedDefault;
- int sizeTop = mRoundedDefaultTop;
- int sizeBottom = mRoundedDefaultBottom;
- if (newValue != null) {
- try {
- size = (int) (Integer.parseInt(newValue) * mDensity);
- } catch (Exception e) {
+ mHandler.post(() -> {
+ if (mOverlay == null) return;
+ if (SIZE.equals(key)) {
+ int size = mRoundedDefault;
+ int sizeTop = mRoundedDefaultTop;
+ int sizeBottom = mRoundedDefaultBottom;
+ if (newValue != null) {
+ try {
+ size = (int) (Integer.parseInt(newValue) * mDensity);
+ } catch (Exception e) {
+ }
}
- }
- if (sizeTop == 0) {
- sizeTop = size;
- }
- if (sizeBottom == 0) {
- sizeBottom = size;
- }
+ if (sizeTop == 0) {
+ sizeTop = size;
+ }
+ if (sizeBottom == 0) {
+ sizeBottom = size;
+ }
- setSize(mOverlay.findViewById(R.id.left), sizeTop);
- setSize(mOverlay.findViewById(R.id.right), sizeTop);
- setSize(mBottomOverlay.findViewById(R.id.left), sizeBottom);
- setSize(mBottomOverlay.findViewById(R.id.right), sizeBottom);
- }
+ setSize(mOverlay.findViewById(R.id.left), sizeTop);
+ setSize(mOverlay.findViewById(R.id.right), sizeTop);
+ setSize(mBottomOverlay.findViewById(R.id.left), sizeBottom);
+ setSize(mBottomOverlay.findViewById(R.id.right), sizeBottom);
+ }
+ });
}
private void setSize(View view, int pixelSize) {
@@ -457,6 +511,11 @@
mVisibilityChangedListener = visibilityChangedListener;
mDecorations = decorations;
setId(R.id.display_cutout);
+ if (DEBUG) {
+ getViewTreeObserver().addOnDrawListener(() -> Log.i(TAG,
+ (mInitialStart ? "OverlayTop" : "OverlayBottom")
+ + " drawn in rot " + mRotation));
+ }
}
public void setColor(int color) {
@@ -692,20 +751,66 @@
private class RestartingPreDrawListener implements ViewTreeObserver.OnPreDrawListener {
private final View mView;
+ private final int mTargetRotation;
- private RestartingPreDrawListener(View view) {
+ private RestartingPreDrawListener(View view, int targetRotation) {
+ mView = view;
+ mTargetRotation = targetRotation;
+ }
+
+ @Override
+ public boolean onPreDraw() {
+ mView.getViewTreeObserver().removeOnPreDrawListener(this);
+
+ if (mTargetRotation == mRotation) {
+ if (DEBUG) {
+ Log.i(TAG, (mView == mOverlay ? "OverlayTop" : "OverlayBottom")
+ + " already in target rot "
+ + mTargetRotation + ", allow draw without restarting it");
+ }
+ return true;
+ }
+
+ mPendingRotationChange = false;
+ // This changes the window attributes - we need to restart the traversal for them to
+ // take effect.
+ updateOrientation();
+ if (DEBUG) {
+ Log.i(TAG, (mView == mOverlay ? "OverlayTop" : "OverlayBottom")
+ + " restarting listener fired, restarting draw for rot " + mRotation);
+ }
+ mView.invalidate();
+ return false;
+ }
+ }
+
+ /**
+ * A pre-draw listener, that validates that the rotation we draw in matches the displays
+ * rotation before continuing the draw.
+ *
+ * This is to prevent a race condition, where we have not received the display changed event
+ * yet, and would thus draw in an old orientation.
+ */
+ private class ValidatingPreDrawListener implements ViewTreeObserver.OnPreDrawListener {
+
+ private final View mView;
+
+ public ValidatingPreDrawListener(View view) {
mView = view;
}
@Override
public boolean onPreDraw() {
- mPendingRotationChange = false;
- mView.getViewTreeObserver().removeOnPreDrawListener(this);
- // This changes the window attributes - we need to restart the traversal for them to
- // take effect.
- updateOrientation();
- mView.invalidate();
- return false;
+ final int displayRotation = RotationUtils.getExactRotation(mContext);
+ if (displayRotation != mRotation && !mPendingRotationChange) {
+ if (DEBUG) {
+ Log.i(TAG, "Drawing rot " + mRotation + ", but display is at rot "
+ + displayRotation + ". Restarting draw");
+ }
+ mView.invalidate();
+ return false;
+ }
+ return true;
}
}
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/ScreenDecorationsTest.java b/packages/SystemUI/tests/src/com/android/systemui/ScreenDecorationsTest.java
index f1bf31d..644c0b3 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/ScreenDecorationsTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/ScreenDecorationsTest.java
@@ -34,8 +34,10 @@
import android.app.Fragment;
import android.content.res.Configuration;
+import android.os.Handler;
import android.support.test.filters.SmallTest;
import android.testing.AndroidTestingRunner;
+import android.testing.TestableLooper;
import android.testing.TestableLooper.RunWithLooper;
import android.view.Display;
import android.view.View;
@@ -60,6 +62,7 @@
@SmallTest
public class ScreenDecorationsTest extends SysuiTestCase {
+ private TestableLooper mTestableLooper;
private ScreenDecorations mScreenDecorations;
private StatusBar mStatusBar;
private WindowManager mWindowManager;
@@ -71,6 +74,10 @@
@Before
public void setup() {
+ mTestableLooper = TestableLooper.get(this);
+ mDependency.injectTestDependency(Dependency.MAIN_HANDLER,
+ new Handler(mTestableLooper.getLooper()));
+
mStatusBar = mock(StatusBar.class);
mWindowManager = mock(WindowManager.class);
mView = spy(new StatusBarWindowView(mContext, null));
@@ -88,7 +95,31 @@
mTunerService = mDependency.injectMockDependency(TunerService.class);
- mScreenDecorations = new ScreenDecorations();
+
+ mScreenDecorations = new ScreenDecorations() {
+ @Override
+ public void start() {
+ super.start();
+ mTestableLooper.processAllMessages();
+ }
+
+ @Override
+ Handler startHandlerThread() {
+ return new Handler(mTestableLooper.getLooper());
+ }
+
+ @Override
+ protected void onConfigurationChanged(Configuration newConfig) {
+ super.onConfigurationChanged(newConfig);
+ mTestableLooper.processAllMessages();
+ }
+
+ @Override
+ public void onTuningChanged(String key, String newValue) {
+ super.onTuningChanged(key, newValue);
+ mTestableLooper.processAllMessages();
+ }
+ };
mScreenDecorations.mContext = mContext;
mScreenDecorations.mComponents = mContext.getComponents();
diff --git a/services/core/java/com/android/server/wm/ForcedSeamlessRotator.java b/services/core/java/com/android/server/wm/ForcedSeamlessRotator.java
index f25ec5c..58c9d958 100644
--- a/services/core/java/com/android/server/wm/ForcedSeamlessRotator.java
+++ b/services/core/java/com/android/server/wm/ForcedSeamlessRotator.java
@@ -21,6 +21,7 @@
import android.graphics.Matrix;
import android.view.DisplayInfo;
+import android.view.Surface.Rotation;
import com.android.server.wm.utils.CoordinateTransforms;
@@ -65,6 +66,16 @@
}
/**
+ * Returns the rotation of the display before it started rotating.
+ *
+ * @return the old rotation of the display
+ */
+ @Rotation
+ public int getOldRotation() {
+ return mOldRotation;
+ }
+
+ /**
* Removes the transform to the window token's surface that undoes the effect of the global
* display rotation.
*
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index 009f393..1ae680f 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -681,6 +681,10 @@
void forceSeamlesslyRotateIfAllowed(int oldRotation, int rotation) {
if (mForceSeamlesslyRotate) {
+ if (mPendingForcedSeamlessRotate != null) {
+ oldRotation = mPendingForcedSeamlessRotate.getOldRotation();
+ }
+
mPendingForcedSeamlessRotate = new ForcedSeamlessRotator(
oldRotation, rotation, getDisplayInfo());
mPendingForcedSeamlessRotate.unrotate(this.mToken);
