Commit a76f65e3 authored by Olga Gerchikov's avatar Olga Gerchikov Committed by Commit Bot

Ensure uniqueness of TIME_UPDATED events per animation passed to begin main frame

This change reduces number of TIME_UPDATED events passed to main thread.
When syncing time from compositor to main thread, only the last time
value produced on the compositor thread is relevant. Passing all other
values is unnecessary overhead.

Bug: 1013654
Change-Id: Id950d48f2b36c6afb2d74cf26605c490767121b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2001260
Commit-Queue: Olga Gerchikov <gerchiko@microsoft.com>
Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732872}
parent b01dc913
...@@ -96,6 +96,9 @@ class CC_ANIMATION_EXPORT Animation : public base::RefCounted<Animation> { ...@@ -96,6 +96,9 @@ class CC_ANIMATION_EXPORT Animation : public base::RefCounted<Animation> {
virtual void UpdateState(bool start_ready_keyframe_models, virtual void UpdateState(bool start_ready_keyframe_models,
AnimationEvents* events); AnimationEvents* events);
// Adds TIME_UPDATED event generated in the current frame to the given
// animation events.
virtual void TakeTimeUpdatedEvent(AnimationEvents* events) {}
virtual void Tick(base::TimeTicks monotonic_time); virtual void Tick(base::TimeTicks monotonic_time);
void AddToTicking(); void AddToTicking();
......
...@@ -63,12 +63,12 @@ AnimationEvent& AnimationEvent::operator=(const AnimationEvent& other) { ...@@ -63,12 +63,12 @@ AnimationEvent& AnimationEvent::operator=(const AnimationEvent& other) {
AnimationEvent::~AnimationEvent() = default; AnimationEvent::~AnimationEvent() = default;
AnimationEvents::AnimationEvents() = default; AnimationEvents::AnimationEvents() : needs_time_updated_events_(false) {}
AnimationEvents::~AnimationEvents() = default; AnimationEvents::~AnimationEvents() = default;
bool AnimationEvents::IsEmpty() const { bool AnimationEvents::IsEmpty() const {
return events_.empty(); return events_.empty() && !needs_time_updated_events_;
} }
bool AnimationEvent::ShouldDispatchToKeyframeEffectAndModel() const { bool AnimationEvent::ShouldDispatchToKeyframeEffectAndModel() const {
......
...@@ -65,7 +65,17 @@ class CC_ANIMATION_EXPORT AnimationEvents : public MutatorEvents { ...@@ -65,7 +65,17 @@ class CC_ANIMATION_EXPORT AnimationEvents : public MutatorEvents {
~AnimationEvents() override; ~AnimationEvents() override;
bool IsEmpty() const override; bool IsEmpty() const override;
bool needs_time_updated_events() const { return needs_time_updated_events_; }
void set_needs_time_updated_events(bool value) {
needs_time_updated_events_ = value;
}
// TODO(gerchiko): Make events_ a private member variable with methods to add
// and retrieve the events.
std::vector<AnimationEvent> events_; std::vector<AnimationEvent> events_;
private:
bool needs_time_updated_events_;
}; };
} // namespace cc } // namespace cc
......
...@@ -472,6 +472,17 @@ bool AnimationHost::UpdateAnimationState(bool start_ready_animations, ...@@ -472,6 +472,17 @@ bool AnimationHost::UpdateAnimationState(bool start_ready_animations,
return true; return true;
} }
void AnimationHost::TakeTimeUpdatedEvents(MutatorEvents* events) {
auto* animation_events = static_cast<AnimationEvents*>(events);
if (!animation_events->needs_time_updated_events())
return;
for (auto& it : ticking_animations_)
it->TakeTimeUpdatedEvent(animation_events);
animation_events->set_needs_time_updated_events(false);
}
void AnimationHost::PromoteScrollTimelinesPendingToActive() { void AnimationHost::PromoteScrollTimelinesPendingToActive() {
for (auto& animation : ticking_animations_) { for (auto& animation : ticking_animations_) {
animation->PromoteScrollTimelinePendingToActive(); animation->PromoteScrollTimelinePendingToActive();
......
...@@ -119,6 +119,7 @@ class CC_ANIMATION_EXPORT AnimationHost : public MutatorHost, ...@@ -119,6 +119,7 @@ class CC_ANIMATION_EXPORT AnimationHost : public MutatorHost,
void TickWorkletAnimations() override; void TickWorkletAnimations() override;
bool UpdateAnimationState(bool start_ready_animations, bool UpdateAnimationState(bool start_ready_animations,
MutatorEvents* events) override; MutatorEvents* events) override;
void TakeTimeUpdatedEvents(MutatorEvents* events) override;
void PromoteScrollTimelinesPendingToActive() override; void PromoteScrollTimelinesPendingToActive() override;
std::unique_ptr<MutatorEvents> CreateEvents() override; std::unique_ptr<MutatorEvents> CreateEvents() override;
......
...@@ -177,7 +177,7 @@ TEST_F(AnimationHostTest, FastLayerTreeMutatorUpdateTakesEffectInSameFrame) { ...@@ -177,7 +177,7 @@ TEST_F(AnimationHostTest, FastLayerTreeMutatorUpdateTakesEffectInSameFrame) {
// Ticking host should cause layer tree mutator to update output state which // Ticking host should cause layer tree mutator to update output state which
// should take effect in the same animation frame. // should take effect in the same animation frame.
TickAnimationsTransferEvents(base::TimeTicks(), 1u); TickAnimationsTransferEvents(base::TimeTicks(), 0u);
// Emulate behavior in PrepareToDraw. Animation worklet updates are best // Emulate behavior in PrepareToDraw. Animation worklet updates are best
// effort, and the animation tick is deferred until draw to allow time for the // effort, and the animation tick is deferred until draw to allow time for the
......
...@@ -113,11 +113,14 @@ void WorkletAnimation::Tick(base::TimeTicks monotonic_time) { ...@@ -113,11 +113,14 @@ void WorkletAnimation::Tick(base::TimeTicks monotonic_time) {
void WorkletAnimation::UpdateState(bool start_ready_animations, void WorkletAnimation::UpdateState(bool start_ready_animations,
AnimationEvents* events) { AnimationEvents* events) {
Animation::UpdateState(start_ready_animations, events); Animation::UpdateState(start_ready_animations, events);
if (last_synced_local_time_ != local_time_)
events->set_needs_time_updated_events(true);
}
void WorkletAnimation::TakeTimeUpdatedEvent(AnimationEvents* events) {
DCHECK(events->needs_time_updated_events());
if (last_synced_local_time_ != local_time_) { if (last_synced_local_time_ != local_time_) {
AnimationEvent event(animation_timeline()->id(), id_, local_time_); AnimationEvent event(animation_timeline()->id(), id_, local_time_);
// TODO(http://crbug.com/1013654): Instead of pushing multiple events per
// single main frame, push just the recent one to be handled by next main
// frame.
events->events_.push_back(event); events->events_.push_back(event);
last_synced_local_time_ = local_time_; last_synced_local_time_ = local_time_;
} }
......
...@@ -66,6 +66,7 @@ class CC_ANIMATION_EXPORT WorkletAnimation final : public Animation { ...@@ -66,6 +66,7 @@ class CC_ANIMATION_EXPORT WorkletAnimation final : public Animation {
void UpdateState(bool start_ready_animations, void UpdateState(bool start_ready_animations,
AnimationEvents* events) override; AnimationEvents* events) override;
void TakeTimeUpdatedEvent(AnimationEvents* events) override;
void UpdateInputState(MutatorInputState* input_state, void UpdateInputState(MutatorInputState* input_state,
base::TimeTicks monotonic_time, base::TimeTicks monotonic_time,
const ScrollTree& scroll_tree, const ScrollTree& scroll_tree,
......
...@@ -128,6 +128,8 @@ TEST_F(WorkletAnimationTest, AnimationEventLocalTimeUpdate) { ...@@ -128,6 +128,8 @@ TEST_F(WorkletAnimationTest, AnimationEventLocalTimeUpdate) {
worklet_animation_->UpdateState(true, animation_events); worklet_animation_->UpdateState(true, animation_events);
// One event is generated as a result of update state. // One event is generated as a result of update state.
EXPECT_TRUE(animation_events->needs_time_updated_events());
worklet_animation_->TakeTimeUpdatedEvent(animation_events);
EXPECT_EQ(1u, animation_events->events_.size()); EXPECT_EQ(1u, animation_events->events_.size());
AnimationEvent event = animation_events->events_[0]; AnimationEvent event = animation_events->events_[0];
EXPECT_EQ(AnimationEvent::TIME_UPDATED, event.type); EXPECT_EQ(AnimationEvent::TIME_UPDATED, event.type);
...@@ -138,14 +140,14 @@ TEST_F(WorkletAnimationTest, AnimationEventLocalTimeUpdate) { ...@@ -138,14 +140,14 @@ TEST_F(WorkletAnimationTest, AnimationEventLocalTimeUpdate) {
mutator_events = host_->CreateEvents(); mutator_events = host_->CreateEvents();
animation_events = static_cast<AnimationEvents*>(mutator_events.get()); animation_events = static_cast<AnimationEvents*>(mutator_events.get());
worklet_animation_->UpdateState(true, animation_events); worklet_animation_->UpdateState(true, animation_events);
EXPECT_EQ(0u, animation_events->events_.size()); EXPECT_FALSE(animation_events->needs_time_updated_events());
// If local time is set to the same value no event is generated. // If local time is set to the same value no event is generated.
worklet_animation_->SetOutputState(state); worklet_animation_->SetOutputState(state);
mutator_events = host_->CreateEvents(); mutator_events = host_->CreateEvents();
animation_events = static_cast<AnimationEvents*>(mutator_events.get()); animation_events = static_cast<AnimationEvents*>(mutator_events.get());
worklet_animation_->UpdateState(true, animation_events); worklet_animation_->UpdateState(true, animation_events);
EXPECT_EQ(0u, animation_events->events_.size()); EXPECT_FALSE(animation_events->needs_time_updated_events());
// If local time is set to null value, an animation event with null local // If local time is set to null value, an animation event with null local
// time is generated. // time is generated.
...@@ -156,6 +158,8 @@ TEST_F(WorkletAnimationTest, AnimationEventLocalTimeUpdate) { ...@@ -156,6 +158,8 @@ TEST_F(WorkletAnimationTest, AnimationEventLocalTimeUpdate) {
mutator_events = host_->CreateEvents(); mutator_events = host_->CreateEvents();
animation_events = static_cast<AnimationEvents*>(mutator_events.get()); animation_events = static_cast<AnimationEvents*>(mutator_events.get());
worklet_animation_->UpdateState(true, animation_events); worklet_animation_->UpdateState(true, animation_events);
EXPECT_TRUE(animation_events->needs_time_updated_events());
worklet_animation_->TakeTimeUpdatedEvent(animation_events);
EXPECT_EQ(1u, animation_events->events_.size()); EXPECT_EQ(1u, animation_events->events_.size());
EXPECT_EQ(local_time, animation_events->events_[0].local_time); EXPECT_EQ(local_time, animation_events->events_[0].local_time);
} }
......
...@@ -3400,6 +3400,7 @@ LayerTreeHostImpl::TakeCompletedImageDecodeRequests() { ...@@ -3400,6 +3400,7 @@ LayerTreeHostImpl::TakeCompletedImageDecodeRequests() {
std::unique_ptr<MutatorEvents> LayerTreeHostImpl::TakeMutatorEvents() { std::unique_ptr<MutatorEvents> LayerTreeHostImpl::TakeMutatorEvents() {
std::unique_ptr<MutatorEvents> events = mutator_host_->CreateEvents(); std::unique_ptr<MutatorEvents> events = mutator_host_->CreateEvents();
std::swap(events, mutator_events_); std::swap(events, mutator_events_);
mutator_host_->TakeTimeUpdatedEvents(events.get());
return events; return events;
} }
......
...@@ -78,6 +78,9 @@ class MutatorHost { ...@@ -78,6 +78,9 @@ class MutatorHost {
virtual void TickWorkletAnimations() = 0; virtual void TickWorkletAnimations() = 0;
virtual bool UpdateAnimationState(bool start_ready_animations, virtual bool UpdateAnimationState(bool start_ready_animations,
MutatorEvents* events) = 0; MutatorEvents* events) = 0;
// Returns TIME_UPDATED events generated in this frame to be handled by
// BeginMainFrame.
virtual void TakeTimeUpdatedEvents(MutatorEvents* events) = 0;
virtual void PromoteScrollTimelinesPendingToActive() = 0; virtual void PromoteScrollTimelinesPendingToActive() = 0;
virtual std::unique_ptr<MutatorEvents> CreateEvents() = 0; virtual std::unique_ptr<MutatorEvents> CreateEvents() = 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment