SF: VSyncReactor: correct distribute timestamp
Timestamp being distributed via the choreographer callback was the
anticipated vsync timestamp. This might make sense in the future, but
the correct legacy behavior was to distribute the event timestamp.
(code with problem was flagged off)
Test: correction of 2 unit tests
Test: boot to home
Test: uibench scroll, jitter not observed.
Fixes: 147487378
Change-Id: I142a6eaf849479dfe5b754db138f55d9e5870afd
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatch.h b/services/surfaceflinger/Scheduler/VSyncDispatch.h
index 56b3252..a6fb3a4 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatch.h
+++ b/services/surfaceflinger/Scheduler/VSyncDispatch.h
@@ -39,6 +39,14 @@
virtual ~VSyncDispatch();
/*
+ * A callback that can be registered to be awoken at a given time relative to a vsync event.
+ * \param [in] vsyncTime The timestamp of the vsync the callback is for.
+ * \param [in] targetWakeupTime The timestamp of intended wakeup time of the cb.
+ *
+ */
+ using Callback = std::function<void(nsecs_t vsyncTime, nsecs_t targetWakeupTime)>;
+
+ /*
* Registers a callback that will be called at designated points on the vsync timeline.
* The callback can be scheduled, rescheduled targeting vsync times, or cancelled.
* The token returned must be cleaned up via unregisterCallback.
@@ -51,7 +59,7 @@
* invocation of callbackFn.
*
*/
- virtual CallbackToken registerCallback(std::function<void(nsecs_t)> const& callbackFn,
+ virtual CallbackToken registerCallback(Callback const& callbackFn,
std::string callbackName) = 0;
/*
@@ -112,7 +120,7 @@
*/
class VSyncCallbackRegistration {
public:
- VSyncCallbackRegistration(VSyncDispatch&, std::function<void(nsecs_t)> const& callbackFn,
+ VSyncCallbackRegistration(VSyncDispatch&, VSyncDispatch::Callback const& callbackFn,
std::string const& callbackName);
VSyncCallbackRegistration(VSyncCallbackRegistration&&);
VSyncCallbackRegistration& operator=(VSyncCallbackRegistration&&);
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index 48f2abb..2e5b6e9 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -29,7 +29,7 @@
TimeKeeper::~TimeKeeper() = default;
VSyncDispatchTimerQueueEntry::VSyncDispatchTimerQueueEntry(std::string const& name,
- std::function<void(nsecs_t)> const& cb,
+ VSyncDispatch::Callback const& cb,
nsecs_t minVsyncDistance)
: mName(name),
mCallback(cb),
@@ -97,13 +97,13 @@
return *mLastDispatchTime;
}
-void VSyncDispatchTimerQueueEntry::callback(nsecs_t t) {
+void VSyncDispatchTimerQueueEntry::callback(nsecs_t vsyncTimestamp, nsecs_t wakeupTimestamp) {
{
std::lock_guard<std::mutex> lk(mRunningMutex);
mRunning = true;
}
- mCallback(t);
+ mCallback(vsyncTimestamp, wakeupTimestamp);
std::lock_guard<std::mutex> lk(mRunningMutex);
mRunning = false;
@@ -171,7 +171,8 @@
void VSyncDispatchTimerQueue::timerCallback() {
struct Invocation {
std::shared_ptr<VSyncDispatchTimerQueueEntry> callback;
- nsecs_t timestamp;
+ nsecs_t vsyncTimestamp;
+ nsecs_t wakeupTimestamp;
};
std::vector<Invocation> invocations;
{
@@ -186,7 +187,7 @@
if (*wakeupTime < mIntendedWakeupTime + mTimerSlack) {
callback->executing();
invocations.emplace_back(
- Invocation{callback, *callback->lastExecutedVsyncTarget()});
+ Invocation{callback, *callback->lastExecutedVsyncTarget(), *wakeupTime});
}
}
@@ -195,12 +196,12 @@
}
for (auto const& invocation : invocations) {
- invocation.callback->callback(invocation.timestamp);
+ invocation.callback->callback(invocation.vsyncTimestamp, invocation.wakeupTimestamp);
}
}
VSyncDispatchTimerQueue::CallbackToken VSyncDispatchTimerQueue::registerCallback(
- std::function<void(nsecs_t)> const& callbackFn, std::string callbackName) {
+ Callback const& callbackFn, std::string callbackName) {
std::lock_guard<decltype(mMutex)> lk(mMutex);
return CallbackToken{
mCallbacks
@@ -271,7 +272,7 @@
}
VSyncCallbackRegistration::VSyncCallbackRegistration(VSyncDispatch& dispatch,
- std::function<void(nsecs_t)> const& callbackFn,
+ VSyncDispatch::Callback const& callbackFn,
std::string const& callbackName)
: mDispatch(dispatch),
mToken(dispatch.registerCallback(callbackFn, callbackName)),
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
index 0c9b4fe..087acc7 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
@@ -36,7 +36,7 @@
// Valid transition: disarmed -> armed ( when scheduled )
// Valid transition: armed -> running -> disarmed ( when timer is called)
// Valid transition: armed -> disarmed ( when cancelled )
- VSyncDispatchTimerQueueEntry(std::string const& name, std::function<void(nsecs_t)> const& fn,
+ VSyncDispatchTimerQueueEntry(std::string const& name, VSyncDispatch::Callback const& fn,
nsecs_t minVsyncDistance);
std::string_view name() const;
@@ -62,14 +62,14 @@
nsecs_t executing();
// End: functions that are not threadsafe.
- // Invoke the callback with the timestamp, moving the state from running->disarmed.
- void callback(nsecs_t timestamp);
+ // Invoke the callback with the two given timestamps, moving the state from running->disarmed.
+ void callback(nsecs_t vsyncTimestamp, nsecs_t wakeupTimestamp);
// Block calling thread while the callback is executing.
void ensureNotRunning();
private:
std::string const mName;
- std::function<void(nsecs_t)> const mCallback;
+ VSyncDispatch::Callback const mCallback;
nsecs_t mWorkDuration;
nsecs_t mEarliestVsync;
@@ -104,8 +104,7 @@
nsecs_t timerSlack, nsecs_t minVsyncDistance);
~VSyncDispatchTimerQueue();
- CallbackToken registerCallback(std::function<void(nsecs_t)> const& callbackFn,
- std::string callbackName) final;
+ CallbackToken registerCallback(Callback const& callbackFn, std::string callbackName) final;
void unregisterCallback(CallbackToken token) final;
ScheduleResult schedule(CallbackToken token, nsecs_t workDuration, nsecs_t earliestVsync) final;
CancelResult cancel(CallbackToken token) final;
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index a652053..6c0a64e 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -48,7 +48,8 @@
nsecs_t period, nsecs_t offset, nsecs_t notBefore)
: mCallback(cb),
mRegistration(dispatch,
- std::bind(&CallbackRepeater::callback, this, std::placeholders::_1),
+ std::bind(&CallbackRepeater::callback, this, std::placeholders::_1,
+ std::placeholders::_2),
std::string(name)),
mPeriod(period),
mOffset(offset),
@@ -85,15 +86,13 @@
}
private:
- void callback(nsecs_t vsynctime) {
- nsecs_t period = 0;
+ void callback(nsecs_t vsynctime, nsecs_t wakeupTime) {
{
std::lock_guard<std::mutex> lk(mMutex);
- period = mPeriod;
mLastCallTime = vsynctime;
}
- mCallback->onDispSyncEvent(vsynctime - period);
+ mCallback->onDispSyncEvent(wakeupTime);
{
std::lock_guard<std::mutex> lk(mMutex);