- Revision
- 266439
- Author
- [email protected]
- Date
- 2020-09-01 18:19:38 -0700 (Tue, 01 Sep 2020)
Log Message
Cherry-pick r266241. rdar://problem/68177624
REGRESSION (r263506): timing of CSS Animation on https://animate.style is incorrect
https://bugs.webkit.org/show_bug.cgi?id=215807
<rdar://problem/66770136>
Reviewed by Simon Fraser.
Source/WebCore:
Test: webanimations/accelerated-css-animation-with-easing.html
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. Alas, this change broke handling
of timing functions with implicit animation-timing-function on keyframes.
That code change assumed that the Animation object ultimately passed to GraphicsLayerCA::setupAnimation()
would return the animation-wide timing function through Animation::timingFunction(). This was correct for
JS-originated animations, but not for CSS Animations, since that value was set based on the computed CSS
property animation-timing-function for the target element. However, that CSS property does not specify
the animation-wide timing function, but rather the default timing function to use for keyframes should
they fail to specify an explicit timing function.
To fix this, first, we ensure that the animation-wide timing function is set on the Animation object,
changing KeyframeEffect::backingAnimationForCompositedRenderer() to always generate an Animation object
based on the effect, divorcing itself from the Animation object created through CSS.
Then, we add a new member to Animation to specify the default timing function for keyframes, allowing
GraphicsLayerCA to use it to determine the timing function when building keyframes.
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::backingAnimationForCompositedRenderer const): Always generate an Animation
object based on the effect's properties, also setting the new defaultTimingFunctionForKeyframes in the
case of a CSS Animation object.
* platform/animation/Animation.h:
(WebCore::Animation::defaultTimingFunctionForKeyframes const):
(WebCore::Animation::setDefaultTimingFunctionForKeyframes):
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::animationHasStepsTimingFunction): Use the optional defaultTimingFunctionForKeyframes to determine
whether a keyframe uses a steps timing function.
(WebCore::GraphicsLayerCA::timingFunctionForAnimationValue): Use the optional defaultTimingFunctionForKeyframes
to determine the timing function for a keyframe.
LayoutTests:
Add a new test that checks that a CSS Animation with implicit and explicit animation-timing-function
values behave the same when running accelerated.
* webanimations/accelerated-css-animation-with-easing-expected.html: Added.
* webanimations/accelerated-css-animation-with-easing.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266241 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-610-branch/LayoutTests/ChangeLog (266438 => 266439)
--- branches/safari-610-branch/LayoutTests/ChangeLog 2020-09-02 01:19:35 UTC (rev 266438)
+++ branches/safari-610-branch/LayoutTests/ChangeLog 2020-09-02 01:19:38 UTC (rev 266439)
@@ -1,3 +1,73 @@
+2020-09-01 Alan Coon <[email protected]>
+
+ Cherry-pick r266241. rdar://problem/68177624
+
+ REGRESSION (r263506): timing of CSS Animation on https://animate.style is incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=215807
+ <rdar://problem/66770136>
+
+ Reviewed by Simon Fraser.
+
+ Source/WebCore:
+
+ Test: webanimations/accelerated-css-animation-with-easing.html
+
+ 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. Alas, this change broke handling
+ of timing functions with implicit animation-timing-function on keyframes.
+
+ That code change assumed that the Animation object ultimately passed to GraphicsLayerCA::setupAnimation()
+ would return the animation-wide timing function through Animation::timingFunction(). This was correct for
+ JS-originated animations, but not for CSS Animations, since that value was set based on the computed CSS
+ property animation-timing-function for the target element. However, that CSS property does not specify
+ the animation-wide timing function, but rather the default timing function to use for keyframes should
+ they fail to specify an explicit timing function.
+
+ To fix this, first, we ensure that the animation-wide timing function is set on the Animation object,
+ changing KeyframeEffect::backingAnimationForCompositedRenderer() to always generate an Animation object
+ based on the effect, divorcing itself from the Animation object created through CSS.
+
+ Then, we add a new member to Animation to specify the default timing function for keyframes, allowing
+ GraphicsLayerCA to use it to determine the timing function when building keyframes.
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::backingAnimationForCompositedRenderer const): Always generate an Animation
+ object based on the effect's properties, also setting the new defaultTimingFunctionForKeyframes in the
+ case of a CSS Animation object.
+ * platform/animation/Animation.h:
+ (WebCore::Animation::defaultTimingFunctionForKeyframes const):
+ (WebCore::Animation::setDefaultTimingFunctionForKeyframes):
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::animationHasStepsTimingFunction): Use the optional defaultTimingFunctionForKeyframes to determine
+ whether a keyframe uses a steps timing function.
+ (WebCore::GraphicsLayerCA::timingFunctionForAnimationValue): Use the optional defaultTimingFunctionForKeyframes
+ to determine the timing function for a keyframe.
+
+ LayoutTests:
+
+ Add a new test that checks that a CSS Animation with implicit and explicit animation-timing-function
+ values behave the same when running accelerated.
+
+ * webanimations/accelerated-css-animation-with-easing-expected.html: Added.
+ * webanimations/accelerated-css-animation-with-easing.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266241 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-08-27 Antoine Quint <[email protected]>
+
+ REGRESSION (r263506): timing of CSS Animation on https://animate.style is incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=215807
+ <rdar://problem/66770136>
+
+ Reviewed by Simon Fraser.
+
+ Add a new test that checks that a CSS Animation with implicit and explicit animation-timing-function
+ values behave the same when running accelerated.
+
+ * webanimations/accelerated-css-animation-with-easing-expected.html: Added.
+ * webanimations/accelerated-css-animation-with-easing.html: Added.
+
2020-09-01 Russell Epstein <[email protected]>
Cherry-pick r266118. rdar://problem/68160083
Added: branches/safari-610-branch/LayoutTests/webanimations/accelerated-css-animation-with-easing-expected.html (0 => 266439)
--- branches/safari-610-branch/LayoutTests/webanimations/accelerated-css-animation-with-easing-expected.html (rev 0)
+++ branches/safari-610-branch/LayoutTests/webanimations/accelerated-css-animation-with-easing-expected.html 2020-09-02 01:19:38 UTC (rev 266439)
@@ -0,0 +1,2 @@
+<body style="background-color: green">
+</body>
Added: branches/safari-610-branch/LayoutTests/webanimations/accelerated-css-animation-with-easing.html (0 => 266439)
--- branches/safari-610-branch/LayoutTests/webanimations/accelerated-css-animation-with-easing.html (rev 0)
+++ branches/safari-610-branch/LayoutTests/webanimations/accelerated-css-animation-with-easing.html 2020-09-02 01:19:38 UTC (rev 266439)
@@ -0,0 +1,61 @@
+<body>
+<style>
+
+ body {
+ background-color: green;
+ }
+
+ div {
+ position: absolute;
+ top: 0;
+ height: 100px;
+ animation-duration: 1s;
+ }
+
+ #implicit {
+ left: 0;
+ width: 100px;
+ background-color: red;
+ animation-name: implicit;
+ }
+
+ /* Allow for a 1px error on either side */
+ #explicit {
+ left: -1px;
+ width: 102px;
+ background-color: green;
+ animation-name: explicit;
+ animation-timing-function: ease;
+ }
+
+ @keyframes implicit {
+ 0.001% { transform: translate(400px) }
+ }
+
+ @keyframes explicit {
+ 0.001% {
+ transform: translate(400px);
+ animation-timing-function: ease;
+ }
+ }
+
+</style>
+<div id="implicit"></div>
+<div id="explicit"></div>
+<script>
+
+(async function() {
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ await Promise.all(document.getAnimations().map(animation => animation.ready));
+ await new Promise(requestAnimationFrame);
+ await new Promise(requestAnimationFrame);
+ await new Promise(requestAnimationFrame);
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+})();
+
+</script>
+</body>
Modified: branches/safari-610-branch/Source/WebCore/ChangeLog (266438 => 266439)
--- branches/safari-610-branch/Source/WebCore/ChangeLog 2020-09-02 01:19:35 UTC (rev 266438)
+++ branches/safari-610-branch/Source/WebCore/ChangeLog 2020-09-02 01:19:38 UTC (rev 266439)
@@ -1,5 +1,103 @@
2020-09-01 Alan Coon <[email protected]>
+ Cherry-pick r266241. rdar://problem/68177624
+
+ REGRESSION (r263506): timing of CSS Animation on https://animate.style is incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=215807
+ <rdar://problem/66770136>
+
+ Reviewed by Simon Fraser.
+
+ Source/WebCore:
+
+ Test: webanimations/accelerated-css-animation-with-easing.html
+
+ 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. Alas, this change broke handling
+ of timing functions with implicit animation-timing-function on keyframes.
+
+ That code change assumed that the Animation object ultimately passed to GraphicsLayerCA::setupAnimation()
+ would return the animation-wide timing function through Animation::timingFunction(). This was correct for
+ JS-originated animations, but not for CSS Animations, since that value was set based on the computed CSS
+ property animation-timing-function for the target element. However, that CSS property does not specify
+ the animation-wide timing function, but rather the default timing function to use for keyframes should
+ they fail to specify an explicit timing function.
+
+ To fix this, first, we ensure that the animation-wide timing function is set on the Animation object,
+ changing KeyframeEffect::backingAnimationForCompositedRenderer() to always generate an Animation object
+ based on the effect, divorcing itself from the Animation object created through CSS.
+
+ Then, we add a new member to Animation to specify the default timing function for keyframes, allowing
+ GraphicsLayerCA to use it to determine the timing function when building keyframes.
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::backingAnimationForCompositedRenderer const): Always generate an Animation
+ object based on the effect's properties, also setting the new defaultTimingFunctionForKeyframes in the
+ case of a CSS Animation object.
+ * platform/animation/Animation.h:
+ (WebCore::Animation::defaultTimingFunctionForKeyframes const):
+ (WebCore::Animation::setDefaultTimingFunctionForKeyframes):
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::animationHasStepsTimingFunction): Use the optional defaultTimingFunctionForKeyframes to determine
+ whether a keyframe uses a steps timing function.
+ (WebCore::GraphicsLayerCA::timingFunctionForAnimationValue): Use the optional defaultTimingFunctionForKeyframes
+ to determine the timing function for a keyframe.
+
+ LayoutTests:
+
+ Add a new test that checks that a CSS Animation with implicit and explicit animation-timing-function
+ values behave the same when running accelerated.
+
+ * webanimations/accelerated-css-animation-with-easing-expected.html: Added.
+ * webanimations/accelerated-css-animation-with-easing.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266241 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-08-27 Antoine Quint <[email protected]>
+
+ REGRESSION (r263506): timing of CSS Animation on https://animate.style is incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=215807
+ <rdar://problem/66770136>
+
+ Reviewed by Simon Fraser.
+
+ Test: webanimations/accelerated-css-animation-with-easing.html
+
+ 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. Alas, this change broke handling
+ of timing functions with implicit animation-timing-function on keyframes.
+
+ That code change assumed that the Animation object ultimately passed to GraphicsLayerCA::setupAnimation()
+ would return the animation-wide timing function through Animation::timingFunction(). This was correct for
+ JS-originated animations, but not for CSS Animations, since that value was set based on the computed CSS
+ property animation-timing-function for the target element. However, that CSS property does not specify
+ the animation-wide timing function, but rather the default timing function to use for keyframes should
+ they fail to specify an explicit timing function.
+
+ To fix this, first, we ensure that the animation-wide timing function is set on the Animation object,
+ changing KeyframeEffect::backingAnimationForCompositedRenderer() to always generate an Animation object
+ based on the effect, divorcing itself from the Animation object created through CSS.
+
+ Then, we add a new member to Animation to specify the default timing function for keyframes, allowing
+ GraphicsLayerCA to use it to determine the timing function when building keyframes.
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::backingAnimationForCompositedRenderer const): Always generate an Animation
+ object based on the effect's properties, also setting the new defaultTimingFunctionForKeyframes in the
+ case of a CSS Animation object.
+ * platform/animation/Animation.h:
+ (WebCore::Animation::defaultTimingFunctionForKeyframes const):
+ (WebCore::Animation::setDefaultTimingFunctionForKeyframes):
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::animationHasStepsTimingFunction): Use the optional defaultTimingFunctionForKeyframes to determine
+ whether a keyframe uses a steps timing function.
+ (WebCore::GraphicsLayerCA::timingFunctionForAnimationValue): Use the optional defaultTimingFunctionForKeyframes
+ to determine the timing function for a keyframe.
+
+2020-09-01 Alan Coon <[email protected]>
+
Cherry-pick r266291. rdar://problem/68177666
No need to run full can-use-for (fast inline layout codepath) check on every style change.
Modified: branches/safari-610-branch/Source/WebCore/animation/KeyframeEffect.cpp (266438 => 266439)
--- branches/safari-610-branch/Source/WebCore/animation/KeyframeEffect.cpp 2020-09-02 01:19:35 UTC (rev 266438)
+++ branches/safari-610-branch/Source/WebCore/animation/KeyframeEffect.cpp 2020-09-02 01:19:38 UTC (rev 266439)
@@ -1662,8 +1662,6 @@
Ref<const Animation> KeyframeEffect::backingAnimationForCompositedRenderer() const
{
auto effectAnimation = animation();
- if (is<DeclarativeAnimation>(effectAnimation))
- return downcast<DeclarativeAnimation>(effectAnimation)->backingAnimation();
// FIXME: The iterationStart and endDelay AnimationEffectTiming properties do not have
// corresponding Animation properties.
@@ -1705,6 +1703,12 @@
break;
}
+ // In the case of CSS Animations, we must set the default timing function for keyframes to match
+ // the current value set for animation-timing-function on the target element which affects only
+ // keyframes and not the animation-wide timing.
+ if (is<CSSAnimation>(effectAnimation))
+ animation->setDefaultTimingFunctionForKeyframes(downcast<CSSAnimation>(effectAnimation)->backingAnimation().timingFunction());
+
return animation;
}
Modified: branches/safari-610-branch/Source/WebCore/platform/animation/Animation.h (266438 => 266439)
--- branches/safari-610-branch/Source/WebCore/platform/animation/Animation.h 2020-09-02 01:19:35 UTC (rev 266438)
+++ branches/safari-610-branch/Source/WebCore/platform/animation/Animation.h 2020-09-02 01:19:38 UTC (rev 266439)
@@ -127,6 +127,7 @@
TransitionProperty property() const { return m_property; }
const String& unknownProperty() const { return m_unknownProperty; }
TimingFunction* timingFunction() const { return m_timingFunction.get(); }
+ TimingFunction* defaultTimingFunctionForKeyframes() const { return m_defaultTimingFunctionForKeyframes.get(); }
void setDelay(double c) { m_delay = c; m_delaySet = true; }
void setDirection(AnimationDirection d) { m_direction = d; m_directionSet = true; }
@@ -144,6 +145,7 @@
void setProperty(TransitionProperty t) { m_property = t; m_propertySet = true; }
void setUnknownProperty(const String& property) { m_unknownProperty = property; }
void setTimingFunction(RefPtr<TimingFunction>&& function) { m_timingFunction = WTFMove(function); m_timingFunctionSet = true; }
+ void setDefaultTimingFunctionForKeyframes(RefPtr<TimingFunction>&& function) { m_defaultTimingFunctionForKeyframes = WTFMove(function); }
void setIsNoneAnimation(bool n) { m_isNone = n; }
@@ -173,6 +175,7 @@
double m_duration;
double m_playbackRate { 1 };
RefPtr<TimingFunction> m_timingFunction;
+ RefPtr<TimingFunction> m_defaultTimingFunctionForKeyframes;
Style::ScopeOrdinal m_nameStyleScopeOrdinal { Style::ScopeOrdinal::Element };
Modified: branches/safari-610-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (266438 => 266439)
--- branches/safari-610-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2020-09-02 01:19:35 UTC (rev 266438)
+++ branches/safari-610-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2020-09-02 01:19:38 UTC (rev 266439)
@@ -279,12 +279,14 @@
{
if (is<StepsTimingFunction>(anim->timingFunction()))
return true;
-
+
+ bool hasStepsDefaultTimingFunctionForKeyframes = is<StepsTimingFunction>(anim->defaultTimingFunctionForKeyframes());
for (unsigned i = 0; i < valueList.size(); ++i) {
if (const TimingFunction* timingFunction = valueList.at(i).timingFunction()) {
if (is<StepsTimingFunction>(timingFunction))
return true;
- }
+ } else if (hasStepsDefaultTimingFunctionForKeyframes)
+ return true;
}
return false;
@@ -3338,6 +3340,8 @@
if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled()) {
if (animValue.timingFunction())
return *animValue.timingFunction();
+ else if (anim.defaultTimingFunctionForKeyframes())
+ return *anim.defaultTimingFunctionForKeyframes();
return LinearTimingFunction::sharedLinearTimingFunction();
}
