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

core(non-composited-animations): add CLS savings as always 0 #15099

Merged
merged 3 commits into from
Jun 15, 2023

Conversation

adamraine
Copy link
Member

This PR attempts to estimate the CLS impact of non-composited animations by using the CLS score of the node being animated. Unfortunately, this method can have false positives and false negatives:

  • False positive example: The animated node had a layout shift but this shift wasn't caused by the animation
  • False negative example: An animation shifts content below the animated node, in which case the CLS score is applied to the nodes below but not the animated node itself

One alternatives is to just estimate the CLS impact of this audit as 0 all the time. Guaranteeing layout-shift-elements always has the highest impact doesn't seem like a bad idea.

@adamraine adamraine requested a review from a team as a code owner May 19, 2023 22:23
@adamraine adamraine requested review from brendankenny and removed request for a team May 19, 2023 22:23
@brendankenny
Copy link
Member

One alternatives is to just estimate the CLS impact of this audit as 0 all the time. Guaranteeing layout-shift-elements always has the highest impact doesn't seem like a bad idea.

I feel like this is the best option because it won't be applicable for a large majority of sites anyways (have to have non-composited animations and have the animated elements be layout shifted), and for the ones where it will apply our estimate could be pretty wrong for the reasons you mention.

Too much additional info:

I took a look at CLS and non-composited animations last year (colab here, sorry to external observers, limited access), part of which was how much page CLS could be directly attributed to these animated nodes (section titled "CLS from animated nodes").

87% of the 13.5M pages with a successful LH run in the October 2022 dataset had some amount of CLS, and while the presence of non-composited animations was a pretty good predictor of CLS, only 6% of all pages had any nodes with both CLS attributed and a non-composited animation.

(one important caveat, until the fix in #15067, we were over counting node-level CLS, because we weren't accounting for had_recent_input in TraceElements. e.g. for about 10% of pages with animated nodes with CLS, the total node-level CLS exceeded the page's CLS, which isn't possible. After that fix, the 6% is presumably slightly lower, but we won't have data on that for a while)

So, whatever we decide to do, this won't affect ~94% of the top landing pages out there.

Looking at just the 6% of pages with nodes that had both a non-composited animation and CLS:

  • the median page had 27% or less of the page-level CLS made up of the animated-node CLS
  • 40% of those pages had 50% of their page-level CLS made up of animated-node CLS
  • 27% of those pages had 100% of their page-level CLS made up of animated-node CLS

So we could say maybe 3% of pages will have a quarter or more of their CLS explained by this audit (lower now due to #15067, though probably not a lot lower), except only some non-composited animations will cause layout shifts (e.g. the most common non-composited animated property is background-color, which obviously won't change node positions).

If we limit ourselves to the 4% of pages with nodes with both CLS and any animated property that could possibly (but not necessarily) cause a layout shift

  • 58% of those pages had a quarter or more of their CLS explained by this audit
  • 48% of those page had 50% of their page-level CLS made up of shiftable-animated-node CLS
  • 32% had 100% of their page-level CLS explained

So it narrows it down to about 2% of all pages with a quarter or more of their CLS explained with this audit. In addition to the CLS fix, we would probably narrow that further because of the false positives you mentioned, and because many of these properties that could cause a layout shift can't cause a layout shift on the node itself (e.g. animating margin-top can cause the node to get CLS, but animating margin-bottom can cause CLS, but not on the node itself), so the node's CLS contribution has to be from something else.

We can also make a ballpark estimate of at least one class of false negatives based on nodes with CLS and animated properties that couldn't possibly be causing layout shifts:

  • 2% of all pages had some amount of CLS on nodes with an animation that couldn't have caused the CLS
  • 1% of all pages had a quarter or more of their CLS covered by animations that couldn't have caused the CLS
  • 0.4% of all pages had 100% of their CLS covered by non-composited animations that couldn't have caused the CLS

@connorjclark connorjclark changed the title core(non-composited-animations): add CLS savings core(non-composited-animations): add CLS savings as always 0 Jun 15, 2023
@adamraine adamraine merged commit dffeb67 into main Jun 15, 2023
@adamraine adamraine deleted the composit-anim-savings branch June 15, 2023 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants