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

Commit 3dad52d3 authored by Marissa Wall's avatar Marissa Wall
Browse files

blast: transaction callbacks should come in order 

Previously, if no SurfaceControls where marked to get callbacks,
a callback was sent directly from SurfaceComposerClient so we could
save a binder call into SurfaceFlinger and a binder call for the
callback.

Although this saved us 2 binder calls, it made the transactions
callbacks come out of order. The public api guarantees that all
callbacks must come in order.

This patch moves the callback from SurfaceComposerClient into
SurfaceFlinger so the callbacks are in order.

Bug: 128519264
Test: SurfaceFlinger_test
Change-Id: Ia1cadb81adb69b58a4d6d43ae453c96a1572f833
parent 78b7220f

libs/gui/ISurfaceComposer.cpp

+21 −2
Original line number Diff line number Diff line
@@ -69,7 +69,8 @@ public: 
                                     const sp<IBinder>& applyToken,

                                     const InputWindowCommands& commands,

                                     int64_t desiredPresentTime,

                                     const cached_buffer_t& uncacheBuffer) {

                                     const cached_buffer_t& uncacheBuffer,

                                     const std::vector<ListenerCallbacks>& listenerCallbacks) {

        Parcel data, reply;

        data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
@@ -89,6 +90,14 @@ public: 
        data.writeInt64(desiredPresentTime);

        data.writeStrongBinder(uncacheBuffer.token);

        data.writeUint64(uncacheBuffer.cacheId);



        if (data.writeVectorSize(listenerCallbacks) == NO_ERROR) {

            for (const auto& [listener, callbackIds] : listenerCallbacks) {

                data.writeStrongBinder(IInterface::asBinder(listener));

                data.writeInt64Vector(callbackIds);

            }

        }



        remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE, data, &reply);

    }
