Skip to content

Commit

Permalink
Snap after animated wheel scroll.
Browse files Browse the repository at this point in the history
This patch supports snapping after animated wheel scroll.

More generally, we should snap after a scroll animation if the
animation wasn't for snapping itself.

Bug: 826416
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ic1dba95157bd4d7d36f25b92646c86561b2ecbe9
Reviewed-on: https://chromium-review.googlesource.com/1050398
Commit-Queue: Sandra Sun <[email protected]>
Reviewed-by: Majid Valipour <[email protected]>
Reviewed-by: David Bokan <[email protected]>
Cr-Commit-Position: refs/heads/master@{#567066}
  • Loading branch information
sunyunjia authored and Commit Bot committed Jun 14, 2018
1 parent e4b3934 commit 9debaa0
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 34 deletions.
13 changes: 11 additions & 2 deletions cc/trees/layer_tree_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,8 @@ LayerTreeHostImpl::LayerTreeHostImpl(
base::Unretained(this)),
settings_.enable_image_animation_resync),
frame_metrics_(LTHI_FrameMetricsSettings(settings_)),
skipped_frame_tracker_(&frame_metrics_) {
skipped_frame_tracker_(&frame_metrics_),
is_animating_for_snap_(false) {
DCHECK(mutator_host_);
mutator_host_->SetMutatorHostClient(this);

Expand Down Expand Up @@ -3392,6 +3393,7 @@ InputHandler::ScrollStatus LayerTreeHostImpl::ScrollBeginImpl(
}
scroll_status.thread = SCROLL_ON_IMPL_THREAD;
mutator_host_->ScrollAnimationAbort();
is_animating_for_snap_ = false;

browser_controls_offset_manager_->ScrollBegin();

Expand Down Expand Up @@ -3722,6 +3724,8 @@ InputHandler::ScrollStatus LayerTreeHostImpl::ScrollAnimated(
viewport()->ScrollAnimated(pending_delta, delayed_by);
// Viewport::ScrollAnimated returns pending_delta as long as it starts
// an animation.
did_scroll_x_for_scroll_gesture_ |= scrolled.x() != 0;
did_scroll_y_for_scroll_gesture_ |= scrolled.y() != 0;
if (scrolled == pending_delta) {
scroll_animating_latched_element_id_ = scroll_node->element_id;
return scroll_status;
Expand All @@ -3732,6 +3736,8 @@ InputHandler::ScrollStatus LayerTreeHostImpl::ScrollAnimated(
gfx::Vector2dF scroll_delta =
ComputeScrollDelta(*scroll_node, pending_delta);
if (ScrollAnimationCreate(scroll_node, scroll_delta, delayed_by)) {
did_scroll_x_for_scroll_gesture_ |= scroll_delta.x() != 0;
did_scroll_y_for_scroll_gesture_ |= scroll_delta.y() != 0;
scroll_animating_latched_element_id_ = scroll_node->element_id;
return scroll_status;
}
Expand Down Expand Up @@ -4264,6 +4270,8 @@ bool LayerTreeHostImpl::SnapAtScrollEnd() {
} else {
ScrollAnimationCreate(scroll_node, delta, base::TimeDelta());
}
DCHECK(!is_animating_for_snap_);
is_animating_for_snap_ = true;
return true;
}

Expand Down Expand Up @@ -4323,6 +4331,7 @@ void LayerTreeHostImpl::ClearCurrentlyScrollingNode() {
accumulated_root_overscroll_ = gfx::Vector2dF();
did_scroll_x_for_scroll_gesture_ = false;
did_scroll_y_for_scroll_gesture_ = false;
is_animating_for_snap_ = false;
}

void LayerTreeHostImpl::ScrollEndImpl(ScrollState* scroll_state) {
Expand Down Expand Up @@ -5307,7 +5316,7 @@ void LayerTreeHostImpl::ScrollOffsetAnimationFinished() {
// TODO(majidvp): We should pass in the original starting scroll position here
ScrollStateData scroll_state_data;
ScrollState scroll_state(scroll_state_data);
ScrollEndImpl(&scroll_state);
ScrollEnd(&scroll_state, !is_animating_for_snap_);
}

gfx::ScrollOffset LayerTreeHostImpl::GetScrollOffsetForAnimation(
Expand Down
5 changes: 5 additions & 0 deletions cc/trees/layer_tree_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,10 @@ class CC_EXPORT LayerTreeHostImpl
virtual void SetVisible(bool visible);
bool visible() const { return visible_; }

bool is_animating_for_snap_for_testing() const {
return is_animating_for_snap_;
}

void SetNeedsCommit() { client_->SetNeedsCommitOnImplThread(); }
void SetNeedsOneBeginImplFrame();
void SetNeedsRedraw();
Expand Down Expand Up @@ -1103,6 +1107,7 @@ class CC_EXPORT LayerTreeHostImpl
base::circular_deque<FrameTokenInfo> frame_token_infos_;
ui::FrameMetrics frame_metrics_;
ui::SkippedFrameTracker skipped_frame_tracker_;
bool is_animating_for_snap_;

DISALLOW_COPY_AND_ASSIGN(LayerTreeHostImpl);
};
Expand Down
150 changes: 118 additions & 32 deletions cc/trees/layer_tree_host_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1542,19 +1542,21 @@ TEST_F(LayerTreeHostImplTest, ScrollByReturnsCorrectValue) {
.did_scroll);
}

// TODO(sunyunjia): Move scroll snap tests to a separate file.
// https://crbug.com/851690
TEST_F(LayerTreeHostImplTest, ScrollSnapOnX) {
LayerImpl* overflow = CreateLayerForSnapping();

gfx::Point scroll_position(10, 10);
gfx::Point pointer_position(10, 10);
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

gfx::Vector2dF x_delta(20, 0);
host_impl_->ScrollBy(UpdateState(scroll_position, x_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, x_delta).get());

viz::BeginFrameArgs begin_frame_args =
viz::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
Expand All @@ -1573,16 +1575,16 @@ TEST_F(LayerTreeHostImplTest, ScrollSnapOnX) {
TEST_F(LayerTreeHostImplTest, ScrollSnapOnY) {
LayerImpl* overflow = CreateLayerForSnapping();

gfx::Point scroll_position(10, 10);
gfx::Point pointer_position(10, 10);
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

gfx::Vector2dF y_delta(0, 20);
host_impl_->ScrollBy(UpdateState(scroll_position, y_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, y_delta).get());

viz::BeginFrameArgs begin_frame_args =
viz::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
Expand All @@ -1601,16 +1603,16 @@ TEST_F(LayerTreeHostImplTest, ScrollSnapOnY) {
TEST_F(LayerTreeHostImplTest, ScrollSnapOnBoth) {
LayerImpl* overflow = CreateLayerForSnapping();

gfx::Point scroll_position(10, 10);
gfx::Point pointer_position(10, 10);
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

gfx::Vector2dF delta(20, 20);
host_impl_->ScrollBy(UpdateState(scroll_position, delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, delta).get());

viz::BeginFrameArgs begin_frame_args =
viz::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
Expand All @@ -1626,24 +1628,108 @@ TEST_F(LayerTreeHostImplTest, ScrollSnapOnBoth) {
EXPECT_VECTOR_EQ(gfx::Vector2dF(50, 50), overflow->CurrentScrollOffset());
}

TEST_F(LayerTreeHostImplTest, ScrollSnapAfterAnimatedScroll) {
LayerImpl* overflow = CreateLayerForSnapping();

gfx::Point pointer_position(10, 10);
gfx::Vector2dF delta(20, 20);

EXPECT_EQ(InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_->ScrollAnimated(pointer_position, delta).thread);

EXPECT_EQ(overflow->scroll_tree_index(),
host_impl_->CurrentlyScrollingNode()->id);

base::TimeTicks start_time =
base::TimeTicks() + base::TimeDelta::FromMilliseconds(100);
viz::BeginFrameArgs begin_frame_args =
viz::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
BeginImplFrameAndAnimate(begin_frame_args, start_time);

// Animating for the wheel scroll.
BeginImplFrameAndAnimate(begin_frame_args,
start_time + base::TimeDelta::FromMilliseconds(50));
EXPECT_FALSE(host_impl_->is_animating_for_snap_for_testing());
gfx::ScrollOffset current_offset = overflow->CurrentScrollOffset();
EXPECT_LT(0, current_offset.x());
EXPECT_GT(20, current_offset.x());
EXPECT_LT(0, current_offset.y());
EXPECT_GT(20, current_offset.y());

// Animating for the snap.
BeginImplFrameAndAnimate(
begin_frame_args, start_time + base::TimeDelta::FromMilliseconds(1000));
EXPECT_TRUE(host_impl_->is_animating_for_snap_for_testing());

// Finish the animation.
BeginImplFrameAndAnimate(
begin_frame_args, start_time + base::TimeDelta::FromMilliseconds(1500));
EXPECT_VECTOR_EQ(gfx::Vector2dF(50, 50), overflow->CurrentScrollOffset());
EXPECT_FALSE(host_impl_->is_animating_for_snap_for_testing());
}

TEST_F(LayerTreeHostImplTest, SnapAnimationCancelledByScroll) {
LayerImpl* overflow = CreateLayerForSnapping();

gfx::Point pointer_position(10, 10);
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

gfx::Vector2dF x_delta(20, 0);
host_impl_->ScrollBy(UpdateState(pointer_position, x_delta).get());
EXPECT_FALSE(host_impl_->is_animating_for_snap_for_testing());

viz::BeginFrameArgs begin_frame_args =
viz::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
host_impl_->ScrollEnd(EndState().get(), true);
base::TimeTicks start_time =
base::TimeTicks() + base::TimeDelta::FromMilliseconds(100);
BeginImplFrameAndAnimate(begin_frame_args, start_time);

// Animating for the snap.
BeginImplFrameAndAnimate(begin_frame_args,
start_time + base::TimeDelta::FromMilliseconds(100));
EXPECT_TRUE(host_impl_->is_animating_for_snap_for_testing());
gfx::ScrollOffset current_offset = overflow->CurrentScrollOffset();
EXPECT_GT(50, current_offset.x());
EXPECT_LT(20, current_offset.x());
EXPECT_EQ(0, current_offset.y());

// Interrup the snap animation with ScrollBegin.
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_FALSE(host_impl_->is_animating_for_snap_for_testing());
BeginImplFrameAndAnimate(begin_frame_args,
start_time + base::TimeDelta::FromMilliseconds(150));
EXPECT_VECTOR_EQ(ScrollOffsetToVector2dF(current_offset),
overflow->CurrentScrollOffset());
}

TEST_F(LayerTreeHostImplTest, GetSnapFlingInfoWhenZoomed) {
LayerImpl* overflow = CreateLayerForSnapping();
// Scales the page to its 1/5.
host_impl_->active_tree()->PushPageScaleFromMainThread(0.2f, 0.1f, 5.f);

// Should be (10, 10) in the scroller's coordinate.
gfx::Point scroll_position(2, 2);
gfx::Point pointer_position(2, 2);
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

// Should be (20, 20) in the scroller's coordinate.
gfx::Vector2dF delta(4, 4);
InputHandlerScrollResult result =
host_impl_->ScrollBy(UpdateState(scroll_position, delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, delta).get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 20), overflow->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(4, 4), result.current_visual_offset);

Expand Down Expand Up @@ -1673,21 +1759,21 @@ TEST_F(LayerTreeHostImplTest, OverscrollBehaviorPreventsPropagation) {
scroll_layer->SetCurrentScrollOffset(gfx::ScrollOffset(30, 30));

DrawFrame();
gfx::Point scroll_position(50, 50);
gfx::Point pointer_position(50, 50);

// OverscrollBehaviorTypeAuto shouldn't prevent scroll propagation.
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(30, 30), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(), overflow->CurrentScrollOffset());

gfx::Vector2dF x_delta(-10, 0);
gfx::Vector2dF y_delta(0, -10);
gfx::Vector2dF diagonal_delta(-10, -10);
host_impl_->ScrollBy(UpdateState(scroll_position, x_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, x_delta).get());
host_impl_->ScrollEnd(EndState().get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 30), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());
Expand All @@ -1704,12 +1790,12 @@ TEST_F(LayerTreeHostImplTest, OverscrollBehaviorPreventsPropagation) {
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 30), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

host_impl_->ScrollBy(UpdateState(scroll_position, x_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, x_delta).get());
host_impl_->ScrollEnd(EndState().get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 30), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());
Expand All @@ -1719,12 +1805,12 @@ TEST_F(LayerTreeHostImplTest, OverscrollBehaviorPreventsPropagation) {
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 30), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

host_impl_->ScrollBy(UpdateState(scroll_position, y_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, y_delta).get());
host_impl_->ScrollEnd(EndState().get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());
Expand All @@ -1734,12 +1820,12 @@ TEST_F(LayerTreeHostImplTest, OverscrollBehaviorPreventsPropagation) {
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

host_impl_->ScrollBy(UpdateState(scroll_position, diagonal_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, diagonal_delta).get());
host_impl_->ScrollEnd(EndState().get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());
Expand All @@ -1757,12 +1843,12 @@ TEST_F(LayerTreeHostImplTest, OverscrollBehaviorPreventsPropagation) {
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(20, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

host_impl_->ScrollBy(UpdateState(scroll_position, x_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, x_delta).get());
host_impl_->ScrollEnd(EndState().get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(10, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());
Expand All @@ -1772,12 +1858,12 @@ TEST_F(LayerTreeHostImplTest, OverscrollBehaviorPreventsPropagation) {
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(10, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

host_impl_->ScrollBy(UpdateState(scroll_position, y_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, y_delta).get());
host_impl_->ScrollEnd(EndState().get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(10, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());
Expand All @@ -1787,12 +1873,12 @@ TEST_F(LayerTreeHostImplTest, OverscrollBehaviorPreventsPropagation) {
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(10, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

host_impl_->ScrollBy(UpdateState(scroll_position, diagonal_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, diagonal_delta).get());
host_impl_->ScrollEnd(EndState().get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(10, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());
Expand All @@ -1809,15 +1895,15 @@ TEST_F(LayerTreeHostImplTest, OverscrollBehaviorPreventsPropagation) {
EXPECT_EQ(
InputHandler::SCROLL_ON_IMPL_THREAD,
host_impl_
->ScrollBegin(BeginState(scroll_position).get(), InputHandler::WHEEL)
->ScrollBegin(BeginState(pointer_position).get(), InputHandler::WHEEL)
.thread);
EXPECT_VECTOR_EQ(gfx::Vector2dF(10, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(0, 0), overflow->CurrentScrollOffset());

host_impl_->ScrollBy(UpdateState(scroll_position, x_delta).get());
host_impl_->ScrollBy(UpdateState(scroll_position, -x_delta).get());
host_impl_->ScrollBy(UpdateState(scroll_position, y_delta).get());
host_impl_->ScrollBy(UpdateState(scroll_position, -y_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, x_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, -x_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, y_delta).get());
host_impl_->ScrollBy(UpdateState(pointer_position, -y_delta).get());
host_impl_->ScrollEnd(EndState().get());
EXPECT_VECTOR_EQ(gfx::Vector2dF(10, 20), scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(gfx::Vector2dF(10, 10), overflow->CurrentScrollOffset());
Expand Down

0 comments on commit 9debaa0

Please sign in to comment.