- Notifications
You must be signed in to change notification settings - Fork20.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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
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 usejQuery.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 increateFxNow() which, I think, is not limited to old browsers?
markelog commentedJun 9, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I am skeptical, since neither gsap nor velocity doing this and there is open tickets in vendor trackers about this |
037ce12 to03d873cComparetest/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 asjQuery.now is defined asDate.now so if you mockDate.nowjQuery.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 inhttps://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? |
dmethvin commentedJul 22, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 ishttps://bugzilla.mozilla.org/show_bug.cgi?id=1278408 which is just saying that right now this change won't improve Firefox. |
markelog commentedJul 22, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
In some environments that support the requestAnimationFrame timestamp callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
In some environments that support the requestAnimationFrame timestamp callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
In some environments that support the requestAnimationFrame timestamp callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
In some environments that support the requestAnimationFrame timestamp callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
In some environments that support the requestAnimationFrame timestamp callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
In some environments that support the requestAnimationFrame timestamp callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
In some environments that support the requestAnimationFrame timestamp callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
In some environments that support the requestAnimationFrame timestamp callbackparameter using it results in smoother animations.Note: the rAF timestamp is using the same API as performance.now() under thehood so they're compatible with each other. However, some browsers support rAF(with a timestamp parameter) but not performance.now() so using them bothwould introduce an error. This commit stops using rAF in browsers that don'tsupport performance.now(). From all the browsers jQuery supports this onlyaffects iOS <9 (currently less than 5% of all iOS users) which will now notuse rAF.Fixesjquerygh-3143Closesjquerygh-3151
Closing & re-opening the PR to trigger the EasyCLA check... |
LifeIsStrange commentedAug 20, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@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. |
LifeIsStrange commentedAug 20, 2022
@mgol I wanted to thank you for your excellent work, not all heroes wear capes :) |
Uh oh!
There was an error while loading.Please reload this page.
Summary
In some environments that support the requestAnimationFrame timestamp callback
parameter using it results in smoother animations.
Fixesgh-3143
Currently it's +35 bytes, most likely due to the
getTimestampfunction 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 athttps://github.com/jquery/api.jquery.comrAF/Date.now()Thanks! Bots and humans will be around shortly to check it out.