Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[web-animations] keyframes should be recomputed when "bolder" or "lig…
…hter" is used on a "font-weight" property

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

Reviewed by Antti Koivisto.

Keyframes can set the "font-weight" property to "bolder" or "lighter". When such values are used, we
recompute keyframes if the inherited "font-weight" value changes while an animation is active.

* LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/fontWeight-expected.txt:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::updateBlendingKeyframes):
(WebCore::KeyframeEffect::computeCSSAnimationBlendingKeyframes):
* Source/WebCore/animation/KeyframeEffect.h:
(WebCore::KeyframeEffect::hasRelativeFontWeight const):
* Source/WebCore/animation/KeyframeEffectStack.cpp:
(WebCore::KeyframeEffectStack::applyKeyframeEffects):
* Source/WebCore/style/StyleResolver.cpp:
(WebCore::Style::Resolver::keyframeStylesForAnimation):
* Source/WebCore/style/StyleResolver.h:

Canonical link: https://commits.webkit.org/259740@main
  • Loading branch information
graouts committed Feb 2, 2023
1 parent 5292316 commit 91d2570
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 6 deletions.
@@ -1,4 +1,4 @@

FAIL Relative font weight bolder responsive to style changes assert_not_equals: got disallowed value "400"
FAIL Relative font weight lighter responsive to style changes assert_not_equals: got disallowed value "700"
PASS Relative font weight bolder responsive to style changes
PASS Relative font weight lighter responsive to style changes

5 changes: 4 additions & 1 deletion Source/WebCore/animation/KeyframeEffect.cpp
Expand Up @@ -896,6 +896,7 @@ void KeyframeEffect::updateBlendingKeyframes(RenderStyle& elementStyle, const St
m_inheritedProperties.clear();
m_currentColorProperties.clear();
m_containsCSSVariableReferences = false;
m_hasRelativeFontWeight = false;

for (auto& keyframe : m_parsedKeyframes) {
KeyframeValue keyframeValue(keyframe.computedOffset, nullptr);
Expand Down Expand Up @@ -927,6 +928,8 @@ void KeyframeEffect::updateBlendingKeyframes(RenderStyle& elementStyle, const St
m_inheritedProperties.add(property.id());
else if (valueId == CSSValueCurrentcolor)
m_currentColorProperties.add(property.id());
else if (property.id() == CSSPropertyFontWeight && (valueId == CSSValueBolder || valueId == CSSValueLighter))
m_hasRelativeFontWeight = true;
}
}
}
Expand Down Expand Up @@ -1072,7 +1075,7 @@ void KeyframeEffect::computeCSSAnimationBlendingKeyframes(const RenderStyle& una

KeyframeList keyframeList(AtomString { backingAnimation.name().string });
if (auto* styleScope = Style::Scope::forOrdinal(*m_target, backingAnimation.nameStyleScopeOrdinal()))
styleScope->resolver().keyframeStylesForAnimation(*m_target, unanimatedStyle, resolutionContext, keyframeList, m_containsCSSVariableReferences, m_inheritedProperties, m_currentColorProperties);
styleScope->resolver().keyframeStylesForAnimation(*m_target, unanimatedStyle, resolutionContext, keyframeList, m_containsCSSVariableReferences, m_hasRelativeFontWeight, m_inheritedProperties, m_currentColorProperties);

// Ensure resource loads for all the frames.
for (auto& keyframe : keyframeList) {
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/animation/KeyframeEffect.h
Expand Up @@ -183,6 +183,7 @@ class KeyframeEffect : public AnimationEffect, public CSSPropertyBlendingClient
bool hasExplicitlyInheritedKeyframeProperty() const { return m_hasExplicitlyInheritedKeyframeProperty; }
bool hasPropertySetToCurrentColor() const;
bool hasColorSetToCurrentColor() const;
bool hasRelativeFontWeight() const { return m_hasRelativeFontWeight; }

private:
KeyframeEffect(Element*, PseudoId);
Expand Down Expand Up @@ -268,6 +269,7 @@ class KeyframeEffect : public AnimationEffect, public CSSPropertyBlendingClient
bool m_hasKeyframeComposingAcceleratedProperty { false };
bool m_containsCSSVariableReferences { false };
bool m_hasExplicitlyInheritedKeyframeProperty { false };
bool m_hasRelativeFontWeight { false };
};

} // namespace WebCore
Expand Down
7 changes: 6 additions & 1 deletion Source/WebCore/animation/KeyframeEffectStack.cpp
Expand Up @@ -197,8 +197,13 @@ OptionSet<AnimationImpact> KeyframeEffectStack::applyKeyframeEffects(RenderStyle
&& previousLastStyleChangeEventStyle->color() != targetStyle.color();
};

