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
diff --git a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
index dc8f12b..623a5e0 100644
--- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
@@ -439,6 +439,68 @@
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 @@
BufferSurfaceFrame_ReplaceValidTokenBufferWithInvalidTokenBuffer();
}
+TEST_F(TransactionSurfaceFrameTest, MultipleCommitsBeforeLatch) {
+ MultipleCommitsBeforeLatch();
+}
+
} // namespace android
\ No newline at end of file