-
-
Notifications
You must be signed in to change notification settings - Fork 35.6k
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
creating Vectors with NaN values replaces most of them with 0 #19649
Comments
I guess we can fix this by doing this here? this.w = ( w !== undefined ) ? w || 0 : 1; |
three.js does not support
|
The handling of w was (relatively) correct as it was. It was the handling of x, y, z that is at issue. A question is if you still want to let through other values ('', null, etc) to become 0? It could break other apps if you stop allowing those. I was very surprised to find that The best I can think of that gives what I consider to be the 'appropriate' answer is Sort of related, it would be nice if renderer.setRenderTarget() set renderTarget to null. (Reminds me why I was so anti database null values back in the 70's.) When you say 'I do not think three.js should be responsible for converting them to zeros or ones.', I say 'I don't think three.js should irresponsibly take it upon itself to convert them to zeros or ones'. |
Sorry, but you make it unnecessarily complicated. App level code has to ensure to pass numbers to The current code in the ctor just ensures default values. When converting to classes, the ctor can be updated too so it will look like so:
|
I was passing a number. I'm pleased to say that your new code will have the correct effect and accept NaN values. Is there any reason to wait for conversion to classes; By the way, I'm not trying to pick holes in three.js just for the sake of it; it is an excellent system. Just with my slightly unusual usage finding and reporting bits that unexpectedly don't work as they logically might. I have a genuine need for NaN values in shaders and rendertarget/textures (to get round the lack of geometry shaders in WebGL), When I read these back for debug I can see the NaN values in the raw texture data, but putting them through a utility to make the read back textures more inspectable Vector4 values then the Nan values were polluted. When I didn't see expected NaN values (except in the .w field) I naturally assumed my shader was wrong. It took quite a bit of debug time to track down the issue. As usual the fix was trivial; I used Sadly javascript is a pretty horrible language in many ways which makes consistency harder to achieve than it ought to be. |
Even if NaN is of type
Yes, see #17425 (comment). |
The NaN name implies it is not a number but actually means it isn't a normal number; the typing says it is. There is a well known convention for NaN handing and propagation in hardware and all programming languages that support NaN (including javascript) which three.js is breaking. #17425 indicates a reason to defer the conversion to classes, but does not give a reason not to change the current function style code to |
This made my blood boil for so many reasons... |
Setting any THREE.Vectorx with NaN values causes the NaN value to be replaced by 0,
except for the w field of a Vector4.
eg
new Vector3(NaN, 3, 4)
returnsVector3(0, 3, 4)
new Vector4(NaN, NaN, NaN, NaN)
returnsVector4(0, 0, 0, NaN)
I can see fixing this would add a fraction to the common code path,
but it causes some very unexpected failure to propagate NaN values.
Three.js version
Browser
OS
Hardware Requirements (graphics card, VR Device, ...)
The text was updated successfully, but these errors were encountered: