Merge "Make sure VRI waits for SV to draw before report" into tm-dev am: 4c2c7ebfaa

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/18820396

Change-Id: Ie40bf29c89acb2043c80736144b72e10d4717576
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Chavi Weingarten
2022-06-13 23:12:10 +00:00
committed by Automerger Merge Worker
4 changed files with 237 additions and 37 deletions

View File

@@ -41,11 +41,13 @@ import android.os.Handler;
import android.os.IBinder;
import android.os.Looper;
import android.os.SystemClock;
import android.util.ArraySet;
import android.util.AttributeSet;
import android.util.Log;
import android.view.SurfaceControl.Transaction;
import android.view.accessibility.AccessibilityNodeInfo;
import android.view.accessibility.IAccessibilityEmbeddedConnection;
import android.window.SurfaceSyncer;
import com.android.internal.view.SurfaceCallbackHelper;
@@ -204,6 +206,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
private int mSurfaceFlags = SurfaceControl.HIDDEN;
private final SurfaceSyncer mSurfaceSyncer = new SurfaceSyncer();
private final ArraySet<Integer> mSyncIds = new ArraySet<>();
/**
* Transaction that should be used from the render thread. This transaction is only thread safe
* with other calls directly from the render thread.
@@ -1018,7 +1023,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
if (shouldSyncBuffer) {
handleSyncBufferCallback(callbacks, syncBufferTransactionCallback);
} else {
redrawNeededAsync(callbacks, this::onDrawFinished);
handleSyncNoBuffer(callbacks);
}
}
}
@@ -1061,7 +1066,23 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
syncBufferCallback.onBufferReady(t);
onDrawFinished();
}));
}
private void handleSyncNoBuffer(SurfaceHolder.Callback[] callbacks) {
final int syncId = mSurfaceSyncer.setupSync(this::onDrawFinished);
mSurfaceSyncer.addToSync(syncId, syncBufferCallback -> redrawNeededAsync(callbacks,
() -> {
syncBufferCallback.onBufferReady(null);
synchronized (mSyncIds) {
mSyncIds.remove(syncId);
}
}));
mSurfaceSyncer.markSyncReady(syncId);
synchronized (mSyncIds) {
mSyncIds.add(syncId);
}
}
private void redrawNeededAsync(SurfaceHolder.Callback[] callbacks,
@@ -1070,6 +1091,21 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
sch.dispatchSurfaceRedrawNeededAsync(mSurfaceHolder, callbacks);
}
/**
* @hide
*/
@Override
public void surfaceSyncStarted() {
ViewRootImpl viewRoot = getViewRootImpl();
if (viewRoot != null) {
synchronized (mSyncIds) {
for (int syncId : mSyncIds) {
viewRoot.mergeSync(syncId, mSurfaceSyncer);
}
}
}
}
private static class SyncBufferTransactionCallback {
private final CountDownLatch mCountDownLatch = new CountDownLatch(1);
private Transaction mTransaction;

View File

@@ -2048,6 +2048,7 @@ public final class ViewRootImpl implements ViewParent,
void surfaceCreated(Transaction t);
void surfaceReplaced(Transaction t);
void surfaceDestroyed();
default void surfaceSyncStarted() {};
}
private final ArrayList<SurfaceChangedCallback> mSurfaceChangedCallbacks = new ArrayList<>();
@@ -2082,6 +2083,12 @@ public final class ViewRootImpl implements ViewParent,
}
}
private void notifySurfaceSyncStarted() {
for (int i = 0; i < mSurfaceChangedCallbacks.size(); i++) {
mSurfaceChangedCallbacks.get(i).surfaceSyncStarted();
}
}
/**
* @return child layer with the same bounds as its parent {@code mSurface} and cropped to the
* surface insets. If the layer does not exist, it is created.
@@ -3541,6 +3548,7 @@ public final class ViewRootImpl implements ViewParent,
Log.d(mTag, "Setup new sync id=" + mSyncId);
}
mSurfaceSyncer.addToSync(mSyncId, mSyncTarget);
notifySurfaceSyncStarted();
}
private void notifyContentCatpureEvents() {
@@ -10965,4 +10973,11 @@ public final class ViewRootImpl implements ViewParent,
scheduleTraversals();
}
}
void mergeSync(int syncId, SurfaceSyncer otherSyncer) {
if (!isInLocalSync()) {
return;
}
mSurfaceSyncer.merge(mSyncId, syncId, otherSyncer);
}
}

View File

@@ -117,13 +117,18 @@ public class SurfaceSyncer {
*/
public int setupSync(@NonNull Consumer<Transaction> syncRequestComplete) {
synchronized (mSyncSetLock) {
mIdCounter++;
final int syncId = mIdCounter++;
if (DEBUG) {
Log.d(TAG, "setupSync " + mIdCounter);
Log.d(TAG, "setupSync " + syncId);
}
SyncSet syncSet = new SyncSet(mIdCounter, syncRequestComplete);
mSyncSets.put(mIdCounter, syncSet);
return mIdCounter;
SyncSet syncSet = new SyncSet(syncId, transaction -> {
synchronized (mSyncSetLock) {
mSyncSets.remove(syncId);
}
syncRequestComplete.accept(transaction);
});
mSyncSets.put(syncId, syncSet);
return syncId;
}
}
@@ -138,7 +143,6 @@ public class SurfaceSyncer {
SyncSet syncSet;
synchronized (mSyncSetLock) {
syncSet = mSyncSets.get(syncId);
mSyncSets.remove(syncId);
}
if (syncSet == null) {
Log.e(TAG, "Failed to find syncSet for syncId=" + syncId);
@@ -147,6 +151,31 @@ public class SurfaceSyncer {
syncSet.markSyncReady();
}
/**
* Merge another SyncSet into the specified syncId.
* @param syncId The current syncId to merge into
* @param otherSyncId The other syncId to be merged
* @param otherSurfaceSyncer The other SurfaceSyncer where the otherSyncId is from
*/
public void merge(int syncId, int otherSyncId, SurfaceSyncer otherSurfaceSyncer) {
SyncSet syncSet;
synchronized (mSyncSetLock) {
syncSet = mSyncSets.get(syncId);
}
SyncSet otherSyncSet = otherSurfaceSyncer.getAndValidateSyncSet(otherSyncId);
if (otherSyncSet == null) {
return;
}
if (DEBUG) {
Log.d(TAG,
"merge id=" + otherSyncId + " from=" + otherSurfaceSyncer + " into id=" + syncId
+ " from" + this);
}
syncSet.merge(otherSyncSet);
}
/**
* Add a SurfaceView to a sync set. This is different than {@link #addToSync(int, View)} because
* it requires the caller to notify the start and finish drawing in order to sync.
@@ -199,8 +228,7 @@ public class SurfaceSyncer {
if (DEBUG) {
Log.d(TAG, "addToSync id=" + syncId);
}
syncSet.addSyncableSurface(syncTarget);
return true;
return syncSet.addSyncableSurface(syncTarget);
}
/**
@@ -284,14 +312,21 @@ public class SurfaceSyncer {
private final Set<SyncTarget> mSyncTargets = new ArraySet<>();
private final int mSyncId;
private final Consumer<Transaction> mSyncRequestCompleteCallback;
@GuardedBy("mLock")
private Consumer<Transaction> mSyncRequestCompleteCallback;
@GuardedBy("mLock")
private final Set<SyncSet> mMergedSyncSets = new ArraySet<>();
@GuardedBy("mLock")
private boolean mFinished;
private SyncSet(int syncId, Consumer<Transaction> syncRequestComplete) {
mSyncId = syncId;
mSyncRequestCompleteCallback = syncRequestComplete;
}
void addSyncableSurface(SyncTarget syncTarget) {
boolean addSyncableSurface(SyncTarget syncTarget) {
SyncBufferCallback syncBufferCallback = new SyncBufferCallback() {
@Override
public void onBufferReady(Transaction t) {
@@ -306,10 +341,16 @@ public class SurfaceSyncer {
};
synchronized (mLock) {
if (mSyncReady) {
Log.e(TAG, "Sync " + mSyncId + " was already marked as ready. No more "
+ "SyncTargets can be added.");
return false;
}
mPendingSyncs.add(syncBufferCallback.hashCode());
mSyncTargets.add(syncTarget);
}
syncTarget.onReadyToSync(syncBufferCallback);
return true;
}
void markSyncReady() {
@@ -321,10 +362,11 @@ public class SurfaceSyncer {
@GuardedBy("mLock")
private void checkIfSyncIsComplete() {
if (!mSyncReady || !mPendingSyncs.isEmpty()) {
if (!mSyncReady || !mPendingSyncs.isEmpty() || !mMergedSyncSets.isEmpty()) {
if (DEBUG) {
Log.d(TAG, "Syncable is not complete. mSyncReady=" + mSyncReady
+ " mPendingSyncs=" + mPendingSyncs.size());
+ " mPendingSyncs=" + mPendingSyncs.size() + " mergedSyncs="
+ mMergedSyncSets.size());
}
return;
}
@@ -338,6 +380,7 @@ public class SurfaceSyncer {
}
mSyncTargets.clear();
mSyncRequestCompleteCallback.accept(mTransaction);
mFinished = true;
}
/**
@@ -349,6 +392,50 @@ public class SurfaceSyncer {
mTransaction.merge(t);
}
}
public void updateCallback(Consumer<Transaction> transactionConsumer) {
synchronized (mLock) {
if (mFinished) {
Log.e(TAG, "Attempting to merge SyncSet " + mSyncId + " when sync is"
+ " already complete");
transactionConsumer.accept(new Transaction());
}
final Consumer<Transaction> oldCallback = mSyncRequestCompleteCallback;
mSyncRequestCompleteCallback = transaction -> {
oldCallback.accept(new Transaction());
transactionConsumer.accept(transaction);
};
}
}
/**
* Merge a SyncSet into this SyncSet. Since SyncSets could still have pending SyncTargets,
* we need to make sure those can still complete before the mergeTo syncSet is considered
* complete.
*
* We keep track of all the merged SyncSets until they are marked as done, and then they
* are removed from the set. This SyncSet is not considered done until all the merged
* SyncSets are done.
*
* When the merged SyncSet is complete, it will invoke the original syncRequestComplete
* callback but send an empty transaction to ensure the changes are applied early. This
* is needed in case the original sync is relying on the callback to continue processing.
*
* @param otherSyncSet The other SyncSet to merge into this one.
*/
public void merge(SyncSet otherSyncSet) {
synchronized (mLock) {
mMergedSyncSets.add(otherSyncSet);
}
otherSyncSet.updateCallback(transaction -> {
synchronized (mLock) {
mMergedSyncSets.remove(otherSyncSet);
mTransaction.merge(transaction);
checkIfSyncIsComplete();
}
});
}
}
/**

View File

@@ -48,11 +48,11 @@ public class SurfaceSyncerTest {
public void testSyncOne() throws InterruptedException {
final CountDownLatch finishedLatch = new CountDownLatch(1);
int startSyncId = mSurfaceSyncer.setupSync(transaction -> finishedLatch.countDown());
Syncable syncable = new Syncable();
mSurfaceSyncer.addToSync(startSyncId, syncable);
SyncTarget syncTarget = new SyncTarget();
mSurfaceSyncer.addToSync(startSyncId, syncTarget);
mSurfaceSyncer.markSyncReady(startSyncId);
syncable.onBufferReady();
syncTarget.onBufferReady();
finishedLatch.await(5, TimeUnit.SECONDS);
assertEquals(0, finishedLatch.getCount());
@@ -62,22 +62,22 @@ public class SurfaceSyncerTest {
public void testSyncMultiple() throws InterruptedException {
final CountDownLatch finishedLatch = new CountDownLatch(1);
int startSyncId = mSurfaceSyncer.setupSync(transaction -> finishedLatch.countDown());
Syncable syncable1 = new Syncable();
Syncable syncable2 = new Syncable();
Syncable syncable3 = new Syncable();
SyncTarget syncTarget1 = new SyncTarget();
SyncTarget syncTarget2 = new SyncTarget();
SyncTarget syncTarget3 = new SyncTarget();
mSurfaceSyncer.addToSync(startSyncId, syncable1);
mSurfaceSyncer.addToSync(startSyncId, syncable2);
mSurfaceSyncer.addToSync(startSyncId, syncable3);
mSurfaceSyncer.addToSync(startSyncId, syncTarget1);
mSurfaceSyncer.addToSync(startSyncId, syncTarget2);
mSurfaceSyncer.addToSync(startSyncId, syncTarget3);
mSurfaceSyncer.markSyncReady(startSyncId);
syncable1.onBufferReady();
syncTarget1.onBufferReady();
assertNotEquals(0, finishedLatch.getCount());
syncable3.onBufferReady();
syncTarget3.onBufferReady();
assertNotEquals(0, finishedLatch.getCount());
syncable2.onBufferReady();
syncTarget2.onBufferReady();
finishedLatch.await(5, TimeUnit.SECONDS);
assertEquals(0, finishedLatch.getCount());
@@ -85,7 +85,7 @@ public class SurfaceSyncerTest {
@Test
public void testInvalidSyncId() {
assertFalse(mSurfaceSyncer.addToSync(0, new Syncable()));
assertFalse(mSurfaceSyncer.addToSync(0, new SyncTarget()));
}
@Test
@@ -93,14 +93,14 @@ public class SurfaceSyncerTest {
final CountDownLatch finishedLatch = new CountDownLatch(1);
int startSyncId = mSurfaceSyncer.setupSync(transaction -> finishedLatch.countDown());
Syncable syncable1 = new Syncable();
Syncable syncable2 = new Syncable();
SyncTarget syncTarget1 = new SyncTarget();
SyncTarget syncTarget2 = new SyncTarget();
assertTrue(mSurfaceSyncer.addToSync(startSyncId, syncable1));
assertTrue(mSurfaceSyncer.addToSync(startSyncId, syncTarget1));
mSurfaceSyncer.markSyncReady(startSyncId);
// Adding to a sync that has been completed is also invalid since the sync id has been
// cleared.
assertFalse(mSurfaceSyncer.addToSync(startSyncId, syncable2));
assertFalse(mSurfaceSyncer.addToSync(startSyncId, syncTarget2));
}
@Test
@@ -110,27 +110,89 @@ public class SurfaceSyncerTest {
int startSyncId1 = mSurfaceSyncer.setupSync(transaction -> finishedLatch1.countDown());
int startSyncId2 = mSurfaceSyncer.setupSync(transaction -> finishedLatch2.countDown());
Syncable syncable1 = new Syncable();
Syncable syncable2 = new Syncable();
SyncTarget syncTarget1 = new SyncTarget();
SyncTarget syncTarget2 = new SyncTarget();
assertTrue(mSurfaceSyncer.addToSync(startSyncId1, syncable1));
assertTrue(mSurfaceSyncer.addToSync(startSyncId2, syncable2));
assertTrue(mSurfaceSyncer.addToSync(startSyncId1, syncTarget1));
assertTrue(mSurfaceSyncer.addToSync(startSyncId2, syncTarget2));
mSurfaceSyncer.markSyncReady(startSyncId1);
mSurfaceSyncer.markSyncReady(startSyncId2);
syncable1.onBufferReady();
syncTarget1.onBufferReady();
finishedLatch1.await(5, TimeUnit.SECONDS);
assertEquals(0, finishedLatch1.getCount());
assertNotEquals(0, finishedLatch2.getCount());
syncable2.onBufferReady();
syncTarget2.onBufferReady();
finishedLatch2.await(5, TimeUnit.SECONDS);
assertEquals(0, finishedLatch2.getCount());
}
private static class Syncable implements SurfaceSyncer.SyncTarget {
@Test
public void testMergeSync() throws InterruptedException {
final CountDownLatch finishedLatch1 = new CountDownLatch(1);
final CountDownLatch finishedLatch2 = new CountDownLatch(1);
int startSyncId1 = mSurfaceSyncer.setupSync(transaction -> finishedLatch1.countDown());
int startSyncId2 = mSurfaceSyncer.setupSync(transaction -> finishedLatch2.countDown());
SyncTarget syncTarget1 = new SyncTarget();
SyncTarget syncTarget2 = new SyncTarget();
assertTrue(mSurfaceSyncer.addToSync(startSyncId1, syncTarget1));
assertTrue(mSurfaceSyncer.addToSync(startSyncId2, syncTarget2));
mSurfaceSyncer.markSyncReady(startSyncId1);
mSurfaceSyncer.merge(startSyncId2, startSyncId1, mSurfaceSyncer);
mSurfaceSyncer.markSyncReady(startSyncId2);
// Finish syncTarget2 first to test that the syncSet is not complete until the merged sync
// is also done.
syncTarget2.onBufferReady();
finishedLatch2.await(1, TimeUnit.SECONDS);
// Sync did not complete yet
assertNotEquals(0, finishedLatch2.getCount());
syncTarget1.onBufferReady();
// The first sync will still get a callback when it's sync requirements are done.
finishedLatch1.await(5, TimeUnit.SECONDS);
assertEquals(0, finishedLatch1.getCount());
finishedLatch2.await(5, TimeUnit.SECONDS);
assertEquals(0, finishedLatch2.getCount());
}
@Test
public void testMergeSyncAlreadyComplete() throws InterruptedException {
final CountDownLatch finishedLatch1 = new CountDownLatch(1);
final CountDownLatch finishedLatch2 = new CountDownLatch(1);
int startSyncId1 = mSurfaceSyncer.setupSync(transaction -> finishedLatch1.countDown());
int startSyncId2 = mSurfaceSyncer.setupSync(transaction -> finishedLatch2.countDown());
SyncTarget syncTarget1 = new SyncTarget();
SyncTarget syncTarget2 = new SyncTarget();
assertTrue(mSurfaceSyncer.addToSync(startSyncId1, syncTarget1));
assertTrue(mSurfaceSyncer.addToSync(startSyncId2, syncTarget2));
mSurfaceSyncer.markSyncReady(startSyncId1);
syncTarget1.onBufferReady();
// The first sync will still get a callback when it's sync requirements are done.
finishedLatch1.await(5, TimeUnit.SECONDS);
assertEquals(0, finishedLatch1.getCount());
mSurfaceSyncer.merge(startSyncId2, startSyncId1, mSurfaceSyncer);
mSurfaceSyncer.markSyncReady(startSyncId2);
syncTarget2.onBufferReady();
// Verify that the second sync will receive complete since the merged sync was already
// completed before the merge.
finishedLatch2.await(5, TimeUnit.SECONDS);
assertEquals(0, finishedLatch2.getCount());
}
private static class SyncTarget implements SurfaceSyncer.SyncTarget {
private SurfaceSyncer.SyncBufferCallback mSyncBufferCallback;
@Override