Three.js: OBJLoader: unexpected change in normals calculation

[yellowtailfan]

PR #19338 changed how OBJ files without normals are handled by OBJLoader. Previously if an OBJ file had no normals, then BufferGeometry.computeVertexNormals() was called. After the PR, missing face normals are calculated via OBJLoader.addFaceNormal(), and computeVertexNormals() is no longer called.

We identified this change as the cause of an issue for our team, where our render lighting stopped working for a loaded OBJ file with no normals. It took some time to identify the cause of our issue because the change was not apparent to us from the release notes or migration guide.

A work around is to explicitly compute vertex normals after the OBJ file is loaded, e.g.:

mesh.traverse(function(child) {
  if (child instanceof THREE.Mesh) {
    child.geometry.computeVertexNormals();
  }
});

To Reproduce

1. Use OBJLoader to load a mesh with face values that have no normals:

f 2/2 3/3 62/62
f 3/3 4/4 63/63
f 4/4 5/5 64/64
...

1. Rendering of the mesh before PR #19338 has correct lighting on the mesh.
2. Rendering of the mesh after PR #19338 has no lighting on the mesh.

Suggested resolution

One of these options might be a good resolution:

1. Change the normal calculation in OBJLoader to match the algorithm in BufferGeometry, or
2. Add a note on the changed behaviour to the migration guide for r117, or
3. This bug report might help others who encounter this problem.

[Mugen87]

OBJLoader does not generate indexed geometries. So computeVertexNormals() has already effectively produced face normals.

Can you please describe in more detail how the previous and current implementation differ in their visual result? Can you please share a OBJ file for testing, too?

[yellowtailfan]

Vertex normals matter to our use case (face normals don't). We have a custom shader which relies on vertex normals to produce the specular response. Prior to r117, vertex normals were being generated by OBJLoader and our specular response was correct. After r117,OBJLoader stopped generating vertex normals, and our specular response was missing.

Here's an example OBJ file in the style that we're using:
https://www.bandicootimaging.com.au/retail/biv_gallery/textures/yellow_bag/brdf-mesh.obj

[gkjohnson]

@Mugen87 I just took a look and I don't think this is an issue of vertex vs face normals. Rather when loading an OBJ that has no normal data described then it will not set the normals at all whereas it was previously calling computeVertexNormals in that case. I'll make a PR.

[yellowtailfan]

Just confirming that this merge has resolved our problem: with this commit we get correct normals and specular rendering without having to call computeVertexNormals() after calling OBJLoader.

Thanks all!