Bug 1572755 [wpt PR 18356] - Fix bug where composite: replace did not replace previous effects, a=testonly
authorStephen McGruer <smcgruer@chromium.org>
Wed, 14 Aug 2019 10:56:48 +0000
changeset 488116 b9780850967fe06ee0e91cf86d055554d69996f2
parent 488115 1054904199286a37d07e4cf547e3370d312337b2
child 488117 74a2012e3573c4725ed808e9dd18e29d0ecf2d8c
push id36435
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:46:49 +0000
treeherdermozilla-central@0db07ff50ab5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1572755, 18356, 992378, 1746302, 686024
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1572755 [wpt PR 18356] - Fix bug where composite: replace did not replace previous effects, a=testonly Automatic update from web-platform-tests Fix bug where composite: replace did not replace previous effects Consider a set of effects: e.animate([{ "width": "0" }, { "width": "5px" }], { duration: 100 }); e.animate([{ "width": "0" }, { "width": "5px" }], { duration: 100, composite: 'add' }); e.animate([{ "width": "0" }, { "width": "2px" }], { duration: 100 }); e.animate([{ "width": "0" }, { "width": "2px" }], { duration: 100, composite: 'add' }); Previously the code in CopyToActiveInterpolationsMap would incorrectly move the third effect to the start of the list to replace the original keyframe pair, resulting in an effect stack of: { "width": "0" }, { "width": "2px" } { "width": "0" }, { "width": "5px" } { "width": "0" }, { "width": "2px" } This is wrong; not only has it retained an effect it shouldn't have, it has also re-ordered keyframes which might break non-commutative additive properties. This CL fixes the logic to properly clear out existing effects when a composite: 'replace' effect is put onto the stack. Bug: 992378 Change-Id: I94ae54429ac7d4d28a0702d397ab64c2e45dee65 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1746302 Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: Yi Gu <yigu@chromium.org> Cr-Commit-Position: refs/heads/master@{#686024} -- wpt-commits: a851a6a8e8b11160bcd6f5861553cf8419258136 wpt-pr: 18356
testing/web-platform/tests/web-animations/animation-model/combining-effects/effect-composition.html
--- a/testing/web-platform/tests/web-animations/animation-model/combining-effects/effect-composition.html
+++ b/testing/web-platform/tests/web-animations/animation-model/combining-effects/effect-composition.html
@@ -77,9 +77,38 @@ for (const composite of ['accumulate', '
     anim.effect.composite = composite;
     anim.currentTime = 50;                       // (10 + (10 + 20)) * 0.5
     assert_equals(getComputedStyle(div).marginLeft, '20px',
       'Animated style at 50%');
   }, 'unspecified composite mode on a keyframe is overriden by setting'
       + ` ${composite} of the effect`);
 }
 
+test(t => {
+  const div = createDiv(t);
+  const anims = [];
+  anims.push(div.animate({ marginLeft: ['10px', '20px'],
+                           composite: 'replace' },
+                         100));
+  anims.push(div.animate({ marginLeft: ['0px', '10px'],
+                           composite: 'add' },
+                         100));
+  // This should fully replace the previous effects.
+  anims.push(div.animate({ marginLeft: ['20px', '30px'],
+                           composite: 'replace' },
+                         100));
+  anims.push(div.animate({ marginLeft: ['30px', '40px'],
+                           composite: 'add' },
+                         100));
+
+  for (const anim of anims) {
+    anim.currentTime = 50;
+  }
+
+  // The result of applying the above effect stack is:
+  //  underlying = 0.5 * 20 + 0.5 * 30 = 25
+  //  result = 0.5 * (underlying + 30px) + 0.5 * (underlying + 40px)
+  //         = 60
+  assert_equals(getComputedStyle(div).marginLeft, '60px',
+    'Animated style at 50%');
+}, 'Composite replace fully replaces the underlying animation value');
+
 </script>