Refactor VR Shell Input. Locks input to click/scroll targets.

chrome/browser/android/vr_shell/vr_shell_gl.cc

Index: chrome/browser/android/vr_shell/vr_shell_gl.cc
diff --git a/chrome/browser/android/vr_shell/vr_shell_gl.cc b/chrome/browser/android/vr_shell/vr_shell_gl.cc
index 8cc084495e187ba2461f9dcd2d37b6672bf8eb17..c72c138a45661cdd096a655cca666f7a69682f2e 100644
--- a/chrome/browser/android/vr_shell/vr_shell_gl.cc
+++ b/chrome/browser/android/vr_shell/vr_shell_gl.cc
@@ -28,6 +28,7 @@
 #include "device/vr/android/gvr/gvr_device.h"
 #include "device/vr/android/gvr/gvr_gamepad_data_provider.h"
 #include "device/vr/vr_math.h"
+#include "third_party/WebKit/public/platform/WebGestureEvent.h"
 #include "third_party/WebKit/public/platform/WebInputEvent.h"
 #include "third_party/WebKit/public/platform/WebMouseEvent.h"
 #include "ui/gl/android/scoped_java_surface.h"
@@ -137,14 +138,14 @@ gvr::Mat4f PerspectiveMatrixFromView(const gvr::Rectf& fov,
   return result;
 }
 
