[vr] Sort UiElements by tree order

Previously, we'd sorted along world-space z which provided a stable
ordering given that the elements did not move in world space in a way
that would upset the element ordering.

However, with viewport-aware elements, this has now changed. With this
change, we instead order the elements semantically. When we add effects
which require sorting by depth (i.e., where elements dynamically change
their ordering), we can augment UiRenderer::GetElementsInDrawOrder to
account for the view vector. Currently, however, this is not required.

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

chrome/browser/vr/BUILD.gn

@@ -226,6 +226,7 @@ test("vr_common_unittests") {
     "test/ui_scene_manager_test.h",
     "ui_input_manager_unittest.cc",
     "ui_pixeltest.cc",
+    "ui_renderer_unittest.cc",
     "ui_scene_manager_unittest.cc",
     "ui_scene_unittest.cc",
     "vr_gl_util_unittest.cc",

chrome/browser/vr/ui_input_manager_unittest.cc

@@ -11,12 +11,12 @@
 #include "chrome/browser/vr/content_input_delegate.h"
 #include "chrome/browser/vr/elements/ui_element.h"
 #include "chrome/browser/vr/test/animation_utils.h"
+#include "chrome/browser/vr/test/constants.h"
 #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 "testing/gmock/include/gmock/gmock.h"
 #include "third_party/WebKit/public/platform/WebGestureEvent.h"
-#include "ui/gfx/geometry/vector3d_f.h"
 
 namespace vr {
 
@@ -26,8 +26,7 @@ class UiInputManagerTest : public UiSceneManagerTest {
     UiSceneManagerTest::SetUp();
     MakeManager(kNotInCct, kNotInWebVr);
     input_manager_ = base::MakeUnique<UiInputManager>(scene_.get());
-    scene_->OnBeginFrame(MicrosecondsToTicks(1),
-                         gfx::Vector3dF(0.f, 0.f, -1.0f));
+    scene_->OnBeginFrame(MicrosecondsToTicks(1), kForwardVector);
   }
 
  protected:

chrome/browser/vr/ui_renderer.cc

@@ -123,8 +123,7 @@ void UiRenderer::DrawUiView(const RenderInfo& render_info,
                             UiRenderer::ReticleMode reticle_mode) {
   TRACE_EVENT0("gpu", "VrShellGl::DrawUiView");
 
-  auto sorted_elements =
-      GetElementsInDrawOrder(render_info.head_pose, elements);
+  auto sorted_elements = GetElementsInDrawOrder(elements);
 
   for (auto& eye_info :
        {render_info.left_eye_info, render_info.right_eye_info}) {
@@ -179,25 +178,16 @@ void UiRenderer::DrawElement(const gfx::Transform& view_proj_matrix,
 }
 
 std::vector<const UiElement*> UiRenderer::GetElementsInDrawOrder(
-    const gfx::Transform& view_matrix,
     const std::vector<const UiElement*>& elements) {
   std::vector<const UiElement*> sorted_elements = elements;
 
   // Sort elements primarily based on their draw phase (lower draw phase first)
-  // and secondarily based on their z-axis distance (more distant first).
-  // TODO(mthiesse, crbug.com/721356): 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->world_space_transform().matrix().get(2, 3) <
-                       second->world_space_transform().matrix().get(2, 3);
-              }
-            });
+  // and secondarily based on their tree order (as specified by the sorted
+  // |elements| vector).
+  std::stable_sort(sorted_elements.begin(), sorted_elements.end(),
+                   [](const UiElement* first, const UiElement* second) {
+                     return first->draw_phase() < second->draw_phase();
+                   });
 
   return sorted_elements;
 }

chrome/browser/vr/ui_renderer.h

@@ -60,6 +60,9 @@ class UiRenderer {
   void DrawWebVrOverlayForeground(const RenderInfo& render_info,
                                   const ControllerInfo& controller_info);
 
+  static std::vector<const UiElement*> GetElementsInDrawOrder(
+      const std::vector<const UiElement*>& elements);
+
  private:
   enum ReticleMode { kReticleVisible, kReticleHidden };
 
@@ -79,9 +82,6 @@ class UiRenderer {
                     ReticleMode reticle_mode);
   void DrawElement(const gfx::Transform& view_proj_matrix,
                    const UiElement& element);
-  std::vector<const UiElement*> GetElementsInDrawOrder(
-      const gfx::Transform& view_matrix,
-      const std::vector<const UiElement*>& elements);
   void DrawReticle(const gfx::Transform& render_matrix,
                    const RenderInfo& render_info,
                    const ControllerInfo& controller_info);

chrome/browser/vr/ui_renderer_unittest.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/vr/ui_renderer.h"
+
+#include <memory>
+
+#include "base/memory/ptr_util.h"
+#include "base/time/time.h"
+#include "chrome/browser/vr/elements/ui_element.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"
+#include "chrome/browser/vr/ui_scene.h"
+
+namespace vr {
+
+typedef UiScene::Elements (UiScene::*GeneratorFn)() const;
+struct TestParams {
+  GeneratorFn f;
+  std::vector<UiElementName> expected_order;
+};
+
+class UiRendererTest : public UiSceneManagerTest,
+                       public ::testing::WithParamInterface<TestParams> {
+ public:
+  void SetUp() override {
+    UiSceneManagerTest::SetUp();
+    MakeManager(kNotInCct, kNotInWebVr);
+    for (auto& e : scene_->root_element()) {
+      e.SetTransitionedProperties({});
+      e.SetVisible(true);
+    }
+    scene_->OnBeginFrame(MicrosecondsToTicks(1), kForwardVector);
+  }
+};
+
+TEST_P(UiRendererTest, UiRendererSortingTest) {
+  auto unsorted = ((*scene_).*GetParam().f)();
+  auto sorted = UiRenderer::GetElementsInDrawOrder(unsorted);
+
+  EXPECT_EQ(GetParam().expected_order.size(), sorted.size());
+
+  for (size_t i = 0; i < sorted.size(); ++i) {
+    EXPECT_EQ(GetParam().expected_order[i], sorted[i]->name());
+  }
+}
+
+TestParams params[] = {
+    {&UiScene::GetVisible2dBrowsingElements,
+     {
+         kBackgroundFront,
+         kBackgroundLeft,
+         kBackgroundBack,
+         kBackgroundRight,
+         kBackgroundTop,
+         kBackgroundBottom,
+         kFloor,
+         kCeiling,
+         kBackplane,
+         kContentQuad,
+         kAudioCaptureIndicator,
+         kVideoCaptureIndicator,
+         kScreenCaptureIndicator,
+         kBluetoothConnectedIndicator,
+         kLocationAccessIndicator,
+         kExitPrompt,
+         kExitPromptBackplane,
+         kUrlBar,
+         kUnderDevelopmentNotice,
+         kCloseButton,
+         kExclusiveScreenToast,
+
+     }},
+    {&UiScene::GetVisible2dBrowsingOverlayElements,
+     {
+         kScreenDimmer, kExitWarning,
+     }},
+    {&UiScene::GetVisibleSplashScreenElements,
+     {
+         kSplashScreenBackground, kSplashScreenText,
+     }},
+    {&UiScene::GetVisibleWebVrOverlayForegroundElements,
+     {
+         kWebVrUrlToast, kExclusiveScreenToastViewportAware,
+     }},
+};
+
+INSTANTIATE_TEST_CASE_P(SortingTests,
+                        UiRendererTest,
+                        testing::ValuesIn(params));
+
+}  // namespace vr