Bug 213495 – REGRESSION (r260360): easing curves are broken on JS-originated animations

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Liam DeBeasi]

Created attachment 402521 [details]
Code Reproduction

Easing curves are completely broken in WebKit on iOS 14 Beta 1 and Safari Tech Preview 108.

Steps to reproduce:

1. Open attached reproduction. This code animates a block with "ease" set for easing.
2. Notice that the animation runs with a linear curve.

I would expect that the square is animated with the easing curve that I specified.

I can only reproduce this with Web Animations. I cannot reproduce this with CSS Animations.

[Liam DeBeasi]

Created attachment 402522 [details]
The Actual Code Reproduction

Sorry, I uploaded the wrong file. Please check "The Actual Code Reproduction" for a reproduction of the issue :-). The old file I uploaded shows the code working properly with CSS Animations.

[Niklas Merz]

*** Bug 213504 has been marked as a duplicate of this bug. ***

[Radar WebKit Bug Importer]

<rdar://problem/64649747>

[Antoine Quint]

This is specific to accelerated animations, animating margin-left instead has the intended effect.

[Antoine Quint]

This most likely regressed with https://trac.webkit.org/changeset/260360/.

[Antoine Quint]

Interesting, this makes it look like setting timing functions on individual keyframes overrides the CAAnimation-wide timing function.

[Antoine Quint]

It doesn't look like we ever set the CAAnimation timingFunction but rather set timing function on the keyframes directly, at least this is what GraphicsLayerCA::timingFunctionForAnimationValue() looks like it's doing. This isn't surprising since until Web Animations there was no notion of an animation-wide timing function in CSS Animations and CSS Transitions. We'll need to add pluming to pass the animation-wide timing function to the CAAnimation.

[Simon Fraser (smfr)]

How did this not break tests? I know we have tests for easing functions on accelerated animations.

[Antoine Quint]

Because we have relatively few tests for the few features, such as this one, that only exist in Web Animations in the accelerated case.

[Antoine Quint]

Created attachment 402647 [details]
Patch

[Darin Adler]

Comment on attachment 402647 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402647&action=review

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3082
> +static bool isKeyframe(const KeyframeValueList& valueList)
> +{
> +    if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
> +        return valueList.size() > 1;
> +    return valueList.size() > 2;
> +}

In such a short function, I like the idea of an even shorter local variable name like "list".

[Antoine Quint]

Committed r263466: <https://trac.webkit.org/changeset/263466>

[Antoine Quint]

(In reply to Darin Adler from comment #11)
> Comment on attachment 402647 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402647&action=review
> 
> > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3082
> > +static bool isKeyframe(const KeyframeValueList& valueList)
> > +{
> > +    if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
> > +        return valueList.size() > 1;
> > +    return valueList.size() > 2;
> > +}
> 
> In such a short function, I like the idea of an even shorter local variable
> name like "list".

I renamed `valueList` as `list` in the commit.

[Jason Lawrence]

Reverted r263466 for reason:

This commit caused 50+ crashes on multiple queues internally.

Committed r263481: <https://trac.webkit.org/changeset/263481>

[Antoine Quint]

Committed r263506: <https://trac.webkit.org/changeset/263506>

[Niklas Merz]

Created attachment 405997 [details]
Comparison Safari, Technology Preview, Firefox

Is this really fixed or not yet deployed? I still see a difference in Safari Technology Preview and Safari 13. Esspecially page transitions which are using these animations are not good in our app.

The video compares the attached example from Liam. I tried to get the animation somewhat in sync. Top right is Safari Technolog Preview and it appears slower. Top left is Firefox and bottom right current Safari.

It's is really visible and bad for the user experience if you look at the animations the Ionic framework built for page transitions. Sample app: https://ionic-repro.web.app

[Niklas Merz]

Created attachment 405998 [details]
Slower Transitions on iOS 14

[Niklas Merz]

Created attachment 405999 [details]
Working Transitions iOS 13

[Simon Fraser (smfr)]

This change should be in STP 110.

[Liam DeBeasi]

The issue is not resolved in iOS 14 beta 4, but I can confirm that it is resolved in Safari Tech Preview (tested in STP 111).