From ce3460b1f2f32e6a07dcd625f831ec04cb8a1fd9 Mon Sep 17 00:00:00 2001 From: Jorge Gil Date: Wed, 12 Apr 2023 17:24:30 +0000 Subject: [PATCH] Apply the window decor transaction in sync with the View's draw When the window decoration needs to #relayout from an onTaskInfoChanged() call (usually due to resizing), the SC.Transaction that set the new task crop was applied at the same time as the View's layout was scheduled to draw, which meant that when the View is slow to draw, the new task size is visible on screen before the caption View is visible, which looks like a flicker. This change merges the transaction with the buffer transaction from the view root using #applyTransactionOnDraw, so that both are visible at the same time on screen. Bug: 270202228 Test: atest WindowDecorationTests Test: manual - drag resize a freeform task, no flickering around the edge of the caption. Change-Id: Iccdc0acdd9935b0a89759704b2e4426802f2b44a --- .../android/view/SurfaceControlViewHost.java | 7 +++ .../CaptionWindowDecorViewModel.java | 7 ++- .../windowdecor/CaptionWindowDecoration.java | 13 +++-- .../DesktopModeWindowDecorViewModel.java | 7 ++- .../DesktopModeWindowDecoration.java | 13 +++-- .../shell/windowdecor/WindowDecoration.java | 58 +++++++++++-------- .../windowdecor/WindowDecorationTests.java | 49 ++++++++++++++++ 7 files changed, 112 insertions(+), 42 deletions(-) diff --git a/core/java/android/view/SurfaceControlViewHost.java b/core/java/android/view/SurfaceControlViewHost.java index 16c133523303c..659dba3be13ff 100644 --- a/core/java/android/view/SurfaceControlViewHost.java +++ b/core/java/android/view/SurfaceControlViewHost.java @@ -406,6 +406,13 @@ public class SurfaceControlViewHost { } } + /** + * @hide + */ + public @NonNull AttachedSurfaceControl getRootSurfaceControl() { + return mViewRoot; + } + /** * Set the root view of the SurfaceControlViewHost. This view will render in to * the SurfaceControl, and receive input based on the SurfaceControls positioning on diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModel.java b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModel.java index 8e8facadfd5d4..e8a6a159cb199 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModel.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModel.java @@ -127,7 +127,7 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { if (decoration == null) { createWindowDecoration(taskInfo, taskSurface, startT, finishT); } else { - decoration.relayout(taskInfo, startT, finishT); + decoration.relayout(taskInfo, startT, finishT, false /* applyStartTransactionOnDraw */); } } @@ -139,7 +139,7 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { final CaptionWindowDecoration decoration = mWindowDecorByTaskId.get(taskInfo.taskId); if (decoration == null) return; - decoration.relayout(taskInfo, startT, finishT); + decoration.relayout(taskInfo, startT, finishT, false /* applyStartTransactionOnDraw */); } @Override @@ -192,7 +192,8 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { windowDecoration.setCaptionListeners(touchEventListener, touchEventListener); windowDecoration.setDragPositioningCallback(taskPositioner); windowDecoration.setDragDetector(touchEventListener.mDragDetector); - windowDecoration.relayout(taskInfo, startT, finishT); + windowDecoration.relayout(taskInfo, startT, finishT, + false /* applyStartTransactionOnDraw */); setupCaptionColor(taskInfo, windowDecoration); } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecoration.java b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecoration.java index dfde7e6feff5e..116af7094e131 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecoration.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecoration.java @@ -90,15 +90,15 @@ public class CaptionWindowDecoration extends WindowDecoration { - transaction.merge(t); - t.close(); - }); + // Use |applyStartTransactionOnDraw| so that the transaction (that applies task crop) is + // synced with the buffer transaction (that draws the View). Both will be shown on screen + // at the same, whereas applying them independently causes flickering. See b/270202228. + relayout(taskInfo, t, t, true /* applyStartTransactionOnDraw */); } void relayout(RunningTaskInfo taskInfo, - SurfaceControl.Transaction startT, SurfaceControl.Transaction finishT) { + SurfaceControl.Transaction startT, SurfaceControl.Transaction finishT, + boolean applyStartTransactionOnDraw) { final int shadowRadiusID = taskInfo.isFocused ? R.dimen.freeform_decor_shadow_focused_thickness : R.dimen.freeform_decor_shadow_unfocused_thickness; @@ -115,6 +115,7 @@ public class CaptionWindowDecoration extends WindowDecoration { - transaction.merge(t); - t.close(); - }); + // Use |applyStartTransactionOnDraw| so that the transaction (that applies task crop) is + // synced with the buffer transaction (that draws the View). Both will be shown on screen + // at the same, whereas applying them independently causes flickering. See b/270202228. + relayout(taskInfo, t, t, true /* applyStartTransactionOnDraw */); } void relayout(ActivityManager.RunningTaskInfo taskInfo, - SurfaceControl.Transaction startT, SurfaceControl.Transaction finishT) { + SurfaceControl.Transaction startT, SurfaceControl.Transaction finishT, + boolean applyStartTransactionOnDraw) { final int shadowRadiusID = taskInfo.isFocused ? R.dimen.freeform_decor_shadow_focused_thickness : R.dimen.freeform_decor_shadow_unfocused_thickness; @@ -216,6 +216,7 @@ public class DesktopModeWindowDecoration extends WindowDecoration startT.setWindowCrop(mCaptionContainerSurface, captionWidth, captionHeight) .show(mCaptionContainerSurface); - if (mCaptionWindowManager == null) { - // Put caption under a container surface because ViewRootImpl sets the destination frame - // of windowless window layers and BLASTBufferQueue#update() doesn't support offset. - mCaptionWindowManager = new WindowlessWindowManager( - mTaskInfo.getConfiguration(), mCaptionContainerSurface, - null /* hostInputToken */); - } - - // Caption view - mCaptionWindowManager.setConfiguration(taskConfig); - final WindowManager.LayoutParams lp = - new WindowManager.LayoutParams(captionWidth, captionHeight, - WindowManager.LayoutParams.TYPE_APPLICATION, - WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE, PixelFormat.TRANSPARENT); - lp.setTitle("Caption of Task=" + mTaskInfo.taskId); - lp.setTrustedOverlay(); - if (mViewHost == null) { - mViewHost = mSurfaceControlViewHostFactory.create(mDecorWindowContext, mDisplay, - mCaptionWindowManager); - mViewHost.setView(outResult.mRootView, lp); - } else { - mViewHost.relayout(lp); - } - if (ViewRootImpl.CAPTION_ON_SHELL) { outResult.mRootView.setTaskFocusState(mTaskInfo.isFocused); @@ -287,6 +263,36 @@ public abstract class WindowDecoration .show(mTaskSurface); finishT.setPosition(mTaskSurface, taskPosition.x, taskPosition.y) .setWindowCrop(mTaskSurface, outResult.mWidth, outResult.mHeight); + + if (mCaptionWindowManager == null) { + // Put caption under a container surface because ViewRootImpl sets the destination frame + // of windowless window layers and BLASTBufferQueue#update() doesn't support offset. + mCaptionWindowManager = new WindowlessWindowManager( + mTaskInfo.getConfiguration(), mCaptionContainerSurface, + null /* hostInputToken */); + } + + // Caption view + mCaptionWindowManager.setConfiguration(taskConfig); + final WindowManager.LayoutParams lp = + new WindowManager.LayoutParams(captionWidth, captionHeight, + WindowManager.LayoutParams.TYPE_APPLICATION, + WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE, PixelFormat.TRANSPARENT); + lp.setTitle("Caption of Task=" + mTaskInfo.taskId); + lp.setTrustedOverlay(); + if (mViewHost == null) { + mViewHost = mSurfaceControlViewHostFactory.create(mDecorWindowContext, mDisplay, + mCaptionWindowManager); + if (params.mApplyStartTransactionOnDraw) { + mViewHost.getRootSurfaceControl().applyTransactionOnDraw(startT); + } + mViewHost.setView(outResult.mRootView, lp); + } else { + if (params.mApplyStartTransactionOnDraw) { + mViewHost.getRootSurfaceControl().applyTransactionOnDraw(startT); + } + mViewHost.relayout(lp); + } } /** @@ -411,6 +417,8 @@ public abstract class WindowDecoration int mCaptionX; int mCaptionY; + boolean mApplyStartTransactionOnDraw; + void reset() { mLayoutResId = Resources.ID_NULL; mCaptionHeightId = Resources.ID_NULL; @@ -419,6 +427,8 @@ public abstract class WindowDecoration mCaptionX = 0; mCaptionY = 0; + + mApplyStartTransactionOnDraw = false; } } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/WindowDecorationTests.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/WindowDecorationTests.java index c1e53a90b7e02..fc4bfd9754a04 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/WindowDecorationTests.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/WindowDecorationTests.java @@ -33,6 +33,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.same; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.app.ActivityManager; import android.content.Context; @@ -42,6 +43,7 @@ import android.graphics.Point; import android.graphics.Rect; import android.testing.AndroidTestingRunner; import android.util.DisplayMetrics; +import android.view.AttachedSurfaceControl; import android.view.Display; import android.view.SurfaceControl; import android.view.SurfaceControlViewHost; @@ -97,6 +99,8 @@ public class WindowDecorationTests extends ShellTestCase { @Mock private SurfaceControlViewHost mMockSurfaceControlViewHost; @Mock + private AttachedSurfaceControl mMockRootSurfaceControl; + @Mock private TestView mMockView; @Mock private WindowContainerTransaction mMockWindowContainerTransaction; @@ -129,6 +133,8 @@ public class WindowDecorationTests extends ShellTestCase { doReturn(mMockSurfaceControlViewHost).when(mMockSurfaceControlViewHostFactory) .create(any(), any(), any()); + when(mMockSurfaceControlViewHost.getRootSurfaceControl()) + .thenReturn(mMockRootSurfaceControl); } @Test @@ -461,6 +467,43 @@ public class WindowDecorationTests extends ShellTestCase { verify(mMockSurfaceControlStartT).show(captionContainerSurface); } + @Test + public void testRelayout_applyTransactionInSyncWithDraw() { + final Display defaultDisplay = mock(Display.class); + doReturn(defaultDisplay).when(mMockDisplayController) + .getDisplay(Display.DEFAULT_DISPLAY); + + final SurfaceControl decorContainerSurface = mock(SurfaceControl.class); + final SurfaceControl.Builder decorContainerSurfaceBuilder = + createMockSurfaceControlBuilder(decorContainerSurface); + mMockSurfaceControlBuilders.add(decorContainerSurfaceBuilder); + final SurfaceControl captionContainerSurface = mock(SurfaceControl.class); + final SurfaceControl.Builder captionContainerSurfaceBuilder = + createMockSurfaceControlBuilder(captionContainerSurface); + mMockSurfaceControlBuilders.add(captionContainerSurfaceBuilder); + + final SurfaceControl.Transaction t = mock(SurfaceControl.Transaction.class); + mMockSurfaceControlTransactions.add(t); + final ActivityManager.TaskDescription.Builder taskDescriptionBuilder = + new ActivityManager.TaskDescription.Builder() + .setBackgroundColor(Color.YELLOW); + final ActivityManager.RunningTaskInfo taskInfo = new TestRunningTaskInfoBuilder() + .setDisplayId(Display.DEFAULT_DISPLAY) + .setTaskDescriptionBuilder(taskDescriptionBuilder) + .setBounds(TASK_BOUNDS) + .setPositionInParent(TASK_POSITION_IN_PARENT.x, TASK_POSITION_IN_PARENT.y) + .setVisible(true) + .build(); + taskInfo.isFocused = true; + taskInfo.configuration.densityDpi = DisplayMetrics.DENSITY_DEFAULT * 2; + final SurfaceControl taskSurface = mock(SurfaceControl.class); + final TestWindowDecoration windowDecor = createWindowDecoration(taskInfo, taskSurface); + + windowDecor.relayout(taskInfo, true /* applyStartTransactionOnDraw */); + + verify(mMockRootSurfaceControl).applyTransactionOnDraw(mMockSurfaceControlStartT); + } + private TestWindowDecoration createWindowDecoration( ActivityManager.RunningTaskInfo taskInfo, SurfaceControl testSurface) { return new TestWindowDecoration(InstrumentationRegistry.getInstrumentation().getContext(), @@ -516,6 +559,12 @@ public class WindowDecorationTests extends ShellTestCase { @Override void relayout(ActivityManager.RunningTaskInfo taskInfo) { + relayout(taskInfo, false /* applyStartTransactionOnDraw */); + } + + void relayout(ActivityManager.RunningTaskInfo taskInfo, + boolean applyStartTransactionOnDraw) { + mRelayoutParams.mApplyStartTransactionOnDraw = applyStartTransactionOnDraw; relayout(mRelayoutParams, mMockSurfaceControlStartT, mMockSurfaceControlFinishT, mMockWindowContainerTransaction, mMockView, mRelayoutResult); }