From fbd8ea649f960ca998ae0444dbabddd4a940f9fd Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Tue, 16 Oct 2018 17:09:49 -0700 Subject: [PATCH] Fix 2 graphical issues for drag resizing. 1. There used to be double offset from the origin. This is because we used to (in NYC) make the surface position to display origin and draw content with a offset in the surface. However we lately let the surface position be inherited from task surface position, so there will be an offset from surface position and an offset from drawing content in that surface. To fix the bug I removed the offset in drawing content. That offset is provided by WindowState#getBackdropFrame() so it just solved the issue by moving frame to the origin. 2. Window quickly jumps when user starts drag resizing a window. The reason is out of synchronization between surface insets change and graphical buffer update. When user is drag resizing, we suppress window shadow to save some graphical resources, which will consequently change surface insets. Change in surface insets will cause the surface being repositioned to reflect the new surface insets because window frame doesn't change. However the content is still drawn at old location with old surface insets for the first a few frames, so the content jumps to a wrong location for a split second. This also happens when users stop drag resizing. I kept the old surface insets when user is resizing so there won't be surface reposition at the beginning and end of drag resizing, but still suppress the shadow by adjusting the elevation of DecorView. Also fixed a synchronization issue we found in BackdropFrameRenderer, and cleaned up code in it. Bug: 113254346 Test: Manual tests show drag resizing for both freeform and split screen works. Change-Id: I42349f88f14af35fac7c65e784462b5f2e1a71c7 --- core/java/android/view/ViewRootImpl.java | 8 +- .../policy/BackdropFrameRenderer.java | 74 ++++++++++--------- .../android/internal/policy/DecorView.java | 11 ++- .../com/android/server/wm/WindowState.java | 7 +- 4 files changed, 58 insertions(+), 42 deletions(-) diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index 7da31ebe4a176..fdbead6022f34 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -2316,10 +2316,12 @@ public final class ViewRootImpl implements ViewParent, mResizeMode = freeformResizing ? RESIZE_MODE_FREEFORM : RESIZE_MODE_DOCKED_DIVIDER; + final boolean backdropSizeMatchesFrame = + mWinFrame.width() == mPendingBackDropFrame.width() + && mWinFrame.height() == mPendingBackDropFrame.height(); // TODO: Need cutout? - startDragResizing(mPendingBackDropFrame, - mWinFrame.equals(mPendingBackDropFrame), mPendingVisibleInsets, - mPendingStableInsets, mResizeMode); + startDragResizing(mPendingBackDropFrame, !backdropSizeMatchesFrame, + mPendingVisibleInsets, mPendingStableInsets, mResizeMode); } else { // We shouldn't come here, but if we come we should end the resize. endDragResizing(); diff --git a/core/java/com/android/internal/policy/BackdropFrameRenderer.java b/core/java/com/android/internal/policy/BackdropFrameRenderer.java index f14007bd7ac68..cc958f4ed7358 100644 --- a/core/java/com/android/internal/policy/BackdropFrameRenderer.java +++ b/core/java/com/android/internal/policy/BackdropFrameRenderer.java @@ -69,7 +69,6 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame private ColorDrawable mNavigationBarColor; private boolean mOldFullscreen; private boolean mFullscreen; - private final int mResizeMode; private final Rect mOldSystemInsets = new Rect(); private final Rect mOldStableInsets = new Rect(); private final Rect mSystemInsets = new Rect(); @@ -79,7 +78,7 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame public BackdropFrameRenderer(DecorView decorView, ThreadedRenderer renderer, Rect initialBounds, Drawable resizingBackgroundDrawable, Drawable captionBackgroundDrawable, Drawable userCaptionBackgroundDrawable, int statusBarColor, int navigationBarColor, - boolean fullscreen, Rect systemInsets, Rect stableInsets, int resizeMode) { + boolean fullscreen, Rect systemInsets, Rect stableInsets) { setName("ResizeFrame"); mRenderer = renderer; @@ -100,7 +99,6 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame mStableInsets.set(stableInsets); mOldSystemInsets.set(systemInsets); mOldStableInsets.set(stableInsets); - mResizeMode = resizeMode; // Kick off our draw thread. start(); @@ -109,33 +107,35 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame void onResourcesLoaded(DecorView decorView, Drawable resizingBackgroundDrawable, Drawable captionBackgroundDrawableDrawable, Drawable userCaptionBackgroundDrawable, int statusBarColor, int navigationBarColor) { - mDecorView = decorView; - mResizingBackgroundDrawable = resizingBackgroundDrawable != null - && resizingBackgroundDrawable.getConstantState() != null - ? resizingBackgroundDrawable.getConstantState().newDrawable() - : null; - mCaptionBackgroundDrawable = captionBackgroundDrawableDrawable != null - && captionBackgroundDrawableDrawable.getConstantState() != null - ? captionBackgroundDrawableDrawable.getConstantState().newDrawable() - : null; - mUserCaptionBackgroundDrawable = userCaptionBackgroundDrawable != null - && userCaptionBackgroundDrawable.getConstantState() != null - ? userCaptionBackgroundDrawable.getConstantState().newDrawable() - : null; - if (mCaptionBackgroundDrawable == null) { - mCaptionBackgroundDrawable = mResizingBackgroundDrawable; - } - if (statusBarColor != 0) { - mStatusBarColor = new ColorDrawable(statusBarColor); - addSystemBarNodeIfNeeded(); - } else { - mStatusBarColor = null; - } - if (navigationBarColor != 0) { - mNavigationBarColor = new ColorDrawable(navigationBarColor); - addSystemBarNodeIfNeeded(); - } else { - mNavigationBarColor = null; + synchronized (this) { + mDecorView = decorView; + mResizingBackgroundDrawable = resizingBackgroundDrawable != null + && resizingBackgroundDrawable.getConstantState() != null + ? resizingBackgroundDrawable.getConstantState().newDrawable() + : null; + mCaptionBackgroundDrawable = captionBackgroundDrawableDrawable != null + && captionBackgroundDrawableDrawable.getConstantState() != null + ? captionBackgroundDrawableDrawable.getConstantState().newDrawable() + : null; + mUserCaptionBackgroundDrawable = userCaptionBackgroundDrawable != null + && userCaptionBackgroundDrawable.getConstantState() != null + ? userCaptionBackgroundDrawable.getConstantState().newDrawable() + : null; + if (mCaptionBackgroundDrawable == null) { + mCaptionBackgroundDrawable = mResizingBackgroundDrawable; + } + if (statusBarColor != 0) { + mStatusBarColor = new ColorDrawable(statusBarColor); + addSystemBarNodeIfNeeded(); + } else { + mStatusBarColor = null; + } + if (navigationBarColor != 0) { + mNavigationBarColor = new ColorDrawable(navigationBarColor); + addSystemBarNodeIfNeeded(); + } else { + mNavigationBarColor = null; + } } } @@ -186,7 +186,7 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame * All resources of the renderer will be released. This function can be called from the * the UI thread as well as the renderer thread. */ - public void releaseRenderer() { + void releaseRenderer() { synchronized (this) { if (mRenderer != null) { // Invalidate the current content bounds. @@ -268,7 +268,7 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame * @param ySize The height of the content. * @return true if a frame should be requested after the content is drawn; false otherwise. */ - public boolean onContentDrawn(int xOffset, int yOffset, int xSize, int ySize) { + boolean onContentDrawn(int xOffset, int yOffset, int xSize, int ySize) { synchronized (this) { final boolean firstCall = mLastContentWidth == 0; // The current content buffer is drawn here. @@ -291,7 +291,7 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame } } - public void onRequestDraw(boolean reportNextDraw) { + void onRequestDraw(boolean reportNextDraw) { synchronized (this) { mReportNextDraw = reportNextDraw; mOldTargetRect.set(0, 0, 0, 0); @@ -329,8 +329,8 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame return; } - // Since the surface is spanning the entire screen, we have to add the start offset of - // the bounds to get to the surface location. + // Content may not be drawn at the surface origin, so we want to keep the offset when we're + // resizing it. final int left = mLastXOffset + newBounds.left; final int top = mLastYOffset + newBounds.top; final int width = newBounds.width(); @@ -414,6 +414,8 @@ public class BackdropFrameRenderer extends Thread implements Choreographer.Frame } void setUserCaptionBackgroundDrawable(Drawable userCaptionBackgroundDrawable) { - mUserCaptionBackgroundDrawable = userCaptionBackgroundDrawable; + synchronized (this) { + mUserCaptionBackgroundDrawable = userCaptionBackgroundDrawable; + } } } diff --git a/core/java/com/android/internal/policy/DecorView.java b/core/java/com/android/internal/policy/DecorView.java index 94140ab7f4409..aa7bdb62a87c9 100644 --- a/core/java/com/android/internal/policy/DecorView.java +++ b/core/java/com/android/internal/policy/DecorView.java @@ -2093,7 +2093,7 @@ public class DecorView extends FrameLayout implements RootViewSurfaceTaker, Wind initialBounds, mResizingBackgroundDrawable, mCaptionBackgroundDrawable, mUserCaptionBackgroundDrawable, getCurrentColor(mStatusColorViewState), getCurrentColor(mNavigationColorViewState), fullscreen, systemInsets, - stableInsets, resizeMode); + stableInsets); // Get rid of the shadow while we are resizing. Shadow drawing takes considerable time. // If we want to get the shadow shown while resizing, we would need to elevate a new @@ -2229,7 +2229,14 @@ public class DecorView extends FrameLayout implements RootViewSurfaceTaker, Wind // or it didn't change. if ((wasAdjustedForStack || mElevationAdjustedForStack) && getElevation() != elevation) { - mWindow.setElevation(elevation); + if (!isResizing()) { + mWindow.setElevation(elevation); + } else { + // Just suppress the shadow when resizing, don't adjust surface insets because it'll + // cause a flicker when drag resize for freeform window starts. #onContentDrawn() + // will compensate the offset when passing to BackdropFrameRenderer. + setElevation(elevation); + } } } diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index f16008de09674..2f89d5ced6cc9 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -2949,7 +2949,12 @@ class WindowState extends WindowContainer implements WindowManagerP // isDragResizing() or isDragResizeChanged() is true. boolean resizing = isDragResizing() || isDragResizeChanged(); if (getWindowConfiguration().useWindowFrameForBackdrop() || !resizing) { - return frame; + // Surface position is now inherited from parent, and BackdropFrameRenderer uses + // backdrop frame to position content. Thus we just keep the size of backdrop frame, and + // remove the offset to avoid double offset from display origin. + mTmpRect.set(frame); + mTmpRect.offsetTo(0, 0); + return mTmpRect; } final DisplayInfo displayInfo = getDisplayInfo(); mTmpRect.set(0, 0, displayInfo.logicalWidth, displayInfo.logicalHeight);