@@ -978,8 +987,18 @@ status_t BnSurfaceComposer::onTransact( 
            uncachedBuffer.token = data.readStrongBinder();

            uncachedBuffer.cacheId = data.readUint64();



            std::vector<ListenerCallbacks> listenerCallbacks;

            int32_t listenersSize = data.readInt32();

            for (int32_t i = 0; i < listenersSize; i++) {

                auto listener =

                        interface_cast<ITransactionCompletedListener>(data.readStrongBinder());

                std::vector<CallbackId> callbackIds;

                data.readInt64Vector(&callbackIds);

                listenerCallbacks.emplace_back(listener, callbackIds);

            }



            setTransactionState(state, displays, stateFlags, applyToken, inputWindowCommands,

                                desiredPresentTime, uncachedBuffer);

                                desiredPresentTime, uncachedBuffer, listenerCallbacks);

            return NO_ERROR;

        }

        case BOOT_FINISHED: {

libs/gui/LayerState.cpp

+5 −20
Original line number Diff line number Diff line
@@ -86,14 +86,7 @@ status_t layer_state_t::write(Parcel& output) const 
    memcpy(output.writeInplace(16 * sizeof(float)),

           colorTransform.asArray(), 16 * sizeof(float));

    output.writeFloat(cornerRadius);



    if (output.writeVectorSize(listenerCallbacks) == NO_ERROR) {

        for (const auto& [listener, callbackIds] : listenerCallbacks) {

            output.writeStrongBinder(IInterface::asBinder(listener));

            output.writeInt64Vector(callbackIds);

        }

    }



    output.writeBool(hasListenerCallbacks);

    output.writeStrongBinder(cachedBuffer.token);

    output.writeUint64(cachedBuffer.cacheId);

    output.writeParcelable(metadata);
@@ -163,15 +156,7 @@ status_t layer_state_t::read(const Parcel& input) 


    colorTransform = mat4(static_cast<const float*>(input.readInplace(16 * sizeof(float))));

    cornerRadius = input.readFloat();



    int32_t listenersSize = input.readInt32();

    for (int32_t i = 0; i < listenersSize; i++) {

        auto listener = interface_cast<ITransactionCompletedListener>(input.readStrongBinder());

        std::vector<CallbackId> callbackIds;

        input.readInt64Vector(&callbackIds);

        listenerCallbacks.emplace_back(listener, callbackIds);

    }



    hasListenerCallbacks = input.readBool();

    cachedBuffer.token = input.readStrongBinder();

    cachedBuffer.cacheId = input.readUint64();

    input.readParcelable(&metadata);
@@ -376,9 +361,9 @@ void layer_state_t::merge(const layer_state_t& other) { 
        what |= eColorTransformChanged;

        colorTransform = other.colorTransform;

    }

    if (other.what & eListenerCallbacksChanged) {

        what |= eListenerCallbacksChanged;

        listenerCallbacks = other.listenerCallbacks;

    if (other.what & eHasListenerCallbacksChanged) {

        what |= eHasListenerCallbacksChanged;

        hasListenerCallbacks = other.hasListenerCallbacks;

    }



#ifndef NO_INPUT

libs/gui/SurfaceComposerClient.cpp

+9 −10
Original line number Diff line number Diff line
@@ -381,7 +381,7 @@ void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle 
    s.state.parentHandleForChild = nullptr;



    composerStates.add(s);

    sf->setTransactionState(composerStates, displayStates, 0, nullptr, {}, -1, {});

    sf->setTransactionState(composerStates, displayStates, 0, nullptr, {}, -1, {}, {});

}



void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {
@@ -391,7 +391,7 @@ void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) { 
    uncacheBuffer.token = BufferCache::getInstance().getToken();

    uncacheBuffer.cacheId = cacheId;



    sf->setTransactionState({}, {}, 0, nullptr, {}, -1, uncacheBuffer);

    sf->setTransactionState({}, {}, 0, nullptr, {}, -1, uncacheBuffer, {});

}



void SurfaceComposerClient::Transaction::cacheBuffers() {
@@ -434,6 +434,8 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { 


    sp<ISurfaceComposer> sf(ComposerService::getComposerService());



    std::vector<ListenerCallbacks> listenerCallbacks;



    // For every listener with registered callbacks

    for (const auto& [listener, callbackInfo] : mListenerCallbacks) {

        auto& [callbackIds, surfaceControls] = callbackInfo;
@@ -441,11 +443,7 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { 
            continue;

        }



        // If the listener does not have any SurfaceControls set on this Transaction, send the

        // callback now

        if (surfaceControls.empty()) {

            listener->onTransactionCompleted(ListenerStats::createEmpty(listener, callbackIds));

        }

        listenerCallbacks.emplace_back(listener, std::move(callbackIds));



        // If the listener has any SurfaceControls set on this Transaction update the surface state

        for (const auto& surfaceControl : surfaceControls) {
@@ -454,8 +452,8 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { 
                ALOGE("failed to get layer state");

                continue;

            }

            s->what |= layer_state_t::eListenerCallbacksChanged;

            s->listenerCallbacks.emplace_back(listener, std::move(callbackIds));

            s->what |= layer_state_t::eHasListenerCallbacksChanged;

            s->hasListenerCallbacks = true;

        }

    }

    mListenerCallbacks.clear();
@@ -494,7 +492,8 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { 
    sp<IBinder> applyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance());

    sf->setTransactionState(composerStates, displayStates, flags, applyToken, mInputWindowCommands,

                            mDesiredPresentTime,

                            {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/);

                            {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/,

                            listenerCallbacks);

    mInputWindowCommands.clear();

    mStatus = NO_ERROR;

    return NO_ERROR;

libs/gui/include/gui/ISurfaceComposer.h

+9 −6
Original line number Diff line number Diff line
@@ -20,13 +20,10 @@ 
#include <stdint.h>

#include <sys/types.h>



#include <utils/RefBase.h>

#include <utils/Errors.h>

#include <utils/Timers.h>

#include <utils/Vector.h>



#include <binder/IInterface.h>



#include <gui/ITransactionCompletedListener.h>



#include <ui/ConfigStoreTypes.h>

#include <ui/DisplayedFrameStats.h>

#include <ui/FrameStats.h>
@@ -34,6 +31,11 @@ 
#include <ui/GraphicTypes.h>

#include <ui/PixelFormat.h>



#include <utils/Errors.h>

#include <utils/RefBase.h>

#include <utils/Timers.h>

#include <utils/Vector.h>



#include <optional>

#include <vector>
@@ -133,7 +135,8 @@ public: 
                                     const sp<IBinder>& applyToken,

                                     const InputWindowCommands& inputWindowCommands,

                                     int64_t desiredPresentTime,

                                     const cached_buffer_t& uncacheBuffer) = 0;

                                     const cached_buffer_t& uncacheBuffer,

                                     const std::vector<ListenerCallbacks>& listenerCallbacks) = 0;



    /* signal that we're done booting.

     * Requires ACCESS_SURFACE_FLINGER permission

libs/gui/include/gui/LayerState.h

+3 −3
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@ 
#include <utils/Errors.h>



#include <gui/IGraphicBufferProducer.h>

#include <gui/ITransactionCompletedListener.h>

#include <math/mat4.h>



#ifndef NO_INPUT
@@ -86,7 +85,7 @@ struct layer_state_t { 
        eApiChanged = 0x04000000,

        eSidebandStreamChanged = 0x08000000,

        eColorTransformChanged = 0x10000000,

        eListenerCallbacksChanged = 0x20000000,

        eHasListenerCallbacksChanged = 0x20000000,

        eInputInfoChanged = 0x40000000,

        eCornerRadiusChanged = 0x80000000,

        eFrameChanged = 0x1'00000000,
@@ -120,6 +119,7 @@ struct layer_state_t { 
            surfaceDamageRegion(),

            api(-1),

            colorTransform(mat4()),

            hasListenerCallbacks(false),

            bgColorAlpha(0),

            bgColorDataspace(ui::Dataspace::UNKNOWN),

            colorSpaceAgnostic(false) {
@@ -182,7 +182,7 @@ struct layer_state_t { 
    sp<NativeHandle> sidebandStream;

    mat4 colorTransform;



    std::vector<ListenerCallbacks> listenerCallbacks;

    bool hasListenerCallbacks;

#ifndef NO_INPUT

    InputWindowInfo inputInfo;

#endif