[vr] Add UiElement iterators

With this CL you can now iterate across elements both forward and
backward (for rendering and hit testing, respectively).

Bug: None
Change-Id: Iadc1f43353ac5ed8cef550103bac223129802035
Reviewed-on: https://chromium-review.googlesource.com/672605
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502926}

chrome/browser/vr/BUILD.gn

@@ -73,6 +73,7 @@ static_library("vr_common") {
     "elements/transience_manager.h",
     "elements/ui_element.cc",
     "elements/ui_element.h",
+    "elements/ui_element_iterator.h",
     "elements/ui_element_name.h",
     "elements/ui_element_transform_operations.cc",
     "elements/ui_element_transform_operations.h",
@@ -154,6 +155,7 @@ test("vr_common_unittests") {
     "elements/exit_prompt_unittest.cc",
     "elements/linear_layout_unittest.cc",
     "elements/transience_manager_unittest.cc",
+    "elements/ui_element_iterator_unittest.cc",
     "elements/ui_element_unittest.cc",
     "elements/url_bar_texture_unittest.cc",
     "elements/viewport_aware_root_unittest.cc",

chrome/browser/vr/elements/ui_element.cc

@@ -36,14 +36,6 @@ bool GetRayPlaneDistance(const gfx::Point3F& ray_origin,
   return true;
 }
 
-template <typename T>
-void GetAllElementsInSubtree(T* e, std::vector<T*>* elements) {
-  elements->push_back(e);
-  for (auto& child : e->children()) {
-    GetAllElementsInSubtree<T>(child.get(), elements);
-  }
-}
-
 }  // namespace
 
 UiElement::UiElement() : id_(AllocateId()) {
@@ -191,18 +183,6 @@ void UiElement::SetMode(ColorScheme::Mode mode) {
   OnSetMode();
 }
 
-std::vector<UiElement*> UiElement::AllElementsInSubtree() {
-  std::vector<UiElement*> elements;
-  GetAllElementsInSubtree(this, &elements);
-  return elements;
-}
-
-std::vector<const UiElement*> UiElement::AllElementsInSubtree() const {
-  std::vector<const UiElement*> elements;
-  GetAllElementsInSubtree(this, &elements);
-  return elements;
-}
-
 void UiElement::OnSetMode() {}
 void UiElement::OnUpdatedInheritedProperties() {}
 

chrome/browser/vr/elements/ui_element.h

@@ -15,6 +15,7 @@
 #include "chrome/browser/vr/animation_player.h"
 #include "chrome/browser/vr/color_scheme.h"
 #include "chrome/browser/vr/elements/draw_phase.h"
+#include "chrome/browser/vr/elements/ui_element_iterator.h"
 #include "chrome/browser/vr/elements/ui_element_name.h"
 #include "chrome/browser/vr/target_property.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -191,6 +192,7 @@ class UiElement : public cc::AnimationTarget {
   void AddChild(std::unique_ptr<UiElement> child);
   void RemoveChild(UiElement* child);
   UiElement* parent() { return parent_; }
+  const UiElement* parent() const { return parent_; }
 
   gfx::Point3F GetCenter() const;
   gfx::Vector3dF GetNormal() const;
@@ -249,8 +251,37 @@ class UiElement : public cc::AnimationTarget {
     return children_;
   }
 
-  std::vector<UiElement*> AllElementsInSubtree();
-  std::vector<const UiElement*> AllElementsInSubtree() const;
+  typedef ForwardUiElementIterator iterator;
+  typedef ConstForwardUiElementIterator const_iterator;
+  typedef ReverseUiElementIterator reverse_iterator;
+  typedef ConstReverseUiElementIterator const_reverse_iterator;
+
+  iterator begin() { return iterator(this); }
+  iterator end() { return iterator(nullptr); }
+  const_iterator begin() const { return const_iterator(this); }
+  const_iterator end() const { return const_iterator(nullptr); }
+
+  reverse_iterator rbegin() { return reverse_iterator(this); }
+  reverse_iterator rend() { return reverse_iterator(nullptr); }
+  const_reverse_iterator rbegin() const { return const_reverse_iterator(this); }
+  const_reverse_iterator rend() const {
+    return const_reverse_iterator(nullptr);
+  }
+
+  template <typename T>
+  struct Reversed {
+    explicit Reversed(T* root) : root(root) {}
+    reverse_iterator begin() { return root->rbegin(); }
+    reverse_iterator end() { return root->rend(); }
+    const_reverse_iterator begin() const { return root->rbegin(); }
+    const_reverse_iterator end() const { return root->rend(); }
+    T* root;
+  };
+
+  Reversed<UiElement> reversed() { return Reversed<UiElement>(this); }
+  const Reversed<const UiElement> reversed() const {
+    return Reversed<const UiElement>(this);
+  }
 
  protected:
   virtual void OnSetMode();

chrome/browser/vr/elements/ui_element_iterator.h

+// 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.
+
+#ifndef CHROME_BROWSER_VR_ELEMENTS_UI_ELEMENT_ITERATOR_H_
+#define CHROME_BROWSER_VR_ELEMENTS_UI_ELEMENT_ITERATOR_H_
+
+#include <vector>
+
+namespace vr {
+
+class UiElement;
+
+// The UiElementIterator traverses a UiElement subtree. Do not use this class
+// directly. You should, instead, use UiElement::end/begin/rend/rbegin (or the
+// convenience functions on UiScene).
+template <typename T, typename U>
+class UiElementIterator {
+ public:
+  explicit UiElementIterator(T* root) {
+    current_ = U::Increment(&index_stack_, root, true);
+  }
+  ~UiElementIterator() {}
+
+  void operator++() { current_ = U::Increment(&index_stack_, current_, false); }
+  void operator++(int) {
+    current_ = U::Increment(&index_stack_, current_, false);
+  }
+  T& operator*() { return *current_; }
+  bool operator!=(const UiElementIterator& rhs) {
+    return current_ != rhs.current_ ||
+           index_stack_.empty() != rhs.index_stack_.empty();
+  }
+
+ private:
+  T* current_ = nullptr;
+
+  // The iterator maintains a stack of indices which represent the current index
+  // in each list of children along the ancestor path.
+  std::vector<size_t> index_stack_;
+};
+
+template <typename T>
+struct ForwardIncrementer {
+  static T* Increment(std::vector<size_t>* index_stack, T* e, bool init) {
+    if (!e || init)
+      return e;
+
+    if (!e->children().empty()) {
+      index_stack->push_back(0lu);
+      return e->children().front().get();
+    }
+
+    while (e->parent() && !index_stack->empty() &&
+           index_stack->back() + 1lu >= e->parent()->children().size()) {
+      index_stack->pop_back();
+      e = e->parent();
+    }
+
+    if (!e->parent() || index_stack->empty())
+      return nullptr;
+
+    index_stack->back()++;
+    return e->parent()->children()[index_stack->back()].get();
+  }
+};
+
+template <typename T>
+struct ReverseIncrementer {
+  static T* Increment(std::vector<size_t>* index_stack, T* e, bool init) {
+    if (!e || (index_stack->empty() && !init))
+      return nullptr;
+
+    bool should_descend = false;
+    if (index_stack->empty()) {
+      should_descend = true;
+    } else if (e->parent() && index_stack->back() > 0lu) {
+      index_stack->back()--;
+      e = e->parent()->children()[index_stack->back()].get();
+      should_descend = true;
+    }
+
+    if (should_descend) {
+      while (!e->children().empty()) {
+        index_stack->push_back(e->children().size() - 1lu);
+        e = e->children().back().get();
+      }
+      return e;
+    }
+
+    index_stack->pop_back();
+    return e->parent();
+  }
+};
+
+typedef UiElementIterator<UiElement, ForwardIncrementer<UiElement>>
+    ForwardUiElementIterator;
+typedef UiElementIterator<const UiElement, ForwardIncrementer<const UiElement>>
+    ConstForwardUiElementIterator;
+typedef UiElementIterator<UiElement, ReverseIncrementer<UiElement>>
+    ReverseUiElementIterator;
+typedef UiElementIterator<const UiElement, ReverseIncrementer<const UiElement>>
+    ConstReverseUiElementIterator;
+
+}  // namespace vr
+
+#endif  // CHROME_BROWSER_VR_ELEMENTS_UI_ELEMENT_ITERATOR_H_

chrome/browser/vr/elements/ui_element_iterator_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/elements/ui_element.h"
+
+#include "base/memory/ptr_util.h"
+#include "base/test/gtest_util.h"
+#include "chrome/browser/vr/ui_scene.h"
+
+namespace vr {
+
+namespace {
+
+// Constructs a tree of the following form
+// 1 kRoot
+//   2 k2dBrowsingRoot
+//     3 kFloor
+//     4 k2dBrowsingContentGroup
+//       5 kBackplane
+//       6 kContentQuad
+//       7 kUrlBar
+//     8 kCeiling
+void MakeTree(UiScene* scene) {
+  auto element = base::MakeUnique<UiElement>();
+  element->set_name(k2dBrowsingRoot);
+  scene->AddUiElement(kRoot, std::move(element));
+
+  element = base::MakeUnique<UiElement>();
+  element->set_name(kFloor);
+  scene->AddUiElement(k2dBrowsingRoot, std::move(element));
+
+  element = base::MakeUnique<UiElement>();
+  element->set_name(k2dBrowsingContentGroup);
+  scene->AddUiElement(k2dBrowsingRoot, std::move(element));
+
+  element = base::MakeUnique<UiElement>();
+  element->set_name(kBackplane);
+  scene->AddUiElement(k2dBrowsingContentGroup, std::move(element));
+
+  element = base::MakeUnique<UiElement>();
+  element->set_name(kContentQuad);
+  scene->AddUiElement(k2dBrowsingContentGroup, std::move(element));
+
+  element = base::MakeUnique<UiElement>();
+  element->set_name(kUrlBar);
+  scene->AddUiElement(k2dBrowsingContentGroup, std::move(element));
+
+  element = base::MakeUnique<UiElement>();
+  element->set_name(kCeiling);
+  scene->AddUiElement(k2dBrowsingRoot, std::move(element));
+}
+
+template <typename T>
+void CollectElements(T* e, std::vector<T*>* elements) {
+  elements->push_back(e);
+  for (auto& child : e->children()) {
+    CollectElements(child.get(), elements);
+  }
+}
+
+}  // namespace
+
+struct UiElementIteratorTestCase {
+  UiElementName root;
+  size_t num_elements_in_subtree;
+};
+
+class UiElementIteratorTest
+    : public ::testing::TestWithParam<UiElementIteratorTestCase> {};
+
+TEST_P(UiElementIteratorTest, VerifyTraversal) {
+  UiScene scene;
+  MakeTree(&scene);
+  std::vector<UiElement*> elements;
+  CollectElements(scene.GetUiElementByName(GetParam().root), &elements);
+
+  size_t i = 0;
+  for (auto& e : *(scene.GetUiElementByName(GetParam().root))) {
+    EXPECT_GT(elements.size(), i);
+    EXPECT_EQ(elements[i++]->id(), e.id());
+  }
+  EXPECT_EQ(elements.size(), i);
+  EXPECT_EQ(GetParam().num_elements_in_subtree, i);
+
+  i = 0;
+  for (auto& e : *const_cast<const UiElement*>(
+           scene.GetUiElementByName(GetParam().root))) {
+    EXPECT_GT(elements.size(), i);
+    EXPECT_EQ(elements[i++]->id(), e.id());
+  }
+  EXPECT_EQ(elements.size(), i);
+  EXPECT_EQ(GetParam().num_elements_in_subtree, i);
+
+  // Since the reversed iteration happens using a different type
+  // (UiElement::Reversed), we'll do that separately here rather than using
+  // conditionals and another test param.
+  i = 0;
+  for (auto& e : scene.GetUiElementByName(GetParam().root)->reversed()) {
+    EXPECT_GT(elements.size(), i);
+    EXPECT_EQ(elements[elements.size() - i++ - 1lu]->id(), e.id());
+  }
+  EXPECT_EQ(elements.size(), i);
+  EXPECT_EQ(GetParam().num_elements_in_subtree, i);
+
+  i = 0;
+  for (auto& e :
+       const_cast<const UiElement*>(scene.GetUiElementByName(GetParam().root))
+           ->reversed()) {
+    EXPECT_GT(elements.size(), i);
+    EXPECT_EQ(elements[elements.size() - i++ - 1lu]->id(), e.id());
+  }
+  EXPECT_EQ(elements.size(), i);
+  EXPECT_EQ(GetParam().num_elements_in_subtree, i);
+}
+
+const std::vector<UiElementIteratorTestCase> iterator_test_cases = {
+    {kRoot, 8},
+    {k2dBrowsingContentGroup, 4},
+    {kCeiling, 1},
+};
+
+INSTANTIATE_TEST_CASE_P(UiElementIteratorTestCases,
+                        UiElementIteratorTest,
+                        ::testing::ValuesIn(iterator_test_cases));
+
+}  // namespace vr

chrome/browser/vr/ui_scene.cc

@@ -18,35 +18,13 @@ namespace vr {
 
 namespace {
 
-template <typename F>
-void ForAllElements(UiElement* e, F f) {
-  f(e);
-  for (auto& child : e->children()) {
-    ForAllElements(child.get(), f);
-  }
-}
-
-template <typename P>
-UiElement* FindElement(UiElement* e, P predicate) {
-  if (predicate(e)) {
-    return e;
-  }
-  for (const auto& child : e->children()) {
-    if (UiElement* match = FindElement(child.get(), predicate)) {
-      return match;
-    }
-  }
-  return nullptr;
-}
-
 template <typename P>
-UiScene::Elements GetVisibleElements(UiElement* e, P predicate) {
+UiScene::Elements GetVisibleElements(UiElement* root, P predicate) {
   UiScene::Elements elements;
-  ForAllElements(e, [&elements, predicate](UiElement* element) {
-    if (element->IsVisible() && predicate(element)) {
-      elements.push_back(element);
-    }
-  });
+  for (auto& element : *root) {
+    if (element.IsVisible() && predicate(&element))
+      elements.push_back(&element);
+  }
   return elements;
 }
 
@@ -82,22 +60,22 @@ void UiScene::RemoveAnimation(int element_id, int animation_id) {
 
 void UiScene::OnBeginFrame(const base::TimeTicks& current_time,
                            const gfx::Vector3dF& look_at) {
-  ForAllElements(root_element_.get(), [current_time](UiElement* element) {
+  for (auto& element : *root_element_) {
     // Process all animations before calculating object transforms.
-    element->Animate(current_time);
-  });
+    element.Animate(current_time);
+  }
 
-  ForAllElements(root_element_.get(), [look_at](UiElement* element) {
-    element->LayOutChildren();
-    element->AdjustRotationForHeadPose(look_at);
-  });
+  for (auto& element : *root_element_) {
+    element.LayOutChildren();
+    element.AdjustRotationForHeadPose(look_at);
+  }
 
   root_element_->UpdateInheritedProperties();
 }
 
 void UiScene::PrepareToDraw() {
-  ForAllElements(root_element_.get(),
-                 [](UiElement* element) { element->PrepareToDraw(); });
+  for (auto& element : *root_element_)
+    element.PrepareToDraw();
 }
 
 UiElement& UiScene::root_element() {
@@ -105,16 +83,19 @@ UiElement& UiScene::root_element() {
 }
 
 UiElement* UiScene::GetUiElementById(int element_id) const {
-  return FindElement(root_element_.get(), [element_id](UiElement* element) {
-    return element->id() == element_id;
-  });
+  for (auto& element : *root_element_) {
+    if (element.id() == element_id)
+      return &element;
+  }
+  return nullptr;
 }
 
 UiElement* UiScene::GetUiElementByName(UiElementName name) const {
-  DCHECK(name != UiElementName::kNone);
-  return FindElement(root_element_.get(), [name](UiElement* element) {
-    return element->name() == name;
-  });
+  for (auto& element : *root_element_) {
+    if (element.name() == name)
+      return &element;
+  }
+  return nullptr;
 }
 
 UiScene::Elements UiScene::GetVisible2dBrowsingElements() const {
@@ -162,8 +143,8 @@ UiScene::~UiScene() = default;
 // initialize when the binding fires, automatically.
 void UiScene::OnGlInitialized() {
   gl_initialized_ = true;
-  ForAllElements(root_element_.get(),
-                 [](UiElement* element) { element->Initialize(); });
+  for (auto& element : *root_element_)
+    element.Initialize();
 }
 
 }  // namespace vr

chrome/browser/vr/ui_scene.h

@@ -11,6 +11,8 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/vr/color_scheme.h"
+#include "chrome/browser/vr/elements/ui_element.h"
+#include "chrome/browser/vr/elements/ui_element_iterator.h"
 #include "chrome/browser/vr/elements/ui_element_name.h"
 #include "third_party/skia/include/core/SkColor.h"
 
@@ -32,15 +34,6 @@ class UiElement;
 
 class UiScene {
  public:
-  enum Command {
-    ADD_ELEMENT,
-    UPDATE_ELEMENT,
-    REMOVE_ELEMENT,
-    ADD_ANIMATION,
-    REMOVE_ANIMATION,
-    CONFIGURE_SCENE,
-  };
-
   UiScene();
   virtual ~UiScene();
 
@@ -98,6 +91,23 @@ class UiScene {
 
   void OnGlInitialized();
 
+  UiElement::iterator begin() { return root_element_->begin(); }
+  UiElement::iterator end() { return root_element_->end(); }
+
+  UiElement::const_iterator begin() const {
+    return const_cast<const UiElement&>(*root_element_).begin();
+  }
+  UiElement::const_iterator end() const {
+    return const_cast<const UiElement&>(*root_element_).end();
+  }
+
+  UiElement::Reversed<UiElement> reversed() {
+    return UiElement::Reversed<UiElement>(root_element_.get());
+  }
+  const UiElement::Reversed<const UiElement> reversed() const {
+    return UiElement::Reversed<const UiElement>(root_element_.get());
+  }
+
  private:
   void Animate(const base::TimeTicks& current_time);
 

chrome/browser/vr/ui_scene_manager_unittest.cc

@@ -500,11 +500,11 @@ TEST_F(UiSceneManagerTest, UiUpdatesForWebVR) {
   manager_->SetBluetoothConnectedIndicator(true);
 
   auto* web_vr_root = scene_->GetUiElementByName(kWebVrRoot);
-  for (auto* element : web_vr_root->AllElementsInSubtree()) {
-    SCOPED_TRACE(element->name());
-    EXPECT_TRUE(element->draw_phase() == kPhaseNone ||
-                element->draw_phase() == kPhaseOverlayBackground ||
-                element->draw_phase() == kPhaseOverlayForeground);
+  for (auto& element : *web_vr_root) {
+    SCOPED_TRACE(element.name());
+    EXPECT_TRUE(element.draw_phase() == kPhaseNone ||
+                element.draw_phase() == kPhaseOverlayBackground ||
+                element.draw_phase() == kPhaseOverlayForeground);
   }
 
   // All elements should be hidden.