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

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

Only attach descendants if parent was detached 

If a layer is reparented, we also attach the children. We traverse down
the heirarchy and attach all descendants. However, we really only want
to attach descendants if they were initially detached because the layer
that's getting reparented detached them.

For example, Child called detachChildren and its Grandchild was
detached. Parent calls reparent. We want to attach Child if it was
detached and then proceed down its children. If Child wasn't detached,
we don't want to attach Grandchild since the layer that detached it
hasn't been reparented.

As cleanup, also moved DetachChildren test to their own file

Test: DetachChildren.ReparentParentLayerOfDetachedChildren
Change-Id: I53b6d9cb165b810e9c55da8a9ba86c7b53a13c0e
parent c6a2f3a4

services/surfaceflinger/Layer.cpp

+2 −2
Original line number Diff line number Diff line
@@ -1820,7 +1820,7 @@ bool Layer::reparent(const sp<IBinder>& newParentHandle) { 
        onRemovedFromCurrentState();

    }



    if (callSetTransactionFlags || attachChildren()) {

    if (attachChildren() || callSetTransactionFlags) {

        setTransactionFlags(eTransactionNeeded);

    }

    return true;
@@ -1848,9 +1848,9 @@ bool Layer::attachChildren() { 
        if (client != nullptr && parentClient != client) {

            if (child->mLayerDetached) {

                child->mLayerDetached = false;

                child->attachChildren();

                changed = true;

            }

            changed |= child->attachChildren();

        }

    }

services/surfaceflinger/tests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ cc_test { 
        "CommonTypes_test.cpp",

        "Credentials_test.cpp",

        "DereferenceSurfaceControl_test.cpp",

        "DetachChildren_test.cpp",

        "DisplayConfigs_test.cpp",

        "EffectLayer_test.cpp",

        "InvalidHandles_test.cpp",

services/surfaceflinger/tests/DetachChildren_test.cpp

0 → 100644
+322 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2020 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#pragma clang diagnostic push

#pragma clang diagnostic ignored "-Wconversion"



#include "LayerTransactionTest.h"



namespace android {



class DetachChildren : public LayerTransactionTest {

protected:

    virtual void SetUp() {

        LayerTransactionTest::SetUp();



        mMainSurface = createLayer(String8("Main Test Surface"), mMainSurfaceBounds.width(),

                                   mMainSurfaceBounds.height(), 0, mBlackBgSurface.get());



        ASSERT_TRUE(mMainSurface != nullptr);

        ASSERT_TRUE(mMainSurface->isValid());



        TransactionUtils::fillSurfaceRGBA8(mMainSurface, mMainSurfaceColor);



        asTransaction([&](Transaction& t) {

            t.setLayer(mMainSurface, INT32_MAX - 1)

                    .setPosition(mMainSurface, mMainSurfaceBounds.left, mMainSurfaceBounds.top)

                    .show(mMainSurface);

        });

    }



    virtual void TearDown() {

        LayerTransactionTest::TearDown();

        mMainSurface = 0;

    }



    sp<SurfaceControl> mMainSurface;

    Color mMainSurfaceColor = {195, 63, 63, 255};

    Rect mMainSurfaceBounds = Rect(64, 64, 128, 128);

    std::unique_ptr<ScreenCapture> mCapture;

};



TEST_F(DetachChildren, RelativesAreNotDetached) {

    Color relativeColor = {10, 10, 10, 255};

    Rect relBounds = Rect(64, 64, 74, 74);



    sp<SurfaceControl> relative =

            createLayer(String8("relativeTestSurface"), relBounds.width(), relBounds.height(), 0);

    TransactionUtils::fillSurfaceRGBA8(relative, relativeColor);



    Transaction{}

            .setRelativeLayer(relative, mMainSurface->getHandle(), 1)

            .setPosition(relative, relBounds.left, relBounds.top)

            .apply();



    {

        // The relative should be on top of the FG control.

        mCapture = screenshot();

        mCapture->expectColor(relBounds, relativeColor);

    }

    Transaction{}.detachChildren(mMainSurface).apply();



    {

        // Nothing should change at this point.

        mCapture = screenshot();

        mCapture->expectColor(relBounds, relativeColor);

    }



    Transaction{}.hide(relative).apply();



    {

        // Ensure that the relative was actually hidden, rather than

        // being left in the detached but visible state.

        mCapture = screenshot();

        mCapture->expectColor(mMainSurfaceBounds, mMainSurfaceColor);

    }

}



TEST_F(DetachChildren, DetachChildrenSameClient) {

    Color childColor = {200, 200, 200, 255};

    Rect childBounds = Rect(74, 74, 84, 84);

    sp<SurfaceControl> child = createLayer(String8("Child surface"), childBounds.width(),

                                           childBounds.height(), 0, mMainSurface.get());

    ASSERT_TRUE(child->isValid());



    TransactionUtils::fillSurfaceRGBA8(child, childColor);



    asTransaction([&](Transaction& t) {

        t.show(child);

        t.setPosition(child, childBounds.left - mMainSurfaceBounds.left,

                      childBounds.top - mMainSurfaceBounds.top);

    });



    {

        mCapture = screenshot();

        // Expect main color around the child surface

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectColor(childBounds, childColor);

    }



    asTransaction([&](Transaction& t) { t.detachChildren(mMainSurface); });



    asTransaction([&](Transaction& t) { t.hide(child); });



    // Since the child has the same client as the parent, it will not get

    // detached and will be hidden.

    {

        mCapture = screenshot();

        mCapture->expectColor(mMainSurfaceBounds, mMainSurfaceColor);

    }

}



TEST_F(DetachChildren, DetachChildrenDifferentClient) {

    Color childColor = {200, 200, 200, 255};

    Rect childBounds = Rect(74, 74, 84, 84);



    sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;

    sp<SurfaceControl> childNewClient =

            createSurface(newComposerClient, "New Child Test Surface", childBounds.width(),

                          childBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());

    ASSERT_TRUE(childNewClient->isValid());



    TransactionUtils::fillSurfaceRGBA8(childNewClient, childColor);



    asTransaction([&](Transaction& t) {

        t.show(childNewClient);

        t.setPosition(childNewClient, childBounds.left - mMainSurfaceBounds.left,

                      childBounds.top - mMainSurfaceBounds.top);

    });



    {

        mCapture = screenshot();

        // Expect main color around the child surface

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectColor(childBounds, childColor);

    }



    asTransaction([&](Transaction& t) { t.detachChildren(mMainSurface); });



    asTransaction([&](Transaction& t) { t.hide(childNewClient); });



    // Nothing should have changed.

    {

        mCapture = screenshot();

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectColor(childBounds, childColor);

    }

}



TEST_F(DetachChildren, DetachChildrenThenAttach) {

    Color childColor = {200, 200, 200, 255};

    Rect childBounds = Rect(74, 74, 84, 84);



    sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;

    sp<SurfaceControl> childNewClient =

            createSurface(newComposerClient, "New Child Test Surface", childBounds.width(),

                          childBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());

    ASSERT_TRUE(childNewClient->isValid());



    TransactionUtils::fillSurfaceRGBA8(childNewClient, childColor);



    Transaction()

            .show(childNewClient)

            .setPosition(childNewClient, childBounds.left - mMainSurfaceBounds.left,

                         childBounds.top - mMainSurfaceBounds.top)

            .apply();



    {

        mCapture = screenshot();

        // Expect main color around the child surface

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectColor(childBounds, childColor);

    }



    Transaction().detachChildren(mMainSurface).apply();

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



    // Nothing should have changed.

    {

        mCapture = screenshot();

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectColor(childBounds, childColor);

    }



    Color newParentColor = Color::RED;

    Rect newParentBounds = Rect(20, 20, 52, 52);

    sp<SurfaceControl> newParentSurface =

            createLayer(String8("New Parent Surface"), newParentBounds.width(),

                        newParentBounds.height(), 0);

    TransactionUtils::fillSurfaceRGBA8(newParentSurface, newParentColor);

    Transaction()

            .setLayer(newParentSurface, INT32_MAX - 1)

            .show(newParentSurface)

            .setPosition(newParentSurface, newParentBounds.left, newParentBounds.top)

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

            .apply();

    {

        mCapture = screenshot();

        // Child is now hidden.

        mCapture->expectColor(newParentBounds, newParentColor);

    }

}



TEST_F(DetachChildren, DetachChildrenWithDeferredTransaction) {

    Color childColor = {200, 200, 200, 255};

    Rect childBounds = Rect(74, 74, 84, 84);



    sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;

    sp<SurfaceControl> childNewClient =

            createSurface(newComposerClient, "New Child Test Surface", childBounds.width(),

                          childBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());

    ASSERT_TRUE(childNewClient->isValid());



    TransactionUtils::fillSurfaceRGBA8(childNewClient, childColor);



    Transaction()

            .show(childNewClient)

            .setPosition(childNewClient, childBounds.left - mMainSurfaceBounds.left,

                         childBounds.top - mMainSurfaceBounds.top)

            .apply();



    {

        mCapture = screenshot();

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectColor(childBounds, childColor);

    }



    Transaction()

            .deferTransactionUntil_legacy(childNewClient, mMainSurface->getHandle(),

                                          mMainSurface->getSurface()->getNextFrameNumber())

            .apply();

    Transaction().detachChildren(mMainSurface).apply();

    ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(mMainSurface, Color::RED,

                                                      mMainSurfaceBounds.width(),

                                                      mMainSurfaceBounds.height()));



    // BufferLayer can still dequeue buffers even though there's a detached layer with a

    // deferred transaction.

    {

        SCOPED_TRACE("new buffer");

        mCapture = screenshot();

        mCapture->expectBorder(childBounds, Color::RED);

        mCapture->expectColor(childBounds, childColor);

    }

}



TEST_F(DetachChildren, ReparentParentLayerOfDetachedChildren) {

    Color childColor = {200, 200, 200, 255};

    Rect childBounds = Rect(74, 74, 94, 94);

    Color grandchildColor = Color::RED;

    Rect grandchildBounds = Rect(80, 80, 90, 90);



    sp<SurfaceComposerClient> newClient1 = new SurfaceComposerClient;

    sp<SurfaceComposerClient> newClient2 = new SurfaceComposerClient;



    sp<SurfaceControl> childSurface =

            createSurface(newClient1, "Child surface", childBounds.width(), childBounds.height(),

                          PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());

    sp<SurfaceControl> grandchildSurface =

            createSurface(newClient2, "Grandchild Surface", grandchildBounds.width(),

                          grandchildBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, childSurface.get());



    TransactionUtils::fillSurfaceRGBA8(childSurface, childColor);

    TransactionUtils::fillSurfaceRGBA8(grandchildSurface, grandchildColor);



    Transaction()

            .show(childSurface)

            .show(grandchildSurface)

            .setPosition(childSurface, childBounds.left - mMainSurfaceBounds.left,

                         childBounds.top - mMainSurfaceBounds.top)

            .setPosition(grandchildSurface, grandchildBounds.left - childBounds.left,

                         grandchildBounds.top - childBounds.top)

            .apply();



    {

        mCapture = screenshot();

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectBorder(grandchildBounds, childColor);

        mCapture->expectColor(grandchildBounds, grandchildColor);

    }



    Transaction().detachChildren(childSurface).apply();



    // Remove main surface offscreen

    Transaction().reparent(mMainSurface, nullptr).apply();

    {

        mCapture = screenshot();

        mCapture->expectColor(mMainSurfaceBounds, Color::BLACK);

    }



    Transaction().reparent(mMainSurface, mBlackBgSurface->getHandle()).apply();

    {

        mCapture = screenshot();

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectBorder(grandchildBounds, childColor);

        mCapture->expectColor(grandchildBounds, grandchildColor);

    }



    Transaction().hide(grandchildSurface).apply();



    // grandchild is still detached so it will not hide

    {

        mCapture = screenshot();

        mCapture->expectBorder(childBounds, mMainSurfaceColor);

        mCapture->expectBorder(grandchildBounds, childColor);

        mCapture->expectColor(grandchildBounds, grandchildColor);

    }

}



} // namespace android
 
 No newline at end of file

services/surfaceflinger/tests/LayerUpdate_test.cpp

+0 −203
Original line number Diff line number Diff line
@@ -103,41 +103,6 @@ protected: 
    sp<SurfaceControl> mSyncSurfaceControl;

};



