Skip to content

Commit

Permalink
Revert "Reland Group effects support in cc::AnimationPlayer"
Browse files Browse the repository at this point in the history
This reverts commit 5978ffa.

Reason for revert: Seems to cause failure in linux-chromeos-rel test:
  telemetry.internal.actions.scroll_unittest.ScrollActionTest.testWheelScrollDistanceWhileZoomed

Original change's description:
> Reland Group effects support in cc::AnimationPlayer
> 
> 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}

TBR=vollick@chromium.org,yigu@chromium.org

Change-Id: Ic82337555158a49ec24ae10fb7a6d63db3114828
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 767043
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/891998
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532623}
  • Loading branch information
samuelhuang authored and Commit Bot committed Jan 29, 2018
1 parent 6a6ad1c commit 9d7782d
Show file tree
Hide file tree
Showing 33 changed files with 927 additions and 1,915 deletions.
2 changes: 0 additions & 2 deletions cc/animation/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ 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: 10 additions & 9 deletions cc/animation/animation_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#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 @@ -124,10 +123,10 @@ void AnimationHost::UnregisterElement(ElementId element_id,
element_animations->ElementUnregistered(element_id, list_type);
}

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

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

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

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

scoped_refptr<ElementAnimations> element_animations =
GetElementAnimationsForElementId(element_id);
DCHECK(element_animations);
element_animations->RemoveTicker(ticker);
element_animations->RemoveTicker(player->animation_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: 3 additions & 4 deletions cc/animation/animation_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class ScrollOffset;
namespace cc {

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

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

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

#include "base/threading/thread_task_runner_handle.h"
#include "cc/animation/animation_id_provider.h"
#include "cc/animation/animation_ticker.h"
#include "cc/animation/animation_player.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 @@ -70,13 +69,13 @@ class AnimationHostPerfTest : public testing::Test {
root_layer_->AddChild(layer);
layer->SetElementId(LayerIdToElementIdForTesting(layer->id()));

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

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

// Create impl players.
Expand Down
Loading

0 comments on commit 9d7782d

Please sign in to comment.