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

Commit 12f62a02 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Check transaction queues when dropping or queuing buffers 

When checking if a transaction is ready to be presented, we need to
consider the buffers in the current transaction queue. The current
state is only updated after the transactionIsReadyToBeApplied check.

Without this fix, we were incorrectly dropping buffers that did not
have presentation timestamps.

Test: atest SurfaceViewBufferTests
Test: go/wm-smoke
Bug: b/178148035
Change-Id: Idb108063ea2694f4a5ab5bbc9fc039f26579a16b
parent 4b1a37bb

services/surfaceflinger/Layer.h

+0 −1
Original line number Diff line number Diff line
@@ -904,7 +904,6 @@ public: 
    bool mPendingHWCDestroy{false};



    bool backpressureEnabled() { return mDrawingState.flags & layer_state_t::eEnableBackpressure; }

    bool hasPendingBuffer() { return mCurrentState.buffer != mDrawingState.buffer; };



protected:

    class SyncPoint {

services/surfaceflinger/SurfaceFlinger.cpp

+66 −58
Original line number Diff line number Diff line
@@ -3268,6 +3268,10 @@ void SurfaceFlinger::flushTransactionQueues() { 
    // we must keep a copy of the transactions (specifically the composer

    // states) around outside the scope of the lock

    std::vector<const TransactionState> transactions;

    // Layer handles that have transactions with buffers that are ready to be applied.

    std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>> pendingBuffers;

    {

        Mutex::Autolock _l(mStateLock);

        {

            Mutex::Autolock _l(mQueueLock);

            // Collect transactions from pending transaction queue.
@@ -3279,7 +3283,9 @@ void SurfaceFlinger::flushTransactionQueues() { 
                    const auto& transaction = transactionQueue.front();

                    if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp,

                                                       transaction.desiredPresentTime,

                                                   transaction.states)) {

                                                       transaction.states,

                                                       false /* updateTransactionCounters*/,

                                                       pendingBuffers)) {

                        setTransactionFlags(eTransactionFlushNeeded);

                        break;

                    }
@@ -3304,10 +3310,13 @@ void SurfaceFlinger::flushTransactionQueues() { 
                bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) !=

                        mPendingTransactionQueues.end();

                // Call transactionIsReadyToBeApplied first in case we need to

            // incrementPendingBufferCount if the transaction contains a buffer.

                // incrementPendingBufferCount and keep track of pending buffers

                // if the transaction contains a buffer.

                if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp,

                                               transaction.desiredPresentTime, transaction.states,

                                               true) ||

                                                   transaction.desiredPresentTime,

                                                   transaction.states,

                                                   true /* updateTransactionCounters */,

                                                   pendingBuffers) ||

                    pendingTransactions) {

                    mPendingTransactionQueues[transaction.applyToken].push(transaction);

                } else {
@@ -3318,15 +3327,15 @@ void SurfaceFlinger::flushTransactionQueues() { 
        }



        // Now apply all transactions.

    Mutex::Autolock _l(mStateLock);

        for (const auto& transaction : transactions) {

            applyTransactionState(transaction.frameTimelineInfo, transaction.states,

                                  transaction.displays, transaction.flags,

                                  transaction.inputWindowCommands, transaction.desiredPresentTime,

                              transaction.isAutoTimestamp, transaction.buffer, transaction.postTime,

                              transaction.privileged, transaction.hasListenerCallbacks,

                              transaction.listenerCallbacks, transaction.originPid,

                              transaction.originUid, transaction.id);

                                  transaction.isAutoTimestamp, transaction.buffer,

                                  transaction.postTime, transaction.privileged,

                                  transaction.hasListenerCallbacks, transaction.listenerCallbacks,

                                  transaction.originPid, transaction.originUid, transaction.id);

        }

    }

}
@@ -3335,9 +3344,10 @@ bool SurfaceFlinger::transactionFlushNeeded() { 
    return !mPendingTransactionQueues.empty();

}



bool SurfaceFlinger::transactionIsReadyToBeApplied(bool isAutoTimestamp, int64_t desiredPresentTime,

                                                   const Vector<ComposerState>& states,

                                                   bool updateTransactionCounters) {

bool SurfaceFlinger::transactionIsReadyToBeApplied(

        bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states,

        bool updateTransactionCounters,

        std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers) {

    const nsecs_t expectedPresentTime = mExpectedPresentTime.load();

    bool ready = true;

    // Do not present if the desiredPresentTime has not passed unless it is more than one second
@@ -3356,7 +3366,6 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(bool isAutoTimestamp, int64_t 
            ready = false;

        }



        Mutex::Autolock _l(mStateLock);

        sp<Layer> layer = nullptr;

        if (s.surface) {

            layer = fromHandleLocked(s.surface).promote();
@@ -3379,12 +3388,11 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(bool isAutoTimestamp, int64_t 


        // If backpressure is enabled and we already have a buffer to commit, keep the transaction

        // in the queue.

        bool hasBuffer = s.what & layer_state_t::eBufferChanged ||

                s.what & layer_state_t::eCachedBufferChanged;

        if (hasBuffer && layer->backpressureEnabled() && layer->hasPendingBuffer() &&

            isAutoTimestamp) {

        const bool hasPendingBuffer = pendingBuffers.find(s.surface) != pendingBuffers.end();

        if (layer->backpressureEnabled() && hasPendingBuffer && isAutoTimestamp) {

            ready = false;

        }

        pendingBuffers.insert(s.surface);

    }

    return ready;

}

services/surfaceflinger/SurfaceFlinger.h

+5 −3
Original line number Diff line number Diff line
@@ -760,9 +760,11 @@ private: 
    uint32_t setTransactionFlags(uint32_t flags, TransactionSchedule);

    void commitTransaction() REQUIRES(mStateLock);

    void commitOffscreenLayers();

    bool transactionIsReadyToBeApplied(bool isAutoTimestamp, int64_t desiredPresentTime,

                                       const Vector<ComposerState>& states,

                                       bool updateTransactionCounters = false);

    bool transactionIsReadyToBeApplied(

            bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states,

            bool updateTransactionCounters,

            std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers)

            REQUIRES(mStateLock);

    uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock);

    uint32_t addInputWindowCommands(const InputWindowCommands& inputWindowCommands)

            REQUIRES(mStateLock);