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

atan2 is implemented as atan(y/x) in gpu backend #647

Open
danbgoldman opened this issue Nov 20, 2020 · 3 comments · May be fixed by #683
Open

atan2 is implemented as atan(y/x) in gpu backend #647

danbgoldman opened this issue Nov 20, 2020 · 3 comments · May be fixed by #683

Comments

@danbgoldman
Copy link

slow down egghead

What is wrong?

atan2 should return values from -pi to pi, but instead returns values from -pi/2 to pi/2 in gpu mode. It also returns undefined results for x=0.

Where does it happen?

In Chrome browser.

How do we replicate the issue?

I'm attaching a repro case - correct behavior can be seen in mode:cpu, incorrect behavior in mode:gpu.

atan2-bug.zip

How important is this (1-5)?

3 - I can work around, but also should be an easy fix.

Expected behavior (i.e. solution)

The backend GLSL implementations of atan2 use the single-argument version of atan(y_over_x), instead of the two-argument version atan(y, x).

@harshkhandeparkar
Copy link

https://tc39.es/ecma262/#sec-math.atan2

The spec for atan2 is pretty sophisticated and long...
Does GLSL have an atan2 already?

@danbgoldman
Copy link
Author

IIUC, GLSL atan is overloaded to take both 1 argument (like Math.atan) and 2 arguments (like Math.atan2). AFAIK the second implementation is identical to C/javascript.

https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/atan.xhtml

@ted-piotrowski
Copy link

Can confirm that pull request fixes supplied repro case.

Screenshot 2021-03-23 at 13 41 18

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

Successfully merging a pull request may close this issue.

3 participants