1.2.1 version breaks webgl program

[w0rm]

This change 7c8d96a has broken my webgl game in Chrome 56.0.2924.87. Using linear-algebra 1.2.0 fixes the problem.

[dannyob]

Yep, me too. The problem is that gl is expecting Float32Arrays, and the new linear algebra is defaulting to Float64Arrays.

[jvoigtlaender]

Comments on this, @felixLam?

[felixLam]

What is your interface with GL? If you are using a port it might be better to "downcast" in the native code.

[w0rm]

@felixLam the main purpose of linear-algebra is to work with webgl.

[felixLam]

Given that we cannot configure this packages externally, I would opt for the higher precision default and reduce precision as late as possible in any implementation.

[felixLam]

@w0rm the simplest would probably be to upgrade the WebGL code to use 64 bit precision as well. Which might be as simple as replacing 32 with 64 in https://github.com/elm-community/webgl/blob/master/src/Native/WebGL.js#L201

This does not work

[felixLam]

@w0rm I just check out the examples, and they all appear to work fine.

[w0rm]

@felixLam maybe you don't quite get the impact of this change. It has silently broken multiple applications that use webgl. In my opinion it should be reverted asap, and then we can discuss the change and run benchmarks to see if it has perf impact and works across the browsers

[felixLam]

@w0rm I am aware now and am happy to maintain a 64 bit clone of this package if that is the only way to resolve this, but was hoping that there is a way to make webGL work with 64 bit as the examples run fine as they are.

[eeue56]

This is a major change. It should've been a major patch bump. I will handle fixing this.

[felixLam]

Would it be possible to keep the change and only change the versioning? As pointed out in the original PR #5 there are legitimate use cases (also in a WebGL context) that require the added precision.

[eeue56]

@felixLam Yes, that's what I'm doing

[felixLam]

Awesome, thanks

[w0rm]

Thanks @eeue56!

[felixLam]

Maybe we should add a test that uses WebGL as a package if that is one of the principal applications of this package.

[eeue56]

I've published the old 32 bit version again as 1.2.2, meaning it will be preferred. I've published the 64 bit change as 2.0.0. I sadly had to do some tricks in order to get that package version to work - by adding a dummy constructor (http://package.elm-lang.org/packages/elm-community/linear-algebra/2.0.0/Math-Vector3) to Vec3. This won't affect anyone's code, since Vec3 will be used in the same way.

However, I do recommend that the next PR to be merged to this repo is one that stops exposing the constructors. These are fake constructors and will cause breakage if anyone actually tries to use them.

[cplotter]

Whatever happened here broke elm-github-install for me. It does not want to grab version 1.2.2

[eeue56]

@cplotter You're gonna need to explain and expand a bit more if you want help.

[cplotter]

@eeue56 More of a note to anyone else that finds this thread. I had to manually set it to 1.2.0 to make it work. For some reason it does not pick up 1.2.2

[fredcy]

Can someone suggest some test cases that we can add that would have caught this problem?

[felixLam]

@fredcy The code that probably needs to be triggered in the unit test is most likely: https://github.com/elm-community/webgl/blob/master/src/Native/WebGL.js#L201

Unfortunately the WebGL examples do not seem to be affected.

[fredcy]

I don't understand WebGL anywhere near well enough to see how to invoke the internal getAttributeInfo function referenced above. Any help?

[w0rm]

The exception that I got was not in attributes, but in uniforms. I don't see any good way to export the native part from webgl for testing. The native representation of linear algebra types is an implicit contract between the elm compiler, linear algebra and webgl.

[dannyob]

Might it be possible to make the accuracy of linear-algebra a compile-time option in _elm_community$linear_algebra$Native_MJS ?

Linear-algebra needs to produce Float32Arrays to be compatible with WebGL because that's the only type that functions like https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/uniformMatrix accept, but MJS will (I think) happily switch between types if MJS_FLOAT_ARRAY_TYPE is set correctly at https://github.com/elm-community/linear-algebra/blob/master/src/Native/MJS.js#L69.

WebGLFloatArray was dropped in favour of Float32Array, so we can't set it to that.

[w0rm]

This was fixed in WebGL, by converting Float64Array to Float32Array, see elm-community/webgl#42