Closed
Bug 753453
Opened 13 years ago
Closed 12 years ago
requestAnimationFrame callback should return DOMHighResTimeStamp
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla23
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)
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•13 years ago
|
||
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
Reporter | ||
Updated•13 years ago
|
Blocks: gecko-games
Version: Trunk → 15 Branch
Reporter | ||
Updated•13 years ago
|
Version: 15 Branch → unspecified
Reporter | ||
Updated•13 years ago
|
Version: unspecified → Trunk
Reporter | ||
Updated•12 years ago
|
blocking-basecamp: --- → ?
blocking-kilimanjaro: --- → ?
Whiteboard: [games:p1]
Comment 2•12 years ago
|
||
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: ? → -
Reporter | ||
Comment 3•12 years ago
|
||
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.
Assignee | ||
Comment 5•12 years ago
|
||
> 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).
Reporter | ||
Comment 6•12 years ago
|
||
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?
Summary: requestAnimationFrame callback returns DOMHighResTimeStamp → requestAnimationFrame callback should return DOMHighResTimeStamp
Assignee | ||
Comment 9•12 years ago
|
||
> 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
Assignee | ||
Comment 10•12 years ago
|
||
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.
Comment 11•12 years ago
|
||
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)
Assignee | ||
Comment 12•12 years ago
|
||
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?
Comment 13•12 years ago
|
||
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.
Assignee | ||
Comment 14•12 years ago
|
||
Thomas, thanks for the data!
Comment 15•12 years ago
|
||
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/
Assignee | ||
Comment 16•12 years ago
|
||
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...
Comment 17•12 years ago
|
||
FWIW, Google reverted their change to the prefixed version: http://code.google.com/p/chromium/issues/detail?id=158910
Assignee | ||
Comment 18•12 years ago
|
||
Attachment #738917 -
Flags: review?(roc)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → bzbarsky
Assignee | ||
Comment 19•12 years ago
|
||
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.
Assignee | ||
Updated•12 years ago
|
Whiteboard: [games:p1] → [need review][games:p1]
Attachment #738917 -
Flags: review?(roc) → review+
Assignee | ||
Updated•12 years ago
|
Depends on: 862629
Whiteboard: [need review][games:p1] → [need bug 862629 fixed][games:p1]
Assignee | ||
Comment 20•12 years ago
|
||
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
Comment 21•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 22•12 years ago
|
||
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
Assignee | ||
Comment 23•12 years ago
|
||
That looks great; thank you!
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Updated•12 years ago
|
Keywords: dev-doc-complete → dev-doc-needed
Comment 24•11 years ago
|
||
I've fixed the doc:
https://developer.mozilla.org/en-US/docs/Web/API/window.requestAnimationFrame
and
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/23
Keywords: dev-doc-needed → dev-doc-complete
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•