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

[Feature policy: animations] Compositor accelerated animations should not be allowed #10

Open
birtles opened this issue Aug 20, 2018 · 6 comments
Labels
duplicate This issue or pull request already exists

Comments

@birtles
Copy link

birtles commented Aug 20, 2018

As per my message to blink-dev I have strong concerns with this feature allowing compositor animations but disallowing others including:

  • Doing so would require either adopting a lowest common denominator approach (whereby Blink would be required to disallow filter animations, contrary to what the explainer currently states) or would produce different results in different engines (creating a Web compatibility nightmare since content that, for example, waits for a transitionend from a certain animation might never get it).

  • Using a whitelist of properties puts the burden on site maintainers to update (or forever hold Web content back) and will quickly become unwieldy. In 2019 authors might need to write:

<iframe allow="animations(transform,translate,scale,rotate,opacity,background-color,filter,offset,offset-distance,clip-path)"></iframe>
  • The conditions that govern whether a particular animation can be run on the compositor are very subtle and change over time. As a result even if a property is part of the set of "lower common denominator" properties or an author-specified whitelist, it might not be able to be animated on the compositor. That seems to undermine the whole purpose of this feature policy which is to "ensure smooth animations".

I suggest this feature policy should restrict all declarative animations which would at least make it useful for limiting CPU usage.

@ehsan-karamad
Copy link

