SF: VSyncPredictor correct calculation
Correct a miscalculation in vsync predictions that led to
a missed callback.
Bug: 144707443
Fixes: 145667109
Test: boot to home, scroll shade and news
Test: 1 new unit test that forces the error condition:
Change-Id: Idef8018326f965fab42a450e8a954cb93fcad905
diff --git a/services/surfaceflinger/Scheduler/Timer.cpp b/services/surfaceflinger/Scheduler/Timer.cpp
index 2394ed2..fb4f315 100644
--- a/services/surfaceflinger/Scheduler/Timer.cpp
+++ b/services/surfaceflinger/Scheduler/Timer.cpp
@@ -85,7 +85,7 @@
};
if (timerfd_settime(mTimerFd, 0, &new_timer, &old_timer)) {
- ALOGW("Failed to set timerfd");
+ ALOGW("Failed to set timerfd %s (%i)", strerror(errno), errno);
}
}
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index 3894992..3b99a58 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -23,6 +23,7 @@
#include <utils/Trace.h>
#include <algorithm>
#include <chrono>
+#include <sstream>
#include "SchedulerUtils.h"
namespace android::scheduler {
@@ -153,12 +154,24 @@
}
auto const oldest = *std::min_element(timestamps.begin(), timestamps.end());
- auto const ordinalRequest = (timePoint - oldest + slope) / slope;
+
+ // See b/145667109, the ordinal calculation must take into account the intercept.
+ auto const zeroPoint = oldest + intercept;
+ auto const ordinalRequest = (timePoint - zeroPoint + slope) / slope;
auto const prediction = (ordinalRequest * slope) + intercept + oldest;
- ALOGV("prediction made from: %" PRId64 " prediction: %" PRId64 " (+%" PRId64 ") slope: %" PRId64
- " intercept: %" PRId64,
- timePoint, prediction, prediction - timePoint, slope, intercept);
+ auto const printer = [&, slope = slope, intercept = intercept] {
+ std::stringstream str;
+ str << "prediction made from: " << timePoint << "prediction: " << prediction << " (+"
+ << prediction - timePoint << ") slope: " << slope << " intercept: " << intercept
+ << "oldestTS: " << oldest << " ordinal: " << ordinalRequest;
+ return str.str();
+ };
+
+ ALOGV("%s", printer().c_str());
+ LOG_ALWAYS_FATAL_IF(prediction < timePoint, "VSyncPredictor: model miscalculation: %s",
+ printer().c_str());
+
return prediction;
}
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index d0c8090..4cb6a38 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -319,4 +319,36 @@
}
}
+// See b/145667109, and comment in prod code under test.
+TEST_F(VSyncPredictorTest, doesNotPredictBeforeTimePointWithHigherIntercept) {
+ std::vector<nsecs_t> const simulatedVsyncs{
+ 158929578733000,
+ 158929306806205, // oldest TS in ringbuffer
+ 158929650879052,
+ 158929661969209,
+ 158929684198847,
+ 158929695268171,
+ 158929706370359,
+ };
+ auto const idealPeriod = 11111111;
+ auto const expectedPeriod = 11113500;
+ auto const expectedIntercept = -395335;
+
+ tracker.setPeriod(idealPeriod);
+ for (auto const& timestamp : simulatedVsyncs) {
+ tracker.addVsyncTimestamp(timestamp);
+ }
+
+ auto [slope, intercept] = tracker.getVSyncPredictionModel();
+ EXPECT_THAT(slope, IsCloseTo(expectedPeriod, mMaxRoundingError));
+ EXPECT_THAT(intercept, IsCloseTo(expectedIntercept, mMaxRoundingError));
+
+ // (timePoint - oldestTS) % expectedPeriod works out to be: 395334
+ // (timePoint - oldestTS) / expectedPeriod works out to be: 38.96
+ // so failure to account for the offset will floor the ordinal to 38, which was in the past.
+ auto const timePoint = 158929728723871;
+ auto const prediction = tracker.nextAnticipatedVSyncTimeFrom(timePoint);
+ EXPECT_THAT(prediction, Ge(timePoint));
+}
+
} // namespace android::scheduler