[SPIGOT-5240] Bukkit's Vector#angle(Vector) is broken Created: 02/Aug/19  Updated: 02/Aug/19  Resolved: 02/Aug/19

Status: Resolved
Project: Spigot
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Minor
Reporter: Islandscout Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: bukkit, util

Version: git-Spigot-cbcc8e8-06efc9e (MC: 1.14.4) (Implementing API version 1.14.4-R0.1-SNAPSHOT)
Guidelines Read: Yes

 Description   

It appears that for certain vectors, Vector#angle(Vector) returns NaN. This doesn't make sense since there is always an angle between two non-zero vectors which should always be 0 to 180 degrees inclusive.
 
This is the current implementation in Bukkit:

public float angle(@NotNull final Vector other) {
    final double dot = this.dot(other) / (this.length() * other.length());
    return (float)Math.acos(dot);
}

 
Although this is correct mathematically, there is an issue that needs to be addressed. The value of "dot" should be within the domain [-1, 1]. For some vectors pointing extremely close towards the same direction, "dot" will lie slightly out of the domain due to floating point precision errors. If "dot" does not lie within that domain, then Math.acos(double) will return NaN.
 
I suggest to wrap the dot computation inside of Math.min(double, double) and Math.max(double, double) to ensure that "dot" is within the domain [-1, 1].

public float angle(@NotNull final Vector other) {
    double dot = Math.min(Math.max(this.dot(other) / (this.length() * other.length()), -1), 1);
    return (float)Math.acos(dot);
}

 Comments   
Comment by md_5 [ 02/Aug/19 ]

> Isn't Double.NaN == Double.NaN always false?

 

Right, thanks

Comment by Islandscout [ 02/Aug/19 ]

Yeah, the normalization is done before the min/max.

md_5, what is that assertNotEquals method? Isn't Double.NaN == Double.NaN always false? Wouldn't that mean doing Double.NaN == (a number that is NaN) will also always be false? Try doing Double.isNaN(a.angle(b))

I also want to mention that this fix will not break the condition where at least one of the vectors is zero, which should be one of the only few conditions that returns NaN.

Comment by Brokkonaut [ 02/Aug/19 ] 
Vector a = new Vector(-0.13154885489775203, 0.0, 0.12210868381700482);
Vector b = new Vector(-0.7329152226448059, -0.0, 0.6803199648857117);
System.out.println(a.angle(b));
System.out.println(a.dot(b) / (a.length() * b.length()));

Output:

 

NaN
1.0000000000000002

Afaik his fix should work for not normalized vectors too, because its divided by a.length() * b.length() (this is the normalization) before the min/max is applied.

 

Comment by md_5 [ 02/Aug/19 ]

Your fix looks only to be valid for normalized vectors also.

Comment by md_5 [ 02/Aug/19 ] 
    @Test
    public void testSmallAngle() {
        Vector a = new Vector(-0.13154885489775203, 0.0, 0.12210868381700482);
        Vector b = new Vector(-0.7329152226448059, -0.0, 0.6803199648857117);        assertNotEquals(Double.NaN, a.angle(b));
    }

Seems to work ok for me.

Assuming you are also using an x86 64 bit processor this should most definitely not be hardware dependent.

Comment by Islandscout [ 02/Aug/19 ]

Try with this:

 

Vector a = new Vector(-0.13154885489775203,0.0,0.12210868381700482);
Vector b = new Vector(-0.7329152226448059,-0.0,0.6803199648857117);
Bukkit.broadcastMessage(a.angle(b));

It could be hardware dependent, but regardless of what hardware the code runs on, it should not be returning NaN (at least for most hardware).

 The value of "dot" is 1.0000000000000002, which is clearly not within [-1, 1]

Comment by md_5 [ 02/Aug/19 ]

Can you provide a test case?

Generated at Thu Apr 03 16:26:13 UTC 2025 using Jira 10.3.3#10030003-sha1:d220e3fefc8dfc6d47f522d3b9a20c1455e12b7b.