-
Notifications
You must be signed in to change notification settings - Fork 20.6k
Effects: use requestAnimationFrame timestamp if available #3151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There's a typo in your commit message, see the title change I just made. |
Thanks, fixed.
|
} | ||
} | ||
|
||
// 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() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just use jQuery.now()
for all the browsers that don't have the rAF timestamp? Which browsers are affected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is also used in createFxNow()
which, I think, is not limited to old browsers?
I am skeptical, since neither gsap nor velocity doing this and there is open tickets in vendor trackers about this |
/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 |
037ce12
to
03d873c
Compare
test/unit/effects.js
Outdated
return Date.now() - 99999.6394; | ||
}; | ||
} | ||
|
||
jQuery.now = Date.now; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly germane, but I don't think this line is useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. :/
@timmywil This is not a new feature so it could move it to |
Did we already agree on landing it? Can we have conformation from mozilla people on this? |
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. |
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. |
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? |
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. |
We already received comments from @julianshapiro - #3143 (comment), you want them to be more descriptive? Furthermore we have explanation in description ticket - #3143
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
Mm, not sure what do you mean - https://bugzilla.mozilla.org/show_bug.cgi?id=1278408#c0
|
I interpreted the @joliss reply in #3143 (comment) as saying that although dropping frames is worse, @joliss didn't see it dropping frames. |
@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. |
I don't think these conditions were met -
That's why I would want to have conformation from the browser people - was it happening? How it looks like? Was it fixed? |
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 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 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 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 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 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 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 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 jquerygh-3143 Closes jquerygh-3151
Closing & re-opening the PR to trigger the EasyCLA check... |
@mgol What's blocking this PR? It seems it would be a great performance/ux improvement |
@LifeIsStrange mainly the lack of my time to investigate all the issues with using the time stamp, other priorities & cross-browser issues. There were e.g. some ordering issues in Edge Legacy which makes this infeasible to land in v3. |
@mgol I wanted to thank you for your excellent work, not all heroes wear capes :) |
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 increateFxNow()
. 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.New tests have been added to show the fix or feature worksIf needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.comrAF
/Date.now()
Thanks! Bots and humans will be around shortly to check it out.