Get the transform from the root target when preparing hit-test data.

When preparing hit-test data for browser and renderer, use the inverse
of render_pass->transform_to_root_target as the transform from the root
target. This transform is identity in most cases but not in very
complicated cases (e.g. https://codepen.io/anon/pen/WMqxrE by weiliangc).

Bug: 806144
Test: viz_unittests
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I08e46b45a49da1b42ae1c32f6aafaf7a54aabf42
Reviewed-on: https://chromium-review.googlesource.com/951842Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: weiliangc <weiliangc@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542251}

components/viz/client/hit_test_data_provider_draw_quad.cc

@@ -23,6 +23,14 @@ mojom::HitTestRegionListPtr HitTestDataProviderDrawQuad::GetHitTestData(
   hit_test_region_list->bounds.set_size(compositor_frame.size_in_pixels());
 
   for (const auto& render_pass : compositor_frame.render_pass_list) {
+    // Skip the render_pass if the transform is not invertible (i.e. it will not
+    // be able to receive events).
+    gfx::Transform transform_from_root_target;
+    if (!render_pass->transform_to_root_target.GetInverse(
+            &transform_from_root_target)) {
+      continue;
+    }
+
     for (const DrawQuad* quad : render_pass->quad_list) {
       if (quad->material == DrawQuad::SURFACE_CONTENT) {
         // Skip the quad if the transform is not invertible (i.e. it will not
@@ -44,7 +52,8 @@ mojom::HitTestRegionListPtr HitTestDataProviderDrawQuad::GetHitTestData(
         if (should_ask_for_child_region_)
           hit_test_region->flags |= mojom::kHitTestAsk;
         hit_test_region->rect = surface_quad->rect;
-        hit_test_region->transform = target_to_quad_transform;
+        hit_test_region->transform =
+            target_to_quad_transform * transform_from_root_target;
         hit_test_region_list->regions.push_back(std::move(hit_test_region));
       }
     }

components/viz/client/hit_test_data_provider_draw_quad_unittest.cc

@@ -25,24 +25,32 @@ namespace viz {
 
 namespace {
 
-CompositorFrame MakeCompositorFrameWithChildSurface(
+SurfaceId CreateChildSurfaceId(uint32_t id) {
+  LocalSurfaceId child_local_surface_id(id, base::UnguessableToken::Create());
+  FrameSinkId frame_sink_id(id, 0);
+  SurfaceId child_surface_id(frame_sink_id, child_local_surface_id);
+  return child_surface_id;
+}
+
+std::unique_ptr<RenderPass> CreateRenderPassWithChildSurface(
     const SurfaceId& child_surface_id,
     const gfx::Rect& rect,
     const gfx::Rect& child_rect,
-    const gfx::Transform& transform) {
+    const gfx::Transform& render_pass_transform,
+    const gfx::Transform& shared_state_transform) {
   auto pass = RenderPass::Create();
-  pass->SetNew(1, rect, rect, gfx::Transform());
+  pass->SetNew(1, rect, rect, render_pass_transform);
 
   auto* shared_state = pass->CreateAndAppendSharedQuadState();
-  shared_state->SetAll(transform, rect, rect, rect, false, false, 1,
-                       SkBlendMode::kSrcOver, 0);
+  shared_state->SetAll(shared_state_transform, rect, rect, rect, false, false,
+                       1, SkBlendMode::kSrcOver, 0);
 
   auto* surface_quad = pass->CreateAndAppendDrawQuad<SurfaceDrawQuad>();
   surface_quad->SetNew(pass->shared_quad_state_list.back(), child_rect,
                        child_rect, child_surface_id, base::nullopt,
                        SK_ColorWHITE, false);
 
-  return CompositorFrameBuilder().AddRenderPass(std::move(pass)).Build();
+  return pass;
 }
 
 }  // namespace
@@ -69,14 +77,18 @@ TEST(HitTestDataProviderDrawQuad, HitTestDataRenderer) {
 
   // Ensure that a CompositorFrame with a child surface only set kHitTestAsk
   // for its child surface.
-  LocalSurfaceId child_local_surface_id(2, base::UnguessableToken::Create());
-  FrameSinkId frame_sink_id(2, 0);
-  SurfaceId child_surface_id(frame_sink_id, child_local_surface_id);
+  SurfaceId child_surface_id = CreateChildSurfaceId(2);
   gfx::Rect child_rect(200, 100);
-  gfx::Transform transform;
-  transform.Translate(-200, -100);
-  compositor_frame = MakeCompositorFrameWithChildSurface(
-      child_surface_id, kFrameRect, child_rect, transform);
+  gfx::Transform render_pass_transform;
+  render_pass_transform.Translate(-50, -100);
+  render_pass_transform.Skew(2, 3);
+  gfx::Transform shared_state_transform;
+  shared_state_transform.Translate(-200, -100);
+  auto pass = CreateRenderPassWithChildSurface(
+      child_surface_id, kFrameRect, child_rect, render_pass_transform,
+      shared_state_transform);
+  compositor_frame =
+      CompositorFrameBuilder().AddRenderPass(std::move(pass)).Build();
   hit_test_region_list =
       hit_test_data_provider->GetHitTestData(compositor_frame);
 
@@ -92,9 +104,70 @@ TEST(HitTestDataProviderDrawQuad, HitTestDataRenderer) {
                 mojom::kHitTestChildSurface | mojom::kHitTestAsk,
             hit_test_region_list->regions[0]->flags);
   EXPECT_EQ(child_rect, hit_test_region_list->regions[0]->rect);
-  gfx::Transform transform_inverse;
-  EXPECT_TRUE(transform.GetInverse(&transform_inverse));
-  EXPECT_EQ(transform_inverse, hit_test_region_list->regions[0]->transform);
+  gfx::Transform render_pass_transform_inverse;
+  EXPECT_TRUE(render_pass_transform.GetInverse(&render_pass_transform_inverse));
+  gfx::Transform shared_state_transform_inverse;
+  EXPECT_TRUE(
+      shared_state_transform.GetInverse(&shared_state_transform_inverse));
+  EXPECT_EQ(shared_state_transform_inverse * render_pass_transform_inverse,
+            hit_test_region_list->regions[0]->transform);
+}
+
+// Test to ensure that we skip regions with a non-invertible transform when
+// preparing hit-test data.
+TEST(HitTestDataProviderDrawQuad, HitTestDataInvertibleTransform) {
+  std::unique_ptr<HitTestDataProvider> hit_test_data_provider =
+      std::make_unique<HitTestDataProviderDrawQuad>(
+          true /* should_ask_for_child_region */);
+
+  constexpr gfx::Rect kFrameRect(0, 0, 1024, 768);
+  gfx::Rect child_rect(200, 100);
+
+  // A degenerate matrix of all zeros is not invertible.
+  gfx::Transform not_invertible_transform;
+  not_invertible_transform.matrix().set(0, 0, 0.f);
+  not_invertible_transform.matrix().set(1, 1, 0.f);
+  not_invertible_transform.matrix().set(2, 2, 0.f);
+  not_invertible_transform.matrix().set(3, 3, 0.f);
+
+  gfx::Transform invertible_transform;
+  invertible_transform.Translate(-200, -100);
+
+  RenderPassList pass_list;
+
+  // A render pass that has non-invertible transform.
+  SurfaceId child_surface_id1 = CreateChildSurfaceId(2);
+  auto pass1 = CreateRenderPassWithChildSurface(
+      child_surface_id1, kFrameRect, child_rect, not_invertible_transform,
+      invertible_transform);
+  pass_list.push_back(std::move(pass1));
+
+  // A render pass with a draw quad that has non-invertible transform.
+  SurfaceId child_surface_id2 = CreateChildSurfaceId(3);
+  auto pass2 = CreateRenderPassWithChildSurface(
+      child_surface_id2, kFrameRect, child_rect, invertible_transform,
+      not_invertible_transform);
+  pass_list.push_back(std::move(pass2));
+
+  // A render pass and its draw quad both have invertible transforms
+  SurfaceId child_surface_id3 = CreateChildSurfaceId(4);
+  auto pass3 = CreateRenderPassWithChildSurface(
+      child_surface_id3, kFrameRect, child_rect, invertible_transform,
+      invertible_transform);
+  pass_list.push_back(std::move(pass3));
+
+  auto compositor_frame =
+      CompositorFrameBuilder().SetRenderPassList(std::move(pass_list)).Build();
+  mojom::HitTestRegionListPtr hit_test_region_list =
+      hit_test_data_provider->GetHitTestData(compositor_frame);
+
+  // Only pass3 should have a hit-test region that corresponds to
+  // child_surface_id3.
+  EXPECT_EQ(1u, hit_test_region_list->regions.size());
+  EXPECT_EQ(child_surface_id3.frame_sink_id(),
+            hit_test_region_list->regions[0]->frame_sink_id);
+  EXPECT_EQ(child_surface_id3.local_surface_id(),
+            hit_test_region_list->regions[0]->local_surface_id);
 }
 
 }  // namespace viz

components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc

@@ -198,6 +198,14 @@ mojom::HitTestRegionListPtr DirectLayerTreeFrameSink::CreateHitTestData(
   hit_test_region_list->bounds.set_size(frame.size_in_pixels());
 
   for (const auto& render_pass : frame.render_pass_list) {
+    // Skip the render_pass if the transform is not invertible (i.e. it will not
+    // be able to receive events).
+    gfx::Transform transform_from_root_target;
+    if (!render_pass->transform_to_root_target.GetInverse(
+            &transform_from_root_target)) {
+      continue;
+    }
+
     for (const DrawQuad* quad : render_pass->quad_list) {
       if (quad->material == DrawQuad::SURFACE_CONTENT) {
         // Skip the quad if the transform is not invertible (i.e. it will not
@@ -217,7 +225,8 @@ mojom::HitTestRegionListPtr DirectLayerTreeFrameSink::CreateHitTestData(
         hit_test_region->flags = mojom::kHitTestMouse | mojom::kHitTestTouch |
                                  mojom::kHitTestChildSurface;
         hit_test_region->rect = surface_quad->rect;
-        hit_test_region->transform = target_to_quad_transform;
+        hit_test_region->transform =
+            target_to_quad_transform * transform_from_root_target;
         hit_test_region_list->regions.push_back(std::move(hit_test_region));
       }
     }