Effects: use requestAnimationFrame timestamp if available #3151

mgol · 2016-06-08T12:51:53Z

Summary

In some environments that support the requestAnimationFrame timestamp callback
parameter using it results in smoother animations.

Fixes gh-3143

Currently it's +35 bytes, most likely due to the getTimestamp function used in createFxNow(). Any ideas on how to avoid this size tax?

Checklist

Mark an [x] for completed items, if you're not sure leave them unchecked and we can assist.

All authors have signed the CLA at https://contribute.jquery.com/CLA/
~~New tests have been added to show the fix or feature works~~
Grunt build and unit tests pass locally with these changes
~~If needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com~~
Unit tests need to run on code with rAF enabled, currently we disable it
Add one test using non-mocked rAF/Date.now()

Thanks! Bots and humans will be around shortly to check it out.

scottgonzalez · 2016-06-08T12:58:06Z

There's a typo in your commit message, see the title change I just made.

mgol · 2016-06-08T13:03:41Z

Thanks, fixed.

dmethvin · 2016-06-08T14:34:24Z

src/effects.js

 	}
 }

+// We need to be using jQuery.now() or performance.now() consistently as they return different
+// values: performance.now() counter starts on page load.
+function getTimestamp() {


Can we just use jQuery.now() for all the browsers that don't have the rAF timestamp? Which browsers are affected?

This function is also used in createFxNow() which, I think, is not limited to old browsers?

markelog · 2016-06-09T09:58:45Z

src/effects.js

+// We need to be using jQuery.now() or performance.now() consistently as they return different
+// values: performance.now() counter starts on page load.
+function getTimestamp() {
+    return window.performance && typeof window.performance.now === "function" ?


Would you mind add a Support comment?

Sure, will do.

In what browsers we do not have now in performance object?

Also, could you try to use if..else instead of a ternary?

In what browsers we do not have now in performance object?

IE 9, Android <4.4, iOS 7.

And Node.js. :-)

Also, could you try to use if..else instead of a ternary?

Sure.

markelog · 2016-06-09T10:02:12Z

test/unit/effects.js

+
+		// Fake performance.now() returning lower values than Date.now()
+		// and that its values are fractional.
+		window.performance.now = function() {


This will break in IE9 right?

Nice catch.

So extrapolating from your comment https://github.com/jquery/jquery/pull/3151/files/c0d05944f93309dc8c0329316d4b6cf699e21d4d#r66414166 it will not break in IE9, but will break in iOS7 and similar envs

Why not? IE 9 doesn't have the performance object.

mm, i'm confused - https://developer.mozilla.org/en-US/docs/Web/API/Performance#Browser_compatibility

Ah, you're right. :) But it's good to avoid useless assignments anyway.

I wrapped it in an if & added support comments.

markelog · 2016-06-09T10:04:37Z

I am skeptical, since neither gsap nor velocity doing this and there is open tickets in vendor trackers about this

markelog · 2016-06-09T10:05:09Z

/cc @gnarf

/cc @paulirish since you authored the https://developers.google.com/web/updates/2012/05/requestAnimationFrame-API-now-with-sub-millisecond-precision?hl=en

gibson042 · 2016-07-08T18:58:08Z

test/unit/effects.js

+				return Date.now() - 99999.6394;
+			};
+		}
+
 		jQuery.now = Date.now;


Not directly germane, but I don't think this line is useful.

It turns out it was useful as jQuery.now is defined as Date.now so if you mock Date.now jQuery.now would remain unmocked.

This is yet another manifestation of how broken our current mocking that happens after jQuery has already loaded & run really is. :/

gibson042 · 2016-07-08T18:58:24Z

src/effects.js

