Last Comment Bug 630127 - mozRequestAnimationFrame can produce poor framerates if callback function takes close to 16ms
: mozRequestAnimationFrame can produce poor framerates if callback function tak...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 All
: P1 normal (vote)
: mozilla5
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
http://webstuff.nfshost.com/anim-timi...
Depends on: 650379
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-30 23:34 PST by James Robinson
Modified: 2013-04-04 13:53 PDT (History)
12 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Like so (5.79 KB, patch)
2011-02-01 09:35 PST, Boris Zbarsky [:bz]
dbaron: review-
Details | Diff | Splinter Review
With that fixed (8.06 KB, patch)
2011-02-02 08:47 PST, Boris Zbarsky [:bz]
dbaron: review+
Details | Diff | Splinter Review
The testcase, to make sure it does not go away (2.57 KB, text/html)
2011-04-21 17:52 PDT, Boris Zbarsky [:bz]
no flags Details

Description James Robinson 2011-01-30 23:34:30 PST
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b11pre) Gecko/20110126 Firefox/4.0b11pre
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b11pre) Gecko/20110126 Firefox/4.0b11pre

Animation framerates using mozRequestAnimationFrame are limited in such a way that it is currently impossible to achieve 60FPS even if the animation update logic takes less than 1/60th of a second.  The following animation loop:

function callback(time) {
  window.mozRequestAnimationFrame(callback);
  doWork();
}

runs at ~30FPS if doWork() takes 1/60th of a second, assuming everything else runs infinitely fast.  The same animation would run at 60FPS if it used setTimeout(callback, 16) instead.  It appears that the callback always fires at ~16ms after javascript yields, rather than being scheduled for 16ms after the callback is invoked which would produce 60FPS animations if everything ran quickly enough.

To test navigate here: http://webstuff.nfshost.com/anim-timing/raftime.html and adjust the "Set delay" time.  With a delay of 0, the animation should update at 60FPS (although on my laptop I'm getting around 55).  With a delay of 16ms the animation should still animate at 60FPS but instead it hovers just below 30.

Reproducible: Always
Comment 1 Boris Zbarsky [:bz] 2011-01-31 05:36:26 PST
This is somewhat on purpose for now.  The timer used to drive web page rendering (including the animation callbacks) is a "slack" timer, which means that it fires 16ms after the previous firing finished, to avoid starving the event queue.  This is controlled by the "layout.frame_rate.precise" preference (defaults to false, can be switched to true to experiment).

See also discussion in bug 587887.

We could flip the timer to being precise if we have an animation frame DOM callback, I suppose, even if it's not precise in general.  roc, dbaron, thoughts?
Comment 2 Boris Zbarsky [:bz] 2011-01-31 05:45:42 PST
Oh, and to be clear the behavior is independent of whether you request an animation frame at the beginning of the loop or at the end of the loop.
Comment 3 James Robinson 2011-01-31 09:46:03 PST
That makes sense, however I'm concerned that it will be really difficult to to get authors to use mozRequestAnimationFrame instead of setTimeout or setInterval if they notice that it's not possible to achieve 60FPS with mozRequestAnimationFrame for code that can easily hit it using setTimeout().
Comment 4 Boris Zbarsky [:bz] 2011-01-31 09:51:00 PST
Right, that's why I said "for now".  We're going to ship the current behavior in Firefox 4, but we're still experimenting both with the animation frame API and with the refresh driver code in general.

I do think that we should try to do a precise timer here at least for the animation frame case....
Comment 5 James Robinson 2011-01-31 11:23:02 PST
How will web authors know when mozRequestAnimationFrame can provide 60FPS and when it is limited to something lower?  I suspect most will only want to use codepaths that provide 60FPS.
Comment 6 Boris Zbarsky [:bz] 2011-01-31 12:06:49 PST
I think there's an incorrect assumption here.  The attached testcase doesn't measure FPS.  It measures callback rate.  There's a big difference; see http://weblogs.mozillazine.org/roc/archives/2010/11/measuring_fps.html

In particular, when using the setTimeout approach the actual FPS you get will depend on how that timer interleaves with the refresh timer.  You could get anything from 30 to 60fps depending on the relative timing of those two timers (and that's without getting into the second-order effects that can happen due to the timer code's attempts to correct for timers firing late; I suspect based on past experiments that you could easily get nasty ringing, possibly of amplitude greater than 16ms).  If an author thinks they're actually getting "60fps" reliably from setTimeout in Gecko, they're just deluded (or measuring a poor proxy for fps, as here, which amounts to the same thing).

The thing that mozRequestAnimationFrame gives you is that your code will be called when the browser wants to paint and not more often than that.  So it provides better guarantees that your measured callback rate and the fps will actually match than setTimeout does.

At no point will any of this can possibly guarantee 60fps, obviously, especially not if the script itself is taking 16ms per callback, if there's work other than the script to be done (which there is, even in your simple testcase: all the restyling, layout, and painting).  So we should think about ways to improve the behavior there, but this won't be a "breaking change" by any means.  So I don't think there's a need for detection, just like there's no real need for detection right now of whether the browser is painting your content at all (in a background tab, it's not!).  

Now there _is_ a real issue here, which is that the API is not doing a good job of providing a 60Hz heartbeat in the face of callbacks that take a while.  I do think we should see if we can fix that.  I don't think web developers need a way to detect that behavior.

Back on the technical end of this, one interesting option may be to introduce a new kind of repeating XPCOM timer, which allows the callee to indicate which part of the callback time to charge against the next repetition (the "precise" case is "all of it", while the "slack" case is "none of it").  Then if we wanted we could charge the scripted callbacks to the next repetition while not charging our internal time (layout, etc).

But again, maybe just using a precise timer here is simpler...
Comment 7 James Robinson 2011-01-31 12:39:10 PST
(In reply to comment #6)
> I think there's an incorrect assumption here.  The attached testcase doesn't
> measure FPS.  It measures callback rate.  There's a big difference; see
> http://weblogs.mozillazine.org/roc/archives/2010/11/measuring_fps.html

A valid point, but the animation itself looks a lot worse when the observed callback rate is 30FPS compared to when it is 60FPS.  The actual animation rate for an imperative animation cannot exceed the callback rate (although it definitely can be lower than the callback rate).  Even if authors are not measuring the callback rate they will definitely notice that their setTimeout-based animations are smoother than their mozRequestAnimationFrame ones.

> 
> In particular, when using the setTimeout approach the actual FPS you get will
> depend on how that timer interleaves with the refresh timer.  You could get
> anything from 30 to 60fps depending on the relative timing of those two timers
> (and that's without getting into the second-order effects that can happen due
> to the timer code's attempts to correct for timers firing late; I suspect based
> on past experiments that you could easily get nasty ringing, possibly of
> amplitude greater than 16ms).  If an author thinks they're actually getting
> "60fps" reliably from setTimeout in Gecko, they're just deluded (or measuring a
> poor proxy for fps, as here, which amounts to the same thing).
> 
> The thing that mozRequestAnimationFrame gives you is that your code will be
> called when the browser wants to paint and not more often than that.  So it
> provides better guarantees that your measured callback rate and the fps will
> actually match than setTimeout does.
> 
> At no point will any of this can possibly guarantee 60fps, obviously,
> especially not if the script itself is taking 16ms per callback, if there's
> work other than the script to be done (which there is, even in your simple
> testcase: all the restyling, layout, and painting).  So we should think about
> ways to improve the behavior there, but this won't be a "breaking change" by
> any means.  So I don't think there's a need for detection, just like there's no
> real need for detection right now of whether the browser is painting your
> content at all (in a background tab, it's not!).  
> 
> Now there _is_ a real issue here, which is that the API is not doing a good job
> of providing a 60Hz heartbeat in the face of callbacks that take a while.  I do
> think we should see if we can fix that.  I don't think web developers need a
> way to detect that behavior.
> 
> Back on the technical end of this, one interesting option may be to introduce a
> new kind of repeating XPCOM timer, which allows the callee to indicate which
> part of the callback time to charge against the next repetition (the "precise"
> case is "all of it", while the "slack" case is "none of it").  Then if we
> wanted we could charge the scripted callbacks to the next repetition while not
> charging our internal time (layout, etc).
> 
> But again, maybe just using a precise timer here is simpler...
Comment 8 Boris Zbarsky [:bz] 2011-01-31 12:43:38 PST
> but the animation itself looks a lot worse when

Right; that's a problem we need to solve.
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-01-31 16:55:48 PST
Using a precise timer when mozRequestAnimationFrame callbacks are queued sounds like a good idea to me. It'll give us a great platform for testing :)
Comment 10 Boris Zbarsky [:bz] 2011-02-01 09:35:48 PST
Created attachment 508794 [details] [diff] [review]
Like so
Comment 11 Boris Zbarsky [:bz] 2011-02-01 09:40:40 PST
A few notes:

1)  Since I'm assuming this is post-2.0 work, that patch depends on my patch in
    bug 614733.

2)  On my machine with this patch and a 16ms delay we hit about 50 callbacks
    per second.  We're at 60-ish with a 10ms delay.  Changing delays will do
    weird stuff for a bit due to the delay line in the xpcom timer code, of
    course, so if you run with one delay for a while and then change delays
    you'll need to wait for the numbers to stabilize.

