 # Angle Object AngleBetweenTwoPoints is Wrong

The method `BABYLON.Angle.BetweenTwoPoints` seems to have been written for two 2D points given by position vectors, hence the name rather than `BABYLON.Angle.BetweenTwoVectors`. Unfortunately however it was intended it does not work.

Let A and B be two points, in the 2D Cartesian plane, with position vectors a and b as in the diagram below The angle between a and b is the red one.

The calculation for

``````BABYLON.Angle.BetweenTwoPoints(a, b);
``````

uses

``````var delta = b.subtract(a);
var theta = Math.atan2(delta.y, delta.x);
return new Angle(theta);
``````

and unless I am missing something this results in the green angle not the red angle.

I would suggest the correct calculation, giving the angle between 0 and π is

``````var theta = Math.acos(Vector2.Dot(a.clone().normalize(), b.clone.normalize()));
return new Angle(theta)
``````

This would then work as Angle.BetweenTwoVectors and could work for Vector3s

atan2 range is [-pi, pi] and acos range is [0,pi]
Moreover, I’d suggest you to clamp the Dot result to [-1,1]. Back in the days, I’ve found that sometime, dot returns 1.000001 for some colinear values resulting in acos returning NaN. Better be safe.

@Cedric I shouldn’t rush to post before dashing off to lunch. I missed a bracket so image didn’t show. Also forgot in my ‘solution’ to consider colinear points.

I do know that ranges for atan2 and acos are different. However that is not the issue.

Now that the image is displayed would anyone agree that the calculation for `Angle.BetweenTwoPoints` is flawed.

Sticking to the case I think the `Angle.BetweenTwoPoints` was probably designed for and to give answers in the range [-π π] I think the method should be more clearly defined as

``````/**
* Gets a new Angle object valued with the angle value in radians between the two 2D points given as position vectors
* @param a defines first position vector2
* @param b defines second position vector2
* @returns a new Angle from a to b
*/
public static BetweenTwoPoints(a: DeepImmutable<Vector2>, b: DeepImmutable<Vector2>): Angle {
var thetaA = Math.atan2(a.y, a.x);
var thetaB = Math.atan2(b.y, b.x);
var theta = thetaB - thetaA;
return new Angle(theta);
}
``````

You can see the error in this PG Babylon.js Playground

The current behavior is what I would personally expect, with it giving the angle to point `b` with `a` as origin

@Gijs that interpretation makes sense of the result. However that is not how it is described in the source and API Angle - Babylon.js Documentation

which is

Gets a new Angle object valued with the angle value in radians between the two given vectors

So suggest that if your interpretation is what is intended comments are changed from

``````/**
* Gets a new Angle object valued with the angle value in radians between the two given vectors
* @param a defines first vector
* @param b defines second vector
* @returns a new Angle
*/
public static BetweenTwoPoints(a: DeepImmutable<Vector2>, b: DeepImmutable<Vector2>): Angle {
var delta = b.subtract(a);
var theta = Math.atan2(delta.y, delta.x);
return new Angle(theta);
}
``````
``````/**
* Given a point A with a position vector2 and another point B, as a position vector2, with the line AX parallel to the X axis gets a new Angle object giving the angle XAB in radians
* @param a defines origin vector2
* @param b defines second vector2
* @returns a new Angle
*/
public static BetweenTwoPoints(a: DeepImmutable<Vector2>, b: DeepImmutable<Vector2>): Angle {
var delta = b.subtract(a);
var theta = Math.atan2(delta.y, delta.x);
return new Angle(theta);
}
``````

Ok, I understand your confusion now, because of vectors not actually being vectors. Replacing it with “point” and making clear `a` is the origin should clear it up right? Or else it becomes a bit verbose IMO:

``````/**
* Gets a new Angle object valued with the angle value in radians between the two given points
* @param a defines first point as the origin
* @param b defines second point
* @returns a new Angle
*/
public static BetweenTwoPoints(a: DeepImmutable<Vector2>, b: DeepImmutable<Vector2>): Angle {
var delta = b.subtract(a).normalize();
var theta = Math.acos(delta.y, delta.x);
return new Angle(theta);
}
``````

And it would be great to add support for 3d points as well since they could both use the dot product instead to get the angle, but I don’t know of a way to pick the right class (`Vector2`|`Vector3`) without checking the type, which is bad for performance…

Unfortunately the term angle between two points has no meaning in maths so can be interpreted in too many different ways

I could settle for

``````/**
* Gets a new Angle object valued with the gradient angle in radians of the line joining two points
* @param a defines first point as the origin
* @param b defines second point
* @returns a new Angle
*/
public static BetweenTwoPoints(a: DeepImmutable<Vector2>, b: DeepImmutable<Vector2>): Angle {
var delta = b.subtract(a).normalize();
var theta = Math.acos(delta.y, delta.x);
return new Angle(theta);
}
``````

https://doc.babylonjs.com/api/classes/babylon.vector3#getanglebetweenvectors

though it returns an angle not an Angle object.

Could do same for vector2

3 Likes

@JohnK Did you mean to put `Math.acos` in the quoted code? Changing the math for BetweenTwoPoints has implications since there is existing code that use it.

You are correct I meant

``````/**
* Gets a new Angle object valued with the gradient angle in radians of the line joining two points
* @param a defines first point as the origin
* @param b defines second point
* @returns a new Angle
*/
public static BetweenTwoPoints(a: DeepImmutable<Vector2>, b: DeepImmutable<Vector2>): Angle {
var delta = b.subtract(a);
var theta = Math.atan2(delta.y, delta.x);
return new Angle(theta);
}
``````

It is only the comments I meant to change at this point having now understood the purpose. There were some copy and paste confusion.

2 Likes