[composite-bgcolor-animation] Decouple paint and compositing

Currently, the CheckCanStartAnimationOnCompositor is called twice for composite background-color animation, once during paint time and once during the compositing stage. The reason is that we need the decision during paint and compositing consistent. That is, if the paint stage says the background color must be paint on the main thread, then compositing stage has to agree with that, and vice versa. However, this is dangerous because between the paint and compositing stage, things could change, especially the PaintArtifactCompositor, which is used by the CheckCanStartAnimationOnCompositor. For example, it could happen that at paint time we have not produced / updated the property nodes for the current frame and we can make decision based on what was composited on the previous frame. Then at Precommit we have potentially updated / added / removed property tree nodes. In this case, the return value of CheckCanStartAnimationOnCompositor can be different, as a result, the background color animation won't run correctly. The reason we needed to know whether the animation could be composited here is that we didn't have a way to paint the background color off the main thread. More specifically, the BackgroundColorPaintWorklet::Paint() function can paint the background color only if the animation is running on the compositor thread. This CL makes following changes: 1. Make the BackgroundColorPaintWorklet::Paint() have the ability to paint the background color even if the animation is running on the main thread. The function needs two things: the current progress of the animation and the artifacts about the animation. So all we need is just getting the progress when the animation is running on the main thread. 2. With #1 being done, we no longer need to call the CheckCanStartAnimationOnCompositor during the paint step. As a result, whether or not the animation can be running on the compositor thread is solely the decision during the compositing stage. This is much safer than the current code, because we no longer need to make a compositing decision during the paint stage. We don't need to add any new tests because we already have sufficient layout tests for background color animation being run on the compositor as well as on the main thread. As long as all tests pass, this should be safe. The main benefit of this change is that the code is now more robust, meaning that we don't need to worry about the decision made by the paint and compositing stage being different. This change is also a performance win because we no longer need to call the CheckCanStartAnimationOnCompositor twice. Bug: 1185272, 1182261 Change-Id: Ie072714fd1d05e6537e05cad45ad1da99e20125b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2740697 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#863622}
bocoup · Mar 17, 2021 · d3b5e81 · d3b5e81
1 parent 423f107
commit d3b5e81
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 0 deletions.
diff --git a/css/css-backgrounds/animations/background-color-animation-single-keyframe-ref.html b/css/css-backgrounds/animations/background-color-animation-single-keyframe-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<body>
+  <canvas id="canvas" width="100" height="100"></canvas>
+</body>
+<script>
+  var canvas = document.getElementById('canvas');
+  var ctx = canvas.getContext('2d');
+  ctx.fillStyle = 'rgb(200, 0, 0)';
+  ctx.fillRect(0, 0, 100, 100);
+</script>
diff --git a/css/css-backgrounds/animations/background-color-animation-single-keyframe.html b/css/css-backgrounds/animations/background-color-animation-single-keyframe.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<link rel="help" href="https://drafts.csswg.org/css-backgrounds-3/#background-color">
+<link rel="match" href="background-color-animation-single-keyframe-ref.html">
+<style>
+.container {
+  width: 100px;
+  height: 100px;
+  background-color: rgb(200, 0, 0);
+}
+</style>
+<script src="/common/reftest-wait.js"></script>
+<body>
+<div class="container" id="target"></div>
+
+<script>
+document.getElementById("target").animate({backgroundColor: 'rgb(0, 200, 0)'}, 100000000000);
+takeScreenshot();
+</script>
+</body>
+</html>
diff --git a/css/css-backgrounds/animations/background-color-animation-with-zero-playbackRate-ref.html b/css/css-backgrounds/animations/background-color-animation-with-zero-playbackRate-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<body>
+  <canvas id="canvas" width="100" height="100"></canvas>
+</body>
+<script>
+  var canvas = document.getElementById('canvas');
+  var ctx = canvas.getContext('2d');
+  ctx.fillStyle = 'rgb(100, 0, 100)';
+  ctx.fillRect(0, 0, 100, 100);
+</script>
diff --git a/css/css-backgrounds/animations/background-color-animation-with-zero-playbackRate.html b/css/css-backgrounds/animations/background-color-animation-with-zero-playbackRate.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<link rel="help" href="https://drafts.csswg.org/css-backgrounds-3/#background-color">
+<link rel="match" href="background-color-animation-with-zero-playbackRate-ref.html">
+<style>
+.container {
+  width: 100px;
+  height: 100px;
+  /* Use a long animation that start at 50% progress where the slope of the
+     selected timing function is zero. By setting up the animation in this way,
+     we accommodate lengthy delays in running the test without a potential drift
+     in the animated property value. This is important for avoiding flakes,
+     especially on debug builds. The screenshots are taken as soon as the
+     animation is ready, thus the long animation duration has no bearing on
+     the actual duration of the test. */
+  animation: bgcolor 1000000s cubic-bezier(0,1,1,0) -500000s;
+}
+@keyframes bgcolor {
+  0% { background-color: rgb(200, 0, 0); }
+  100% { background-color: rgb(0, 0, 200); }
+}
+</style>
+<script src="/common/reftest-wait.js"></script>
+<body>
+<div class="container"></div>
+
+<script>
+var animation = document.getAnimations()[0];
+animation.ready.then(() => {
+  animation.playbackRate = 0;
+  takeScreenshot();
+});
+</script>
+</body>
+</html>