auto fontWeightChanged = [&]() {
return effect->hasRelativeFontWeight() && previousLastStyleChangeEventStyle
&& previousLastStyleChangeEventStyle->fontWeight() != targetStyle.fontWeight();
};

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

animation->resolve(targetStyle, resolutionContext);
Expand Down
5 changes: 4 additions & 1 deletion Source/WebCore/style/StyleResolver.cpp
Expand Up @@ -430,7 +430,7 @@ Vector<Ref<StyleRuleKeyframe>> Resolver::keyframeRulesForName(const AtomString&
return deduplicatedKeyframes;
}

void Resolver::keyframeStylesForAnimation(const Element& element, const RenderStyle& elementStyle, const ResolutionContext& context, KeyframeList& list, bool& containsCSSVariableReferences, HashSet<CSSPropertyID>& inheritedProperties, HashSet<CSSPropertyID>& currentColorProperties)
void Resolver::keyframeStylesForAnimation(const Element& element, const RenderStyle& elementStyle, const ResolutionContext& context, KeyframeList& list, bool& containsCSSVariableReferences, bool& hasRelativeFontWeight, HashSet<CSSPropertyID>& inheritedProperties, HashSet<CSSPropertyID>& currentColorProperties)
{
inheritedProperties.clear();
currentColorProperties.clear();
Expand All @@ -442,6 +442,7 @@ void Resolver::keyframeStylesForAnimation(const Element& element, const RenderSt
return;

containsCSSVariableReferences = false;
hasRelativeFontWeight = false;
// Construct and populate the style for each keyframe.
for (auto& keyframeRule : keyframeRules) {
// Add this keyframe style to all the indicated key times
Expand All @@ -465,6 +466,8 @@ void Resolver::keyframeStylesForAnimation(const Element& element, const RenderSt
inheritedProperties.add(property.id());
else if (valueId == CSSValueCurrentcolor)
currentColorProperties.add(property.id());
else if (property.id() == CSSPropertyFontWeight && (valueId == CSSValueBolder || valueId == CSSValueLighter))
hasRelativeFontWeight = true;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/style/StyleResolver.h
Expand Up @@ -95,7 +95,7 @@ class Resolver : public RefCounted<Resolver> {

ResolvedStyle styleForElement(const Element&, const ResolutionContext&, RuleMatchingBehavior = RuleMatchingBehavior::MatchAllRules);

void keyframeStylesForAnimation(const Element&, const RenderStyle& elementStyle, const ResolutionContext&, KeyframeList&, bool& containsCSSVariableReferences, HashSet<CSSPropertyID>& inheritedProperties, HashSet<CSSPropertyID>& currentColorProperties);
void keyframeStylesForAnimation(const Element&, const RenderStyle& elementStyle, const ResolutionContext&, KeyframeList&, bool& containsCSSVariableReferences, bool& hasRelativeFontWeight, HashSet<CSSPropertyID>& inheritedProperties, HashSet<CSSPropertyID>& currentColorProperties);

WEBCORE_EXPORT std::optional<ResolvedStyle> styleForPseudoElement(const Element&, const PseudoElementRequest&, const ResolutionContext&);

Expand Down

0 comments on commit 91d2570

Please sign in to comment.