Closed
Bug 588174
Opened 14 years ago
Closed 14 years ago
Add optional callback parameter to mozRequestAnimationFrame
Categories
(Core :: Layout, defect, P1)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla2.0b7
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta7+ |
People
(Reporter: roc, Assigned: bzbarsky)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file)
(deleted),
patch
|
roc
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
A commenter on hacks.mozilla.org suggested that instead of the pattern of adding a MozBeforePaint Listener and then calling mozRequestAnimationFrame, it would be slightly simpler if mozRequestAnimationFrame took a callback parameter that would be run at the next frame and then discarded. So the example here:
http://hacks.mozilla.org/2010/08/more-efficient-javascript-animations-with-mozrequestanimationframe/
would become
var start = window.mozAnimationStartTime;
function step(event) {
var progress = event.timeStamp - start;
d.style.left = Math.min(progress/10, 200) + "px";
if (progress < 2000) {
window.mozRequestAnimationFrame(step);
}
}
window.mozRequestAnimationFrame(step);
which is a bit simpler and avoids the possibility of the author forgetting to unregister the event listener.
We could leave the MozBeforePaint event support in --- it's likely to be useful for other things --- and make the callback parameter optional, so authors could use either pattern.
Reporter | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Reporter | ||
Comment 1•14 years ago
|
||
I suppose the callback should probably just take the timestamp as a parameter, so the example would be
var start = window.mozAnimationStartTime;
function step(timeStamp) {
var progress = timeStamp - start;
d.style.left = Math.min(progress/10, 200) + "px";
if (progress < 2000) {
window.mozRequestAnimationFrame(step);
}
}
window.mozRequestAnimationFrame(step);
Assignee | ||
Updated•14 years ago
|
Reporter | ||
Comment 2•14 years ago
|
||
I guess we need to get this in for beta6 if we get it in at all. If it slips, we can do it post-FF4.
blocking2.0: ? → beta6+
Assignee | ||
Comment 3•14 years ago
|
||
Assignee | ||
Comment 4•14 years ago
|
||
Comment on attachment 473384 [details] [diff] [review]
Make it possible to pass an explicit callback function to mozRequestAnimationFrame.
I made sure that if you only use the callback version we don't also fire the event. If it's ok to fire the event in addition to calling the callbacks, then I can probably simplify the code a little bit... but this seemed like cleaner api for web pages.
Attachment #473384 -
Attachment description: . Make it possible to pass an explicit callback function to mozRequestAnimationFrame. → Make it possible to pass an explicit callback function to mozRequestAnimationFrame.
Attachment #473384 -
Flags: review?(roc)
Assignee | ||
Updated•14 years ago
|
Whiteboard: [need review]
Reporter | ||
Comment 5•14 years ago
|
||
(In reply to comment #4)
> I made sure that if you only use the callback version we don't also fire the
> event. If it's ok to fire the event in addition to calling the callbacks, then
> I can probably simplify the code a little bit... but this seemed like cleaner
> api for web pages.
Actually I think we should fire the event for every paint.
Reporter | ||
Comment 6•14 years ago
|
||
(In reply to comment #5)
> Actually I think we should fire the event for every paint.
I mean, not now, but eventually.
Reporter | ||
Comment 7•14 years ago
|
||
Comment on attachment 473384 [details] [diff] [review]
Make it possible to pass an explicit callback function to mozRequestAnimationFrame.
Is that the right place for nsIAnimationFrameListener? I expected dom/interfaces somewhere.
ForgetAnimationFrameListeners should be called TakeAnimationFrameListeners, maybe?
Why does ScheduleAnimationFrameListeners return a value? I suggest it can just return void.
This should probably get super-review. Hopefully dbaron's around...
Attachment #473384 -
Flags: superreview?(dbaron)
Attachment #473384 -
Flags: review?(roc)
Attachment #473384 -
Flags: review+
Comment 8•14 years ago
|
||
Comment on attachment 473384 [details] [diff] [review]
Make it possible to pass an explicit callback function to mozRequestAnimationFrame.
Do the other [function] interfaces exposed to the Web not have classinfo? It seems a little odd, but I guess I can't see how it's harmful.
Put a newline at the end of the file, though.
Attachment #473384 -
Flags: superreview?(dbaron) → superreview+
Assignee | ||
Comment 9•14 years ago
|
||
> Do the other [function] interfaces exposed to the Web not have classinfo?
Yes. Classinfo is per-implementation, and the only expected implementation of this interface is a JS Function object. Classinfo doesn't even make sense there.
> Put a newline at the end of the file, though.
Done.
Assignee | ||
Comment 10•14 years ago
|
||
> Actually I think we should fire the event for every paint.
Per irc discussion, leaving this part as is for now.
> Is that the right place for nsIAnimationFrameListener?
Excellent question. I could put it in dom/interface/base, I suppose. We don't have a good dumping ground for that sort of thing.
> ForgetAnimationFrameListeners should be called TakeAnimationFrameListeners,
Sold.
> Why does ScheduleAnimationFrameListeners return a value?
It doesn't really need to given the existing consumer code, but if we ever want to know whether the listener really got added... I can remove it for now; easy enough to restore if we need it.
Assignee | ||
Comment 11•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6
Assignee | ||
Updated•14 years ago
|
Whiteboard: [need review]
Assignee | ||
Updated•14 years ago
|
Keywords: dev-doc-needed
Comment 12•14 years ago
|
||
I've updated the doc here:
https://developer.mozilla.org/en/DOM/window.mozRequestAnimationFrame
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•