diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index 7e264cea46bb4..536a0ac6403b6 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -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 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; diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index ad3379f4c46e3..f163530f997bb 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -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 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); + } } diff --git a/core/java/android/window/SurfaceSyncer.java b/core/java/android/window/SurfaceSyncer.java index 0e011bb0d0b3e..e6eb07162a3bc 100644 --- a/core/java/android/window/SurfaceSyncer.java +++ b/core/java/android/window/SurfaceSyncer.java @@ -117,13 +117,18 @@ public class SurfaceSyncer { */ public int setupSync(@NonNull Consumer 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 mSyncTargets = new ArraySet<>(); private final int mSyncId; - private final Consumer mSyncRequestCompleteCallback; + @GuardedBy("mLock") + private Consumer mSyncRequestCompleteCallback; + + @GuardedBy("mLock") + private final Set mMergedSyncSets = new ArraySet<>(); + + @GuardedBy("mLock") + private boolean mFinished; private SyncSet(int syncId, Consumer 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 transactionConsumer) { + synchronized (mLock) { + if (mFinished) { + Log.e(TAG, "Attempting to merge SyncSet " + mSyncId + " when sync is" + + " already complete"); + transactionConsumer.accept(new Transaction()); + } + + final Consumer 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(); + } + }); + } } /** diff --git a/services/tests/wmtests/src/com/android/server/wm/SurfaceSyncerTest.java b/services/tests/wmtests/src/com/android/server/wm/SurfaceSyncerTest.java index cc28ea6f55d2f..87382952314b1 100644 --- a/services/tests/wmtests/src/com/android/server/wm/SurfaceSyncerTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/SurfaceSyncerTest.java @@ -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