Deduplication for @keyframes rules should account for animation-composition
https://bugs.webkit.org/show_bug.cgi?id=235596
Reviewed by Dean Jackson.
LayoutTests/imported/w3c:
Import test recently added in WPT via https://github.com/web-platform-tests/wpt/pull/32495.
We pass them all with the source change.
* web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:
* web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html:
Source/WebCore:
The CSS Animations Level 2 spec recently changed to account for animation-composition
when deduplicating @keyframes rules (see https://github.com/w3c/csswg-drafts/pull/6974).
* animation/CompositeOperation.h:
* style/StyleResolver.cpp:
(WebCore::Style::Resolver::keyframeRulesForName const):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288571 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
index 6bffcae..c1cb3ce 100644
--- a/LayoutTests/imported/w3c/ChangeLog
+++ b/LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,16 @@
+2022-01-25 Antoine Quint <graouts@webkit.org>
+
+ Deduplication for @keyframes rules should account for animation-composition
+ https://bugs.webkit.org/show_bug.cgi?id=235596
+
+ Reviewed by Dean Jackson.
+
+ Import test recently added in WPT via https://github.com/web-platform-tests/wpt/pull/32495.
+ We pass them all with the source change.
+
+ * web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:
+ * web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html:
+
2022-01-25 Myles C. Maxfield <mmaxfield@apple.com>
REGRESSION(r281419): iCloud.com Notes web app fonts render incorrectly
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt
index bc1dfc2..5559b00 100644
--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt
+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt
@@ -17,6 +17,8 @@
PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time, and all with the same easing function
PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different easing functions
PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different but equivalent easing functions
+PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different composite operations
+PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different easing functions and composite operations
PASS KeyframeEffect.getKeyframes() returns expected frames for overlapping keyframes
PASS KeyframeEffect.getKeyframes() returns expected values for animations with filter properties and missing keyframes
PASS KeyframeEffect.getKeyframes() returns expected values for animation with drop-shadow of filter property
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html
index 57bcf99..df319c2 100644
--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html
+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html
@@ -125,6 +125,28 @@
to { margin-top: 20px; margin-right: 20px; margin-bottom: 20px; }
}
+@keyframes anim-merge-offset-and-composite {
+ from { color: rgb(0, 0, 0); animation-composition: add; }
+ to { color: rgb(255, 255, 255); }
+ from { margin-top: 8px; animation-composition: accumulate; }
+ to { margin-top: 16px; }
+ from { font-size: 16px; animation-composition: add; }
+ to { font-size: 32px; }
+ from { padding-left: 2px; animation-composition: accumulate; }
+ to { padding-left: 4px; }
+}
+
+@keyframes anim-merge-offset-easing-and-composite {
+ from { color: rgb(0, 0, 0); animation-composition: add; }
+ to { color: rgb(255, 255, 255); }
+ from { margin-top: 8px; animation-composition: accumulate; }
+ to { margin-top: 16px; }
+ from { font-size: 16px; animation-composition: add; animation-timing-function: linear; }
+ to { font-size: 32px; }
+ from { padding-left: 2px; animation-composition: accumulate; }
+ to { padding-left: 4px; }
+}
+
@keyframes anim-overriding {
from { padding-top: 50px }
50%, from { padding-top: 30px } /* wins: 0% */
@@ -530,6 +552,48 @@
test(t => {
const div = addDiv(t);
+ div.style.animation = 'anim-merge-offset-and-composite 100s';
+
+ const frames = getKeyframes(div);
+
+ const expected = [
+ { offset: 0, computedOffset: 0, easing: "ease", composite: "add",
+ color: "rgb(0, 0, 0)", fontSize: "16px" },
+ { offset: 0, computedOffset: 0, easing: "ease", composite: "accumulate",
+ marginTop: "8px", paddingLeft: "2px" },
+ { offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
+ color: "rgb(255, 255, 255)", fontSize: "32px", marginTop: "16px",
+ paddingLeft: "4px" },
+ ];
+ assert_frame_lists_equal(frames, expected);
+}, 'KeyframeEffect.getKeyframes() returns expected frames for an ' +
+ 'animation with multiple keyframes for the same time and with ' +
+ 'different composite operations');
+
+test(t => {
+ const div = addDiv(t);
+ div.style.animation = 'anim-merge-offset-easing-and-composite 100s';
+
+ const frames = getKeyframes(div);
+
+ const expected = [
+ { offset: 0, computedOffset: 0, easing: "ease", composite: "add",
+ color: "rgb(0, 0, 0)" },
+ { offset: 0, computedOffset: 0, easing: "ease", composite: "accumulate",
+ marginTop: "8px", paddingLeft: "2px" },
+ { offset: 0, computedOffset: 0, easing: "linear", composite: "add",
+ fontSize: "16px" },
+ { offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
+ color: "rgb(255, 255, 255)", fontSize: "32px", marginTop: "16px",
+ paddingLeft: "4px" },
+ ];
+ assert_frame_lists_equal(frames, expected);
+}, 'KeyframeEffect.getKeyframes() returns expected frames for an ' +
+ 'animation with multiple keyframes for the same time and with ' +
+ 'different easing functions and composite operations');
+
+test(t => {
+ const div = addDiv(t);
div.style.animation = 'anim-overriding 100s';
const frames = getKeyframes(div);
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 58f22de..32200b7 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2022-01-25 Antoine Quint <graouts@webkit.org>
+
+ Deduplication for @keyframes rules should account for animation-composition
+ https://bugs.webkit.org/show_bug.cgi?id=235596
+
+ Reviewed by Dean Jackson.
+
+ The CSS Animations Level 2 spec recently changed to account for animation-composition
+ when deduplicating @keyframes rules (see https://github.com/w3c/csswg-drafts/pull/6974).
+
+ * animation/CompositeOperation.h:
+ * style/StyleResolver.cpp:
+ (WebCore::Style::Resolver::keyframeRulesForName const):
+
2022-01-25 Myles C. Maxfield <mmaxfield@apple.com>
REGRESSION(r281419): iCloud.com Notes web app fonts render incorrectly
diff --git a/Source/WebCore/animation/CompositeOperation.h b/Source/WebCore/animation/CompositeOperation.h
index 4cbbc14..74a80e6 100644
--- a/Source/WebCore/animation/CompositeOperation.h
+++ b/Source/WebCore/animation/CompositeOperation.h
@@ -26,6 +26,7 @@
#pragma once
#include <optional>
+#include <wtf/HashTraits.h>
namespace WebCore {
@@ -36,3 +37,7 @@
std::optional<CompositeOperation> toCompositeOperation(const CSSValue&);
} // namespace WebCore
+
+namespace WTF {
+template<> struct DefaultHash<WebCore::CompositeOperation> : IntHash<WebCore::CompositeOperation> { };
+} // namespace WTF
diff --git a/Source/WebCore/style/StyleResolver.cpp b/Source/WebCore/style/StyleResolver.cpp
index f535cc63..191c6e0 100644
--- a/Source/WebCore/style/StyleResolver.cpp
+++ b/Source/WebCore/style/StyleResolver.cpp
@@ -320,6 +320,14 @@
if (it == m_keyframesRuleMap.end())
return { };
+ auto compositeOperationForKeyframe = [](Ref<StyleRuleKeyframe> keyframe) -> CompositeOperation {
+ if (auto compositeOperationCSSValue = keyframe->properties().getPropertyCSSValue(CSSPropertyAnimationComposition)) {
+ if (auto compositeOperation = toCompositeOperation(*compositeOperationCSSValue))
+ return *compositeOperation;
+ }
+ return Animation::initialCompositeOperation();
+ };
+
auto timingFunctionForKeyframe = [](Ref<StyleRuleKeyframe> keyframe) -> RefPtr<const TimingFunction> {
if (auto timingFunctionCSSValue = keyframe->properties().getPropertyCSSValue(CSSPropertyAnimationTimingFunction)) {
if (auto timingFunction = TimingFunction::createFromCSSValue(*timingFunctionCSSValue))
@@ -342,13 +350,14 @@
auto* keyframesRule = it->value.get();
auto* keyframes = &keyframesRule->keyframes();
- using KeyframeUniqueKey = std::pair<double, RefPtr<const TimingFunction>>;
+ using KeyframeUniqueKey = std::tuple<double, RefPtr<const TimingFunction>, CompositeOperation>;
auto hasDuplicateKeys = [&]() -> bool {
HashSet<KeyframeUniqueKey> uniqueKeyframeKeys;
for (auto& keyframe : *keyframes) {
+ auto compositeOperation = compositeOperationForKeyframe(keyframe);
auto timingFunction = uniqueTimingFunctionForKeyframe(keyframe);
for (auto key : keyframe->keys()) {
- if (!uniqueKeyframeKeys.add({ key, timingFunction }))
+ if (!uniqueKeyframeKeys.add({ key, timingFunction, compositeOperation }))
return true;
}
}
@@ -360,19 +369,21 @@
// FIXME: If HashMaps could have Ref<> as value types, we wouldn't need
// to copy the HashMap into a Vector.
- Vector<Ref<StyleRuleKeyframe>> deduplicatedKeyframes;
// Merge keyframes with a similar offset and timing function.
+ Vector<Ref<StyleRuleKeyframe>> deduplicatedKeyframes;
HashMap<KeyframeUniqueKey, RefPtr<StyleRuleKeyframe>> keyframesMap;
for (auto& originalKeyframe : *keyframes) {
+ auto compositeOperation = compositeOperationForKeyframe(originalKeyframe);
auto timingFunction = uniqueTimingFunctionForKeyframe(originalKeyframe);
for (auto key : originalKeyframe->keys()) {
- if (auto keyframe = keyframesMap.get({ key, timingFunction }))
+ KeyframeUniqueKey uniqueKey { key, timingFunction, compositeOperation };
+ if (auto keyframe = keyframesMap.get(uniqueKey))
keyframe->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());
else {
auto styleRuleKeyframe = StyleRuleKeyframe::create(MutableStyleProperties::create());
styleRuleKeyframe.ptr()->setKey(key);
styleRuleKeyframe.ptr()->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());
- keyframesMap.set({ key, timingFunction }, styleRuleKeyframe.ptr());
+ keyframesMap.set(uniqueKey, styleRuleKeyframe.ptr());
deduplicatedKeyframes.append(styleRuleKeyframe);
}
}
