- Revision
- 290201
- Author
- grao...@webkit.org
- Date
- 2022-02-19 00:48:22 -0800 (Sat, 19 Feb 2022)
Log Message
REGRESSION (r287524): hihello.me does not show sliding sheet at the bottom of the page
https://bugs.webkit.org/show_bug.cgi?id=236838
rdar://88672183
Reviewed by Dean Jackson.
LayoutTests/imported/w3c:
Add new WPT tests to check we correctly compute implicit keyframes when a 0% and/or 100% keyframe
is defined but only specifies a timing function. One test checks the output of getKeyframes() and
the other that we correctly account for the implicit vaues when computing styles.
* web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:
* web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html:
* web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function-expected.txt: Added.
* web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function.html: Added.
Source/WebCore:
When we fixed bug 234799 we fixed the behavior of keyframe deduplication in
Style::Resolver::keyframeRulesForName(). While this was a good fix, code that
relied on the KeyframeList that would eventually be yielded from that function
did not quite correctly add implicit keyframes for the 0% and 100% case in some
relatively obscure situation.
The site hihello.me made this shortcoming apparent. This site has this odd keyframe rule:
from, 60%, 75%, 90%, to {
animation-timing-function: cubic-bezier(0.215, 0.61, 0.355, 1);
}
It appears the intention of the author with this rule is to replicate the timing function
on multiple keyframes. However, this does not work. This timing function will not be used
for *anything* since if a rule is specified without an animation-timing-function value it
will use the value set on the element, not one on a different keyframe.
This also means that while there are explicit 0% and 100% keyframes, they are not adequate
to then compute implicit properties since the timing function wouldn't match the default
timing function (unless the element that those keyframes are applied to would happen to
specify that exact same timing function).
To correctly handle this, we need to do several things.
First of all, we remove the implicit keyframe code found in KeyframeEffect::getKeyframes()
and let KeyframeList::fillImplicitKeyframes() do all the work to correctly fill-in values
for properties not eplicitly specified on a 0% or 100% keyframe.
This means we need to improve that function to correctly do the task that it's supposed to
do. Now provided with a KeyframeEffect and an underlying style as parameters, for 0% and
100% it correctly:
1. compiles a list of implicit properties for the given keyframe
2. find a compatible keyframe for the implicit properties or create one
3. set the implicit properties on that compatible keyframe to match the values found
in the underlying style
This correctly takes cares of calls to getKeyframes() as well as the generation of keyframes
passed down to RenderLayerBacking when starting an accelerated animation.
But RenderLayerBacking also had some outdated assumptions on the KeyframeList it receives.
Indeed, it would always assume that a 0% or 100% keyframe would *always* specify properties
for the animated property. That was incorrect since we correctly deduplicate keyframes and
having several 0% or 100% keyframes is perfectly valid. Now we don't give special treatment
to 0% or 100% keyframes in RenderLayerBacking::startAnimation() and always check that a keyframe
features values for the animated property before creating an animation value to send down
to GraphicsLayer.
Finally, another place we make assumptions on implicit keyframes was when resolving styles
as effects are applied in KeyframeEffect::setAnimatedPropertiesInStyle(). There we assumed
that a 0% or 100% keyframe would always qualify as a keyframe containing the animated property,
whereas the steps for resolving styles as specified by the Web Animations specification has
logic to deal with the case where we don't find a 0% or 100% keyframe with an explicit value
for the animated property. So we simplified the checks there to only ever check for an
explicit value.
This rather obscure way to specify keyframes was not previously tested by WPT, so this patch
improves the testing coverage in a way that would have caught this regression in the first place.
Test: imported/w3c/web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function.html
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::getKeyframes):
(WebCore::KeyframeEffect::setAnimatedPropertiesInStyle):
(WebCore::KeyframeEffect::applyPendingAcceleratedActions):
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::startAnimation):
* rendering/style/KeyframeList.cpp:
(WebCore::KeyframeList::insert):
(WebCore::KeyframeList::fillImplicitKeyframes):
* rendering/style/KeyframeList.h:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (290200 => 290201)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2022-02-19 07:22:47 UTC (rev 290200)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2022-02-19 08:48:22 UTC (rev 290201)
@@ -1,3 +1,20 @@
+2022-02-19 Antoine Quint <grao...@webkit.org>
+
+ REGRESSION (r287524): hihello.me does not show sliding sheet at the bottom of the page
+ https://bugs.webkit.org/show_bug.cgi?id=236838
+ rdar://88672183
+
+ Reviewed by Dean Jackson.
+
+ Add new WPT tests to check we correctly compute implicit keyframes when a 0% and/or 100% keyframe
+ is defined but only specifies a timing function. One test checks the output of getKeyframes() and
+ the other that we correctly account for the implicit vaues when computing styles.
+
+ * web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:
+ * web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html:
+ * web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function-expected.txt: Added.
+ * web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function.html: Added.
+
2022-02-18 Tim Nguyen <n...@apple.com>
Stop propagating inertness through iframes in Node::deprecatedIsInert()
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt (290200 => 290201)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt 2022-02-19 07:22:47 UTC (rev 290200)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt 2022-02-19 08:48:22 UTC (rev 290201)
@@ -29,4 +29,5 @@
PASS KeyframeEffect.getKeyframes() returns expected values for animations with a CSS variable which is overriden by the value in keyframe
PASS KeyframeEffect.getKeyframes() returns expected values for animations with only custom property in a keyframe
PASS KeyframeEffect.getKeyframes() reflects changes to @keyframes rules
+PASS KeyframeEffect.getKeyframes() returns expected values for animations with implicit values and a non-default timingfunction specified for 0% and 100%
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html (290200 => 290201)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html 2022-02-19 07:22:47 UTC (rev 290200)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html 2022-02-19 08:48:22 UTC (rev 290201)
@@ -190,6 +190,12 @@
from { transform: translate(100px, 0) }
to { --not-used: 200px }
}
+
+@keyframes anim-only-timing-function-for-from-and-to {
+ from, to { animation-timing-function: linear }
+ 50% { left: 10px }
+}
+
</style>
<body>
<div id="log"></div>
@@ -830,5 +836,23 @@
);
}, 'KeyframeEffect.getKeyframes() reflects changes to @keyframes rules');
+test(t => {
+ const div = addDiv(t);
+ div.style.animation = 'anim-only-timing-function-for-from-and-to 100s';
+
+ const frames = getKeyframes(div);
+
+ const expected = [
+ { offset: 0, computedOffset: 0, easing: "ease", composite: "auto", left: "auto" },
+ { offset: 0, computedOffset: 0, easing: "linear", composite: "auto" },
+ { offset: 0.5, computedOffset: 0.5, easing: "ease", composite: "auto", left: "10px" },
+ { offset: 1, computedOffset: 1, easing: "linear", composite: "auto" },
+ { offset: 1, computedOffset: 1, easing: "ease", composite: "auto", left: "auto" }
+ ];
+ assert_frame_lists_equal(frames, expected);
+}, 'KeyframeEffect.getKeyframes() returns expected values for ' +
+ 'animations with implicit values and a non-default timing' +
+ 'function specified for 0% and 100%');
+
</script>
</body>
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function-expected.txt (0 => 290201)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function-expected.txt (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function-expected.txt 2022-02-19 08:48:22 UTC (rev 290201)
@@ -0,0 +1,3 @@
+
+PASS Animations should not use an implicit value when the first rule within a @keyframes rule specifies "from" and "to" but only specifies a timing function.
+
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function.html (0 => 290201)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function.html (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function.html 2022-02-19 08:48:22 UTC (rev 290201)
@@ -0,0 +1,27 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>Multiple from and to keyframes</title>
+<link rel="help" href=""
+<script src=""
+<script src=""
+<script src=""
+<style>
+
+@keyframes multiple-from-to-keyframes {
+ from, to { animation-timing-function: cubic-bezier(0, 0, 1, 1) }
+ from { left: 50px }
+ to { left: 150px }
+}
+
+</style>
+<div id="log"></div>
+<script>
+'use strict';
+
+test(t => {
+ const div = addDiv(t);
+ div.style.animation = 'multiple-from-to-keyframes 100s -50s linear';
+ assert_equals(getComputedStyle(div).left, "100px");
+}, 'Animations should not use an implicit value when the first rule within a @keyframes rule specifies "from" and "to" but only specifies a timing function.');
+
+</script>
Modified: trunk/Source/WebCore/ChangeLog (290200 => 290201)
--- trunk/Source/WebCore/ChangeLog 2022-02-19 07:22:47 UTC (rev 290200)
+++ trunk/Source/WebCore/ChangeLog 2022-02-19 08:48:22 UTC (rev 290201)
@@ -1,3 +1,83 @@
+2022-02-19 Antoine Quint <grao...@webkit.org>
+
+ REGRESSION (r287524): hihello.me does not show sliding sheet at the bottom of the page
+ https://bugs.webkit.org/show_bug.cgi?id=236838
+ rdar://88672183
+
+ Reviewed by Dean Jackson.
+
+ When we fixed bug 234799 we fixed the behavior of keyframe deduplication in
+ Style::Resolver::keyframeRulesForName(). While this was a good fix, code that
+ relied on the KeyframeList that would eventually be yielded from that function
+ did not quite correctly add implicit keyframes for the 0% and 100% case in some
+ relatively obscure situation.
+
+ The site hihello.me made this shortcoming apparent. This site has this odd keyframe rule:
+
+ from, 60%, 75%, 90%, to {
+ animation-timing-function: cubic-bezier(0.215, 0.61, 0.355, 1);
+ }
+
+ It appears the intention of the author with this rule is to replicate the timing function
+ on multiple keyframes. However, this does not work. This timing function will not be used
+ for *anything* since if a rule is specified without an animation-timing-function value it
+ will use the value set on the element, not one on a different keyframe.
+
+ This also means that while there are explicit 0% and 100% keyframes, they are not adequate
+ to then compute implicit properties since the timing function wouldn't match the default
+ timing function (unless the element that those keyframes are applied to would happen to
+ specify that exact same timing function).
+
+ To correctly handle this, we need to do several things.
+
+ First of all, we remove the implicit keyframe code found in KeyframeEffect::getKeyframes()
+ and let KeyframeList::fillImplicitKeyframes() do all the work to correctly fill-in values
+ for properties not eplicitly specified on a 0% or 100% keyframe.
+
+ This means we need to improve that function to correctly do the task that it's supposed to
+ do. Now provided with a KeyframeEffect and an underlying style as parameters, for 0% and
+ 100% it correctly:
+
+ 1. compiles a list of implicit properties for the given keyframe
+ 2. find a compatible keyframe for the implicit properties or create one
+ 3. set the implicit properties on that compatible keyframe to match the values found
+ in the underlying style
+
+ This correctly takes cares of calls to getKeyframes() as well as the generation of keyframes
+ passed down to RenderLayerBacking when starting an accelerated animation.
+
+ But RenderLayerBacking also had some outdated assumptions on the KeyframeList it receives.
+ Indeed, it would always assume that a 0% or 100% keyframe would *always* specify properties
+ for the animated property. That was incorrect since we correctly deduplicate keyframes and
+ having several 0% or 100% keyframes is perfectly valid. Now we don't give special treatment
+ to 0% or 100% keyframes in RenderLayerBacking::startAnimation() and always check that a keyframe
+ features values for the animated property before creating an animation value to send down
+ to GraphicsLayer.
+
+ Finally, another place we make assumptions on implicit keyframes was when resolving styles
+ as effects are applied in KeyframeEffect::setAnimatedPropertiesInStyle(). There we assumed
+ that a 0% or 100% keyframe would always qualify as a keyframe containing the animated property,
+ whereas the steps for resolving styles as specified by the Web Animations specification has
+ logic to deal with the case where we don't find a 0% or 100% keyframe with an explicit value
+ for the animated property. So we simplified the checks there to only ever check for an
+ explicit value.
+
+ This rather obscure way to specify keyframes was not previously tested by WPT, so this patch
+ improves the testing coverage in a way that would have caught this regression in the first place.
+
+ Test: imported/w3c/web-platform-tests/css/css-animations/animation-multiple-from-to-keyframes-with-only-timing-function.html
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::getKeyframes):
+ (WebCore::KeyframeEffect::setAnimatedPropertiesInStyle):
+ (WebCore::KeyframeEffect::applyPendingAcceleratedActions):
+ * rendering/RenderLayerBacking.cpp:
+ (WebCore::RenderLayerBacking::startAnimation):
+ * rendering/style/KeyframeList.cpp:
+ (WebCore::KeyframeList::insert):
+ (WebCore::KeyframeList::fillImplicitKeyframes):
+ * rendering/style/KeyframeList.h:
+
2022-02-18 Ben Nham <n...@apple.com>
Hook up PushManager to permission state
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (290200 => 290201)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-02-19 07:22:47 UTC (rev 290200)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-02-19 08:48:22 UTC (rev 290201)
@@ -611,7 +611,7 @@
KeyframeList computedKeyframeList(m_blendingKeyframes.animationName());
computedKeyframeList.copyKeyframes(m_blendingKeyframes);
- computedKeyframeList.fillImplicitKeyframes(*m_target, m_target->styleResolver(), elementStyle, nullptr);
+ computedKeyframeList.fillImplicitKeyframes(*this, elementStyle);
auto keyframeRules = [&]() -> const Vector<Ref<StyleRuleKeyframe>> {
if (!is<CSSAnimation>(animation()))
@@ -646,22 +646,7 @@
}
}
- // We need to establish which properties are implicit for 0% and 100%.
- auto zeroKeyframeProperties = computedKeyframeList.properties();
- auto _oneKeyframeProperties_ = computedKeyframeList.properties();
- zeroKeyframeProperties.remove(CSSPropertyCustom);
- oneKeyframeProperties.remove(CSSPropertyCustom);
for (auto& keyframe : computedKeyframeList) {
- if (!keyframe.key()) {
- for (auto cssPropertyId : keyframe.properties())
- zeroKeyframeProperties.remove(cssPropertyId);
- } else if (keyframe.key() == 1) {
- for (auto cssPropertyId : keyframe.properties())
- oneKeyframeProperties.remove(cssPropertyId);
- }
- }
-
- for (auto& keyframe : computedKeyframeList) {
auto& style = *keyframe.style();
auto* keyframeRule = keyframeRuleForKey(keyframe.key());
@@ -703,19 +688,6 @@
addPropertyToKeyframe(cssPropertyId);
}
- // Now add the implicit properties in case there are any and we're dealing with a 0% or 100% keyframe.
- if (lastStyleChangeEventStyle) {
- if (!keyframe.key()) {
- for (auto cssPropertyId : zeroKeyframeProperties)
- addPropertyToKeyframe(cssPropertyId);
- zeroKeyframeProperties.clear();
- } else if (keyframe.key() == 1) {
- for (auto cssPropertyId : oneKeyframeProperties)
- addPropertyToKeyframe(cssPropertyId);
- oneKeyframeProperties.clear();
- }
- }
-
computedKeyframes.append(WTFMove(computedKeyframe));
}
@@ -1432,12 +1404,8 @@
Vector<const KeyframeValue*> propertySpecificKeyframes;
for (auto& keyframe : m_blendingKeyframes) {
auto offset = keyframe.key();
- if (!keyframe.containsProperty(cssPropertyId)) {
- // If we're dealing with a CSS animation, we consider the first and last keyframes to always have the property listed
- // since the underlying style was provided and should be captured.
- if (m_blendingKeyframesSource == BlendingKeyframesSource::WebAnimation || (offset && offset < 1))
- continue;
- }
+ if (!keyframe.containsProperty(cssPropertyId))
+ continue;
if (!offset)
numberOfKeyframesWithZeroOffset++;
if (offset == 1)
@@ -1804,7 +1772,7 @@
KeyframeList explicitKeyframes(m_blendingKeyframes.animationName());
explicitKeyframes.copyKeyframes(m_blendingKeyframes);
- explicitKeyframes.fillImplicitKeyframes(*m_target, m_target->styleResolver(), *underlyingStyle, nullptr);
+ explicitKeyframes.fillImplicitKeyframes(*this, *underlyingStyle);
return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), explicitKeyframes) ? RunningAccelerated::Yes : RunningAccelerated::No;
};
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (290200 => 290201)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2022-02-19 07:22:47 UTC (rev 290200)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2022-02-19 08:48:22 UTC (rev 290201)
@@ -3703,27 +3703,26 @@
auto* tf = currentKeyframe.timingFunction();
- bool isFirstOrLastKeyframe = key == 0 || key == 1;
- if ((hasRotate && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyRotate))
+ if (currentKeyframe.containsProperty(CSSPropertyRotate))
rotateVector.insert(makeUnique<TransformAnimationValue>(key, keyframeStyle->rotate(), tf));
- if ((hasScale && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyScale))
+ if (currentKeyframe.containsProperty(CSSPropertyScale))
scaleVector.insert(makeUnique<TransformAnimationValue>(key, keyframeStyle->scale(), tf));
- if ((hasTranslate && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyTranslate))
+ if (currentKeyframe.containsProperty(CSSPropertyTranslate))
translateVector.insert(makeUnique<TransformAnimationValue>(key, keyframeStyle->translate(), tf));
- if ((hasTransform && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyTransform))
+ if (currentKeyframe.containsProperty(CSSPropertyTransform))
transformVector.insert(makeUnique<TransformAnimationValue>(key, keyframeStyle->transform(), tf));
- if ((hasOpacity && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyOpacity))
+ if (currentKeyframe.containsProperty(CSSPropertyOpacity))
opacityVector.insert(makeUnique<FloatAnimationValue>(key, keyframeStyle->opacity(), tf));
- if ((hasFilter && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyFilter))
+ if (currentKeyframe.containsProperty(CSSPropertyFilter))
filterVector.insert(makeUnique<FilterAnimationValue>(key, keyframeStyle->filter(), tf));
#if ENABLE(FILTERS_LEVEL_2)
- if ((hasBackdropFilter && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyWebkitBackdropFilter))
+ if (currentKeyframe.containsProperty(CSSPropertyWebkitBackdropFilter))
backdropFilterVector.insert(makeUnique<FilterAnimationValue>(key, keyframeStyle->backdropFilter(), tf));
#endif
}
Modified: trunk/Source/WebCore/rendering/style/KeyframeList.cpp (290200 => 290201)
--- trunk/Source/WebCore/rendering/style/KeyframeList.cpp 2022-02-19 07:22:47 UTC (rev 290200)
+++ trunk/Source/WebCore/rendering/style/KeyframeList.cpp 2022-02-19 08:48:22 UTC (rev 290201)
@@ -23,8 +23,12 @@
#include "KeyframeList.h"
#include "Animation.h"
+#include "CSSAnimation.h"
#include "CSSKeyframeRule.h"
#include "CSSPropertyAnimation.h"
+#include "CompositeOperation.h"
+#include "Element.h"
+#include "KeyframeEffect.h"
#include "RenderObject.h"
#include "StyleResolver.h"
@@ -69,7 +73,7 @@
break;
}
}
-
+
if (!inserted)
m_keyframes.append(WTFMove(keyframe));
@@ -116,22 +120,90 @@
return rule.get().get();
}
-void KeyframeList::fillImplicitKeyframes(const Element& element, Style::Resolver& styleResolver, const RenderStyle& elementStyle, const RenderStyle* parentElementStyle)
+void KeyframeList::fillImplicitKeyframes(const KeyframeEffect& effect, const RenderStyle& underlyingStyle)
{
- // If the 0% keyframe is missing, create it (but only if there is at least one other keyframe).
- auto initialSize = size();
- if (initialSize > 0 && m_keyframes[0].key()) {
- KeyframeValue keyframeValue(0, nullptr);
- keyframeValue.setStyle(styleResolver.styleForKeyframe(element, elementStyle, { parentElementStyle }, zeroPercentKeyframe(), keyframeValue));
- insert(WTFMove(keyframeValue));
+ if (isEmpty())
+ return;
+
+ ASSERT(effect.target());
+ auto& element = *effect.target();
+ auto& styleResolver = element.styleResolver();
+
+ // We need to establish which properties are implicit for 0% and 100%.
+ // We start each list off with the full list of properties, and see if
+ // any 0% and 100% keyframes specify them.
+ auto zeroKeyframeImplicitProperties = m_properties;
+ auto _oneKeyframeImplicitProperties_ = m_properties;
+ zeroKeyframeImplicitProperties.remove(CSSPropertyCustom);
+ oneKeyframeImplicitProperties.remove(CSSPropertyCustom);
+
+ KeyframeValue* implicitZeroKeyframe = nullptr;
+ KeyframeValue* implicitOneKeyframe = nullptr;
+
+ auto isSuitableKeyframeForImplicitValues = [&](const KeyframeValue& keyframe) {
+ auto* timingFunction = keyframe.timingFunction();
+
+ // If there is no timing function set on the keyframe, then it uses the element's
+ // timing function, which makes this keyframe suitable.
+ if (!timingFunction)
+ return true;
+
+ if (auto* cssAnimation = dynamicDowncast<CSSAnimation>(effect.animation())) {
+ auto* animationWideTimingFunction = cssAnimation->backingAnimation().defaultTimingFunctionForKeyframes();
+ // If we're dealing with a CSS Animation and if that CSS Animation's backing animation
+ // has a default timing function set, then if that keyframe's timing function matches,
+ // that keyframe is suitable.
+ if (animationWideTimingFunction)
+ return timingFunction == animationWideTimingFunction;
+ // Otherwise, the keyframe will be suitable if its timing function matches the default.
+ return timingFunction == &CubicBezierTimingFunction::defaultTimingFunction();
+ }
+
+ return false;
+ };
+
+ for (auto& keyframe : m_keyframes) {
+ if (!keyframe.key()) {
+ for (auto cssPropertyId : keyframe.properties())
+ zeroKeyframeImplicitProperties.remove(cssPropertyId);
+ if (!implicitZeroKeyframe && isSuitableKeyframeForImplicitValues(keyframe))
+ implicitZeroKeyframe = &keyframe;
+ } else if (keyframe.key() == 1) {
+ for (auto cssPropertyId : keyframe.properties())
+ oneKeyframeImplicitProperties.remove(cssPropertyId);
+ if (!implicitOneKeyframe && isSuitableKeyframeForImplicitValues(keyframe))
+ implicitOneKeyframe = &keyframe;
+ }
}
- // If the 100% keyframe is missing, create it (but only if there is at least one other keyframe).
- if (initialSize > 0 && (m_keyframes[size() - 1].key() != 1)) {
- KeyframeValue keyframeValue(1, nullptr);
- keyframeValue.setStyle(styleResolver.styleForKeyframe(element, elementStyle, { parentElementStyle }, hundredPercentKeyframe(), keyframeValue));
- insert(WTFMove(keyframeValue));
- }
+ auto addImplicitKeyframe = [&](double key, const HashSet<CSSPropertyID>& implicitProperties, const StyleRuleKeyframe& keyframeRule, KeyframeValue* existingImplicitKeyframeValue) {
+ // If we're provided an existing implicit keyframe, we need to add all the styles for the implicit properties.
+ if (existingImplicitKeyframeValue) {
+ ASSERT(existingImplicitKeyframeValue->style());
+ auto keyframeStyle = RenderStyle::clonePtr(*existingImplicitKeyframeValue->style());
+ for (auto cssPropertyId : implicitProperties) {
+ CSSPropertyAnimation::blendProperties(&effect, cssPropertyId, *keyframeStyle, underlyingStyle, underlyingStyle, 1, CompositeOperation::Replace);
+ existingImplicitKeyframeValue->addProperty(cssPropertyId);
+ }
+ existingImplicitKeyframeValue->setStyle(WTFMove(keyframeStyle));
+ return;
+ }
+
+ // Otherwise we create a new keyframe.
+ KeyframeValue keyframeValue(key, nullptr);
+ keyframeValue.setStyle(styleResolver.styleForKeyframe(element, underlyingStyle, { nullptr }, keyframeRule, keyframeValue));
+ for (auto cssPropertyId : implicitProperties)
+ keyframeValue.addProperty(cssPropertyId);
+ if (!key)
+ m_keyframes.insert(0, WTFMove(keyframeValue));
+ else
+ m_keyframes.append(WTFMove(keyframeValue));
+ };
+
+ if (!zeroKeyframeImplicitProperties.isEmpty())
+ addImplicitKeyframe(0, zeroKeyframeImplicitProperties, zeroPercentKeyframe(), implicitZeroKeyframe);
+ if (!oneKeyframeImplicitProperties.isEmpty())
+ addImplicitKeyframe(1, oneKeyframeImplicitProperties, hundredPercentKeyframe(), implicitOneKeyframe);
}
bool KeyframeList::containsAnimatableProperty() const
Modified: trunk/Source/WebCore/rendering/style/KeyframeList.h (290200 => 290201)
--- trunk/Source/WebCore/rendering/style/KeyframeList.h 2022-02-19 07:22:47 UTC (rev 290200)
+++ trunk/Source/WebCore/rendering/style/KeyframeList.h 2022-02-19 08:48:22 UTC (rev 290201)
@@ -32,7 +32,7 @@
namespace WebCore {
-class Element;
+class KeyframeEffect;
class RenderStyle;
class TimingFunction;
@@ -100,7 +100,7 @@
void copyKeyframes(KeyframeList&);
bool hasImplicitKeyframes() const;
- void fillImplicitKeyframes(const Element&, Style::Resolver&, const RenderStyle& elementStyle, const RenderStyle* parentElementStyle);
+ void fillImplicitKeyframes(const KeyframeEffect&, const RenderStyle& elementStyle);
auto begin() const { return m_keyframes.begin(); }
auto end() const { return m_keyframes.end(); }
