Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector2 angle inconsistency due to negative zero #43407

Open
goatchurchprime opened this issue Nov 9, 2020 · 13 comments
Open

Vector2 angle inconsistency due to negative zero #43407

goatchurchprime opened this issue Nov 9, 2020 · 13 comments

Comments

@goatchurchprime
Copy link

Godot version:
3.2.3-stable

OS/device including version:
Windows 10

Issue description:

var zv = Vector2(0,0)
print("Zero vector angle: ", zv.angle())
print("Negated zero vector angle: ", (-zv).angle())

Zero vector angle: 0
Negated zero vector angle: -3.141593

I don't know why Vector2 (and Vector3) are in particular allowing this problematic negative zero value to get into the system when it doesn't happen in scalar arithmetic, as seen here:

var v = 0
var nv = -v
var nnv = (-Vector2(v, v)).x
print("v: ", v)
print("nv: ", nv)
print("nnv: ", nnv)
print("v == nnv: ", (v == nnv))	

v: 0
nv: 0
nnv: -0
nv == nnv: True

A different issue caused by Vector2 providing a gateway to negative zero pollution was patched over in a previous pull-request here: #6698

Steps to reproduce:
See above

Minimal reproduction project:
N/A

@lawnjelly
Copy link
Member

lawnjelly commented Nov 9, 2020

Does this cause any problems in practice? i.e. This is correct behaviour as determined by the CPU, afaik, which can store both positive and negative zero according to IEEE 754 standard.

@goatchurchprime
Copy link
Author

The problem was discovered because I was dividing a network of points and lines into polygons by sorting the edges from each point according to the angle. My code broke because I had assumed that if p and q are both type Vector2 then

(p - q).angle() == (-(q - p)).angle()

This obviously true statement is unexpectedly false when p == q. This took a bit of debugging to find out and now requires some special case code.

