[animationworklet] Only call LayerTreeMutator::Mutate once per frame |
|||||
Issue descriptionAs described on the e-mail thread and document here: https://groups.google.com/a/chromium.org/forum/#!topic/animations-dev/Uwx-y6rOJXs We should only call LayerTreeMutator::Mutate once per frame. Since we have potentially different animations on the active and pending trees we can call mutate for the union of the animations on both trees and only use the results for the tree we need.
,
Dec 12 2017
,
Jan 31 2018
,
Mar 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b208e554c9f82733d7cc1e9a5dca8272431ade75 commit b208e554c9f82733d7cc1e9a5dca8272431ade75 Author: Yi Gu <yigu@chromium.org> Date: Fri Mar 02 14:08:41 2018 Optimize logic upon impl side invalidation by removing Animate calls Previously we run animation on pending tree upon impl side invalidation to calculate valid animation state. However it's unnecessary because the recycle tree that the pending tree is relying on has already been updated. Accordingly, the animation logic should stay in CommitComplete to avoid extra cost. Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Change-Id: I3182c3d1a2f9f13bc2758d216d1dd8bbade8f1ab Bug: 791279 Reviewed-on: https://chromium-review.googlesource.com/919602 Commit-Queue: Yi Gu <yigu@chromium.org> Reviewed-by: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/master@{#540510} [modify] https://crrev.com/b208e554c9f82733d7cc1e9a5dca8272431ade75/cc/trees/layer_tree_host_impl.cc [modify] https://crrev.com/b208e554c9f82733d7cc1e9a5dca8272431ade75/cc/trees/layer_tree_host_impl_unittest.cc [modify] https://crrev.com/b208e554c9f82733d7cc1e9a5dca8272431ade75/cc/trees/layer_tree_host_unittest_animation.cc
,
Mar 6 2018
,
Jun 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/173ad65bd33ae90ec2259f483bfa3f4360eb5448 commit 173ad65bd33ae90ec2259f483bfa3f4360eb5448 Author: Yi Gu <yigu@chromium.org> Date: Fri Jun 15 17:33:46 2018 Only mutate running worklet animations which have an updated input time Previously any time any worklet animations were run, all worklet animations are run regardless of whether their input time has changed. This avoids running animations which haven't changed and reuses their value from the previous animation call. Note that worklet animations that are being removed still get mutated even though the input time doesn't change. Bug: 791279 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I212d1da620899ab3f02fceae07eda092d2b39fdb Reviewed-on: https://chromium-review.googlesource.com/1101655 Commit-Queue: Yi Gu <yigu@chromium.org> Reviewed-by: Robert Flack <flackr@chromium.org> Cr-Commit-Position: refs/heads/master@{#567712} [modify] https://crrev.com/173ad65bd33ae90ec2259f483bfa3f4360eb5448/cc/animation/worklet_animation.cc [modify] https://crrev.com/173ad65bd33ae90ec2259f483bfa3f4360eb5448/cc/animation/worklet_animation_unittest.cc
,
Jun 15 2018
That is great. Now I think we don't need to check NeedsUpdate inside AnimationHost::NeedsTickMutator [1]. Instead we should let CollectWorkletAnimationsState collect state and then we don't call mutate if that state is empty. wdyt? [1]https://codesearch.chromium.org/chromium/src/cc/animation/animation_host.cc?type=cs&sq=package:chromium&g=0&l=289
,
Jun 15 2018
SGTM. Working on it.
,
Jun 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e27dda0104220f4741a63828771202d84d81849a commit e27dda0104220f4741a63828771202d84d81849a Author: Yi Gu <yigu@chromium.org> Date: Fri Jun 15 23:56:54 2018 [cc] Avoid checking WorkletAnimation::NeedsUpdate() twice Currently we check whether ticking mutator is needed and if so we collect mutator input state. There are duplicated logic in the two operations above. This patch is to skip the first check and collect input state directly. Then mutate if the input state is not empty. Bug: 791279 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ib4c7b92ca0b545b42d75b91bc3fd7f9296034d74 Reviewed-on: https://chromium-review.googlesource.com/1103013 Commit-Queue: Yi Gu <yigu@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: Majid Valipour <majidvp@chromium.org> Cr-Commit-Position: refs/heads/master@{#567830} [modify] https://crrev.com/e27dda0104220f4741a63828771202d84d81849a/cc/animation/animation_host.cc [modify] https://crrev.com/e27dda0104220f4741a63828771202d84d81849a/cc/animation/animation_host.h [modify] https://crrev.com/e27dda0104220f4741a63828771202d84d81849a/cc/animation/worklet_animation.h [modify] https://crrev.com/e27dda0104220f4741a63828771202d84d81849a/cc/animation/worklet_animation_unittest.cc [modify] https://crrev.com/e27dda0104220f4741a63828771202d84d81849a/cc/trees/layer_tree_mutator.h
,
Jul 3
Since #6 is landed we are now only mutating those animations whose input has changed. This is fairly optimal so should we close this as fixed?
,
Jul 3
I think the goal of this bug has been achieved. Close it as fixed. Thanks. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by petermayo@chromium.org
, Dec 6 2017