Closed Bug 1119850 Opened 6 years ago Closed 6 years ago
Modify Vsync Timestamp on Mac to be the previous vsync timestamp
The TimeStamp given by the CVDisplayLinkCallbackFunction is for the upcoming vsync, which is in the future from TimeStamp::Now(). On Firefox OS, the vsync timestamp is for the vsync that just occurred. I'd like to unify the vsync timestamp across platforms that come from VsyncSource to the timestamp of the vsync that just occurred when the callback executes. Change the mac implementation of VsyncSource to pass the TimeStamp of the vsync that just occurred.
Comment on attachment 8546726 [details] [diff] [review] Use previous frame's vsync timestamp in OSXVsyncSource The only part I don't really like is that before we start the CVDisplayLinkThread, I set the timestamp to Now(). The delay between calling CVDisplayLinkThread and when the CVDisplayLinkCallback executes on my macbook pro ranges from 1ms - 14 ms. So the very first frame after we enable vsync will be janky along with the second frame as the interval will not be exactly 16.6 ms. Every vsync after that should be good though.
Attachment #8546726 - Flags: review?(mstange)
Why is changing OS X to give the timestamp of the previous frame better than changing all the other platforms to give the timestamp of the next frame? Can we not predict that accurately enough?
I think a couple of reasons. 1) If we have a future timestamp, I see some assertions such as this - http://mxr.mozilla.org/mozilla-central/source/image/src/FrameAnimator.cpp#79 - Where the TimeStamp should not be in the future. I don't have a firm grasp of how much of gecko assumes that painting / composting timestamps should be in the past relative to TimeStamp::Now(), but I don't want to chase down subtle bugs because of this. Before the vsync change, all timestamps were in the past or now, so I'd rather keep this behavior. 2) On b2g, the vsync signal is not quite as stable as it is on OS X. It's ~16.6-~16.7 between intervals, so if we try to predict the future, it won't be as accurate as just listening to the hardware. I haven't tested on other platforms other than b2g / mac.
Attachment #8546726 - Flags: review?(mstange) → review+
Successful try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d535f19ce70
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.