gpujs / gpu.js

GPU Accelerated JavaScript
https://gpu.rocks
MIT License
15.1k stars 652 forks source link

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

Open danbgoldman opened 3 years ago

danbgoldman commented 3 years ago

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 commented 3 years ago

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

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

danbgoldman commented 3 years ago

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 commented 3 years ago

Can confirm that pull request fixes supplied repro case.

Screenshot 2021-03-23 at 13 41 18