Summary: | REGRESSION (r263506): scale transform transitions won't overshoot | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Guan-tin Chien [:timdream] <timdream> | ||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | dino, graouts, j, 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=213495 https://bugs.webkit.org/show_bug.cgi?id=215918 |
||||||||
Attachments: |
|
This regressed with r263506, the fix for bug 213495. Strange, we are setting the correct animation-wide timing function in GraphicsLayerCA::setupAnimation() and then returning the correct, default linear timing function for the keyframe in GraphicsLayerCA::timingFunctionForAnimationValue(). Applying the timing function to the keyframe instead of the CA animation fixes the issue. This could be a Core Animation bug or an unexpected behavior of the API. I confirmed this is an issue with Core Animation: setting the `timingFunction` property on a CAKeyframeAnimation clips the animation when the timing function is outside of the 0-1 range. However, applying the same timing function to the `timingFunctions` (plural) property addresses the issue. This means that for CSS Animations this is not an issue since CSS Animations never apply an animation-wide timing function. For CSS Transitions, we can work around this issue since there is a single interval. However, for JS-originated animations which can have both multiple intervals (more than 2 keyframes) and an animation-wide timing function, we will require a fix to Core Animation. Applying a workaround to fix CSS Transitions seems well worth it since: 1. this is a regression 2. the reported bug is about those specifically 3. they are more common than JS-originated animations at this stage (which never worked for this behavior) My plan is to add a new member to PlatformCAAnimation to return the number of values for an animation, which we'll use to determine whether we have an animation with 2 values or more to determine whether we should use the `timingFunctions` (plural) property rather than the `timingFunction` (singular) property. Actually, it'll be even easier to use Animation::property() so that GraphicsLayerCA can tell we're dealing with a CSS Transition. Created attachment 407461 [details]
Patch
Comment on attachment 407461 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407461&action=review > Source/WebCore/animation/KeyframeEffect.cpp:1680 > + // If we are dealing with a CSS Transition and using a cubic bezier timing function, we set up > + // the Animation object to reflect this which will allow the GraphicsLayerCA code to set the > + // timing function on the single keyframe interval rather than on the animation at large, > + // working around a Core Animation issue where setting a cubic bezier on a CAKeyframeAnimation's > + // timingFunction property can lead to clipped animations should a value be above 1. I think this comment could be reduced to: // Do blah to work around a limitation of Core Animation: webkit.org/b/215918 > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3341 > + // A CSS Transition is the only scenario where Animation::property() will have > + // its mode set to SingleProperty. In this case, we don't set the animation-wide > + // timing function, instead opting to set it using the timingFunctions (plural) > + // property to work around a Core Animation issue where setting a cubic bezier > + // on a CAKeyframeAnimation's timingFunction property (singular) can lead to > + // clipped animations should a value be above 1. > + // FIXME: https://bugs.webkit.org/show_bug.cgi?id=215918 I think this comment could be reduced to: // Do blah to work around a limitation of Core Animation: webkit.org/b/215918 > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3357 > + // A CSS Transition is the only scenario where Animation::property() will have > + // its mode set to SingleProperty. In this case, we chose not to set set the > + // animation-wide timing function, so we set it on the single keyframe interval > + // to work around a Core Animation issue where setting a cubic bezier on a > + // CAKeyframeAnimation's timingFunction property (singular) can lead to clipped > + // animations should a value be above 1. > + // FIXME: https://bugs.webkit.org/show_bug.cgi?id=215918 I think this comment could be reduced to: // Do blah to work around a limitation of Core Animation: webkit.org/b/215918 > LayoutTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html:40 > + if (!UIHelper.isWebKit2()) > + await new Promise(requestAnimationFrame); A bit weird. Can you just await UIHelper.renderingUpdate()? (In reply to Simon Fraser (smfr) from comment #9) > Comment on attachment 407461 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407461&action=review > > > Source/WebCore/animation/KeyframeEffect.cpp:1680 > > + // If we are dealing with a CSS Transition and using a cubic bezier timing function, we set up > > + // the Animation object to reflect this which will allow the GraphicsLayerCA code to set the > > + // timing function on the single keyframe interval rather than on the animation at large, > > + // working around a Core Animation issue where setting a cubic bezier on a CAKeyframeAnimation's > > + // timingFunction property can lead to clipped animations should a value be above 1. > > I think this comment could be reduced to: > // Do blah to work around a limitation of Core Animation: webkit.org/b/215918 I'll try to tone those down to only capture the bits relevant to the immediate code. > > LayoutTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html:40 > > + if (!UIHelper.isWebKit2()) > > + await new Promise(requestAnimationFrame); > > A bit weird. Can you just await UIHelper.renderingUpdate()? Will do. Committed r266280: <https://trac.webkit.org/changeset/266280> *** Bug 216107 has been marked as a duplicate of this bug. *** |
Created attachment 407234 [details] scale-test.html STR: See test case, a <div> comes with `transition: 1s transform cubic-bezier(0, 2, 1, 2)` set. Expected: The bezier curve should get the div to scale up a bit and does down to the final size during the CSS transition. Actual: The transition looks linear and disregard the curve set? Note: Firefox and Chrome goes with the expected behavior. Version: Release 110 (Safari 14.0, WebKit 15610.1.21.0.2) macOS 10.15.5 (19F101)