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

Commit 1684c708 authored by Alec Mouri's avatar Alec Mouri
Browse files

Only cache framebuffers for internal displays for drawLayers. 

Currently SkiaGlRenderEngine leaks memory when virtual displays are
created and destroyed because their output buffers are cached into a
texture and then never evicted.

The simplest workaround which is implemented in this patch is to never
cache framebuffers for virtual displays; we only cache when driving
internal displays.

On phones, there is usually one primary internal display and that
display is never destroyed, so this assumption is safe for the majority
of phone form factors. In the very near future we should remove this
assumption by changing the RenderEngine interface to take in a struct
that manages GPU textures on creation and destruction so that
SurfaceFlinger can maintain its own cached set, but that change is
longer than a few lines of code, which motivates this patch.

Bug: 178539829
Test: builds, boots
Change-Id: I8e501d297090b6f5056a0b0ead598f85bb4b9f07
parent e5d7b701

services/surfaceflinger/CompositionEngine/src/Output.cpp

+8 −1
Original line number Diff line number Diff line
@@ -989,9 +989,16 @@ std::optional<base::unique_fd> Output::composeSurfaces( 
                   });



    const nsecs_t renderEngineStart = systemTime();

    // Only use the framebuffer cache when rendering to an internal display

    // TODO(b/173560331): This is only to help mitigate memory leaks from virtual displays because

    // right now we don't have a concrete eviction policy for output buffers: GLESRenderEngine

    // bounds its framebuffer cache but Skia RenderEngine has no current policy. The best fix is

    // probably to encapsulate the output buffer into a structure that dispatches resource cleanup

    // over to RenderEngine, in which case this flag can be removed from the drawLayers interface.

    const bool useFramebufferCache = outputState.layerStackInternal;

    status_t status =

            renderEngine.drawLayers(clientCompositionDisplay, clientCompositionLayerPointers, buf,

                                    /*useFramebufferCache=*/true, std::move(fd), &readyFence);

                                    useFramebufferCache, std::move(fd), &readyFence);



    if (status != NO_ERROR && mClientCompositionRequestCache) {

        // If rendering was not successful, remove the request from the cache.

services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp

+41 −11
Original line number Diff line number Diff line
@@ -3003,7 +3003,7 @@ TEST_F(OutputComposeSurfacesTest, handlesZeroCompositionRequests) { 
            .WillRepeatedly(Return());



    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillRepeatedly(Return(mOutputBuffer));

    EXPECT_CALL(mRenderEngine, drawLayers(_, IsEmpty(), _, true, _, _))

    EXPECT_CALL(mRenderEngine, drawLayers(_, IsEmpty(), _, false, _, _))

            .WillRepeatedly(Return(NO_ERROR));



    verify().execute().expectAFenceWasReturned();
@@ -3016,6 +3016,36 @@ TEST_F(OutputComposeSurfacesTest, buildsAndRendersRequestList) { 
    r1.geometry.boundaries = FloatRect{1, 2, 3, 4};

    r2.geometry.boundaries = FloatRect{5, 6, 7, 8};



    EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));

    EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));

    EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));

    EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));

    EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))

            .WillRepeatedly(Return(std::vector<LayerFE::LayerSettings>{r1}));

    EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))

            .WillRepeatedly(

                    Invoke([&](const Region&,

                               std::vector<LayerFE::LayerSettings>& clientCompositionLayers) {

                        clientCompositionLayers.emplace_back(r2);

                    }));



    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillRepeatedly(Return(mOutputBuffer));

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, false, _, _))

            .WillRepeatedly(Return(NO_ERROR));



    verify().execute().expectAFenceWasReturned();

}



TEST_F(OutputComposeSurfacesTest,

       buildsAndRendersRequestListAndCachesFramebufferForInternalLayers) {

    LayerFE::LayerSettings r1;

    LayerFE::LayerSettings r2;



    r1.geometry.boundaries = FloatRect{1, 2, 3, 4};

    r2.geometry.boundaries = FloatRect{5, 6, 7, 8};

    const constexpr uint32_t kInternalLayerStack = 1234;

    mOutput.setLayerStackFilter(kInternalLayerStack, true);



    EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));

    EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));

    EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
@@ -3054,7 +3084,7 @@ TEST_F(OutputComposeSurfacesTest, renderDuplicateClientCompositionRequestsWithou 
            .WillRepeatedly(Return());



    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillRepeatedly(Return(mOutputBuffer));

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, true, _, _))

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, false, _, _))

            .Times(2)

            .WillOnce(Return(NO_ERROR));
