commit b1e10d15e6b38ac8524f5155736e78db5d223a24
author Ady Abraham <adyabr@google.com> Mon Mar 13 15:23:54 2023 -0700
committer Ady Abraham <adyabr@google.com> Tue Mar 14 10:31:00 2023 -0700
tree 85a462dafbf488d64d2d1a5ae260dd905ad03a7a
parent 87ad6cb122891edfe01e4beca7be0ff1c7d0a6d2

SF: handle presentTime in the past

presentTime in the past would result in Perfetto showing
unterminated slices. Fixing this issue by checking for this
case specifically, marking it janky, and fixing the presentTime to
be the endTime instead.

Test: SF unit tests
Bug: 271105399
Change-Id: I7e5d641dfe58717322b7a82b0609fc728e9a69fc

services/surfaceflinger/FrameTimeline/FrameTimeline.cpp

diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
index 925f111..ded734e 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
@@ -885,15 +885,17 @@
 
 void FrameTimeline::DisplayFrame::classifyJank(nsecs_t& deadlineDelta, nsecs_t& deltaToVsync,
                                                nsecs_t previousPresentTime) {
-    if (mPredictionState == PredictionState::Expired ||
-        mSurfaceFlingerActuals.presentTime == Fence::SIGNAL_TIME_INVALID) {
+    const bool presentTimeValid =
+            mSurfaceFlingerActuals.presentTime >= mSurfaceFlingerActuals.startTime;
+    if (mPredictionState == PredictionState::Expired || !presentTimeValid) {
         // Cannot do jank classification with expired predictions or invalid signal times. Set the
         // deltas to 0 as both negative and positive deltas are used as real values.
         mJankType = JankType::Unknown;
         deadlineDelta = 0;
         deltaToVsync = 0;
-        if (mSurfaceFlingerActuals.presentTime == Fence::SIGNAL_TIME_INVALID) {
+        if (!presentTimeValid) {
             mSurfaceFlingerActuals.presentTime = mSurfaceFlingerActuals.endTime;
+            mJankType |= JankType::DisplayHAL;
         }
 
         return;

services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp

diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
index abd7789..d26ef3c 100644
--- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
@@ -489,7 +489,7 @@
 
     auto displayFrame0 = getDisplayFrame(0);
     EXPECT_EQ(displayFrame0->getActuals().presentTime, 59);
-    EXPECT_EQ(displayFrame0->getJankType(), JankType::Unknown);
+    EXPECT_EQ(displayFrame0->getJankType(), JankType::Unknown | JankType::DisplayHAL);
     EXPECT_EQ(surfaceFrame1->getActuals().presentTime, -1);
     EXPECT_EQ(surfaceFrame1->getJankType(), JankType::Unknown);
 }
@@ -2259,6 +2259,7 @@
     mFrameTimeline->setSfWakeUp(sfToken3, 72, Fps::fromPeriodNsecs(11));
     mFrameTimeline->setSfPresent(80, validPresentFence);
 
+    erroneousPresentFence2->signalForTest(2);
     validPresentFence->signalForTest(80);
 
     addEmptyDisplayFrame();
@@ -2268,14 +2269,14 @@
         EXPECT_EQ(displayFrame->getActuals().presentTime, 26);
         EXPECT_EQ(displayFrame->getFramePresentMetadata(), FramePresentMetadata::UnknownPresent);
         EXPECT_EQ(displayFrame->getFrameReadyMetadata(), FrameReadyMetadata::UnknownFinish);
-        EXPECT_EQ(displayFrame->getJankType(), JankType::Unknown);
+        EXPECT_EQ(displayFrame->getJankType(), JankType::Unknown | JankType::DisplayHAL);
     }
     {
         auto displayFrame = getDisplayFrame(1);
         EXPECT_EQ(displayFrame->getActuals().presentTime, 60);
         EXPECT_EQ(displayFrame->getFramePresentMetadata(), FramePresentMetadata::UnknownPresent);
         EXPECT_EQ(displayFrame->getFrameReadyMetadata(), FrameReadyMetadata::UnknownFinish);
-        EXPECT_EQ(displayFrame->getJankType(), JankType::Unknown);
+        EXPECT_EQ(displayFrame->getJankType(), JankType::Unknown | JankType::DisplayHAL);
     }
     {
         auto displayFrame = getDisplayFrame(2);