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

Commit c027ac61 authored by Chavi Weingarten's avatar Chavi Weingarten Committed by Automerger Merge Worker
Browse files

Merge "Account for release callbacks when determing if sync is finished" into... 

Merge "Account for release callbacks when determing if sync is finished" into tm-qpr-dev am: 4b383323 am: 23a7c8a1

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



Change-Id: I3f59ae397ae474db2b9a5b316c9e020bea6f50df
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 1695bf33 23a7c8a1

libs/gui/BLASTBufferQueue.cpp

+24 −18
Original line number Diff line number Diff line
@@ -288,18 +288,17 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/, 


                // We need to check if we were waiting for a transaction callback in order to

                // process any pending buffers and unblock. It's possible to get transaction

                // callbacks for previous requests so we need to ensure the frame from this

                // transaction callback matches the last acquired buffer. Since acquireNextBuffer

                // will stop processing buffers when mWaitForTransactionCallback is set, we know

                // that mLastAcquiredFrameNumber is the frame we're waiting on.

                // We also want to check if mNextTransaction is null because it's possible another

                // callbacks for previous requests so we need to ensure that there are no pending

                // frame numbers that were in a sync. We remove the frame from mSyncedFrameNumbers

                // set and then check if it's empty. If there are no more pending syncs, we can

                // proceed with flushing the shadow queue.

                // We also want to check if mSyncTransaction is null because it's possible another

                // sync request came in while waiting, but it hasn't started processing yet. In that

                // case, we don't actually want to flush the frames in between since they will get

                // processed and merged with the sync transaction and released earlier than if they

                // were sent to SF

                if (mWaitForTransactionCallback && mSyncTransaction == nullptr &&

                    currFrameNumber >= mLastAcquiredFrameNumber) {

                    mWaitForTransactionCallback = false;

                mSyncedFrameNumbers.erase(currFrameNumber);

                if (mSyncedFrameNumbers.empty() && mSyncTransaction == nullptr) {

                    flushShadowQueue();

                }

            } else {
@@ -417,9 +416,11 @@ void BLASTBufferQueue::releaseBufferCallback( 
        const auto releasedBuffer = mPendingRelease.front();

        mPendingRelease.pop_front();

        releaseBuffer(releasedBuffer.callbackId, releasedBuffer.releaseFence);

        // Don't process the transactions here if mWaitForTransactionCallback is set. Instead, let

        // onFrameAvailable handle processing them since it will merge with the syncTransaction.

        if (!mWaitForTransactionCallback) {

        // Don't process the transactions here if mSyncedFrameNumbers is not empty. That means

        // are still transactions that have sync buffers in them that have not been applied or

        // dropped. Instead, let onFrameAvailable handle processing them since it will merge with

        // the syncTransaction.

        if (mSyncedFrameNumbers.empty()) {

            acquireNextBufferLocked(std::nullopt);

        }

    }
@@ -443,6 +444,9 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, 
    BQA_LOGV("released %s", callbackId.to_string().c_str());

    mBufferItemConsumer->releaseBuffer(it->second, releaseFence);

    mSubmitted.erase(it);

    // Remove the frame number from mSyncedFrameNumbers since we can get a release callback

    // without getting a transaction committed if the buffer was dropped.

    mSyncedFrameNumbers.erase(callbackId.framenumber);

}



void BLASTBufferQueue::acquireNextBufferLocked(
@@ -609,7 +613,7 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() { 
}



void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& lock) {

    if (mWaitForTransactionCallback && mNumFrameAvailable > 0) {

    if (!mSyncedFrameNumbers.empty() && mNumFrameAvailable > 0) {

        // We are waiting on a previous sync's transaction callback so allow another sync

        // transaction to proceed.

        //
@@ -636,6 +640,8 @@ void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& l 
void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {

    std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr;

    SurfaceComposerClient::Transaction* prevTransaction = nullptr;

    bool waitForTransactionCallback = !mSyncedFrameNumbers.empty();



    {

        BBQ_TRACE();

        std::unique_lock _lock{mMutex};
@@ -667,7 +673,7 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { 


        // add to shadow queue

        mNumFrameAvailable++;

        if (mWaitForTransactionCallback && mNumFrameAvailable >= 2) {

        if (waitForTransactionCallback && mNumFrameAvailable >= 2) {

            acquireAndReleaseBuffer();

        }

        ATRACE_INT(mQueuedBufferTrace.c_str(),
@@ -684,14 +690,14 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { 
            incStrong((void*)transactionCommittedCallbackThunk);

            mSyncTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk,

                                                              static_cast<void*>(this));

            mWaitForTransactionCallback = true;

            mSyncedFrameNumbers.emplace(item.mFrameNumber);

            if (mAcquireSingleBuffer) {

                prevCallback = mTransactionReadyCallback;

                prevTransaction = mSyncTransaction;

                mTransactionReadyCallback = nullptr;

                mSyncTransaction = nullptr;

            }

        } else if (!mWaitForTransactionCallback) {

        } else if (!waitForTransactionCallback) {

            acquireNextBufferLocked(std::nullopt);

        }

    }
@@ -1098,9 +1104,9 @@ void BLASTBufferQueue::abandon() { 
    }



    // Clear sync states

    if (mWaitForTransactionCallback) {

        BQA_LOGD("mWaitForTransactionCallback cleared");

        mWaitForTransactionCallback = false;

    if (!mSyncedFrameNumbers.empty()) {

        BQA_LOGD("mSyncedFrameNumbers cleared");

        mSyncedFrameNumbers.clear();

    }



    if (mSyncTransaction != nullptr) {

libs/gui/include/gui/BLASTBufferQueue.h

+2 −1
Original line number Diff line number Diff line
@@ -251,7 +251,6 @@ private: 
    std::queue<sp<SurfaceControl>> mSurfaceControlsWithPendingCallback GUARDED_BY(mMutex);



    uint32_t mCurrentMaxAcquiredBufferCount;

    bool mWaitForTransactionCallback GUARDED_BY(mMutex) = false;



    // Flag to determine if syncTransaction should only acquire a single buffer and then clear or

    // continue to acquire buffers until explicitly cleared
@@ -279,6 +278,8 @@ private: 
    uint64_t mLastAppliedFrameNumber = 0;



    std::function<void(bool)> mTransactionHangCallback;



    std::unordered_set<uint64_t> mSyncedFrameNumbers GUARDED_BY(mMutex);

};



} // namespace android

libs/gui/tests/BLASTBufferQueue_test.cpp

+37 −0
Original line number Diff line number Diff line
@@ -162,6 +162,10 @@ public: 
        ASSERT_EQ(numFramesSubmitted, mBlastBufferQueueAdapter->mSubmitted.size());

    }



    void mergeWithNextTransaction(Transaction* merge, uint64_t frameNumber) {

        mBlastBufferQueueAdapter->mergeWithNextTransaction(merge, frameNumber);

    }



private:

    sp<TestBLASTBufferQueue> mBlastBufferQueueAdapter;

};
@@ -1113,6 +1117,39 @@ TEST_F(BLASTBufferQueueTest, SyncNextTransactionOverwrite) { 
    ASSERT_TRUE(receivedCallback);

}



TEST_F(BLASTBufferQueueTest, SyncNextTransactionDropBuffer) {

    uint8_t r = 255;

    uint8_t g = 0;

    uint8_t b = 0;



    BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);



    sp<IGraphicBufferProducer> igbProducer;

    setUpProducer(adapter, igbProducer);



    Transaction sync;

    adapter.setSyncTransaction(sync);

    queueBuffer(igbProducer, 0, 255, 0, 0);



    // Merge a transaction that has a complete callback into the next frame so we can get notified

    // when to take a screenshot

    CallbackHelper transactionCallback;

    Transaction t;

    t.addTransactionCompletedCallback(transactionCallback.function,

                                      transactionCallback.getContext());

    adapter.mergeWithNextTransaction(&t, 2);

    queueBuffer(igbProducer, r, g, b, 0);



    // Drop the buffer, but ensure the next one continues to get processed.

    sync.setBuffer(mSurfaceControl, nullptr);



    CallbackData callbackData;

    transactionCallback.getCallbackData(&callbackData);

    ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults));

    ASSERT_NO_FATAL_FAILURE(

            checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight}));

}



// This test will currently fail because the old surfacecontrol will steal the last presented buffer

// until the old surface control is destroyed. This is not necessarily a bug but to document a

// limitation with the update API and to test any changes to make the api more robust. The current