Three.js: creating Vectors with NaN values replaces most of them with 0

[sjpt]

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) returns Vector3(0, 3, 4)
new Vector4(NaN, NaN, NaN, NaN) returns Vector4(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

• [ ] Dev
• [x ] r117
• [ ] ...

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [x] All of them
• [ ] Windows
• [ ] macOS
• [ ] Linux
• [ ] Android
• [ ] iOS

Hardware Requirements (graphics card, VR Device, ...)

[mrdoob]

I guess we can fix this by doing this here?

this.w = ( w !== undefined ) ? w || 0 : 1;

[WestLangley]

three.js does not support NaNs. I do not think three.js should be responsible for converting them to zeros or ones.

NaNs should be handled at the app level, IMO -- and not passed into constructors.

[sjpt]

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 NaN || x returns x, but I see it makes sense in some curious way and is certainly as defined.

The best I can think of that gives what I consider to be the 'appropriate' answer is
this.x = x == 0 ? 0 : x;
I don't know performance implications, probably trivial compared to the new object overhead.

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_'.

[Mugen87]

Sorry, but you make it unnecessarily complicated. App level code has to ensure to pass numbers to Vector3 or Vector4. Besides, three.js does in general not sanitize arguments.

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:

class Vector3 {

    constructor( x = 0, y = 0, z = 0 ) {

        this.x = x;
        this.y = y;
        this.z = z;

    }
}

[sjpt]

I was passing a number. typeof NaN is 'number'.
Converting one number (NaN) to another (0) is not sanitizing, its polluting.

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;
can't you just use Vector3( x = 0, y = 0, z = 0 ) , Vector4( x = 0, y = 0, z = 0, w = 1 ) etc now, or will that break on some old version of Javascript that you still support?
Failing that this.x = x === undefined ? 0 : x.

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 new Vector4().set(x,y,z,w). The reason I think it should be fixed within three.js rather than outside is that others may also waste time debugging under similar circumstances. Whereever possible a system should behave as users would expect, and converting NaN to 0 is not expected.

Sadly javascript is a pretty horrible language in many ways which makes consistency harder to achieve than it ought to be.

[Mugen87]

Even if NaN is of type number, the name implies that it is _not a number_. Hence, it's not a valid argument.

Is there any reason to wait for conversion to classes;

Yes, see https://github.com/mrdoob/three.js/pull/17425#issuecomment-636168218.

[sjpt]

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 function Vector4( x=0, y=0, z=0, w=1 ) style defaulting.

[mrdoob]

19678

[mrdoob]

I was passing a number. typeof NaN is 'number'.

This made my blood boil for so many reasons...