3)  I tested the Chrome 10 dev builds on the testcase, and it looks like in
    Chrome it matters whether the animation is requested before or after the
    delay loop.  James, do you guys have an open ticket on that?
Comment 12 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-02-01 09:44:08 PST
Comment on attachment 508794 [details] [diff] [review]
Like so

>+  if (mThrottled || mTimerIsPrecise != HaveAnimationFrameListeners()) {

I think this breaks the timer-is-precise preference since it'll restart the timer every time if the pref is set and we don't have animation frame listeners, which seems like it would make the timer not so precise.


Other than that, it looks fine.
Comment 13 Boris Zbarsky [:bz] 2011-02-01 09:48:12 PST
Er, yes.  Good catch.
Comment 14 James Robinson 2011-02-01 12:02:07 PST
(In reply to comment #11)
> 3)  I tested the Chrome 10 dev builds on the testcase, and it looks like in
>     Chrome it matters whether the animation is requested before or after the
>     delay loop.  James, do you guys have an open ticket on that?

Yup - I have a patch at http://codereview.chromium.org/6409007/ to improve Chrome's scheduling.  It looks like we're taking fairly different approaches to implementing this - the Chrome version is less tightly coupled to painting.
Comment 15 Boris Zbarsky [:bz] 2011-02-01 12:50:19 PST
Our version is not coupled to painting, but it _is_ coupled to style change computation.  That's somewhat required so that this API can be synchronized reasonably with CSS transitions and the like.
Comment 16 Boris Zbarsky [:bz] 2011-02-02 08:47:42 PST
Created attachment 509120 [details] [diff] [review]
With that fixed
Comment 17 Boris Zbarsky [:bz] 2011-02-02 09:15:24 PST
Though one note....  Actually using a 16ms delay with the above patch makes the UI _really_ laggy.
Comment 18 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-03-05 22:18:50 PST
Comment on attachment 509120 [details] [diff] [review]
With that fixed

>+/* static */
>+void nsRefreshDriver::InitializeStatics()

local style puts the void on the same line as the /* static */, not the line below.

r=dbaron with that fixed.

Sorry for the delay getting to this.
Comment 19 Steve Roussey (:sroussey) 2011-03-06 10:42:20 PST
So, just so I understand, mozRequestAnimationFrame is available in FF4, but setTimeout-based animations are smoother than their mozRequestAnimationFrame, until some future version (say FF5?). That leads to an issue where JavaScript libraries need to check browser version rather than just the existence of mozRequestAnimationFrame to determine runtime code paths (answer to comment 5 is check browser version number -- ick).

My opinion is that this should be hidden behind an option until it is *really* a preferred method for JS animation. Then user's just need to check is mozRequestAnimationFrame is available.
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-03-06 12:36:27 PST
We'll be dropping support for FF4 no later than six months after FF5 comes out, which with our current planning means around the end of this year. So this is not going to be a long-term problem.
Comment 21 Steve Roussey (:sroussey) 2011-03-06 12:52:32 PST
I understand it is really to late to change. But I've been dealing with some people from each of the big JS toolkit and library creators, and this is one of the things I evangelized. But the net result has been so far: nice idea, found and supported the "Chrome version". It is my fault, I didn't do a deep dive test like James did, so I looked like a retarded Mozilla fanboy. Then again, they probably would not have supported the Chrome version if it weren't for my nagging, so there is some user win there.

I'll add a reminder to my calendar for around March 2012 to revisit this feature.
Comment 22 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-03-06 13:05:26 PST
I'm sorry about that. We should have escalated this earlier but strangely enough the early feedback (since we first announced the feature in August) didn't bring this up.
Comment 23 Steve Roussey (:sroussey) 2011-03-06 13:15:36 PST
Funny, since I pushed it back in August too... and only got feedback myself recently as people are scrambling to deal with compatibility issues and advantages of FF4 now that it looks like it is going to ship. Sigh. I actually put it in my calendar for Dec 2011 for revisiting (assuming that no one will agree to version string checking). With JS so fast these days, this looks more like a power saving feature, and that can wait anyhow.

Faster releases are going to help. :) Thankfully not as fast as Chrome (they can't triage bugs as fast as they make releases, making for a really rocky experience).
Comment 24 Boris Zbarsky [:bz] 2011-03-07 08:56:55 PST
> but setTimeout-based animations are smoother than their
> mozRequestAnimationFrame,

That.... depends.  On the exact number of timeouts used, on how long your callback takes, on how the timeouts interleave with the refresh timer, etc, etc.

In the edge case of this bug, and measuring callback frequency as opposed to the actual visual look, timeouts give you a higher callback rate.  That will be true no matter what, since timeouts will go to 100Hz by default while the refresh timer is capped at 60Hz by default.

Are people seeing actual visual smoothness issues and mentioning those to you?

I don't think we should be hiding it or putting it behind a browser switch.  This API provides the #1 benefit it's supposed to provide (lower power usage; playing nice with background tabs) right now.  It's slighly less smooth than setTimeout in some cases; slightly more smooth in others.  The patch in this bug will change those sets a bit, but that's it.
Comment 25 Boris Zbarsky [:bz] 2011-03-07 08:59:34 PST
Pushed http://hg.mozilla.org/projects/birch/rev/fb9913c4e92a
Comment 26 David Humphrey (:humph) 2011-03-07 12:52:10 PST
We've used mozRequestAnimationFrame a lot, and in scenarios where we are doing a ton of work per frame in JS (audio analysis, xml/json parsing, webgl, etc.).  It's been great for us.  I still say you should tell people to use this, unless they are spending more than 16ms per frame, in which case they have other issues if their goal is ~60fps.
Comment 27 Steve Roussey (:sroussey) 2011-03-07 13:14:50 PST
What do people use to do automatic testing of animation? Is there enough instrumentation in FF to do it?
Comment 28 David Humphrey (:humph) 2011-03-07 13:17:14 PST
I like https://github.com/pcwalton/firefox-framerate-monitor for this, which itself uses mozRequestAnimationFrame to get its data.
Comment 29 Boris Zbarsky [:bz] 2011-03-07 13:21:13 PST
> What do people use to do automatic testing of animation?

In Gecko, https://developer.mozilla.org/en/DOM/window.mozPaintCount should work ok.  You can poll it off a high-resolution timer (e.g. bump your setInterval delay pref so you can get 250Hz intervals, and poll off of those) to get an idea of how often we paint.

See also http://weblogs.mozillazine.org/roc/archives/2010/11/measuring_fps.html

But yes, the frame rate monitor wraps this up nicely.
Comment 30 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-03-23 17:55:01 PDT
https://hg.mozilla.org/mozilla-central/rev/fb9913c4e92a
Comment 31 David Geary 2011-03-29 17:04:05 PDT
(In reply to comment #30)
> https://hg.mozilla.org/mozilla-central/rev/fb9913c4e92a

Nice to see a patch submitted for this. Can anyone say what Firefox release will contain the fix? I would like to know for my upcoming book on Canvas.

btw, even when I eschew mozRequestAnimationFrame in favor of setInterval( function() {...}, 1000/60), I still get very jumpy animations in FF4, even though I get ~60fps, whereas the same code in Chrome, for example, is rock solid smooth. I'm not sure why that is.

Thanks.
Comment 32 Boris Zbarsky [:bz] 2011-03-29 18:13:32 PDT
> Can anyone say what Firefox release will contain the fix?

Firefox next.  I'm not sure the number it'll be numbered with has been decided yet.  Probably 5, but maybe it'll be 4.2.

> I'm not sure why that is.

Given the information so far, neither am I.  I'd have to see an example and probably profile it to know what's going on.  Please feel free to file a bug on that, with a link to your testcase.
Comment 33 David Geary 2011-03-30 10:47:57 PDT
(In reply to comment #32)
> > Can anyone say what Firefox release will contain the fix?
> 
> Firefox next.  I'm not sure the number it'll be numbered with has been decided
> yet.  Probably 5, but maybe it'll be 4.2.

Thanks for the info.
 
> > I'm not sure why that is.
> 
> Given the information so far, neither am I.  I'd have to see an example and
> probably profile it to know what's going on.  Please feel free to file a bug on
> that, with a link to your testcase.

Touche. Sorry, I should know better. I'll file a bug report.
Comment 34 Boris Zbarsky [:bz] 2011-03-30 10:59:23 PDT
> I'll file a bug report.

Thanks!  To be clear, I wasn't being snarky in comment 32.  I really just don't have enough info, so the best I could do is list the usual causes of choppiness.
Comment 35 David Geary 2011-04-01 08:41:37 PDT
(In reply to comment #34)
> > I'll file a bug report.
> 
> Thanks!  To be clear, I wasn't being snarky in comment 32.  I really just don't
> have enough info, so the best I could do is list the usual causes of
> choppiness.

No problem. I did not mean to be argumentative, either. I should know better than to make an offhand remark instead of filing a bug report.

Anyway, I've filed a bug report: #647238. I'd love to be proven wrong, and shown the errors of my transgressions so I can get smooth animations in FF4. I would also love to know the list of usual causes of choppiness.

Thanks for taking the time to look into this.
Comment 36 Boris Zbarsky [:bz] 2011-04-21 17:52:36 PDT
Created attachment 527703 [details]
The testcase, to make sure it does not go away

Note You need to log in before you can comment on or make changes to this bug.