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

Commit d11eb6bd authored by Adithya Srinivasan's avatar Adithya Srinivasan
Browse files

Dropping SurfaceFrames from Drawing State 

Our assumption is that a buffer committed into drawing state will be
latched and presented. Based on this assumption, we present a
BufferSurfaceFrame only at latch time instead of commit time. However,
during back to back invalidates, there is a chance that the first
committed buffer is not latched. It could be replaced by another buffer
in the second invalidate. This leads to the SurfaceFrame getting stuck
in the pending classification list. To prevent this, have a check in
commitTransaction that drops the SurfaceFrame properly if it hasn't been
presented before.

Back to back invalidate can currently be triggered by just flashing a
clean build and launching the dialer.

Bug: 182214053
Test: libsurfaceflinger_unittest
Change-Id: I6cd9c4cbfb2ca50b96654ed240664758dad86f19
parent 785adddc

services/surfaceflinger/Layer.cpp

+9 −0
Original line number Diff line number Diff line
@@ -1058,6 +1058,15 @@ uint32_t Layer::doTransaction(uint32_t flags) { 
}



void Layer::commitTransaction(State& stateToCommit) {

    if (auto& bufferSurfaceFrame = mDrawingState.bufferSurfaceFrameTX;

        mDrawingState.buffer != stateToCommit.buffer && bufferSurfaceFrame != nullptr &&

        bufferSurfaceFrame->getPresentState() != PresentState::Presented) {

        // If the previous buffer was committed but not latched (refreshPending - happens during

        // back to back invalidates), it gets silently dropped here. Mark the corresponding

        // SurfaceFrame as dropped to prevent it from getting stuck in the pending classification

        // list.

        addSurfaceFrameDroppedForBuffer(bufferSurfaceFrame);

    }

    mDrawingState = stateToCommit;



    // Set the present state for all bufferlessSurfaceFramesTX to Presented. The

services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp

+66 −0
Original line number Diff line number Diff line
@@ -439,6 +439,68 @@ public: 
        EXPECT_EQ(true, presentedSurfaceFrame->getIsBuffer());

        EXPECT_EQ(PresentState::Presented, presentedSurfaceFrame->getPresentState());

    }



    void MultipleCommitsBeforeLatch() {

        sp<BufferStateLayer> layer = createBufferStateLayer();

        uint32_t surfaceFramesPendingClassification = 0;

        std::vector<std::shared_ptr<frametimeline::SurfaceFrame>> bufferlessSurfaceFrames;

        for (int i = 0; i < 10; i += 2) {

            sp<Fence> fence1(new Fence());

            sp<GraphicBuffer> buffer1{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)};

            layer->setBuffer(buffer1, fence1, 10, 20, false, mClientCache, 1, std::nullopt,

                             {/*vsyncId*/ 1, /*inputEventId*/ 0});

            layer->setFrameTimelineVsyncForBufferlessTransaction({/*vsyncId*/ 2,

                                                                  /*inputEventId*/ 0},

                                                                 10);

            ASSERT_NE(nullptr, layer->mCurrentState.bufferSurfaceFrameTX);

            EXPECT_EQ(1u, layer->mCurrentState.bufferlessSurfaceFramesTX.size());

            auto& bufferlessSurfaceFrame =

                    layer->mCurrentState.bufferlessSurfaceFramesTX.at(/*vsyncId*/ 2);

            bufferlessSurfaceFrames.push_back(bufferlessSurfaceFrame);



            commitTransaction(layer.get());

            surfaceFramesPendingClassification += 2;

            EXPECT_EQ(surfaceFramesPendingClassification,

                      layer->mPendingJankClassifications.size());

        }



        auto presentedBufferSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX;

        bool computeVisisbleRegions;

        layer->updateTexImage(computeVisisbleRegions, 15, 0);

        // BufferlessSurfaceFrames are immediately set to presented and added to the DisplayFrame.

        // Since we don't have access to DisplayFrame here, trigger an onPresent directly.

        for (auto& surfaceFrame : bufferlessSurfaceFrames) {

            surfaceFrame->onPresent(20, JankType::None, Fps::fromPeriodNsecs(11),

                                    /*displayDeadlineDelta*/ 0, /*displayPresentDelta*/ 0);

        }

        presentedBufferSurfaceFrame->onPresent(20, JankType::None, Fps::fromPeriodNsecs(11),

                                               /*displayDeadlineDelta*/ 0,

                                               /*displayPresentDelta*/ 0);



        // There should be 10 bufferlessSurfaceFrames and 1 bufferSurfaceFrame

        ASSERT_EQ(10u, surfaceFramesPendingClassification);

        ASSERT_EQ(surfaceFramesPendingClassification, layer->mPendingJankClassifications.size());



        // For the frames upto 8, the bufferSurfaceFrame should have been dropped while the

        // bufferlessSurfaceFrame presented

        for (uint32_t i = 0; i < 8; i += 2) {

            auto& bufferSurfaceFrame = layer->mPendingJankClassifications[i];

            auto& bufferlessSurfaceFrame = layer->mPendingJankClassifications[i + 1];

            EXPECT_EQ(bufferSurfaceFrame->getPresentState(), PresentState::Dropped);

            EXPECT_EQ(bufferlessSurfaceFrame->getPresentState(), PresentState::Presented);

        }

        {

            auto& bufferSurfaceFrame = layer->mPendingJankClassifications[8u];

            auto& bufferlessSurfaceFrame = layer->mPendingJankClassifications[9u];

            EXPECT_EQ(bufferSurfaceFrame->getPresentState(), PresentState::Presented);

            EXPECT_EQ(bufferlessSurfaceFrame->getPresentState(), PresentState::Presented);

        }



        layer->releasePendingBuffer(25);



        // There shouldn't be any pending classifications. Everything should have been cleared.

        EXPECT_EQ(0u, layer->mPendingJankClassifications.size());

    }

};



TEST_F(TransactionSurfaceFrameTest, PresentedBufferlessSurfaceFrame) {
@@ -484,4 +546,8 @@ TEST_F(TransactionSurfaceFrameTest, 
    BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer();

}



TEST_F(TransactionSurfaceFrameTest, MultipleCommitsBeforeLatch) {

    MultipleCommitsBeforeLatch();

}



} // namespace android
 
 No newline at end of file