Skip to content

Commit

Permalink
Reland Group effects support in cc::AnimationPlayer
Browse files Browse the repository at this point in the history
Patch https://chromium-review.googlesource.com/c/chromium/src/+/742162
got reverted because it caused cc_unittest failure on CFI bot. This
patch fixed the bug.

Commit message from the original patch:

The current cc/animations logic assumes a single animation has a single
keyframe effect and can only affect a single layer. To enable animations
with multiple keyframe effects, cc::AnimationPlayer need to support
multiple AnimationTickers each corresponding to one keyframe effect.

Currently there is a 1:1 relationship between AnimationPlayer and
AnimationTicker. This patch is to extend it to 1:n. Here is a summary of
changes:
- Introduce a sub-class of AnimationPlayer, a.k.a
SingleTickerAnimationPlayer, to handle the existing logic (single
effect). SingleTickerAnimationPlayer owns only one AnimationTicker as
the AnimationPlayer does today.
- Currently a AnimationTicker is created upon creating AnimationPlayer.
In this patch, tickers are created separately and added to the player
afterwards. Tickers that the player owns may belong to different targets
therefore the player needs to coordinate with AnimationHost regarding
this situation.
- Adjust existing unit tests according to the changes above.

Bug: 767043
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: If21bad1285c35bbc048fef6b619c8272c0760551
Reviewed-on: https://chromium-review.googlesource.com/890724
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532519}
  • Loading branch information
yi-gu authored and Commit Bot committed Jan 29, 2018
1 parent e7fa755 commit 5978ffa
Show file tree
Hide file tree
Showing 33 changed files with 1,915 additions and 927 deletions.
2 changes: 2 additions & 0 deletions cc/animation/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ cc_component("animation") {
"scroll_offset_animations_impl.h",
"scroll_timeline.cc",
"scroll_timeline.h",
"single_ticker_animation_player.cc",
"single_ticker_animation_player.h",
"timing_function.cc",
"timing_function.h",
"transform_operation.cc",
Expand Down
19 changes: 9 additions & 10 deletions cc/animation/animation_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "cc/animation/animation_events.h"
#include "cc/animation/animation_id_provider.h"
#include "cc/animation/animation_player.h"
#include "cc/animation/animation_ticker.h"
#include "cc/animation/animation_timeline.h"
#include "cc/animation/element_animations.h"
#include "cc/animation/scroll_offset_animation_curve.h"
Expand Down Expand Up @@ -123,10 +124,10 @@ void AnimationHost::UnregisterElement(ElementId element_id,
element_animations->ElementUnregistered(element_id, list_type);
}

void AnimationHost::RegisterPlayerForElement(ElementId element_id,
AnimationPlayer* player) {
void AnimationHost::RegisterTickerForElement(ElementId element_id,
AnimationTicker* ticker) {
DCHECK(element_id);
DCHECK(player);
DCHECK(ticker);

scoped_refptr<ElementAnimations> element_animations =
GetElementAnimationsForElementId(element_id);
Expand All @@ -142,26 +143,24 @@ void AnimationHost::RegisterPlayerForElement(ElementId element_id,
element_animations->InitAffectedElementTypes();
}

element_animations->AddTicker(player->animation_ticker());
element_animations->AddTicker(ticker);
}

void AnimationHost::UnregisterPlayerForElement(ElementId element_id,
AnimationPlayer* player) {
void AnimationHost::UnregisterTickerForElement(ElementId element_id,
AnimationTicker* ticker) {
DCHECK(element_id);
DCHECK(player);
DCHECK(ticker);

scoped_refptr<ElementAnimations> element_animations =
GetElementAnimationsForElementId(element_id);
DCHECK(element_animations);
element_animations->RemoveTicker(player->animation_ticker());
element_animations->RemoveTicker(ticker);

if (element_animations->IsEmpty()) {
element_animations->ClearAffectedElementTypes();
element_to_animations_map_.erase(element_animations->element_id());
element_animations->SetAnimationHost(nullptr);
}

RemoveFromTicking(player);
}

void AnimationHost::SetMutatorHostClient(MutatorHostClient* client) {
Expand Down
7 changes: 4 additions & 3 deletions cc/animation/animation_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class ScrollOffset;
namespace cc {

class AnimationPlayer;
class AnimationTicker;
class AnimationTimeline;
class ElementAnimations;
class LayerTreeHost;
Expand Down Expand Up @@ -61,9 +62,9 @@ class CC_ANIMATION_EXPORT AnimationHost : public MutatorHost,
void RemoveAnimationTimeline(scoped_refptr<AnimationTimeline> timeline);
AnimationTimeline* GetTimelineById(int timeline_id) const;

void RegisterPlayerForElement(ElementId element_id, AnimationPlayer* player);
void UnregisterPlayerForElement(ElementId element_id,
AnimationPlayer* player);
void RegisterTickerForElement(ElementId element_id, AnimationTicker* ticker);
void UnregisterTickerForElement(ElementId element_id,
AnimationTicker* ticker);

scoped_refptr<ElementAnimations> GetElementAnimationsForElementId(
ElementId element_id) const;
Expand Down
9 changes: 5 additions & 4 deletions cc/animation/animation_host_perftest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@

#include "base/threading/thread_task_runner_handle.h"
#include "cc/animation/animation_id_provider.h"
#include "cc/animation/animation_player.h"
#include "cc/animation/animation_ticker.h"
#include "cc/animation/animation_timeline.h"
#include "cc/animation/single_ticker_animation_player.h"
#include "cc/base/lap_timer.h"
#include "cc/test/fake_impl_task_runner_provider.h"
#include "cc/test/fake_layer_tree_host.h"
Expand Down Expand Up @@ -69,13 +70,13 @@ class AnimationHostPerfTest : public testing::Test {
root_layer_->AddChild(layer);
layer->SetElementId(LayerIdToElementIdForTesting(layer->id()));

scoped_refptr<AnimationPlayer> player =
AnimationPlayer::Create(last_player_id_);
scoped_refptr<SingleTickerAnimationPlayer> player =
SingleTickerAnimationPlayer::Create(last_player_id_);
last_player_id_ = AnimationIdProvider::NextPlayerId();

all_players_timeline_->AttachPlayer(player);
player->AttachElement(layer->element_id());
EXPECT_TRUE(player->element_animations());
EXPECT_TRUE(player->element_animations(player->animation_ticker()->id()));
}

// Create impl players.
Expand Down
Loading

0 comments on commit 5978ffa

Please sign in to comment.