Summary: | REGRESSION (r260360): easing curves are broken on JS-originated animations | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Liam DeBeasi <ldebeasi> | ||||||||||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | darin, dino, graouts, graouts, jonlee, Lawrence.j, niklasmerz, simon.fraser, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | Safari Technology Preview | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=210526 https://bugs.webkit.org/show_bug.cgi?id=213758 https://bugs.webkit.org/show_bug.cgi?id=215826 https://bugs.webkit.org/show_bug.cgi?id=215853 |
||||||||||||||||
Attachments: |
|
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.
*** Bug 213504 has been marked as a duplicate of this bug. *** This is specific to accelerated animations, animating margin-left instead has the intended effect. This most likely regressed with https://trac.webkit.org/changeset/260360/. Interesting, this makes it look like setting timing functions on individual keyframes overrides the CAAnimation-wide timing function. 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. How did this not break tests? I know we have tests for easing functions on accelerated animations. Because we have relatively few tests for the few features, such as this one, that only exist in Web Animations in the accelerated case. Created attachment 402647 [details]
Patch
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". Committed r263466: <https://trac.webkit.org/changeset/263466> (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. Reverted r263466 for reason: This commit caused 50+ crashes on multiple queues internally. Committed r263481: <https://trac.webkit.org/changeset/263481> Committed r263506: <https://trac.webkit.org/changeset/263506> 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 Created attachment 405998 [details]
Slower Transitions on iOS 14
Created attachment 405999 [details]
Working Transitions iOS 13
This change should be in STP 110. 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). |
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.