Cache render surface in animations in CrOS. |
||||||||||
Issue descriptionCreate this master issue to add cache render surface in various animations in CrOS.
,
Aug 8 2017
,
Aug 8 2017
Maybe we should integrate this with ui::LayerAnimator? Some background; We don't want to force render surface caching for all UI animations but for a large set we'd like to use this. Anything that animates opacity typically wants this but there are cases where we animate opacity and should avoid it (one layer in the animation) and there are cases where we're not animating opacity but would still like to enable this.
,
Aug 8 2017
Integrating with ui::LayerAnimator sounds like a good approach. Given that the decision to enable/disable isn't strictly a function of the animation type, is this something that would need to be added as a flag on LayerAnimationSequence?
,
Aug 8 2017
LayerAnimationSequence sounds good. Looks like we have the following changes: 1. Because the SetCacheRenderSurface flag is set on ui::Layer, we need to modify the LayerAnimationDelegate to have SetCacheRenderSurfaceFromAnimation(bool). 2. Add a flag needs_cache_render_surface in LayerAnimationSequence, this flag can be set from ScopedLayerAnimationSettings or LayerAnimationSequence, similar to LayerAnimationSequence::SetAnimationMetricsReporter [1]. [1] https://cs.chromium.org/chromium/src/ui/compositor/layer_animation_sequence.cc?l=247&rcl=299a73a1f9756946d1df44ce03aa281647c9fc56 3. In LayerAnimationSequence, modify OnStart, OnProgress, ProgressToEnd, OnAbort to set the layer cache flag accordingly to new and old cache flag values.
,
Aug 9 2017
Seems the plan in #3 and #5 may not work. One layer (animator) might have multiple sequences, when one sequence start/finish, it will change the cc_layer_->cache_render_surface flag. This will affect how other sequences to get and set this flag. So this flag still need to stay at LayerAnimator or ui::Layer level, and only be cleared after all the animating sequences finished, which can only be done by adding an observer. I will try to add this internal observer for this flag clear up.
,
Aug 10 2017
I am considering to add an internal observer to ScopedLayerAnimationSettings, and a function: ScopedLayerAnimationSettings::CacheRenderSurfaceForLayer(Layer* layer).
void ScopedLayerAnimationSettings::CacheRenderSurfaceForLayer(
ui::Layer* layer) {
AddObserver(new CleanupCacheRenderSurfaceObserver(layer));
}
In the observer ctor, layer->SetCacheRenderSurface(true).
in the observer dtor, layer->SetCacheRenderSurface(old_true).
Will upload a cl soon.
,
Aug 10 2017
Uploaded a cl: https://chromium-review.googlesource.com/c/610939
,
Aug 18 2017
,
Aug 18 2017
,
Aug 18 2017
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/52217d1a68d9a011960d8f901db0bf19592b6bf1 commit 52217d1a68d9a011960d8f901db0bf19592b6bf1 Author: wutao <wutao@chromium.org> Date: Fri Aug 18 02:40:31 2017 Add CacheRenderSurfaceObserver to ScopedLayerAnimationSettings. We want to force render surface caching for a large set of animations in CrOS. We would like to set the cache flag on the animating layer and unset the flag upon animation finish. Adding an observer to achieve this. Changes: 1. A counter in ui::Layer to maintain the number of requests for render surface caching. 2. New observer in ScopedLayerAnimationSettings observing both layer destroyed and animation completion. 3. In the new observer increase the counter in the ctor and decrease the request counter when called. using this new settings. Bug: 753224 Test: New LayerAnimator unittest and Checking cache in CrossFade animation Change-Id: I08f1078acbd2e3fb40a90ad3397caef8ed1ebeba Reviewed-on: https://chromium-review.googlesource.com/610939 Commit-Queue: Tao Wu <wutao@chromium.org> Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org> Reviewed-by: David Reveman <reveman@chromium.org> Reviewed-by: Ali Juma <ajuma@chromium.org> Cr-Commit-Position: refs/heads/master@{#495437} [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ash/wallpaper/wallpaper_widget_controller.cc [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ash/wallpaper/wallpaper_widget_controller.h [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/layer.cc [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/layer.h [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/layer_animation_delegate.h [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/layer_animator_unittest.cc [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/layer_observer.h [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/layer_unittest.cc [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/scoped_layer_animation_settings.cc [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/scoped_layer_animation_settings.h [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/test/test_layer_animation_delegate.cc [modify] https://crrev.com/52217d1a68d9a011960d8f901db0bf19592b6bf1/ui/compositor/test/test_layer_animation_delegate.h
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/265bdd924659ecc28f1dac34653f38ce1d2fd082 commit 265bdd924659ecc28f1dac34653f38ce1d2fd082 Author: wutao <wutao@chromium.org> Date: Fri Aug 18 17:10:40 2017 CrOS: Cache render surface in cross fade animation. Cache render surface in animation can improve the UI performance significantly. We have built up fundamental blocks already. In this cl, we are caching the render surface in cross fade animation. Performance improvement: Linux build: ~50% LINK: ~20% KEVIN: ~15% Bug: 756695 , 753224 Test: Tested improvement with/out cache. Change-Id: Iad42f124b4e77c3c171bd0d59ea7b6f2831ae7f3 Reviewed-on: https://chromium-review.googlesource.com/619931 Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Commit-Queue: Tao Wu <wutao@chromium.org> Cr-Commit-Position: refs/heads/master@{#495591} [modify] https://crrev.com/265bdd924659ecc28f1dac34653f38ce1d2fd082/ash/wm/window_animations.cc
,
Sep 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb6613a035e615799d30f141cc4d140dc06d134f commit fb6613a035e615799d30f141cc4d140dc06d134f Author: wutao <wutao@chromium.org> Date: Thu Sep 07 21:43:36 2017 CrOS: Cache render surface in hide animation. Cache render surface in animation can improve the UI performance significantly. We have built up fundamental blocks already. In this cl, we are caching the render surface in hide animation. Performance improvement: Linux build: ~30% KEVIN: ~15% Bug: 756693 , 753224 Test: Tested improvement with/out cache. Change-Id: I539a8407ae9742593a4a44de8f2542fb5f2630f9 Reviewed-on: https://chromium-review.googlesource.com/620132 Commit-Queue: Tao Wu <wutao@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: David Reveman <reveman@chromium.org> Cr-Commit-Position: refs/heads/master@{#500398} [modify] https://crrev.com/fb6613a035e615799d30f141cc4d140dc06d134f/ui/wm/core/window_animations.cc
,
Sep 13 2017
,
Sep 21 2017
,
Aug 1
,
Oct 5
We do not have plan to apply this into more animations for now. Close this bug. And will create new one if needed. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by wutao@chromium.org
, Aug 8 2017