New issue
Advanced search Search tips

Issue 592873 link

Starred by 2 users
Status: Fixed
Owner:
loyso@chromium.org
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature



Sign in to add a comment

Compositor Animation: Move players between timelines without any animation losses.

Project Member Reported by loyso@chromium.org, Mar 8 2016 
In general, we must allow to move AnimationPlayers between timelines within same Compositor(AnimationHost) without any animation losses. (To be designed separately)

See this issue: https://codereview.chromium.org/1776513002/

Also, we must rename ui::LayerAnimator::Se/ResetCompositor.

Comment 1 by loyso@chromium.org, Mar 17 2016

Blockedon: 595571

Comment 2 by loyso@chromium.org, Mar 17 2016

Blockedon: -595571

Comment 3 by loyso@chromium.org, May 11 2016

Labels: OS-All

Comment 4 by loyso@chromium.org, Aug 25 2016 

ElementAnimations::AddAnimation is a legacy from LAC. We want to move animations_
array (together with Add/Remove etc methods) from ElementAnimations to
AnimationPlayer.

Overall, we decided with vollick@ that we should make ElementAnimations as small
as possible and hide it eventually.

Also, we must to rewrite ElementAnimations tests to use AnimationPlayer instead.

Comment 5 by loyso@chromium.org, Sep 7 2016

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db00688793f63e1f3ccd71f883ec3129bd169abe

commit db00688793f63e1f3ccd71f883ec3129bd169abe
Author: loyso <loyso@chromium.org>
Date: Wed Sep 14 00:31:51 2016

CC Animation: Move animations_ from ElementAnimations to AnimationPlayer.

- Also move all accompanying methods.
- We don't have a notion of pending animations anymore.

