AcademySoftwareFoundation / MaterialX

MaterialX is an open standard for the exchange of rich material and look-development content across applications and renderers.
http://www.materialx.org/
Apache License 2.0
1.83k stars 335 forks source link

Add 'bitangent' input to 'normalmap' node #945

Closed pablode closed 2 months ago

pablode commented 2 years ago

The normalmap node always calculates the bitangent and ignores existing bitangent data: https://github.com/AcademySoftwareFoundation/MaterialX/blob/46de5f9153aec01243fce24590b15fd71da89f8e/libraries/stdlib/genglsl/mx_normalmap.glsl#L9-L11

While doing so, it makes a hard assumption on the handedness of the tangent space, which isn't necessarily shared by other standards (such as glTF).

A bitangent input should therefore be added to the normalmap node with a default value of Bworld.

jstone-lucasfilm commented 2 years ago

@pablode This sounds like a good proposal to me, and it's more general-purpose than the alternative of supporting a handedness flag on the normalmap node.

pablode commented 1 year ago

Hello! Just wanted to ask if this proposal has been discussed in the context of an upcoming version of the spec?

I believe @kwokcb might be interested in this too, since it's required for truthful support of glTF normal mapping.

jstone-lucasfilm commented 1 year ago

@pablode I do think this feature should be included in the 1.39 specification, and it should be safe to implement it in an upcoming 1.38 point release, since its functionality is purely additive.

Would you be interested in proposing the language for this feature in the 1.39 specification? If so, then reach out to @dbsmythe on the ASWF Slack to get access to the in-progress document.

pablode commented 1 year ago

Unfortunately, I don't think this change can be treated as purely additive. This may be true for nearly all uses in the wild, however, it is only the case for default normal and tangent input values.

If the user has connected nodes to these inputs, a <crossproduct> node which connects to the original input nodes needs to be inserted and connected to the new bitangent input in order to keep the old behaviour. This can be done by the document upgrade logic, however, it would require this change to be introduced with 1.39 (and not earlier).

The change in the spec should should be minimal, and I'll contact Doug about that, thanks! I can also set up a draft PR with the implementation which can be merged once 1.39 is released.

jstone-lucasfilm commented 2 months ago

Thanks for this original report, @pablode, as well as for the fix in https://github.com/AcademySoftwareFoundation/MaterialX/pull/1911.