TEST_F(LayerUpdateTest, RelativesAreNotDetached) {

    std::unique_ptr<ScreenCapture> sc;



    sp<SurfaceControl> relative = createLayer(String8("relativeTestSurface"), 10, 10, 0);

    TransactionUtils::fillSurfaceRGBA8(relative, 10, 10, 10);

    waitForPostedBuffers();



    Transaction{}

            .setRelativeLayer(relative, mFGSurfaceControl->getHandle(), 1)

            .setPosition(relative, 64, 64)

            .apply();



    {

        // The relative should be on top of the FG control.

        ScreenCapture::captureScreen(&sc);

        sc->checkPixel(64, 64, 10, 10, 10);

    }

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



    {

        // Nothing should change at this point.

        ScreenCapture::captureScreen(&sc);

        sc->checkPixel(64, 64, 10, 10, 10);

    }



    Transaction{}.hide(relative).apply();



    {

        // Ensure that the relative was actually hidden, rather than

        // being left in the detached but visible state.

        ScreenCapture::captureScreen(&sc);

        sc->expectFGColor(64, 64);

    }

}



class GeometryLatchingTest : public LayerUpdateTest {

protected:

    void EXPECT_INITIAL_STATE(const char* trace) {
@@ -588,174 +553,6 @@ TEST_F(ChildLayerTest, ChildrenRelativeZSurvivesParentDestruction) { 
    }

}



TEST_F(ChildLayerTest, DetachChildrenSameClient) {

    asTransaction([&](Transaction& t) {

        t.show(mChild);

        t.setPosition(mChild, 10, 10);

        t.setPosition(mFGSurfaceControl, 64, 64);

    });



    {

        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);

    }