Thanks for posting here as well. I think the explainer needs revisions (as also requested in issue w3c/webappsec-permissions-policy#203).

I will try to respond to the concerns in line. Hopefully more people familiar with animations and web platform will chime in.

Doing so would require either adopting a lowest common denominator approach (whereby Blink would be required to disallow filter animations, contrary to what the explainer currently states)

Thanks. Based on the idea that animations policy should eventually be parametric, I believe the lowest common denominator approach is not necessarily an issue since it will be up to the developer to select the white list. If however we go for a non-parametric version and common denominator, then filter should not be part of the policy at this time.

or would produce different results in different engines (creating a Web compatibility nightmare since content that, for example, waits for a transitionend from a certain animation might never get it).

I don't think allowing for arbitrarily different behaviors across the browsers is the goal here. If an animation is disallowed it should still fire the same events but only jump from initial state to final state
(as discussed in issue w3c/webappsec-permissions-policy#204 ). If it is allowed, it should run as expected regardless of it being actually composited.

On that note, if a website relies on something such as the computed style during the animation then there could be a problem.

Using a whitelist of properties puts the burden on site maintainers to update (or forever hold Web content back) and will quickly become unwieldy. In 2019 authors might need to write:

Yes admittedly this is an inconvenience, but, it also provides granularity in setting what is (not) allowed.

For development and testing I wouldn't think compiling a long list of animations is too taxing. I think this feature is very useful for that purpose.

If the feature is used for performance reasons, I agree that compiling an up-to-date list is cumbersome; but I'd argue that not having a perfect list is no worse (if not better) than not using the feature altogether. Also I'd assume a typical use case for the feature is setting it in the HTTP headers as opposed to individually setting allow for each <iframe>.

Also a random idea which might partially alleviate this problem: I am wondering if it also makes sense to use wildcards or even coin new terms for a selecting a group of animations - e.g., border-*-color or transforms for all of transform,translate,scale,rotate.

The conditions that govern whether a particular animation can be run on the compositor are very subtle and change over time.

I don't believe running on the compositor is (should be) a condition here (I will try to update the explainer soon to reflect the correct expectation from animations blocked/allowed). Current experimental implementation in Chromium only includes the set of composited animations (on Chrome) as those included with the policy. However the policy is expected to be parametric anyway.

I also think composited should refer to animations that would normally run on compositor. When a certain animation is allowed, then it should run as expected on the page; regardless of whether or not it actually runs composited.

I suggest this feature policy should restrict all declarative animations which would at least make it useful for limiting CPU usage.

This is a very good suggestion and could be introduced as a special use case of the parametric version.

To disable all animations for a frame, perhaps something like:

<iframe allow="animations() https://example.com'"></iframe>

would disallow all animations for https://example.com. To allow all animations for an origin we should just not set the feature for the origin.

@birtles
Copy link
Author

birtles commented Aug 21, 2018

I don't think allowing for arbitrarily different behaviors across the browsers is the goal here.

On the contrary, I think we very much want behavior to be consistent between browsers.

I'm still concerned about this policy. It might be easier to support if very specific and compelling use cases are made available.

I had thought it might be possible to try defining the set of allowed animations as something less engine-specific than, "Can be animated on the compositor" and more spec-related such as, "Does not influence layout" but there is feedback from developers that sometimes an animation that triggers layout can still perform better than one that runs on the compositor.

@ehsan-karamad
Copy link

On the contrary, I think we very much want behavior to be consistent between browsers.

Yes, this is exactly what I meant (sorry for convoluted wording).

I'm still concerned about this policy. It might be easier to support if very specific and compelling use cases are made available.

I think so far I can think of two use cases for this policy:

  • Disable all or most animations on a page to reduce CPU usage. If the set of enabled animations is something as small as {transform(s), opacity} this policy could reduce CPU load by disabling the rest. Even going for disable all animations as v0 would serve that purpose.
  • The parametric version can be used in test and measurement by the developers.

I had thought it might be possible to try defining the set of allowed animations as something less engine-specific than, "Can be animated on the compositor" and more spec-related such as, "Does not influence layout" but there is feedback from developers that sometimes an animation that triggers layout can still perform better than one that runs on the compositor.

I agree "being composited" or "does not influence layout" do not, universally, mean that performance gets better. But the parametric policy provides a reasonable tool to the developers to try and match different combinations that works best for their (possibly-too-large-to-individually-change-animations) project.

I think we should be more clear on what we expect in terms of performance here. On one hand, performance might hint at smoother animations; as argued this does not necessarily equal being composited. On the other hand, responsiveness of page could be improved by offloading or disabling some of the main thread work. The latter measure of performance seems to suggest animations that could potentially offload some work on GPU and/or a separate (compositor) thread (depending on the browser and some other factors mentioned before) should be prioritized over those that mainly run on CPU and the main thread.

@birtles
Copy link
Author

birtles commented Aug 21, 2018

I think so far I can think of two use cases for this policy:

Disable all or most animations on a page to reduce CPU usage. If the set of enabled animations is something as small as {transform(s), opacity} this policy could reduce CPU load by disabling the rest. Even going for disable all animations as v0 would serve that purpose.
The parametric version can be used in test and measurement by the developers.

These aren't particularly strong use cases. What is the motivation for this feature in the first place? Is there a business need? Customer request? etc.

@ehsan-karamad
Copy link

The initial motivation as I understand is blocking animations from using up CPU cycles (for main frame responsiveness).

As for an application right now I can think of <amp-carousel>s and <portal>s. I am not aware of any customer requests.

@ehsan-karamad
Copy link

To conclude this thread so far:

  • To have a feature to limit use of declarative animations is useful because

    1. It reduces CPU usage and makes the page more responsive to users,
    2. Potential use <portal> and <amp-carousel> (search).
    3. Powerful tool for developers of larger website project.
  • Allowing only composited animations is not the best practice here as this set is not the same for all browsers.

I will update the explainer and ping you to kindly take another look. The interim goal still is to come up with a set of animations to disable when the policy is used. The set will include the animations that cause layout (or can cause layout).

ehsan-karamad referenced this issue in ehsan-karamad/feature-policy Aug 23, 2018
Revised the animations policy to propose a modified policy that blocks layout inducing animations as opposed to the non-composited animations.

The changed is motivated discussions in issues #202, #203, and #204.
clelland referenced this issue in w3c/webappsec-permissions-policy Sep 20, 2018
Revised the animations policy to propose a modified policy that blocks layout inducing animations as opposed to the non-composited animations.

The changed is motivated discussions in issues #202, #203, and #204.
@clelland clelland transferred this issue from w3c/webappsec-permissions-policy Dec 1, 2020
@clelland clelland added the duplicate This issue or pull request already exists label Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants