[vr] Ensure that hit testing uses the same stacking order as rendering

The rendering code was updated to sort elements in tree rather than
depth order in this cl:
https://chromium-review.googlesource.com/c/chromium/src/+/727342

The hit testing code, however, was not similarly updated. Consequently,
there can be discrepancies. This CL updates the hit testing code to
use nearly the same stacking order as rendering. Note: it is not quite
perfect because the ui renderer still summons swaths of elements from
the scene explicitly. Eventually, the ui renderer will simply sort
and render all elements in the tree and these will be truly identical.
However, until then, this ensures we do use the same element sorting
routines.

Bug: None
Change-Id: I12483713f19b678aa9a69b91e5d910e408b73007
Reviewed-on: https://chromium-review.googlesource.com/754554Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514136}

chrome/browser/vr/ui_input_manager.cc

@@ -6,10 +6,12 @@
 
 #include <algorithm>
 
+#include "base/containers/adapters.h"
 #include "base/macros.h"
 #include "chrome/browser/vr/elements/ui_element.h"
 #include "chrome/browser/vr/model/controller_model.h"
 #include "chrome/browser/vr/model/reticle_model.h"
+#include "chrome/browser/vr/ui_renderer.h"
 #include "chrome/browser/vr/ui_scene.h"
 // TODO(tiborg): Remove include once we use a generic type to pass scroll/fling
 // gestures.
