Merge "SF: VSyncReactor: correct distribute timestamp"
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 158a184..20b6238 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);
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index 5846c77..b51a025 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -101,7 +101,7 @@
RepeatingCallbackReceiver(VSyncDispatch& dispatch, nsecs_t wl)
: mWorkload(wl),
mCallback(
- dispatch, [&](auto time) { callback_called(time); }, "repeat0") {}
+ dispatch, [&](auto time, auto) { callback_called(time); }, "repeat0") {}
void repeatedly_schedule(size_t iterations, std::function<void(nsecs_t)> const& onEachFrame) {
mCallbackTimes.reserve(iterations);
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index 5aff429..6fcb1af 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -97,13 +97,14 @@
CountingCallback(VSyncDispatch& dispatch)
: mDispatch(dispatch),
mToken(dispatch.registerCallback(std::bind(&CountingCallback::counter, this,
- std::placeholders::_1),
+ std::placeholders::_1,
+ std::placeholders::_2),
"test")) {}
~CountingCallback() { mDispatch.unregisterCallback(mToken); }
operator VSyncDispatch::CallbackToken() const { return mToken; }
- void counter(nsecs_t time) { mCalls.push_back(time); }
+ void counter(nsecs_t time, nsecs_t) { mCalls.push_back(time); }
VSyncDispatch& mDispatch;
VSyncDispatch::CallbackToken mToken;
@@ -115,7 +116,8 @@
PausingCallback(VSyncDispatch& dispatch, std::chrono::milliseconds pauseAmount)
: mDispatch(dispatch),
mToken(dispatch.registerCallback(std::bind(&PausingCallback::pause, this,
- std::placeholders::_1),
+ std::placeholders::_1,
+ std::placeholders::_2),
"test")),
mRegistered(true),
mPauseAmount(pauseAmount) {}
@@ -123,7 +125,7 @@
operator VSyncDispatch::CallbackToken() const { return mToken; }
- void pause(nsecs_t) {
+ void pause(nsecs_t, nsecs_t) {
std::unique_lock<std::mutex> lk(mMutex);
mPause = true;
mCv.notify_all();
@@ -462,7 +464,8 @@
EXPECT_CALL(mMockClock, alarmIn(_, 1000)).InSequence(seq);
VSyncDispatch::CallbackToken tmp;
- tmp = mDispatch.registerCallback([&](auto) { mDispatch.schedule(tmp, 100, 2000); }, "o.o");
+ tmp = mDispatch.registerCallback([&](auto, auto) { mDispatch.schedule(tmp, 100, 2000); },
+ "o.o");
mDispatch.schedule(tmp, 100, 1000);
advanceToNextCallback();
@@ -472,7 +475,7 @@
VSyncDispatch::CallbackToken tmp;
std::optional<nsecs_t> lastTarget;
tmp = mDispatch.registerCallback(
- [&](auto timestamp) {
+ [&](auto timestamp, auto) {
EXPECT_EQ(mDispatch.schedule(tmp, 400, timestamp - mVsyncMoveThreshold),
ScheduleResult::Scheduled);
EXPECT_EQ(mDispatch.schedule(tmp, 400, timestamp), ScheduleResult::Scheduled);
@@ -621,7 +624,7 @@
EXPECT_CALL(mMockClock, alarmCancel()).Times(1);
VSyncCallbackRegistration cb(
- mDispatch, [](auto) {}, "");
+ mDispatch, [](auto, auto) {}, "");
VSyncCallbackRegistration cb1(std::move(cb));
cb.schedule(100, 1000);
cb.cancel();
@@ -635,9 +638,9 @@
EXPECT_CALL(mMockClock, alarmCancel()).Times(1);
VSyncCallbackRegistration cb(
- mDispatch, [](auto) {}, "");
+ mDispatch, [](auto, auto) {}, "");
VSyncCallbackRegistration cb1(
- mDispatch, [](auto) {}, "");
+ mDispatch, [](auto, auto) {}, "");
cb1 = std::move(cb);
cb.schedule(100, 1000);
cb.cancel();
@@ -656,7 +659,7 @@
TEST_F(VSyncDispatchTimerQueueEntryTest, stateAfterInitialization) {
std::string name("basicname");
VSyncDispatchTimerQueueEntry entry(
- name, [](auto) {}, mVsyncMoveThreshold);
+ name, [](auto, auto) {}, mVsyncMoveThreshold);
EXPECT_THAT(entry.name(), Eq(name));
EXPECT_FALSE(entry.lastExecutedVsyncTarget());
EXPECT_FALSE(entry.wakeupTime());
@@ -664,7 +667,7 @@
TEST_F(VSyncDispatchTimerQueueEntryTest, stateScheduling) {
VSyncDispatchTimerQueueEntry entry(
- "test", [](auto) {}, mVsyncMoveThreshold);
+ "test", [](auto, auto) {}, mVsyncMoveThreshold);
EXPECT_FALSE(entry.wakeupTime());
EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
@@ -684,7 +687,7 @@
.Times(1)
.WillOnce(Return(10000));
VSyncDispatchTimerQueueEntry entry(
- "test", [](auto) {}, mVsyncMoveThreshold);
+ "test", [](auto, auto) {}, mVsyncMoveThreshold);
EXPECT_FALSE(entry.wakeupTime());
EXPECT_THAT(entry.schedule(500, 994, mStubTracker, now), Eq(ScheduleResult::Scheduled));
@@ -695,12 +698,14 @@
TEST_F(VSyncDispatchTimerQueueEntryTest, runCallback) {
auto callCount = 0;
- auto calledTime = 0;
+ auto vsyncCalledTime = 0;
+ auto wakeupCalledTime = 0;
VSyncDispatchTimerQueueEntry entry(
"test",
- [&](auto time) {
+ [&](auto vsyncTime, auto wakeupTime) {
callCount++;
- calledTime = time;
+ vsyncCalledTime = vsyncTime;
+ wakeupCalledTime = wakeupTime;
},
mVsyncMoveThreshold);
@@ -709,10 +714,11 @@
ASSERT_TRUE(wakeup);
EXPECT_THAT(*wakeup, Eq(900));
- entry.callback(entry.executing());
+ entry.callback(entry.executing(), *wakeup);
EXPECT_THAT(callCount, Eq(1));
- EXPECT_THAT(calledTime, Eq(mPeriod));
+ EXPECT_THAT(vsyncCalledTime, Eq(mPeriod));
+ EXPECT_THAT(wakeupCalledTime, Eq(*wakeup));
EXPECT_FALSE(entry.wakeupTime());
auto lastCalledTarget = entry.lastExecutedVsyncTarget();
ASSERT_TRUE(lastCalledTarget);
@@ -726,7 +732,7 @@
.WillOnce(Return(1020));
VSyncDispatchTimerQueueEntry entry(
- "test", [](auto) {}, mVsyncMoveThreshold);
+ "test", [](auto, auto) {}, mVsyncMoveThreshold);
EXPECT_FALSE(entry.wakeupTime());
entry.update(mStubTracker, 0);
@@ -745,7 +751,7 @@
TEST_F(VSyncDispatchTimerQueueEntryTest, skipsUpdateIfJustScheduled) {
VSyncDispatchTimerQueueEntry entry(
- "test", [](auto) {}, mVsyncMoveThreshold);
+ "test", [](auto, auto) {}, mVsyncMoveThreshold);
EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
entry.update(mStubTracker, 0);
@@ -756,7 +762,7 @@
TEST_F(VSyncDispatchTimerQueueEntryTest, willSnapToNextTargettableVSync) {
VSyncDispatchTimerQueueEntry entry(
- "test", [](auto) {}, mVsyncMoveThreshold);
+ "test", [](auto, auto) {}, mVsyncMoveThreshold);
EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
entry.executing(); // 1000 is executing
// had 1000 not been executing, this could have been scheduled for time 800.
@@ -773,7 +779,7 @@
TEST_F(VSyncDispatchTimerQueueEntryTest,
willRequestNextEstimateWhenSnappingToNextTargettableVSync) {
VSyncDispatchTimerQueueEntry entry(
- "test", [](auto) {}, mVsyncMoveThreshold);
+ "test", [](auto, auto) {}, mVsyncMoveThreshold);
Sequence seq;
EXPECT_CALL(mStubTracker, nextAnticipatedVSyncTimeFrom(500))
@@ -795,7 +801,7 @@
TEST_F(VSyncDispatchTimerQueueEntryTest, reportsScheduledIfStillTime) {
VSyncDispatchTimerQueueEntry entry(
- "test", [](auto) {}, mVsyncMoveThreshold);
+ "test", [](auto, auto) {}, mVsyncMoveThreshold);
EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
EXPECT_THAT(entry.schedule(200, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
EXPECT_THAT(entry.schedule(50, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index 4df20af..188adea 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -73,7 +73,8 @@
class MockVSyncDispatch : public VSyncDispatch {
public:
- MOCK_METHOD2(registerCallback, CallbackToken(std::function<void(nsecs_t)> const&, std::string));
+ MOCK_METHOD2(registerCallback,
+ CallbackToken(std::function<void(nsecs_t, nsecs_t)> const&, std::string));
MOCK_METHOD1(unregisterCallback, void(CallbackToken));
MOCK_METHOD3(schedule, ScheduleResult(CallbackToken, nsecs_t, nsecs_t));
MOCK_METHOD1(cancel, CancelResult(CallbackToken token));
@@ -82,7 +83,7 @@
class VSyncDispatchWrapper : public VSyncDispatch {
public:
VSyncDispatchWrapper(std::shared_ptr<VSyncDispatch> const& dispatch) : mDispatch(dispatch) {}
- CallbackToken registerCallback(std::function<void(nsecs_t)> const& callbackFn,
+ CallbackToken registerCallback(std::function<void(nsecs_t, nsecs_t)> const& callbackFn,
std::string callbackName) final {
return mDispatch->registerCallback(callbackFn, callbackName);
}
@@ -159,14 +160,15 @@
static constexpr nsecs_t mPhase = 3000;
static constexpr nsecs_t mAnotherPhase = 5200;
static constexpr nsecs_t period = 10000;
- static constexpr nsecs_t mFakeCbTime = 2093;
+ static constexpr nsecs_t mFakeVSyncTime = 2093;
+ static constexpr nsecs_t mFakeWakeupTime = 1892;
static constexpr nsecs_t mFakeNow = 2214;
static constexpr const char mName[] = "callbacky";
VSyncDispatch::CallbackToken const mFakeToken{2398};
nsecs_t lastCallbackTime = 0;
StubCallback outerCb;
- std::function<void(nsecs_t)> innerCb;
+ std::function<void(nsecs_t, nsecs_t)> innerCb;
VSyncReactor mReactor;
};
@@ -439,7 +441,8 @@
.WillOnce(DoAll(SaveArg<0>(&innerCb), Return(mFakeToken)));
EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mPhase), mFakeNow))
.InSequence(seq);
- EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mPhase), mFakeCbTime))
+ EXPECT_CALL(*mMockDispatch,
+ schedule(mFakeToken, computeWorkload(period, mPhase), mFakeVSyncTime))
.Times(2)
.InSequence(seq);
EXPECT_CALL(*mMockDispatch, cancel(mFakeToken)).InSequence(seq);
@@ -447,24 +450,25 @@
mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);
ASSERT_TRUE(innerCb);
- innerCb(mFakeCbTime);
- innerCb(mFakeCbTime);
+ innerCb(mFakeVSyncTime, mFakeWakeupTime);
+ innerCb(mFakeVSyncTime, mFakeWakeupTime);
}
-TEST_F(VSyncReactorTest, callbackTimestampReadapted) {
+TEST_F(VSyncReactorTest, callbackTimestampDistributedIsWakeupTime) {
Sequence seq;
EXPECT_CALL(*mMockDispatch, registerCallback(_, _))
.InSequence(seq)
.WillOnce(DoAll(SaveArg<0>(&innerCb), Return(mFakeToken)));
EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mPhase), mFakeNow))
.InSequence(seq);
- EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mPhase), mFakeCbTime))
+ EXPECT_CALL(*mMockDispatch,
+ schedule(mFakeToken, computeWorkload(period, mPhase), mFakeVSyncTime))
.InSequence(seq);
mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);
ASSERT_TRUE(innerCb);
- innerCb(mFakeCbTime);
- EXPECT_THAT(outerCb.lastCallTime(), Optional(mFakeCbTime - period));
+ innerCb(mFakeVSyncTime, mFakeWakeupTime);
+ EXPECT_THAT(outerCb.lastCallTime(), Optional(mFakeWakeupTime));
}
TEST_F(VSyncReactorTest, eventListenersRemovedOnDestruction) {
@@ -540,7 +544,7 @@
mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);
mReactor.changePhaseOffset(&outerCb, mAnotherPhase);
ASSERT_TRUE(innerCb);
- innerCb(mFakeCbTime);
+ innerCb(mFakeVSyncTime, mFakeWakeupTime);
}
TEST_F(VSyncReactorTest, negativeOffsetsApplied) {
@@ -590,7 +594,7 @@
ON_CALL(*mMockDispatch, schedule(_, _, _))
.WillByDefault(Return(ScheduleResult::CannotSchedule));
- EXPECT_DEATH(innerCb(mFakeCbTime), ".*");
+ EXPECT_DEATH(innerCb(mFakeVSyncTime, mFakeWakeupTime), ".*");
}
} // namespace android::scheduler