Commit 74efa339 authored by bokan's avatar bokan Committed by Commit bot

Fix viewport scroll bubbling of wheel events in CC.

The "LocalDelta" version of ScrollLayer in LTHI takes a scroll delta in
viewport space and scrolls a layer in its local space. This means that
the delta is scaled so that the scroll appears to be the same distance
on the user's screen, regardless of the page scale factor (pinch-zoom).
This method returns the amount consumed so that the caller can determine
how much of the original scroll delta remains to bubble. However, the
returned consumed delta is in local space, meaning we were subtracting a
layer space delta from the viewport space delta.

The practical effect of this was that when the page was scaled up the
bubbling logic in the viewport would inappropriately bubble up scrolls to
the inner (visual) viewport since the local delta is divided by the page
scale factor. This caused position:fixed elements to "paralax" scroll
with the page, cool looking but incorrect.

BUG=488549

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

Cr-Commit-Position: refs/heads/master@{#330786}
parent 5a1526c3
...@@ -2587,7 +2587,10 @@ static gfx::Vector2dF ScrollLayerWithLocalDelta( ...@@ -2587,7 +2587,10 @@ static gfx::Vector2dF ScrollLayerWithLocalDelta(
layer_impl->ScrollBy(delta); layer_impl->ScrollBy(delta);
gfx::ScrollOffset scrolled = gfx::ScrollOffset scrolled =
layer_impl->CurrentScrollOffset() - previous_offset; layer_impl->CurrentScrollOffset() - previous_offset;
return gfx::Vector2dF(scrolled.x(), scrolled.y()); gfx::Vector2dF consumed_scroll(scrolled.x(), scrolled.y());
consumed_scroll.Scale(page_scale_factor);
return consumed_scroll;
} }
gfx::Vector2dF LayerTreeHostImpl::ScrollLayer(LayerImpl* layer_impl, gfx::Vector2dF LayerTreeHostImpl::ScrollLayer(LayerImpl* layer_impl,
......
...@@ -1131,6 +1131,75 @@ TEST_F(LayerTreeHostImplTest, ImplPinchZoom) { ...@@ -1131,6 +1131,75 @@ TEST_F(LayerTreeHostImplTest, ImplPinchZoom) {
} }
} }
TEST_F(LayerTreeHostImplTest, ImplPinchZoomWheelBubbleBetweenViewports) {
LayerImpl* inner_scroll_layer =
SetupScrollAndContentsLayers(gfx::Size(100, 100));
// Adjust the content layer to be larger than the outer viewport container so
// that we get scrolling in both viewports.
LayerImpl* content_layer =
host_impl_->OuterViewportScrollLayer()->children().back();
LayerImpl* outer_scroll_layer = host_impl_->OuterViewportScrollLayer();
LayerImpl* inner_clip_layer =
host_impl_->InnerViewportScrollLayer()->parent()->parent();
inner_clip_layer->SetBounds(gfx::Size(100, 100));
inner_clip_layer->SetContentBounds(gfx::Size(100, 100));
outer_scroll_layer->SetBounds(gfx::Size(200, 200));
outer_scroll_layer->SetContentBounds(gfx::Size(200, 200));
content_layer->SetBounds(gfx::Size(200, 200));
content_layer->SetContentBounds(gfx::Size(200, 200));
host_impl_->SetViewportSize(gfx::Size(100, 100));
DrawFrame();
// Zoom into the page by a 2X factor
float min_page_scale = 1.f, max_page_scale = 4.f;
float page_scale_factor = 2.f;
host_impl_->active_tree()->PushPageScaleFromMainThread(
page_scale_factor, min_page_scale, max_page_scale);
host_impl_->SetPageScaleOnActiveTree(page_scale_factor);
// Scroll by a small amount, there should be no bubbling up to the inner
// viewport.
host_impl_->ScrollBegin(gfx::Point(0, 0), InputHandler::WHEEL);
host_impl_->ScrollBy(gfx::Point(0, 0), gfx::Vector2dF(10.f, 20.f));
host_impl_->ScrollEnd();
EXPECT_VECTOR_EQ(
gfx::Vector2dF(5, 10),
outer_scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(
gfx::Vector2dF(),
inner_scroll_layer->CurrentScrollOffset());
// Scroll by the outer viewport's max scroll extent, there the remainder
// should bubble up to the inner viewport.
host_impl_->ScrollBegin(gfx::Point(0, 0), InputHandler::WHEEL);
host_impl_->ScrollBy(gfx::Point(0, 0), gfx::Vector2dF(200.f, 200.f));
host_impl_->ScrollEnd();
EXPECT_VECTOR_EQ(
gfx::Vector2dF(100, 100),
outer_scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(
gfx::Vector2dF(5, 10),
inner_scroll_layer->CurrentScrollOffset());
// Scroll by the inner viewport's max scroll extent, it should all go to the
// inner viewport.
host_impl_->ScrollBegin(gfx::Point(0, 0), InputHandler::WHEEL);
host_impl_->ScrollBy(gfx::Point(0, 0), gfx::Vector2dF(90.f, 80.f));
host_impl_->ScrollEnd();
EXPECT_VECTOR_EQ(
gfx::Vector2dF(100, 100),
outer_scroll_layer->CurrentScrollOffset());
EXPECT_VECTOR_EQ(
gfx::Vector2dF(50, 50),
inner_scroll_layer->CurrentScrollOffset());
}
TEST_F(LayerTreeHostImplTest, ScrollWithSwapPromises) { TEST_F(LayerTreeHostImplTest, ScrollWithSwapPromises) {
ui::LatencyInfo latency_info; ui::LatencyInfo latency_info;
latency_info.trace_id = 1234; latency_info.trace_id = 1234;
......
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