@@ -51,7 +53,6 @@ bool IsScrollEvent(const GestureList& list) {
 
 bool GetTargetLocalPoint(const gfx::Vector3dF& eye_to_target,
                          const UiElement& element,
-                         float max_distance_to_plane,
                          gfx::PointF* out_target_local_point,
                          gfx::Point3F* out_target_point,
                          float* out_distance_to_plane) {
@@ -59,8 +60,7 @@ bool GetTargetLocalPoint(const gfx::Vector3dF& eye_to_target,
     return false;
   }
 
-  if (*out_distance_to_plane < 0 ||
-      *out_distance_to_plane >= max_distance_to_plane) {
+  if (*out_distance_to_plane < 0) {
     return false;
   }
 
@@ -74,37 +74,6 @@ bool GetTargetLocalPoint(const gfx::Vector3dF& eye_to_target,
   return true;
 }
 
-void HitTestElements(UiElement* element,
-                     ReticleModel* reticle_model,
-                     gfx::Vector3dF* out_eye_to_target,
-                     float* out_closest_element_distance) {
-  for (auto& child : element->children()) {
-    HitTestElements(child.get(), reticle_model, out_eye_to_target,
-                    out_closest_element_distance);
-  }
-
-  if (!element->IsHitTestable()) {
-    return;
-  }
-
-  gfx::PointF local_point;
-  gfx::Point3F plane_intersection_point;
-  float distance_to_plane;
-  if (!GetTargetLocalPoint(*out_eye_to_target, *element,
-                           *out_closest_element_distance, &local_point,
-                           &plane_intersection_point, &distance_to_plane)) {
-    return;
-  }
-  if (!element->HitTest(local_point)) {
-    return;
-  }
-
-  *out_closest_element_distance = distance_to_plane;
-  reticle_model->target_point = plane_intersection_point;
-  reticle_model->target_element_id = element->id();
-  reticle_model->target_local_point = local_point;
-}
-
 }  // namespace
 
 UiInputManager::UiInputManager(UiScene* scene) : scene_(scene) {}
@@ -130,7 +99,6 @@ void UiInputManager::HandleInput(base::TimeTicks current_time,
       gfx::Point3F plane_intersection_point;
       float distance_to_plane;
       if (!GetTargetLocalPoint(eye_to_target, *target_element,
-                               2 * scene_->background_distance(),
                                &reticle_model->target_local_point,
                                &plane_intersection_point, &distance_to_plane)) {
         reticle_model->target_local_point = kInvalidTargetPoint;
@@ -403,13 +371,37 @@ void UiInputManager::GetVisualTargetElement(
   *out_eye_to_target = reticle_model->target_point - kOrigin;
   out_eye_to_target->GetNormalized(out_eye_to_target);
 
-  // Determine which UI element (if any) intersects the line between the eyes
-  // and the controller target position.
-  float closest_element_distance =
-      (reticle_model->target_point - kOrigin).Length();
+  std::vector<const UiElement*> elements;
+  for (auto& element : scene_->root_element()) {
+    if (element.IsVisible()) {
+      elements.push_back(&element);
+    }
+  }
+
+  std::vector<const UiElement*> sorted =
+      UiRenderer::GetElementsInDrawOrder(elements);
+
+  for (const auto* element : base::Reversed(sorted)) {
+    if (!element->IsHitTestable()) {
+      continue;
+    }
 
-  HitTestElements(&scene_->root_element(), reticle_model, out_eye_to_target,
-                  &closest_element_distance);
+    gfx::PointF local_point;
+    gfx::Point3F plane_intersection_point;
+    float distance_to_plane;
+    if (!GetTargetLocalPoint(*out_eye_to_target, *element, &local_point,
+                             &plane_intersection_point, &distance_to_plane)) {
+      continue;
+    }
+    if (!element->HitTest(local_point)) {
+      continue;
+    }
+
+    reticle_model->target_point = plane_intersection_point;
+    reticle_model->target_element_id = element->id();
+    reticle_model->target_local_point = local_point;
+    break;
+  }
 }
 
 void UiInputManager::UpdateQuiescenceState(

chrome/browser/vr/ui_input_manager_unittest.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/vr/test/mock_content_input_delegate.h"
 #include "chrome/browser/vr/test/ui_scene_manager_test.h"
 #include "chrome/browser/vr/ui_scene.h"
+#include "chrome/browser/vr/ui_scene_constants.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/WebKit/public/platform/WebGestureEvent.h"
 
@@ -230,6 +231,7 @@ TEST_F(UiInputManagerTest, ElementDeletion) {
 }
 
 TEST_F(UiInputManagerContentTest, NoMouseMovesDuringClick) {
+  EXPECT_TRUE(AnimateBy(MsToDelta(500)));
   // It would be nice if the controller weren't platform specific and we could
   // mock out the underlying sensor data. For now, we will hallucinate
   // parameters to HandleInput.
@@ -261,4 +263,40 @@ TEST_F(UiInputManagerContentTest, NoMouseMovesDuringClick) {
                               &gesture_list);
 }
 
+TEST_F(UiInputManagerContentTest, TreeVsZOrder) {
+  EXPECT_TRUE(AnimateBy(MsToDelta(500)));
+  // It would be nice if the controller weren't platform specific and we could
+  // mock out the underlying sensor data. For now, we will hallucinate
+  // parameters to HandleInput.
+  UiElement* content_quad =
+      scene_->GetUiElementByName(UiElementName::kContentQuad);
+  gfx::Point3F content_quad_center;
+  content_quad->world_space_transform().TransformPoint(&content_quad_center);
+  gfx::Point3F origin;
+
+  ControllerModel controller_model;
+  controller_model.laser_direction = content_quad_center - origin;
+  controller_model.laser_origin = origin;
+  controller_model.touchpad_button_state = UiInputManager::ButtonState::DOWN;
+  ReticleModel reticle_model;
+  GestureList gesture_list;
+  input_manager_->HandleInput(MsToTicks(1), controller_model, &reticle_model,
+                              &gesture_list);
+
+  // We should have hit the content quad if our math was correct.
+  ASSERT_NE(0, reticle_model.target_element_id);
+  EXPECT_EQ(content_quad->id(), reticle_model.target_element_id);
+
+  // We will now move the content quad behind the backplane.
+  content_quad->SetTranslate(0, 0, -2.0 * kTextureOffset);
+  EXPECT_TRUE(AnimateBy(MsToDelta(500)));
+  input_manager_->HandleInput(MsToTicks(1), controller_model, &reticle_model,
+                              &gesture_list);
+
+  // We should have hit the content quad even though, geometrically, it stacks
+  // behind the backplane.
+  ASSERT_NE(0, reticle_model.target_element_id);
+  EXPECT_EQ(content_quad->id(), reticle_model.target_element_id);
+}
+
 }  // namespace vr