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

Commit 8724653a authored by Robert Carr's avatar Robert Carr
Browse files

SurfaceControl life-cycle refactoring. 

We provide a method called "release" which allows the client
to drop its references without reparenting the SurfaceControl
to null as the destructor would. The Java and NDK API's already
have this method but it's not perfectly functional at the moment.
Additionally we remove the strange pass-through of clear to destroy
and expose destroy directly.

Test: Builds
Bug: 123587983
Change-Id: Ia89ada1476daef97e6f30d50a57065c3636a6489
parent 0e875399

libs/gui/SurfaceControl.cpp

+7 −16
Original line number Diff line number Diff line
@@ -66,10 +66,7 @@ SurfaceControl::~SurfaceControl() 
    if (mClient != nullptr && mHandle != nullptr && mOwned) {

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

    }

    mClient.clear();

    mHandle.clear();

    mGraphicBufferProducer.clear();

    IPCThreadState::self()->flushCommands();

    release();

}



void SurfaceControl::destroy()
@@ -79,7 +76,12 @@ void SurfaceControl::destroy() 
    if (isValid() && mOwned) {

        SurfaceComposerClient::Transaction().reparent(this, nullptr).apply();

    }

    // clear all references and trigger an IPC now, to make sure things

    release();

}



void SurfaceControl::release()

{

    // Trigger an IPC now, to make sure things

    // happen without delay, since these resources are quite heavy.

    mClient.clear();

    mHandle.clear();
@@ -87,17 +89,6 @@ void SurfaceControl::destroy() 
    IPCThreadState::self()->flushCommands();

}



void SurfaceControl::clear()

{

    // here, the window manager tells us explicitly that we should destroy

    // the surface's resource. Soon after this call, it will also release

    // its last reference (which will call the dtor); however, it is possible

    // that a client living in the same process still holds references which

    // would delay the call to the dtor -- that is why we need this explicit

    // "clear()" call.

    destroy();

}



void SurfaceControl::disconnect() {

    if (mGraphicBufferProducer != nullptr) {

        mGraphicBufferProducer->disconnect(

libs/gui/include/gui/SurfaceControl.h

+6 −3
Original line number Diff line number Diff line
@@ -58,8 +58,12 @@ public: 
    static bool isSameSurface(

            const sp<SurfaceControl>& lhs, const sp<SurfaceControl>& rhs);



    // release surface data from java

    void        clear();

    // Release the handles assosciated with the SurfaceControl, without reparenting

    // them off-screen. At the moment if this isn't executed before ~SurfaceControl

    // is called then the destructor will reparent the layer off-screen for you.

    void        release();

    // Reparent off-screen and release. This is invoked by the destructor.

    void destroy();



    // disconnect any api that's connected

    void        disconnect();
@@ -98,7 +102,6 @@ private: 


    sp<Surface> generateSurfaceLocked() const;

    status_t validate() const;

    void destroy();



    sp<SurfaceComposerClient>   mClient;

    sp<IBinder>                 mHandle;

services/surfaceflinger/tests/Stress_test.cpp

+1 −1
Original line number Diff line number Diff line
@@ -34,7 +34,7 @@ TEST(SurfaceFlingerStress, create_and_destroy) { 
            auto surf = client->createSurface(String8("t"), 100, 100,

                    PIXEL_FORMAT_RGBA_8888, 0);

            ASSERT_TRUE(surf != nullptr);

            surf->clear();

            surf.clear();

        }

    };

services/surfaceflinger/tests/Transaction_test.cpp

+3 −3
Original line number Diff line number Diff line
@@ -1137,7 +1137,7 @@ TEST_P(LayerTypeAndRenderTypeTransactionTest, SetRelativeZBug64572777) { 
            .setRelativeLayer(layerG, layerR->getHandle(), 1)

            .apply();



    layerG->clear();

    layerG.clear();

    // layerG should have been removed

    getScreenCapture()->expectColor(Rect(0, 0, 32, 32), Color::RED);

}
@@ -4360,7 +4360,7 @@ TEST_F(ChildLayerTest, ChildrenSurviveParentDestruction) { 
        mCapture->checkPixel(64, 64, 111, 111, 111);

    }



    mChild->clear();

    mChild.clear();



    {

        SCOPED_TRACE("After destroying child");
@@ -5232,7 +5232,7 @@ TEST_F(ScreenCaptureTest, CaptureInvalidLayer) { 
    ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60));



    auto redLayerHandle = redLayer->getHandle();

    redLayer->clear();

    redLayer.clear();

    SurfaceComposerClient::Transaction().apply(true);



    sp<GraphicBuffer> outBuffer;