+// We need to be using jQuery.now() or performance.now() consistently as they return different
+// values: performance.now() counter starts on page load.
+// Support: IE <10, Safari <8.0, iOS <9, Android <4.4, Node with jsdom 9.4
+function getTimestamp() {


Given the comments, shouldn't we have something more like this?

getTimestamp = window.performance && typeof window.performance.now === "function" ? // or window.performance.now.bind( window.performance ) jQuery.proxy( window.performance, "now" ) : jQuery.now;

If the complaint is about testing, then we just need an iframe test in which the sandbox is mocked before loading jQuery.

I guess it should work, I'll try it.

Unfortunately, test setup is run after jQuery is sourced so this would freeze getTimestamp to use the initial, non-mocked window.performance.

I could put the if outside of the function definition but that only adds 3 bytes and I suppose such a single if will not cause any grief for any relatively modern browser.

The fact that we can't reliably mock stuff before jQuery is loaded is obviously a problem but this is an issue bigger than this PR.

mgol · 2016-07-21T22:44:03Z

@timmywil This is not a new feature so it could move it to 3.1.1, couldn't we?

markelog · 2016-07-21T22:49:22Z

Did we already agree on landing it? Can we have conformation from mozilla people on this?

mgol · 2016-07-21T22:55:56Z

I guess there wasn't a definite decision, only +1s from @dmethvin & @gibson042: https://irc.jquery.org/%23jquery-meeting/default_%23jquery-meeting_20160718.log.html#t12:53:43

There's a patch being prepared for Firefox in https://bugzilla.mozilla.org/show_bug.cgi?id=1278408.

dmethvin · 2016-07-21T23:06:31Z

As I understood the results, it didn't make any browser animate any jerkier. It just didn't help Firefox animate any smoother, but it helped Chrome a lot. If that's the case I'd say it's still good for 3.2.

markelog · 2016-07-21T23:53:10Z

My concern with with frames dump, judging by the open tickets and because none of the other animate libraries used it seems issue is still present.

So my understanding is that we want to battle test it and ignore accepted practise and vendor bugs?

dmethvin · 2016-07-22T01:40:31Z

So my understanding is that we want to battle test it and ignore accepted practise and vendor bugs?

If there's a reason that GSAP or Velocity don't use it, we don't currently know that reason. I don't think that's ignoring accepted practice. Who knows, maybe it was too flakey back when they looked at this. @julianshapiro are there specific reasons Velocity doesn't use the rAF timestamp?

The only vendor bug I see mentioned is https://bugzilla.mozilla.org/show_bug.cgi?id=1278408 which is just saying that right now this change won't improve Firefox.

markelog · 2016-07-22T04:26:51Z

We already received comments from @julianshapiro - #3143 (comment), you want them to be more descriptive?

Furthermore we have explanation in description ticket - #3143

We ignore RAF's high resolution timestamp since it can be significantly offset
when the browser is under high stress; we opt for choppiness over allowing the browser to drop huge chunks of frames.

I don't think that's ignoring accepted practice

This is how currently those animation libs work, i'm not sure if anyone else is doing what we are trying do to, on the contrary, so i'm not sure what do you mean by this

which is just saying that right now this change won't improve Firefox.

Mm, not sure what do you mean - https://bugzilla.mozilla.org/show_bug.cgi?id=1278408#c0

...the jitter is a problem for tweening code like jQuery's $.animate, because they'd ideally like to calculate tweening values based on the timestamp argument. The jitter then causes animations to feel slightly choppy even at 60fps. Here's a discussion on the jQuery bug tracker: #3143

dmethvin · 2016-07-22T12:57:52Z

We already received comments from @julianshapiro - #3143 (comment), you want them to be more descriptive?

I interpreted the @joliss reply in #3143 (comment) as saying that although dropping frames is worse, @joliss didn't see it dropping frames.

timmywil · 2016-07-22T14:36:06Z

@mgol I see this as an enhancement, which is closer to a feature than a bug fix, so I prefer it be landed in 3.2.

markelog · 2016-07-23T18:05:11Z

@joliss didn't see it dropping frames.

I don't think these conditions were met -

it can be significantly offset when the browser is under high stress

That's why I would want to have conformation from the browser people - was it happening? How it looks like? Was it fixed?

mgol · 2017-03-15T15:34:52Z

I've removed the "Needs review" label as I first have to debug the Chrome underlying issue (& perhaps report it upstream).

In some environments that support the requestAnimationFrame timestamp callback parameter using it results in smoother animations. Note: the rAF timestamp is using the same API as performance.now() under the hood so they're compatible with each other. However, some browsers support rAF (with a timestamp parameter) but not performance.now() so using them both would introduce an error. This commit stops using rAF in browsers that don't support performance.now(). From all the browsers jQuery supports this only affects iOS <9 (currently less than 5% of all iOS users) which will now not use rAF. Fixes jquerygh-3143 Closes jquerygh-3151

The PR is not ready so it will need another review later

In some environments that support the requestAnimationFrame timestamp callback parameter using it results in smoother animations. Note: the rAF timestamp is using the same API as performance.now() under the hood so they're compatible with each other. However, some browsers support rAF (with a timestamp parameter) but not performance.now() so using them both would introduce an error. This commit stops using rAF in browsers that don't support performance.now(). From all the browsers jQuery supports this only affects iOS <9 (currently less than 5% of all iOS users) which will now not use rAF. Fixes jquerygh-3143 Closes jquerygh-3151

In some environments that support the requestAnimationFrame timestamp callback parameter using it results in smoother animations. Note: the rAF timestamp is using the same API as performance.now() under the hood so they're compatible with each other. However, some browsers support rAF (with a timestamp parameter) but not performance.now() so using them both would introduce an error. This commit stops using rAF in browsers that don't support performance.now(). From all the browsers jQuery supports this only affects iOS <9 (currently less than 5% of all iOS users) which will now not use rAF. Fixes gh-3143 Closes gh-3151

jquerybot added the CLA: Valid label Jun 8, 2016

mgol mentioned this pull request Jun 8, 2016

Use RAF's timestamp for smoother animations #3143

Open

scottgonzalez changed the title ~~[WIP] Effects: use requestAnimationTime timestamp if available~~ [WIP] Effects: use requestAnimationFrame timestamp if available Jun 8, 2016

mgol force-pushed the mgol:raf-timestamp branch from 51b27b0 to c0d0594 Jun 8, 2016

dmethvin reviewed Jun 8, 2016
View changes

markelog reviewed Jun 9, 2016
View changes

mgol added the Needs review label Jun 22, 2016

mgol force-pushed the mgol:raf-timestamp branch 2 times, most recently from 037ce12 to 03d873c Jul 6, 2016

gibson042 reviewed Jul 8, 2016
View changes

timmywil added this to the 3.2.0 milestone Jul 13, 2016

mgol self-assigned this Jul 18, 2016

dmethvin closed this Jul 22, 2016

dmethvin reopened this Jul 22, 2016

timmywil modified the milestones: 3.4.0, 3.3.0 Jul 24, 2017

mgol mentioned this pull request Aug 23, 2017

Test failure: "effects: jQuery.easing" jquery/jquery-migrate#255

Closed

mgol force-pushed the mgol:raf-timestamp branch from 734429e to 5664e4f Oct 12, 2017

mgol removed this from the 3.4.0 milestone Oct 3, 2018

mgol force-pushed the mgol:raf-timestamp branch from 5664e4f to eca2f57 Jan 17, 2019

mgol force-pushed the mgol:raf-timestamp branch from eca2f57 to 05a0027 Mar 4, 2019

mgol force-pushed the mgol:raf-timestamp branch from 05a0027 to 20e8501 Mar 11, 2019

mgol changed the title ~~Effects: use requestAnimationFrame timestamp if available~~ [WIP] Effects: use requestAnimationFrame timestamp if available Mar 27, 2019

mgol force-pushed the mgol:raf-timestamp branch from 20e8501 to 590fae3 Apr 17, 2019

mgol force-pushed the mgol:raf-timestamp branch from 590fae3 to 7708b4b Apr 23, 2019

mgol force-pushed the mgol:raf-timestamp branch from 7708b4b to 30e6b8a Apr 23, 2019

mgol force-pushed the mgol:raf-timestamp branch from 30e6b8a to f089df5 Apr 29, 2019

mgol force-pushed the mgol:raf-timestamp branch from f089df5 to 78e6282 Apr 30, 2019

mgol force-pushed the mgol:raf-timestamp branch from 78e6282 to 6fd73d0 Aug 30, 2019

mgol marked this pull request as draft Apr 10, 2020

mgol changed the title ~~[WIP] Effects: use requestAnimationFrame timestamp if available~~ Effects: use requestAnimationFrame timestamp if available Apr 10, 2020

Base automatically changed from master to main Feb 1, 2021

jquery / jquery

Effects: use requestAnimationFrame timestamp if available #3151

Effects: use requestAnimationFrame timestamp if available #3151

mgol commented Jun 8, 2016 •

edited

scottgonzalez commented Jun 8, 2016

mgol commented Jun 8, 2016

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

markelog commented Jun 9, 2016 •

edited

markelog commented Jun 9, 2016

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mgol commented Jul 21, 2016

markelog commented Jul 21, 2016

mgol commented Jul 21, 2016

dmethvin commented Jul 21, 2016

markelog commented Jul 21, 2016

dmethvin commented Jul 22, 2016 •

edited

markelog commented Jul 22, 2016 •

edited

dmethvin commented Jul 22, 2016

timmywil commented Jul 22, 2016

markelog commented Jul 23, 2016

mgol commented Mar 15, 2017

jquery / jquery

Effects: use requestAnimationFrame timestamp if available #3151

Are you sure you want to change the base?

Effects: use requestAnimationFrame timestamp if available #3151

Conversation

mgol commented Jun 8, 2016 • edited

Summary

Checklist

scottgonzalez commented Jun 8, 2016

mgol commented Jun 8, 2016

This comment has been minimized.

dmethvin Jun 8, 2016 Member

This comment has been minimized.

mgol Jun 8, 2016 Author Member

This comment has been minimized.

markelog Jun 9, 2016 • edited Member

This comment has been minimized.

mgol Jun 9, 2016 Author Member

This comment has been minimized.

markelog Jun 9, 2016 • edited Member

This comment has been minimized.

mgol Jun 9, 2016 Author Member

This comment has been minimized.

mgol Jul 6, 2016 Author Member

This comment has been minimized.

markelog Jun 9, 2016 Member

This comment has been minimized.

mgol Jun 9, 2016 Author Member

This comment has been minimized.

markelog Jun 9, 2016 Member

This comment has been minimized.

mgol Jun 9, 2016 Author Member

This comment has been minimized.

markelog Jun 9, 2016 Member

This comment has been minimized.

mgol via email Jun 9, 2016 Author Member

This comment has been minimized.

mgol Jul 6, 2016 Author Member

markelog commented Jun 9, 2016 • edited

markelog commented Jun 9, 2016

This comment has been minimized.

gibson042 Jul 8, 2016 Member

This comment has been minimized.

mgol Mar 9, 2017 Author Member

This comment has been minimized.

gibson042 Jul 8, 2016 Member

This comment has been minimized.

mgol Jul 21, 2016 Author Member

This comment has been minimized.

mgol Dec 19, 2016 Author Member

This comment has been minimized.

mgol Mar 8, 2017 Author Member

mgol commented Jul 21, 2016

markelog commented Jul 21, 2016

mgol commented Jul 21, 2016

dmethvin commented Jul 21, 2016

markelog commented Jul 21, 2016

dmethvin commented Jul 22, 2016 • edited

markelog commented Jul 22, 2016 • edited

dmethvin commented Jul 22, 2016

timmywil commented Jul 22, 2016

markelog commented Jul 23, 2016

mgol commented Mar 15, 2017

mgol commented Jun 8, 2016 •

edited

dmethvin Jun 8, 2016
Member

mgol Jun 8, 2016
Author Member

markelog Jun 9, 2016 •

edited

Member

mgol Jun 9, 2016
Author Member

markelog Jun 9, 2016 •

edited

Member

mgol Jun 9, 2016
Author Member

mgol Jul 6, 2016
Author Member

markelog Jun 9, 2016
Member

mgol Jun 9, 2016
Author Member

markelog Jun 9, 2016
Member

mgol Jun 9, 2016
Author Member

markelog Jun 9, 2016
Member

mgol via email Jun 9, 2016
Author Member

mgol Jul 6, 2016
Author Member

markelog commented Jun 9, 2016 •

edited

gibson042 Jul 8, 2016
Member

mgol Mar 9, 2017
Author Member

gibson042 Jul 8, 2016
Member

mgol Jul 21, 2016
Author Member

mgol Dec 19, 2016
Author Member

mgol Mar 8, 2017
Author Member

dmethvin commented Jul 22, 2016 •

edited

markelog commented Jul 22, 2016 •

edited