Add tangent vectors to 2 models per recent tangent basis clarifications

[emackey]

Fixes #133, I believe.

Accomplished using Don's gltf-transform/cli with the tangents command.

[lexaknyazev]

The issue is certainly fixed here but with two concerns.

1. All meshes now have tangents, including those that do not use them. This leads to a lot of new warnings.
2. How did the engines treat the affected models before this PR? Were they consistent and/or correct?

[emackey]

For (1), generally model authors don't have a lot of control over which exported meshes get tangents vs. not. For example, in both Blender and gltf-transform, the generation of tangents is an all-or-nothing single switch. Perhaps various exporters and pipeline tools could be made to automatically determine the need for tangents per-mesh based on the clarified rules going into the spec here, but that tooling does not currently exist, and the exact rules were not previously known.

Something similar is happening with TEXCOORD_0 here as well. In Blender, I can easily delete Blender's "UVMap" attribute on a per-mesh basis. But in the case of the ClearCoatTest asset, a single test mesh is reused over a dozen times, sometimes with textures, and sometimes not. Blender exports the same mesh to multiple identically-named glTF meshes, using the same attributes but applying a different material index to each one. However, the Blender exporter does not understand which copies of the exported mesh actually need TEXCOORD_0 and which do not, so it is included in all of them. I could fix it by hand I suppose, but we don't have tooling to do this for end users.

For (2), I've only investigated one software package so far, software that I work on internally. It calculates only a single TBN per fragment, using the TANGENT attribute if available, else using the TEXCOORD_0 attribute. There's a special pound-define in the shader for a special case where the normal map (not clearcoat normal) is using index 1, then the code changes to rely on TEXCOORD_1 in place of zero. Likely, the tangents were incorrect in the UV1 column of the Clearcoat Normal row, but this test asset is testing texture transform, not correctness of tangents, so a checkmark still appears.

Did adding tangent vectors fix it? Well it made it less ambiguous that the software should load the TANGENT attribute for use in the TBN, of course. But how did gltf-transform calculate those TANGENT vectors? The Clearcoat UV1 box has two different sets of texture coordinates on it, and no base normal map. Did gltf-transform know to use TEXCOORD_1 (instead of 0) when running its copy of MikktSpace? If not, we may have simply hard-coded some incorrect tangents here.

I certainly won't claim that the shader code I described earlier is desirable or representative of how other software packages work. We will have to investigate the source code for each package that we're curious about.

[lexaknyazev]

This PR technically fixes the missing tangent space issue so it's an improvement in any case.

But how did gltf-transform calculate those TANGENT vectors?

@donmccurdy What UV set is used for generating tangents when there is no normal texture on the base material?

[donmccurdy]

The current behavior is to use the normal map's UVs if present, or TEXCOORD_0 if not:

https://github.com/donmccurdy/glTF-Transform/blob/0d50d8b2dec869a6f5c0c2c3a367be65e6795e31/packages/functions/src/tangents.ts#L123-L135

The operation doesn't look at any other textures, but those would be easy enough to add. I suppose it should look for texcoord overrides in KHR_texture_transform as well, I'd forgotten about that case.

[lexaknyazev]

The intention, as clarified in KhronosGroup/glTF#2409 and KhronosGroup/glTF#2415, is to ensure the following.

1. A mesh cannot have more than one implicit tangent space, i.e., it must not omit TANGENT data and use different texcoords for different textures that need a tangent space, e.g., normalTexture, clearcoatNormalTexture, and/or anisotropyTexture, at the same time.
2. An implicit tangent space can be derived only from the base material. So if the latter does not have a normalTexture, engines must not use texcoords of clearcoatNormalTexture or anisotropyTexture for generating tangents.

That said, it would be great if glTF-Transform could "fix" meshes that do not follow these rules.

[emackey]

Hey @donmccurdy, hope things are going well for you!

The operation doesn't look at any other textures, but those would be easy enough to add. I suppose it should look for texcoord overrides in KHR_texture_transform as well, I'd forgotten about that case.

Yes, this is the conclusion we came to in PBR TSG recently: The TANGENT mesh attribute should be calculated from MikkTSpace of the chosen TEXCOORD_n with KHR_texture_transform applied, where the TEXCOORD index comes from any user of tangents: normalTexture, clearcoatNormalTexture, and anisotropyTexture. If the three don't agree on TEXCOORD index, or if they are transformed with different amounts of texture rotation, then one cannot build a glTF where the TANGENT attribute matches all its users. (We made the decision to disallow multiple TBN matrices per fragment, because most implementations won't or can't do that). The tool could report an error or warning if the asset is trying to use different tangents in the same mesh.

The tool could also discard unused tangents to save some validator warnings. But one caveat is that anisotropy itself can make use of tangents even without any texturemap being present, because anisotropy stretches roughness along tangents regardless of whether or you've textured it. So, tangents are needed by any mesh that has normalTexture, clearcoatNormalTexture, or any anisotropy at all. So, discarding them has that extra wrinkle in the rules, not quite as simple as checking three texturemaps.

[lexaknyazev]

with KHR_texture_transform applied

If texture transforms are animated, that would not work. I think we agreed to split this issue and investigate whether a runtime TBN transformation based on texture transform is possible.

[emackey]

If texture transforms are animated, that would not work.

True. I guess we should investigate what shader changes are needed to incorporate texture rotation into tangent vectors at runtime. My hunch is that it should be just a rotation about the geometric normal, but I haven't done the math in full.

[DRx3D]

Approved after discussion in Tooling TSG. Ed/Alexey to create issue against repo for remaining issues/points.