From 57ad916ab10e7150231870e8d80793c6349c34af Mon Sep 17 00:00:00 2001 From: lilkeet Date: Mon, 24 Jun 2024 01:55:34 -0500 Subject: [PATCH 1/5] fixes #73 by removing the faulty vec2 proc and making the vec3 angle proc work for vec2 --- src/vmath.nim | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/vmath.nim b/src/vmath.nim index b1bb102..46d913a 100644 --- a/src/vmath.nim +++ b/src/vmath.nim @@ -1573,12 +1573,16 @@ 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](a, b: GVec2[T]|GVec3[T]): T = + ## Angle between 2 Vec2 or Vec3. + runnableExamples: + assert angle(vec2(0.0, 1.0), vec2(1.0, 0.0)).toDegrees() == 90.0 + assert angle(vec2(0.0, 1.0), vec2(-1.0, 0.0)).toDegrees() == 90.0 + assert angle(vec2(0.0, 1.0), vec2(0.0, -1.0)).toDegrees() == 180.0 + + assert angle(vec3(0.0, 1.0, 0.0), vec3(1.0, 0.0, 0.0)).toDegrees() == 90.0 + assert angle(vec3(0.0, 1.0, 0.0), vec3(-1.0, 0.0, 0.0)).toDegrees() == 90.0 + assert angle(vec3(0.0, 1.0, 0.0), vec3(0.0, -1.0, 0.0)).toDegrees() == 180.0 var dot = dot(a, b) dot = dot / (a.length * b.length) arccos(dot) From 3d42a8ed2a4d6097dd04301060849cf857ac9fac Mon Sep 17 00:00:00 2001 From: lilkeet Date: Mon, 24 Jun 2024 02:14:37 -0500 Subject: [PATCH 2/5] fix generic typing bug: prev commit allowed mixing vector types in arguments like angle(vec2, vec3) --- src/vmath.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vmath.nim b/src/vmath.nim index 46d913a..1ffe55a 100644 --- a/src/vmath.nim +++ b/src/vmath.nim @@ -1573,7 +1573,7 @@ proc angle*[T](a: GVec2[T]): T = ## Angle of a Vec2. arctan2(a.y, a.x) -proc angle*[T](a, b: GVec2[T]|GVec3[T]): T = +proc angle*[T; S: GVec2[T]|GVec3[T]](a, b: S): T = ## Angle between 2 Vec2 or Vec3. runnableExamples: assert angle(vec2(0.0, 1.0), vec2(1.0, 0.0)).toDegrees() == 90.0 From f6bc3ccc08f8e57bb68cd143c9a4106f99605cf0 Mon Sep 17 00:00:00 2001 From: lilkeet Date: Sat, 27 Jul 2024 00:48:34 -0500 Subject: [PATCH 3/5] remove runnable examples per comment in pr --- src/vmath.nim | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/vmath.nim b/src/vmath.nim index 1ffe55a..ed3af08 100644 --- a/src/vmath.nim +++ b/src/vmath.nim @@ -1575,14 +1575,6 @@ proc angle*[T](a: GVec2[T]): T = proc angle*[T; S: GVec2[T]|GVec3[T]](a, b: S): T = ## Angle between 2 Vec2 or Vec3. - runnableExamples: - assert angle(vec2(0.0, 1.0), vec2(1.0, 0.0)).toDegrees() == 90.0 - assert angle(vec2(0.0, 1.0), vec2(-1.0, 0.0)).toDegrees() == 90.0 - assert angle(vec2(0.0, 1.0), vec2(0.0, -1.0)).toDegrees() == 180.0 - - assert angle(vec3(0.0, 1.0, 0.0), vec3(1.0, 0.0, 0.0)).toDegrees() == 90.0 - assert angle(vec3(0.0, 1.0, 0.0), vec3(-1.0, 0.0, 0.0)).toDegrees() == 90.0 - assert angle(vec3(0.0, 1.0, 0.0), vec3(0.0, -1.0, 0.0)).toDegrees() == 180.0 var dot = dot(a, b) dot = dot / (a.length * b.length) arccos(dot) From c2aa04505b65d6e32bcb30d9b2882dfb6a00ed26 Mon Sep 17 00:00:00 2001 From: lilkeet Date: Sat, 27 Jul 2024 02:01:54 -0500 Subject: [PATCH 4/5] add tests for angle proc --- tests/test.nim | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test.nim b/tests/test.nim index ffba8db..db40f24 100644 --- a/tests/test.nim +++ b/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" From aee0c310fe69aea3078cac446b3193cb6506e692 Mon Sep 17 00:00:00 2001 From: lilkeet Date: Sat, 27 Jul 2024 02:02:49 -0500 Subject: [PATCH 5/5] bug fix for some inputs into angle proc --- src/vmath.nim | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/vmath.nim b/src/vmath.nim index ed3af08..371d1d2 100644 --- a/src/vmath.nim +++ b/src/vmath.nim @@ -1577,7 +1577,11 @@ 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]