WebXR - anchor manager refactor

Refactor part 2/2. Previous part:
https://crrev.com/c/2087701

Separate anchor manager logic into "update" step and "obtain data" step,
similarly to the approach found in plane manager.

Change-Id: Ief886b53c1b933b7dae441bbe572b0302d82d28d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107564
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751563}

chrome/browser/android/vr/arcore_device/arcore_anchor_manager.cc

@@ -22,68 +22,34 @@ ArCoreAnchorManager::ArCoreAnchorManager(util::PassKey<ArCoreImpl> pass_key,
   ArPose_create(
       arcore_session_, nullptr,
       internal::ScopedArCoreObject<ArPose*>::Receiver(ar_pose_).get());
+  DCHECK(ar_pose_.is_valid());
 }
 
 ArCoreAnchorManager::~ArCoreAnchorManager() = default;
 
-mojom::XRAnchorsDataPtr ArCoreAnchorManager::GetAnchorsData(
-    ArFrame* arcore_frame) {
-  auto updated_anchors = GetUpdatedAnchorsData(arcore_frame);
-  auto all_anchor_ids = GetAllAnchorIds();
-
-  return mojom::XRAnchorsData::New(all_anchor_ids, std::move(updated_anchors));
-}
-
-std::vector<mojom::XRAnchorDataPtr> ArCoreAnchorManager::GetUpdatedAnchorsData(
-    ArFrame* arcore_frame) {
-  std::vector<mojom::XRAnchorDataPtr> result;
+mojom::XRAnchorsDataPtr ArCoreAnchorManager::GetAnchorsData() const {
+  std::vector<uint64_t> all_anchors_ids;
+  all_anchors_ids.reserve(anchor_id_to_anchor_object_.size());
+  for (const auto& anchor_id_and_object : anchor_id_to_anchor_object_) {
+    all_anchors_ids.push_back(anchor_id_and_object.first.GetUnsafeValue());
+  }
 
-  ArFrame_getUpdatedAnchors(arcore_session_, arcore_frame,
-                            arcore_anchors_.get());
+  std::vector<mojom::XRAnchorDataPtr> updated_anchors;
+  updated_anchors.reserve(updated_anchor_ids_.size());
+  for (const auto& anchor_id : updated_anchor_ids_) {
+    const device::internal::ScopedArCoreObject<ArAnchor*>& anchor =
+        anchor_id_to_anchor_object_.at(anchor_id);
 
-  ForEachArCoreAnchor(arcore_anchors_.get(), [this,
-                                              &result](ArAnchor* ar_anchor) {
     // pose
-
-    ArAnchor_getPose(arcore_session_, ar_anchor, ar_pose_.get());
+    ArAnchor_getPose(arcore_session_, anchor.get(), ar_pose_.get());
     mojom::Pose pose = GetMojomPoseFromArPose(arcore_session_, ar_pose_.get());
 
-    // ID
-    AnchorId anchor_id;
-    bool created;
-    std::tie(anchor_id, created) = CreateOrGetAnchorId(ar_anchor);
-
-    DCHECK(!created)
-        << "Anchor creation is app-initiated - we should never encounter an "
-           "anchor that was created outside of `ArCoreImpl::CreateAnchor()`.";
-
-    result.push_back(mojom::XRAnchorData::New(anchor_id.GetUnsafeValue(),
-                                              device::mojom::Pose::New(pose)));
-  });
-
-  return result;
-}
-
-std::vector<uint64_t> ArCoreAnchorManager::GetAllAnchorIds() {
-  std::vector<uint64_t> result;
-
-  ArSession_getAllAnchors(arcore_session_, arcore_anchors_.get());
-
-  ForEachArCoreAnchor(arcore_anchors_.get(), [this,
-                                              &result](ArAnchor* ar_anchor) {
-    // ID
-    AnchorId anchor_id;
-    bool created;
-    std::tie(anchor_id, created) = CreateOrGetAnchorId(ar_anchor);
-
-    DCHECK(!created)
-        << "Anchor creation is app-initiated - we should never encounter an "
-           "anchor that was created outside of `ArCoreImpl::CreateAnchor()`.";
-
-    result.emplace_back(anchor_id);
-  });
+    updated_anchors.push_back(mojom::XRAnchorData::New(
+        anchor_id.GetUnsafeValue(), device::mojom::Pose::New(pose)));
+  }
 
-  return result;
+  return mojom::XRAnchorsData::New(std::move(all_anchors_ids),
+                                   std::move(updated_anchors));
 }
 
 template <typename FunctionType>
@@ -108,10 +74,75 @@ void ArCoreAnchorManager::ForEachArCoreAnchor(ArAnchorList* arcore_anchors,
       continue;
     }
 
-    fn(anchor.get());
+    fn(std::move(anchor));
   }
 }
 
