Commit 578e1246 authored by Rahul Arakeri's avatar Rahul Arakeri Committed by Commit Bot

Fix for incorrect sub scroller hit testing.

The parts of a composited scrollbar (like the thumb, buttons etc) were
expressed as being relative to the origin of the owner scroll layer.
This will cause hit testing for the div scrollbars to fail (except
when the div is absolutely positioned at 0,0). To fix this, we need to
tranform the following coordinates and make them relative to the
scrollbar layer's origin:
a) The pointer coordinates before hit testing (in ScrollbarController)
b) The back,forward,track rect getters in ScrollbarLayerDelegate

Bug: 952314
Change-Id: Ib42ed4df80e30a5c0dc983bc003fb21fb6a42df2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1585017
Commit-Queue: Rahul Arakeri <arakeri@microsoft.com>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#656400}
parent 6287f761
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <algorithm> #include <algorithm>
#include "cc/base/math_util.h"
#include "cc/input/scrollbar.h" #include "cc/input/scrollbar.h"
#include "cc/input/scrollbar_controller.h" #include "cc/input/scrollbar_controller.h"
#include "cc/trees/layer_tree_impl.h" #include "cc/trees/layer_tree_impl.h"
...@@ -65,8 +66,23 @@ gfx::ScrollOffset ScrollbarController::GetScrollStateBasedOnHitTest( ...@@ -65,8 +66,23 @@ gfx::ScrollOffset ScrollbarController::GetScrollStateBasedOnHitTest(
static_cast<const ScrollbarLayerImplBase*>(scrollbar_layer_impl); static_cast<const ScrollbarLayerImplBase*>(scrollbar_layer_impl);
const ScrollbarOrientation orientation = scrollbar_layer->orientation(); const ScrollbarOrientation orientation = scrollbar_layer->orientation();
ScrollbarPart scrollbar_part = // position_in_widget needs to be transformed and made relative to the
scrollbar_layer->IdentifyScrollbarPart(position_in_widget); // scrollbar layer because hit testing assumes layer relative coordinates.
ScrollbarPart scrollbar_part = ScrollbarPart::NO_PART;
gfx::Transform inverse_screen_space_transform(
gfx::Transform::kSkipInitialization);
if (scrollbar_layer_impl->ScreenSpaceTransform().GetInverse(
&inverse_screen_space_transform)) {
bool clipped;
gfx::PointF scroller_relative_position(MathUtil::ProjectPoint(
inverse_screen_space_transform, position_in_widget, &clipped));
if (clipped)
return gfx::ScrollOffset(0, 0);
scrollbar_part =
scrollbar_layer->IdentifyScrollbarPart(scroller_relative_position);
}
float scroll_delta = float scroll_delta =
layer_tree_host_impl_->active_tree()->device_scale_factor() * layer_tree_host_impl_->active_tree()->device_scale_factor() *
......
...@@ -73,10 +73,10 @@ void PaintedOverlayScrollbarLayer::PushPropertiesTo(LayerImpl* layer) { ...@@ -73,10 +73,10 @@ void PaintedOverlayScrollbarLayer::PushPropertiesTo(LayerImpl* layer) {
scrollbar_layer->SetThumbThickness(thumb_thickness_); scrollbar_layer->SetThumbThickness(thumb_thickness_);
scrollbar_layer->SetThumbLength(thumb_length_); scrollbar_layer->SetThumbLength(thumb_length_);
if (scrollbar_->Orientation() == HORIZONTAL) { if (scrollbar_->Orientation() == HORIZONTAL) {
scrollbar_layer->SetTrackStart(track_rect_.x() - location_.x()); scrollbar_layer->SetTrackStart(track_rect_.x());
scrollbar_layer->SetTrackLength(track_rect_.width()); scrollbar_layer->SetTrackLength(track_rect_.width());
} else { } else {
scrollbar_layer->SetTrackStart(track_rect_.y() - location_.y()); scrollbar_layer->SetTrackStart(track_rect_.y());
scrollbar_layer->SetTrackLength(track_rect_.height()); scrollbar_layer->SetTrackLength(track_rect_.height());
} }
......
...@@ -74,12 +74,10 @@ void PaintedScrollbarLayer::PushPropertiesTo(LayerImpl* layer) { ...@@ -74,12 +74,10 @@ void PaintedScrollbarLayer::PushPropertiesTo(LayerImpl* layer) {
scrollbar_layer->SetForwardButtonRect(forward_button_rect_); scrollbar_layer->SetForwardButtonRect(forward_button_rect_);
scrollbar_layer->SetThumbLength(thumb_length_); scrollbar_layer->SetThumbLength(thumb_length_);
if (scrollbar_->Orientation() == HORIZONTAL) { if (scrollbar_->Orientation() == HORIZONTAL) {
scrollbar_layer->SetTrackStart( scrollbar_layer->SetTrackStart(track_rect_.x());
track_rect_.x() - location_.x());
scrollbar_layer->SetTrackLength(track_rect_.width()); scrollbar_layer->SetTrackLength(track_rect_.width());
} else { } else {
scrollbar_layer->SetTrackStart( scrollbar_layer->SetTrackStart(track_rect_.y());
track_rect_.y() - location_.y());
scrollbar_layer->SetTrackLength(track_rect_.height()); scrollbar_layer->SetTrackLength(track_rect_.height());
} }
......
...@@ -414,10 +414,13 @@ TEST_F(ScrollbarLayerTest, UpdatePropertiesOfScrollBarWhenThumbRemoved) { ...@@ -414,10 +414,13 @@ TEST_F(ScrollbarLayerTest, UpdatePropertiesOfScrollBarWhenThumbRemoved) {
root_layer->SetScrollOffset(gfx::ScrollOffset(0, 0)); root_layer->SetScrollOffset(gfx::ScrollOffset(0, 0));
scrollbar_layer->SetBounds(gfx::Size(70, 10)); scrollbar_layer->SetBounds(gfx::Size(70, 10));
scrollbar_layer->SetScrollElementId(root_layer->element_id()); scrollbar_layer->SetScrollElementId(root_layer->element_id());
// The track_rect should be relative to the scrollbar's origin.
scrollbar_layer->fake_scrollbar()->set_location(gfx::Point(20, 10)); scrollbar_layer->fake_scrollbar()->set_location(gfx::Point(20, 10));
scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(30, 10, 50, 10)); scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(10, 10, 50, 10));
scrollbar_layer->fake_scrollbar()->set_thumb_thickness(10); scrollbar_layer->fake_scrollbar()->set_thumb_thickness(10);
scrollbar_layer->fake_scrollbar()->set_thumb_length(4); scrollbar_layer->fake_scrollbar()->set_thumb_length(4);
LayerImpl* root_layer_impl = nullptr; LayerImpl* root_layer_impl = nullptr;
PaintedScrollbarLayerImpl* scrollbar_layer_impl = nullptr; PaintedScrollbarLayerImpl* scrollbar_layer_impl = nullptr;
...@@ -452,10 +455,13 @@ TEST_F(ScrollbarLayerTest, ThumbRect) { ...@@ -452,10 +455,13 @@ TEST_F(ScrollbarLayerTest, ThumbRect) {
root_layer->SetScrollOffset(gfx::ScrollOffset(0, 0)); root_layer->SetScrollOffset(gfx::ScrollOffset(0, 0));
scrollbar_layer->SetBounds(gfx::Size(70, 10)); scrollbar_layer->SetBounds(gfx::Size(70, 10));
scrollbar_layer->SetScrollElementId(root_layer->element_id()); scrollbar_layer->SetScrollElementId(root_layer->element_id());
// The track_rect should be relative to the scrollbar's origin.
scrollbar_layer->fake_scrollbar()->set_location(gfx::Point(20, 10)); scrollbar_layer->fake_scrollbar()->set_location(gfx::Point(20, 10));
scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(30, 10, 50, 10)); scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(10, 10, 50, 10));
scrollbar_layer->fake_scrollbar()->set_thumb_thickness(10); scrollbar_layer->fake_scrollbar()->set_thumb_thickness(10);
scrollbar_layer->fake_scrollbar()->set_thumb_length(4); scrollbar_layer->fake_scrollbar()->set_thumb_length(4);
layer_tree_host_->UpdateLayers(); layer_tree_host_->UpdateLayers();
LayerImpl* root_layer_impl = nullptr; LayerImpl* root_layer_impl = nullptr;
PaintedScrollbarLayerImpl* scrollbar_layer_impl = nullptr; PaintedScrollbarLayerImpl* scrollbar_layer_impl = nullptr;
...@@ -493,7 +499,7 @@ TEST_F(ScrollbarLayerTest, ThumbRect) { ...@@ -493,7 +499,7 @@ TEST_F(ScrollbarLayerTest, ThumbRect) {
// Shrink the scrollbar layer to cover only the track. // Shrink the scrollbar layer to cover only the track.
scrollbar_layer->SetBounds(gfx::Size(50, 10)); scrollbar_layer->SetBounds(gfx::Size(50, 10));
scrollbar_layer->fake_scrollbar()->set_location(gfx::Point(30, 10)); scrollbar_layer->fake_scrollbar()->set_location(gfx::Point(30, 10));
scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(30, 10, 50, 10)); scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(0, 10, 50, 10));
UPDATE_AND_EXTRACT_LAYER_POINTERS(); UPDATE_AND_EXTRACT_LAYER_POINTERS();
EXPECT_EQ(gfx::Rect(44, 0, 6, 4).ToString(), EXPECT_EQ(gfx::Rect(44, 0, 6, 4).ToString(),
...@@ -502,7 +508,7 @@ TEST_F(ScrollbarLayerTest, ThumbRect) { ...@@ -502,7 +508,7 @@ TEST_F(ScrollbarLayerTest, ThumbRect) {
// Shrink the track in the non-scrolling dimension so that it only covers the // Shrink the track in the non-scrolling dimension so that it only covers the
// middle third of the scrollbar layer (this does not affect the thumb // middle third of the scrollbar layer (this does not affect the thumb
// position). // position).
scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(30, 12, 50, 6)); scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(0, 12, 50, 6));
UPDATE_AND_EXTRACT_LAYER_POINTERS(); UPDATE_AND_EXTRACT_LAYER_POINTERS();
EXPECT_EQ(gfx::Rect(44, 0, 6, 4).ToString(), EXPECT_EQ(gfx::Rect(44, 0, 6, 4).ToString(),
......
...@@ -81,15 +81,23 @@ int ScrollbarLayerDelegate::ThumbLength() const { ...@@ -81,15 +81,23 @@ int ScrollbarLayerDelegate::ThumbLength() const {
} }
gfx::Rect ScrollbarLayerDelegate::TrackRect() const { gfx::Rect ScrollbarLayerDelegate::TrackRect() const {
return theme_.TrackRect(*scrollbar_); IntRect track_rect = theme_.TrackRect(*scrollbar_);
track_rect.MoveBy(-scrollbar_->Location());
return track_rect;
} }
gfx::Rect ScrollbarLayerDelegate::BackButtonRect() const { gfx::Rect ScrollbarLayerDelegate::BackButtonRect() const {
return theme_.BackButtonRect(*scrollbar_, blink::kBackButtonStartPart); IntRect back_button_rect =
theme_.BackButtonRect(*scrollbar_, blink::kBackButtonStartPart);
back_button_rect.MoveBy(-scrollbar_->Location());
return back_button_rect;
} }
gfx::Rect ScrollbarLayerDelegate::ForwardButtonRect() const { gfx::Rect ScrollbarLayerDelegate::ForwardButtonRect() const {
return theme_.ForwardButtonRect(*scrollbar_, blink::kForwardButtonEndPart); IntRect forward_button_rect =
theme_.ForwardButtonRect(*scrollbar_, blink::kForwardButtonEndPart);
forward_button_rect.MoveBy(-scrollbar_->Location());
return forward_button_rect;
} }
float ScrollbarLayerDelegate::ThumbOpacity() const { float ScrollbarLayerDelegate::ThumbOpacity() const {
......
...@@ -38,9 +38,16 @@ class CORE_EXPORT ScrollbarLayerDelegate : public cc::Scrollbar { ...@@ -38,9 +38,16 @@ class CORE_EXPORT ScrollbarLayerDelegate : public cc::Scrollbar {
gfx::Point Location() const override; gfx::Point Location() const override;
int ThumbThickness() const override; int ThumbThickness() const override;
int ThumbLength() const override; int ThumbLength() const override;
// Returns the track rect relative to the scrollbar's origin.
gfx::Rect TrackRect() const override; gfx::Rect TrackRect() const override;
// Returns the back button rect relative to the scrollbar's origin.
gfx::Rect BackButtonRect() const override; gfx::Rect BackButtonRect() const override;
// Returns the forward button rect relative to the scrollbar's origin.
gfx::Rect ForwardButtonRect() const override; gfx::Rect ForwardButtonRect() const override;
float ThumbOpacity() const override; float ThumbOpacity() const override;
bool NeedsPaintPart(cc::ScrollbarPart part) const override; bool NeedsPaintPart(cc::ScrollbarPart part) const override;
bool HasTickmarks() const override; bool HasTickmarks() const override;
......
...@@ -10,8 +10,8 @@ ...@@ -10,8 +10,8 @@
overflow: scroll; overflow: scroll;
border: 1px solid black; border: 1px solid black;
position: absolute; position: absolute;
top: 0px; top: 100px;
left: 0px; left: 100px;
} }
.fast { .fast {
will-change: transform; will-change: transform;
......
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