Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 351e9cf9 authored by chaviw's avatar chaviw
Browse files

Handle multiple relayout calls with blast sync

The issue is the following:
1. Call relayout from VRI. drawsNeededToReport = 1
2. SV also will need to be updated. drawsNeedToReport = 2
3. VRI is finished rendering, but SV is not. drawsNeedToReport = 1
4. VRI calls relayout again. drawsNeedToReport = 2
5. SV finishes drawing. drawsNeedToReport = 1

BBQ will get stuck since it's waiting indefinitey for a transaction
callback. The previous transaction can never get applied since we're
still waiting until drawsNeededToReport = 0.

This prevents situations like this from happening since it will not
allow a relayout to get called until the previous transaction has been
applied.

Test: Split screen with chrome
Bug: 167202096
Change-Id: I167a90ef00fa6677b3c55239e74eefb6a1384baf
parent 2b5f2d80
Loading
Loading
Loading
Loading
+92 −16
Original line number Diff line number Diff line
@@ -239,6 +239,7 @@ public final class ViewRootImpl implements ViewParent,
    private static final boolean DEBUG_KEEP_SCREEN_ON = false || LOCAL_LOGV;
    private static final boolean DEBUG_CONTENT_CAPTURE = false || LOCAL_LOGV;
    private static final boolean DEBUG_SCROLL_CAPTURE = false || LOCAL_LOGV;
    private static final boolean DEBUG_BLAST = false || LOCAL_LOGV;

    /**
     * Set to false if we do not want to use the multi threaded renderer even though
@@ -722,6 +723,11 @@ public final class ViewRootImpl implements ViewParent,
    // (e.g. SurfaceView)
    private boolean mSendNextFrameToWm = false;

    // Keeps track of whether a traverse was triggered while the UI thread was paused. This can
    // occur when the client is waiting on another process to submit the transaction that contains
    // the buffer. The UI thread needs to wait on the callback before it can submit another buffer.
    private boolean mRequestedTraverseWhilePaused = false;

    private HashSet<ScrollCaptureCallback> mRootScrollCaptureCallbacks;

    /**
@@ -1527,7 +1533,7 @@ public final class ViewRootImpl implements ViewParent,
        mForceNextWindowRelayout = forceNextWindowRelayout;
        mPendingAlwaysConsumeSystemBars = args.argi2 != 0;

        if (msg == MSG_RESIZED_REPORT) {
        if (msg == MSG_RESIZED_REPORT && !mSendNextFrameToWm) {
            reportNextDraw();
        }

@@ -2405,8 +2411,26 @@ public final class ViewRootImpl implements ViewParent,
            host.debug();
        }

        if (host == null || !mAdded)
        if (host == null || !mAdded) {
            return;
        }

        // This is to ensure we don't end up queueing new frames while waiting on a previous frame
        // to get latched. This can happen when there's been a sync request for this window. The
        // frame could be in a transaction that's passed to different processes to ensure
        // synchronization. It continues to block until ViewRootImpl receives a callback that the
        // transaction containing the buffer has been sent to SurfaceFlinger. Once we receive, that
        // signal, we know it's safe to start queuing new buffers.
        //
        // When the callback is invoked, it will trigger a traversal request if
        // mRequestedTraverseWhilePaused is set so there's no need to attempt a retry here.
        if (mSendNextFrameToWm) {
            if (DEBUG_BLAST) {
                Log.w(mTag, "Can't perform draw while waiting for a transaction complete");
            }
            mRequestedTraverseWhilePaused = true;
            return;
        }

        mIsInTraversal = true;
        mWillDrawSoon = true;
@@ -3148,6 +3172,9 @@ public final class ViewRootImpl implements ViewParent,
            reportNextDraw();
        }
        if ((relayoutResult & WindowManagerGlobal.RELAYOUT_RES_BLAST_SYNC) != 0) {
            if (DEBUG_BLAST) {
                Log.d(mTag, "Relayout called with blastSync");
            }
            reportNextDraw();
            setUseBLASTSyncTransaction();
            mSendNextFrameToWm = true;
@@ -3813,6 +3840,9 @@ public final class ViewRootImpl implements ViewParent,
        mDrawsNeededToReport--;
        if (mDrawsNeededToReport == 0) {
            reportDrawFinished();
        } else if (DEBUG_BLAST) {
            Log.d(mTag, "pendingDrawFinished. Waiting on draw reported mDrawsNeededToReport="
                    + mDrawsNeededToReport);
        }
    }

@@ -3822,6 +3852,9 @@ public final class ViewRootImpl implements ViewParent,

    private void reportDrawFinished() {
        try {
            if (DEBUG_BLAST) {
                Log.d(mTag, "reportDrawFinished");
            }
            mDrawsNeededToReport = 0;
            mWindowSession.finishDrawing(mWindow, mSurfaceChangedTransaction);
        } catch (RemoteException e) {
@@ -3837,6 +3870,9 @@ public final class ViewRootImpl implements ViewParent,
        return frameNr -> {
            mBlurRegionAggregator.dispatchBlurTransactionIfNeeded(frameNr);

            if (DEBUG_BLAST) {
                Log.d(mTag, "Received frameCompleteCallback frameNum=" + frameNr);
            }
            // Use a new transaction here since mRtBLASTSyncTransaction can only be accessed by
            // the render thread and mSurfaceChangedTransaction can only be accessed by the UI
            // thread. The temporary transaction is used so mRtBLASTSyncTransaction can be merged
@@ -3870,6 +3906,13 @@ public final class ViewRootImpl implements ViewParent,
                        || (commitCallbacks != null && commitCallbacks.size() > 0)
                        || mBlurRegionAggregator.hasRegions();
        if (needFrameCompleteCallback) {
            if (DEBUG_BLAST) {
                Log.d(mTag, "Creating frameCompleteCallback"
                        + " mNextDrawUseBLASTSyncTransaction=" + mNextDrawUseBLASTSyncTransaction
                        + " mReportNextDraw=" + mReportNextDraw
                        + " commitCallbacks size="
                        + (commitCallbacks == null ? 0 : commitCallbacks.size()));
            }
            mAttachInfo.mThreadedRenderer.setFrameCompleteCallback(
                    createFrameCompleteCallback(mAttachInfo.mHandler, mReportNextDraw,
                            commitCallbacks));
@@ -3881,18 +3924,53 @@ public final class ViewRootImpl implements ViewParent,
    /**
     * The callback will run on a worker thread pool from the render thread.
     */
    private HardwareRenderer.FrameDrawingCallback createFrameDrawingCallback() {
    private HardwareRenderer.FrameDrawingCallback createFrameDrawingCallback(
            boolean addTransactionComplete) {
        return frame -> {
            if (DEBUG_BLAST) {
                Log.d(mTag, "Received frameDrawingCallback frameNum=" + frame + "."
                        + " Creating transactionCompleteCallback=" + addTransactionComplete);
            }
            mRtNextFrameReportedConsumeWithBlast = true;
            if (mBlastBufferQueue != null) {
            if (mBlastBufferQueue == null) {
                return;
            }

            // We don't need to synchronize mRtBLASTSyncTransaction here since it's not
            // being modified and only sent to BlastBufferQueue.
            mBlastBufferQueue.setNextTransaction(mRtBLASTSyncTransaction);
            if (!addTransactionComplete) {
                return;
            }

            mBlastBufferQueue.setTransactionCompleteCallback(frame, frameNumber -> {
                if (DEBUG_BLAST) {
                    Log.d(mTag, "Received transactionCompleteCallback frameNum=" + frame);
                }
                mHandler.postAtFrontOfQueue(() -> {
                    mSendNextFrameToWm = false;
                    if (DEBUG_BLAST) {
                        Log.d(mTag, "Scheduling a traversal=" + mRequestedTraverseWhilePaused
                                + " due to a previous skipped traversal.");
                    }
                    if (mRequestedTraverseWhilePaused) {
                        mRequestedTraverseWhilePaused = false;
                        scheduleTraversals();
                    }
                });
            });
        };
    }

    private void addFrameCallbackIfNeeded() {
        if (DEBUG_BLAST) {
            if (mNextDrawUseBLASTSyncTransaction || mReportNextDraw) {
                Log.d(mTag, "Creating frameDrawingCallback mNextDrawUseBLASTSyncTransaction="
                        + mNextDrawUseBLASTSyncTransaction + " mReportNextDraw=" + mReportNextDraw);
            }
        }

        if (mNextDrawUseBLASTSyncTransaction) {
            // 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 that.
@@ -3900,8 +3978,7 @@ public final class ViewRootImpl implements ViewParent,
            // This is thread safe since mRtNextFrameReportConsumeWithBlast will only be
            // modified in onFrameDraw and then again in onFrameComplete. This is to ensure the
            // next frame completed should be reported with the blast sync transaction.
        if (mNextDrawUseBLASTSyncTransaction) {
            registerRtFrameCallback(createFrameDrawingCallback());
            registerRtFrameCallback(createFrameDrawingCallback(mSendNextFrameToWm));
            mNextDrawUseBLASTSyncTransaction = false;
        } else if (mReportNextDraw) {
            registerRtFrameCallback(frame -> {
@@ -9956,7 +10033,6 @@ public final class ViewRootImpl implements ViewParent,
    private void finishBLASTSyncOnRT(boolean apply, Transaction t) {
        // This is safe to modify on the render thread since the only other place it's modified
        // is on the UI thread when the render thread is paused.
        mSendNextFrameToWm = false;
        if (mRtNextFrameReportedConsumeWithBlast) {
            mRtNextFrameReportedConsumeWithBlast = false;

+64 −1
Original line number Diff line number Diff line
@@ -26,9 +26,47 @@
#include <gui/BLASTBufferQueue.h>
#include <gui/Surface.h>
#include <gui/SurfaceComposerClient.h>
#include "core_jni_helpers.h"

namespace android {

struct {
    jmethodID onTransactionComplete;
} gTransactionCompleteCallback;

class TransactionCompleteCallbackWrapper : public LightRefBase<TransactionCompleteCallbackWrapper> {
public:
    explicit TransactionCompleteCallbackWrapper(JNIEnv* env, jobject jobject) {
        env->GetJavaVM(&mVm);
        mTransactionCompleteObject = env->NewGlobalRef(jobject);
        LOG_ALWAYS_FATAL_IF(!mTransactionCompleteObject, "Failed to make global ref");
    }

    ~TransactionCompleteCallbackWrapper() {
        if (mTransactionCompleteObject) {
            getenv()->DeleteGlobalRef(mTransactionCompleteObject);
            mTransactionCompleteObject = nullptr;
        }
    }

    void onTransactionComplete(int64_t frameNr) {
        if (mTransactionCompleteObject) {
            getenv()->CallVoidMethod(mTransactionCompleteObject,
                                     gTransactionCompleteCallback.onTransactionComplete, frameNr);
        }
    }

private:
    JavaVM* mVm;
    jobject mTransactionCompleteObject;

    JNIEnv* getenv() {
        JNIEnv* env;
        mVm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
        return env;
    }
};

static jlong nativeCreate(JNIEnv* env, jclass clazz, jstring jName, jlong surfaceControl,
                          jlong width, jlong height, jboolean enableTripleBuffering) {
    String8 str8;
@@ -76,20 +114,45 @@ static void nativeFlushShadowQueue(JNIEnv* env, jclass clazz, jlong ptr) {
    queue->flushShadowQueue();
}

static void nativeSetTransactionCompleteCallback(JNIEnv* env, jclass clazz, jlong ptr,
                                                 jlong frameNumber,
                                                 jobject transactionCompleteCallback) {
    sp<BLASTBufferQueue> queue = reinterpret_cast<BLASTBufferQueue*>(ptr);
    if (transactionCompleteCallback == nullptr) {
        queue->setTransactionCompleteCallback(frameNumber, nullptr);
    } else {
        sp<TransactionCompleteCallbackWrapper> wrapper =
                new TransactionCompleteCallbackWrapper{env, transactionCompleteCallback};
        queue->setTransactionCompleteCallback(frameNumber, [wrapper](int64_t frameNr) {
            wrapper->onTransactionComplete(frameNr);
        });
    }
}

static const JNINativeMethod gMethods[] = {
        /* name, signature, funcPtr */
        // clang-format off
        {"nativeCreate", "(Ljava/lang/String;JJJZ)J", (void*)nativeCreate},
        {"nativeGetSurface", "(JZ)Landroid/view/Surface;", (void*)nativeGetSurface},
        {"nativeDestroy", "(J)V", (void*)nativeDestroy},
        {"nativeSetNextTransaction", "(JJ)V", (void*)nativeSetNextTransaction},
        {"nativeUpdate", "(JJJJ)V", (void*)nativeUpdate},
        {"nativeFlushShadowQueue", "(J)V", (void*)nativeFlushShadowQueue}};
        {"nativeFlushShadowQueue", "(J)V", (void*)nativeFlushShadowQueue},
        {"nativeSetTransactionCompleteCallback",
                "(JJLandroid/graphics/BLASTBufferQueue$TransactionCompleteCallback;)V",
                (void*)nativeSetTransactionCompleteCallback}
        // clang-format on
};

int register_android_graphics_BLASTBufferQueue(JNIEnv* env) {
    int res = jniRegisterNativeMethods(env, "android/graphics/BLASTBufferQueue",
            gMethods, NELEM(gMethods));
    LOG_ALWAYS_FATAL_IF(res < 0, "Unable to register native methods.");

    jclass transactionCompleteClass =
            FindClassOrDie(env, "android/graphics/BLASTBufferQueue$TransactionCompleteCallback");
    gTransactionCompleteCallback.onTransactionComplete =
            GetMethodIDOrDie(env, transactionCompleteClass, "onTransactionComplete", "(J)V");
    return 0;
}

+23 −0
Original line number Diff line number Diff line
@@ -34,6 +34,19 @@ public final class BLASTBufferQueue {
    private static native void nativeSetNextTransaction(long ptr, long transactionPtr);
    private static native void nativeUpdate(long ptr, long surfaceControl, long width, long height);
    private static native void nativeFlushShadowQueue(long ptr);
    private static native void nativeSetTransactionCompleteCallback(long ptr, long frameNumber,
            TransactionCompleteCallback callback);

    /**
     * Callback sent to {@link #setTransactionCompleteCallback(long, TransactionCompleteCallback)}
     */
    public interface TransactionCompleteCallback {
        /**
         * Invoked when the transaction has completed.
         * @param frameNumber The frame number of the buffer that was in that transaction
         */
        void onTransactionComplete(long frameNumber);
    }

    /** Create a new connection with the surface flinger. */
    public BLASTBufferQueue(String name, SurfaceControl sc, int width, int height,
@@ -74,6 +87,16 @@ public final class BLASTBufferQueue {
        nativeUpdate(mNativeObject, sc.mNativeObject, width, height);
    }

    /**
     * Set a callback when the transaction with the frame number has been completed.
     * @param frameNumber The frame number to get the transaction complete callback for
     * @param completeCallback The callback that should be invoked.
     */
    public void setTransactionCompleteCallback(long frameNumber,
            @Nullable TransactionCompleteCallback completeCallback) {
        nativeSetTransactionCompleteCallback(mNativeObject, frameNumber, completeCallback);
    }

    @Override
    protected void finalize() throws Throwable {
        try {
+2 −2
Original line number Diff line number Diff line
@@ -140,7 +140,6 @@ import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.RequiresPermission;
import android.app.ActivityManager;
import android.window.TaskSnapshot;
import android.app.ActivityManagerInternal;
import android.app.ActivityTaskManager;
import android.app.ActivityThread;
@@ -267,6 +266,7 @@ import android.view.WindowManager.RemoveContentMode;
import android.view.WindowManagerGlobal;
import android.view.WindowManagerPolicyConstants.PointerEventListener;
import android.window.ClientWindowFrames;
import android.window.TaskSnapshot;

import com.android.internal.R;
import com.android.internal.annotations.VisibleForTesting;
@@ -2202,7 +2202,7 @@ public class WindowManagerService extends IWindowManager.Stub
                win.mPendingPositionChanged = null;
            }

            if (mUseBLASTSync && win.useBLASTSync()) {
            if (mUseBLASTSync && win.useBLASTSync() && viewVisibility != View.GONE) {
                result |= RELAYOUT_RES_BLAST_SYNC;
            }