[vr] Fix reticle sorting

With this change, UiScene inserts the reticle into the lists of elements
that it vends out to the UiRenderer, fixing a reticle rendering
regression.

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

chrome/browser/vr/elements/reticle.cc

@@ -20,6 +20,10 @@ Reticle::Reticle(UiScene* scene, Model* model) : scene_(scene), model_(model) {
 
 Reticle::~Reticle() = default;
 
+UiElement* Reticle::TargetElement() const {
+  return scene_->GetUiElementById(model_->reticle.target_element_id);
+}
+
 void Reticle::Render(UiElementRenderer* renderer,
                      const gfx::Transform& model_view_proj_matrix) const {
   // Scale the reticle to have a fixed FOV size at any distance.
@@ -31,9 +35,7 @@ void Reticle::Render(UiElementRenderer* renderer,
 
   gfx::Quaternion rotation;
 
-  UiElement* target =
-      scene_->GetUiElementById(model_->reticle.target_element_id);
-
+  UiElement* target = TargetElement();
   if (target) {
     // Make the reticle planar to the element it's hitting.
     rotation = gfx::Quaternion(gfx::Vector3dF(0.0f, 0.0f, -1.0f),

chrome/browser/vr/elements/reticle.h

@@ -18,6 +18,8 @@ class Reticle : public UiElement {
   Reticle(UiScene* scene, Model* model);
   ~Reticle() override;
 
+  UiElement* TargetElement() const;
+
  private:
   void Render(UiElementRenderer* renderer,
               const gfx::Transform& model_view_proj_matrix) const final;

chrome/browser/vr/ui_renderer_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/time/time.h"
 #include "chrome/browser/vr/elements/ui_element.h"
+#include "chrome/browser/vr/model/model.h"
 #include "chrome/browser/vr/test/animation_utils.h"
 #include "chrome/browser/vr/test/constants.h"
 #include "chrome/browser/vr/test/ui_scene_manager_test.h"
@@ -35,7 +36,34 @@ class UiRendererTest : public UiSceneManagerTest,
   }
 };
 
+TEST_F(UiRendererTest, ReticleStacking) {
+  UiElement* content = scene_->GetUiElementByName(kContentQuad);
+  EXPECT_TRUE(content);
+  model_->reticle.target_element_id = content->id();
+  auto unsorted = scene_->GetVisible2dBrowsingElements();
+  auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
+  bool saw_target = false;
+  for (auto* e : sorted) {
+    if (e == content) {
+      saw_target = true;
+    } else if (saw_target) {
+      EXPECT_EQ(kReticle, e->name());
+      break;
+    }
+  }
+
+  auto controller_elements = scene_->GetVisibleControllerElements();
+  bool saw_reticle = false;
+  for (auto* e : controller_elements) {
+    if (e->name() == kReticle) {
+      saw_reticle = true;
+    }
+  }
+  EXPECT_FALSE(saw_reticle);
+}
+
 TEST_P(UiRendererTest, UiRendererSortingTest) {
+  model_->reticle.target_element_id = 0;
   auto unsorted = ((*scene_).*GetParam().f)();
   auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
 

chrome/browser/vr/ui_scene.cc

@@ -14,6 +14,7 @@
 #include "base/values.h"
 #include "chrome/browser/vr/databinding/binding_base.h"
 #include "chrome/browser/vr/elements/draw_phase.h"
+#include "chrome/browser/vr/elements/reticle.h"
 #include "chrome/browser/vr/elements/ui_element.h"
 #include "ui/gfx/transform.h"
 
@@ -22,11 +23,18 @@ namespace vr {
 namespace {
 
 template <typename P>
-UiScene::Elements GetVisibleElements(UiElement* root, P predicate) {
+UiScene::Elements GetVisibleElements(UiElement* root,
+                                     UiElement* reticle_element,
+                                     P predicate) {
+  Reticle* reticle = static_cast<Reticle*>(reticle_element);
+  UiElement* target = reticle ? reticle->TargetElement() : nullptr;
   UiScene::Elements elements;
   for (auto& element : *root) {
-    if (element.IsVisible() && predicate(&element))
+    if (element.IsVisible() && predicate(&element)) {
       elements.push_back(&element);
+      if (target && target->id() == element.id())
+        elements.push_back(reticle);
+    }
   }
   return elements;
 }
@@ -182,7 +190,8 @@ UiElement* UiScene::GetUiElementByName(UiElementName name) const {
 
 UiScene::Elements UiScene::GetVisible2dBrowsingElements() const {
   return GetVisibleElements(
-      GetUiElementByName(k2dBrowsingRoot), [](UiElement* element) {
+      GetUiElementByName(k2dBrowsingRoot), GetUiElementByName(kReticle),
+      [](UiElement* element) {
         return element->draw_phase() == kPhaseForeground ||
                element->draw_phase() == kPhaseFloorCeiling ||
                element->draw_phase() == kPhaseBackground;
@@ -191,7 +200,8 @@ UiScene::Elements UiScene::GetVisible2dBrowsingElements() const {
 
 UiScene::Elements UiScene::GetVisible2dBrowsingOverlayElements() const {
   return GetVisibleElements(
-      GetUiElementByName(k2dBrowsingRoot), [](UiElement* element) {
+      GetUiElementByName(k2dBrowsingRoot), GetUiElementByName(kReticle),
+      [](UiElement* element) {
         return element->draw_phase() == kPhaseOverlayBackground ||
                element->draw_phase() == kPhaseOverlayForeground;
       });
@@ -199,7 +209,8 @@ UiScene::Elements UiScene::GetVisible2dBrowsingOverlayElements() const {
 
 UiScene::Elements UiScene::GetVisibleSplashScreenElements() const {
   return GetVisibleElements(
-      GetUiElementByName(kSplashScreenRoot), [](UiElement* element) {
+      GetUiElementByName(kSplashScreenRoot), GetUiElementByName(kReticle),
+      [](UiElement* element) {
         return element->draw_phase() == kPhaseOverlayBackground ||
                element->draw_phase() == kPhaseOverlayForeground;
       });
@@ -207,16 +218,23 @@ UiScene::Elements UiScene::GetVisibleSplashScreenElements() const {
 
 UiScene::Elements UiScene::GetVisibleWebVrOverlayForegroundElements() const {
   return GetVisibleElements(
-      GetUiElementByName(kWebVrRoot), [](UiElement* element) {
+      GetUiElementByName(kWebVrRoot), GetUiElementByName(kReticle),
+      [](UiElement* element) {
         return element->draw_phase() == kPhaseOverlayForeground;
       });
 }
 
 UiScene::Elements UiScene::GetVisibleControllerElements() const {
-  return GetVisibleElements(GetUiElementByName(kControllerGroup),
-                            [](UiElement* element) {
-                              return element->draw_phase() == kPhaseForeground;
-                            });
+  return GetVisibleElements(
+      GetUiElementByName(kControllerGroup), nullptr, [](UiElement* element) {
+        if (element->name() == kReticle) {
+          Reticle* reticle = static_cast<Reticle*>(element);
+          // If the reticle has a non-null target element,
+          // it would have been positioned elsewhere.
+          return !reticle->TargetElement();
+        }
+        return element->draw_phase() == kPhaseForeground;
+      });
 }
 
 UiScene::UiScene() {