Merge pull request #180 from markuswntr/quaternion/patch-2

Fix potential infinite loop in quaternion act

Author: stephentyrone

Sources/QuaternionModule/Transformation.swift

@@ -387,15 +387,21 @@ extension Quaternion {
     // The following expression have been split up so the type-checker
     // can resolve them in a reasonable time.
     let p1 = vector * (real*real - imaginary.lengthSquared)
-    let p2 = 2 * imaginary * imaginary.dot(vector)
-    let p3 = 2 * real * imaginary.cross(vector)
-    let rotatedVector = p1 + p2 + p3
+    let p2 = imaginary * imaginary.dot(vector)
+    let p3 = imaginary.cross(vector) * real 
+    let rotatedVector = p1 + (p2 + p3) * 2
 
     // If the rotation computes without over/underflow, everything is fine
     // and the result is correct. If not, we have to do the computation
     // carefully and first unscale the vector, rotate it again and then
     // rescale the vector
-    if rotatedVector.isNormal { return rotatedVector }
+    if
+        (rotatedVector.x.isNormal || rotatedVector.x.isZero) &&
+        (rotatedVector.y.isNormal || rotatedVector.y.isZero) &&
+        (rotatedVector.z.isNormal || rotatedVector.z.isZero)
+    {
+        return rotatedVector
+    }
     let scale = max(abs(vector.max()), abs(vector.min()))
     return act(on: vector/scale) * scale
   }
@@ -459,12 +465,6 @@ extension SIMD3 where Scalar: FloatingPoint {
     x.isFinite && y.isFinite && z.isFinite
   }
 
-  /// True if all values of this instance are finite
-  @usableFromInline @inline(__always)
-  internal var isNormal: Bool {
-    x.isNormal && y.isNormal && z.isNormal
-  }
-
   /// Returns the squared length of this instance.
   @usableFromInline @inline(__always)
   internal var lengthSquared: Scalar {

Tests/QuaternionTests/TransformationTests.swift

@@ -30,8 +30,8 @@ final class TransformationTests: XCTestCase {
     XCTAssertEqual(Quaternion<T>(angle: .pi, axis: -xAxis).angle, .pi)
     XCTAssertEqual(Quaternion<T>(angle: .pi, axis: -xAxis).axis, -xAxis)
     // Negative angle, negative axis
-    XCTAssertEqual(Quaternion<T>.init(angle: -.pi, axis: -xAxis).angle, .pi)
-    XCTAssertEqual(Quaternion<T>.init(angle: -.pi, axis: -xAxis).axis, xAxis)
+    XCTAssertEqual(Quaternion<T>(angle: -.pi, axis: -xAxis).angle, .pi)
+    XCTAssertEqual(Quaternion<T>(angle: -.pi, axis: -xAxis).axis, xAxis)
   }
 
   func testAngleAxis() {
@@ -221,6 +221,15 @@ final class TransformationTests: XCTestCase {
     let vector = SIMD3<T>(1,1,1)
     let xAxis = SIMD3<T>(1,0,0)
 
+    let singleAxis = Quaternion<T>(angle: .pi/2, axis: SIMD3(0, 1, 0))
+    XCTAssertTrue(singleAxis.act(on: xAxis).x.isApproximatelyEqual(
+        to: .zero, absoluteTolerance: T.ulpOfOne.squareRoot()
+    ))
+    XCTAssertTrue(singleAxis.act(on: xAxis).y.isApproximatelyEqual(
+        to: .zero, absoluteTolerance: T.ulpOfOne.squareRoot()
+    ))
+    XCTAssertEqual(singleAxis.act(on: xAxis).z, -1)
+
     let piHalf = Quaternion<T>(angle: .pi/2, axis: xAxis)
     XCTAssertTrue(closeEnough(piHalf.act(on: vector).x,  1, ulps: 0))
     XCTAssertTrue(closeEnough(piHalf.act(on: vector).y, -1, ulps: 1))