As a result, we will get many (*:1 relationship) collections of animations for each element
(animation's target).

This a part of the plan to make ElementAnimations a hidden
and passive entity (not presented in API and unit tests).

Next steps (separate CLs):
- Shouldn't manually iterate through the list if base::ObserverList
( http://crbug.com/634916 )
- Rework all UpdateClientAnimationState callers to use bitset.
- Remove AnimationPlayer::animations() getter
- Change activation to be on a per-player basis
(so AnimationHost works with a list of active
AnimationPlayers rather than ElementAnimations)
- CompositorPlayer::addAnimation should consume unique_ptr.

BUG= 592873 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2335043002
Cr-Commit-Position: refs/heads/master@{#418439}

[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/cc/animation/animation_player.cc
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/cc/animation/animation_player.h
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/cc/animation/element_animations.cc
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/cc/animation/element_animations.h
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/cc/animation/element_animations_unittest.cc
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/cc/test/animation_timelines_test_common.h
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/third_party/WebKit/Source/platform/animation/CompositorAnimationPlayerTest.cpp
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/ui/compositor/layer.cc
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/ui/compositor/layer.h
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/ui/compositor/layer_animator.cc
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/ui/compositor/layer_animator.h
[modify] https://crrev.com/db00688793f63e1f3ccd71f883ec3129bd169abe/ui/compositor/layer_unittest.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01c072138ff699b606cfca552ab047239482cafd

commit 01c072138ff699b606cfca552ab047239482cafd
Author: loyso <loyso@chromium.org>
Date: Wed Sep 14 04:16:09 2016

Blink Compositor Animation: CompositorPlayer::addAnimation to use unique_ptr.

Also, perform some clean-up.

BUG= 592873 

Review-Url: https://codereview.chromium.org/2338803003
Cr-Commit-Position: refs/heads/master@{#418483}

[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/core/animation/CompositorAnimations.cpp
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/platform/animation/CompositorAnimation.cpp
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/platform/animation/CompositorAnimation.h
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/platform/animation/CompositorAnimationPlayer.cpp
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/platform/animation/CompositorAnimationPlayer.h
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/platform/animation/CompositorAnimationPlayerTest.cpp
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/platform/animation/CompositorAnimationTimeline.cpp
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/platform/graphics/GraphicsLayerTest.cpp
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.cpp
[modify] https://crrev.com/01c072138ff699b606cfca552ab047239482cafd/third_party/WebKit/Source/web/LinkHighlightImpl.cpp

Comment 8 by loyso@chromium.org, Sep 14 2016

Blockedon: 634916

Comment 9 by loyso@chromium.org, Sep 30 2016 

LGTMed CLs:

2340923003: CC Animation: Rework iterations over players to use the range-based for loop.
2349643003: CC Animation: Extract PropertyAnimationState as a non-nested struct.
2357533002: CC Animation: Use std::bitset to update animation state.
2363733004: UI Compositor Animation: Remove cc::ElementAnimations state storage.
2377223002: CC Animations: Rewrite unit tests to work with AnimationPlayer.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 4 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/663ada2d888d28d7d219e9bf4f56b27b287bdb62

commit 663ada2d888d28d7d219e9bf4f56b27b287bdb62
Author: loyso <loyso@chromium.org>
Date: Tue Oct 04 04:48:08 2016

CC Animation: Use std::bitset to update animation state.

BUG= 592873 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2357533002
Cr-Commit-Position: refs/heads/master@{#422709}

[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/animation/animation_player.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/animation/animation_player.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/animation/element_animations.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/animation/element_animations.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/animation/property_animation_state.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/animation/property_animation_state.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/animation/target_property.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/animation/target_property.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/layers/layer.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/layers/layer.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/layers/layer_impl.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/layers/layer_impl.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/test/animation_timelines_test_common.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/trees/layer_tree.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/trees/layer_tree.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/663ada2d888d28d7d219e9bf4f56b27b287bdb62/cc/trees/mutator_host_client.h

Comment 12 by loyso@chromium.org, Oct 4 2016

Blockedon: -634916
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 4 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc8e3dbd19a6f1747cb4df0d8913c9dceb5c56d6

commit cc8e3dbd19a6f1747cb4df0d8913c9dceb5c56d6
Author: loyso <loyso@chromium.org>
Date: Tue Oct 04 05:22:03 2016

UI Compositor Animation: Remove cc::ElementAnimations state storage.

We don't need to preserve the state of cc::ElementAnimations between
detach/attach. All state lives in cc::AnimationPlayer now.

BUG= 592873 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2363733004
Cr-Commit-Position: refs/heads/master@{#422711}

[modify] https://crrev.com/cc8e3dbd19a6f1747cb4df0d8913c9dceb5c56d6/cc/animation/animation_host.cc
[modify] https://crrev.com/cc8e3dbd19a6f1747cb4df0d8913c9dceb5c56d6/cc/animation/animation_host.h
[modify] https://crrev.com/cc8e3dbd19a6f1747cb4df0d8913c9dceb5c56d6/ui/compositor/layer.cc
[modify] https://crrev.com/cc8e3dbd19a6f1747cb4df0d8913c9dceb5c56d6/ui/compositor/layer_animator.cc
[modify] https://crrev.com/cc8e3dbd19a6f1747cb4df0d8913c9dceb5c56d6/ui/compositor/layer_animator.h
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 4 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb50779edb1c0c7a08c5873c9d246c9a30687703

commit bb50779edb1c0c7a08c5873c9d246c9a30687703
Author: loyso <loyso@chromium.org>
Date: Tue Oct 04 05:31:02 2016

CC Animations: Rewrite unit tests to work with AnimationPlayer.

We add/remove/etc animations to AnimationPlayer now.
ElementAnimations is a light helper class.

BUG= 592873 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2377223002
Cr-Commit-Position: refs/heads/master@{#422714}

[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/animation/animation_host.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/animation/animation_player.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/animation/animation_player.h
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/animation/animation_player_unittest.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/animation/element_animations.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/animation/element_animations.h
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/animation/element_animations_unittest.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/animation/scroll_offset_animations_impl.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/test/animation_test_common.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/test/animation_test_common.h
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/test/animation_timelines_test_common.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/test/animation_timelines_test_common.h
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/trees/layer_tree_host_unittest_animation.cc
[modify] https://crrev.com/bb50779edb1c0c7a08c5873c9d246c9a30687703/cc/trees/mutator_host_client.h

Comment 15 by loyso@chromium.org, Oct 4 2016

Status: Fixed (was: Started)
►

Sign in to add a comment