Bug 188050 - [Web Animations] Implied keyframes should not account for animations on siblings
Summary: [Web Animations] Implied keyframes should not account for animations on siblings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-07-26 07:24 PDT by Antoine Quint
Modified: 2020-12-15 06:21 PST (History)
5 users (show)

See Also:


Attachments
Test case (298 bytes, text/html)
2018-07-26 07:24 PDT, Antoine Quint
no flags Details
Test case with changes of underlying value (1.33 KB, text/html)
2018-07-26 14:12 PDT, Antoine Quint
no flags Details
Better test case with dynamic changes of underlying value (1.90 KB, text/html)
2018-07-27 15:57 PDT, Antoine Quint
no flags Details
Patch (162.40 KB, patch)
2020-12-14 13:22 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (161.34 KB, patch)
2020-12-15 03:25 PST, Antoine Quint
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (168.67 KB, patch)
2020-12-15 05:44 PST, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2018-07-26 07:24:58 PDT
Created attachment 345842 [details]
Test case

See the attached example, five <div> with the exact same animation applied at different rates as if they had different timing functions.
Comment 1 Antoine Quint 2018-07-26 07:51:21 PDT
We're using the last animated value as the value for the missing 0% keyframe instead of using a snapshot of the unanimated style when the animation started.
Comment 2 Antoine Quint 2018-07-26 08:09:58 PDT
This only happens with the Web Animations API, CSS transitions and CSS animations are unaffected.
Comment 3 Antoine Quint 2018-07-26 14:08:16 PDT
The crux of the problem is that when we have an implied keyframe, such as a missing "from" in the test case, we use the provided pre-blend target style for the current animation, but this style may be affected by prior animations blending before it.

In the case of CSS Animations, it's a little different, since we compute implied keyframes up-front at the time the animations are created. So if the value at creation time and the value at start time are the same, everything looks right, but if the value changes during the delay phase for instance, or even in-flight, then we're wrong too.
Comment 4 Antoine Quint 2018-07-26 14:12:29 PDT
Created attachment 345870 [details]
Test case with changes of underlying value

In the additional test case we show how changing the underlying value during the delay phase and the running phases should alter the course of the animation. In Firefox and Chrome Canary, the changes are accounted for. In WebKit, we completely disregard it.
Comment 5 Antoine Quint 2018-07-27 15:57:31 PDT
Created attachment 345965 [details]
Better test case with dynamic changes of underlying value
Comment 6 Radar WebKit Bug Importer 2020-11-12 09:50:13 PST
<rdar://problem/71330806>
Comment 7 Antoine Quint 2020-11-12 09:50:51 PST
I think this bug might have a big impact on WPT where we have the wrong behavior with neutral values.
Comment 8 Antoine Quint 2020-12-14 06:50:50 PST
The main issue that is causing the original test case to fail is that style is being shared between siblings. The fix for this is to get SharingResolver::resolve() to return null should the element be targeted by an animation that is using implicit keyframes.

I have a work-in-progress fix for this which yields quite a few WPT progressions.

Looking further into implicit keyframes, and although this test does not cover it, I think we are doing the wrong thing in KeyframeEffect::setAnimatedPropertiesInStyle() when picking the style to use if there is no explicit style for the interval keyframes indexes. We should be using the lastStyleChangeEventStyle() here to avoid other animations in the stack producing values being picked up for implicit keyframe styles.
Comment 9 Antoine Quint 2020-12-14 13:22:03 PST
Created attachment 416191 [details]
Patch
Comment 10 Antoine Quint 2020-12-15 03:25:01 PST
Created attachment 416232 [details]
Patch
Comment 11 Antoine Quint 2020-12-15 05:44:06 PST
Created attachment 416238 [details]
Patch for landing
Comment 12 EWS 2020-12-15 06:14:47 PST
Committed r270837: <https://trac.webkit.org/changeset/270837>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416238 [details].