Closed Bug 753453 Opened 12 years ago Closed 11 years ago

requestAnimationFrame callback should return DOMHighResTimeStamp

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23
blocking-kilimanjaro -
blocking-basecamp -

People

(Reporter: mbest, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [games:p1][See comment 20 for dev-doc info])

Attachments

(1 file)

Do you know if we plan to update the requestAnimationFrame callback to
return a DOMHighResTimeStamp (http://www.w3.org/TR/hr-time/) instead of
a DOMTimeStamp? The spec says it should be a DOMTimeStamp but it makes
sense to switch to DOMHighResTimeStamp while it's still prefixed.

Chrome Canary just updated with DOMHighResTimeStamp being returned in
the requestAnimationFrame callback. I imagine this will screw up some
existing behaviour but it will surely be less painful to switch now than
it would be to do it later.
This would be pretty straightforward, right?

The only questions are:

1) Whether it would make sense to do this independently of switching to a monotonic clock.
2) Whether JS_Now ever in fact returns non-integer numbers of ms.

This last should be easy to test, of course.  Especially if someone writes a patch.  I'm happy to review.  ;)
Version: 15 Branch → Trunk
Blocks: gecko-games
Version: Trunk → 15 Branch
Version: 15 Branch → unspecified
Version: unspecified → Trunk
blocking-basecamp: --- → ?
blocking-kilimanjaro: --- → ?
Whiteboard: [games:p1]
This doesn't block shipping of B2G V1, so minusing. Re-nom for K9o if you think it's an overall blocker for that.
blocking-basecamp: ? → -
blocking-kilimanjaro: ? → -
The reason that I put this up is that micro second timers have landed and we haven't updated the requestAnimationFrame return value use the new resolution.  Chrome has done so already.  This is a relatively minor change and we should do this fix before this feature is widely used.

Change your mind on the flag?
What do you mean by microsecond timers have landed?  Last I checked we were still at multiple-ms timer latency on Windows.
> This is a relatively minor change

It's not quite minor, actually.  For one thing, the 0 values are different, so the change is a compat-breaking change.

What's needed to fix this bug is:

1)  Fixing mozAnimationStartTime to return a DOMHighResTimeStamp
2)  Changing the argument passed to the callback to a DOMHighResTimeStamp
3)  Auditing all in-tree callers of mozRequestAnimationFrame to make sure they're not
    comparing the values to Date.now.
4)  Reorganizing the entire setup for how we call requestAnimationFrame callbacks to make
    all this work, because different callbacks will need to be passed different time
    values (since the zero point of DOMHighResTimeStamp depends on the document, and we
    share refresh drivers across multiple documents).
5)  As #3, but for the web and extensions.  The IE folks say the web is not a big problem
    here, which is good if true.

Depending on the status of extensions, it might make the most sense to do this change as part of unprefixing the API (and leave the prefixed version around with the old behavior for a bit, possibly).
Here is the bug I have been tracking for Timer status: 673105

I see your point about it not being as straight forward as it seems on the surface.  Currently my understanding it that we are still only using mozRequestAnimationFrame rather than requestAnimationFrame so if we are going to make this change, we should do it before the unprefixed version lands as you suggest.  The idea of leaving the prefixed version using the old timers is fine with me if that helps to not break the web.  

Starting to feel like we are getting a clearer picture.  To the original question, does the prefixing need to happen for basecamp or can it wait?
Blocks: 704063
What's the status of this bug?
Keywords: dev-doc-needed
Status is someone needs to do what comment 5 says...
Summary: requestAnimationFrame callback returns DOMHighResTimeStamp → requestAnimationFrame callback should return DOMHighResTimeStamp
> so the change is a compat-breaking change.

As a data point, this exact change broke sites in Chrome.  See http://code.google.com/p/chromium/issues/detail?id=158910
And given the goings-on over there, I'm _really_ glad we didn't do it in May.  Back then there weren't even patches to GWT that people could have deployed...

An open question is whether we should only update the unprefixed version when we do update, by the way.
Back in May, there was no *patch* to GWT that people could have deployed but there was (and still is) a way to disable the use of requestAnimationFrame and only use setTimeout for animations.

And for those running off trunk, the patch came at the end of May: https://code.google.com/p/google-web-toolkit/source/detail?r=10989 It shipped in a RC mid-july and in GA in October: 3 months before the change reaches Chrome's Stable channel.