    asTransaction([&](Transaction& t) { t.detachChildren(mFGSurfaceControl); });



    asTransaction([&](Transaction& t) { t.hide(mChild); });



    // Since the child has the same client as the parent, it will not get

    // detached and will be hidden.

    {

        mCapture = screenshot();

        mCapture->expectFGColor(64, 64);

        mCapture->expectFGColor(74, 74);

        mCapture->expectFGColor(84, 84);

    }

}



TEST_F(ChildLayerTest, DetachChildrenDifferentClient) {

    sp<SurfaceComposerClient> mNewComposerClient = new SurfaceComposerClient;

    sp<SurfaceControl> mChildNewClient =

            createSurface(mNewComposerClient, "New Child Test Surface", 10, 10,

                          PIXEL_FORMAT_RGBA_8888, 0, mFGSurfaceControl.get());



    ASSERT_TRUE(mChildNewClient->isValid());



    TransactionUtils::fillSurfaceRGBA8(mChildNewClient, 200, 200, 200);



    asTransaction([&](Transaction& t) {

        t.hide(mChild);

        t.show(mChildNewClient);

        t.setPosition(mChildNewClient, 10, 10);

        t.setPosition(mFGSurfaceControl, 64, 64);

    });



    {

        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);

    }



    asTransaction([&](Transaction& t) { t.detachChildren(mFGSurfaceControl); });



    asTransaction([&](Transaction& t) { t.hide(mChildNewClient); });



    // Nothing should have changed.

    {

        mCapture = screenshot();

        mCapture->expectFGColor(64, 64);

        mCapture->expectChildColor(74, 74);

        mCapture->expectFGColor(84, 84);

    }

}



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());



    TransactionUtils::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, DetachChildrenWithDeferredTransaction) {

    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());



    TransactionUtils::fillSurfaceRGBA8(childNewClient, 200, 200, 200);



    Transaction()

            .hide(mChild)

            .show(childNewClient)

            .setPosition(childNewClient, 10, 10)

            .setPosition(mFGSurfaceControl, 64, 64)

            .apply();



    {

        mCapture = screenshot();

        Rect rect = Rect(74, 74, 84, 84);

        mCapture->expectBorder(rect, Color{195, 63, 63, 255});

        mCapture->expectColor(rect, Color{200, 200, 200, 255});

    }



    Transaction()

            .deferTransactionUntil_legacy(childNewClient, mFGSurfaceControl->getHandle(),

                                          mFGSurfaceControl->getSurface()->getNextFrameNumber())

            .apply();

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

    ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(mFGSurfaceControl, Color::RED, 32, 32));



    // BufferLayer can still dequeue buffers even though there's a detached layer with a

    // deferred transaction.

    {

        SCOPED_TRACE("new buffer");

        mCapture = screenshot();

        Rect rect = Rect(74, 74, 84, 84);

        mCapture->expectBorder(rect, Color::RED);

        mCapture->expectColor(rect, Color{200, 200, 200, 255});

    }

}



TEST_F(ChildLayerTest, ChildrenInheritNonTransformScalingFromParent) {

    asTransaction([&](Transaction& t) {

        t.show(mChild);