Merge pull request #74 from lilkeet/master

Fix angle proc for 2D vectors

src/vmath.nim

@@ -1573,15 +1573,15 @@ proc angle*[T](a: GVec2[T]): T =
   ## Angle of a Vec2.
   arctan2(a.y, a.x)
 
-proc angle*[T](a, b: GVec2[T]): T =
-  ## Angle between 2 Vec2.
-  fixAngle(arctan2(a.y - b.y, a.x - b.x))
-
-proc angle*[T](a, b: GVec3[T]): T =
-  ## Angle between 2 Vec3.
+proc angle*[T; S: GVec2[T]|GVec3[T]](a, b: S): T =
+  ## Angle between 2 Vec2 or Vec3.
   var dot = dot(a, b)
   dot = dot / (a.length * b.length)
-  arccos(dot)
+  # The cases of angle((1, 1), (-1, -1)) and its 3d counterpart
+  # angle((1, 1, 1), (-1, -1, -1)) result in NaN due to a domain defect going
+  # into the arcos proc: abs(x) > 1.0.
+  # Therefore, we must `clamp` here.
+  arccos(dot.clamp(-1.0, 1.0))
 
 type
   Quat* = GVec4[float32]

tests/test.nim

@@ -1155,4 +1155,34 @@ block:
     else:
       doAssert abs(angleBetween(a.y, b.y - b.z)) < 0.001
 
+block:
+  # Test for https://github.com/treeform/vmath/issues/73
+  template gen2DTestsFor(constructor: untyped): void =
+    doAssert angle(constructor(1, 0), constructor(1, 0)) ~= 0
+    doAssert angle(constructor(1, 1), constructor(-1, -1)) ~= Pi
+    doAssert angle(constructor(1, 0), constructor(0, 1)) ~= Pi/2
+    doAssert angle(constructor(1, 0), constructor(-1, 0)) ~= Pi
+    doAssert angle(constructor(1, 1), constructor(1, -1)) ~= Pi/2
+
+    # Edge cases:
+    doAssert angle(constructor(0, 0), constructor(1, 0)).isNaN()
+
+  gen2DTestsFor vec2
+  gen2DTestsFor dvec2
+
+  template gen3DTestsFor(constructor: untyped): void =
+    doAssert angle(constructor(1, 0, 0), constructor(1, 0, 0)) ~= 0
+    doAssert angle(constructor(1, 1, 1), constructor(-1, -1, -1)) ~= Pi
+    doAssert angle(constructor(1, 0, 0), constructor(0, 1, 0)) ~= Pi/2
+    doAssert angle(constructor(1, 0, 0), constructor(-1, 0, 0)) ~= Pi
+    doAssert angle(constructor(1, 1, 1), constructor(1, -1, 1)) ~= arccos(1/3)
+    doAssert angle(constructor(1, 0, 0), constructor(0, 0, 1)) ~= Pi/2
+    doAssert angle(constructor(1, 1, 1), constructor(-1, -1, 1)) ~= arccos(-1/3)
+
+    # Edge cases:
+    doAssert angle(vec3(0, 0, 0), vec3(1, 0, 0)).isNaN()
+
+  gen3DTestsFor vec3
+  gen3DTestsFor dvec3
+
 echo "test finished successfully"