Fix skinning precision issue on iOS

[zeux]

On iOS, by default the bones that are sampled from the bone texture look
like they are using mediump. This can result in noticeable jitter or
other more severe issues depending on the bone transformation matrix.

Fix this by explicitly declaring the sampler as highp.

Here's the motivating video showing the precision issues:
https://twitter.com/zeuxcg/status/1136158107573211136

Curiously, you can see this in official three.js examples, it's just not as extreme -
if you open animation/skinning/blending sample on an iPhone (I've tested this
on iPhone X with latest iOS 12 update), if you look closely, the head of the soldier
jitters during animation. This is the same precision issue, and this change fixes the
problem.

Fixes #13288.

[zeux]

Please let me know if you'd like me to rebuild build/ files - I didn't include them in the PR since when I run npm run build, there are more changes than just this one (fromEquirectangularTexture addition mainly)

[Mugen87]

Related #13288

BTW: It's better to not update the build files 👍

[WestLangley]

Can you please provide a live example to demonstrate the issue?

[zeux]

@Mugen87 Thanks! Wasn't aware of this - updated the PR to fix that bug when this gets merged.

@WestLangley As noted in the PR description, you can actually see this on the built-in demo. Here's a URL that was used to generate the video from the tweet as well: http://zeux.io/tests/junkrat-fp16

[mrdoob]

Nice! Curious to see if there are side effects. I bet a random Samsung phone user will complain 😁

We can adjust when/if that happens.

[mrdoob]

Thanks!

[zeux]

@mrdoob I believe this fix should have no side-effects on other devices - every device I've tested other than iPhones works well so presumably they all are already using highp.

GLSL ES spec (that I believe WebGL inherits) says:

The vertex language has the following predeclared globally scoped default precision statements:
precision highp float;
precision highp int;
precision lowp sampler2D;
precision lowp samplerCube;
The fragment language has the following predeclared globally scoped default precision statements:
precision mediump int;
precision lowp sampler2D;
precision lowp samplerCube;

So really it looks like iOS is taking advantage of the fact that sampler2D is defined to return low-precision values and uses fp16, whereas all other devices (I've tested) use fp32. So technically Safari is right here, and the fix just makes sure we demand actual highp results from the sampler.

http://zeux.io/tests/junkrat uses a version of three.js after this fix and appears to work well everywhere.

[titansoftime]

Yes! Finally! Thank you. This is my skinned mesh on my wife's brand new iphone:

[zeux]

@titansoftime Hopefully this is before this fix 😅

[titansoftime]

@titansoftime Hopefully this is before this fix

Ha, yes. Will test fix when mrdoob updates the dev build.

[WestLangley]

This is not the solution to the problem.

The model in this example is scaled by 0.0026.

Also, in this example, bindMatrix is the identity, yet bindMatrixInverse is scaled by ( 511, 511, 511 ) and translated by (393, 0, 262 ). Clearly that does not make sense, but that is what three.js does.

three.js implements skinning in world space instead of local space. That leads to precision problems.

There does not seem to be any interest in fixing the real problem, however. And for that, I am disappointed.

[zeux]

@WestLangley On every platform other than iOS, bone data uses fp32, and on iOS it uses fp16. This is the problem that this PR fixes. After this, three.js works the same on all platforms.

World space skinning is an orthogonal issue. fp16 is sometimes insufficient even in local space - see the soldier example from the three.js demos.

[WestLangley]

I did not say not to merge this PR.

[titansoftime]

I've tested this fix (just patched it manually),

Unfortunately WestLangley is correct (as always). The bug remains =[

[zeux]

@titansoftime Have you updated it in your local test sandbox or on titansoftime.com? I definitely see a precision issue on my iPhone but I don't see the update in the .js files. Since desktop works fine I'm pretty sure this should fix the problem that I'm seeing on your game as well, but maybe your screenshot is from a different position in the world where other precision issues kick in on all platforms.

[titansoftime]

Yea I tested live. Same bug on iphones. Everything else is fine (desktops and my android).

[zeux]

I don't see this change in https://www.titansoftime.com/webgl/Three104dev.min.js, presumably this is the file that's being used. This should have "highp" after the change.

[WestLangley]

see the soldier example from the three.js demos.

@zeux That demo has the same problem your demo has: the root is scaled by 1/100, the bind matrix is the identity, while the so-called inverse of the bind matrix has scale 100. Hence, I am not surprised to see artifacts similar to those in your demo.

[titansoftime]

Yea, I reverted the change in my code.

Edit, I put it back so you can see.

[zeux]

@titansoftime Interesting - I do see the issue still. I’m wondering if there’s some other iOS specific precision issue that manifests in your setup. I will look into this more; might be worth creating a separate more specific issue and continue the conversation there.

[titansoftime]

I believe issue #13288 addresses this. Perhaps I am misunderstanding this but even if my mesh is positioned at world coordinates 0, 0, 0 the issue remains (yet oddly somewhat corrects itself as I rotate the mesh when it reaches a certain rotation on the y axis).

The workaround presented in #13288 I'm sure works, but sounds like it limits the usability of skinned meshes as described in comment #13288 (comment)

[zeux]

I think at this point if the iOS bug is still there for some usecases we need a new issue that has a standalone simple repro case. Links in #13288 are dead; the examples I've provided as well as the three.js animation example are fixed with this change as far as I can tell.

[callmegaga]

I hava test when a fbx model，put at (1000000, 1000000,0), it will have the same bug.On win10 and chrome 74

[mrdoob]

@callmegaga considering 1 unit = 1 meter... you're placing an object in 1000km, 1000km, 0. What's your use case?

[callmegaga]

@

@callmegaga considering 1 unit = 1 meter... you're placing an object in 1000km, 1000km, 0. What's your use case?

I just test it. In fact, there is no such case.