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

Commit 4861b10c authored by Tianhao Yao's avatar Tianhao Yao Committed by chaviw
Browse files

Changed setSyncTransaction to syncNextTransaction with callback logic. 

Replaced setSyncTransation with syncNextTransaciton in BBQ. BBQ will
no longer take in transaction pointers but create transactions inside
of itself and invoke the callback when the transaction is ready.
Also added stopContinuousSyncTransction method to inform BBQ to cease
acquiring buffers into the transaction.

Test: go/wm-smoke.
Test: BLASTBufferQueueTest
Bug: 210714235.
Change-Id: I7103a95046d0251e5bdaec21b2741ff194f8e81e
parent 37ac7569

libs/gui/BLASTBufferQueue.cpp

+87 −52
Original line number Diff line number Diff line
@@ -137,6 +137,7 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati 
        mSize(1, 1),

        mRequestedSize(mSize),

        mFormat(PIXEL_FORMAT_RGBA_8888),

        mTransactionReadyCallback(nullptr),

        mSyncTransaction(nullptr),

        mUpdateDestinationFrame(updateDestinationFrame) {

    createBufferQueue(&mProducer, &mConsumer);
@@ -608,23 +609,26 @@ 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;

    {

        BBQ_TRACE();

        std::unique_lock _lock{mMutex};



    const bool syncTransactionSet = mSyncTransaction != nullptr;

        const bool syncTransactionSet = mTransactionReadyCallback != nullptr;

        BQA_LOGV("onFrameAvailable-start syncTransactionSet=%s", boolToString(syncTransactionSet));



        if (syncTransactionSet) {

            bool mayNeedToWaitForBuffer = true;

        // If we are going to re-use the same mSyncTransaction, release the buffer that may already

        // be set in the Transaction. This is to allow us a free slot early to continue processing

        // a new buffer.

            // If we are going to re-use the same mSyncTransaction, release the buffer that may

            // already be set in the Transaction. This is to allow us a free slot early to continue

            // processing a new buffer.

            if (!mAcquireSingleBuffer) {

                auto bufferData = mSyncTransaction->getAndClearBuffer(mSurfaceControl);

                if (bufferData) {

                    BQA_LOGD("Releasing previous buffer when syncing: framenumber=%" PRIu64,

                             bufferData->frameNumber);

                releaseBuffer(bufferData->generateReleaseCallbackId(), bufferData->acquireFence);

                    releaseBuffer(bufferData->generateReleaseCallbackId(),

                                  bufferData->acquireFence);

                    // Because we just released a buffer, we know there's no need to wait for a free

                    // buffer.

                    mayNeedToWaitForBuffer = false;
@@ -644,26 +648,32 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { 
        ATRACE_INT(mQueuedBufferTrace.c_str(),

                   mNumFrameAvailable + mNumAcquired - mPendingRelease.size());



    BQA_LOGV("onFrameAvailable framenumber=%" PRIu64 " syncTransactionSet=%s", item.mFrameNumber,

             boolToString(syncTransactionSet));

        BQA_LOGV("onFrameAvailable framenumber=%" PRIu64 " syncTransactionSet=%s",

                 item.mFrameNumber, boolToString(syncTransactionSet));



        if (syncTransactionSet) {

            acquireNextBufferLocked(mSyncTransaction);



        // Only need a commit callback when syncing to ensure the buffer that's synced has been sent

        // to SF

            // Only need a commit callback when syncing to ensure the buffer that's synced has been

            // sent to SF

            incStrong((void*)transactionCommittedCallbackThunk);

            mSyncTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk,

                                                              static_cast<void*>(this));



            mWaitForTransactionCallback = true;

            if (mAcquireSingleBuffer) {

                prevCallback = mTransactionReadyCallback;

                prevTransaction = mSyncTransaction;

                mTransactionReadyCallback = nullptr;

                mSyncTransaction = nullptr;

            }

        mWaitForTransactionCallback = true;

        } else if (!mWaitForTransactionCallback) {

            acquireNextBufferLocked(std::nullopt);

        }

    }

    if (prevCallback) {

        prevCallback(prevTransaction);

    }

}



void BLASTBufferQueue::onFrameReplaced(const BufferItem& item) {

    BQA_LOGV("onFrameReplaced framenumber=%" PRIu64, item.mFrameNumber);
@@ -680,12 +690,37 @@ void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) { 
    mDequeueTimestamps.erase(bufferId);

};



void BLASTBufferQueue::setSyncTransaction(SurfaceComposerClient::Transaction* t,

void BLASTBufferQueue::syncNextTransaction(

        std::function<void(SurfaceComposerClient::Transaction*)> callback,

        bool acquireSingleBuffer) {

    BBQ_TRACE();

    std::lock_guard _lock{mMutex};

    mSyncTransaction = t;

    mAcquireSingleBuffer = mSyncTransaction ? acquireSingleBuffer : true;

    mTransactionReadyCallback = callback;

    if (callback) {

        mSyncTransaction = new SurfaceComposerClient::Transaction();

    } else {

        mSyncTransaction = nullptr;

    }

    mAcquireSingleBuffer = mTransactionReadyCallback ? acquireSingleBuffer : true;

}



void BLASTBufferQueue::stopContinuousSyncTransaction() {

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

    SurfaceComposerClient::Transaction* prevTransaction = nullptr;

    {

        std::lock_guard _lock{mMutex};

        bool invokeCallback = mTransactionReadyCallback && !mAcquireSingleBuffer;

        if (invokeCallback) {

            prevCallback = mTransactionReadyCallback;

            prevTransaction = mSyncTransaction;

        }

        mTransactionReadyCallback = nullptr;

        mSyncTransaction = nullptr;

        mAcquireSingleBuffer = true;

    }

    if (prevCallback) {

        prevCallback(prevTransaction);

    }

}



bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) {

libs/gui/include/gui/BLASTBufferQueue.h

+5 −1
Original line number Diff line number Diff line
@@ -95,7 +95,9 @@ public: 
                                     const std::vector<SurfaceControlStats>& stats);

    void releaseBufferCallback(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,

                               std::optional<uint32_t> currentMaxAcquiredBufferCount);

    void setSyncTransaction(SurfaceComposerClient::Transaction* t, bool acquireSingleBuffer = true);

    void syncNextTransaction(std::function<void(SurfaceComposerClient::Transaction*)> callback,

                             bool acquireSingleBuffer = true);

    void stopContinuousSyncTransaction();

    void mergeWithNextTransaction(SurfaceComposerClient::Transaction* t, uint64_t frameNumber);

    void applyPendingTransactions(uint64_t frameNumber);

    SurfaceComposerClient::Transaction* gatherPendingTransactions(uint64_t frameNumber);
@@ -213,6 +215,8 @@ private: 
    sp<IGraphicBufferProducer> mProducer;

    sp<BLASTBufferItemConsumer> mBufferItemConsumer;



    std::function<void(SurfaceComposerClient::Transaction*)> mTransactionReadyCallback

            GUARDED_BY(mMutex);

    SurfaceComposerClient::Transaction* mSyncTransaction GUARDED_BY(mMutex);

    std::vector<std::tuple<uint64_t /* framenumber */, SurfaceComposerClient::Transaction>>

            mPendingTransactions GUARDED_BY(mMutex);

libs/gui/tests/BLASTBufferQueue_test.cpp

+35 −22
Original line number Diff line number Diff line
@@ -110,15 +110,27 @@ public: 
        mBlastBufferQueueAdapter->update(sc, width, height, PIXEL_FORMAT_RGBA_8888);

    }



    void setSyncTransaction(Transaction* next, bool acquireSingleBuffer = true) {

        mBlastBufferQueueAdapter->setSyncTransaction(next, acquireSingleBuffer);

    void setSyncTransaction(Transaction& next, bool acquireSingleBuffer = true) {

        auto callback = [&next](Transaction* t) { next.merge(std::move(*t)); };

        mBlastBufferQueueAdapter->syncNextTransaction(callback, acquireSingleBuffer);

    }



    void syncNextTransaction(std::function<void(Transaction*)> callback,

                             bool acquireSingleBuffer = true) {

        mBlastBufferQueueAdapter->syncNextTransaction(callback, acquireSingleBuffer);

    }



    void stopContinuousSyncTransaction() {

        mBlastBufferQueueAdapter->stopContinuousSyncTransaction();

    }



    int getWidth() { return mBlastBufferQueueAdapter->mSize.width; }



    int getHeight() { return mBlastBufferQueueAdapter->mSize.height; }



    Transaction* getSyncTransaction() { return mBlastBufferQueueAdapter->mSyncTransaction; }

    std::function<void(Transaction*)> getTransactionReadyCallback() {

        return mBlastBufferQueueAdapter->mTransactionReadyCallback;

    }



    sp<IGraphicBufferProducer> getIGraphicBufferProducer() {

        return mBlastBufferQueueAdapter->getIGraphicBufferProducer();
@@ -343,7 +355,7 @@ TEST_F(BLASTBufferQueueTest, CreateBLASTBufferQueue) { 
    ASSERT_EQ(mSurfaceControl, adapter.getSurfaceControl());

    ASSERT_EQ(mDisplayWidth, adapter.getWidth());

    ASSERT_EQ(mDisplayHeight, adapter.getHeight());

    ASSERT_EQ(nullptr, adapter.getSyncTransaction());

    ASSERT_EQ(nullptr, adapter.getTransactionReadyCallback());

}



TEST_F(BLASTBufferQueueTest, Update) {
@@ -364,11 +376,12 @@ TEST_F(BLASTBufferQueueTest, Update) { 
    ASSERT_EQ(mDisplayHeight / 2, height);

}



TEST_F(BLASTBufferQueueTest, SetSyncTransaction) {

TEST_F(BLASTBufferQueueTest, SyncNextTransaction) {

    BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);

    Transaction sync;

    adapter.setSyncTransaction(&sync);

    ASSERT_EQ(&sync, adapter.getSyncTransaction());

    ASSERT_EQ(nullptr, adapter.getTransactionReadyCallback());

    auto callback = [](Transaction*) {};

    adapter.syncNextTransaction(callback);

    ASSERT_NE(nullptr, adapter.getTransactionReadyCallback());

}



TEST_F(BLASTBufferQueueTest, DISABLED_onFrameAvailable_ApplyDesiredPresentTime) {
@@ -808,7 +821,7 @@ TEST_F(BLASTBufferQueueTest, SyncThenNoSync) { 
    setUpProducer(adapter, igbProducer);



    Transaction sync;

    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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



    // queue non sync buffer, so this one should get blocked
@@ -848,12 +861,12 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncTransactions) { 
    Transaction mainTransaction;



    Transaction sync;

    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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



    mainTransaction.merge(std::move(sync));



    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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



    mainTransaction.merge(std::move(sync));
@@ -889,7 +902,7 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncTransactionWithNonSync) { 


    Transaction sync;

    // queue a sync transaction

    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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



    mainTransaction.merge(std::move(sync));
@@ -898,7 +911,7 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncTransactionWithNonSync) { 
    queueBuffer(igbProducer, 0, 0, 255, 0);



    // queue another sync transaction

    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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

    // Expect 1 buffer to be released because the non sync transaction should merge

    // with the sync
@@ -937,7 +950,7 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncRunOutOfBuffers) { 


    Transaction sync;

    // queue a sync transaction

    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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



    mainTransaction.merge(std::move(sync));
@@ -948,7 +961,7 @@ TEST_F(BLASTBufferQueueTest, MultipleSyncRunOutOfBuffers) { 
    queueBuffer(igbProducer, 0, 0, 255, 0);



    // queue another sync transaction

    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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

    // Expect 3 buffers to be released because the non sync transactions should merge

    // with the sync
@@ -994,7 +1007,7 @@ TEST_F(BLASTBufferQueueTest, RunOutOfBuffersWaitingOnSF) { 


    Transaction sync;

    // queue a sync transaction

    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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



    mainTransaction.merge(std::move(sync));
@@ -1008,7 +1021,7 @@ TEST_F(BLASTBufferQueueTest, RunOutOfBuffersWaitingOnSF) { 
    mainTransaction.apply();



    // queue another sync transaction

    adapter.setSyncTransaction(&sync);

    adapter.setSyncTransaction(sync);

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

    // Expect 2 buffers to be released because the non sync transactions should merge

    // with the sync
@@ -1031,19 +1044,19 @@ TEST_F(BLASTBufferQueueTest, RunOutOfBuffersWaitingOnSF) { 
            checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight}));

}



TEST_F(BLASTBufferQueueTest, SetSyncTransactionAcquireMultipleBuffers) {

TEST_F(BLASTBufferQueueTest, SyncNextTransactionAcquireMultipleBuffers) {

    BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);



    sp<IGraphicBufferProducer> igbProducer;

    setUpProducer(adapter, igbProducer);



    Transaction next;

    adapter.setSyncTransaction(&next, false);

    adapter.setSyncTransaction(next, false);

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

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

    // There should only be one frame submitted since the first frame will be released.

    adapter.validateNumFramesSubmitted(1);

    adapter.setSyncTransaction(nullptr);

    adapter.stopContinuousSyncTransaction();



    // queue non sync buffer, so this one should get blocked

    // Add a present delay to allow the first screenshot to get taken.
@@ -1097,7 +1110,7 @@ TEST_F(BLASTBufferQueueTest, DISABLED_DisconnectProducerTest) { 
        Transaction next;

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

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

        adapter.setSyncTransaction(&next, false);

        adapter.setSyncTransaction(next, true);

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



        CallbackHelper transactionCallback;
@@ -1140,7 +1153,7 @@ TEST_F(BLASTBufferQueueTest, DISABLED_UpdateSurfaceControlTest) { 
        Transaction next;

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

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

        adapter.setSyncTransaction(&next, false);

        adapter.setSyncTransaction(next, true);

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



        CallbackHelper transactionCallback;