Skip to content
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

Effects: use requestAnimationFrame timestamp if available #3151

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mgol
Copy link
Member

@mgol mgol commented Jun 8, 2016

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.

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

@scottgonzalez scottgonzalez changed the title [WIP] Effects: use requestAnimationTime timestamp if available [WIP] Effects: use requestAnimationFrame timestamp if available Jun 8, 2016
@scottgonzalez
Copy link
Member

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

@mgol
Copy link
Member Author

mgol commented Jun 8, 2016 via email

}
}

// 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() {
Copy link
Member

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?

Copy link
Member Author

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?

@markelog
Copy link
Member

markelog commented Jun 9, 2016

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

@markelog
Copy link
Member

markelog commented Jun 9, 2016

@mgol mgol force-pushed the raf-timestamp branch 2 times, most recently from 037ce12 to 03d873c Compare July 7, 2016 16:07
return Date.now() - 99999.6394;
};
}

jQuery.now = Date.now;
Copy link
Member

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.

Copy link
Member Author

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 timmywil added this to the 3.2.0 milestone Jul 13, 2016
@mgol mgol self-assigned this Jul 18, 2016
@mgol
Copy link
Member Author

mgol commented Jul 21, 2016

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

@markelog
Copy link
Member

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

@mgol
Copy link
Member Author

mgol commented Jul 21, 2016

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

dmethvin commented Jul 22, 2016

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
Copy link
Member

markelog commented Jul 22, 2016

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
Copy link
Member

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.

@dmethvin dmethvin closed this Jul 22, 2016
@dmethvin dmethvin reopened this Jul 22, 2016
@timmywil
Copy link
Member

@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
Copy link
Member

@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 mgol removed this from the 3.4.0 milestone Oct 3, 2018
@mgol mgol dismissed markelog’s stale review December 12, 2018 16:34

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

mgol added a commit to mgol/jquery that referenced this pull request Jan 17, 2019
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
mgol added a commit to mgol/jquery that referenced this pull request Mar 4, 2019
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
mgol added a commit to mgol/jquery that referenced this pull request Mar 11, 2019
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
@mgol mgol changed the title Effects: use requestAnimationFrame timestamp if available [WIP] Effects: use requestAnimationFrame timestamp if available Mar 27, 2019
mgol added a commit to mgol/jquery that referenced this pull request Apr 17, 2019
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
mgol added a commit to mgol/jquery that referenced this pull request Apr 23, 2019
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
mgol added a commit to mgol/jquery that referenced this pull request Apr 23, 2019
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
mgol added a commit to mgol/jquery that referenced this pull request Apr 29, 2019
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
mgol added a commit to mgol/jquery that referenced this pull request Apr 30, 2019
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
@mgol mgol marked this pull request as draft April 10, 2020 16:24
@mgol 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 February 1, 2021 22:02
@mgol
Copy link
Member Author

mgol commented Sep 17, 2021

Closing & re-opening the PR to trigger the EasyCLA check...

@mgol mgol closed this Sep 17, 2021
@mgol mgol reopened this Sep 17, 2021
@LifeIsStrange
Copy link

LifeIsStrange commented Aug 20, 2022

@mgol What's blocking this PR? It seems it would be a great performance/ux improvement

@mgol
Copy link
Member Author

mgol commented Aug 20, 2022

@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
Copy link

@mgol I wanted to thank you for your excellent work, not all heroes wear capes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Use RAF's timestamp for smoother animations
9 participants