Skip to content

Commit

Permalink
Don't skip subtree with input handler while calculating visible rects
Browse files Browse the repository at this point in the history
Property trees skip a subtree if its not drawn. This may break the hit
testing code if the subtree has input handlers. Though we
don't use visible rects during hit testing, we still need
render surface's drawable content rect which depends on
layer's drawable content rect which inturn depends on the
clip rect that is computed while computing the visible
rect.

BUG=537639
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#351876}
  • Loading branch information
jaydasika authored and Commit bot committed Oct 1, 2015
1 parent 703c8cd commit 60f8586
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 13 deletions.
1 change: 0 additions & 1 deletion cc/layers/draw_properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ DrawProperties::DrawProperties()
is_clipped(false),
render_target(nullptr),
num_unclipped_descendants(0),
layer_or_descendant_has_input_handler(false),
has_child_with_a_scroll_parent(false),
last_drawn_render_surface_layer_list_id(0),
maximum_animation_contents_scale(0.f),
Expand Down
3 changes: 0 additions & 3 deletions cc/layers/draw_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ struct CC_EXPORT DrawProperties {
// does not include our clip children because they are clipped by us.
size_t num_unclipped_descendants;

// If true, the layer or one of its descendants has a wheel or touch handler.
bool layer_or_descendant_has_input_handler;

// This is true if the layer has any direct child that has a scroll parent.
// This layer will not be the scroll parent in this case. This information
// lets us avoid work in CalculateDrawPropertiesInternal -- if none of our
Expand Down
1 change: 1 addition & 0 deletions cc/layers/layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ LayerImpl::LayerImpl(LayerTreeImpl* tree_impl,
frame_timing_requests_dirty_(false),
visited_(false),
layer_or_descendant_is_drawn_(false),
layer_or_descendant_has_input_handler_(false),
sorted_for_recursion_(false) {
DCHECK_GT(layer_id_, 0);
DCHECK(layer_tree_impl_);
Expand Down
12 changes: 12 additions & 0 deletions cc/layers/layer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,16 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver,

bool layer_or_descendant_is_drawn() { return layer_or_descendant_is_drawn_; }

void set_layer_or_descendant_has_input_handler(
bool layer_or_descendant_has_input_handler) {
layer_or_descendant_has_input_handler_ =
layer_or_descendant_has_input_handler;
}

bool layer_or_descendant_has_input_handler() {
return layer_or_descendant_has_input_handler_;
}

void set_sorted_for_recursion(bool sorted_for_recursion) {
sorted_for_recursion_ = sorted_for_recursion;
}
Expand Down Expand Up @@ -872,6 +882,8 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver,
bool frame_timing_requests_dirty_;
bool visited_;
bool layer_or_descendant_is_drawn_;
// If true, the layer or one of its descendants has a wheel or touch handler.
bool layer_or_descendant_has_input_handler_;
bool sorted_for_recursion_;

DISALLOW_COPY_AND_ASSIGN(LayerImpl);
Expand Down
9 changes: 9 additions & 0 deletions cc/trees/draw_property_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,15 @@ static inline bool SubtreeShouldBeSkipped(LayerImpl* layer,
if (layer->num_layer_or_descendants_with_copy_request() > 0)
return false;

// We cannot skip the the subtree if a descendant has a wheel or touch handler
// or the hit testing code will break (it requires fresh transforms, etc).
// Though we don't need visible rect for hit testing, we need render surface's
// drawable content rect which depends on layer's drawable content rect which
// in turn depends on layer's clip rect that is computed while computing
// visible rects.
if (layer->layer_or_descendant_has_input_handler())
return false;

// If the layer is not drawn, then skip it and its subtree.
if (!layer_is_drawn)
return true;
Expand Down
6 changes: 3 additions & 3 deletions cc/trees/layer_tree_host_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ static inline bool SubtreeShouldBeSkipped(LayerImpl* layer,

// We cannot skip the the subtree if a descendant has a wheel or touch handler
// or the hit testing code will break (it requires fresh transforms, etc).
if (layer->draw_properties().layer_or_descendant_has_input_handler)
if (layer->layer_or_descendant_has_input_handler())
return false;

// If the layer is not drawn, then skip it and its subtree.
Expand Down Expand Up @@ -1266,8 +1266,8 @@ static void PreCalculateMetaInformationInternal(

layer->draw_properties().num_unclipped_descendants =
recursive_data->num_unclipped_descendants;
layer->draw_properties().layer_or_descendant_has_input_handler =
(recursive_data->num_layer_or_descendants_with_input_handler != 0);
layer->set_layer_or_descendant_has_input_handler(
(recursive_data->num_layer_or_descendants_with_input_handler != 0));
// TODO(enne): this should be synced from the main thread, so is only
// for tests constructing layers on the compositor thread.
layer->set_num_layer_or_descendant_with_copy_request(
Expand Down
36 changes: 30 additions & 6 deletions cc/trees/layer_tree_host_common_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7591,18 +7591,15 @@ TEST_F(LayerTreeHostCommonTest, InputHandlersRecursiveUpdateTest) {
SetLayerPropertiesForTesting(child, identity, gfx::Point3F(), gfx::PointF(),
gfx::Size(100, 100), true, false, false);

EXPECT_EQ(root->draw_properties().layer_or_descendant_has_input_handler,
false);
EXPECT_EQ(root->layer_or_descendant_has_input_handler(), false);

child->SetHaveWheelEventHandlers(true);
ExecuteCalculateDrawProperties(root);
EXPECT_EQ(root->draw_properties().layer_or_descendant_has_input_handler,
true);
EXPECT_EQ(root->layer_or_descendant_has_input_handler(), true);

child->SetHaveWheelEventHandlers(false);
ExecuteCalculateDrawProperties(root);
EXPECT_EQ(root->draw_properties().layer_or_descendant_has_input_handler,
false);
EXPECT_EQ(root->layer_or_descendant_has_input_handler(), false);
}

TEST_F(LayerTreeHostCommonTest, ResetPropertyTreeIndices) {
Expand Down Expand Up @@ -7881,6 +7878,33 @@ TEST_F(LayerTreeHostCommonTest, UnclippedClipParent) {
EXPECT_FALSE(clip_child->is_clipped());
}

TEST_F(LayerTreeHostCommonTest, LayerWithInputHandlerAndZeroOpacity) {
LayerImpl* root = root_layer();
LayerImpl* render_surface = AddChild<LayerImpl>(root);
LayerImpl* test_layer = AddChild<LayerImpl>(render_surface);

const gfx::Transform identity_matrix;
SetLayerPropertiesForTesting(root, identity_matrix, gfx::Point3F(),
gfx::PointF(), gfx::Size(30, 30), true, false,
true);
SetLayerPropertiesForTesting(render_surface, identity_matrix, gfx::Point3F(),
gfx::PointF(), gfx::Size(30, 30), true, false,
true);
SetLayerPropertiesForTesting(test_layer, identity_matrix, gfx::Point3F(),
gfx::PointF(), gfx::Size(20, 20), true, false,
false);

render_surface->SetMasksToBounds(true);
test_layer->SetDrawsContent(true);
test_layer->SetOpacity(0);
test_layer->SetHaveWheelEventHandlers(true);

ExecuteCalculateDrawProperties(root);
EXPECT_EQ(gfx::Rect(20, 20), test_layer->drawable_content_rect());
EXPECT_EQ(gfx::RectF(20, 20),
render_surface->render_surface()->DrawableContentRect());
}

TEST_F(LayerTreeHostCommonTest,
LayerClipRectLargerThanClippingRenderSurfaceRect) {
LayerImpl* root = root_layer();
Expand Down

0 comments on commit 60f8586

Please sign in to comment.