[Web Animations] A number of tests report an incorrect computed offset
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jul 2018 04:10:41 +0000 (04:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jul 2018 04:10:41 +0000 (04:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187410
<rdar://problem/41905790>

Patch by Antoine Quint <graouts@apple.com> on 2018-07-08
Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Mark 16 new WPT progressions.

* web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt:

Source/WebCore:

While we would correctly avoid computing missing offsets when processing the first keyframe following the last
keyframes with a specified offset, we were forgetting to update the index of the last keyframe with a specified
offset which meant we would accidentally override a specified offset with an automically-computed one.

* animation/KeyframeEffectReadOnly.cpp:
(WebCore::computeMissingKeyframeOffsets):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233632 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt
LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt
LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001-expected.txt
LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/animation/KeyframeEffectReadOnly.cpp

index 51a617c..72d029b 100644 (file)
@@ -1,3 +1,18 @@
+2018-07-08  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] A number of tests report an incorrect computed offset
+        https://bugs.webkit.org/show_bug.cgi?id=187410
+        <rdar://problem/41905790>
+
+        Reviewed by Dean Jackson.
+
+        Mark 16 new WPT progressions.
+
+        * web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
+        * web-platform-tests/web-animations/interfaces/KeyframeEffect/constructor-expected.txt:
+        * web-platform-tests/web-animations/interfaces/KeyframeEffect/processing-a-keyframes-argument-001-expected.txt:
+        * web-platform-tests/web-animations/interfaces/KeyframeEffect/setKeyframes-expected.txt:
+
 2018-07-06  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Make WPT test at interfaces/KeyframeEffect/processing-a-keyframes-argument-002.html pass reliably
index f2a5e73..362e7b8 100644 (file)
@@ -21,17 +21,17 @@ PASS Element.animate() accepts a one property one non-array value property-index
 PASS Element.animate() accepts a one property two value property-indexed keyframes specification where the first value is invalid 
 PASS Element.animate() accepts a one property two value property-indexed keyframes specification where the second value is invalid 
 PASS Element.animate() accepts a property-indexed keyframe with a single offset 
-FAIL Element.animate() accepts a property-indexed keyframe with an array of offsets assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.45000000000000007
-FAIL Element.animate() accepts a property-indexed keyframe with an array of offsets that is too short assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
+PASS Element.animate() accepts a property-indexed keyframe with an array of offsets 
+PASS Element.animate() accepts a property-indexed keyframe with an array of offsets that is too short 
 PASS Element.animate() accepts a property-indexed keyframe with an array of offsets that is too long 
 PASS Element.animate() accepts a property-indexed keyframe with an empty array of offsets 
 PASS Element.animate() accepts a property-indexed keyframe with an array of offsets with an embedded null value 
-FAIL Element.animate() accepts a property-indexed keyframe with an array of offsets with a trailing null value assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
-FAIL Element.animate() accepts a property-indexed keyframe with an array of offsets with leading and trailing null values assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
+PASS Element.animate() accepts a property-indexed keyframe with an array of offsets with a trailing null value 
+PASS Element.animate() accepts a property-indexed keyframe with an array of offsets with leading and trailing null values 
 PASS Element.animate() accepts a property-indexed keyframe with an array of offsets with adjacent null values 
 PASS Element.animate() accepts a property-indexed keyframe with an array of offsets with all null values (and too many at that) 
 PASS Element.animate() accepts a property-indexed keyframe with a single null offset 
-FAIL Element.animate() accepts a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.2 but got 0.4
+PASS Element.animate() accepts a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array 
 PASS Element.animate() accepts a property-indexed keyframe without any specified easing 
 PASS Element.animate() accepts a property-indexed keyframe with a single easing 
 PASS Element.animate() accepts a property-indexed keyframe with an array of easings 
index 245a97d..e67c844 100644 (file)
@@ -33,9 +33,9 @@ PASS A KeyframeEffectReadOnly can be constructed with a one property two value p
 PASS A KeyframeEffectReadOnly constructed with a one property two value property-indexed keyframes specification where the second value is invalid roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with a single offset 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with a single offset roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.45000000000000007
+PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets that is too short assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
+PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets that is too short 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets that is too short roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets that is too long 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets that is too long roundtrips 
@@ -43,9 +43,9 @@ PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyfram
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an empty array of offsets roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with an embedded null value 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets with an embedded null value roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with a trailing null value assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
+PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with a trailing null value 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets with a trailing null value roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with leading and trailing null values assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
+PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with leading and trailing null values 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets with leading and trailing null values roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with adjacent null values 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets with adjacent null values roundtrips 
@@ -53,7 +53,7 @@ PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyfram
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets with all null values (and too many at that) roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with a single null offset 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with a single null offset roundtrips 
-FAIL A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.2 but got 0.4
+PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array roundtrips 
 PASS A KeyframeEffectReadOnly can be constructed with a property-indexed keyframe without any specified easing 
 PASS A KeyframeEffectReadOnly constructed with a property-indexed keyframe without any specified easing roundtrips 
index 550df7f..f5bfdb1 100644 (file)
@@ -41,7 +41,7 @@ PASS Equivalent property-indexed and sequenced keyframes: same composite applied
 PASS Keyframes are read from a custom iterator 
 PASS 'easing' and 'offset' are ignored on iterable objects 
 PASS Keyframes are read from a custom iterator with multiple properties specified 
-FAIL Keyframes are read from a custom iterator with where an offset is specified assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.75 but got 0.5
+PASS Keyframes are read from a custom iterator with where an offset is specified 
 FAIL Reading from a custom iterator that returns a non-object keyframe should throw assert_throws: function "() => {
     new KeyframeEffect(null, createIterable([
       { done: false, value: { left: '100px' } },
index 5dd479d..97b53c1 100644 (file)
@@ -14,17 +14,17 @@ PASS Keyframes can be replaced with a one property one non-array value property-
 PASS Keyframes can be replaced with a one property two value property-indexed keyframes specification where the first value is invalid 
 PASS Keyframes can be replaced with a one property two value property-indexed keyframes specification where the second value is invalid 
 PASS Keyframes can be replaced with a property-indexed keyframe with a single offset 
-FAIL Keyframes can be replaced with a property-indexed keyframe with an array of offsets assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.45000000000000007
-FAIL Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is too short assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
+PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets 
+PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is too short 
 PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is too long 
 PASS Keyframes can be replaced with a property-indexed keyframe with an empty array of offsets 
 PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets with an embedded null value 
-FAIL Keyframes can be replaced with a property-indexed keyframe with an array of offsets with a trailing null value assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
-FAIL Keyframes can be replaced with a property-indexed keyframe with an array of offsets with leading and trailing null values assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.25 but got 0.5
+PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets with a trailing null value 
+PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets with leading and trailing null values 
 PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets with adjacent null values 
 PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets with all null values (and too many at that) 
 PASS Keyframes can be replaced with a property-indexed keyframe with a single null offset 
-FAIL Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array assert_equals: value for 'computedOffset' on ComputedKeyframe #1 expected 0.2 but got 0.4
+PASS Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array 
 PASS Keyframes can be replaced with a property-indexed keyframe without any specified easing 
 PASS Keyframes can be replaced with a property-indexed keyframe with a single easing 
 PASS Keyframes can be replaced with a property-indexed keyframe with an array of easings 
index 9dd29c6..bd69f13 100644 (file)
@@ -1,3 +1,18 @@
+2018-07-08  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] A number of tests report an incorrect computed offset
+        https://bugs.webkit.org/show_bug.cgi?id=187410
+        <rdar://problem/41905790>
+
+        Reviewed by Dean Jackson.
+
+        While we would correctly avoid computing missing offsets when processing the first keyframe following the last
+        keyframes with a specified offset, we were forgetting to update the index of the last keyframe with a specified
+        offset which meant we would accidentally override a specified offset with an automically-computed one.
+
+        * animation/KeyframeEffectReadOnly.cpp:
+        (WebCore::computeMissingKeyframeOffsets):
+
 2018-07-08  David Kilzer  <ddkilzer@apple.com>
 
         DOMMatrix.invertSelf() returns garbage values for a non-invertible matrix
index 3ce2528..aac126a 100644 (file)
@@ -140,16 +140,14 @@ static inline void computeMissingKeyframeOffsets(Vector<KeyframeEffectReadOnly::
         auto& keyframe = keyframes[i];
         if (!keyframe.computedOffset)
             continue;
-        if (indexOfLastKeyframeWithNonNullOffset == i - 1)
-            continue;
-
-        double lastNonNullOffset = keyframes[indexOfLastKeyframeWithNonNullOffset].computedOffset;
-        double offsetDelta = keyframe.computedOffset - lastNonNullOffset;
-        double offsetIncrement = offsetDelta / (i - indexOfLastKeyframeWithNonNullOffset);
-        size_t indexOfFirstKeyframeWithNullOffset = indexOfLastKeyframeWithNonNullOffset + 1;
-        for (size_t j = indexOfFirstKeyframeWithNullOffset; j < i; ++j)
-            keyframes[j].computedOffset = lastNonNullOffset + (j - indexOfLastKeyframeWithNonNullOffset) * offsetIncrement;
-
+        if (indexOfLastKeyframeWithNonNullOffset != i - 1) {
+            double lastNonNullOffset = keyframes[indexOfLastKeyframeWithNonNullOffset].computedOffset;
+            double offsetDelta = keyframe.computedOffset - lastNonNullOffset;
+            double offsetIncrement = offsetDelta / (i - indexOfLastKeyframeWithNonNullOffset);
+            size_t indexOfFirstKeyframeWithNullOffset = indexOfLastKeyframeWithNonNullOffset + 1;
+            for (size_t j = indexOfFirstKeyframeWithNullOffset; j < i; ++j)
+                keyframes[j].computedOffset = lastNonNullOffset + (j - indexOfLastKeyframeWithNonNullOffset) * offsetIncrement;
+        }
         indexOfLastKeyframeWithNonNullOffset = i;
     }
 }