Attachment #449938 for bug #235596

View Details Raw Unified Return to Bug 235596



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 NOBODY (OOPS!).

        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  Antoine Quint  <graouts@webkit.org>

        Refactor KeyframeEffect::getKeyframes()

Lines 26-31 a/Source/WebCore/animation/CompositeOperation.h_sec1
- a/Source/WebCore/animation/CompositeOperation.h +5 lines
26	#pragma once	26	#pragma once
27		27
28	#include <optional>	28	#include <optional>
		29	#include <wtf/HashTraits.h>
29		30
30	namespace WebCore {	31	namespace WebCore {
31		32
Lines 36-38 enum class CompositeOperation : uint8_t { Replace, Add, Accumulate }; a/Source/WebCore/animation/CompositeOperation.h_sec2
36	std::optional<CompositeOperation> toCompositeOperation(const CSSValue&);	37	std::optional<CompositeOperation> toCompositeOperation(const CSSValue&);
37		38
38	} // namespace WebCore	39	} // namespace WebCore
		40
		41	namespace WTF {
		42	template<> struct DefaultHash<WebCore::CompositeOperation> : IntHash<WebCore::CompositeOperation> { };
		43	} // namespace WTF



    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))

    auto* keyframesRule = it->value.get();
    auto* keyframes = &keyframesRule->keyframes();

    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, compositeOperation }))
                    return true;
            }
        }


    // 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()) {
            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(uniqueKey, styleRuleKeyframe.ptr());
                deduplicatedKeyframes.append(styleRuleKeyframe);
            }
        }



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 NOBODY (OOPS!).

        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  Antti Koivisto  <antti@apple.com>

        [CSS Container Queries] Parsing support for container-name property

Lines 17-22 PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt_sec1
- a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt +2 lines
17	PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time, and all with the same easing function	17	PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time, and all with the same easing function
18	PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different easing functions	18	PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different easing functions
19	PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different but equivalent easing functions	19	PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different but equivalent easing functions
		20	PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different composite operations
		21	PASS KeyframeEffect.getKeyframes() returns expected frames for an animation with multiple keyframes for the same time and with different easing functions and composite operations
20	PASS KeyframeEffect.getKeyframes() returns expected frames for overlapping keyframes	22	PASS KeyframeEffect.getKeyframes() returns expected frames for overlapping keyframes
21	PASS KeyframeEffect.getKeyframes() returns expected values for animations with filter properties and missing keyframes	23	PASS KeyframeEffect.getKeyframes() returns expected values for animations with filter properties and missing keyframes
22	PASS KeyframeEffect.getKeyframes() returns expected values for animation with drop-shadow of filter property	24	PASS KeyframeEffect.getKeyframes() returns expected values for animation with drop-shadow of filter property



  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% */

   'animation with multiple keyframes for the same time and with ' +
   'different but equivalent easing functions');

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';

Return to Bug 235596

