Properly calculate binormal when creating SurfaceTool from arrays #88725
Conversation
clayjohn
added
bug
topic:import
regression
cherrypick:4.1
clayjohn
force-pushed
the
GLTF-binormal-sign
branch
from
aeb545a to
6c402de
Compare
|
The test plan is to take a photo of before, and after on:
https://imgsli.com/MjQyMDc4 Image comparison with sliders! Check for the correct arrow location on the sphere and the specular.
|
clayjohn
removed
the
cherrypick:4.1
|
Does U mirroring look correct to you? |
Move the camera over to the right and mirrored U will look the same as the others
|
clayjohn
force-pushed
the
GLTF-binormal-sign
branch
from
6c402de to
f267b2a
Compare
| @@ -340,7 +340,7 @@ void _get_axis_angle(const Vector3 &p_normal, const Vector4 &p_tangent, float &r | |||
| if (d < 0.0) { | |||
| r_angle = CLAMP((1.0 - r_angle / Math_PI) * 0.5, 0.0, 0.49999); | |||
| } else { | |||
| r_angle = (r_angle / Math_PI) * 0.5 + 0.5; | |||
| r_angle = CLAMP((r_angle / Math_PI) * 0.5 + 0.5, 0.500008, 1.0); | |||
There was a problem hiding this comment.
Out of curiosity, where does that 0.500008 come from? It might be worth adding a comment.
There was a problem hiding this comment.
Its the smallest number representable by a 16 bit UNORM value in the range 0.5 - 1.0.
Its the flipside of 0.49999 above.
|
Thanks! |
|
Cherry-picked for 4.2.2. |
akien-mga
removed
the
cherrypick:4.2
|
Seems to fix some of my problem cases. Thanks a bunch! |
Properly calculate binormal when creating SurfaceTool from arrays
Fixes: #86131
I suspect that this will also fix some other subtle bugs.
This bug has existed in the SurfaceTool code for many many years. But it wasn't caught as there were very few situations where this code made a major impact. However, we started using
create_vertex_array_from_triangle_arrays()in the default GLTF import path a few years ago. The result was that we ended up forcing the binormal sign of all GLTF meshes to 1.0. This is fine for most meshes, but the binormal sign is necessary when mirroring normal maps, so losing it totally broke those meshes.edit: I have also pushed a related fix that ensures that the binormal sign is properly encoded when sending it to the shader. This fixes a related bug when using the compressed mesh format
Before:
After