Bug 1254419 - Fix zero-length segment handling; r=heycam
Later in this patch series when we convert tests from web-platform tests to
mochitest-chrome tests, some of the test cases that use zero-length segments
(overlapping keyframes at certain offsets) would trigger failed assertions
in KeyframeEffectReadOnly::ComposeStyle. This is because this method was
originally written with CSS animations in mind where segments cannot be
zero-length. Furthermore, when these same tests cases are run as
web-platform-tests, the failed assertions are not visible.
This patch adjusts the handling of segments to allow zero-length segments and
adds a test to check that the handling matches that defined in Web Animations
which is summarized in the following informative note,
"this procedure permits overlapping keyframes. The behavior is that at the
point of overlap the output value jumps to the value of the last defined
keyframe at that offset. For overlapping frames at 0 or 1, the output value
for iteration progress values less than 0 or greater than or equal to 1 is the
value of the first keyframe or the last keyframe in keyframes
respectively."[1]
[1] https://w3c.github.io/web-animations/#the-effect-value-of-a-keyframe-animation-effect
MozReview-Commit-ID: JdyYbGZtbot
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -568,44 +568,54 @@ KeyframeEffectReadOnly::ComposeStyle(Ref
aSetProperties.AddProperty(prop.mProperty);
MOZ_ASSERT(prop.mSegments.Length() > 0,
"property should not be in animations if it has no segments");
// FIXME: Maybe cache the current segment?
const AnimationPropertySegment *segment = prop.mSegments.Elements(),
*segmentEnd = segment + prop.mSegments.Length();
- while (segment->mToKey < computedTiming.mProgress.Value()) {
- MOZ_ASSERT(segment->mFromKey < segment->mToKey, "incorrect keys");
+ while (segment->mToKey <= computedTiming.mProgress.Value()) {
+ MOZ_ASSERT(segment->mFromKey <= segment->mToKey, "incorrect keys");
if ((segment+1) == segmentEnd) {
break;
}
++segment;
MOZ_ASSERT(segment->mFromKey == (segment-1)->mToKey, "incorrect keys");
}
- MOZ_ASSERT(segment->mFromKey < segment->mToKey, "incorrect keys");
+ MOZ_ASSERT(segment->mFromKey <= segment->mToKey, "incorrect keys");
MOZ_ASSERT(segment >= prop.mSegments.Elements() &&
size_t(segment - prop.mSegments.Elements()) <
prop.mSegments.Length(),
"out of array bounds");
if (!aStyleRule) {
// Allocate the style rule now that we know we have animation data.
aStyleRule = new AnimValuesStyleRule();
}
+ StyleAnimationValue* val = aStyleRule->AddEmptyValue(prop.mProperty);
+
+ // Special handling for zero-length segments
+ if (segment->mToKey == segment->mFromKey) {
+ if (computedTiming.mProgress.Value() < 0) {
+ *val = segment->mFromValue;
+ } else {
+ *val = segment->mToValue;
+ }
+ continue;
+ }
+
double positionInSegment =
(computedTiming.mProgress.Value() - segment->mFromKey) /
(segment->mToKey - segment->mFromKey);
double valuePosition =
ComputedTimingFunction::GetPortion(segment->mTimingFunction,
positionInSegment);
- StyleAnimationValue *val = aStyleRule->AddEmptyValue(prop.mProperty);
-
#ifdef DEBUG
bool result =
#endif
StyleAnimationValue::Interpolate(prop.mProperty,
segment->mFromValue,
segment->mToValue,
valuePosition, *val);
MOZ_ASSERT(result, "interpolate must succeed now");
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -28413,16 +28413,20 @@
"path": "web-animations/keyframe-effect/getComputedTiming-progress.html",
"url": "/web-animations/keyframe-effect/getComputedTiming-progress.html"
},
{
"path": "web-animations/keyframe-effect/getComputedTiming.html",
"url": "/web-animations/keyframe-effect/getComputedTiming.html"
},
{
+ "path": "web-animations/keyframe-effect/keyframe-handling.html",
+ "url": "/web-animations/keyframe-effect/keyframe-handling.html"
+ },
+ {
"path": "webaudio/the-audio-api/the-audiobuffer-interface/idl-test.html",
"url": "/webaudio/the-audio-api/the-audiobuffer-interface/idl-test.html"
},
{
"path": "webaudio/the-audio-api/the-audiodestinationnode-interface/idl-test.html",
"url": "/webaudio/the-audio-api/the-audiodestinationnode-interface/idl-test.html"
},
{
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/web-animations/keyframe-effect/keyframe-handling.html
@@ -0,0 +1,76 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Keyframe handling tests</title>
+<link rel="help" href="https://w3c.github.io/web-animations/#the-effect-value-of-a-keyframe-animation-effect">
+<link rel="author" title="Brian Birtles" href="mailto:bbirtles@mozilla.com">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../testcommon.js"></script>
+<link rel="stylesheet" href="/resources/testharness.css">
+<body>
+<div id="log"></div>
+<div id="target"></div>
+<script>
+'use strict';
+
+test(function(t) {
+ var div = createDiv(t);
+ var anim = div.animate([ { offset: 0, opacity: 0 },
+ { offset: 0, opacity: 0.1 },
+ { offset: 0, opacity: 0.2 },
+ { offset: 1, opacity: 0.8 },
+ { offset: 1, opacity: 0.9 },
+ { offset: 1, opacity: 1 } ],
+ { duration: 1000,
+ easing: 'cubic-bezier(0.5, -0.5, 0.5, 1.5)' });
+ assert_equals(getComputedStyle(div).opacity, '0.2',
+ 'When progress is zero the last keyframe with offset 0 should'
+ + ' be used');
+ // http://cubic-bezier.com/#.5,-0.5,.5,1.5
+ // At t=0.15, the progress should be negative
+ anim.currentTime = 150;
+ assert_equals(getComputedStyle(div).opacity, '0',
+ 'When progress is negative, the first keyframe with a 0 offset'
+ + ' should be used');
+ // At t=0.71, the progress should be just less than 1
+ anim.currentTime = 710;
+ assert_approx_equals(parseFloat(getComputedStyle(div).opacity), 0.8, 0.01,
+ 'When progress is just less than 1, the first keyframe with an'
+ + ' offset of 1 should be used as the interval endpoint');
+ // At t=0.85, the progress should be > 1
+ anim.currentTime = 850;
+ assert_equals(getComputedStyle(div).opacity, '1',
+ 'When progress is greater than 1.0, the last keyframe with a 1'
+ + ' offset should be used');
+ anim.finish();
+ assert_equals(getComputedStyle(div).opacity, '1',
+ 'When progress is equal to 1.0, the last keyframe with a 1'
+ + ' offset should be used');
+}, 'Overlapping keyframes at 0 and 1 use the appropriate value when the'
+ + ' progress is outside the range [0, 1]');
+
+test(function(t) {
+ var div = createDiv(t);
+ var anim = div.animate([ { offset: 0, opacity: 0 },
+ { offset: 0.5, opacity: 0.3 },
+ { offset: 0.5, opacity: 0.5 },
+ { offset: 0.5, opacity: 0.7 },
+ { offset: 1, opacity: 1 } ], 1000);
+ anim.currentTime = 250;
+ assert_equals(getComputedStyle(div).opacity, '0.15',
+ 'Before the overlap point, the first keyframe from the'
+ + ' overlap point should be used as interval endpoint');
+ anim.currentTime = 500;
+ assert_equals(getComputedStyle(div).opacity, '0.7',
+ 'At the overlap point, the last keyframe from the'
+ + ' overlap point should be used as interval startpoint');
+ anim.currentTime = 750;
+ assert_equals(getComputedStyle(div).opacity, '0.85',
+ 'After the overlap point, the last keyframe from the'
+ + ' overlap point should be used as interval startpoint');
+}, 'Overlapping keyframes between 0 and 1 use the appropriate value on each'
+ + ' side of the overlap point');
+
+done();
+</script>
+</body>