Skip to content
Permalink
Browse files
[web-animations] support additivity when interpolating custom properties
https://bugs.webkit.org/show_bug.cgi?id=249386

Reviewed by Antti Koivisto.

In order to account for additivity when interpolating custom properties, we change
CSSPropertyAnimation::isPropertyAdditiveOrCumulative() to use the recently-introduced
variant type to allow for both standard and custom properties to be provided.

In the case of a custom property, the returned value is always true.

This means we can remove the CSSPropertyID variant check in KeyframeEffect::setAnimatedPropertiesInStyle()
and deal with additivity also with custom properties.

Finally, in CSSPropertyAnimation::blendProperty() we ensure that we pass the compositeOperation
parameter down the blending code chain.

* LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/animation/custom-property-animation-length-expected.txt:
* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::CSSPropertyAnimation::blendProperty):
(WebCore::CSSPropertyAnimation::isPropertyAdditiveOrCumulative):
* Source/WebCore/animation/CSSPropertyAnimation.h:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::setAnimatedPropertiesInStyle):

Canonical link: https://commits.webkit.org/257920@main
  • Loading branch information
graouts committed Dec 15, 2022
1 parent 5706da0 commit 0abfa98add77f0105f6911af6a369651915db5fb
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 27 deletions.
@@ -1,7 +1,7 @@

PASS Animating a custom property of type <length>
PASS Animating a custom property of type <length> with a single keyframe
FAIL Animating a custom property of type <length> with additivity assert_equals: expected "350px" but got "250px"
FAIL Animating a custom property of type <length> with a single keyframe and additivity assert_equals: expected "250px" but got "200px"
PASS Animating a custom property of type <length> with additivity
PASS Animating a custom property of type <length> with a single keyframe and additivity
FAIL Animating a custom property of type <length> with iterationComposite assert_equals: expected "250px" but got "50px"

@@ -3976,8 +3976,8 @@ void CSSPropertyAnimation::blendProperty(const CSSPropertyBlendingClient& client
[&] (CSSPropertyID propertyId) {
blendStandardProperty(client, propertyId, destination, from, to, progress, compositeOperation, iterationCompositeOperation, currentIteration);
}, [&] (AtomString customProperty) {
// FIXME: we don't deal with compositeOperation or iterationCompositeOpertion for custom properties yet.
blendCustomProperty(client, customProperty, destination, from, to, progress, CompositeOperation::Replace, IterationCompositeOperation::Replace, 0);
// FIXME: we don't deal with iterationCompositeOpertion for custom properties yet.
blendCustomProperty(client, customProperty, destination, from, to, progress, compositeOperation, IterationCompositeOperation::Replace, 0);
}
);
}
@@ -3987,10 +3987,15 @@ bool CSSPropertyAnimation::isPropertyAnimatable(CSSPropertyID property)
return property == CSSPropertyCustom || CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(property);
}

bool CSSPropertyAnimation::isPropertyAdditiveOrCumulative(CSSPropertyID property)
bool CSSPropertyAnimation::isPropertyAdditiveOrCumulative(Property property)
{
AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(property);
return wrapper ? wrapper->isAdditiveOrCumulative() : false;
return WTF::switchOn(property,
[] (CSSPropertyID propertyId) {
if (auto* wrapper = CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(propertyId))
return wrapper->isAdditiveOrCumulative();
return false;
}, [] (AtomString) { return true; }
);
}

bool CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(CSSPropertyID property, const RenderStyle& a, const RenderStyle& b)
@@ -43,7 +43,7 @@ class CSSPropertyAnimation {
using Property = std::variant<CSSPropertyID, AtomString>;

static bool isPropertyAnimatable(CSSPropertyID);
static bool isPropertyAdditiveOrCumulative(CSSPropertyID);
static bool isPropertyAdditiveOrCumulative(Property);
static bool propertyRequiresBlendingForAccumulativeIteration(CSSPropertyID, const RenderStyle& a, const RenderStyle& b);
static bool animationOfPropertyIsAccelerated(CSSPropertyID);
static bool propertiesEqual(CSSPropertyID, const RenderStyle& a, const RenderStyle& b);
@@ -1527,27 +1527,28 @@ void KeyframeEffect::setAnimatedPropertiesInStyle(RenderStyle& targetStyle, doub
// 3. Replace the property value of target property on keyframe with the result of combining underlying value
// (Va) and value to combine (Vb) using the procedure for the composite operation to use corresponding to the
// target property’s animation type.
if (std::holds_alternative<CSSPropertyID>(property)) {
auto cssPropertyId = std::get<CSSPropertyID>(property);
if (CSSPropertyAnimation::isPropertyAdditiveOrCumulative(cssPropertyId)) {
// 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) {
auto startKeyframeCompositeOperation = startKeyframe.compositeOperation().value_or(m_compositeOperation);
if (startKeyframeCompositeOperation != CompositeOperation::Replace)
CSSPropertyAnimation::blendProperty(*this, cssPropertyId, startKeyframeStyle, targetStyle, *startKeyframe.style(), 1, startKeyframeCompositeOperation);
}
if (CSSPropertyAnimation::isPropertyAdditiveOrCumulative(property)) {
// 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) {
auto startKeyframeCompositeOperation = startKeyframe.compositeOperation().value_or(m_compositeOperation);
if (startKeyframeCompositeOperation != CompositeOperation::Replace)
CSSPropertyAnimation::blendProperty(*this, property, startKeyframeStyle, targetStyle, *startKeyframe.style(), 1, startKeyframeCompositeOperation);
}

// 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) {
auto endKeyframeCompositeOperation = endKeyframe.compositeOperation().value_or(m_compositeOperation);
if (endKeyframeCompositeOperation != CompositeOperation::Replace)
CSSPropertyAnimation::blendProperty(*this, cssPropertyId, endKeyframeStyle, targetStyle, *endKeyframe.style(), 1, endKeyframeCompositeOperation);
}
// 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) {
auto endKeyframeCompositeOperation = endKeyframe.compositeOperation().value_or(m_compositeOperation);
if (endKeyframeCompositeOperation != CompositeOperation::Replace)
CSSPropertyAnimation::blendProperty(*this, property, endKeyframeStyle, targetStyle, *endKeyframe.style(), 1, endKeyframeCompositeOperation);
}

// FIXME: handle custom properties here as well.
if (std::holds_alternative<CSSPropertyID>(property)) {
auto cssPropertyId = std::get<CSSPropertyID>(property);
// If this keyframe effect has an iteration composite operation of accumulate,
if (m_iterationCompositeOperation == IterationCompositeOperation::Accumulate && currentIteration && CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(cssPropertyId, startKeyframeStyle, endKeyframeStyle)) {
usedBlendingForAccumulativeIteration = true;

0 comments on commit 0abfa98

Please sign in to comment.