Improve Accuracy/Correctness of AR Hit Tests

By default, ArCore returns hits for planes which are much larger than the actual area detected by
ArCore. This can cause objects to appear to be floating off the edge of a table for example. This
CL changes the algorithm to request ArCore to do a more exact check on planar trackables.

The code also limits hits to planar trackables only, which removes some spurious hits with Point
Clouds. As more trackable types are supported by ArCore we should probably revist this.

    Adjust the current hitTest code in the following ways:
    1) Ignore all hits on trackables other than Planes
    2) Walk the hits returned from AR Core in back to front order
    3) When we have found at least one hit, then restrict any subsequent hits we return to those
       that are within the polygon which ArCore detected for the plane. We assume the first hit
       is the floor and likely to extend beyond the currently detected area.

Bug: 872855
Change-Id: I2649bf10d16b09152f077cd8b9fb2c6fe261162c
Reviewed-on: https://chromium-review.googlesource.com/c/1173446
Commit-Queue: Brice Tebbs <btebbs@chromium.org>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596081}

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

@@ -251,8 +251,16 @@ bool ARCoreImpl::RequestHitTest(
   ArHitResultList_getSize(arcore_session_.get(), arcore_hit_result_list.get(),
                           &arcore_hit_result_list_size);
 
-  for (int i = 0; i < arcore_hit_result_list_size; i++) {
+  // Go through the list in reverse order so the first hit we encounter is the
+  // furthest.
+  // We will accept the furthest hit and then for the rest require that the hit
+  // be within the actual polygon detected by ArCore. This heuristic allows us
+  // to get better results on floors w/o overestimating the size of tables etc.
+  // See https://crbug.com/872855.
+
+  for (int i = arcore_hit_result_list_size - 1; i >= 0; i--) {
     internal::ScopedArCoreObject<ArHitResult*> arcore_hit;
+
     ArHitResult_create(arcore_session_.get(), arcore_hit.receive());
     if (!arcore_hit.is_valid()) {
       DLOG(ERROR) << "ArHitResult_create failed!";
@@ -262,11 +270,48 @@ bool ARCoreImpl::RequestHitTest(
     ArHitResultList_getItem(arcore_session_.get(), arcore_hit_result_list.get(),
                             i, arcore_hit.get());
 
-    mojom::XRHitResultPtr mojo_hit = mojom::XRHitResult::New();
-    if (!ArHitResultToXRHitResult(arcore_hit.get(), mojo_hit.get())) {
+    internal::ScopedArCoreObject<ArTrackable*> ar_trackable;
+
+    ArHitResult_acquireTrackable(arcore_session_.get(), arcore_hit.get(),
+                                 ar_trackable.receive());
+    ArTrackableType ar_trackable_type = AR_TRACKABLE_NOT_VALID;
+    ArTrackable_getType(arcore_session_.get(), ar_trackable.get(),
+                        &ar_trackable_type);
+
+    // Only consider hits with plane trackables
+    // TODO(874985): make this configurable or re-evaluate this decision
+    if (AR_TRACKABLE_PLANE != ar_trackable_type) {
+      continue;
+    }
+
+    internal::ScopedArCoreObject<ArPose*> arcore_pose;
+    ArPose_create(arcore_session_.get(), nullptr, arcore_pose.receive());
+    if (!arcore_pose.is_valid()) {
+      DLOG(ERROR) << "ArPose_create failed!";
       return false;
     }
-    hit_results->push_back(std::move(mojo_hit));
+
+    ArHitResult_getHitPose(arcore_session_.get(), arcore_hit.get(),
+                           arcore_pose.get());
+
+    // After the first (furthest) hit, only return hits that are within the
+    // actual detected polygon and not just within than the larger plane.
+    if (!hit_results->empty()) {
+      int32_t in_polygon = 0;
+      ArPlane* ar_plane = ArAsPlane(ar_trackable.get());
+      ArPlane_isPoseInPolygon(arcore_session_.get(), ar_plane,
+                              arcore_pose.get(), &in_polygon);
+      if (!in_polygon)
+        continue;
+    }
+
+    mojom::XRHitResultPtr mojo_hit = mojom::XRHitResult::New();
+    mojo_hit.get()->hit_matrix.resize(16);
+    ArPose_getMatrix(arcore_session_.get(), arcore_pose.get(),
+                     mojo_hit.get()->hit_matrix.data());
+
+    // Insert new results at head to preserver order from ArCore
+    hit_results->insert(hit_results->begin(), std::move(mojo_hit));
   }
   return true;
 }
@@ -325,26 +370,6 @@ bool ARCoreImpl::TransformRayToScreenSpace(const mojom::XRRayPtr& ray,
   return true;
 }
 
-bool ARCoreImpl::ArHitResultToXRHitResult(ArHitResult* arcore_hit,
-                                          mojom::XRHitResult* hit_result) {
-  DCHECK(IsOnGlThread());
-  DCHECK(arcore_session_.is_valid());
-  DCHECK(arcore_frame_.is_valid());
-
-  internal::ScopedArCoreObject<ArPose*> arcore_pose;
-  ArPose_create(arcore_session_.get(), nullptr, arcore_pose.receive());
-  if (!arcore_pose.is_valid()) {
-    DLOG(ERROR) << "ArPose_create failed!";
-    return false;
-  }
-  ArHitResult_getHitPose(arcore_session_.get(), arcore_hit, arcore_pose.get());
-  hit_result->hit_matrix.resize(16);
-  ArPose_getMatrix(arcore_session_.get(), arcore_pose.get(),
-                   hit_result->hit_matrix.data());
-
-  return true;
-}
-
 bool ARCoreImpl::IsOnGlThread() {
   return gl_thread_task_runner_->BelongsToCurrentThread();
 }

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

@@ -89,9 +89,6 @@ class ARCoreImpl : public ARCore {
                                  const gfx::Size& image_size,
                                  gfx::PointF* screen_point);
 
-  bool ArHitResultToXRHitResult(ArHitResult* ar_hit,
-                                mojom::XRHitResult* hit_result);
-
   bool IsOnGlThread();
   base::WeakPtr<ARCoreImpl> GetWeakPtr() {
     return weak_ptr_factory_.GetWeakPtr();

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

@@ -42,6 +42,10 @@ namespace {
   CALL(ArSession_resume)                 \
   CALL(ArSession_setCameraTextureName)   \
   CALL(ArSession_setDisplayGeometry)     \
+  CALL(ArHitResult_acquireTrackable)     \
+  CALL(ArTrackable_getType)              \
+  CALL(ArTrackable_release)              \
+  CALL(ArPlane_isPoseInPolygon)          \
   CALL(ArSession_update)
 
 #define CALL(fn) decltype(&fn) impl_##fn = nullptr;
@@ -176,6 +180,31 @@ void ArHitResult_getHitPose(const ArSession* session,
   arcore_api->impl_ArHitResult_getHitPose(session, hit_result, out_pose);
 }
 
+void ArHitResult_acquireTrackable(const ArSession* session,
+                                  const ArHitResult* hit_result,
+                                  ArTrackable** out_trackable) {
+  arcore_api->impl_ArHitResult_acquireTrackable(session, hit_result,
+                                                out_trackable);
+}
+
+void ArTrackable_getType(const ArSession* session,
+                         const ArTrackable* trackable,
+                         ArTrackableType* out_trackable_type) {
+  arcore_api->impl_ArTrackable_getType(session, trackable, out_trackable_type);
+}
+
+void ArPlane_isPoseInPolygon(const ArSession* session,
+                             const ArPlane* plane,
+                             const ArPose* pose,
+                             int32_t* out_pose_in_polygon) {
+  arcore_api->impl_ArPlane_isPoseInPolygon(session, plane, pose,
+                                           out_pose_in_polygon);
+}
+
+void ArTrackable_release(ArTrackable* trackable) {
+  arcore_api->impl_ArTrackable_release(trackable);
+}
+
 void ArHitResultList_create(const ArSession* session,
                             ArHitResultList** out_hit_result_list) {
   arcore_api->impl_ArHitResultList_create(session, out_hit_result_list);