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

Commit a722b816 authored by Chavi Weingarten's avatar Chavi Weingarten
Browse files

Don't queue buffer if nothing new to draw in Display 

The current logic only checks if dirty is not empty, but doesn't account
for no content changing. Instead use the same logic that's in
beginFrame to compute mustRecompute and use that when determining if the
display should queue a buffer.

This fixes an issue where a Displays first few frames could be black
until the content is loaded.

Bug: 239888440
Test: First frame from screenrecord is no longer black
Change-Id: Ibb568302bf2d50db167a1f9ca76a0f37e2c65bf5
(cherry picked from commit 09fa1d66)
parent 293041ca

services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h

+5 −0
Original line number Diff line number Diff line
@@ -152,6 +152,8 @@ protected: 
    virtual const compositionengine::CompositionEngine& getCompositionEngine() const = 0;

    virtual void dumpState(std::string& out) const = 0;



    bool mustRecompose() const;



private:

    void dirtyEntireOutput();

    compositionengine::OutputLayer* findLayerRequestingBackgroundComposition() const;
@@ -170,6 +172,9 @@ private: 
    std::unique_ptr<ClientCompositionRequestCache> mClientCompositionRequestCache;

    std::unique_ptr<planner::Planner> mPlanner;

    std::unique_ptr<HwcAsyncWorker> mHwComposerAsyncWorker;



    // Whether the content must be recomposed this frame.

    bool mMustRecompose = false;

};



// This template factory function standardizes the implementation details of the

services/surfaceflinger/CompositionEngine/src/Display.cpp

+1 −1
Original line number Diff line number Diff line
@@ -426,7 +426,7 @@ void Display::finishFrame(const compositionengine::CompositionRefreshArgs& refre 
    // 1) It is being handled by hardware composer, which may need this to

    //    keep its virtual display state machine in sync, or

    // 2) There is work to be done (the dirty region isn't empty)

    if (GpuVirtualDisplayId::tryCast(mId) && getDirtyRegion().isEmpty()) {

    if (GpuVirtualDisplayId::tryCast(mId) && !mustRecompose()) {

        ALOGV("Skipping display composition");

        return;

    }

services/surfaceflinger/CompositionEngine/src/Output.cpp

+10 −6
Original line number Diff line number Diff line
@@ -977,17 +977,17 @@ void Output::beginFrame() { 
    //   frame, then nothing more until we get new layers.

    // - When a display is created with a private layer stack, we won't

    //   emit any black frames until a layer is added to the layer stack.

    const bool mustRecompose = dirty && !(empty && wasEmpty);

    mMustRecompose = dirty && !(empty && wasEmpty);



    const char flagPrefix[] = {'-', '+'};

    static_cast<void>(flagPrefix);

    ALOGV_IF("%s: %s composition for %s (%cdirty %cempty %cwasEmpty)", __FUNCTION__,

             mustRecompose ? "doing" : "skipping", getName().c_str(), flagPrefix[dirty],

    ALOGV("%s: %s composition for %s (%cdirty %cempty %cwasEmpty)", __func__,

          mMustRecompose ? "doing" : "skipping", getName().c_str(), flagPrefix[dirty],

          flagPrefix[empty], flagPrefix[wasEmpty]);



    mRenderSurface->beginFrame(mustRecompose);

    mRenderSurface->beginFrame(mMustRecompose);



    if (mustRecompose) {

    if (mMustRecompose) {

        outputState.lastCompositionHadVisibleLayers = !empty;

    }

}
@@ -1590,5 +1590,9 @@ void Output::finishPrepareFrame() { 
    mRenderSurface->prepareFrame(state.usesClientComposition, state.usesDeviceComposition);

}



bool Output::mustRecompose() const {

    return mMustRecompose;

}



} // namespace impl

} // namespace android::compositionengine

services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp

+29 −1
Original line number Diff line number Diff line
@@ -971,16 +971,40 @@ TEST_F(DisplayFinishFrameTest, skipsCompositionIfNotDirty) { 


    // We expect no calls to queueBuffer if composition was skipped.

    EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(0);

    EXPECT_CALL(*renderSurface, beginFrame(false));



    gpuDisplay->editState().isEnabled = true;

    gpuDisplay->editState().usesClientComposition = false;

    gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1));

    gpuDisplay->editState().dirtyRegion = Region::INVALID_REGION;

    gpuDisplay->editState().lastCompositionHadVisibleLayers = true;



    gpuDisplay->beginFrame();

    gpuDisplay->finishFrame({}, std::move(mResultWithoutBuffer));

}



TEST_F(DisplayFinishFrameTest, performsCompositionIfDirty) {

TEST_F(DisplayFinishFrameTest, skipsCompositionIfEmpty) {

    auto args = getDisplayCreationArgsForGpuVirtualDisplay();

    std::shared_ptr<impl::Display> gpuDisplay = impl::createDisplay(mCompositionEngine, args);



    mock::RenderSurface* renderSurface = new StrictMock<mock::RenderSurface>();

    gpuDisplay->setRenderSurfaceForTest(std::unique_ptr<RenderSurface>(renderSurface));



    // We expect no calls to queueBuffer if composition was skipped.

    EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(0);

    EXPECT_CALL(*renderSurface, beginFrame(false));



    gpuDisplay->editState().isEnabled = true;

    gpuDisplay->editState().usesClientComposition = false;

    gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1));

    gpuDisplay->editState().dirtyRegion = Region(Rect(0, 0, 1, 1));

    gpuDisplay->editState().lastCompositionHadVisibleLayers = false;



    gpuDisplay->beginFrame();

    gpuDisplay->finishFrame({}, std::move(mResultWithoutBuffer));

}



TEST_F(DisplayFinishFrameTest, performsCompositionIfDirtyAndNotEmpty) {

    auto args = getDisplayCreationArgsForGpuVirtualDisplay();

    std::shared_ptr<impl::Display> gpuDisplay = impl::createDisplay(mCompositionEngine, args);
@@ -989,11 +1013,15 @@ TEST_F(DisplayFinishFrameTest, performsCompositionIfDirty) { 


    // We expect a single call to queueBuffer when composition is not skipped.

    EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(1);

    EXPECT_CALL(*renderSurface, beginFrame(true));



    gpuDisplay->editState().isEnabled = true;

    gpuDisplay->editState().usesClientComposition = false;

    gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1));

    gpuDisplay->editState().dirtyRegion = Region(Rect(0, 0, 1, 1));

    gpuDisplay->editState().lastCompositionHadVisibleLayers = true;



    gpuDisplay->beginFrame();

    gpuDisplay->finishFrame({}, std::move(mResultWithBuffer));

}