Changed setSyncTransaction to syncNextTransaction with callback logic.

Modified SurfaceView and ViewRootImpl to provide callbacks when syncing
a transaction instead of sending transactions. This will ensure a clear
ownership of the transaction object since it will be provided in the
callback when BBQ adds a buffer into the transaction. Modified JNI to
add compatibility for native functions.

Test: Manual testing from go/wm-smoke.
Test: BLASTBufferQueueTest
Bug: 210714235.
Change-Id: I1f872d6b2846b0d64d5b33b8866d0b2ec7126c8c
This commit is contained in:
Tianhao Yao 2022-02-07 19:44:33 +00:00 committed by chaviw
parent 0563d4fb32
commit 5ecd6d37de
7 changed files with 201 additions and 124 deletions

View File

@ -51,6 +51,7 @@ import com.android.internal.view.SurfaceCallbackHelper;
import java.util.ArrayList;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer;
/**
* Provides a dedicated drawing surface embedded inside of a view hierarchy.
@ -397,8 +398,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
}
}
private void performDrawFinished(Transaction t) {
mSyncTransaction.merge(t);
private void performDrawFinished(@Nullable Transaction t) {
if (t != null) {
mSyncTransaction.merge(t);
}
if (mPendingReportDraws > 0) {
mDrawFinished = true;
@ -1050,16 +1053,24 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
callbacks = getSurfaceCallbacks();
}
final Transaction t = new Transaction();
if (viewRoot.wasRelayoutRequested()) {
mBlastBufferQueue.setSyncTransaction(t,
false /* acquireSingleBuffer */);
final boolean wasRelayoutRequested = viewRoot.wasRelayoutRequested();
if (wasRelayoutRequested && (mBlastBufferQueue != null)) {
mBlastBufferQueue.syncNextTransaction(
false /* acquireSingleBuffer */,
this::onDrawFinished);
}
mPendingReportDraws++;
viewRoot.drawPending();
SurfaceCallbackHelper sch = new SurfaceCallbackHelper(() -> {
mBlastBufferQueue.setSyncTransaction(null);
onDrawFinished(t);
if (mBlastBufferQueue != null) {
mBlastBufferQueue.stopContinuousSyncTransaction();
}
// If relayout was requested, then a callback from BBQ will
// be invoked with the sync transaction. onDrawFinished will be
// called in there
if (!wasRelayoutRequested) {
onDrawFinished(null);
}
});
sch.dispatchSurfaceRedrawNeededAsync(mSurfaceHolder, callbacks);
}
@ -1191,7 +1202,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
mBlastBufferQueue.update(mBlastSurfaceControl, mSurfaceWidth, mSurfaceHeight, mFormat);
}
private void onDrawFinished(Transaction t) {
private void onDrawFinished(@Nullable Transaction t) {
if (DEBUG) {
Log.i(TAG, System.identityHashCode(this) + " "
+ "finishedDrawing");
@ -1824,7 +1835,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
/**
* @hide
*/
public void syncNextFrame(Transaction t) {
mBlastBufferQueue.setSyncTransaction(t);
public void syncNextFrame(Consumer<Transaction> t) {
mBlastBufferQueue.syncNextTransaction(t);
}
}

View File

@ -223,7 +223,6 @@ import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Objects;
import java.util.Queue;
@ -801,15 +800,6 @@ public final class ViewRootImpl implements ViewParent,
return mHandwritingInitiator;
}
/**
* This is only used on the RenderThread when handling a blast sync. Specifically, it's only
* used when calling {@link BLASTBufferQueue#setSyncTransaction(Transaction)} and then merged
* with a tmp transaction on the Render Thread. The tmp transaction is then merged into
* {@link #mSurfaceChangedTransaction} on the UI Thread, avoiding any threading issues.
*/
private final SurfaceControl.Transaction mRtBLASTSyncTransaction =
new SurfaceControl.Transaction();
/**
* Keeps track of the last frame number that was attempted to draw. Should only be accessed on
* the RenderThread.
@ -4181,43 +4171,49 @@ public final class ViewRootImpl implements ViewParent,
+ " didProduceBuffer=" + didProduceBuffer);
}
final Transaction tmpTransaction = new Transaction();
tmpTransaction.merge(mRtBLASTSyncTransaction);
// If frame wasn't drawn, clear out the next transaction so it doesn't affect the next
// draw attempt. The next transaction and transaction complete callback were only set
// for the current draw attempt.
final Transaction pendingTransactions;
if (!didProduceBuffer) {
mBlastBufferQueue.setSyncTransaction(null);
mBlastBufferQueue.syncNextTransaction(null);
// Get the transactions that were sent to mergeWithNextTransaction since the
// frame didn't draw on this vsync. It's possible the frame will draw later, but
// it's better to not be sync than to block on a frame that may never come.
Transaction pendingTransactions = mBlastBufferQueue.gatherPendingTransactions(
pendingTransactions = mBlastBufferQueue.gatherPendingTransactions(
mRtLastAttemptedDrawFrameNum);
tmpTransaction.merge(pendingTransactions);
if (!useBlastSync && !reportNextDraw) {
pendingTransactions.apply();
}
} else {
pendingTransactions = null;
}
if (!useBlastSync && !reportNextDraw) {
tmpTransaction.apply();
}
// Post at front of queue so the buffer can be processed immediately and allow RT
// to continue processing new buffers. If RT tries to process buffers before the sync
// buffer is applied, the new buffers will not get acquired and could result in a
// deadlock. UI thread would wait on RT, but RT would be blocked waiting for a free
// buffer.
mHandler.postAtFrontOfQueue(() -> {
if (useBlastSync) {
mSurfaceChangedTransaction.merge(tmpTransaction);
if (!didProduceBuffer && useBlastSync) {
mSurfaceChangedTransaction.merge(pendingTransactions);
if (blastSyncConsumer != null) {
blastSyncConsumer.accept(mSurfaceChangedTransaction);
}
}
tmpTransaction.close();
if (reportNextDraw) {
// This is to ensure pendingDrawFinished is only called exactly one time per draw
// attempt when reportNextDraw is true. Since, we sometimes create a sync
// transaction callback, the callback will handle calling pendingDrawFinished.
// However, there are cases where the transaction callback may not be called.
// 1. If useBlastSync is false, then we know that a sync transaction callback was
// not created so we won't invoke pendingDrawFinished there.
// 2. If the draw didn't produce a frame, didProduceBuffer == false, then we know
// the sync transaction callback will not be invoked even if one was set up.
if (reportNextDraw && (!didProduceBuffer || !useBlastSync)) {
pendingDrawFinished();
}
});
};
}
@ -4300,7 +4296,19 @@ public final class ViewRootImpl implements ViewParent,
// Frame callbacks will always occur after submitting draw requests and before
// the draw actually occurs. This will ensure that we set the next transaction
// for the frame that's about to get drawn and not on a previous frame.
mBlastBufferQueue.setSyncTransaction(mRtBLASTSyncTransaction);
mBlastBufferQueue.syncNextTransaction(
t -> {
mHandler.postAtFrontOfQueue(() -> {
mSurfaceChangedTransaction.merge(t);
if (blastSyncConsumer != null) {
blastSyncConsumer.accept(mSurfaceChangedTransaction);
}
if (reportNextDraw) {
pendingDrawFinished();
}
});
});
}
return createFrameCommitCallbackForSync(useBlastSync, reportNextDraw,
@ -10915,24 +10923,23 @@ public final class ViewRootImpl implements ViewParent,
+ frame + ".");
}
final Transaction t = new Transaction();
// If the syncResults are SYNC_LOST_SURFACE_REWARD_IF_FOUND or
// SYNC_CONTEXT_IS_STOPPED it means nothing will draw. There's no need to set up
// any blast sync or commit callback, and the code should directly call
// pendingDrawFinished.
if ((syncResult
& (SYNC_LOST_SURFACE_REWARD_IF_FOUND | SYNC_CONTEXT_IS_STOPPED)) != 0) {
t.merge(mBlastBufferQueue.gatherPendingTransactions(frame));
syncBufferCallback.onBufferReady(t);
syncBufferCallback.onBufferReady(
mBlastBufferQueue.gatherPendingTransactions(frame));
return null;
}
mBlastBufferQueue.setSyncTransaction(t);
if (DEBUG_BLAST) {
Log.d(mTag, "Setting up sync and frameCommitCallback");
}
mBlastBufferQueue.syncNextTransaction(t -> syncBufferCallback.onBufferReady(t));
return didProduceBuffer -> {
if (DEBUG_BLAST) {
Log.d(mTag, "Received frameCommittedCallback"
@ -10944,15 +10951,14 @@ public final class ViewRootImpl implements ViewParent,
// the next draw attempt. The next transaction and transaction complete callback
// were only set for the current draw attempt.
if (!didProduceBuffer) {
mBlastBufferQueue.setSyncTransaction(null);
mBlastBufferQueue.syncNextTransaction(null);
// Gather the transactions that were sent to mergeWithNextTransaction
// since the frame didn't draw on this vsync. It's possible the frame will
// draw later, but it's better to not be sync than to block on a frame that
// may never come.
t.merge(mBlastBufferQueue.gatherPendingTransactions(frame));
syncBufferCallback.onBufferReady(
mBlastBufferQueue.gatherPendingTransactions(frame));
}
syncBufferCallback.onBufferReady(t);
};
}
});

View File

@ -343,35 +343,20 @@ public class SurfaceSyncer {
@Override
public void onReadyToSync(SyncBufferCallback syncBufferCallback) {
Transaction mTransaction = sTransactionFactory.get();
mFrameCallbackConsumer.accept(new SurfaceViewFrameCallback() {
@Override
public void onFrameStarted() {
mSurfaceView.syncNextFrame(mTransaction);
}
@Override
public void onFrameComplete() {
syncBufferCallback.onBufferReady(mTransaction);
}
});
mFrameCallbackConsumer.accept(
() -> mSurfaceView.syncNextFrame(syncBufferCallback::onBufferReady));
}
}
/**
* A frame callback that is used to synchronize SurfaceViews. The owner of the SurfaceView must
* implement onFrameStarted and onFrameComplete when trying to sync the SurfaceView. This is to
* ensure the sync knows when the frame is ready to add to the sync.
* implement onFrameStarted when trying to sync the SurfaceView. This is to ensure the sync
* knows when the frame is ready to add to the sync.
*/
public interface SurfaceViewFrameCallback {
/**
* Called when the SurfaceView is going to render a frame
*/
void onFrameStarted();
/**
* Called when the SurfaceView has finished rendering a frame.
*/
void onFrameComplete();
}
}

View File

@ -35,6 +35,18 @@ static struct {
jmethodID ctor;
} gTransactionClassInfo;
struct {
jmethodID accept;
} gTransactionConsumer;
static JNIEnv* getenv(JavaVM* vm) {
JNIEnv* env;
if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK) {
LOG_ALWAYS_FATAL("Failed to get JNIEnv for JavaVM: %p", vm);
}
return env;
}
static jlong nativeCreate(JNIEnv* env, jclass clazz, jstring jName,
jboolean updateDestinationFrame) {
ScopedUtfChars name(env, jName);
@ -55,11 +67,51 @@ static jobject nativeGetSurface(JNIEnv* env, jclass clazz, jlong ptr,
queue->getSurface(includeSurfaceControlHandle));
}
static void nativeSetSyncTransaction(JNIEnv* env, jclass clazz, jlong ptr, jlong transactionPtr,
jboolean acquireSingleBuffer) {
class JGlobalRefHolder {
public:
JGlobalRefHolder(JavaVM* vm, jobject object) : mVm(vm), mObject(object) {}
virtual ~JGlobalRefHolder() {
getenv(mVm)->DeleteGlobalRef(mObject);
mObject = nullptr;
}
jobject object() { return mObject; }
JavaVM* vm() { return mVm; }
private:
JGlobalRefHolder(const JGlobalRefHolder&) = delete;
void operator=(const JGlobalRefHolder&) = delete;
JavaVM* mVm;
jobject mObject;
};
static void nativeSyncNextTransaction(JNIEnv* env, jclass clazz, jlong ptr, jobject callback,
jboolean acquireSingleBuffer) {
sp<BLASTBufferQueue> queue = reinterpret_cast<BLASTBufferQueue*>(ptr);
auto transaction = reinterpret_cast<SurfaceComposerClient::Transaction*>(transactionPtr);
queue->setSyncTransaction(transaction, acquireSingleBuffer);
JavaVM* vm = nullptr;
LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&vm) != JNI_OK, "Unable to get Java VM");
if (!callback) {
queue->syncNextTransaction(nullptr, acquireSingleBuffer);
} else {
auto globalCallbackRef =
std::make_shared<JGlobalRefHolder>(vm, env->NewGlobalRef(callback));
queue->syncNextTransaction(
[globalCallbackRef](SurfaceComposerClient::Transaction* t) {
JNIEnv* env = getenv(globalCallbackRef->vm());
env->CallVoidMethod(globalCallbackRef->object(), gTransactionConsumer.accept,
env->NewObject(gTransactionClassInfo.clazz,
gTransactionClassInfo.ctor,
reinterpret_cast<jlong>(t)));
},
acquireSingleBuffer);
}
}
static void nativeStopContinuousSyncTransaction(JNIEnv* env, jclass clazz, jlong ptr) {
sp<BLASTBufferQueue> queue = reinterpret_cast<BLASTBufferQueue*>(ptr);
queue->stopContinuousSyncTransaction();
}
static void nativeUpdate(JNIEnv* env, jclass clazz, jlong ptr, jlong surfaceControl, jlong width,
@ -104,7 +156,8 @@ static const JNINativeMethod gMethods[] = {
{"nativeCreate", "(Ljava/lang/String;Z)J", (void*)nativeCreate},
{"nativeGetSurface", "(JZ)Landroid/view/Surface;", (void*)nativeGetSurface},
{"nativeDestroy", "(J)V", (void*)nativeDestroy},
{"nativeSetSyncTransaction", "(JJZ)V", (void*)nativeSetSyncTransaction},
{"nativeSyncNextTransaction", "(JLjava/util/function/Consumer;Z)V", (void*)nativeSyncNextTransaction},
{"nativeStopContinuousSyncTransaction", "(J)V", (void*)nativeStopContinuousSyncTransaction},
{"nativeUpdate", "(JJJJI)V", (void*)nativeUpdate},
{"nativeMergeWithNextTransaction", "(JJJ)V", (void*)nativeMergeWithNextTransaction},
{"nativeGetLastAcquiredFrameNum", "(J)J", (void*)nativeGetLastAcquiredFrameNum},
@ -123,6 +176,10 @@ int register_android_graphics_BLASTBufferQueue(JNIEnv* env) {
gTransactionClassInfo.clazz = MakeGlobalRefOrDie(env, transactionClazz);
gTransactionClassInfo.ctor =
GetMethodIDOrDie(env, gTransactionClassInfo.clazz, "<init>", "(J)V");
jclass consumer = FindClassOrDie(env, "java/util/function/Consumer");
gTransactionConsumer.accept =
GetMethodIDOrDie(env, consumer, "accept", "(Ljava/lang/Object;)V");
return 0;
}

View File

@ -16,10 +16,11 @@
package android.graphics;
import android.annotation.Nullable;
import android.view.Surface;
import android.view.SurfaceControl;
import java.util.function.Consumer;
/**
* @hide
*/
@ -30,8 +31,9 @@ public final class BLASTBufferQueue {
private static native long nativeCreate(String name, boolean updateDestinationFrame);
private static native void nativeDestroy(long ptr);
private static native Surface nativeGetSurface(long ptr, boolean includeSurfaceControlHandle);
private static native void nativeSetSyncTransaction(long ptr, long transactionPtr,
boolean acquireSingleBuffer);
private static native void nativeSyncNextTransaction(long ptr,
Consumer<SurfaceControl.Transaction> callback, boolean acquireSingleBuffer);
private static native void nativeStopContinuousSyncTransaction(long ptr);
private static native void nativeUpdate(long ptr, long surfaceControl, long width, long height,
int format);
private static native void nativeMergeWithNextTransaction(long ptr, long transactionPtr,
@ -74,25 +76,39 @@ public final class BLASTBufferQueue {
}
/**
* Send the transaction to BBQ so the next frame can be added and not applied immediately. This
* gives the caller a chance to apply the transaction when it's ready.
*
* @param t The transaction to add the frame to. This can be null to clear the
* transaction.
* Send a callback that accepts a transaction to BBQ. BBQ will acquire buffers into the a
* transaction it created and will eventually send the transaction into the callback
* when it is ready.
* @param callback The callback invoked when the buffer has been added to the transaction. The
* callback will contain the transaction with the buffer.
* @param acquireSingleBuffer If true, only acquire a single buffer when processing frames. The
* transaction will be cleared once a single buffer has been
* callback will be cleared once a single buffer has been
* acquired. If false, continue to acquire all buffers into the
* transaction until setSyncTransaction is called again with a null
* transaction.
* transaction until stopContinuousSyncTransaction is called.
*/
public void setSyncTransaction(@Nullable SurfaceControl.Transaction t,
boolean acquireSingleBuffer) {
nativeSetSyncTransaction(mNativeObject, t == null ? 0 : t.mNativeObject,
acquireSingleBuffer);
public void syncNextTransaction(boolean acquireSingleBuffer,
Consumer<SurfaceControl.Transaction> callback) {
nativeSyncNextTransaction(mNativeObject, callback, acquireSingleBuffer);
}
public void setSyncTransaction(@Nullable SurfaceControl.Transaction t) {
setSyncTransaction(t, true /* acquireSingleBuffer */);
/**
* Send a callback that accepts a transaction to BBQ. BBQ will acquire buffers into the a
* transaction it created and will eventually send the transaction into the callback
* when it is ready.
* @param callback The callback invoked when the buffer has been added to the transaction. The
* callback will contain the transaction with the buffer.
*/
public void syncNextTransaction(Consumer<SurfaceControl.Transaction> callback) {
syncNextTransaction(true /* acquireSingleBuffer */, callback);
}
/**
* Tell BBQ to stop acquiring buffers into a single transaction. BBQ will send the sync
* transaction callback after this has been called. This should only be used when
* syncNextTransaction was called with acquireSingleBuffer set to false.
*/
public void stopContinuousSyncTransaction() {
nativeStopContinuousSyncTransaction(mNativeObject);
}
/**

View File

@ -179,11 +179,6 @@ public class SurfaceSyncerValidatorTestCase implements ISurfaceValidatorTestCase
mSurfaceHolder.unlockCanvasAndPost(c);
}
if (mFrameCallback != null) {
Log.d(TAG, "onFrameComplete");
mFrameCallback.onFrameComplete();
}
mFrameCallback = null;
}
}

View File

@ -19,6 +19,9 @@ package com.android.test;
import android.annotation.NonNull;
import android.app.Activity;
import android.graphics.Canvas;
import android.graphics.Color;
import android.graphics.Paint;
import android.graphics.Point;
import android.graphics.Rect;
import android.os.Bundle;
import android.os.Handler;
@ -51,6 +54,8 @@ public class SurfaceViewSyncActivity extends Activity implements SurfaceHolder.C
private Button mExpandButton;
private Switch mEnableSyncSwitch;
private int mLastSyncId = -1;
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
@ -71,6 +76,10 @@ public class SurfaceViewSyncActivity extends Activity implements SurfaceHolder.C
}
private void updateSurfaceViewSize(Rect bounds, View container) {
if (mLastSyncId >= 0) {
return;
}
final float height;
if (mLastExpanded) {
height = bounds.height() / 2f;
@ -82,13 +91,8 @@ public class SurfaceViewSyncActivity extends Activity implements SurfaceHolder.C
mLastExpanded = !mLastExpanded;
if (mEnableSyncSwitch.isChecked()) {
int syncId = mSurfaceSyncer.setupSync(() -> { });
mSurfaceSyncer.addToSync(syncId, mSurfaceView, frameCallback ->
mRenderingThread.setFrameCallback(frameCallback));
mSurfaceSyncer.addToSync(syncId, container);
mSurfaceSyncer.markSyncReady(syncId);
} else {
mRenderingThread.renderSlow();
mLastSyncId = mSurfaceSyncer.setupSync(() -> { });
mSurfaceSyncer.addToSync(mLastSyncId, container);
}
ViewGroup.LayoutParams svParams = mSurfaceView.getLayoutParams();
@ -99,13 +103,26 @@ public class SurfaceViewSyncActivity extends Activity implements SurfaceHolder.C
@Override
public void surfaceCreated(@NonNull SurfaceHolder holder) {
final Canvas canvas = holder.lockCanvas();
canvas.drawARGB(255, 100, 100, 100);
canvas.drawARGB(255, 255, 0, 0);
holder.unlockCanvasAndPost(canvas);
mRenderingThread.startRendering();
mRenderingThread.renderFrame(null, mSurfaceView.getWidth(), mSurfaceView.getHeight());
}
@Override
public void surfaceChanged(@NonNull SurfaceHolder holder, int format, int width, int height) {
if (mEnableSyncSwitch.isChecked()) {
if (mLastSyncId < 0) {
mRenderingThread.renderFrame(null, width, height);
return;
}
mSurfaceSyncer.addToSync(mLastSyncId, mSurfaceView, frameCallback ->
mRenderingThread.renderFrame(frameCallback, width, height));
mSurfaceSyncer.markSyncReady(mLastSyncId);
mLastSyncId = -1;
} else {
mRenderingThread.renderFrame(null, width, height);
}
}
@Override
@ -117,31 +134,30 @@ public class SurfaceViewSyncActivity extends Activity implements SurfaceHolder.C
private final SurfaceHolder mSurfaceHolder;
private Handler mHandler;
private SurfaceSyncer.SurfaceViewFrameCallback mFrameCallback;
private boolean mRenderSlow;
private final Point mSurfaceSize = new Point();
int mColorValue = 0;
int mColorDelta = 10;
private final Paint mPaint = new Paint();
RenderingThread(SurfaceHolder holder) {
super("RenderingThread");
mSurfaceHolder = holder;
mPaint.setColor(Color.BLACK);
mPaint.setTextSize(100);
}
public void setFrameCallback(SurfaceSyncer.SurfaceViewFrameCallback frameCallback) {
public void renderFrame(SurfaceSyncer.SurfaceViewFrameCallback frameCallback, int width,
int height) {
if (mHandler != null) {
mHandler.post(() -> {
mFrameCallback = frameCallback;
mRenderSlow = true;
mSurfaceSize.set(width, height);
mRunnable.run();
});
}
}
public void renderSlow() {
if (mHandler != null) {
mHandler.post(() -> mRenderSlow = true);
}
}
private final Runnable mRunnable = new Runnable() {
@Override
public void run() {
@ -149,13 +165,10 @@ public class SurfaceViewSyncActivity extends Activity implements SurfaceHolder.C
mFrameCallback.onFrameStarted();
}
if (mRenderSlow) {
try {
// Long delay from start to finish to mimic slow draw
Thread.sleep(1000);
} catch (InterruptedException e) {
}
mRenderSlow = false;
try {
// Long delay from start to finish to mimic slow draw
Thread.sleep(1000);
} catch (InterruptedException e) {
}
mColorValue += mColorDelta;
@ -164,27 +177,21 @@ public class SurfaceViewSyncActivity extends Activity implements SurfaceHolder.C
}
Canvas c = mSurfaceHolder.lockCanvas();
c.drawRGB(255, mColorValue, 255 - mColorValue);
c.drawRGB(255, 0, 0);
c.drawText("RENDERED CONTENT", 0, mSurfaceSize.y / 2, mPaint);
mSurfaceHolder.unlockCanvasAndPost(c);
if (mFrameCallback != null) {
mFrameCallback.onFrameComplete();
}
mFrameCallback = null;
mHandler.postDelayed(this, 50);
}
};
public void startRendering() {
start();
mHandler = new Handler(getLooper());
mHandler.post(mRunnable);
}
public void stopRendering() {
if (mHandler != null) {
mHandler.post(() -> mHandler.removeCallbacks(mRunnable));
mHandler.removeCallbacks(mRunnable);
}
}
}