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

Commit 891004e1 authored by Adithya Srinivasan's avatar Adithya Srinivasan
Browse files

Remove token invalid check for BufferSurfaceFrame 

Since we already check for token invalid cases while attributing jank
and tracing, we shouldn't perpetuate this to the creation of
SurfaceFrames. Doing so can also result in a case where a buffer is
changed but we don't attribute it to a SurfaceFrame. This change makes
it so that the State's buffer and BufferSurfaceFrame are tied together
and change together.

Bug: 179940619
Test: libsurfaceflinger_unittest
Change-Id: Id420e5924f162d703adc6831edebf4f100db12de
parent 8f751f89

services/surfaceflinger/BufferStateLayer.cpp

+1 −3
Original line number Original line Diff line number Diff line
@@ -370,9 +370,7 @@ bool BufferStateLayer::setBuffer(const sp<GraphicBuffer>& buffer, const sp<Fence 




    addFrameEvent(acquireFence, postTime, isAutoTimestamp ? 0 : desiredPresentTime);

    addFrameEvent(acquireFence, postTime, isAutoTimestamp ? 0 : desiredPresentTime);





    if (info.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) {

    setFrameTimelineVsyncForBufferTransaction(info, postTime);

    setFrameTimelineVsyncForBufferTransaction(info, postTime);

    }





    if (dequeueTime && *dequeueTime != 0) {

    if (dequeueTime && *dequeueTime != 0) {

        const uint64_t bufferId = buffer->getId();

        const uint64_t bufferId = buffer->getId();

services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp

+62 −0
Original line number Original line Diff line number Diff line
@@ -368,6 +368,62 @@ public: 




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

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

    }

    }



    void BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer() {

        sp<BufferStateLayer> layer = createBufferStateLayer();



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

        auto acquireFence1 = fenceFactory.createFenceTimeForTest(fence1);

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

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

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

        const auto droppedSurfaceFrame1 = layer->mCurrentState.bufferSurfaceFrameTX;



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

        auto acquireFence2 = fenceFactory.createFenceTimeForTest(fence2);

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

        auto dropStartTime1 = systemTime();

        layer->setBuffer(buffer2, fence2, 10, 20, false, mClientCache, 1, std::nullopt,

                         {/*vsyncId*/ FrameTimelineInfo::INVALID_VSYNC_ID, /*inputEventId*/ 0});

        auto dropEndTime1 = systemTime();

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

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

        const auto droppedSurfaceFrame2 = layer->mCurrentState.bufferSurfaceFrameTX;



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

        auto acquireFence3 = fenceFactory.createFenceTimeForTest(fence3);

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

        auto dropStartTime2 = systemTime();

        layer->setBuffer(buffer3, fence3, 10, 20, false, mClientCache, 1, std::nullopt,

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

        auto dropEndTime2 = systemTime();

        acquireFence3->signalForTest(12);



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

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

        const auto& presentedSurfaceFrame = layer->mCurrentState.bufferSurfaceFrameTX;



        commitTransaction(layer.get());

        bool computeVisisbleRegions;

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



        EXPECT_EQ(1, droppedSurfaceFrame1->getToken());

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

        EXPECT_EQ(0u, droppedSurfaceFrame1->getActuals().endTime);

        auto dropTime1 = droppedSurfaceFrame1->getDropTime();

        EXPECT_TRUE(dropTime1 > dropStartTime1 && dropTime1 < dropEndTime1);



        EXPECT_EQ(FrameTimelineInfo::INVALID_VSYNC_ID, droppedSurfaceFrame2->getToken());

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

        EXPECT_EQ(0u, droppedSurfaceFrame2->getActuals().endTime);

        auto dropTime2 = droppedSurfaceFrame2->getDropTime();

        EXPECT_TRUE(dropTime2 > dropStartTime2 && dropTime2 < dropEndTime2);



        EXPECT_EQ(2, presentedSurfaceFrame->getToken());

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

    }

};

};





TEST_F(TransactionSurfaceFrameTest, PresentedBufferlessSurfaceFrame) {

TEST_F(TransactionSurfaceFrameTest, PresentedBufferlessSurfaceFrame) {
@@ -407,4 +463,10 @@ TEST_F(TransactionSurfaceFrameTest, 
TEST_F(TransactionSurfaceFrameTest, PendingSurfaceFramesRemovedAfterClassification) {

TEST_F(TransactionSurfaceFrameTest, PendingSurfaceFramesRemovedAfterClassification) {

    PendingSurfaceFramesRemovedAfterClassification();

    PendingSurfaceFramesRemovedAfterClassification();

}

}



TEST_F(TransactionSurfaceFrameTest,

       BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer) {

    BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer();

}



} // namespace android
 
} // namespace android
 
 No newline at end of file