[Web Animations] Interpolation between lengths with an "auto" value should be discrete
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Jul 2018 18:54:05 +0000 (18:54 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Jul 2018 18:54:05 +0000 (18:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187721

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

LayoutTests/imported/w3c:

Mark 2 new WPT progressions.

* web-platform-tests/web-animations/animation-model/animation-types/discrete-expected.txt:

Source/WebCore:

When interpolating between two Length values, if one is "auto", we should use the from-value
from 0 and up to (but excluding) 0.5, and use the to-value from 0.5 to 1.

This change caused a regression in the legacy animation engine since it would create a CSS
transition even when the underlying and target values were non-interpolable. As such, the
underlying value would be used until the transition's mid-point and the tests at
legacy-animation-engine/imported/blink/transitions/transition-not-interpolable.html and
legacy-animation-engine/fast/animation/height-auto-transition-computed-value.html would fail
expecting the target value to be used immediately. We now ensure that no transition is actually
started if two values for a given property cannot be interpolated.

* page/animation/CompositeAnimation.cpp:
(WebCore::CompositeAnimation::updateTransitions):
* platform/Length.cpp:
(WebCore::blend):

LayoutTests:

Make two more tests opt into the new animation engine since they pass and they're not in the legacy-animation-engine directory.
A third test now has some logging due to transitions not actually running, which is expected and correct.

* fast/animation/height-auto-transition-computed-value.html:
* imported/blink/transitions/transition-not-interpolable.html:
* legacy-animation-engine/transitions/transition-to-from-auto-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/fast/animation/height-auto-transition-computed-value.html
LayoutTests/imported/blink/transitions/transition-not-interpolable.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/discrete-expected.txt
LayoutTests/legacy-animation-engine/transitions/transition-to-from-auto-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/page/animation/CompositeAnimation.cpp
Source/WebCore/platform/Length.cpp

index d12e5ac..68d19eb 100644 (file)
@@ -1,3 +1,17 @@
+2018-07-17  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Interpolation between lengths with an "auto" value should be discrete
+        https://bugs.webkit.org/show_bug.cgi?id=187721
+
+        Reviewed by Dean Jackson.
+
+        Make two more tests opt into the new animation engine since they pass and they're not in the legacy-animation-engine directory.
+        A third test now has some logging due to transitions not actually running, which is expected and correct.
+
+        * fast/animation/height-auto-transition-computed-value.html:
+        * imported/blink/transitions/transition-not-interpolable.html:
+        * legacy-animation-engine/transitions/transition-to-from-auto-expected.txt:
+
 2018-07-17  John Wilander  <wilander@apple.com>
 
         Add completion handlers to TestRunner functions setStatisticsLastSeen(), setStatisticsPrevalentResource(), setStatisticsVeryPrevalentResource(), setStatisticsHasHadUserInteraction(), and setStatisticsHasHadNonRecentUserInteraction()
index b9275b0..468d764 100644 (file)
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html><!-- webkit-test-runner [ enableWebAnimationsCSSIntegration=true ] -->
 <html>
 <head>
 <title>This tests that auto transition returns the proper computed value.</title>
index f2ecb81..3aaa72a 100644 (file)
@@ -1,4 +1,4 @@
-<!doctype html>
+<!doctype html><!-- webkit-test-runner [ enableWebAnimationsCSSIntegration=true ] -->
 <style>
 #test {
   height: 0px;
index 3ba389f..25d2e59 100644 (file)
@@ -1,3 +1,14 @@
+2018-07-17  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Interpolation between lengths with an "auto" value should be discrete
+        https://bugs.webkit.org/show_bug.cgi?id=187721
+
+        Reviewed by Dean Jackson.
+
+        Mark 2 new WPT progressions.
+
+        * web-platform-tests/web-animations/animation-model/animation-types/discrete-expected.txt:
+
 2018-07-11  Youenn Fablet  <youenn@apple.com>
 
         Fix remaining Cross-Origin-Resource-Policy failures, if any
index c20cbd9..6522d46 100644 (file)
@@ -1,7 +1,7 @@
 
 FAIL Test animating discrete values assert_equals: Animation produces 'from' value just before the middle of the interval expected "normal" but got "oblique 9.75deg"
-FAIL Test discrete animation is used when interpolation fails assert_equals: Animation produces 'from' value at start of interval expected "0px" but got "200px"
-FAIL Test discrete animation is used only for pairs of values that cannot be interpolated assert_equals: Animation produces 'from' value at start of interval expected "0px" but got "200px"
+PASS Test discrete animation is used when interpolation fails 
+PASS Test discrete animation is used only for pairs of values that cannot be interpolated 
 FAIL Test the 50% switch point for discrete animation is based on the effect easing assert_equals: Animation produces 'from' value at 94% of the iteration time expected "normal" but got "oblique 8.5deg"
 FAIL Test the 50% switch point for discrete animation is based on the keyframe easing assert_equals: Animation produces 'from' value at 94% of the iteration time expected "normal" but got "oblique 8.5deg"
 
index 529dcfa..acb3625 100644 (file)
@@ -1,3 +1,5 @@
+CONSOLE MESSAGE: line 371: Failed to pause 'left' transition on element 'test1'
+CONSOLE MESSAGE: line 371: Failed to pause 'left' transition on element 'test2'
 PASS - "left" property for "test1" element at 1s saw something close to: 0
 PASS - "left" property for "test2" element at 1s saw something close to: 100
 PASS - "left" property for "test3" element at 1s saw something close to: 50
index 38f3b7a..3e19d2a 100644 (file)
@@ -1,3 +1,26 @@
+2018-07-17  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Interpolation between lengths with an "auto" value should be discrete
+        https://bugs.webkit.org/show_bug.cgi?id=187721
+
+        Reviewed by Dean Jackson.
+
+        When interpolating between two Length values, if one is "auto", we should use the from-value
+        from 0 and up to (but excluding) 0.5, and use the to-value from 0.5 to 1.
+
+        This change caused a regression in the legacy animation engine since it would create a CSS
+        transition even when the underlying and target values were non-interpolable. As such, the
+        underlying value would be used until the transition's mid-point and the tests at 
+        legacy-animation-engine/imported/blink/transitions/transition-not-interpolable.html and
+        legacy-animation-engine/fast/animation/height-auto-transition-computed-value.html would fail
+        expecting the target value to be used immediately. We now ensure that no transition is actually
+        started if two values for a given property cannot be interpolated.
+
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::updateTransitions):
+        * platform/Length.cpp:
+        (WebCore::blend):
+
 2018-07-17  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Add an SPI hook to allow clients to yield document parsing and script execution
index 4d5e4f0..a8feca2 100644 (file)
@@ -159,7 +159,7 @@ void CompositeAnimation::updateTransitions(Element& element, const RenderStyle*
                     }
                 } else {
                     // We need to start a transition if it is active and the properties don't match
-                    equal = !isActiveTransition || CSSPropertyAnimation::propertiesEqual(prop, fromStyle, &targetStyle);
+                    equal = !isActiveTransition || CSSPropertyAnimation::propertiesEqual(prop, fromStyle, &targetStyle) || !CSSPropertyAnimation::canPropertyBeInterpolated(prop, fromStyle, &targetStyle);
                 }
 
                 // We can be in this loop with an inactive transition (!isActiveTransition). We need
index 4658fd0..d26364b 100644 (file)
@@ -313,7 +313,10 @@ static Length blendMixedTypes(const Length& from, const Length& to, double progr
 
 Length blend(const Length& from, const Length& to, double progress)
 {
-    if (from.isAuto() || from.isUndefined() || to.isAuto() || to.isUndefined())
+    if (from.isAuto() || to.isAuto())
+        return progress < 0.5 ? from : to;
+
+    if (from.isUndefined() || to.isUndefined())
         return to;
 
     if (from.type() == Calculated || to.type() == Calculated)