Skip to content
Permalink
Browse files
WAAPI composite animations not working correctly
https://bugs.webkit.org/show_bug.cgi?id=248944

Reviewed by Antti Koivisto.

We would only ever compute the blended additive or accumulative keyframes if the offset was either 0 or 1,
completely ignoring any keyframe in between.

* LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/combining-effects/effect-composition-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/combining-effects/effect-composition.html:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::setAnimatedPropertiesInStyle):

Canonical link: https://commits.webkit.org/257731@main
  • Loading branch information
graouts committed Dec 12, 2022
1 parent 019d6d9 commit 4374420d956e8963d4efd9ff87e80959f916d4be
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 2 deletions.
@@ -1,12 +1,14 @@

PASS accumulate onto the base value
PASS accumulate onto the base value when the interval does not include the 0 or 1 keyframe
PASS accumulate onto an underlying animation value
PASS accumulate onto an underlying animation value with implicit from values
PASS accumulate onto an underlying animation value with implicit to values
PASS Composite when mixing accumulate and replace
PASS accumulate specified on a keyframe overrides the composite mode of the effect
PASS unspecified composite mode on a keyframe is overriden by setting accumulate of the effect
PASS add onto the base value
PASS add onto the base value when the interval does not include the 0 or 1 keyframe
PASS add onto an underlying animation value
PASS add onto an underlying animation value with implicit from values
PASS add onto an underlying animation value with implicit to values
@@ -21,6 +21,18 @@
'Animated margin-left style at 50%');
}, `${composite} onto the base value`);

test(t => {
const div = createDiv(t);
div.style.marginLeft = '10px';
const anim =
div.animate([{ marginLeft: '20px', offset: 0.25 }, { marginLeft: '30px', offset: 0.75 }],
{ duration: 100, composite });

anim.currentTime = 50;
assert_equals(getComputedStyle(div).marginLeft, '35px',
'Animated margin-left style at 50%');
}, `${composite} onto the base value when the interval does not include the 0 or 1 keyframe`);

test(t => {
const div = createDiv(t);
const anims = [];
@@ -1533,7 +1533,7 @@ void KeyframeEffect::setAnimatedPropertiesInStyle(RenderStyle& targetStyle, doub
// Only do this for the 0 keyframe if it was provided explicitly, since otherwise we want to use the "neutral value
// for composition" which really means we don't want to do anything but rather just use the underlying style which
// is already set on startKeyframe.
if (!startKeyframe.key() && !hasImplicitZeroKeyframe) {
if (startKeyframe.key() || !hasImplicitZeroKeyframe) {
auto startKeyframeCompositeOperation = startKeyframe.compositeOperation().value_or(m_compositeOperation);
if (startKeyframeCompositeOperation != CompositeOperation::Replace)
CSSPropertyAnimation::blendProperties(this, cssPropertyId, startKeyframeStyle, targetStyle, *startKeyframe.style(), 1, startKeyframeCompositeOperation);
@@ -1542,7 +1542,7 @@ void KeyframeEffect::setAnimatedPropertiesInStyle(RenderStyle& targetStyle, doub
// Only do this for the 1 keyframe if it was provided explicitly, since otherwise we want to use the "neutral value
// for composition" which really means we don't want to do anything but rather just use the underlying style which
// is already set on endKeyframe.
if (endKeyframe.key() == 1 && !hasImplicitOneKeyframe) {
if (endKeyframe.key() != 1 || !hasImplicitOneKeyframe) {
auto endKeyframeCompositeOperation = endKeyframe.compositeOperation().value_or(m_compositeOperation);
if (endKeyframeCompositeOperation != CompositeOperation::Replace)
CSSPropertyAnimation::blendProperties(this, cssPropertyId, endKeyframeStyle, targetStyle, *endKeyframe.style(), 1, endKeyframeCompositeOperation);

0 comments on commit 4374420

Please sign in to comment.