Merge "Ensure overlapping syncs don't submit buffers out of order" into udc-dev

This commit is contained in:
Chavi Weingarten
2023-04-20 14:04:43 +00:00
committed by Android (Google) Code Review
5 changed files with 298 additions and 7 deletions

View File

@@ -885,6 +885,16 @@ public final class ViewRootImpl implements ViewParent,
*/
private SurfaceSyncGroup mActiveSurfaceSyncGroup;
private final Object mPreviousSyncSafeguardLock = new Object();
/**
* Wraps the TransactionCommitted callback for the previous SSG so it can be added to the next
* SSG if started before previous has completed.
*/
@GuardedBy("mPreviousSyncSafeguardLock")
private SurfaceSyncGroup mPreviousSyncSafeguard;
private static final Object sSyncProgressLock = new Object();
// The count needs to be static since it's used to enable or disable RT animations which is
// done at a global level per process. If any VRI syncs are in progress, we can't enable RT
@@ -11329,6 +11339,61 @@ public final class ViewRootImpl implements ViewParent,
});
}
/**
* This code will ensure that if multiple SurfaceSyncGroups are created for the same
* ViewRootImpl the SurfaceSyncGroups will maintain an order. The scenario that could occur
* is the following:
* <p>
* 1. SSG1 is created that includes the target VRI. There could be other VRIs in SSG1
* 2. The target VRI draws its frame and marks its own active SSG as ready, but SSG1 is still
* waiting on other things in the SSG
* 3. Another SSG2 is created for the target VRI. The second frame renders and marks its own
* second SSG as complete. SSG2 has nothing else to wait on, so it will apply at this point,
* even though SSG1 has not finished.
* 4. Frame2 will get to SF first and Frame1 will later get to SF when SSG1 completes.
* <p>
* The code below ensures the SSGs that contains the VRI maintain an order. We create a new SSG
* that's a safeguard SSG. Its only job is to prevent the next active SSG from completing.
* The current active SSG for VRI will add a transaction committed callback and when that's
* invoked, it will mark the safeguard SSG as ready. If a new request to create a SSG comes
* in and the safeguard SSG is not null, it's added as part of the new active SSG. A new
* safeguard SSG is created to correspond to the new active SSG. This creates a chain to
* ensure the latter SSG always waits for the former SSG's transaction to get to SF.
*/
private void safeguardOverlappingSyncs(SurfaceSyncGroup activeSurfaceSyncGroup) {
SurfaceSyncGroup safeguardSsg = new SurfaceSyncGroup("VRI-Safeguard");
// Always disable timeout on the safeguard sync
safeguardSsg.toggleTimeout(false /* enable */);
synchronized (mPreviousSyncSafeguardLock) {
if (mPreviousSyncSafeguard != null) {
activeSurfaceSyncGroup.add(mPreviousSyncSafeguard, null /* runnable */);
// Temporarily disable the timeout on the SSG that will contain the buffer. This
// is to ensure we don't timeout the active SSG before the previous one completes to
// ensure the order is maintained. The previous SSG has a timeout on its own SSG
// so it's guaranteed to complete.
activeSurfaceSyncGroup.toggleTimeout(false /* enable */);
mPreviousSyncSafeguard.addSyncCompleteCallback(mSimpleExecutor, () -> {
// Once we receive that the previous sync guard has been invoked, we can re-add
// the timeout on the active sync to ensure we eventually complete so it's not
// stuck permanently.
activeSurfaceSyncGroup.toggleTimeout(true /*enable */);
});
}
mPreviousSyncSafeguard = safeguardSsg;
}
Transaction t = new Transaction();
t.addTransactionCommittedListener(mSimpleExecutor, () -> {
safeguardSsg.markSyncReady();
synchronized (mPreviousSyncSafeguardLock) {
if (mPreviousSyncSafeguard == safeguardSsg) {
mPreviousSyncSafeguard = null;
}
}
});
activeSurfaceSyncGroup.addTransaction(t);
}
@Override
public SurfaceSyncGroup getOrCreateSurfaceSyncGroup() {
boolean newSyncGroup = false;
@@ -11355,6 +11420,7 @@ public final class ViewRootImpl implements ViewParent,
mHandler.post(runnable);
}
});
safeguardOverlappingSyncs(mActiveSurfaceSyncGroup);
updateSyncInProgressCount(mActiveSurfaceSyncGroup);
newSyncGroup = true;
}

