New issue
Advanced search Search tips

Issue 922698 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 836890



Sign in to add a comment

Two SitePerProcessHitTestDataGenerationBrowserTest tests fail with BlinkGenPropertyTrees

Project Member Reported by pdr@chromium.org, Jan 16 (6 days ago)

Issue description

The following tests fail with BlinkGenPropertyTrees enabled:
SitePerProcessHitTestDataGenerationBrowserTest.ClipPathOOPIF/2
SitePerProcessHitTestDataGenerationBrowserTest.ClippedRotatedOOPIF/2

Here's an example of the assert that is failing:
expected_region.ApproximatelyEqual(hit_test_data[2].rect, 1 + device_scale_factor)

Without BGPT, the hit test rect is 0,0 160x160, whereas with BGPT it is 0,0 200x200. This difference comes from LayerTreeHostImpl::BuildHitTestData.
 

Comment 1 by sunxd@chromium.org, Jan 17 (5 days ago)

Is surface layer's |is_clipped| bit false in BGPT settings?

Comment 2 by pdr@chromium.org, Jan 17 (5 days ago)

Cc: -sunxd@chromium.org
Owner: sunxd@chromium.org
Status: Assigned (was: Untriaged)
With BGPT enabled (change BlinkGenPropertyTree's status="experimental" to status="stable" in runtime_enabled_features.json5), I get the following surface layer impl:
{
   "Bounds": [ 80, 80 ],
   "ContentsOpaque": false,
   "DrawsContent": true,
   "HitTestableWithoutDrawsContent": false,
   "Is3dSorted": false,
   "LayerId": 15,
   "LayerType": "cc::PictureLayerImpl",
   "OffsetToTransformParent": [ 0, 0 ],
   "Opacity": 1,
   "Transform": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
   "clip_tree_index": 3,
   "effect_tree_index": 3,
   "is_surface_layer": false,
   "scroll_tree_index": 3,
   "transform_tree_index": 7
}
is_clipped is false and clip node 3 has a clip rect of [ 0, 0, 80, 80 ].

Without BGPT, I get the following surface layer impl:
{
   "Bounds": [ 100, 100 ],
   "ContentsOpaque": false,
   "DrawsContent": true,
   "HitTestableWithoutDrawsContent": false,
   "Is3dSorted": false,
   "LayerId": 12,
   "LayerType": "cc::SurfaceLayerImpl",
   "OffsetToTransformParent": [ 0, 0 ],
   "Opacity": 1,
   "ShouldGenerateSurfaceHitTestData": true,
   "Transform": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
   "clip_tree_index": 4,
   "effect_tree_index": 3,
   "has_pointer_events_none": false,
   "is_surface_layer": true,
   "scroll_tree_index": 3,
   "transform_tree_index": 6
}
is_clipped is true and clip node 4 has a rect of [ 0, 0, 100, 100 ].


I am not sure what is causing this difference in is_clipped. BlinkGenPropertyTrees fixes some clip-path bugs (e.g.,  https://crbug.com/792280  and  https://crbug.com/907175 ) and I think the clip tree difference may be intentional (for example, the clip path might be on the surface layer's clip node without BGPT, but it might be on the surface layer's clip node ancestor with BGPT, or something like that).

This may just be a testing failure where we were relying on a specific clip node being on the surface layer. Would you be able to look into these failures?

Comment 3 by sunxd@chromium.org, Jan 17 (5 days ago)

Cc: weiliangc@chromium.org
Hi Wei, can you confirm the meaning of is_clipped? If we have:

Layer 1
+--Layer 2 (clipped by layer 1)
+----Surface layer

Should surface layer's is_clipped be set to true?

Comment 4 by sunxd@chromium.org, Jan 17 (5 days ago)

I ran the test and got this:

Without BGPT:

[1:8:0117/153655.740951:ERROR:layer_tree_host_impl.cc(2663)] {
   "Bounds": [ 100, 100 ],
   "ContentsOpaque": false,
   "DrawsContent": true,
   "HitTestableWithoutDrawsContent": false,
   "Is3dSorted": false,
   "LayerId": 12,
   "LayerType": "cc::SurfaceLayerImpl",
   "OffsetToTransformParent": [ 0, 0 ],
   "Opacity": 1,
   "Transform": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
   "clip_tree_index": 4,
   "effect_tree_index": 3,
   "scroll_tree_index": 3,
   "transform_tree_index": 6
}

With BGPT:

[1:8:0117/152642.833364:ERROR:layer_tree_host_impl.cc(2668)] {
   "Bounds": [ 100, 100 ],
   "ContentsOpaque": false,
   "DrawsContent": true,
   "HitTestableWithoutDrawsContent": false,
   "Is3dSorted": false,
   "LayerId": 13,
   "LayerType": "cc::SurfaceLayerImpl",
   "OffsetToTransformParent": [ 0, 0 ],
   "Opacity": 1,
   "Transform": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
   "clip_tree_index": 3,
   "effect_tree_index": 2,
   "scroll_tree_index": 3,
   "transform_tree_index": 7
}

Comment 5 by sunxd@chromium.org, Jan 17 (5 days ago)

Before BGPT:

{
   "clip_tree": {
      "nodes": [ {
         "clip": [ 0, 0, 0, 0 ],
         "clip_type": 0,
         "id": 0,
         "parent_id": -1,
         "transform_id": -1
      }, {
         "clip": [ 0, 0, 804, 644 ],
         "clip_type": 0,
         "id": 1,
         "parent_id": 0,
         "transform_id": 0
      }, {
         "clip": [ 2, 39, 800, 600 ],
         "clip_type": 0,
         "id": 2,
         "parent_id": 1,
         "transform_id": 1
      }, {
         "clip": [ 2, 39, 800, 600 ],
         "clip_type": 0,
         "id": 3,
         "parent_id": 2,
         "transform_id": 1
      } ]
   },
   "effect_tree": {
      ... skipped ...
   },
   "scroll_tree": {
      "nodes": [ {
         "bounds": {
            "height": 0,
            "width": 0
         },
         "container_bounds": {
            "height": 0,
            "width": 0
         },
         "element_id": {
            "id_": 0
         },
         "id": 0,
         "offset_to_transform_parent": [ 0, 0 ],
         "overscroll_behavior_x": 1,
         "overscroll_behavior_y": 1,
         "parent_id": -1,
         "scrollable": false,
         "should_flatten": false,
         "transform_id": 0,
         "user_scrollable_horizontal": false,
         "user_scrollable_vertical": false
      }, {
         "bounds": {
            "height": 644,
            "width": 804
         },
         "container_bounds": {
            "height": 0,
            "width": 0
         },
         "element_id": {
            "id_": 0
         },
         "id": 1,
         "offset_to_transform_parent": [ 0, 0 ],
         "overscroll_behavior_x": 1,
         "overscroll_behavior_y": 1,
         "parent_id": 0,
         "scrollable": false,
         "should_flatten": false,
         "transform_id": 1,
         "user_scrollable_horizontal": true,
         "user_scrollable_vertical": true
      } ]
   },
   "sequence_number": 1,
   "transform_tree": {
      "nodes": [ {
         "element_id": {
            "id_": 0
         },
         "flattens_inherited_transform": 0,
         "id": 0,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": -1,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": -1
      }, {
         "element_id": {
            "id_": 0
         },
         "flattens_inherited_transform": 0,
         "id": 1,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": 0,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": 0
      } ]
   }
}

With BGPT:

   "clip_tree": {
      "nodes": [ {
         "clip": [ 0, 0, 0, 0 ],
         "clip_type": 0,
         "id": 0,
         "parent_id": -1,
         "transform_id": -1
      }, {
         "clip": [ 0, 0, 800, 600 ],
         "clip_type": 0,
         "id": 1,
         "parent_id": 0,
         "transform_id": 0
      }, {
         "clip": [ 0, 0, 800, 600 ],
         "clip_type": 0,
         "id": 2,
         "parent_id": 1,
         "transform_id": 5
      }, {
         "clip": [ 0, 0, 80, 80 ],
         "clip_type": 0,
         "id": 3,
         "parent_id": 2,
         "transform_id": 7
      } ]
   },
   "effect_tree": {
      ... skipped ...
   },
   "scroll_tree": {
      "nodes": [ {
         "bounds": {
            "height": 0,
            "width": 0
         },
         "container_bounds": {
            "height": 0,
            "width": 0
         },
         "element_id": {
            "id_": 0
         },
         "id": 0,
         "offset_to_transform_parent": [ 0, 0 ],
         "overscroll_behavior_x": 1,
         "overscroll_behavior_y": 1,
         "parent_id": -1,
         "scrollable": false,
         "should_flatten": false,
         "transform_id": 0,
         "user_scrollable_horizontal": false,
         "user_scrollable_vertical": false
      }, {
         "bounds": {
            "height": 0,
            "width": 0
         },
         "container_bounds": {
            "height": 0,
            "width": 0
         },
         "element_id": {
            "id_": 0
         },
         "id": 1,
         "offset_to_transform_parent": [ 0, 0 ],
         "overscroll_behavior_x": 1,
         "overscroll_behavior_y": 1,
         "parent_id": 0,
         "scrollable": false,
         "should_flatten": false,
         "transform_id": 1,
         "user_scrollable_horizontal": false,
         "user_scrollable_vertical": false
      }, {
         "bounds": {
            "height": 600,
            "width": 800
         },
         "container_bounds": {
            "height": 600,
            "width": 800
         },
         "element_id": {
            "id_": 18
         },
         "id": 2,
         "offset_to_transform_parent": [ 0, 0 ],
         "overscroll_behavior_x": 1,
         "overscroll_behavior_y": 1,
         "parent_id": 1,
         "scrollable": true,
         "should_flatten": false,
         "transform_id": 4,
         "user_scrollable_horizontal": true,
         "user_scrollable_vertical": true
      }, {
         "bounds": {
            "height": 600,
            "width": 800
         },
         "container_bounds": {
            "height": 600,
            "width": 800
         },
         "element_id": {
            "id_": 114
         },
         "id": 3,
         "offset_to_transform_parent": [ 0, 0 ],
         "overscroll_behavior_x": 1,
         "overscroll_behavior_y": 1,
         "parent_id": 2,
         "scrollable": true,
         "should_flatten": false,
         "transform_id": 6,
         "user_scrollable_horizontal": true,
         "user_scrollable_vertical": true
      } ]
   },
   "sequence_number": 3,
   "transform_tree": {
      "nodes": [ {
         "element_id": {
            "id_": 0
         },
         "flattens_inherited_transform": 0,
         "id": 0,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": -1,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": -1
      }, {
         "element_id": {
            "id_": 0
         },
         "flattens_inherited_transform": 0,
         "id": 1,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": 0,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": 0
      }, {
         "element_id": {
            "id_": 27
         },
         "flattens_inherited_transform": 0,
         "id": 2,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": 1,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": 1
      }, {
         "element_id": {
            "id_": 16
         },
         "flattens_inherited_transform": 0,
         "id": 3,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": 2,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": 2
      }, {
         "element_id": {
            "id_": 0
         },
         "flattens_inherited_transform": 0,
         "id": 4,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": 3,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": 3
      }, {
         "element_id": {
            "id_": 0
         },
         "flattens_inherited_transform": 1,
         "id": 5,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": 4,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": 4
      }, {
         "element_id": {
            "id_": 0
         },
         "flattens_inherited_transform": 1,
         "id": 6,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": 5,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": 5
      }, {
         "element_id": {
            "id_": 0
         },
         "flattens_inherited_transform": 1,
         "id": 7,
         "local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "parent_id": 6,
         "post_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "pre_local": [ 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1 ],
         "scroll_offset": [ 0, 0 ],
         "snap_amount": [ 0, 0 ],
         "sorting_context_id": 0,
         "source_node_id": 6
      } ]
   }
}


Comment 6 by sunxd@chromium.org, Jan 17 (5 days ago)

It seems that due to changes in property trees, ComputeAccumulatedClip in cc::draw_property_utils returns different results (is_clipped & visible_layer_rects).

The failures may not be a serious problem: the ClipPathOOPIF/2 test actually wants to make sure that we do async targeting if the iframe has clip-path. We do check if a surface layer has mask when generating hit test data, so it should not affect viz hit test's correctness. the ClippedRotatedOOPIF/2 test tests that we do async targeting if the iframe is effective clip is not a rectangle, and both BGPT and no BGPT version correctly mark the surface layer "async".

Both of the tests failed because visible layer rect is different.

However it's a bit worrying that the computed visible layer rect has changed, I've asked Wei to take a look at that part.

Comment 7 by sunxd@chromium.org, Jan 17 (5 days ago)

correction for c6: s/if the iframe is effective clip is/if the iframe's effective clip is/g.


Comment 8 by weiliangc@chromium.org, Jan 17 (5 days ago)

For layer, is_clipped is calculated as "whether an extra clip need to be apply between me and my target render surface". This is because render surface has limited size, and thus applies clip implicitly. So to be lazy we avoid applying clip if it is bounded by render surface.

So for clip path (implemented by a mask thus a render surface), the layer itself won't set is clipped since it would rely on the render surface's clip.

The difference in whether a layer is clipped is probably due to which layer has the mask layer. When the mask layer is on the surface layer's target, the surface layer would be is_clipped false. If the mask layer is on surface layer, surface layer's target would be one level up, and surface layer would be is_clipped true.

It is possible that hit test data should not rely on is_clipped like this.

For the rotation case, could check clip_in_layer_space in 
https://cs.chromium.org/chromium/src/cc/trees/draw_property_utils.cc?sq=package:chromium&g=0&l=655
This function is calculating visible layer rect by looking at its target's clip, and any other clip between it and its target, and then put back to layer space and compare with layer bounds.


Comment 9 by pdr@chromium.org, Today (5 hours ago)

If this difference is unrelated to BGTP, can the test be updated so it passes with BGPT on? This is the only remaining failure on my trial run of BGPT: https://chromium-review.googlesource.com/c/chromium/src/+/1413850

Sign in to add a comment