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

Commit f03652dd authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Store layer state changes by layer handle in Transaction objects 

Layer state changes are stored in an unordered map with the surface control as the key
and state changes as the value. This causes a few problems when merging the transactions. If
transactions contained state changes from a cloned surface control then it will not be merged.
This will cause ordering issues when the transaction is applied since the changes are stored in
and unordered map.

When parcelling transactions, a new surface control is created from the existing one and this
surfaces the problem more frequently.

Instead we store the layer changes by the layer handle which is consistent across processes.

Test: atest SurfaceFlinger_test
Test: go/wm-smoke

Change-Id: I2e041d70ae24db2c1f26ada003532ad97f667167
parent f9f627bf

libs/gui/LayerState.cpp

+0 −2
Original line number Diff line number Diff line
@@ -169,12 +169,10 @@ status_t layer_state_t::read(const Parcel& input) 
}



status_t ComposerState::write(Parcel& output) const {

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

    return state.write(output);

}



status_t ComposerState::read(const Parcel& input) {

    client = interface_cast<ISurfaceComposerClient>(input.readStrongBinder());

    return state.read(input);

}

libs/gui/SurfaceComposerClient.cpp

+19 −21
Original line number Diff line number Diff line
@@ -202,7 +202,8 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 
     * callbackIds to generate one super map that contains all the sp<IBinder> to sp<SurfaceControl>

     * that could possibly exist for the callbacks.

     */

    std::unordered_map<sp<IBinder>, sp<SurfaceControl>, IBinderHash> surfaceControls;

    std::unordered_map<sp<IBinder>, sp<SurfaceControl>, SurfaceComposerClient::IBinderHash>

            surfaceControls;

    for (const auto& transactionStats : listenerStats.transactionStats) {

        for (auto callbackId : transactionStats.callbackIds) {

            auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];
@@ -365,16 +366,16 @@ status_t SurfaceComposerClient::Transaction::readFromParcel(const Parcel* parcel 
    if (count > parcel->dataSize()) {

        return BAD_VALUE;

    }

    std::unordered_map<sp<SurfaceControl>, ComposerState, SCHash> composerStates;

    std::unordered_map<sp<IBinder>, ComposerState, IBinderHash> composerStates;

    composerStates.reserve(count);

    for (size_t i = 0; i < count; i++) {

        sp<SurfaceControl> surfaceControl = SurfaceControl::readFromParcel(parcel);

        sp<IBinder> surfaceControlHandle = parcel->readStrongBinder();



        ComposerState composerState;

        if (composerState.read(*parcel) == BAD_VALUE) {

            return BAD_VALUE;

        }

        composerStates[surfaceControl] = composerState;

        composerStates[surfaceControlHandle] = composerState;

    }



    InputWindowCommands inputWindowCommands;
@@ -408,8 +409,8 @@ status_t SurfaceComposerClient::Transaction::writeToParcel(Parcel* parcel) const 
    }



    parcel->writeUint32(static_cast<uint32_t>(mComposerStates.size()));

    for (auto const& [surfaceControl, composerState] : mComposerStates) {

        surfaceControl->writeToParcel(parcel);

    for (auto const& [surfaceHandle, composerState] : mComposerStates) {

        parcel->writeStrongBinder(surfaceHandle);

        composerState.write(*parcel);

    }
@@ -418,11 +419,11 @@ status_t SurfaceComposerClient::Transaction::writeToParcel(Parcel* parcel) const 
}



SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Transaction&& other) {

    for (auto const& kv : other.mComposerStates) {

        if (mComposerStates.count(kv.first) == 0) {

            mComposerStates[kv.first] = kv.second;

    for (auto const& [surfaceHandle, composerState] : other.mComposerStates) {

        if (mComposerStates.count(surfaceHandle) == 0) {

            mComposerStates[surfaceHandle] = composerState;

        } else {

            mComposerStates[kv.first].state.merge(kv.second.state);

            mComposerStates[surfaceHandle].state.merge(composerState.state);

        }

    }
@@ -465,14 +466,12 @@ void SurfaceComposerClient::Transaction::clear() { 
    mDesiredPresentTime = -1;

}



void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle,

        const sp<ISurfaceComposerClient>& client) {

void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle) {

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

    Vector<ComposerState> composerStates;

    Vector<DisplayState> displayStates;



    ComposerState s;

    s.client = client;

    s.state.surface = handle;

    s.state.what |= layer_state_t::eReparent;

    s.state.parentHandleForChild = nullptr;
@@ -499,8 +498,8 @@ void SurfaceComposerClient::Transaction::cacheBuffers() { 
    }



    size_t count = 0;

    for (auto& [sc, cs] : mComposerStates) {

        layer_state_t* s = getLayerState(sc);

    for (auto& [handle, cs] : mComposerStates) {

        layer_state_t* s = getLayerState(handle);

        if (!(s->what & layer_state_t::eBufferChanged)) {

            continue;

        }
@@ -639,16 +638,15 @@ void SurfaceComposerClient::Transaction::setEarlyWakeup() { 
    mEarlyWakeup = true;

}



layer_state_t* SurfaceComposerClient::Transaction::getLayerState(const sp<SurfaceControl>& sc) {

    if (mComposerStates.count(sc) == 0) {

layer_state_t* SurfaceComposerClient::Transaction::getLayerState(const sp<IBinder>& handle) {

    if (mComposerStates.count(handle) == 0) {

        // we don't have it, add an initialized layer_state to our list

        ComposerState s;

        s.client = sc->getClient()->mClient;

        s.state.surface = sc->getHandle();

        mComposerStates[sc] = s;

        s.state.surface = handle;

        mComposerStates[handle] = s;

    }



    return &(mComposerStates[sc].state);

    return &(mComposerStates[handle].state);

}



void SurfaceComposerClient::Transaction::registerSurfaceControlForCallback(

libs/gui/SurfaceControl.cpp

+2 −2
Original line number Diff line number Diff line
@@ -65,8 +65,8 @@ SurfaceControl::~SurfaceControl() 
{

    // Avoid reparenting the server-side surface to null if we are not the owner of it,

    // meaning that we retrieved it from another process.

    if (mClient != nullptr && mHandle != nullptr && mOwned) {

        SurfaceComposerClient::doDropReferenceTransaction(mHandle, mClient->getClient());

    if (mHandle != nullptr && mOwned) {

        SurfaceComposerClient::doDropReferenceTransaction(mHandle);

    }

    release();

}

libs/gui/include/gui/LayerState.h

+0 −3
Original line number Diff line number Diff line
@@ -206,7 +206,6 @@ struct layer_state_t { 
};



struct ComposerState {

    sp<ISurfaceComposerClient> client;

    layer_state_t state;

    status_t write(Parcel& output) const;

    status_t read(const Parcel& input);
@@ -274,8 +273,6 @@ struct InputWindowCommands { 
};



static inline int compare_type(const ComposerState& lhs, const ComposerState& rhs) {

    if (lhs.client < rhs.client) return -1;

    if (lhs.client > rhs.client) return 1;

    if (lhs.state.surface < rhs.state.surface) return -1;

    if (lhs.state.surface > rhs.state.surface) return 1;

    return 0;

libs/gui/include/gui/SurfaceComposerClient.h

+14 −11
Original line number Diff line number Diff line
@@ -163,8 +163,7 @@ public: 
     * Called from SurfaceControl d'tor to 'destroy' the surface (or rather, reparent it

     * to null), but without needing an sp<SurfaceControl> to avoid infinite ressurection.

     */

    static void doDropReferenceTransaction(const sp<IBinder>& handle,

            const sp<ISurfaceComposerClient>& client);

    static void doDropReferenceTransaction(const sp<IBinder>& handle);



    /**

     * Uncaches a buffer in ISurfaceComposer. It must be uncached via a transaction so that it is
@@ -270,6 +269,12 @@ public: 
        }

    };



    struct IBinderHash {

        std::size_t operator()(const sp<IBinder>& iBinder) const {

            return std::hash<IBinder*>{}(iBinder.get());

        }

    };



    struct TCLHash {

        std::size_t operator()(const sp<ITransactionCompletedListener>& tcl) const {

            return std::hash<IBinder*>{}((tcl) ? IInterface::asBinder(tcl).get() : nullptr);
@@ -286,7 +291,7 @@ public: 
    };



    class Transaction : Parcelable {

        std::unordered_map<sp<SurfaceControl>, ComposerState, SCHash> mComposerStates;

        std::unordered_map<sp<IBinder>, ComposerState, IBinderHash> mComposerStates;

        SortedVector<DisplayState > mDisplayStates;

        std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash>

                mListenerCallbacks;
@@ -314,7 +319,10 @@ public: 
        InputWindowCommands mInputWindowCommands;

        int mStatus = NO_ERROR;



        layer_state_t* getLayerState(const sp<SurfaceControl>& sc);

        layer_state_t* getLayerState(const sp<IBinder>& surfaceHandle);

        layer_state_t* getLayerState(const sp<SurfaceControl>& sc) {

            return getLayerState(sc->getHandle());

        }

        DisplayState& getDisplayState(const sp<IBinder>& token);



        void cacheBuffers();
@@ -560,15 +568,10 @@ class TransactionCompletedListener : public BnTransactionCompletedListener { 


    CallbackId mCallbackIdCounter GUARDED_BY(mMutex) = 1;



    struct IBinderHash {

        std::size_t operator()(const sp<IBinder>& iBinder) const {

            return std::hash<IBinder*>{}(iBinder.get());

        }

    };



    struct CallbackTranslation {

        TransactionCompletedCallback callbackFunction;

        std::unordered_map<sp<IBinder>, sp<SurfaceControl>, IBinderHash> surfaceControls;

        std::unordered_map<sp<IBinder>, sp<SurfaceControl>, SurfaceComposerClient::IBinderHash>

                surfaceControls;

    };



    std::unordered_map<CallbackId, CallbackTranslation> mCallbacks GUARDED_BY(mMutex);