mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.18k stars 35.34k forks source link

Precision issue with r49 #1898

Closed ColasMarko closed 11 years ago

ColasMarko commented 12 years ago

I updated my Three lib to r49 but i'm dealing with many precision issue when rendering my old scene (it was working fine with r48).

The issues only occur when rendering far away from the world origin (further than 2 000 000 on some axis).

1st : my ground is formed of many Meshs built with PlaneGeometry and i now have some 1 pixel width holes between my meshs when moving the camera.

2nd : i'm dealing with a strong precision issue (>50 world unit error) when using projector and Matrix / Vector multiplication.

I tried to revert the Matrix optimization stuff but i didn't manage to get it working with all the function change in this release :-/

mrdoob commented 12 years ago

Can you share ascreenshot?

ColasMarko commented 12 years ago

i'm sorry but i can't show more than this partial screenshot (with a red background to see the holes):

http://i40.servimg.com/u/f40/13/20/89/97/screen10.jpg

These holes are not always visible depending on the camera distance/angle. The camer target is x = 538711, y = 5742553, when i move near the world origin there is no issue.

NB: i didn't change the webgl renderer precision (still highp)

alteredq commented 12 years ago

Hmmm, this is interesting / bad.

I wonder if this is just some bug introduced by optimizations or there is something inherently different when doing math directly on typed arrays instead of JS floats (if I remember well, these are rather large, not sure if types match).

BTW precision specifier shouldn't matter on desktop GPUs, it's supposed to be just for mobile ones.

Edit: Yup, found it. JS numbers are 64 bit floats while new optimized Matrix4 uses 32 bit float typed arrays.

http://www.hunlock.com/blogs/The_Complete_Javascript_Number_Reference

I wonder how much of performance gain actually came from this instead of from typed arrays themselves.

@mrdoob I don't think we should switch to Float64Array everywhere but maybe it could be optional?

@kovleouf Could you try to change these lines to use Float64Array to see if it helps for you?

https://github.com/mrdoob/three.js/blob/master/src/core/Matrix4.js#L15 https://github.com/mrdoob/three.js/blob/master/src/core/Matrix3.js#L7

ColasMarko commented 12 years ago

Using Float64Array lead to this error on 1st frame :

Uncaught TypeError: Type error in WebGLRenderer.js:4754

gero3 commented 12 years ago

I beleive support is missing for Float64Array in firefox

ColasMarko commented 12 years ago

I'm using Chrome 18.0

alteredq commented 12 years ago

Ah no, I think it's because now we send Matrix4's typed arrays directly to WebGL and there it must be 32 bit floats again.

Hmmm, you would need to cast these somehow.

@gero3 Firefox supports Float64Array. It's in typed array specs and I just checked in console ;)

gero3 commented 12 years ago

Ah I'll look into it, then. It will probably not get much slower by changing the matrixes and casting it later. But I have to look into it.

alteredq commented 12 years ago

Hmmm, Float32Array vs Float64Array performance seems quite surprising:

http://jsperf.com/float32array-vs-float64array/2

May be worth to try 64 bit floats?

gero3 commented 12 years ago

https://github.com/gero3/three.js/commit/d0d472d6096eb308c10575c36739f9f2a4d81440

This is my first try on it. As I expected that float64array would be at least as fast as float32

alteredq commented 12 years ago

I'm getting much lower performance with 64 bit floats - 35 fps vs 60 fps on webgl_performance stress test (using @gero3's commit).

ColasMarko commented 12 years ago

I tried with @gero3's commit and it fixes my precision issues.

alteredq commented 12 years ago

Yay! So I guess this should be toggleable by some flag (with default set to use Float32Array).

@mrdoob Maybe something like this?

THREE.FloatArrayType = Float32Array;
THREE.FloatArrayType = Float64Array;

Not sure though how to override this nicely from the application layer, there are objects which are created simply by including the lib, so these must be somehow aware of the override :/.

It could be something like this, but it's quite ugly:

<script>
var THREE =  { FloatArrayType: Float64Array };
</script>

<script src="../build/Three.js"></script>

And then inside Three.js:

if ( THREE.FloatArrayType === undefined ) {

    THREE.FloatArrayType = Float32Array;

}

And in Matrix4:

this.elements = new THREE.FloatArrayType( 16 );
mrdoob commented 12 years ago

Maybe something like THREE.Matrix4.set64bit()? which would set a static flag that every matrix would check at instantiation time. It's basically Matrix4, right?

alteredq commented 12 years ago

Matrix4 and Matrix3.

