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

Commit 83df0349 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Clean up tracking of layers with buffers 

The function to check if the transaction is ready to be applied
should not modify any state. Instead keep track of layers with
ready to be presented outside the check.

Bug: 184063141
Test: atest SurfaceFlinger_test ASurfaceControlTest
Change-Id: I671eabb36195b11c66a1aeefadc78c3d194caeca
parent ddf9b5c4

services/surfaceflinger/SurfaceFlinger.cpp

+36 −23
Original line number Diff line number Diff line
@@ -3319,7 +3319,7 @@ void SurfaceFlinger::flushTransactionQueues() { 
    // 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;

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

    {

        Mutex::Autolock _l(mStateLock);

        {
@@ -3335,10 +3335,13 @@ void SurfaceFlinger::flushTransactionQueues() { 
                                                       transaction.isAutoTimestamp,

                                                       transaction.desiredPresentTime,

                                                       transaction.originUid, transaction.states,

                                                       pendingBuffers)) {

                                                       bufferLayersReadyToPresent)) {

                        setTransactionFlags(eTransactionFlushNeeded);

                        break;

                    }

                    transaction.traverseStatesWithBuffers([&](const layer_state_t& state) {

                        bufferLayersReadyToPresent.insert(state.surface);

                    });

                    transactions.emplace_back(std::move(transaction));

                    transactionQueue.pop();

                }
@@ -3359,14 +3362,17 @@ void SurfaceFlinger::flushTransactionQueues() { 
                auto& transaction = mTransactionQueue.front();

                bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) !=

                        mPendingTransactionQueues.end();

                if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo,

                if (pendingTransactions ||

                    !transactionIsReadyToBeApplied(transaction.frameTimelineInfo,

                                                   transaction.isAutoTimestamp,

                                                   transaction.desiredPresentTime,

                                                   transaction.originUid, transaction.states,

                                                   pendingBuffers) ||

                    pendingTransactions) {

                                                   bufferLayersReadyToPresent)) {

                    mPendingTransactionQueues[transaction.applyToken].push(std::move(transaction));

                } else {

                    transaction.traverseStatesWithBuffers([&](const layer_state_t& state) {

                        bufferLayersReadyToPresent.insert(state.surface);

                    });

                    transactions.emplace_back(std::move(transaction));

                }

                mTransactionQueue.pop();