Just because it's in the IEEE standard doesn't mean we have to like it. I mean, I really hate radians which stops us from doing something simple like turning the playing area upside down with a 180 rotation (there's always a crappy 0.0000001 factor in there when you specify angles in this useless unit).

GDscript seems to have kept negative zeros from showing up in normal calculations (eg you don't get one when you write -0), and when Vector2 is used as a key in a hash-table (see the pull request above), so we could have the following line at:
https://github.com/godotengine/godot/blob/master/core/math/vector2.cpp#L33

real_t Vector2::angle() const {
	return (x != 0 && y != 0 ? Math::atan2(y, x) : 0);
}

This isn't going to add much overhead to the already heavily slow trig function. If we were after raw speed and efficiency, there is the alternative of the DiamondArg function which has many advantages to it:
https://www.freesteel.co.uk/wpblog/2009/06/05/encoding-2d-angles-without-trigonometry/

@akien-mga
Copy link
Member

akien-mga commented Nov 19, 2020

Your scalar arithmetic example uses integers, in which there are no signed zeros:

var v = 0
var nv = -v  # this is still 0

Try with floats:

var v = 0.0
var nv = -v  # this is now -0.0

As for Vector2.angle() behaving differently for (+0, +0) and (-0, -0), that's due to the implementation of atan2 in the C++ standard:
https://en.cppreference.com/w/cpp/numeric/math/atan2

If y is ±0 and x is negative or -0, ±π is returned
If y is ±0 and x is positive or +0, ±0 is returned

We could add a hack to work it around in Math::atan2() if we agree on what the correct output is. (0, 0) is a singularity for atan2() and depending on what side you approach it from (negative x, negative y, etc.), you can have different expectations, as the atan2() implementation notes show it. Forcing a 0 output might be an unexpected jump for some users like you're having yourself between 0 and -PI.

@akien-mga akien-mga added discussion and removed bug labels Nov 19, 2020
@madmiraal
Copy link
Contributor

This is a non-issue. Seeking the angle of a zero vector to the x-axis is meaningless; so the correct answer is "undefined". The fact that C++ provides a consistent result (either 0 or ±π depending on the ±0 of the components) doesn't change the fact that the answer is meaningless.

This obviously true statement is unexpectedly false when p == q. This took a bit of debugging to find out and now requires some special case code.

Yes, the same way one needs to check for zero before attempting to divide by zero, one should check for a zero length vector before trying to find its angle.

@goatchurchprime
Copy link
Author

If that is the case, we should safely assume that any time angle() is applied to a zero vector it is certainly a mistake, and we should therefore print a warning or error message to help people out.

As there was no documentation about this condition I tested the function on Vector2(0,0) and it returned zero, so I assumed everything was fine and that the case was being handled efficiently in the C++ level so I wouldn't need to waste precious cycles in the interpreted code testing for it. I did not know much about this -0.0 curiosity. In all other cases the use of 0.0 and -0.0 are totally indistinguishable. You could only tell the difference between them by examining its bit pattern or printing to ascii and looking for a minus sign. Until now I did not know there was a single mathematical function that treated them differently. Now I know one. It seems the special cases for it have been adopted off-spec from some conventional C++ release for the sake of reproducibility. But they are also useless. I'm willing to bet my bottom dollar that no programmer has ever intentionally relied on these cases. There might be cases where it's accidentally used, which is why consistency is important, but it's never on purpose.

If this function is not going to give an answer that is predictable enough to design against (and these -0.0 case differences are not) then we should get a runtime warning at the minimum.

@madmiraal
Copy link
Contributor

I disagree, the answer is correct; so no warning is needed.

atan2 has a clear and specific definition. It has discontinuities around y = 0, which is why there is a difference between y = ±0. The problem is that trying to get the angle of a zero vector is meaningless. In the specific example provided, what is attempted is to find the angle between two points. When the two points are on top of each other, any answer is correct. The question is meaningless, but the answer is correct, because any answer would be correct.

So, as I said before, the same way one needs to check for zero before attempting to divide by zero, one should check for a negligible distance between two points, before trying to find the angle between those two points. This is the user's responsibility, not the engine's, because the engine will return the right answer regardless of what the user is trying to do.

@ivanvoid
Copy link

ivanvoid commented May 3, 2023

I do lerp(velocity, Vector3(0.,0.,0.), delta*MOVE_LERP_SPEED) and get negative velocities (-0) for CharacterBody3D, in godot 4.0.2

@AThousandShips
Copy link
Member

AThousandShips commented May 3, 2023

@ivanvoid how does lerp relate to angles? Lerp can return negative angles if the value is negative, which is perfectly valid, what ranges of values for velocity are you looking at?

@ivanvoid
Copy link

ivanvoid commented May 4, 2023

@AThousandShips way to reproduce:

var v = Vector3(-3, -3, -3)
func _physics_process(delta):
    v = lerp(v , Vector3(0, 0, 0), delta)
    print(v)

will output
(-0, -0, -0)
why? expected to be positive zeros, no?

@AThousandShips
Copy link
Member

AThousandShips commented May 4, 2023

Why? It's supposed to be somewhere between -3 and 0, it could only be 0 if delta is exactly 1, you are again dealing with precision errors, also are you certain it is exactly -0 or just rounded to -0, for example -0.0000001

But again this issue isn't about negative zero in general but angles, if you still think this is a bug please open a separate issue as this is unrelated

@ivanvoid
Copy link

ivanvoid commented May 4, 2023

But as pointed above it's not that important I guess. Just kinda unexpected behavior that you need to work around.

@AThousandShips
Copy link
Member

AThousandShips commented May 4, 2023

I've explained to you? And said to open a separate report, why aren't you listening to what I'm explaining and suggesting, this is off topic to this issue which is about Vector2.angle()

@AThousandShips
Copy link
Member

AThousandShips commented May 4, 2023

But I'll explain it again:
lerp(v, Vector3(0, 0, 0), delta) will only become exactly Vector3(0, 0, 0) if delta is 1, but delta is never 1, it's mostly much smaller than one, so it gradually drifts but stays on the side of v, please check if the values are actually exactly -0 and not, as I suggested, -0.000001 or similar

And in fact as time goes on the value should eventually become stuck at the smallest number below -0 as when it reaches that it won't become 0 unless delta is large enough, at least 0.25 or 0.5, depending on various internals of floating point math. The reason being that v, being the smallest number that can be represented, is multiplied by some number smaller than 0.5, rounds down, and you never move away from v, as lerp in this case is v - v*delta

I hope this has explained why it probably won't work as you expect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants