From ccdf0c3561ac261b74565e96232e1a3fdd6617f9 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 18 Jul 2020 05:34:35 +0000 Subject: [PATCH] Bug 1653737 - Add output time to VsyncEvent. r=nical This timestamp is provided by the system on macOS, and estimated as "the next vsync" on all other platforms. On macOS, the output time increments in very consistent amounts. The timestamp is independent of when exactly the vsync callback ends up running, so it is less vulnerable to unfortunate thread scheduling. This makes it a more reliable source for picking video frames, for example. Differential Revision: https://phabricator.services.mozilla.com/D83828 --- gfx/layers/ipc/LayersMessageUtils.h | 4 +++- gfx/thebes/SoftwareVsyncSource.cpp | 18 ++++++++++++------ gfx/thebes/SoftwareVsyncSource.h | 3 ++- gfx/thebes/VsyncSource.cpp | 5 +++-- gfx/thebes/VsyncSource.h | 29 +++++++++++++++++++---------- gfx/thebes/gfxAndroidPlatform.cpp | 5 ++++- gfx/thebes/gfxPlatformGtk.cpp | 3 ++- gfx/thebes/gfxPlatformMac.cpp | 8 ++++---- gfx/thebes/gfxWindowsPlatform.cpp | 2 +- widget/gtk/WaylandVsyncSource.cpp | 8 ++++++-- 10 files changed, 56 insertions(+), 29 deletions(-) diff --git a/gfx/layers/ipc/LayersMessageUtils.h b/gfx/layers/ipc/LayersMessageUtils.h index 2c425ac67f3a..c114fa5432e8 100644 --- a/gfx/layers/ipc/LayersMessageUtils.h +++ b/gfx/layers/ipc/LayersMessageUtils.h @@ -61,11 +61,13 @@ struct ParamTraits { static void Write(Message* msg, const paramType& param) { WriteParam(msg, param.mId); WriteParam(msg, param.mTime); + WriteParam(msg, param.mOutputTime); } static bool Read(const Message* msg, PickleIterator* iter, paramType* result) { return ReadParam(msg, iter, &result->mId) && - ReadParam(msg, iter, &result->mTime); + ReadParam(msg, iter, &result->mTime) && + ReadParam(msg, iter, &result->mOutputTime); } }; diff --git a/gfx/thebes/SoftwareVsyncSource.cpp b/gfx/thebes/SoftwareVsyncSource.cpp index 51f2267ffa19..e3fc64f8f765 100644 --- a/gfx/thebes/SoftwareVsyncSource.cpp +++ b/gfx/thebes/SoftwareVsyncSource.cpp @@ -47,7 +47,9 @@ void SoftwareDisplay::EnableVsync() { } MOZ_ASSERT(IsInSoftwareVsyncThread()); - NotifyVsync(mozilla::TimeStamp::Now()); + TimeStamp vsyncTime = TimeStamp::Now(); + TimeStamp outputTime = vsyncTime + mVsyncRate; + NotifyVsync(vsyncTime, outputTime); } void SoftwareDisplay::DisableVsync() { @@ -79,7 +81,8 @@ bool SoftwareDisplay::IsInSoftwareVsyncThread() { return mVsyncThread->thread_id() == PlatformThread::CurrentId(); } -void SoftwareDisplay::NotifyVsync(mozilla::TimeStamp aVsyncTimestamp) { +void SoftwareDisplay::NotifyVsync(const mozilla::TimeStamp& aVsyncTimestamp, + const mozilla::TimeStamp& aOutputTimestamp) { MOZ_ASSERT(IsInSoftwareVsyncThread()); mozilla::TimeStamp displayVsyncTime = aVsyncTimestamp; @@ -93,7 +96,7 @@ void SoftwareDisplay::NotifyVsync(mozilla::TimeStamp aVsyncTimestamp) { displayVsyncTime = now; } - Display::NotifyVsync(displayVsyncTime); + Display::NotifyVsync(displayVsyncTime, aOutputTimestamp); // Prevent skew by still scheduling based on the original // vsync timestamp @@ -111,9 +114,12 @@ void SoftwareDisplay::ScheduleNextVsync(mozilla::TimeStamp aVsyncTimestamp) { nextVsync = mozilla::TimeStamp::Now(); } - mCurrentVsyncTask = NewCancelableRunnableMethod( - "SoftwareDisplay::NotifyVsync", this, &SoftwareDisplay::NotifyVsync, - nextVsync); + TimeStamp outputTime = nextVsync + mVsyncRate; + + mCurrentVsyncTask = + NewCancelableRunnableMethod( + "SoftwareDisplay::NotifyVsync", this, &SoftwareDisplay::NotifyVsync, + nextVsync, outputTime); RefPtr addrefedTask = mCurrentVsyncTask; mVsyncThread->message_loop()->PostDelayedTask(addrefedTask.forget(), diff --git a/gfx/thebes/SoftwareVsyncSource.h b/gfx/thebes/SoftwareVsyncSource.h index 18a60043accd..addf71452083 100644 --- a/gfx/thebes/SoftwareVsyncSource.h +++ b/gfx/thebes/SoftwareVsyncSource.h @@ -21,7 +21,8 @@ class SoftwareDisplay final : public mozilla::gfx::VsyncSource::Display { void DisableVsync() override; bool IsVsyncEnabled() override; bool IsInSoftwareVsyncThread(); - void NotifyVsync(mozilla::TimeStamp aVsyncTimestamp) override; + void NotifyVsync(const mozilla::TimeStamp& aVsyncTimestamp, + const mozilla::TimeStamp& aOutputTimestamp) override; mozilla::TimeDuration GetVsyncRate() override; void ScheduleNextVsync(mozilla::TimeStamp aVsyncTimestamp); void Shutdown() override; diff --git a/gfx/thebes/VsyncSource.cpp b/gfx/thebes/VsyncSource.cpp index b4d9be8ac731..77e0d2b2358f 100644 --- a/gfx/thebes/VsyncSource.cpp +++ b/gfx/thebes/VsyncSource.cpp @@ -88,7 +88,8 @@ VsyncSource::Display::~Display() { MOZ_ASSERT(mEnabledCompositorVsyncDispatchers.Length() == 0); } -void VsyncSource::Display::NotifyVsync(TimeStamp aVsyncTimestamp) { +void VsyncSource::Display::NotifyVsync(const TimeStamp& aVsyncTimestamp, + const TimeStamp& aOutputTimestamp) { // Called on the vsync thread MutexAutoLock lock(mDispatcherLock); @@ -107,7 +108,7 @@ void VsyncSource::Display::NotifyVsync(TimeStamp aVsyncTimestamp) { (mLastVsyncIdSentToMainThread == mLastMainThreadProcessedVsyncId); mVsyncId = mVsyncId.Next(); - const VsyncEvent event(mVsyncId, aVsyncTimestamp); + const VsyncEvent event(mVsyncId, aVsyncTimestamp, aOutputTimestamp); for (size_t i = 0; i < mEnabledCompositorVsyncDispatchers.Length(); i++) { mEnabledCompositorVsyncDispatchers[i]->NotifyVsync(event); diff --git a/gfx/thebes/VsyncSource.h b/gfx/thebes/VsyncSource.h index 17e42ce292ff..638264ac3a4f 100644 --- a/gfx/thebes/VsyncSource.h +++ b/gfx/thebes/VsyncSource.h @@ -39,16 +39,23 @@ class VsyncSource { public: Display(); - // Notified when this display's vsync occurs, on the vsync thread - // The aVsyncTimestamp should normalize to the Vsync time that just occured - // However, different platforms give different vsync notification times. - // OSX - The vsync timestamp of the upcoming frame, in the future + // Notified when this display's vsync callback occurs, on the vsync thread + // Different platforms give different aVsyncTimestamp values. + // macOS: TimeStamp::Now() or the output time of the previous vsync + // callback, whichever is older. // Windows: It's messy, see gfxWindowsPlatform. // Android: TODO - // All platforms should normalize to the vsync that just occured. - // Large parts of Gecko assume TimeStamps should not be in the future such - // as animations - virtual void NotifyVsync(TimeStamp aVsyncTimestamp); + // + // @param aVsyncTimestamp The time of the Vsync that just occured. Needs to + // be at or before the time of the NotifyVsync call. + // @param aOutputTimestamp The estimated timestamp at which drawing will + // appear on the screen, if the drawing happens within a certain + // (unknown) budget. Useful for Audio/Video sync. On platforms where + // this timestamp is provided by the system (macOS), it is a much more + // stable and consistent timing source than the time at which the vsync + // callback is called. + virtual void NotifyVsync(const TimeStamp& aVsyncTimestamp, + const TimeStamp& aOutputTimestamp); void NotifyGenericObservers(VsyncEvent aEvent); RefPtr GetRefreshTimerVsyncDispatcher(); @@ -126,9 +133,11 @@ class VsyncSource { struct VsyncEvent { VsyncId mId; TimeStamp mTime; + TimeStamp mOutputTime; // estimate - VsyncEvent(const VsyncId& aId, const TimeStamp& aTime) - : mId(aId), mTime(aTime) {} + VsyncEvent(const VsyncId& aId, const TimeStamp& aVsyncTime, + const TimeStamp& aOutputTime) + : mId(aId), mTime(aVsyncTime), mOutputTime(aOutputTime) {} VsyncEvent() = default; }; diff --git a/gfx/thebes/gfxAndroidPlatform.cpp b/gfx/thebes/gfxAndroidPlatform.cpp index a4847a026b2d..2c070ce4b490 100644 --- a/gfx/thebes/gfxAndroidPlatform.cpp +++ b/gfx/thebes/gfxAndroidPlatform.cpp @@ -297,7 +297,10 @@ class AndroidVsyncSource final : public VsyncSource { using Base::DisposeNative; static void NotifyVsync() { - GetDisplayInstance().NotifyVsync(TimeStamp::Now()); + Display& display = GetDisplayInstance(); + TimeStamp vsyncTime = TimeStamp::Now(); + TimeStamp outputTime = vsyncTime + display.GetVsyncRate(); + display.NotifyVsync(vsyncTime, outputTime); } }; diff --git a/gfx/thebes/gfxPlatformGtk.cpp b/gfx/thebes/gfxPlatformGtk.cpp index da1978ed9893..78a5d21da4e8 100644 --- a/gfx/thebes/gfxPlatformGtk.cpp +++ b/gfx/thebes/gfxPlatformGtk.cpp @@ -666,7 +666,8 @@ class GtkVsyncSource final : public VsyncSource { } lastVsync = TimeStamp::Now(); - NotifyVsync(lastVsync); + TimeStamp outputTime = lastVsync + GetVsyncRate(); + NotifyVsync(lastVsync, outputTime); } } diff --git a/gfx/thebes/gfxPlatformMac.cpp b/gfx/thebes/gfxPlatformMac.cpp index 00f002055e59..9db137089283 100644 --- a/gfx/thebes/gfxPlatformMac.cpp +++ b/gfx/thebes/gfxPlatformMac.cpp @@ -518,10 +518,10 @@ static CVReturn VsyncCallback(CVDisplayLinkRef aDisplayLink, // Executed on OS X hardware vsync thread OSXVsyncSource::OSXDisplay* display = (OSXVsyncSource::OSXDisplay*)aDisplayLinkContext; - int64_t nextVsyncTimestamp = aOutputTime->hostTime; - mozilla::TimeStamp nextVsync = - mozilla::TimeStamp::FromSystemTime(nextVsyncTimestamp); + mozilla::TimeStamp outputTime = + mozilla::TimeStamp::FromSystemTime(aOutputTime->hostTime); + mozilla::TimeStamp nextVsync = outputTime; mozilla::TimeStamp previousVsync = display->mPreviousTimestamp; mozilla::TimeStamp now = TimeStamp::Now(); @@ -540,7 +540,7 @@ static CVReturn VsyncCallback(CVDisplayLinkRef aDisplayLink, display->mPreviousTimestamp = nextVsync; - display->NotifyVsync(previousVsync); + display->NotifyVsync(previousVsync, outputTime); return kCVReturnSuccess; } diff --git a/gfx/thebes/gfxWindowsPlatform.cpp b/gfx/thebes/gfxWindowsPlatform.cpp index 6e0fb7447a67..03d74f856747 100644 --- a/gfx/thebes/gfxWindowsPlatform.cpp +++ b/gfx/thebes/gfxWindowsPlatform.cpp @@ -1820,7 +1820,7 @@ class D3DVsyncSource final : public VsyncSource { // Large parts of gecko assume that the refresh driver timestamp // must be <= Now() and cannot be in the future. MOZ_ASSERT(vsync <= TimeStamp::Now()); - Display::NotifyVsync(vsync); + Display::NotifyVsync(vsync, vsync + mVsyncRate); // DwmComposition can be dynamically enabled/disabled // so we have to check every time that it's available. diff --git a/widget/gtk/WaylandVsyncSource.cpp b/widget/gtk/WaylandVsyncSource.cpp index 3955d0d725b2..e9a96a2e1704 100644 --- a/widget/gtk/WaylandVsyncSource.cpp +++ b/widget/gtk/WaylandVsyncSource.cpp @@ -72,7 +72,9 @@ void WaylandVsyncSource::WaylandDisplay::Refresh() { // Vsync is enabled, but we don't have a callback configured. Set one up so // we can get to work. SetupFrameCallback(); - NotifyVsync(TimeStamp::Now()); + TimeStamp vsyncTimestamp = TimeStamp::Now(); + TimeStamp outputTimestamp = vsyncTimestamp + GetVsyncRate(); + NotifyVsync(vsyncTimestamp, outputTimestamp); } void WaylandVsyncSource::WaylandDisplay::EnableMonitor() { @@ -127,7 +129,9 @@ void WaylandVsyncSource::WaylandDisplay::FrameCallback() { SetupFrameCallback(); } - NotifyVsync(TimeStamp::Now()); + TimeStamp vsyncTimestamp = TimeStamp::Now(); + TimeStamp outputTimestamp = vsyncTimestamp + GetVsyncRate(); + NotifyVsync(vsyncTimestamp, outputTimestamp); } void WaylandVsyncSource::WaylandDisplay::EnableVsync() {