-std::unique_ptr<blink::WebMouseEvent> MakeMouseEvent(WebInputEvent::Type type,
-                                                     double timestamp,
-                                                     float x,
-                                                     float y) {
+std::unique_ptr<blink::WebMouseEvent> MakeMouseEvent(
+    blink::WebInputEvent::Type type,
+    double timestamp,
+    const gfx::Point& loc) {
   std::unique_ptr<blink::WebMouseEvent> mouse_event(new blink::WebMouseEvent(
       type, blink::WebInputEvent::kNoModifiers, timestamp));
   mouse_event->pointer_type = blink::WebPointerProperties::PointerType::kMouse;
-  mouse_event->SetPositionInWidget(x, y);
+  mouse_event->SetPositionInWidget(loc.x(), loc.y());
   mouse_event->click_count = 1;
 
   return mouse_event;
@@ -183,6 +184,10 @@ gfx::RectF GfxRectFromUV(gvr::Rectf rect) {
                     rect.top - rect.bottom);
 }
 
+double NowSeconds() {
+  return (base::TimeTicks::Now() - base::TimeTicks()).InSecondsF();
+}
+
 }  // namespace
 
 VrShellGl::VrShellGl(VrBrowserInterface* browser,
@@ -504,23 +509,7 @@ void VrShellGl::UpdateController(const gfx::Vector3dF& head_direction) {
 }
 
 void VrShellGl::HandleControllerInput(const gfx::Vector3dF& head_direction) {
-  if (ShouldDrawWebVr()) {
-    // Process screen touch events for Cardboard button compatibility.
-    // Also send tap events for controller "touchpad click" events.
-    if (touch_pending_ ||
-        controller_->ButtonUpHappened(
-            gvr::ControllerButton::GVR_CONTROLLER_BUTTON_CLICK)) {
-      touch_pending_ = false;
-      std::unique_ptr<WebGestureEvent> gesture(new WebGestureEvent(
-          WebInputEvent::kGestureTapDown, WebInputEvent::kNoModifiers,
-          (base::TimeTicks::Now() - base::TimeTicks()).InSecondsF()));
-      gesture->source_device = blink::kWebGestureDeviceTouchpad;
-      gesture->x = 0;
-      gesture->y = 0;
-      SendGestureToContent(std::move(gesture));
-      DVLOG(1) << __FUNCTION__ << ": sent CLICK gesture";
-    }
-  }
+  HandleWebVrCompatClick();

[cjgrant, 2017/05/10 20:16:25]

s/Compat/Compatibility/

[mthiesse, 2017/05/10 21:09:08]

Done.

 
   gfx::Vector3dF ergo_neutral_pose;
   if (!controller_->IsConnected()) {
@@ -540,10 +529,253 @@ void VrShellGl::HandleControllerInput(const gfx::Vector3dF& head_direction) {
 
   HandleControllerAppButtonActivity(controller_direction);
 
-  if (ShouldDrawWebVr()) {
+  if (ShouldDrawWebVr())
+    return;

[cjgrant, 2017/05/10 20:16:26]

Blank line after return?

[mthiesse, 2017/05/10 21:09:08]

Done.

+  gfx::PointF target_local_point;
+  gfx::Vector3dF eye_to_target;
+  cursor_render_target_ = nullptr;
+  GetVisualTargetElement(controller_direction, eye_to_target, target_point_,
+                         &cursor_render_target_, target_local_point);
+
+  UiElement* target_element = nullptr;
+  if (input_locked_element_) {
+    gfx::Point3F plane_intersection_point;
+    float distance_to_plane;
+    GetTargetLocalPoint(eye_to_target, *input_locked_element_,
+                        2 * scene_->GetBackgroundDistance(), target_local_point,
+                        plane_intersection_point, distance_to_plane);
+    target_element = input_locked_element_;
+  } else if (!in_scroll_ && !in_click_) {
+    target_element = cursor_render_target_;
+  }
+
+  // Handle input targeting on the content quad, ignoring any other elements.
+  // Content is treated specially to accomodate scrolling, flings, etc.
+  gfx::Point local_point_pixels;
+  if (target_element && (target_element->fill() == Fill::CONTENT)) {
+    gfx::RectF pixel_rect(0, 0, content_tex_css_width_,
+                          content_tex_css_height_);
+    local_point_pixels.set_x(pixel_rect.x() +
+                             pixel_rect.width() * target_local_point.x());
+    local_point_pixels.set_y(pixel_rect.y() +
+                             pixel_rect.height() * target_local_point.y());
+  }
+  std::unique_ptr<GestureList> gesture_list_ptr = controller_->DetectGestures();
+  GestureList& gesture_list = *gesture_list_ptr;
+  for (const std::unique_ptr<blink::WebGestureEvent>& gesture : gesture_list) {
+    gesture->x = local_point_pixels.x();
+    gesture->y = local_point_pixels.y();
+  }
+  SendFlingCancel(gesture_list);
+  // For simplicity, don't allow scrolling while clicking until we need to.
+  if (!in_click_) {
+    SendScrollEnd(gesture_list);
+    if (!SendScrollBegin(target_element, gesture_list)) {
+      SendScrollUpdate(gesture_list);
+    }
+  }
+  // If we're still scrolling, don't hover (and we can't be clicking, because
+  // click ends scroll).
+  if (in_scroll_)
+    return;
+  SendHoverLeave(target_element);
+  if (!SendHoverEnter(target_element, target_local_point, local_point_pixels)) {
+    SendHoverMove(target_local_point, local_point_pixels);
+  }
+  SendButtonDown(target_element, target_local_point);
+  if (!SendButtonUp(target_element, target_local_point))
+    SendTap(target_element, target_local_point, local_point_pixels);
+}
+
+void VrShellGl::HandleWebVrCompatClick() {
+  if (!ShouldDrawWebVr())
+    return;
+  // Process screen touch events for Cardboard button compatibility.
+  // Also send tap events for controller "touchpad click" events.
+  if (touch_pending_ ||
+      controller_->ButtonUpHappened(gvr::kControllerButtonClick)) {
+    touch_pending_ = false;
+    std::unique_ptr<blink::WebGestureEvent> gesture(new blink::WebGestureEvent(
+        blink::WebInputEvent::kGestureTapDown,
+        blink::WebInputEvent::kNoModifiers, NowSeconds()));
+    gesture->source_device = blink::kWebGestureDeviceTouchpad;
+    gesture->x = 0;
+    gesture->y = 0;
+    SendGestureToContent(std::move(gesture));
+    DVLOG(1) << __FUNCTION__ << ": sent CLICK gesture";
+  }
+}
+
+void VrShellGl::SendFlingCancel(GestureList& gesture_list) {
+  if (!fling_target_)
+    return;
+  if (gesture_list.empty() || (gesture_list.front()->GetType() !=
+                               blink::WebInputEvent::kGestureFlingCancel))
+    return;
+  // Scrolling currently only supported on content window.
+  DCHECK_EQ(fling_target_->fill(), Fill::CONTENT);
+  SendGestureToContent(std::move(gesture_list.front()));
+  gesture_list.erase(gesture_list.begin());
+}
+
+void VrShellGl::SendScrollEnd(GestureList& gesture_list) {
+  if (!in_scroll_)
+    return;
+  DCHECK_NE(input_locked_element_, nullptr);
+  if (controller_->ButtonDownHappened(gvr::kControllerButtonClick)) {
+    DCHECK_GT(gesture_list.size(), 0LU);
+    DCHECK_EQ(gesture_list.front()->GetType(),
+              blink::WebInputEvent::kGestureScrollEnd);
+  }
+  // Scrolling currently only supported on content window.
+  DCHECK_EQ(input_locked_element_->fill(), Fill::CONTENT);
+  if (gesture_list.empty() || (gesture_list.front()->GetType() !=
+                               blink::WebInputEvent::kGestureScrollEnd))
+    return;
+  DCHECK_LE(gesture_list.size(), 2LU);
+  SendGestureToContent(std::move(gesture_list.front()));
+  gesture_list.erase(gesture_list.begin());
+  if (!gesture_list.empty()) {
+    DCHECK_EQ(gesture_list.front()->GetType(),
+              blink::WebInputEvent::kGestureFlingStart);
+    SendGestureToContent(std::move(gesture_list.front()));
+    fling_target_ = input_locked_element_;
+    gesture_list.erase(gesture_list.begin());
+  }
+  input_locked_element_ = nullptr;
+  in_scroll_ = false;
+}
+
+bool VrShellGl::SendScrollBegin(UiElement* target, GestureList& gesture_list) {
+  if (in_scroll_ || !target)
+    return false;
+  // Scrolling currently only supported on content window.
+  if (target->fill() != Fill::CONTENT)
+    return false;
+  if (gesture_list.empty() || (gesture_list.front()->GetType() !=
+                               blink::WebInputEvent::kGestureScrollBegin))
+    return false;
+  input_locked_element_ = target;
+  in_scroll_ = true;
+
+  SendGestureToContent(std::move(gesture_list.front()));
+  gesture_list.erase(gesture_list.begin());
+  return true;
+}
+
+void VrShellGl::SendScrollUpdate(GestureList& gesture_list) {
+  if (!in_scroll_)
+    return;
+  DCHECK(input_locked_element_);
+  if (gesture_list.empty() || (gesture_list.front()->GetType() !=
+                               blink::WebInputEvent::kGestureScrollUpdate))
+    return;
+  // Scrolling currently only supported on content window.
+  DCHECK_EQ(input_locked_element_->fill(), Fill::CONTENT);
+  SendGestureToContent(std::move(gesture_list.front()));
+  gesture_list.erase(gesture_list.begin());
+}
+
+void VrShellGl::SendHoverLeave(UiElement* target) {
+  if (!hover_target_ || (target == hover_target_))
+    return;
+  if (hover_target_->fill() == Fill::CONTENT) {
+    SendGestureToContent(MakeMouseEvent(blink::WebInputEvent::kMouseLeave,
+                                        NowSeconds(), gfx::Point()));
+  } else {
+    hover_target_->OnHoverLeave();
+  }
+  hover_target_ = nullptr;
+}
+
+bool VrShellGl::SendHoverEnter(UiElement* target,
+                               const gfx::PointF& target_point,
+                               const gfx::Point& local_point_pixels) {
+  if (!target || target == hover_target_)
+    return false;
+  if (target->fill() == Fill::CONTENT) {
+    SendGestureToContent(MakeMouseEvent(blink::WebInputEvent::kMouseEnter,
+                                        NowSeconds(), local_point_pixels));
+  } else {
+    target->OnHoverEnter(target_point);
+  }
+  hover_target_ = target;
+  return true;
+}
+
+void VrShellGl::SendHoverMove(const gfx::PointF& target_point,
+                              const gfx::Point& local_point_pixels) {
+  if (!hover_target_)
+    return;
+  if (hover_target_->fill() == Fill::CONTENT) {
+    SendGestureToContent(MakeMouseEvent(blink::WebInputEvent::kMouseMove,
+                                        NowSeconds(), local_point_pixels));
+  } else {
+    hover_target_->OnMove(target_point);
+  }
+}
+
+void VrShellGl::SendButtonDown(UiElement* target,
+                               const gfx::PointF& target_point) {
+  if (in_click_)
+    return;
+  if (!controller_->ButtonDownHappened(gvr::kControllerButtonClick))

[cjgrant, 2017/05/10 20:16:26]

Isn't it more intuitive to not call SendButtonDown if a controller button-down
did not happen?

[mthiesse, 2017/05/10 21:09:08]

So this is a stylistic choice. I decided to avoid cluttering up
HandleControllerInput with logic about which events to send, and pushed all of
the logic down into the functions.

I could do something like call them all "MaybeSendButtonDown", but all of these
functions are consistent and hopefully clear in what they're doing, so I don't
know that that's necessary.

+    return;
+  input_locked_element_ = target;
+  in_click_ = true;
+  // We don't support down/up for content yet.
+  if (!target || target->fill() == Fill::CONTENT)
+    return;
+  target->OnButtonDown(target_point);
+}
+
+bool VrShellGl::SendButtonUp(UiElement* target,
+                             const gfx::PointF& target_point) {
+  if (!in_click_)
+    return false;
+  if (!controller_->ButtonUpHappened(gvr::kControllerButtonClick))
+    return false;
+  DCHECK(input_locked_element_ == target);
+  input_locked_element_ = nullptr;
+  in_click_ = false;
+  // We don't support down/up for content yet.
+  if (target->fill() == Fill::CONTENT)
+    return false;
+  target->OnButtonUp(target_point);
+  return true;
+}
+
+void VrShellGl::SendTap(UiElement* target,
+                        const gfx::PointF& target_point,
+                        const gfx::Point& local_point_pixels) {
+  if (!target)
     return;
+  if (controller_->ButtonUpHappened(gvr::kControllerButtonClick) &&
+      target->fill() == Fill::CONTENT)
+    touch_pending_ = true;
+  if (!touch_pending_)
+    return;
+  touch_pending_ = false;
+  if (target->fill() == Fill::CONTENT) {
+    auto gesture = base::MakeUnique<blink::WebGestureEvent>(
+        blink::WebInputEvent::kGestureTapDown,
+        blink::WebInputEvent::kNoModifiers, NowSeconds());
+    gesture->source_device = blink::kWebGestureDeviceTouchpad;
+    gesture->x = local_point_pixels.x();
+    gesture->y = local_point_pixels.y();
+    SendGestureToContent(std::move(gesture));
+  } else {
+    target->OnButtonDown(target_point);
+    target->OnButtonUp(target_point);
   }
+}
 
+void VrShellGl::GetVisualTargetElement(
+    const gfx::Vector3dF& controller_direction,
+    gfx::Vector3dF& eye_to_target,
+    gfx::Point3F& target_point,
+    UiElement** target_element,
+    gfx::PointF& target_local_point) const {
   // If we place the reticle based on elements intersecting the controller beam,
   // we can end up with the reticle hiding behind elements, or jumping laterally
   // in the field of view. This is physically correct, but hard to use. For
@@ -559,63 +791,55 @@ void VrShellGl::HandleControllerInput(const gfx::Vector3dF& head_direction) {
   // that the sphere is centered at the controller, rather than the eye, for
   // simplicity.
   float distance = scene_->GetBackgroundDistance();
-  target_point_ =
+  target_point =
       vr::GetRayPoint(pointer_start_, controller_direction, distance);
-  gfx::Vector3dF eye_to_target = target_point_ - kOrigin;
+  eye_to_target = target_point - kOrigin;
   vr::NormalizeVector(&eye_to_target);
 
   // Determine which UI element (if any) intersects the line between the eyes
   // and the controller target position.
-  float closest_element_distance = (target_point_ - kOrigin).Length();
-  previous_target_element_ = target_element_;
-  target_element_ = nullptr;
-  float target_x;
-  float target_y;
-
-  for (auto& plane : scene_->GetUiElements()) {
-    if (!plane->IsHitTestable())
-      continue;
+  float closest_element_distance = (target_point - kOrigin).Length();
 
-    float distance_to_plane;
-    if (!plane->GetRayDistance(kOrigin, eye_to_target, &distance_to_plane))
+  for (auto& element : scene_->GetUiElements()) {
+    if (!element->IsHitTestable())
       continue;
-
-    if (distance_to_plane < 0 || distance_to_plane >= closest_element_distance)
+    gfx::PointF local_point;
+    gfx::Point3F plane_intersection_point;
+    float distance_to_plane;
+    if (!GetTargetLocalPoint(eye_to_target, *element.get(),

[cjgrant, 2017/05/10 20:16:26]

So to be totally clear, the idea of having an element determine whether it's
being hit is a follow-on change...

[mthiesse, 2017/05/10 21:09:08]

yes.

+                             closest_element_distance, local_point,
+                             plane_intersection_point, distance_to_plane))
       continue;
 
-    gfx::Point3F plane_intersection_point =
-        vr::GetRayPoint(kOrigin, eye_to_target, distance_to_plane);
-    gfx::PointF unit_xy_point =
-        plane->GetUnitRectangleCoordinates(plane_intersection_point);
-
-    float x = 0.5f + unit_xy_point.x();
-    float y = 0.5f - unit_xy_point.y();
-    if (x < 0.0f || x >= 1.0f || y < 0.0f || y >= 1.0f)
+    if (local_point.x() < 0.0f || local_point.x() >= 1.0f ||
+        local_point.y() < 0.0f || local_point.y() >= 1.0f)
       continue;
 
     closest_element_distance = distance_to_plane;
-    target_point_ = plane_intersection_point;
-    target_element_ = plane.get();
-    target_x = x;
-    target_y = y;
+    target_point = plane_intersection_point;
+    *target_element = element.get();
+    target_local_point = local_point;
   }
+}
 
-  // Handle input targeting on the content quad, ignoring any other elements.
-  // Content is treated specially to accomodate scrolling, flings, etc.
-  InputTarget input_target = InputTarget::NONE;
-  int pixel_x = 0;
-  int pixel_y = 0;
-  if (target_element_ != nullptr && target_element_->fill() == Fill::CONTENT) {
-    input_target = InputTarget::CONTENT;
-    gfx::RectF pixel_rect(0, 0, content_tex_css_width_,
-                          content_tex_css_height_);
-    pixel_x = pixel_rect.x() + pixel_rect.width() * target_x;
-    pixel_y = pixel_rect.y() + pixel_rect.height() * target_y;
-  }
-  SendInputToContent(input_target, pixel_x, pixel_y);
+bool VrShellGl::GetTargetLocalPoint(const gfx::Vector3dF& eye_to_target,
+                                    const UiElement& element,
+                                    float max_distance_to_plane,
+                                    gfx::PointF& target_local_point,
+                                    gfx::Point3F& target_point,
+                                    float& distance_to_plane) const {
+  if (!element.GetRayDistance(kOrigin, eye_to_target, &distance_to_plane))
+    return false;
+
+  if (distance_to_plane < 0 || distance_to_plane >= max_distance_to_plane)
+    return false;
+
+  target_point = vr::GetRayPoint(kOrigin, eye_to_target, distance_to_plane);
+  gfx::PointF unit_xy_point = element.GetUnitRectangleCoordinates(target_point);
 
-  // Handle input targeting on all non-content elements.
-  SendInputToUiElements(target_element_);
+  target_local_point.set_x(0.5f + unit_xy_point.x());
+  target_local_point.set_y(0.5f - unit_xy_point.y());
+  return true;
 }
 
 void VrShellGl::HandleControllerAppButtonActivity(
@@ -648,125 +872,6 @@ void VrShellGl::HandleControllerAppButtonActivity(
   }
 }
 
-void VrShellGl::SendInputToContent(InputTarget input_target,
-                                   int pixel_x,
-                                   int pixel_y) {
-  std::vector<std::unique_ptr<WebGestureEvent>> gesture_list =
-      controller_->DetectGestures();
-  double timestamp = gesture_list.front()->TimeStampSeconds();
-
-  if (touch_pending_) {
-    touch_pending_ = false;
-    std::unique_ptr<WebGestureEvent> event(
-        new WebGestureEvent(WebInputEvent::kGestureTapDown,
-                            WebInputEvent::kNoModifiers, timestamp));
-    event->source_device = blink::kWebGestureDeviceTouchpad;
-    event->x = pixel_x;
-    event->y = pixel_y;
-    gesture_list.push_back(std::move(event));
-  }
-
-  for (auto& gesture : gesture_list) {
-    gesture->x = pixel_x;
-    gesture->y = pixel_y;
-    auto movableGesture = base::MakeUnique<WebGestureEvent>(*gesture);
-
-    switch (gesture->GetType()) {
-      // Once the user starts scrolling send all the scroll events to this
-      // element until the scrolling stops.
-      case WebInputEvent::kGestureScrollBegin:
-        current_scroll_target_ = input_target;
-        if (current_scroll_target_ != InputTarget::NONE) {
-          SendGestureToContent(std::move(movableGesture));
-        }
-        break;
-      case WebInputEvent::kGestureScrollEnd:
-        if (current_scroll_target_ != InputTarget::NONE) {
-          SendGestureToContent(std::move(movableGesture));
-        }
-        current_fling_target_ = current_scroll_target_;
-        current_scroll_target_ = InputTarget::NONE;
-        break;
-      case WebInputEvent::kGestureScrollUpdate:
-        if (current_scroll_target_ != InputTarget::NONE) {
-          SendGestureToContent(std::move(movableGesture));
-        }
-        break;
-      case WebInputEvent::kGestureFlingStart:
-        if (current_fling_target_ != InputTarget::NONE) {
-          SendGestureToContent(std::move(movableGesture));
-        }
-        current_fling_target_ = InputTarget::NONE;
-        break;
-      case WebInputEvent::kGestureFlingCancel:
-        current_fling_target_ = InputTarget::NONE;
-        if (input_target != InputTarget::NONE) {
-          SendGestureToContent(std::move(movableGesture));
-        }
-        break;
-      case WebInputEvent::kGestureTapDown:
-        current_fling_target_ = InputTarget::NONE;
-        if (input_target != InputTarget::NONE) {
-          SendGestureToContent(std::move(movableGesture));
-        }
-        break;
-      case WebInputEvent::kUndefined:
-        break;
-      default:
-        NOTREACHED();
-    }
-  }
-
-  // Hover support
-  bool new_target = input_target != current_input_target_;
-  if (new_target && current_input_target_ != InputTarget::NONE) {
-    // Send a move event indicating that the pointer moved off of an element.
-    SendGestureToContent(
-        MakeMouseEvent(WebInputEvent::kMouseLeave, timestamp, 0, 0));
-  }
-  current_input_target_ = input_target;
-  if (current_input_target_ != InputTarget::NONE) {
-    WebInputEvent::Type type =
-        new_target ? WebInputEvent::kMouseEnter : WebInputEvent::kMouseMove;
-    SendGestureToContent(MakeMouseEvent(type, timestamp, pixel_x, pixel_y));
-  }
-}
-
-void VrShellGl::SendInputToUiElements(UiElement* target_element) {
-  if (target_element != previous_target_element_) {
-    if (previous_target_element_ &&
-        previous_target_element_->fill() != Fill::CONTENT) {
-      task_runner_->PostTask(
-          FROM_HERE, base::Bind(&UiElement::OnHoverLeave,
-                                base::Unretained(previous_target_element_)));
-    }
-    if (target_element && target_element->fill() != Fill::CONTENT) {
-      task_runner_->PostTask(FROM_HERE,
-                             base::Bind(&UiElement::OnHoverEnter,
-                                        base::Unretained(target_element)));
-    }
-    click_target_element_ = nullptr;
-  }
-  if (target_element && target_element->fill() != Fill::CONTENT) {
-    if (controller_->ButtonDownHappened(
-            gvr::ControllerButton::GVR_CONTROLLER_BUTTON_CLICK)) {
-      task_runner_->PostTask(FROM_HERE,
-                             base::Bind(&UiElement::OnButtonDown,
-                                        base::Unretained(target_element)));
-      click_target_element_ = target_element;
-    }
-    if (controller_->ButtonUpHappened(
-            gvr::ControllerButton::GVR_CONTROLLER_BUTTON_CLICK)) {
-      if (click_target_element_ == target_element) {
-        task_runner_->PostTask(FROM_HERE,
-                               base::Bind(&UiElement::OnButtonUp,
-                                          base::Unretained(target_element)));
-      }
-      click_target_element_ = nullptr;
-    }
-  }
-}
-
 void VrShellGl::SendGestureToContent(
     std::unique_ptr<blink::WebInputEvent> event) {
   browser_->ProcessContentGesture(std::move(event));
@@ -1041,38 +1146,23 @@ void VrShellGl::DrawElements(const vr::Mat4f& view_proj_matrix,
 std::vector<const UiElement*> VrShellGl::GetElementsInDrawOrder(
     const vr::Mat4f& view_matrix,
     const std::vector<const UiElement*>& elements) {
-  typedef std::pair<float, const UiElement*> DistanceElementPair;
-  std::vector<DistanceElementPair> zOrderedElementPairs;
-  zOrderedElementPairs.reserve(elements.size());
-
-  for (const auto* element : elements) {
-    // Distance is the abs(z) value in view space.
-    gfx::Vector3dF element_position =
-        vr::GetTranslation(element->TransformMatrix());
-
-    float distance =
-        std::fabs(vr::MatrixVectorMul(view_matrix, element_position).z());
-    zOrderedElementPairs.push_back(std::make_pair(distance, element));
-  }
+  std::vector<const UiElement*> sorted_elements = elements;
 
   // Sort elements primarily based on their draw phase (lower draw phase first)
-  // and secondarily based on their distance (larger distance first).
-  std::sort(
-      zOrderedElementPairs.begin(), zOrderedElementPairs.end(),
-      [](const DistanceElementPair& first, const DistanceElementPair& second) {
-        if (first.second->draw_phase() != second.second->draw_phase()) {
-          return first.second->draw_phase() < second.second->draw_phase();
-        } else {
-          return first.first > second.first;
-        }
-      });
-
-  std::vector<const UiElement*> zOrderedElements;
-  zOrderedElements.reserve(elements.size());
-  for (auto distanceElementPair : zOrderedElementPairs) {
-    zOrderedElements.push_back(distanceElementPair.second);
-  }
-  return zOrderedElements;
+  // and secondarily based on their distance (larger z-axis first).

[cjgrant, 2017/05/10 20:16:26]

"...based on their Z-axis distance..." maybe?

[mthiesse, 2017/05/10 21:09:08]

Done.

[mthiesse, 2017/05/10 21:09:08]

Didn't mean to include these changes in this CL. Will separate out.

+  // TODO(mthiesse): This will not work well for elements not directly in front
+  // of the user, but works well enough for our initial release, and provides a
+  // consistent ordering that we can easily design around.
+  std::sort(sorted_elements.begin(), sorted_elements.end(),
+            [](const UiElement* first, const UiElement* second) {
+              if (first->draw_phase() != second->draw_phase()) {
+                return first->draw_phase() < second->draw_phase();
+              } else {
+                return first->translation().z() < second->translation().z();
+              }
+            });
+
+  return sorted_elements;
 }
 
 void VrShellGl::DrawCursor(const vr::Mat4f& render_matrix) {
@@ -1090,9 +1180,9 @@ void VrShellGl::DrawCursor(const vr::Mat4f& render_matrix) {
       &mat);
 
   vr::Quatf rotation;
-  if (target_element_ != nullptr) {
+  if (cursor_render_target_ != nullptr) {
     // Make the reticle planar to the element it's hitting.
-    rotation = GetRotationFromZAxis(target_element_->GetNormal());
+    rotation = GetRotationFromZAxis(cursor_render_target_->GetNormal());
   } else {
     // Rotate the cursor to directly face the eyes.
     rotation = GetRotationFromZAxis(target_point_ - kOrigin);