atteneder / glTFast

Efficient glTF 3D import / export package for Unity
Other
1.25k stars 251 forks source link

Missing normal must be calculated as flat normals #340

Open asus4 opened 2 years ago

asus4 commented 2 years ago

Describe the bug

First of all, thank you for continuing to develop such a great library.

I found some rendering differences between glTFast and web-viewers. It seems to happen when normal is not included in glTF/glb file.

compare

This is my export setting in Blender. It seems to only happen if I exported glTF without normal. And other cases worked well.

export-setting

Files

normal-test.zip

Following files are included:

To Reproduce Import the attached file to Unity.

Expected behavior

Tangent should be not smoothed if there are no normals included in glTF.

Desktop (please complete the following information):

atteneder commented 2 years ago

@asus4 Thanks for reporting. I can confirm that this a bug. The glTF spec says:

When normals are not specified, client implementations MUST calculate flat normals and the provided tangents (if present) MUST be ignored.

glTFast uses Mesh.RecalculateNormals to calculate missing normals. It considers all all connected vertices of a vertex, which does something different in case a vertex is assigned to more than one triangle (of different face-normal).

I selected split edges on your test icosphere and as you can see, some vertices share more than one triangle:

image

The solution would be to:

Or alternatively (and probably faster)

As you can see, this is a bit more complicated and that's why I dodged the topic so far.

atteneder commented 2 years ago

On another note, again from the spec:

When normals are not specified, client implementations MUST calculate flat normals and the provided tangents (if present) MUST be ignored.

Given this information it makes no sense that Blender's export settings allow you to not export normals but DO export tangents.

asus4 commented 2 years ago

@atteneder Thanks for looking into it.

I had this issue when supporting the C4D default exporter. I guess it has similar options. https://storage.opensea.io/files/91525ba8a789da1b79155ff63c3c3c5a.gltf https://storage.opensea.io/files/336992a385208bb9d2e8675f05f47140.gltf

In Three.js GLTF loader, it seems to override normals in the shader (with flat-shading option) instead of modifying the mesh itself. Would it be simpler?

atteneder commented 2 years ago

In Three.js GLTF loader, it seems to override normals in the shader (with flat-shading option) instead of modifying the mesh itself. Would it be simpler?

Yes, it would be. I did not consider this option though, because Unity compiles shaders at build-time. Offering this option would double the number of shader variants, which is already very high.