Lines 320-325 Vector<Ref<StyleRuleKeyframe>> Resolver::keyframeRulesForName(const AtomString& a/Source/WebCore/style/StyleResolver.cpp_sec1
- a/Source/WebCore/style/StyleResolver.cpp -5 / +16 lines
320	if (it == m_keyframesRuleMap.end())	320	if (it == m_keyframesRuleMap.end())
321	return { };	321	return { };
322		322
		323	auto compositeOperationForKeyframe = [](Ref<StyleRuleKeyframe> keyframe) -> CompositeOperation {
		324	if (auto compositeOperationCSSValue = keyframe->properties().getPropertyCSSValue(CSSPropertyAnimationComposition)) {
		325	if (auto compositeOperation = toCompositeOperation(*compositeOperationCSSValue))
		326	return *compositeOperation;
		327	}
		328	return Animation::initialCompositeOperation();
		329	};
		330
323	auto timingFunctionForKeyframe = [](Ref<StyleRuleKeyframe> keyframe) -> RefPtr<const TimingFunction> {	331	auto timingFunctionForKeyframe = [](Ref<StyleRuleKeyframe> keyframe) -> RefPtr<const TimingFunction> {
324	if (auto timingFunctionCSSValue = keyframe->properties().getPropertyCSSValue(CSSPropertyAnimationTimingFunction)) {	332	if (auto timingFunctionCSSValue = keyframe->properties().getPropertyCSSValue(CSSPropertyAnimationTimingFunction)) {
325	if (auto timingFunction = TimingFunction::createFromCSSValue(*timingFunctionCSSValue))	333	if (auto timingFunction = TimingFunction::createFromCSSValue(*timingFunctionCSSValue))
Lines 342-354 Vector<Ref<StyleRuleKeyframe>> Resolver::keyframeRulesForName(const AtomString& a/Source/WebCore/style/StyleResolver.cpp_sec2
342	auto* keyframesRule = it->value.get();	350	auto* keyframesRule = it->value.get();
343	auto* keyframes = &keyframesRule->keyframes();	351	auto* keyframes = &keyframesRule->keyframes();
344		352
345	using KeyframeUniqueKey = std::pair<double, RefPtr<const TimingFunction>>;	353	using KeyframeUniqueKey = std::tuple<double, RefPtr<const TimingFunction>, CompositeOperation>;
346	auto hasDuplicateKeys = [&]() -> bool {	354	auto hasDuplicateKeys = [&]() -> bool {
347	HashSet<KeyframeUniqueKey> uniqueKeyframeKeys;	355	HashSet<KeyframeUniqueKey> uniqueKeyframeKeys;
348	for (auto& keyframe : *keyframes) {	356	for (auto& keyframe : *keyframes) {
		357	auto compositeOperation = compositeOperationForKeyframe(keyframe);
349	auto timingFunction = uniqueTimingFunctionForKeyframe(keyframe);	358	auto timingFunction = uniqueTimingFunctionForKeyframe(keyframe);
350	for (auto key : keyframe->keys()) {	359	for (auto key : keyframe->keys()) {
351	if (!uniqueKeyframeKeys.add({ key, timingFunction }))	360	if (!uniqueKeyframeKeys.add({ key, timingFunction, compositeOperation }))
352	return true;	361	return true;
353	}	362	}
354	}	363	}
Lines 360-378 Vector<Ref<StyleRuleKeyframe>> Resolver::keyframeRulesForName(const AtomString& a/Source/WebCore/style/StyleResolver.cpp_sec3
360		369
361	// FIXME: If HashMaps could have Ref<> as value types, we wouldn't need	370	// FIXME: If HashMaps could have Ref<> as value types, we wouldn't need
362	// to copy the HashMap into a Vector.	371	// to copy the HashMap into a Vector.
363	Vector<Ref<StyleRuleKeyframe>> deduplicatedKeyframes;
364	// Merge keyframes with a similar offset and timing function.	372	// Merge keyframes with a similar offset and timing function.
		373	Vector<Ref<StyleRuleKeyframe>> deduplicatedKeyframes;
365	HashMap<KeyframeUniqueKey, RefPtr<StyleRuleKeyframe>> keyframesMap;	374	HashMap<KeyframeUniqueKey, RefPtr<StyleRuleKeyframe>> keyframesMap;
366	for (auto& originalKeyframe : *keyframes) {	375	for (auto& originalKeyframe : *keyframes) {
		376	auto compositeOperation = compositeOperationForKeyframe(originalKeyframe);
367	auto timingFunction = uniqueTimingFunctionForKeyframe(originalKeyframe);	377	auto timingFunction = uniqueTimingFunctionForKeyframe(originalKeyframe);
368	for (auto key : originalKeyframe->keys()) {	378	for (auto key : originalKeyframe->keys()) {
369	if (auto keyframe = keyframesMap.get({ key, timingFunction }))	379	KeyframeUniqueKey uniqueKey { key, timingFunction, compositeOperation };
		380	if (auto keyframe = keyframesMap.get(uniqueKey))
370	keyframe->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());	381	keyframe->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());
371	else {	382	else {
372	auto styleRuleKeyframe = StyleRuleKeyframe::create(MutableStyleProperties::create());	383	auto styleRuleKeyframe = StyleRuleKeyframe::create(MutableStyleProperties::create());
373	styleRuleKeyframe.ptr()->setKey(key);	384	styleRuleKeyframe.ptr()->setKey(key);
374	styleRuleKeyframe.ptr()->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());	385	styleRuleKeyframe.ptr()->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());
375	keyframesMap.set({ key, timingFunction }, styleRuleKeyframe.ptr());	386	keyframesMap.set(uniqueKey, styleRuleKeyframe.ptr());
376	deduplicatedKeyframes.append(styleRuleKeyframe);	387	deduplicatedKeyframes.append(styleRuleKeyframe);
377	}	388	}
378	}	389	}

Lines 125-130 a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html_sec1
- a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html +64 lines
125	to { margin-top: 20px; margin-right: 20px; margin-bottom: 20px; }	125	to { margin-top: 20px; margin-right: 20px; margin-bottom: 20px; }
126	}	126	}
127		127
		128	@keyframes anim-merge-offset-and-composite {
		129	from { color: rgb(0, 0, 0); animation-composition: add; }
		130	to { color: rgb(255, 255, 255); }
		131	from { margin-top: 8px; animation-composition: accumulate; }
		132	to { margin-top: 16px; }
		133	from { font-size: 16px; animation-composition: add; }
		134	to { font-size: 32px; }
		135	from { padding-left: 2px; animation-composition: accumulate; }
		136	to { padding-left: 4px; }
		137	}
		138
		139	@keyframes anim-merge-offset-easing-and-composite {
		140	from { color: rgb(0, 0, 0); animation-composition: add; }
		141	to { color: rgb(255, 255, 255); }
		142	from { margin-top: 8px; animation-composition: accumulate; }
		143	to { margin-top: 16px; }
		144	from { font-size: 16px; animation-composition: add; animation-timing-function: linear; }
		145	to { font-size: 32px; }
		146	from { padding-left: 2px; animation-composition: accumulate; }
		147	to { padding-left: 4px; }
		148	}
		149
128	@keyframes anim-overriding {	150	@keyframes anim-overriding {
129	from { padding-top: 50px }	151	from { padding-top: 50px }
130	50%, from { padding-top: 30px } /* wins: 0% */	152	50%, from { padding-top: 30px } /* wins: 0% */
Lines 528-533 test(t => { a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html_sec2
528	'animation with multiple keyframes for the same time and with ' +	550	'animation with multiple keyframes for the same time and with ' +
529	'different but equivalent easing functions');	551	'different but equivalent easing functions');
530		552
		553	test(t => {
		554	const div = addDiv(t);
		555	div.style.animation = 'anim-merge-offset-and-composite 100s';
		556
		557	const frames = getKeyframes(div);
		558
		559	const expected = [
		560	{ offset: 0, computedOffset: 0, easing: "ease", composite: "add",
		561	color: "rgb(0, 0, 0)", fontSize: "16px" },
		562	{ offset: 0, computedOffset: 0, easing: "ease", composite: "accumulate",
		563	marginTop: "8px", paddingLeft: "2px" },
		564	{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
		565	color: "rgb(255, 255, 255)", fontSize: "32px", marginTop: "16px",
		566	paddingLeft: "4px" },
		567	];
		568	assert_frame_lists_equal(frames, expected);
		569	}, 'KeyframeEffect.getKeyframes() returns expected frames for an ' +
		570	'animation with multiple keyframes for the same time and with ' +
		571	'different composite operations');
		572
		573	test(t => {
		574	const div = addDiv(t);
		575	div.style.animation = 'anim-merge-offset-easing-and-composite 100s';
		576
		577	const frames = getKeyframes(div);
		578
		579	const expected = [
		580	{ offset: 0, computedOffset: 0, easing: "ease", composite: "add",
		581	color: "rgb(0, 0, 0)" },
		582	{ offset: 0, computedOffset: 0, easing: "ease", composite: "accumulate",
		583	marginTop: "8px", paddingLeft: "2px" },
		584	{ offset: 0, computedOffset: 0, easing: "linear", composite: "add",
		585	fontSize: "16px" },
		586	{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
		587	color: "rgb(255, 255, 255)", fontSize: "32px", marginTop: "16px",
		588	paddingLeft: "4px" },
		589	];
		590	assert_frame_lists_equal(frames, expected);
		591	}, 'KeyframeEffect.getKeyframes() returns expected frames for an ' +
		592	'animation with multiple keyframes for the same time and with ' +
		593	'different easing functions and composite operations');
		594
531	test(t => {	595	test(t => {
532	const div = addDiv(t);	596	const div = addDiv(t);
533	div.style.animation = 'anim-overriding 100s';	597	div.style.animation = 'anim-overriding 100s';

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +14 lines
		1	2022-01-25 Antoine Quint <graouts@webkit.org>
		2
		3	Deduplication for @keyframes rules should account for animation-composition
		4	https://bugs.webkit.org/show_bug.cgi?id=235596
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	The CSS Animations Level 2 spec recently changed to account for animation-composition
		9	when deduplicating @keyframes rules (see https://github.com/w3c/csswg-drafts/pull/6974).
		10
		11	* animation/CompositeOperation.h:
		12	* style/StyleResolver.cpp:
		13	(WebCore::Style::Resolver::keyframeRulesForName const):
		14
1	2022-01-25 Antoine Quint <graouts@webkit.org>	15	2022-01-25 Antoine Quint <graouts@webkit.org>
2		16
3	Refactor KeyframeEffect::getKeyframes()	17	Refactor KeyframeEffect::getKeyframes()

Lines 1-3 a/LayoutTests/imported/w3c/ChangeLog_sec1
- a/LayoutTests/imported/w3c/ChangeLog +13 lines
		1	2022-01-25 Antoine Quint <graouts@webkit.org>
		2
		3	Deduplication for @keyframes rules should account for animation-composition
		4	https://bugs.webkit.org/show_bug.cgi?id=235596
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	Import test recently added in WPT via https://github.com/web-platform-tests/wpt/pull/32495.
		9	We pass them all with the source change.
		10
		11	* web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:
		12	* web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html:
		13
1	2022-01-25 Antti Koivisto <antti@apple.com>	14	2022-01-25 Antti Koivisto <antti@apple.com>
2		15
3	[CSS Container Queries] Parsing support for container-name property	16	[CSS Container Queries] Parsing support for container-name property