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

Commit 5aedec95 authored by chaviw's avatar chaviw
Browse files

Mark children as detached instead of actually detaching them. 

When detachChildren is called, mark children as detached instead of removing
them from the client. This is so the children can be re-attached again when
they're reparented. Check if layer is detached before copying currentState
to drawingState so detached layers aren't updated. Also check if layer
is detached before removing the layer since they should no longer be
reachable if they're detached.

Test: DetachChildrenThenAttach
Bug: 111297488
Change-Id: I31b9eb1398f02e9d68dd3f7a1f231e1cef9fb0f8
parent 910c0aa1

services/surfaceflinger/Layer.cpp

+32 −1
Original line number Diff line number Diff line
@@ -1021,6 +1021,10 @@ uint32_t Layer::doTransactionResize(uint32_t flags, State* stateToCommit) { 
uint32_t Layer::doTransaction(uint32_t flags) {

    ATRACE_CALL();



    if (mLayerDetached) {

        return 0;

    }



    pushPendingState();

    State c = getCurrentState();

    if (!applyPendingStates(&c)) {
@@ -1553,6 +1557,9 @@ bool Layer::reparentChildren(const sp<IBinder>& newParentHandle) { 
        return false;

    }



    if (attachChildren()) {

        setTransactionFlags(eTransactionNeeded);

    }

    for (const sp<Layer>& child : mCurrentChildren) {

        newParent->addChild(child);
@@ -1597,6 +1604,13 @@ bool Layer::reparent(const sp<IBinder>& newParentHandle) { 
        client->updateParent(newParent);

    }



    if (mLayerDetached) {

        mLayerDetached = false;

        setTransactionFlags(eTransactionNeeded);

    }

    if (attachChildren()) {

        setTransactionFlags(eTransactionNeeded);

    }

    return true;

}
@@ -1605,7 +1619,7 @@ bool Layer::detachChildren() { 
        sp<Client> parentClient = mClientRef.promote();

        sp<Client> client(child->mClientRef.promote());

        if (client != nullptr && parentClient != client) {

            client->detachLayer(child.get());

            child->mLayerDetached = true;

            child->detachChildren();

        }

    }
@@ -1613,6 +1627,23 @@ bool Layer::detachChildren() { 
    return true;

}



bool Layer::attachChildren() {

    bool changed = false;

    for (const sp<Layer>& child : mCurrentChildren) {

        sp<Client> parentClient = mClientRef.promote();

        sp<Client> client(child->mClientRef.promote());

        if (client != nullptr && parentClient != client) {

            if (child->mLayerDetached) {

                child->mLayerDetached = false;

                changed = true;

            }

            changed |= child->attachChildren();

        }

    }



    return changed;

}



bool Layer::setColorTransform(const mat4& matrix) {

    static const mat4 identityMatrix = mat4();

services/surfaceflinger/Layer.h

+5 −1
Original line number Diff line number Diff line
@@ -263,6 +263,8 @@ public: 
    virtual void setChildrenDrawingParent(const sp<Layer>& layer);

    virtual bool reparent(const sp<IBinder>& newParentHandle);

    virtual bool detachChildren();

    bool attachChildren();

    bool isLayerDetached() const { return mLayerDetached; }

    virtual bool setColorTransform(const mat4& matrix);

    virtual const mat4& getColorTransform() const;

    virtual bool hasColorTransform() const;
@@ -355,7 +357,6 @@ public: 
    // to avoid grabbing the lock again to avoid deadlock

    virtual bool isCreatedFromMainThread() const { return false; }





    bool isRemovedFromCurrentState() const;



    void writeToProto(LayerProto* layerInfo,
@@ -776,6 +777,9 @@ protected: 


    mutable LayerBE mBE;



    // Can only be accessed with the SF state lock held.

    bool mLayerDetached{false};



private:

    /**

     * Returns an unsorted vector of all layers that are part of this tree.

services/surfaceflinger/SurfaceFlinger.cpp

+4 −0
Original line number Diff line number Diff line
@@ -3198,6 +3198,10 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) { 
}



status_t SurfaceFlinger::removeLayerLocked(const Mutex& lock, const sp<Layer>& layer) {

    if (layer->isLayerDetached()) {

        return NO_ERROR;

    }



    const auto& p = layer->getParent();

    ssize_t index;

    if (p != nullptr) {

services/surfaceflinger/tests/Transaction_test.cpp

+55 −0
Original line number Diff line number Diff line
@@ -3356,6 +3356,61 @@ TEST_F(ChildLayerTest, DetachChildrenDifferentClient) { 
    }

}



TEST_F(ChildLayerTest, DetachChildrenThenAttach) {

    sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;

    sp<SurfaceControl> childNewClient =

            newComposerClient->createSurface(String8("New Child Test Surface"), 10, 10,

                                             PIXEL_FORMAT_RGBA_8888, 0, mFGSurfaceControl.get());



    ASSERT_TRUE(childNewClient != nullptr);

    ASSERT_TRUE(childNewClient->isValid());



    fillSurfaceRGBA8(childNewClient, 200, 200, 200);



    Transaction()

            .hide(mChild)

            .show(childNewClient)

            .setPosition(childNewClient, 10, 10)

            .setPosition(mFGSurfaceControl, 64, 64)

            .apply();



    {

        mCapture = screenshot();

        // Top left of foreground must now be visible

        mCapture->expectFGColor(64, 64);

        // But 10 pixels in we should see the child surface

        mCapture->expectChildColor(74, 74);

        // And 10 more pixels we should be back to the foreground surface

        mCapture->expectFGColor(84, 84);

    }



    Transaction().detachChildren(mFGSurfaceControl).apply();

    Transaction().hide(childNewClient).apply();



    // Nothing should have changed.

    {

        mCapture = screenshot();

        mCapture->expectFGColor(64, 64);

        mCapture->expectChildColor(74, 74);

        mCapture->expectFGColor(84, 84);

    }



    sp<SurfaceControl> newParentSurface = createLayer(String8("New Parent Surface"), 32, 32, 0);

    fillLayerColor(ISurfaceComposerClient::eFXSurfaceBufferQueue, newParentSurface, Color::RED, 32,

                   32);

    Transaction()

            .setLayer(newParentSurface, INT32_MAX - 1)

            .show(newParentSurface)

            .setPosition(newParentSurface, 20, 20)

            .reparent(childNewClient, newParentSurface->getHandle())

            .apply();

    {

        mCapture = screenshot();

        // Child is now hidden.

        mCapture->expectColor(Rect(20, 20, 52, 52), Color::RED);

    }

}



TEST_F(ChildLayerTest, ChildrenInheritNonTransformScalingFromParent) {

    asTransaction([&](Transaction& t) {

        t.show(mChild);