But trouble are static matrices that are created at lib loading time - if we set it like this, they would have been already created in a different way and thus incompatible :S.

Now we could reset these in THREE.Matrix4.set64bit() but this would be also bad design, fragile as we would need to collect all such static matrices there.

mrdoob commented 12 years ago

Meh...

@kovleouf would anything change if you divided all your values by 1000?

alteredq commented 12 years ago

@mrdoob Another option could be doing this at the build time (we would build the lib with either 32 or 64 bit typedef file included).

Then we would have a special version of the lib Three64.js, to be used if application needs higher precision (somehow similar to how binary applications have 32 and 64 bit versions).

@kovleouf Sanity check - do you use proper mathematical constants everywhere?

For example I remember having precision artefacts when I used numbers like 3.14 instead of Math.PI. This was ok when my scenes were small, but on large scenes imperfections were popping up (especially treacherous were rotations of larger planes, tiny angular difference can make a huge displacement).

mrdoob commented 12 years ago

So seems like the only option is having it run slower. I wonder if that's ok with @kovleouf or whether there's something that he can do on his app code instead ;)

ColasMarko commented 12 years ago

@mrdoob : i can't divide everything by 1000 because i'm working on an earth map using real lon / lat coordinate system so it would be a pain to convert everything.

@alteredq : i'm using Math.XXX constants, the precision error is due to the huge size of my world.

I will investigate the performances with r48, r49 32bits and r49 with the quick 64bits hack today

ColasMarko commented 12 years ago

I'm running with 6-10 ms / frame on r48 and r49 and 10-15 ms / frame on r49 with float64Array.

I think i'll keep my project under r48 for now

mrdoob commented 12 years ago

What browser/system is that on?

ColasMarko commented 12 years ago

Window XP SP3 Intel core 2 Duo 2.4Ghz 3.50 Go RAM GeForce GTX 260 Ultra Chrome 18.0.

I tried with the 2nd version of @gero3 (Pull Request #1903) and i'm running with 8-12ms / frame. (i can't give a real FPS because i refresh the frame only when something changes)

mrdoob commented 12 years ago

So you get 6-10ms with Number, 10-15 with Float32Array and 8-12 with Float64Array?

ColasMarko commented 12 years ago

no : 6-10ms with Number 6-10ms with Float32Array but precision issue 8-12ms with Float64Array and conversion code to prevent precision issue

cipri-tom commented 7 years ago

@kovleouf do you happen to remember how you dealt with precision issue ? I'm also working with earth and the single precision cannot hold all the values. Thank you!

m-schuetz commented 7 years ago

@cipri-tom

In Matrix4.js, replace

this.elements = new Float32Array( [

with

this.elements = new Float64Array( [

and in WebGLUniforms.js replace

gl.uniformMatrix4fv( this.addr, false, v.elements || v );

with

gl.uniformMatrix4fv( this.addr, false, new Float32Array(v.elements) || v );

Seems to work for me. The matrices are converted to single precision float for the shaders which is fine because the translational terms of the world and the view matrices tend to cancel each other out. Make sure to use the combined "modelViewMatrix" in your shader.

On a related note, I'm just now switching to using Float64Array for the matrices in order to avoid confusing floating origin transformations. I can do a comparison for my use case when I'm done. Kind of glad to see that you just have to modify 2 lines to get double precision math on the CPU side.

cipri-tom commented 7 years ago

@m-schuetz Thank you for the solution! It's a good idea and should work in most scenarios.

I'll give it a try, but I'm not sure it would work for me as the elements are 'static' in the scene (drawing up the earth) so there are no further translations to bring them into the range of single precision. You simply cannot fit all of them at the same time in single precision.

The way I solved it in OpenGL (in a C program) was to use the "old-style" of GL, with push-pop matrices and drawing separate sections.

mshopf commented 7 years ago

@m-schuetz: Thanks for your analysis - I had the same issue with in a similar setting. I was already thinking about more complex solutions (like in http://github.prideout.net/emulating-double-precision ), but it doesn't seem to be required in my case as well.

I'd just suggest to use a local conversion Float32Array, so that you don't need to create a new Object every time; also I think the use of || is wrong in your code (I might have missunderstood it, though):

 +    var convert32Array16= new Float32Array(16);
 [...]
      function setValue4fm( gl, v ) {
 -       gl.uniformMatrix4fv( this.addr, false, v.elements || v );
 +        convert32Array16.set (v.elements || v);
 +       gl.uniformMatrix4fv( this.addr, false, convert32Array16 );
      }