@@ -3083,7 +3113,7 @@ TEST_F(OutputComposeSurfacesTest, skipDuplicateClientCompositionRequests) { 
            .WillRepeatedly(Return());



    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillRepeatedly(Return(mOutputBuffer));

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, true, _, _))

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, false, _, _))

            .WillOnce(Return(NO_ERROR));

    EXPECT_CALL(mOutput, setExpensiveRenderingExpected(false));
@@ -3115,7 +3145,7 @@ TEST_F(OutputComposeSurfacesTest, clientCompositionIfBufferChanges) { 
    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_))

            .WillOnce(Return(mOutputBuffer))

            .WillOnce(Return(otherOutputBuffer));

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, true, _, _))

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, false, _, _))

            .WillRepeatedly(Return(NO_ERROR));



    verify().execute().expectAFenceWasReturned();
@@ -3145,9 +3175,9 @@ TEST_F(OutputComposeSurfacesTest, clientCompositionIfRequestChanges) { 
            .WillRepeatedly(Return());



    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillRepeatedly(Return(mOutputBuffer));

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, true, _, _))

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r2)), _, false, _, _))

            .WillOnce(Return(NO_ERROR));

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r3)), _, true, _, _))

    EXPECT_CALL(mRenderEngine, drawLayers(_, ElementsAre(Pointee(r1), Pointee(r3)), _, false, _, _))

            .WillOnce(Return(NO_ERROR));



    verify().execute().expectAFenceWasReturned();
@@ -3197,7 +3227,7 @@ struct OutputComposeSurfacesTest_UsesExpectedDisplaySettings : public OutputComp 
    struct ExpectDisplaySettingsState

          : public CallOrderStateMachineHelper<TestType, ExpectDisplaySettingsState> {

        auto thenExpectDisplaySettingsUsed(renderengine::DisplaySettings settings) {

            EXPECT_CALL(getInstance()->mRenderEngine, drawLayers(settings, _, _, true, _, _))

            EXPECT_CALL(getInstance()->mRenderEngine, drawLayers(settings, _, _, false, _, _))

                    .WillOnce(Return(NO_ERROR));

            return nextState<ExecuteState>();

        }
@@ -3296,7 +3326,7 @@ struct OutputComposeSurfacesTest_HandlesProtectedContent : public OutputComposeS 
        EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))

                .WillRepeatedly(Return());

        EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillRepeatedly(Return(mOutputBuffer));

        EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, true, _, _))

        EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, false, _, _))

                .WillRepeatedly(Return(NO_ERROR));

    }
@@ -3349,7 +3379,7 @@ TEST_F(OutputComposeSurfacesTest_HandlesProtectedContent, ifNotEnabled) { 
    EXPECT_CALL(*mRenderSurface, setProtected(true));

    // Must happen after setting the protected content state.

    EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillRepeatedly(Return(mOutputBuffer));

    EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, true, _, _)).WillOnce(Return(NO_ERROR));

    EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, false, _, _)).WillOnce(Return(NO_ERROR));



    mOutput.composeSurfaces(kDebugRegion, kDefaultRefreshArgs);

}
@@ -3419,7 +3449,7 @@ TEST_F(OutputComposeSurfacesTest_SetsExpensiveRendering, IfExepensiveOutputDatas 
    InSequence seq;



    EXPECT_CALL(mOutput, setExpensiveRenderingExpected(true));

    EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, true, _, _)).WillOnce(Return(NO_ERROR));

    EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, false, _, _)).WillOnce(Return(NO_ERROR));



    mOutput.composeSurfaces(kDebugRegion, kDefaultRefreshArgs);

}
@@ -3434,7 +3464,7 @@ struct OutputComposeSurfacesTest_SetsExpensiveRendering_ForBlur 
        EXPECT_CALL(mLayer.outputLayer, writeStateToHWC(false));

        EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))

                .WillOnce(Return(std::vector<LayerFE::LayerSettings>{}));

        EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, true, _, _)).WillOnce(Return(NO_ERROR));

        EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, false, _, _)).WillOnce(Return(NO_ERROR));

        EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(1u));

        EXPECT_CALL(mOutput, getOutputLayerOrderedByZByIndex(0u))

                .WillRepeatedly(Return(&mLayer.outputLayer));