Skip to content

Commit

Permalink
Replace CurrentPhysicalTimeTicks with a fallback in CurrentFrameTimeT…
Browse files Browse the repository at this point in the history
…icks.

CurrentPhysicalTimeTicks was needed because some tasks are scheduled
independently from frames.  They may fall hundreds of milliseconds after
the last frame, so it's not appropriate to use the last frame time for
them.  So we used CurrentFrameTimeTicks for mid-animation timestamps and
CurrentPhysicalTimeTicks for input/random tasks (including the
scheduling of delayed animations).  But there are two problems with this:
1) It's very easy to use the wrong one by accident.
2) Providing alternately frame and physical time to the same method can
lead to nonmonotonic time.  The scrollbar fade controller currently
suffers from this and sometimes fails to fade the scrollbar as a result
(see bug).

This patch changes CurrentFrameTimeTicks to use physical time as a
fallback when frame time is unavailable.  This is monotonic and can't be
used incorrectly.

NOTRY=true
BUG=356032

Review URL: https://codereview.chromium.org/210793002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259735 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
aelias@chromium.org committed Mar 26, 2014
1 parent 09f8f54 commit 1dc0616
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 32 deletions.
2 changes: 1 addition & 1 deletion cc/layers/layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1285,7 +1285,7 @@ void LayerImpl::SetScrollbarPosition(ScrollbarLayerImplBase* scrollbar_layer,
// viewport).
if (scrollbar_animation_controller_) {
bool should_animate = scrollbar_animation_controller_->DidScrollUpdate(
layer_tree_impl_->CurrentPhysicalTimeTicks());
layer_tree_impl_->CurrentFrameTimeTicks());
if (should_animate)
layer_tree_impl_->StartScrollbarAnimation();
}
Expand Down
30 changes: 14 additions & 16 deletions cc/trees/layer_tree_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2462,8 +2462,7 @@ void LayerTreeHostImpl::MouseMoveAt(const gfx::Point& viewport_point) {
scroll_layer_impl ? scroll_layer_impl->scrollbar_animation_controller()
: NULL;
if (animation_controller) {
animation_controller->DidMouseMoveOffScrollbar(
CurrentPhysicalTimeTicks());
animation_controller->DidMouseMoveOffScrollbar(CurrentFrameTimeTicks());
StartScrollbarAnimation();
}
scroll_layer_id_when_mouse_over_scrollbar_ = 0;
Expand Down Expand Up @@ -2492,7 +2491,7 @@ void LayerTreeHostImpl::MouseMoveAt(const gfx::Point& viewport_point) {
DeviceSpaceDistanceToLayer(device_viewport_point, *it));

bool should_animate = animation_controller->DidMouseMoveNear(
CurrentPhysicalTimeTicks(), distance_to_scrollbar / device_scale_factor_);
CurrentFrameTimeTicks(), distance_to_scrollbar / device_scale_factor_);
if (should_animate)
StartScrollbarAnimation();
}
Expand All @@ -2506,7 +2505,7 @@ bool LayerTreeHostImpl::HandleMouseOverScrollbar(LayerImpl* layer_impl,
scroll_layer_id_when_mouse_over_scrollbar_ = scroll_layer_id;
bool should_animate =
layer_impl->scrollbar_animation_controller()->DidMouseMoveNear(
CurrentPhysicalTimeTicks(), 0);
CurrentFrameTimeTicks(), 0);
if (should_animate)
StartScrollbarAnimation();
} else {
Expand Down Expand Up @@ -2822,7 +2821,7 @@ void LayerTreeHostImpl::AnimateScrollbarsRecursive(LayerImpl* layer,

void LayerTreeHostImpl::StartScrollbarAnimation() {
TRACE_EVENT0("cc", "LayerTreeHostImpl::StartScrollbarAnimation");
StartScrollbarAnimationRecursive(RootLayer(), CurrentPhysicalTimeTicks());
StartScrollbarAnimationRecursive(RootLayer(), CurrentFrameTimeTicks());
}

void LayerTreeHostImpl::StartScrollbarAnimationRecursive(LayerImpl* layer,
Expand Down Expand Up @@ -2854,22 +2853,21 @@ void LayerTreeHostImpl::SetTreePriority(TreePriority priority) {
DidModifyTilePriorities();
}

void LayerTreeHostImpl::ResetCurrentFrameTimeForNextFrame() {
current_frame_timeticks_ = base::TimeTicks();
void LayerTreeHostImpl::UpdateCurrentFrameTime() {
DCHECK(current_frame_timeticks_.is_null());
current_frame_timeticks_ = gfx::FrameTime::Now();
}

void LayerTreeHostImpl::UpdateCurrentFrameTime(base::TimeTicks* ticks) const {
if (ticks->is_null()) {
*ticks = CurrentPhysicalTimeTicks();
}
void LayerTreeHostImpl::ResetCurrentFrameTimeForNextFrame() {
current_frame_timeticks_ = base::TimeTicks();
}

base::TimeTicks LayerTreeHostImpl::CurrentFrameTimeTicks() {
UpdateCurrentFrameTime(&current_frame_timeticks_);
return current_frame_timeticks_;
}

base::TimeTicks LayerTreeHostImpl::CurrentPhysicalTimeTicks() const {
// Try to use the current frame time to keep animations non-jittery. But if
// we're not in a frame (because this is during an input event or a delayed
// task), fall back to physical time. This should still be monotonic.
if (!current_frame_timeticks_.is_null())
return current_frame_timeticks_;
return gfx::FrameTime::Now();
}

Expand Down
5 changes: 1 addition & 4 deletions cc/trees/layer_tree_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,10 @@ class CC_EXPORT LayerTreeHostImpl

void SetTreePriority(TreePriority priority);

void UpdateCurrentFrameTime();
void ResetCurrentFrameTimeForNextFrame();
virtual base::TimeTicks CurrentFrameTimeTicks();

virtual base::TimeTicks CurrentPhysicalTimeTicks() const;

scoped_ptr<base::Value> AsValue() const { return AsValueWithFrame(NULL); }
scoped_ptr<base::Value> AsValueWithFrame(FrameData* frame) const;
scoped_ptr<base::Value> ActivationStateAsValue() const;
Expand Down Expand Up @@ -505,8 +504,6 @@ class CC_EXPORT LayerTreeHostImpl
void AnimateScrollbarsRecursive(LayerImpl* layer,
base::TimeTicks time);

void UpdateCurrentFrameTime(base::TimeTicks* ticks) const;

LayerImpl* FindScrollLayerForDeviceViewportPoint(
const gfx::PointF& device_viewport_point,
InputHandler::ScrollInputType type,
Expand Down
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_host_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ class LayerTreeHostImplOverridePhysicalTime : public LayerTreeHostImpl {
manager,
0) {}

virtual base::TimeTicks CurrentPhysicalTimeTicks() const OVERRIDE {
virtual base::TimeTicks CurrentFrameTimeTicks() OVERRIDE {
return fake_current_physical_time_;
}

Expand Down
8 changes: 2 additions & 6 deletions cc/trees/layer_tree_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ void LayerTreeImpl::SetCurrentlyScrollingLayer(LayerImpl* layer) {

if (currently_scrolling_layer_ &&
currently_scrolling_layer_->scrollbar_animation_controller())
currently_scrolling_layer_->scrollbar_animation_controller()->
DidScrollGestureEnd(CurrentPhysicalTimeTicks());
currently_scrolling_layer_->scrollbar_animation_controller()
->DidScrollGestureEnd(CurrentFrameTimeTicks());
currently_scrolling_layer_ = layer;
if (layer && layer->scrollbar_animation_controller())
layer->scrollbar_animation_controller()->DidScrollGestureBegin();
Expand Down Expand Up @@ -682,10 +682,6 @@ base::TimeTicks LayerTreeImpl::CurrentFrameTimeTicks() const {
return layer_tree_host_impl_->CurrentFrameTimeTicks();
}

base::TimeTicks LayerTreeImpl::CurrentPhysicalTimeTicks() const {
return layer_tree_host_impl_->CurrentPhysicalTimeTicks();
}

void LayerTreeImpl::SetNeedsCommit() {
layer_tree_host_impl_->SetNeedsCommit();
}
Expand Down
1 change: 0 additions & 1 deletion cc/trees/layer_tree_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class CC_EXPORT LayerTreeImpl {
int MaxTextureSize() const;
bool PinchGestureActive() const;
base::TimeTicks CurrentFrameTimeTicks() const;
base::TimeTicks CurrentPhysicalTimeTicks() const;
void SetNeedsCommit();
gfx::Size DrawViewportSize() const;
void StartScrollbarAnimation();
Expand Down
6 changes: 3 additions & 3 deletions cc/trees/thread_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ void ThreadProxy::BeginImplFrame(const BeginFrameArgs& args) {

// Sample the frame time now. This time will be used for updating animations
// when we draw.
impl().layer_tree_host_impl->CurrentFrameTimeTicks();
impl().layer_tree_host_impl->UpdateCurrentFrameTime();

impl().scheduler->BeginImplFrame(args);
}
Expand Down Expand Up @@ -771,7 +771,7 @@ void ThreadProxy::ScheduledActionSendBeginMainFrame() {
scoped_ptr<BeginMainFrameAndCommitState> begin_main_frame_state(
new BeginMainFrameAndCommitState);
begin_main_frame_state->monotonic_frame_begin_time =
impl().layer_tree_host_impl->CurrentPhysicalTimeTicks();
impl().layer_tree_host_impl->CurrentFrameTimeTicks();
begin_main_frame_state->scroll_info =
impl().layer_tree_host_impl->ProcessScrollDeltas();

Expand Down Expand Up @@ -1629,7 +1629,7 @@ void ThreadProxy::RenewTreePriority() {
impl().layer_tree_host_impl->IsCurrentlyScrolling() ||
impl().layer_tree_host_impl->page_scale_animation_active();

base::TimeTicks now = impl().layer_tree_host_impl->CurrentPhysicalTimeTicks();
base::TimeTicks now = impl().layer_tree_host_impl->CurrentFrameTimeTicks();

// Update expiration time if smoothness currently takes priority.
if (smoothness_takes_priority) {
Expand Down

0 comments on commit 1dc0616

Please sign in to comment.