Three.js: The normal matrix should not be used to transform tangents

[WestLangley]

The normal is transformed by the normalMatrix, which is based on the inverse-transform.

The tangent should be transformed like any other direction vector so it remains in the tangent plane.

However, BufferGeometry.applyMatrix() transforms the tangent like so:

normalMatrix.applyToBufferAttribute( tangent );

This is not correct. It also leaves the tangent unnormalized.

Similarly, in the shader, defaultnormal_vertex.glsl.js does this:

vec3 transformedTangent = normalMatrix * objectTangent;

In the examples, TerrainShaderuses a similar pattern:

vTangent = normalize( normalMatrix * tangent.xyz );

[mrdoob]

Ops...

[donmccurdy]

That sounds like my mistake. 😓

https://github.com/mrdoob/three.js/issues/18006 looks good!

[WestLangley]

@donmccurdy What do you think is the better pattern to follow in fixing defaultnormal_vertex.glsl.js.

vec3 transformedTangent = ( modelViewMatrix * vec4( objectTangent, 0.0 ) ).xyz; // un-normalized

// or

vec3 transformedTangent = transformDirection( objectTangent, modelViewMatrix ); // normalized

The 2nd pattern uses the built-in method, but performs an unnecessary normalization because the tangent is normalized again later in a different chunk.

[WestLangley]

@mrdoob Are you compelled to retain the TerrainShader example? It is computing a vertex normal and then redefining it later. TBH, I'm not sure I want to deal with it... but I expect it brings back memories for you. :-)

vNormal = normalize( normalMatrix * normal );
...
vNormal = normalMatrix * normalTex;

[mrdoob]

Actually, I wouldn't mind removing the TerrainShader to be honest 😇

[WestLangley]

TerrainShader has been removed.

Tangent transform in the shader has been fixed.