third_party/WebKit/LayoutTests/animations/sample-on-last-keyframe.html - Issue 2702483002: Paint some content for animated elements in composited animation tests.

Side by Side Diff: third_party/WebKit/LayoutTests/animations/sample-on-last-keyframe.html

Issue 2702483002: Paint some content for animated elements in composited animation tests. (Closed)

Patch Set: Created 3 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« no previous file with comments | « third_party/WebKit/LayoutTests/animations/resources/composited-animation-test.js ('k') | third_party/WebKit/LayoutTests/animations/zoom-responsive-transform-animation.html » ('j') | third_party/WebKit/LayoutTests/animations/zoom-responsive-transform-animation.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <html>	2 <html>

3 <head>	3 <head>

4 <style type="text/css" media="screen">	4 <style type="text/css" media="screen">

5 #test {	5 #test {

6 animation-name: test;	6 animation-name: test;

7 animation-direction: reverse;	7 animation-direction: reverse;

8 /*	8 /*

9 A non-reversed iteration runs in the range [0, <duration>), so to sample	9 A non-reversed iteration runs in the range [0, <duration>), so to sample

10 on the last keyframe we have to sample at the start of a reversed	10 on the last keyframe we have to sample at the start of a reversed

(...skipping 16 matching lines...) Expand all Loading...
27 const expectedValues = [	27 const expectedValues = [

28 // [time, element-id, property, expected-value, tolerance]	28 // [time, element-id, property, expected-value, tolerance]

29 [0, "test", "opacity", 1.0, 0.1],	29 [0, "test", "opacity", 1.0, 0.1],

30 ];	30 ];

31	31

32 runAnimationTest(expectedValues);	32 runAnimationTest(expectedValues);

33 </script>	33 </script>

34 </head>	34 </head>

35 <body>	35 <body>

36 <p>Tests sampling a keyframe animation on the last keyframe.</p>	36 <p>Tests sampling a keyframe animation on the last keyframe.</p>

37 <div id="test"></div>	37 <div id="test">x</div>
	alancutter (OOO until 2018) 2017/02/16 01:12:30 This test doesn't depend on the animation being co This test doesn't depend on the animation being composited as it uses getComputedStyle(). It should be left as is. wkorman 2017/02/17 21:42:31 This test hangs when run in SPv2 via: 1. comment Show quoted text On 2017/02/16 at 01:12:30, alancutter wrote: > This test doesn't depend on the animation being composited as it uses getComputedStyle(). It should be left as is. This test hangs when run in SPv2 via: 1. comment out the entry for the test in third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2) 2. % ./third_party/WebKit/Tools/Scripts/run-webkit-tests --additional-driver-flag=--enable-slimming-paint-v2 virtual/threaded/animations/sample-on-last-keyframe.html AFAIK this is due to the animation being considered compositable as it is a straightforward opacity animation from 0 to 1, but per the linked bug, without an 'x' or some painted content SPv2 doesn't create a layer for the animation, and so the animation is never ticked/started. Through debug logging I find that animation-test-helpers.js is adding an event listener on webkitAnimationStart which never triggers. This can be explained by the above -- the animation metadata comes into existence but it is never started thus the event never fires. Adding some painted content fixes for same reason in rest of change. Thoughts? alancutter (OOO until 2018) 2017/02/20 00:18:30 I consider this test failing as an indication that Show quoted text On 2017/02/17 at 21:42:31, wkorman wrote: > On 2017/02/16 at 01:12:30, alancutter wrote: > > This test doesn't depend on the animation being composited as it uses getComputedStyle(). It should be left as is. > > This test hangs when run in SPv2 via: > > 1. comment out the entry for the test in third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2) > 2. % ./third_party/WebKit/Tools/Scripts/run-webkit-tests --additional-driver-flag=--enable-slimming-paint-v2 virtual/threaded/animations/sample-on-last-keyframe.html > > AFAIK this is due to the animation being considered compositable as it is a straightforward opacity animation from 0 to 1, but per the linked bug, without an 'x' or some painted content SPv2 doesn't create a layer for the animation, and so the animation is never ticked/started. > > Through debug logging I find that animation-test-helpers.js is adding an event listener on webkitAnimationStart which never triggers. This can be explained by the above -- the animation metadata comes into existence but it is never started thus the event never fires. > > Adding some painted content fixes for same reason in rest of change. Thoughts? I consider this test failing as an indication that bug 692310 is active. I think we should set failure expectations rather than add workarounds to avoid the bug. The other test changes have been things that depend on implementation specific details and need updating to match the future animation compositing behaviour. wkorman 2017/02/23 21:36:58 I see what you're saying, but let me make sure I u Show quoted text On 2017/02/20 00:18:30, alancutter wrote: > On 2017/02/17 at 21:42:31, wkorman wrote: > > On 2017/02/16 at 01:12:30, alancutter wrote: > > > This test doesn't depend on the animation being composited as it uses > getComputedStyle(). It should be left as is. > > > > This test hangs when run in SPv2 via: > > > > 1. comment out the entry for the test in > third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2) > > 2. % ./third_party/WebKit/Tools/Scripts/run-webkit-tests > --additional-driver-flag=--enable-slimming-paint-v2 > virtual/threaded/animations/sample-on-last-keyframe.html > > > > AFAIK this is due to the animation being considered compositable as it is a > straightforward opacity animation from 0 to 1, but per the linked bug, without > an 'x' or some painted content SPv2 doesn't create a layer for the animation, > and so the animation is never ticked/started. > > > > Through debug logging I find that animation-test-helpers.js is adding an event > listener on webkitAnimationStart which never triggers. This can be explained by > the above -- the animation metadata comes into existence but it is never started > thus the event never fires. > > > > Adding some painted content fixes for same reason in rest of change. Thoughts? > > I consider this test failing as an indication that bug 692310 is active. I think > we should set failure expectations rather than add workarounds to avoid the bug. > > The other test changes have been things that depend on implementation specific > details and need updating to match the future animation compositing behaviour. I see what you're saying, but let me make sure I understand what you're thinking. So once we fix 692310, for SPv2, this test as currently written will cease to consider this animation a composited animation when run as virtual/threaded/animations/sample-on-last-keyframe.html, because it paints no content and so has no layer. But isn't the intent of having this test included under the virtual/threaded test suite to ensure that this case (on last keyframe, the opacity is what we expect, within some tolerance) runs correctly as both a composited and non-composited animation? And so once we have "fixed" 692310, unless we modify this test to paint some content, it will essentially be running as a non-composited animation even when run under virtual/threaded. So it doesn't seem to me like it would then be testing what it's aiming to test. But I am quite likely missing something.
38 <div id="result">	38 <div id="result">

39 </div>	39 </div>

40 </body>	40 </body>

41 </html>	41 </html>

OLD	NEW