+void ArCoreAnchorManager::Update(ArFrame* ar_frame) {
+  // First, ask ARCore about all Anchors updated in the current frame.
+  ArFrame_getUpdatedAnchors(arcore_session_, ar_frame, arcore_anchors_.get());
+
+  // Collect the IDs of updated anchors.
+  std::set<AnchorId> updated_anchor_ids;
+  ForEachArCoreAnchor(arcore_anchors_.get(), [this, &updated_anchor_ids](
+                                                 device::internal::
+                                                     ScopedArCoreObject<
+                                                         ArAnchor*> ar_anchor) {
+    // ID
+    AnchorId anchor_id;
+    bool created;
+    std::tie(anchor_id, created) = CreateOrGetAnchorId(ar_anchor.get());
+
+    DCHECK(!created)
+        << "Anchor creation is app-initiated - we should never encounter an "
+           "anchor that was created outside of `ArCoreImpl::CreateAnchor()`.";
+
+    updated_anchor_ids.insert(anchor_id);
+  });
+
+  DVLOG(3) << __func__
+           << ": updated_anchor_ids.size()=" << updated_anchor_ids.size();
+
+  // Then, ask about all Anchors that are still tracked.
+  ArSession_getAllAnchors(arcore_session_, arcore_anchors_.get());
+
+  // Collect the objects of all currently tracked anchors.
+  // |ar_plane_address_to_id_| should *not* grow.
+  std::map<AnchorId, device::internal::ScopedArCoreObject<ArAnchor*>>
+      anchor_id_to_anchor_object;
+  ForEachArCoreAnchor(arcore_anchors_.get(), [this,
+                                              &anchor_id_to_anchor_object](
+                                                 device::internal::
+                                                     ScopedArCoreObject<
+                                                         ArAnchor*> ar_anchor) {
+    // ID
+    AnchorId anchor_id;
+    bool created;
+    std::tie(anchor_id, created) = CreateOrGetAnchorId(ar_anchor.get());
+
+    DCHECK(!created)
+        << "Anchor creation is app-initiated - we should never encounter an "
+           "anchor that was created outside of `ArCoreImpl::CreateAnchor()`.";
+
+    anchor_id_to_anchor_object[anchor_id] = std::move(ar_anchor);
+  });
+
+  DVLOG(3) << __func__ << ": anchor_id_to_anchor_object.size()="
+           << anchor_id_to_anchor_object.size();
+
+  // Shrink |ar_plane_address_to_id_|, removing all planes that are no longer
+  // tracked or were subsumed - if they do not show up in
+  // |plane_id_to_plane_object| map, they are no longer tracked.
+  base::EraseIf(
+      ar_anchor_address_to_id_,
+      [&anchor_id_to_anchor_object](const auto& anchor_address_and_id) {
+        return !base::Contains(anchor_id_to_anchor_object,
+                               anchor_address_and_id.second);
+      });
+  anchor_id_to_anchor_object_.swap(anchor_id_to_anchor_object);
+  updated_anchor_ids_.swap(updated_anchor_ids);
+}
+
 std::pair<AnchorId, bool> ArCoreAnchorManager::CreateOrGetAnchorId(
     void* anchor_address) {
   auto it = ar_anchor_address_to_id_.find(anchor_address);
@@ -162,7 +193,8 @@ base::Optional<AnchorId> ArCoreAnchorManager::CreateAnchor(
 
   DVLOG(2) << __func__ << ": plane_id=" << plane_id;
 
-  auto ar_anchor = plane_manager->CreateAnchor(plane_id, pose);
+  auto ar_anchor = plane_manager->CreateAnchor(
+      util::PassKey<ArCoreAnchorManager>(), plane_id, pose);
   if (!ar_anchor.is_valid()) {
     return base::nullopt;
   }

chrome/browser/android/vr/arcore_device/arcore_anchor_manager.h

@@ -31,7 +31,7 @@ class ArCoreAnchorManager {
   // anchors.
   void Update(ArFrame* ar_frame);
 
-  mojom::XRAnchorsDataPtr GetAnchorsData(ArFrame* arcore_frame);
+  mojom::XRAnchorsDataPtr GetAnchorsData() const;
 
   bool AnchorExists(AnchorId id) const;
 
@@ -50,19 +50,11 @@ class ArCoreAnchorManager {
 
  private:
   // Executes |fn| for each still tracked, anchor present in |arcore_anchors|.
-  // |fn| will receive a non-owning `ArAnchor*`.
+  // |fn| will receive a `device::internal::ScopedArCoreObject<ArAnchor*>` that
+  // can be stored.
   template <typename FunctionType>
   void ForEachArCoreAnchor(ArAnchorList* arcore_anchors, FunctionType fn);
 
-  // Returns vector containing information about all anchors updated in the
-  // current frame.
-  std::vector<mojom::XRAnchorDataPtr> GetUpdatedAnchorsData(
-      ArFrame* arcore_frame);
-
-  // The result will contain IDs of all anchors still tracked in the current
-  // frame.
-  std::vector<uint64_t> GetAllAnchorIds();
-
   // Owned by ArCoreImpl - non-owning pointer is fine since ArCoreAnchorManager
   // is also owned by ArCoreImpl.
   ArSession* arcore_session_;

chrome/browser/android/vr/arcore_device/arcore_impl.cc

@@ -10,7 +10,6 @@
 #include "base/optional.h"
 #include "base/trace_event/trace_event.h"
 #include "base/util/type_safety/pass_key.h"
-#include "chrome/browser/android/vr/arcore_device/arcore_java_utils.h"
 #include "chrome/browser/android/vr/arcore_device/arcore_plane_manager.h"
 #include "chrome/browser/android/vr/arcore_device/type_converters.h"
 #include "device/vr/public/mojom/vr_service.mojom.h"
@@ -459,6 +458,10 @@ mojom::VRPosePtr ArCoreImpl::Update(bool* camera_updated) {
   plane_manager_->Update(arcore_frame_.get());
   TRACE_EVENT_END0("gpu", "ArCorePlaneManager Update");
 
+  TRACE_EVENT_BEGIN0("gpu", "ArCoreAnchorManager Update");
+  anchor_manager_->Update(arcore_frame_.get());
+  TRACE_EVENT_END0("gpu", "ArCoreAnchorManager Update");
+
   return GetMojomVRPoseFromArPose(arcore_session_.get(), arcore_pose.get());
 }
 
@@ -475,7 +478,7 @@ mojom::XRAnchorsDataPtr ArCoreImpl::GetAnchorsData() {
 
   TRACE_EVENT0("gpu", __func__);
 
-  return anchor_manager_->GetAnchorsData(arcore_frame_.get());
+  return anchor_manager_->GetAnchorsData();
 }
 
 mojom::XRLightEstimationDataPtr ArCoreImpl::GetLightEstimationData() {

chrome/browser/android/vr/arcore_device/arcore_plane_manager.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/android/vr/arcore_device/arcore_plane_manager.h"
 
 #include "base/stl_util.h"
-#include "base/trace_event/trace_event.h"
 #include "chrome/browser/android/vr/arcore_device/type_converters.h"
 
 namespace device {
@@ -199,8 +198,6 @@ void ArCorePlaneManager::Update(ArFrame* ar_frame) {
 
 mojom::XRPlaneDetectionDataPtr ArCorePlaneManager::GetDetectedPlanesData()
     const {
-  TRACE_EVENT0("gpu", __FUNCTION__);
-
   std::vector<uint64_t> all_plane_ids;
   all_plane_ids.reserve(plane_id_to_plane_object_.size());
   for (const auto& plane_id_and_object : plane_id_to_plane_object_) {
@@ -220,13 +217,8 @@ mojom::XRPlaneDetectionDataPtr ArCorePlaneManager::GetDetectedPlanesData()
     ArPlane_getType(arcore_session_, ar_plane, &plane_type);
 
     // pose
-    internal::ScopedArCoreObject<ArPose*> plane_pose;
-    ArPose_create(
-        arcore_session_, nullptr,
-        internal::ScopedArCoreObject<ArPose*>::Receiver(plane_pose).get());
-    ArPlane_getCenterPose(arcore_session_, ar_plane, plane_pose.get());
-    mojom::Pose pose =
-        GetMojomPoseFromArPose(arcore_session_, plane_pose.get());
+    ArPlane_getCenterPose(arcore_session_, ar_plane, ar_pose_.get());
+    mojom::Pose pose = GetMojomPoseFromArPose(arcore_session_, ar_pose_.get());
 
     // polygon
     int32_t polygon_size;
@@ -305,7 +297,8 @@ base::Optional<gfx::Transform> ArCorePlaneManager::GetMojoFromPlane(
 }
 
 device::internal::ScopedArCoreObject<ArAnchor*>
-ArCorePlaneManager::CreateAnchor(PlaneId id,
+ArCorePlaneManager::CreateAnchor(util::PassKey<ArCoreAnchorManager> pass_key,
+                                 PlaneId id,
                                  const device::mojom::Pose& pose) const {
   auto it = plane_id_to_plane_object_.find(id);
   if (it == plane_id_to_plane_object_.end()) {

chrome/browser/android/vr/arcore_device/arcore_plane_manager.h

@@ -16,6 +16,7 @@
 namespace device {
 
 class ArCoreImpl;
+class ArCoreAnchorManager;
 
 using PlaneId = util::IdTypeU64<class PlaneTag>;
 
@@ -55,8 +56,9 @@ class ArCorePlaneManager {
 
   // Creates Anchor object given a plane ID. This is needed since Plane objects
   // are managed by this class in its entirety and are not accessible outside
-  // it.
+  // it. Callable only from ArCoreAnchorManager.
   device::internal::ScopedArCoreObject<ArAnchor*> CreateAnchor(
+      util::PassKey<ArCoreAnchorManager> pass_key,
       PlaneId id,
       const device::mojom::Pose& pose) const;