View File

@@ -38,6 +38,7 @@ import android.view.SurfaceView;
import android.view.WindowManagerGlobal;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicInteger;
@@ -63,7 +64,12 @@ public final class SurfaceSyncGroup {
private static final int MAX_COUNT = 100;
private static final AtomicInteger sCounter = new AtomicInteger(0);
private static final int TRANSACTION_READY_TIMEOUT = 1000 * Build.HW_TIMEOUT_MULTIPLIER;
/**
* @hide
*/
@VisibleForTesting
public static final int TRANSACTION_READY_TIMEOUT = 1000 * Build.HW_TIMEOUT_MULTIPLIER;
private static Supplier<Transaction> sTransactionFactory = Transaction::new;
@@ -123,6 +129,14 @@ public final class SurfaceSyncGroup {
@GuardedBy("mLock")
private boolean mTimeoutAdded;
/**
* Disable the timeout for this SSG so it will never be set until there's an explicit call to
* add a timeout.
*/
@GuardedBy("mLock")
private boolean mTimeoutDisabled;
private static boolean isLocalBinder(IBinder binder) {
return !(binder instanceof BinderProxy);
}
@@ -223,6 +237,10 @@ public final class SurfaceSyncGroup {
*/
public void addSyncCompleteCallback(Executor executor, Runnable runnable) {
synchronized (mLock) {
if (mFinished) {
executor.execute(runnable);
return;
}
mSyncCompleteCallbacks.add(new Pair<>(executor, runnable));
}
}
@@ -768,6 +786,21 @@ public final class SurfaceSyncGroup {
}
}
/**
* @hide
*/
public void toggleTimeout(boolean enable) {
synchronized (mLock) {
mTimeoutDisabled = !enable;
if (mTimeoutAdded && !enable) {
mHandler.removeCallbacksAndMessages(this);
mTimeoutAdded = false;
} else if (!mTimeoutAdded && enable) {
addTimeout();
}
}
}
private void addTimeout() {
synchronized (sHandlerThreadLock) {
if (sHandlerThread == null) {
@@ -777,7 +810,7 @@ public final class SurfaceSyncGroup {
}
synchronized (mLock) {
if (mTimeoutAdded) {
if (mTimeoutAdded || mTimeoutDisabled) {
// We only need one timeout for the entire SurfaceSyncGroup since we just want to
// ensure it doesn't stay stuck forever.
return;

View File

@@ -168,14 +168,13 @@ class BLASTSyncEngine {
class CommitCallback implements Runnable {
// Can run a second time if the action completes after the timeout.
boolean ran = false;
public void onCommitted() {
public void onCommitted(SurfaceControl.Transaction t) {
synchronized (mWm.mGlobalLock) {
if (ran) {
return;
}
mHandler.removeCallbacks(this);
ran = true;
SurfaceControl.Transaction t = new SurfaceControl.Transaction();
for (WindowContainer wc : wcAwaitingCommit) {
wc.onSyncTransactionCommitted(t);
}
@@ -194,12 +193,12 @@ class BLASTSyncEngine {
Slog.e(TAG, "WM sent Transaction to organized, but never received" +
" commit callback. Application ANR likely to follow.");
Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
onCommitted();
onCommitted(merged);
}
};
CommitCallback callback = new CommitCallback();
merged.addTransactionCommittedListener((r) -> { r.run(); }, callback::onCommitted);
merged.addTransactionCommittedListener(Runnable::run,
() -> callback.onCommitted(new SurfaceControl.Transaction()));
mHandler.postDelayed(callback, BLAST_TIMEOUT_DURATION);
Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "onTransactionReady");

View File

@@ -89,6 +89,12 @@
<activity android:name="com.android.server.wm.SurfaceControlViewHostTests$TestActivity" />
<activity android:name="android.server.wm.scvh.SurfaceSyncGroupActivity"
android:screenOrientation="locked"
android:turnScreenOn="true"
android:theme="@style/WhiteBackgroundTheme"
android:exported="true"/>
<service android:name="android.view.cts.surfacevalidator.LocalMediaProjectionService"
android:foregroundServiceType="mediaProjection"
android:enabled="true">

View File

@@ -0,0 +1,187 @@
/*
* Copyright (C) 2023 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.server.wm;
import static android.window.SurfaceSyncGroup.TRANSACTION_READY_TIMEOUT;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import android.app.Instrumentation;
import android.graphics.Bitmap;
import android.graphics.Color;
import android.graphics.PixelFormat;
import android.graphics.Rect;
import android.os.Handler;
import android.os.HandlerThread;
import android.platform.test.annotations.Presubmit;
import android.server.wm.scvh.SurfaceSyncGroupActivity;
import android.view.SurfaceControl;
import android.view.View;
import android.view.ViewTreeObserver;
import android.view.WindowManager;
import android.view.cts.surfacevalidator.BitmapPixelChecker;
import android.window.SurfaceSyncGroup;
import androidx.test.InstrumentationRegistry;
import androidx.test.rule.ActivityTestRule;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@Presubmit
public class SurfaceSyncGroupTests {
private static final String TAG = "SurfaceSyncGroupTests";
@Rule
public ActivityTestRule<SurfaceSyncGroupActivity> mActivityRule = new ActivityTestRule<>(
SurfaceSyncGroupActivity.class);
private SurfaceSyncGroupActivity mActivity;
Instrumentation mInstrumentation;
private final HandlerThread mHandlerThread = new HandlerThread("applyTransaction");
private Handler mHandler;
@Before
public void setup() {
mActivity = mActivityRule.getActivity();
mInstrumentation = InstrumentationRegistry.getInstrumentation();
mHandlerThread.start();
mHandler = mHandlerThread.getThreadHandler();
}
@Test
public void testOverlappingSyncsEnsureOrder_WhenTimeout() throws InterruptedException {
WindowManager.LayoutParams params = new WindowManager.LayoutParams();
params.format = PixelFormat.TRANSLUCENT;
CountDownLatch secondDrawCompleteLatch = new CountDownLatch(1);
CountDownLatch bothSyncGroupsComplete = new CountDownLatch(2);
final SurfaceSyncGroup firstSsg = new SurfaceSyncGroup(TAG + "-first");
final SurfaceSyncGroup secondSsg = new SurfaceSyncGroup(TAG + "-second");
final SurfaceSyncGroup infiniteSsg = new SurfaceSyncGroup(TAG + "-infinite");
SurfaceControl.Transaction t = new SurfaceControl.Transaction();
t.addTransactionCommittedListener(Runnable::run, bothSyncGroupsComplete::countDown);
firstSsg.addTransaction(t);
View backgroundView = mActivity.getBackgroundView();
firstSsg.add(backgroundView.getRootSurfaceControl(),
() -> mActivity.runOnUiThread(() -> backgroundView.setBackgroundColor(Color.RED)));
addSecondSyncGroup(secondSsg, secondDrawCompleteLatch, bothSyncGroupsComplete);
assertTrue("Failed to draw two frames",
secondDrawCompleteLatch.await(5, TimeUnit.SECONDS));
mHandler.postDelayed(() -> {
// Don't add a markSyncReady for the first sync group until after it's added to another
// SSG to ensure the timeout is longer than the second frame's timeout. The infinite SSG
// will never complete to ensure it reaches the timeout, but only after the second SSG
// had a chance to reach its timeout.
infiniteSsg.add(firstSsg, null /* runnable */);
firstSsg.markSyncReady();
}, 200);
assertTrue("Failed to wait for both SurfaceSyncGroups to apply",
bothSyncGroupsComplete.await(5, TimeUnit.SECONDS));
validateScreenshot();
}
@Test
public void testOverlappingSyncsEnsureOrder_WhileHoldingTransaction()
throws InterruptedException {
WindowManager.LayoutParams params = new WindowManager.LayoutParams();
params.format = PixelFormat.TRANSLUCENT;
CountDownLatch secondDrawCompleteLatch = new CountDownLatch(1);
CountDownLatch bothSyncGroupsComplete = new CountDownLatch(2);
final SurfaceSyncGroup firstSsg = new SurfaceSyncGroup(TAG + "-first",
transaction -> mHandler.postDelayed(() -> {
try {
assertTrue("Failed to draw two frames",
secondDrawCompleteLatch.await(5, TimeUnit.SECONDS));
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
transaction.apply();
}, TRANSACTION_READY_TIMEOUT + 200));
final SurfaceSyncGroup secondSsg = new SurfaceSyncGroup(TAG + "-second");
SurfaceControl.Transaction t = new SurfaceControl.Transaction();
t.addTransactionCommittedListener(Runnable::run, bothSyncGroupsComplete::countDown);
firstSsg.addTransaction(t);
View backgroundView = mActivity.getBackgroundView();
firstSsg.add(backgroundView.getRootSurfaceControl(),
() -> mActivity.runOnUiThread(() -> backgroundView.setBackgroundColor(Color.RED)));
firstSsg.markSyncReady();
addSecondSyncGroup(secondSsg, secondDrawCompleteLatch, bothSyncGroupsComplete);
assertTrue("Failed to wait for both SurfaceSyncGroups to apply",
bothSyncGroupsComplete.await(5, TimeUnit.SECONDS));
validateScreenshot();
}
private void addSecondSyncGroup(SurfaceSyncGroup surfaceSyncGroup,
CountDownLatch waitForSecondDraw, CountDownLatch bothSyncGroupsComplete) {
View backgroundView = mActivity.getBackgroundView();
ViewTreeObserver viewTreeObserver = backgroundView.getViewTreeObserver();
viewTreeObserver.registerFrameCommitCallback(() -> mHandler.post(() -> {
surfaceSyncGroup.add(backgroundView.getRootSurfaceControl(),
() -> mActivity.runOnUiThread(
() -> backgroundView.setBackgroundColor(Color.BLUE)));
SurfaceControl.Transaction t = new SurfaceControl.Transaction();
t.addTransactionCommittedListener(Runnable::run, bothSyncGroupsComplete::countDown);
surfaceSyncGroup.addTransaction(t);
surfaceSyncGroup.markSyncReady();
viewTreeObserver.registerFrameCommitCallback(waitForSecondDraw::countDown);
}));
}
private void validateScreenshot() {
Bitmap screenshot = mInstrumentation.getUiAutomation().takeScreenshot(
mActivity.getWindow());
assertNotNull("Failed to generate a screenshot", screenshot);
Bitmap swBitmap = screenshot.copy(Bitmap.Config.ARGB_8888, false);
screenshot.recycle();
BitmapPixelChecker pixelChecker = new BitmapPixelChecker(Color.BLUE);
int halfWidth = swBitmap.getWidth() / 2;
int halfHeight = swBitmap.getHeight() / 2;
// We don't need to check all the pixels since we only care that at least some of them are
// blue. If the buffers were submitted out of order, all the pixels will be red.
Rect bounds = new Rect(halfWidth, halfHeight, halfWidth + 10, halfHeight + 10);
int numMatchingPixels = pixelChecker.getNumMatchingPixels(swBitmap, bounds);
assertEquals("Expected 100 received " + numMatchingPixels + " matching pixels", 100,
numMatchingPixels);
swBitmap.recycle();
}
}