VR: Draw the reticle after planar chidren of the hit element.

To properly render the reticle, it needs to be drawn on the element it
is hitting, rather than last in the scene (otherwise, translucent
un-hittable elements may appear to have the reticle instead).  In the
future, we may transition to drawing the reticle last, always, for
simplicity.

However, if we don't do that, we need a way to have the reticle appear
properly on top of composite UI elements - elements made up of
sub-elements in a plane.  Without this change, something like a button
backplane may have the reticle, but portions of the reticle may appear
under a different button foreground element even though it's on the same
plane.  With this change, we traverse the hit element's subtree, and
draw the reticle after the last planar, visible child instead.  This
way, it's guaranteed to render on top of the composed element, unless
the element has hittable children floating in front.

BUG=
R=mthiesse, vollick

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I33a1aef392901b74e3b5814338a4e4c5fb58eac9
Reviewed-on: https://chromium-review.googlesource.com/791151
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Biao She <bshe@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519720}

chrome/browser/vr/ui_scene.cc

@@ -9,6 +9,7 @@
 
 #include "base/containers/adapters.h"
 #include "base/memory/ptr_util.h"
+#include "base/numerics/ranges.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "base/values.h"
@@ -22,6 +23,21 @@ namespace vr {
 
 namespace {
 
+constexpr float kTolerance = 1e-5f;
+
+// Returns true if two elements are in the same plane.
+bool ArePlanar(const UiElement& e1, const UiElement& e2) {
+  gfx::Vector3dF n1 = e1.GetNormal();
+  gfx::Vector3dF n2 = e2.GetNormal();
+  if (!base::IsApproximatelyEqual(n1.x(), n2.x(), kTolerance) ||
+      !base::IsApproximatelyEqual(n1.y(), n2.y(), kTolerance) ||
+      !base::IsApproximatelyEqual(n1.z(), n2.z(), kTolerance)) {
+    return false;
+  }
+  return base::IsApproximatelyEqual(
+      0.0f, gfx::DotProduct(n1, e1.GetCenter() - e2.GetCenter()), kTolerance);
+}
+
 template <typename P>
 UiScene::Elements GetVisibleElements(UiElement* root,
                                      UiElement* reticle_element,
@@ -29,14 +45,30 @@ UiScene::Elements GetVisibleElements(UiElement* root,
   Reticle* reticle = static_cast<Reticle*>(reticle_element);
   UiElement* target = reticle ? reticle->TargetElement() : nullptr;
   UiScene::Elements elements;
+  int reticle_parent_id = 0;
   for (auto& element : *root) {
     if (element.IsVisible() && predicate(&element)) {
       elements.push_back(&element);
       if (target && target->id() == element.id()) {
-        elements.push_back(reticle);
-        reticle->set_draw_phase(element.draw_phase());
+        reticle_parent_id = element.id();
+        // Draw the reticle after the last child that resides in the same plane
+        // as this element. This way, the reticle can't be partially hidden as
+        // it passes across portions of a composite element. By walking the
+        // element's subtree in reverse pre-order, we start at the last-rendered
+        // child and work backwards, possibly as far as the originally targeted
+        // element.
+        for (auto& child : base::Reversed(element)) {
+          if (predicate(&child) && ArePlanar(element, child)) {
+            reticle_parent_id = child.id();
+            break;
+          }
+        }
       }
     }
+    if (reticle_parent_id == element.id()) {
+      elements.push_back(reticle);
+      reticle->set_draw_phase(element.draw_phase());
+    }
   }
   return elements;
 }

chrome/browser/vr/ui_unittest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/vr/ui_scene_creator.h"
 
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/numerics/ranges.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -994,6 +995,44 @@ TEST_F(UiTest, ExitPresentAndFullscreenOnAppButtonClick) {
   ui_->OnAppButtonClicked();
 }
 
+TEST(UiReticleTest, ReticleRenderedOnPlanarChildren) {
+  UiScene scene;
+  Model model;
+
+  auto reticle = base::MakeUnique<Reticle>(&scene, &model);
+  reticle->set_draw_phase(kPhaseNone);
+  scene.root_element().AddChild(std::move(reticle));
+
+  auto element = base::MakeUnique<UiElement>();
+  UiElement* parent = element.get();
+  parent->set_draw_phase(kPhaseForeground);
+  parent->set_name(k2dBrowsingRoot);
+  scene.root_element().AddChild(std::move(element));
+  model.reticle.target_element_id = parent->id();
+
+  // Add 4 children to the parent:
+  // - Parent (hit element, initially having reticle)
+  //   - Planar
+  //   - Planar but offset (should receive reticle)
+  //   - Parallel
+  //   - Rotated
+  for (int i = 0; i < 4; i++) {
+    element = base::MakeUnique<UiElement>();
+    element->set_draw_phase(kPhaseForeground);
+    parent->AddChild(std::move(element));
+  }
+  parent->children()[1]->SetTranslate(1, 0, 0);
+  parent->children()[2]->SetTranslate(0, 0, -1);
+  parent->children()[3]->SetRotate(1, 0, 0, 1);
+  scene.OnBeginFrame(MsToTicks(0), kForwardVector);
+
+  // Sorted set should have 1 parent, 4 children and 1 reticle.
+  UiScene::Elements sorted = scene.GetVisible2dBrowsingElements();
+  EXPECT_EQ(6u, sorted.size());
+  // Reticle should be after the parent and first two children.
+  EXPECT_EQ(sorted[3]->name(), kReticle);
+}
+
 // This test ensures that the render order matches the expected tree state. All
 // phases are merged, as the order is the same. Unnamed and unrenderable
 // elements are skipped.