Fix XRObjectSpace::MojoFromNative(), XRSession::CreateAnchor()

Fixes XRObjectSpace::MojoFromNative() returning incorrect (inverted)
results. Fixes XRSession::CreateAnchor() incorrectly assuming that
passed in initial pose is expressed relative to mojo, and creating
resolver too early in the method.

Other minor refactorings:
- XRHitTestResult::getPose() - no functional changes, clarified naming
- XRObjectSpace now assumes that T has MojoFromObject() method
- Changed XRPlane and XRAnchor - renamed poseMatrix() to
MojoFromObject() to match the changed expectations in XRObjectSpace.

Fixed: 1032070, 1031684
Change-Id: I5aeefec31c2f501c56cff5b419af9bdff3eb1159
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1958428
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723412}

third_party/blink/renderer/modules/xr/xr_anchor.cc

@@ -49,7 +49,7 @@ XRSpace* XRAnchor::anchorSpace() const {
   return anchor_space_;
 }
 
-TransformationMatrix XRAnchor::poseMatrix() const {
+TransformationMatrix XRAnchor::MojoFromObject() const {
   if (anchor_data_) {
     return *anchor_data_->pose_matrix_;
   }

third_party/blink/renderer/modules/xr/xr_anchor.h

@@ -32,7 +32,7 @@ class XRAnchor : public ScriptWrappable {
 
   XRSpace* anchorSpace() const;
 
-  TransformationMatrix poseMatrix() const;
+  TransformationMatrix MojoFromObject() const;
 
   double lastChangedTime(bool& is_null) const;
 

third_party/blink/renderer/modules/xr/xr_hit_test_result.cc

@@ -13,19 +13,23 @@ namespace blink {
 XRHitTestResult::XRHitTestResult(const TransformationMatrix& pose)
     : pose_(std::make_unique<TransformationMatrix>(pose)) {}
 
-XRPose* XRHitTestResult::getPose(XRSpace* relative_to) {
-  DCHECK(relative_to->MojoFromNative());
+XRPose* XRHitTestResult::getPose(XRSpace* other) {
+  auto maybe_other_space_native_from_mojo = other->NativeFromMojo();
+  DCHECK(maybe_other_space_native_from_mojo);
 
-  auto mojo_from_this = *pose_;
+  auto mojo_from_this =
+      *pose_;  // Hit test results do not have origin-offset so pose_ contains
+               // mojo_from_this with origin-offset (identity) already applied.
 
-  auto mojo_from_other = *relative_to->MojoFromNative();
-  DCHECK(mojo_from_other.IsInvertible());
+  auto other_native_from_mojo = *maybe_other_space_native_from_mojo;
+  auto other_offset_from_other_native = other->OffsetFromNativeMatrix();
 
-  auto other_from_mojo = mojo_from_other.Inverse();
+  auto other_offset_from_mojo =
+      other_offset_from_other_native * other_native_from_mojo;
 
-  auto other_from_this = other_from_mojo * mojo_from_this;
+  auto other_offset_from_this = other_offset_from_mojo * mojo_from_this;
 
-  return MakeGarbageCollected<XRPose>(other_from_this, false);
+  return MakeGarbageCollected<XRPose>(other_offset_from_this, false);
 }
 
 }  // namespace blink

third_party/blink/renderer/modules/xr/xr_object_space.h

@@ -22,13 +22,7 @@ class XRObjectSpace : public XRSpace {
       : XRSpace(session), object_(object) {}
 
   std::unique_ptr<TransformationMatrix> MojoFromNative() override {
-    auto object_from_mojo = object_->poseMatrix();
-
-    if (!object_from_mojo.IsInvertible()) {
-      return nullptr;
-    }
-
-    return std::make_unique<TransformationMatrix>(object_from_mojo.Inverse());
+    return std::make_unique<TransformationMatrix>(object_->MojoFromObject());
   }
 
   std::unique_ptr<TransformationMatrix> NativeFromMojo() final {

third_party/blink/renderer/modules/xr/xr_plane.cc

@@ -53,7 +53,7 @@ XRSpace* XRPlane::planeSpace() const {
   return plane_space_;
 }
 
-TransformationMatrix XRPlane::poseMatrix() const {
+TransformationMatrix XRPlane::MojoFromObject() const {
   return *pose_matrix_;
 }
 

third_party/blink/renderer/modules/xr/xr_plane.h

@@ -42,7 +42,7 @@ class XRPlane : public ScriptWrappable {
 
   XRSpace* planeSpace() const;
 
-  TransformationMatrix poseMatrix() const;
+  TransformationMatrix MojoFromObject() const;
 
   String orientation() const;
   HeapVector<Member<DOMPointReadOnly>> polygon() const;

third_party/blink/renderer/modules/xr/xr_session.cc

@@ -78,9 +78,9 @@ const char kAnchorsNotSupported[] = "Device does not support anchors!";
 
 const char kDeviceDisconnected[] = "The XR device has been disconnected.";
 
-const char kNonInvertibleMatrix[] =
-    "The operation encountered non-invertible matrix and could not be "
-    "completed.";
+const char kUnableToRetrieveMatrix[] =
+    "The operation was unable to retrieve a matrix from passed in space and "
+    "could not be completed.";
 
 const char kUnableToDecomposeMatrix[] =
     "The operation was unable to decompose a matrix and could not be "
@@ -542,18 +542,19 @@ ScriptPromise XRSession::requestReferenceSpace(
   return promise;
 }
 
-ScriptPromise XRSession::CreateAnchor(ScriptState* script_state,
-                                      XRRigidTransform* initial_pose,
-                                      XRSpace* space,
-                                      XRPlane* plane,
-                                      ExceptionState& exception_state) {
+ScriptPromise XRSession::CreateAnchor(
+    ScriptState* script_state,
+    XRRigidTransform* offset_space_from_anchor_transform,
+    XRSpace* space,
+    XRPlane* plane,
+    ExceptionState& exception_state) {
   if (ended_) {
     exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
                                       kSessionEnded);
     return ScriptPromise();
   }
 
-  if (!initial_pose) {
+  if (!offset_space_from_anchor_transform) {
     exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
                                       kNoRigidTransformSpecified);
     return ScriptPromise();
@@ -572,41 +573,34 @@ ScriptPromise XRSession::CreateAnchor(ScriptState* script_state,
     return ScriptPromise();
   }
 
-  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
-  ScriptPromise promise = resolver->Promise();
-
   // Transformation from passed in |space| to mojo space.
   std::unique_ptr<TransformationMatrix> mojo_from_native =
       space->MojoFromNative();
 
-  DVLOG(3) << __func__
-           << ": mojo_from_native = " << mojo_from_native->ToString(true);
-
-  // Matrix will be null if transformation from object space to mojo space is
-  // not invertible, log & bail out in that case.
-  if (!mojo_from_native || !mojo_from_native->IsInvertible()) {
+  if (!mojo_from_native) {
     exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
-                                      kNonInvertibleMatrix);
+                                      kUnableToRetrieveMatrix);
     return ScriptPromise();
   }
 
-  auto native_from_mojo = mojo_from_native->Inverse();
-
   DVLOG(3) << __func__
-           << ": native_from_mojo = " << native_from_mojo.ToString(true);
+           << ": mojo_from_native = " << mojo_from_native->ToString(true);
 
   // Transformation from passed in pose to |space|.
-  auto mojo_from_initial_pose = initial_pose->TransformMatrix();
-  auto space_from_initial_pose = native_from_mojo * mojo_from_initial_pose;
+  auto offset_space_from_anchor =
+      offset_space_from_anchor_transform->TransformMatrix();
+  auto native_space_from_offset_space = space->NativeFromOffsetMatrix();
+  auto native_space_from_anchor =
+      native_space_from_offset_space * offset_space_from_anchor;
 
-  DVLOG(3) << __func__ << ": mojo_from_initial_pose = "
-           << mojo_from_initial_pose.ToString(true);
+  auto mojo_from_native_space = *mojo_from_native;
+  auto mojo_from_anchor = mojo_from_native_space * native_space_from_anchor;
 
-  DVLOG(3) << __func__ << ": space_from_initial_pose = "
-           << space_from_initial_pose.ToString(true);
+  DVLOG(3) << __func__
+           << ": mojo_from_anchor = " << mojo_from_anchor.ToString(true);
 
   TransformationMatrix::DecomposedType decomposed;
-  if (!space_from_initial_pose.Decompose(decomposed)) {
+  if (!mojo_from_anchor.Decompose(decomposed)) {
     exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
                                       kUnableToDecomposeMatrix);
     return ScriptPromise();
@@ -624,6 +618,9 @@ ScriptPromise XRSession::CreateAnchor(ScriptState* script_state,
            << ": pose_ptr->orientation = " << pose_ptr->orientation.ToString()
            << ", pose_ptr->position = " << pose_ptr->position.ToString();
 
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  ScriptPromise promise = resolver->Promise();
+
   if (plane) {
     xr_->xrEnvironmentProviderRemote()->CreatePlaneAnchor(
         std::move(pose_ptr), plane->id(),
@@ -640,12 +637,13 @@ ScriptPromise XRSession::CreateAnchor(ScriptState* script_state,
   return promise;
 }
 
-ScriptPromise XRSession::createAnchor(ScriptState* script_state,
-                                      XRRigidTransform* initial_pose,
-                                      XRSpace* space,
-                                      ExceptionState& exception_state) {
-  return CreateAnchor(script_state, initial_pose, space, nullptr,
-                      exception_state);
+ScriptPromise XRSession::createAnchor(
+    ScriptState* script_state,
+    XRRigidTransform* offset_space_from_anchor_transform,
+    XRSpace* offset_space,
+    ExceptionState& exception_state) {
+  return CreateAnchor(script_state, offset_space_from_anchor_transform,
+                      offset_space, nullptr, exception_state);
 }
 
 int XRSession::requestAnimationFrame(V8XRFrameRequestCallback* callback) {

third_party/webxr_test_pages/webxr-samples/proposals/phone-ar-plane-detection-anchors.html

@@ -322,6 +322,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
       // Creates new anchor for object and places it there.
       function addARObjectAt(session, frame, plane, pose, space) {
+        console.debug("Creating anchor", pose, space);
         if(plane != null) {
           plane.createAnchor(pose, space).then(onCreateAnchor);
         } else {
@@ -329,13 +330,6 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
         }
       }
 
-      function DOMPointFromVec4(vector) {
-        return new DOMPointReadOnly(vector[0], vector[1], vector[2], vector[3]);
-      }
-
-      let rayOrigin = vec4.create();
-      let rayDirection = vec4.create();
-
       function onSelect(event) {
         if (useReticle.checked && arObject.visible) {
           // If we're using the reticle then we've already got a mesh positioned
@@ -364,20 +358,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
           let inputPose = event.frame.getPose(event.inputSource.targetRaySpace, xrRefSpace);
           if (inputPose) {
             let targetRay = new XRRay(inputPose.transform);
-            vec4.set(rayOrigin,
-                targetRay.origin.x,
-                targetRay.origin.y,
-                targetRay.origin.z,
-                targetRay.origin.w);
-            vec4.set(rayDirection,
-                targetRay.direction.x,
-                targetRay.direction.y,
-                targetRay.direction.z,
-                targetRay.direction.w);
-
-            let ray = new XRRay(DOMPointFromVec4(rayOrigin), DOMPointFromVec4(rayDirection));
-
-            const hitTestResults = hitTest(event.frame, ray, xrRefSpace);
+            const hitTestResults = hitTest(event.frame, targetRay, xrRefSpace);
             const hitTestFiltered = filterHitTestResults(hitTestResults,
                                                          extendPlanesEnabled.checked,
                                                          false,
@@ -541,14 +522,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
         // continuous hit test. For the moment we're just using the flower
         // as the "reticle".
         if (useReticle.checked && pose && pose.transform.matrix) {
-          vec4.set(rayOrigin, 0, 0, 0, 1);
-          vec4.transformMat4(rayOrigin, rayOrigin, pose.transform.matrix);
-
-          vec4.set(rayDirection, 0, 0, -1, 0);
-          vec4.transformMat4(rayDirection, rayDirection, pose.transform.matrix);
-          vec4.normalize(rayDirection, rayDirection);
-
-          const ray = new XRRay(DOMPointFromVec4(rayOrigin), DOMPointFromVec4(rayDirection));
+          const ray = new XRRay(pose.transform);
 
           rayObject.matrix = ray.matrix.slice();
           rayObject.visible = true;