The problems are:
 - we (GWT) should probably haven't used prefixed APIs, at least without an easy (I mean, easier than it is already) and well-documented switch to turn it off (or possibly turned off by default); we're going to fix that (add a well-documented global switch, possibly turned off by default)
 - people don't update their apps to the latest GWT version fast enough and/or don't test with non-stable browsers (they'd have caught it 6 weeks earlier, before it impacted their users); this is unfortunately not going to change: most GWT apps are "Enterprisey", with lots of bureaucracy and "if it ain't broke don't fix it" culture (no preventive maintenance, continuous testing of apps with beta versions of browsers)
Thomas, just to make sure I understand the current GWT status...

We have two options for this bug:

1)  Make requestAnimationFrame pass in a DOMHighResTimeStamp while mozRequestAnimationFrame continues to have the behavior it has now.

2)  Make both pass in a DOMHighResTimeStamp.

Which one is more likely to work correctly with GWT?  Or does it not matter?
Changing mozRequestAnimationFrame's behavior will have the same effect as for Chrome: people using GWT 2.5 (or trunk after r10989) won't see a difference, this is because we don't use the callback's argument, we always '(new Date()).getTime()' to compute the animations' progress. Previously (in GWT 2.4), we used the callback's argument as a DOMTimeStamp for both webkitRAF and mozRAF.

GWT doesn't yet use the unprefixed rAF but the patch is in review. It uses the same approach as for mozRAF: don't rely on the return value of rAF –i.e. don't use cancelAnimationFrame, set a flag that's checked in the callback instead– and don't use the callback's argument.

You can thus safely change rAF: not yet in GWT, will be immune to the change anyway when added.
Changing mozRAF will break everyone not using GWT 2.5+, just like for Chrome. The workaround is similar (disable the use of mozRAF, use setTimeout instead). I therefore don't think it's risky: because Chrome crossed the line before Firefox, people already know how to deal with it (if they only applied the workaround) or have already upgraded to GWT 2.5 and thus won't notice the change.
Thomas, thanks for the data!
By the way, *removing* mozRequestAnimationFrame would not break GWT 2.4 (much) since it will just fall back on a timer-based implementation. I suspect that's true in general since most JavaScript code has a fallback for a missing vendor-prefixed symbol, but can't deal with arbitrary changes in behavior since those can't be predicted or tested in advance.

So for completeness, a third possibility if you're not ready to unprefix it would be to provide the new behavior in mozRequestAnimationFrame2 and (optionally) delete the old name. For GWT we'd rather this didn't happen since it will cause GWT 2.5 and trunk to fall back to timers as well, and I'd rather have something else in place first.

Whatever you decide, I'd like to know what to put in the patch so that GWT 2.5.1 and trunk will be able to use requestAnimationFrame on Firefox. The (incomplete) patch is here:
https://gwt-review.googlesource.com/#/c/1780/
Brian,

I don't think we have any plans to do mozRequestAnimationFrame2.  

We also have no plans to do an unprefixed requestAnimationFrame without fixing this bug first.

The plan is to fix this bug and unprefix at the same time; the only thing I'm still undecided on is what to do with mozRequestAnimationFrame when I do that.

Does that help?  Your GWT patch to use unprefixed requestAnimationFrame else fall back to timers looks reasonable to me, for what it's worth...
FWIW, Google reverted their change to the prefixed version: http://code.google.com/p/chromium/issues/detail?id=158910
Assignee: nobody → bzbarsky
So I'm going to take the same approach Chrome took.  In this bug I'll implement the infrastructure for passing a DOMHighResTimeStamp to requestAnimationFrame callbacks.  In bug 704063 I'll add a window.requestAnimationFrame that will make use of that.  The behavior of window.mozRequestAnimationFrame and mozAnimationStartTime is not going to change.
Whiteboard: [games:p1] → [need review][games:p1]
Depends on: 862629
Whiteboard: [need review][games:p1] → [need bug 862629 fixed][games:p1]
https://hg.mozilla.org/integration/mozilla-inbound/rev/bff130404258

So to recap the behavior, for dev-doc purposes:

* mozRequestAnimationFrame passes an epoch-based time to callbacks.  The passed-in value
  has millisecond precision and can be compared to mozAnimationStartTime.
* requestAnimationFrame passes a DOMHighResTimeStamp to callbacks.  It has microsecond
  precision and can be compared to performance.now(), kinda.
Flags: in-testsuite+
Whiteboard: [need bug 862629 fixed][games:p1] → [games:p1][See comment 20 for dev-doc info]
Target Milestone: --- → mozilla23
https://hg.mozilla.org/mozilla-central/rev/bff130404258
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 866402
Depends on: 866545
I've added this bug to the compatibility doc. Please correct the info if wrong.
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23
That looks great; thank you!
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: