Skip to content
Permalink
Browse files
[web-animations] changing writing-mode or direction on an element tha…
…t has an animation targeting a logical property should ensure animation resolution is scheduled

https://bugs.webkit.org/show_bug.cgi?id=247895

Reviewed by Antti Koivisto.

When an effect in an effect stack is finished and is entirely superseded by another animation targeting the same property,
it can be removed. The procedure to remove replaced animations [0] runs as part of the procedure to update animations and
send events, which in our code happens as a result of page rendering being updated.

In the case where an effect targets a logical property, the *resolved* property it targets can change if the "writing-mode"
or "direction" properties change. And if the resolved target property changes, then the replaced state of the associated
animation may change as well. As such, we must make sure to schedule an animation update when the resolved target property
changes.

To do this, we now track whether a KeyframeValue targets a property that was resolved from a logical property and set that
boolean flag in Style::Resolver::styleForKeyframe(), which populates the generated KeyframeList for both script-originated
animations and CSS Animations. CSS Transitions aren't a concern here since they cannot change resolved property in-flight,
instead being canceled altogether if "writing-mode" or "direction" changes.

Then in KeyframeEffectStack::applyKeyframeEffects() where we already track whether the "writing-mode" or "direction" property
changes, we ask each processed effect whether they also target a logical property allowing us to determine whether the
resolved animated property changed and ensuring we schedule an update in such a case.

[0] https://drafts.csswg.org/web-animations-1/#removing-replaced-animations

* LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement-expected.txt:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::animatesDirectionAwareProperty const):
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/KeyframeEffectStack.cpp:
(WebCore::KeyframeEffectStack::applyKeyframeEffects):
* Source/WebCore/rendering/style/KeyframeList.cpp:
(WebCore::KeyframeList::containsDirectionAwareProperty const):
* Source/WebCore/rendering/style/KeyframeList.h:
(WebCore::KeyframeValue::KeyframeValue):
(WebCore::KeyframeValue::containsDirectionAwareProperty const):
(WebCore::KeyframeValue::setContainsDirectionAwareProperty):
* Source/WebCore/style/StyleResolver.cpp:
(WebCore::Style::Resolver::styleForKeyframe):

Canonical link: https://commits.webkit.org/256667@main
  • Loading branch information
graouts committed Nov 14, 2022
1 parent 7821448 commit 1b793da552f71613cc9d8b9b68ffe280890d2428
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 6 deletions.
@@ -19,7 +19,7 @@ PASS Removes an animation when another animation uses a shorthand
PASS Removes an animation that uses a shorthand
PASS Removes an animation by another animation using logical properties
PASS Removes an animation using logical properties
FAIL Removes an animation by another animation using logical properties after updating the context assert_equals: expected "removed" but got "active"
PASS Removes an animation by another animation using logical properties after updating the context
PASS Removes an animation after updating another animation's effect's target
PASS Removes an animation after updating its effect's target
PASS Removes an animation after updating another animation's effect to one with a different target
@@ -936,6 +936,21 @@ bool KeyframeEffect::animatesProperty(CSSPropertyID property) const
return false;
}

bool KeyframeEffect::animatesDirectionAwareProperty() const
{
if (!m_blendingKeyframes.isEmpty())
return m_blendingKeyframes.containsDirectionAwareProperty();

for (auto& keyframe : m_parsedKeyframes) {
for (auto property : keyframe.styleStrings.keys()) {
if (CSSProperty::isDirectionAwareProperty(property))
return true;
}
}

return false;
}

bool KeyframeEffect::forceLayoutIfNeeded()
{
if (!m_needsForcedLayout || !m_target)
@@ -163,6 +163,7 @@ class KeyframeEffect : public AnimationEffect
const HashSet<AtomString>& animatedCustomProperties();
const HashSet<CSSPropertyID>& inheritedProperties() const { return m_inheritedProperties; }
bool animatesProperty(CSSPropertyID) const;
bool animatesDirectionAwareProperty() const;

bool computeExtentOfTransformAnimation(LayoutRect&) const;
bool computeTransformedExtentViaTransformList(const FloatRect&, const RenderStyle&, LayoutRect&) const;
@@ -149,6 +149,7 @@ OptionSet<AnimationImpact> KeyframeEffectStack::applyKeyframeEffects(RenderStyle

for (const auto& effect : sortedEffects()) {
ASSERT(effect->animation());
auto* animation = effect->animation();

auto inheritedPropertyChanged = [&]() {
if (previousLastStyleChangeEventStyle) {
@@ -160,10 +161,11 @@ OptionSet<AnimationImpact> KeyframeEffectStack::applyKeyframeEffects(RenderStyle
return false;
};

if (propertyAffectingLogicalPropertiesChanged || fontSizeChanged || inheritedPropertyChanged())
auto logicalPropertyDidChange = propertyAffectingLogicalPropertiesChanged && effect->animatesDirectionAwareProperty();
if (logicalPropertyDidChange || fontSizeChanged || inheritedPropertyChanged())
effect->propertyAffectingKeyframeResolutionDidChange(unanimatedStyle, resolutionContext);

effect->animation()->resolve(targetStyle, resolutionContext);
animation->resolve(targetStyle, resolutionContext);

if (effect->isRunningAccelerated() || effect->isAboutToRunAccelerated())
impact.add(AnimationImpact::RequiresRecomposite);
@@ -173,6 +175,12 @@ OptionSet<AnimationImpact> KeyframeEffectStack::applyKeyframeEffects(RenderStyle

if (transformRelatedPropertyChanged && effect->isRunningAcceleratedTransformRelatedAnimation())
effect->transformRelatedPropertyDidChange();

// If one of the effect's resolved property changed it could affect whether that effect's animation is removed.
if (logicalPropertyDidChange) {
ASSERT(animation->timeline());
animation->timeline()->animationTimingDidChange(*animation);
}
}

return impact;
@@ -225,6 +225,15 @@ bool KeyframeList::containsAnimatableProperty() const
return false;
}

bool KeyframeList::containsDirectionAwareProperty() const
{
for (auto& keyframe : m_keyframes) {
if (keyframe.containsDirectionAwareProperty())
return true;
}
return false;
}

bool KeyframeList::usesContainerUnits() const
{
for (auto& keyframe : m_keyframes) {
@@ -69,13 +69,17 @@ class KeyframeValue {
std::optional<CompositeOperation> compositeOperation() const { return m_compositeOperation; }
void setCompositeOperation(std::optional<CompositeOperation> op) { m_compositeOperation = op; }

bool containsDirectionAwareProperty() const { return m_containsDirectionAwareProperty; }
void setContainsDirectionAwareProperty(bool containsDirectionAwareProperty) { m_containsDirectionAwareProperty = containsDirectionAwareProperty; }

private:
double m_key;
HashSet<CSSPropertyID> m_properties; // The properties specified in this keyframe.
HashSet<AtomString> m_customProperties; // The custom properties being animated.
std::unique_ptr<RenderStyle> m_style;
RefPtr<TimingFunction> m_timingFunction;
std::optional<CompositeOperation> m_compositeOperation;
bool m_containsDirectionAwareProperty { false };
};

class KeyframeList {
@@ -97,6 +101,7 @@ class KeyframeList {
bool containsProperty(CSSPropertyID prop) const { return m_properties.contains(prop); }
const HashSet<CSSPropertyID>& properties() const { return m_properties; }
bool containsAnimatableProperty() const;
bool containsDirectionAwareProperty() const;

void addCustomProperty(const AtomString& customProperty) { m_customProperties.add(customProperty); }
bool containsCustomProperty(const AtomString& customProperty) const { return m_customProperties.contains(customProperty); }
@@ -281,13 +281,16 @@ std::unique_ptr<RenderStyle> Resolver::styleForKeyframe(const Element& element,
bool hasRevert = false;
for (unsigned i = 0; i < propertyCount; ++i) {
auto propertyReference = keyframe.properties().propertyAt(i);
auto property = CSSProperty::resolveDirectionAwareProperty(propertyReference.id(), elementStyle.direction(), elementStyle.writingMode());
auto unresolvedProperty = propertyReference.id();
auto resolvedProperty = CSSProperty::resolveDirectionAwareProperty(unresolvedProperty, elementStyle.direction(), elementStyle.writingMode());
// The animation-composition and animation-timing-function within keyframes are special
// because they are not animated; they just describe the composite operation and timing
// function between this keyframe and the next.
bool isAnimatableValue = property != CSSPropertyAnimationTimingFunction && property != CSSPropertyAnimationComposition;
bool isAnimatableValue = resolvedProperty != CSSPropertyAnimationTimingFunction && resolvedProperty != CSSPropertyAnimationComposition;
if (isAnimatableValue)
keyframeValue.addProperty(property);
keyframeValue.addProperty(resolvedProperty);
if (CSSProperty::isDirectionAwareProperty(unresolvedProperty))
keyframeValue.setContainsDirectionAwareProperty(true);
if (auto* value = propertyReference.value()) {
if (isAnimatableValue && value->isCustomPropertyValue())
keyframeValue.addCustomProperty(downcast<CSSCustomPropertyValue>(*value).name());

0 comments on commit 1b793da

Please sign in to comment.