Bring XRRay.matrix and XRRay constructors up to spec

Minor update to XRRay.matrix algorithm to recompute internal matrix
if it was detached.

Modification to XRRay's constructors to throw InvalidStateError in
case direction normalization cannot be performed.

Update WPT tests to account for the change.

Bug: 957763
Change-Id: I7e65b660de013badbc2cff249d5a75a5fa75ab96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1671871Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671779}

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

@@ -12,6 +12,7 @@
 #include "third_party/blink/renderer/core/geometry/dom_point_read_only.h"
 #include "third_party/blink/renderer/modules/xr/xr_rigid_transform.h"
 #include "third_party/blink/renderer/modules/xr/xr_utils.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/transforms/transformation_matrix.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 #include "ui/gfx/geometry/quaternion.h"
@@ -19,16 +20,19 @@
 
 namespace blink {
 
-XRRay::XRRay(const TransformationMatrix& matrix) {
-  Set(matrix);
+XRRay::XRRay(const TransformationMatrix& matrix,
+             ExceptionState& exception_state) {
+  Set(matrix, exception_state);
 }
 
-XRRay::XRRay(XRRigidTransform* transform) {
+XRRay::XRRay(XRRigidTransform* transform, ExceptionState& exception_state) {
   DOMFloat32Array* m = transform->matrix();
-  Set(DOMFloat32ArrayToTransformationMatrix(m));
+  Set(DOMFloat32ArrayToTransformationMatrix(m), exception_state);
 }
 
-XRRay::XRRay(DOMPointInit* origin, DOMPointInit* direction) {
+XRRay::XRRay(DOMPointInit* origin,
+             DOMPointInit* direction,
+             ExceptionState& exception_state) {
   FloatPoint3D o;
   if (origin) {
     o = FloatPoint3D(origin->x(), origin->y(), origin->z());
@@ -43,27 +47,32 @@ XRRay::XRRay(DOMPointInit* origin, DOMPointInit* direction) {
     d = FloatPoint3D(0.f, 0.f, -1.f);
   }
 
-  Set(o, d);
+  Set(o, d, exception_state);
 }
 
-void XRRay::Set(const TransformationMatrix& matrix) {
+void XRRay::Set(const TransformationMatrix& matrix,
+                ExceptionState& exception_state) {
   FloatPoint3D origin = matrix.MapPoint(FloatPoint3D(0, 0, 0));
   FloatPoint3D direction = matrix.MapPoint(FloatPoint3D(0, 0, -1));
   direction.Move(-origin.X(), -origin.Y(), -origin.Z());
 
-  Set(origin, direction);
+  Set(origin, direction, exception_state);
 }
 
 // Sets member variables from passed in |origin| and |direction|.
 // All constructors eventually invoke this method.
 // If the |direction|'s length is 0, this method will initialize direction to
 // default vector (0, 0, -1).
-void XRRay::Set(FloatPoint3D origin, FloatPoint3D direction) {
+void XRRay::Set(FloatPoint3D origin,
+                FloatPoint3D direction,
+                ExceptionState& exception_state) {
   DVLOG(3) << __FUNCTION__ << ": origin=" << origin.ToString()
            << ", direction=" << direction.ToString();
 
   if (direction.length() == 0.0f) {
-    direction = FloatPoint3D{0, 0, -1};
+    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
+                                      kUnableToNormalizeZeroLength);
+    return;
   } else {
     direction.Normalize();
   }
@@ -73,20 +82,48 @@ void XRRay::Set(FloatPoint3D origin, FloatPoint3D direction) {
                                         direction.Z(), 0.0);
 }
 
-XRRay* XRRay::Create(XRRigidTransform* transform) {
-  return MakeGarbageCollected<XRRay>(transform);
+XRRay* XRRay::Create(XRRigidTransform* transform,
+                     ExceptionState& exception_state) {
+  auto* result = MakeGarbageCollected<XRRay>(transform, exception_state);
+
+  if (exception_state.HadException()) {
+    return nullptr;
+  }
+
+  return result;
 }
 
-XRRay* XRRay::Create() {
-  return MakeGarbageCollected<XRRay>(nullptr, nullptr);
+XRRay* XRRay::Create(ExceptionState& exception_state) {
+  auto* result = MakeGarbageCollected<XRRay>(nullptr, nullptr, exception_state);
+
+  if (exception_state.HadException()) {
+    return nullptr;
+  }
+
+  return result;
 }
 
-XRRay* XRRay::Create(DOMPointInit* origin) {
-  return MakeGarbageCollected<XRRay>(origin, nullptr);
+XRRay* XRRay::Create(DOMPointInit* origin, ExceptionState& exception_state) {
+  auto* result = MakeGarbageCollected<XRRay>(origin, nullptr, exception_state);
+
+  if (exception_state.HadException()) {
+    return nullptr;
+  }
+
+  return result;
 }
 
-XRRay* XRRay::Create(DOMPointInit* origin, DOMPointInit* direction) {
-  return MakeGarbageCollected<XRRay>(origin, direction);
+XRRay* XRRay::Create(DOMPointInit* origin,
+                     DOMPointInit* direction,
+                     ExceptionState& exception_state) {
+  auto* result =
+      MakeGarbageCollected<XRRay>(origin, direction, exception_state);
+
+  if (exception_state.HadException()) {
+    return nullptr;
+  }
+
+  return result;
 }
 
 XRRay::~XRRay() {}
@@ -94,52 +131,62 @@ XRRay::~XRRay() {}
 DOMFloat32Array* XRRay::matrix() {
   DVLOG(3) << __FUNCTION__;
 
-  if (!matrix_) {
+  // A page may take the matrix value and detach it so matrix_ is a detached
+  // array buffer.  If that's the case, recompute the matrix.
+  // Step 1. If transform’s internal matrix is not null, perform the following
+  // steps:
+  //    Step 1. If the operation IsDetachedBuffer on internal matrix is false,
+  //    return transform’s internal matrix.
+  if (!matrix_ || !matrix_->View() || !matrix_->View()->Data()) {
     // Returned matrix should represent transformation from ray originating at
     // (0,0,0) with direction (0,0,-1) into ray originating at |origin_| with
     // direction |direction_|.
 
     TransformationMatrix matrix;
 
+    const blink::FloatPoint3D desiredRayDirection = {
+        direction_->x(), direction_->y(), direction_->z()};
+
     // Translation from 0 to |origin_| is simply translation by |origin_|.
+    // (implicit) Step 6: Let translation be the translation matrix with
+    // components corresponding to ray’s origin
     matrix.Translate3d(origin_->x(), origin_->y(), origin_->z());
 
+    // Step 2: Let z be the vector [0, 0, -1]
     const blink::FloatPoint3D initialRayDirection =
         blink::FloatPoint3D{0.f, 0.f, -1.f};
-    const blink::FloatPoint3D desiredRayDirection = {
-        direction_->x(), direction_->y(), direction_->z()};
 
+    // Step 3: Let axis be the vector cross product of z and ray’s direction,
+    // z × direction
+    blink::FloatPoint3D axis = initialRayDirection.Cross(desiredRayDirection);
+
+    // Step 4: Let cos_angle be the scalar dot product of z and ray’s direction,
+    // z · direction
     float cos_angle = initialRayDirection.Dot(desiredRayDirection);
 
+    // Step 5: Set rotation based on the following:
     if (cos_angle > 0.9999) {
       // Vectors are co-linear or almost co-linear & face the same direction,
       // no rotation is needed.
+
     } else if (cos_angle < -0.9999) {
       // Vectors are co-linear or almost co-linear & face the opposite
       // direction, rotation by 180 degrees is needed & can be around any vector
       // perpendicular to (0,0,-1) so let's rotate by (1, 0, 0).
-      blink::FloatPoint3D axis = FloatPoint3D{1, 0, 0};
-      cos_angle = -1;
-
-      matrix.Rotate3d(axis.X(), axis.Y(), axis.Z(),
-                      rad2deg(std::acos(cos_angle)));
+      matrix.Rotate3d(1, 0, 0, 180);
     } else {
       // Rotation needed - create it from axis-angle.
-      blink::FloatPoint3D axis = initialRayDirection.Cross(desiredRayDirection);
-
       matrix.Rotate3d(axis.X(), axis.Y(), axis.Z(),
                       rad2deg(std::acos(cos_angle)));
     }
 
+    // Step 7: Let matrix be the result of premultiplying rotation from the left
+    // onto translation (i.e. translation * rotation) in column-vector notation.
+    // Step 8: Set ray’s internal matrix to matrix
     matrix_ = transformationMatrixToDOMFloat32Array(matrix);
   }
 
-  if (!matrix_ || !matrix_->View() || !matrix_->View()->Data()) {
-    // A page may take the matrix value and detach it so matrix_ is a detached
-    // array buffer.  This breaks the inspector, so return null instead.
-    return nullptr;
-  }
-
+  // Step 9: Return matrix
   return matrix_;
 }
 

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

@@ -16,6 +16,7 @@ namespace blink {
 
 class DOMPointInit;
 class DOMPointReadOnly;
+class ExceptionState;
 class TransformationMatrix;
 class XRRigidTransform;
 
@@ -23,25 +24,33 @@ class XRRay final : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  explicit XRRay(const TransformationMatrix& matrix);
-  explicit XRRay(XRRigidTransform* transform);
-  XRRay(DOMPointInit* origin, DOMPointInit* direction);
+  explicit XRRay(const TransformationMatrix& matrix,
+                 ExceptionState& exception_state);
+  explicit XRRay(XRRigidTransform* transform, ExceptionState& exception_state);
+  XRRay(DOMPointInit* origin,
+        DOMPointInit* direction,
+        ExceptionState& exception_state);
   ~XRRay() override;
 
   DOMPointReadOnly* origin() const { return origin_; }
   DOMPointReadOnly* direction() const { return direction_; }
   DOMFloat32Array* matrix();
 
-  static XRRay* Create();
-  static XRRay* Create(DOMPointInit* origin);
-  static XRRay* Create(DOMPointInit* origin, DOMPointInit* direction);
-  static XRRay* Create(XRRigidTransform* transform);
+  static XRRay* Create(ExceptionState& exception_state);
+  static XRRay* Create(DOMPointInit* origin, ExceptionState& exception_state);
+  static XRRay* Create(DOMPointInit* origin,
+                       DOMPointInit* direction,
+                       ExceptionState& exception_state);
+  static XRRay* Create(XRRigidTransform* transform,
+                       ExceptionState& exception_state);
 
   void Trace(blink::Visitor*) override;
 
  private:
-  void Set(const TransformationMatrix& matrix);
-  void Set(FloatPoint3D origin, FloatPoint3D direction);
+  void Set(const TransformationMatrix& matrix, ExceptionState& exception_state);
+  void Set(FloatPoint3D origin,
+           FloatPoint3D direction,
+           ExceptionState& exception_state);
 
   Member<DOMPointReadOnly> origin_;
   Member<DOMPointReadOnly> direction_;

third_party/blink/renderer/modules/xr/xr_ray.idl

@@ -10,7 +10,8 @@
     Constructor(),
     Constructor(DOMPointInit origin),
     Constructor(DOMPointInit origin, DOMPointInit direction),
-    Constructor(XRRigidTransform transform)
+    Constructor(XRRigidTransform transform),
+    RaisesException=Constructor
 ] interface XRRay {
   [SameObject] readonly attribute DOMPointReadOnly origin;
   [SameObject] readonly attribute DOMPointReadOnly direction;

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

@@ -26,6 +26,9 @@ DOMPointReadOnly* makeNormalizedQuaternion(double x,
                                            double z,
                                            double w);
 
+constexpr char kUnableToNormalizeZeroLength[] =
+    "Unable to normalize vector of length 0.";
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_MODULES_XR_XR_UTILS_H_

third_party/blink/web_tests/external/wpt/webxr/xrRay_constructor.https.html

@@ -90,22 +90,25 @@ let constructor_tests = function() {
   }
 
   {
-    // Check that we don't crash on direction too close to 0,0,0:
+    // Check that we throw exception on direction too close to 0,0,0:
     let originDict = {x : 10.0, y : 10.0, z : 10.0, w : 1.0};
     let directionDict = {x : 0.0, y : 0.0, z : 0.0, w : 0.0};
 
-    let xrRay = new XRRay(
-      DOMPoint.fromPoint(originDict),
-      DOMPoint.fromPoint(directionDict));
-
-    assert_point_approx_equals(
-      xrRay.origin, originDict,
-      FLOAT_EPSILON, "origin-custom-direction-zero:");
-
-    // cannot validate direction's & matrix's values w/o making it depend on current
-    // implementation, just check that they're not null
-    assert_not_equals(xrRay.direction, null, "origin-custom-direction-zero:direction should not be null");
-    assert_not_equals(xrRay.matrix, null, "origin-custom-direction-zero:matrix should not be null");
+    try {
+      let xrRay = new XRRay(
+        DOMPoint.fromPoint(originDict),
+        DOMPoint.fromPoint(directionDict));
+
+      assert(false, "origin-custom-direction-zero:exception not thrown");
+    }
+    catch(e)
+    {
+      if(e instanceof DOMException) {
+        assert_equals(e.name, "InvalidStateError", "origin-custom-direction-zero:incorrect DOM exception thrown");
+      } else {
+        assert(false, "origin-custom-direction-zero:other exception thrown: " + e);
+      }
+    }
   }
 
   //