@@ -3421,28 +3427,28 @@ bool SurfaceFlinger::frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId) 
bool SurfaceFlinger::transactionIsReadyToBeApplied(

        const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,

        uid_t originUid, const Vector<ComposerState>& states,

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

        const std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>&

                bufferLayersReadyToPresent) const {

    ATRACE_CALL();

    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

    // in the future. We ignore timestamps more than 1 second in the future for stability reasons.

    if (!isAutoTimestamp && desiredPresentTime >= expectedPresentTime &&

        desiredPresentTime < expectedPresentTime + s2ns(1)) {

        ATRACE_NAME("not current");

        ready = false;

        return false;

    }



    if (!mScheduler->isVsyncValid(expectedPresentTime, originUid)) {

        ATRACE_NAME("!isVsyncValid");

        ready = false;

        return false;

    }



    // If the client didn't specify desiredPresentTime, use the vsyncId to determine the expected

    // present time of this transaction.

    if (isAutoTimestamp && frameIsEarly(expectedPresentTime, info.vsyncId)) {

        ATRACE_NAME("frameIsEarly");

        ready = false;

        return false;

    }



    for (const ComposerState& state : states) {
@@ -3451,7 +3457,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( 
        if (acquireFenceChanged && s.acquireFence &&

            s.acquireFence->getStatus() == Fence::Status::Unsignaled) {

            ATRACE_NAME("fence unsignaled");

            ready = false;

            return false;

        }



        sp<Layer> layer = nullptr;
@@ -3470,15 +3476,15 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( 
        if (s.hasBufferChanges()) {

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

            // transaction in the queue.

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

            const bool hasPendingBuffer =

                    bufferLayersReadyToPresent.find(s.surface) != bufferLayersReadyToPresent.end();

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

                ATRACE_NAME("hasPendingBuffer");

                ready = false;

                return false;

            }

            pendingBuffers.insert(s.surface);

        }

    }

    return ready;

    return true;

}



void SurfaceFlinger::queueTransaction(TransactionState& state) {
@@ -3548,13 +3554,6 @@ status_t SurfaceFlinger::setTransactionState( 
        const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) {

    ATRACE_CALL();



    // Check for incoming buffer updates and increment the pending buffer count.

    for (const auto& state : states) {

        if (state.state.hasBufferChanges() && (state.state.surface)) {

            mBufferCountTracker.increment(state.state.surface->localBinder());

        }

    }



    uint32_t permissions =

            callingThreadHasUnscopedSurfaceFlingerAccess() ? Permission::ACCESS_SURFACE_FLINGER : 0;

    // Avoid checking for rotation permissions if the caller already has ACCESS_SURFACE_FLINGER
@@ -3583,6 +3582,11 @@ status_t SurfaceFlinger::setTransactionState( 
                           permissions,        hasListenerCallbacks,

                           listenerCallbacks,  originPid,

                           originUid,          transactionId};



    // Check for incoming buffer updates and increment the pending buffer count.

    state.traverseStatesWithBuffers([&](const layer_state_t& state) {

        mBufferCountTracker.increment(state.surface->localBinder());

    });

    queueTransaction(state);



    // Check the pending state to make sure the transaction is synchronous.
@@ -6320,7 +6324,7 @@ wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) { 
    return fromHandleLocked(handle);

}



wp<Layer> SurfaceFlinger::fromHandleLocked(const sp<IBinder>& handle) {

wp<Layer> SurfaceFlinger::fromHandleLocked(const sp<IBinder>& handle) const {

    BBinder* b = nullptr;

    if (handle) {

        b = handle->localBinder();
@@ -6562,6 +6566,15 @@ status_t SurfaceFlinger::getExtraBufferCount(int* extraBuffers) const { 
    return NO_ERROR;

}



void SurfaceFlinger::TransactionState::traverseStatesWithBuffers(

        std::function<void(const layer_state_t&)> visitor) {

    for (const auto& state : states) {

        if (state.state.hasBufferChanges() && (state.state.surface)) {

            visitor(state.state);

        }

    }

}



} // namespace android



#if defined(__gl_h_)

services/surfaceflinger/SurfaceFlinger.h

+5 −3
Original line number Diff line number Diff line
@@ -329,7 +329,7 @@ public: 
    // Otherwise, returns a weak reference so that callers off the main-thread

    // won't accidentally hold onto the last strong reference.

    wp<Layer> fromHandle(const sp<IBinder>& handle);

    wp<Layer> fromHandleLocked(const sp<IBinder>& handle) REQUIRES(mStateLock);

    wp<Layer> fromHandleLocked(const sp<IBinder>& handle) const REQUIRES(mStateLock);



    // Inherit from ClientCache::ErasedRecipient

    void bufferErased(const client_cache_t& clientCacheId) override;
@@ -536,6 +536,8 @@ private: 
                originUid(originUid),

                id(transactionId) {}



        void traverseStatesWithBuffers(std::function<void(const layer_state_t&)> visitor);



        FrameTimelineInfo frameTimelineInfo;

        Vector<ComposerState> states;

        Vector<DisplayState> displays;
@@ -841,8 +843,8 @@ private: 
    bool transactionIsReadyToBeApplied(

            const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,

            uid_t originUid, const Vector<ComposerState>& states,

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

            REQUIRES(mStateLock);

            const std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>&

                    bufferLayersReadyToPresent) const REQUIRES(mStateLock);

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

    uint32_t addInputWindowCommands(const InputWindowCommands& inputWindowCommands)

            REQUIRES(mStateLock);