Bug 215807 - REGRESSION (r263506): timing of CSS Animation on https://animate.style is incorrect
Summary: REGRESSION (r263506): timing of CSS Animation on https://animate.style is inc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL: https://animate.style
Keywords: InRadar
: 218224 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-08-25 07:59 PDT by Antoine Quint
Modified: 2020-11-02 03:35 PST (History)
4 users (show)

See Also:


Attachments
Patch (10.36 KB, patch)
2020-08-25 08:19 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for EWS (7.66 KB, patch)
2020-08-27 04:05 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (10.54 KB, patch)
2020-08-27 05:55 PDT, Antoine Quint
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2020-08-25 07:59:06 PDT
If you go to animate.style you'll notice the animations are playing faster than they do in other browsers (Chrome and Firefox for instance).
Comment 1 Antoine Quint 2020-08-25 07:59:27 PDT
<rdar://problem/66770136>
Comment 2 Antoine Quint 2020-08-25 08:19:41 PDT
Created attachment 407189 [details]
Patch
Comment 3 Simon Fraser (smfr) 2020-08-25 08:27:04 PDT
Comment on attachment 407189 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        In r263506, we added a way for accelerated animations to adhere to both a timing function set on the
> +        animation, affecting the timing of the entire animation, as well as a timing function set on individual
> +        keyframes, affecting the timing of the animation in a given interval.

That's a weird way of saying that accelerated animation timing functions were broken.

> Source/WebCore/animation/KeyframeEffect.cpp:1662
>  Ref<const Animation> KeyframeEffect::backingAnimationForCompositedRenderer() const

It's weird that this has to exist, and weird that it returns a Ref<const Animation>.

> LayoutTests/webanimations/accelerated-css-animation-with-easing.html:54
> +    await new Promise(requestAnimationFrame);
> +    await new Promise(requestAnimationFrame);
> +    await new Promise(requestAnimationFrame);

I feel like this test will be flakey.
Comment 4 Antoine Quint 2020-08-25 08:29:28 PDT
(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 407189 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407189&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        In r263506, we added a way for accelerated animations to adhere to both a timing function set on the
> > +        animation, affecting the timing of the entire animation, as well as a timing function set on individual
> > +        keyframes, affecting the timing of the animation in a given interval.
> 
> That's a weird way of saying that accelerated animation timing functions
> were broken.

Not *all* of them, it was required to have more than 2 keyframes and use implicit timing functions on keyframes.

> > Source/WebCore/animation/KeyframeEffect.cpp:1662
> >  Ref<const Animation> KeyframeEffect::backingAnimationForCompositedRenderer() const
> 
> It's weird that this has to exist, and weird that it returns a Ref<const
> Animation>.

Agreed. When we remove the legacy animation code (real soon now) we can remove the use of Animation here and use WebAnimation objects instead.

> > LayoutTests/webanimations/accelerated-css-animation-with-easing.html:54
> > +    await new Promise(requestAnimationFrame);
> > +    await new Promise(requestAnimationFrame);
> > +    await new Promise(requestAnimationFrame);
> 
> I feel like this test will be flakey.

This was based on webanimations/accelerated-animation-with-easing.html which proved to be solid after some initial issues were ironed out.
Comment 5 Antoine Quint 2020-08-27 04:05:23 PDT
Created attachment 407388 [details]
Patch for EWS
Comment 6 Antoine Quint 2020-08-27 05:55:18 PDT
Created attachment 407393 [details]
Patch
Comment 7 Antoine Quint 2020-08-27 10:02:35 PDT
Committed r266241: <https://trac.webkit.org/changeset/266241>
Comment 8 Antoine Quint 2020-11-02 03:35:20 PST
*** Bug 218224 has been marked as a duplicate of this bug. ***