Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add glTF PBR #861

Merged
merged 2 commits into from
Mar 14, 2022
Merged

Conversation

proog128
Copy link
Contributor

@proog128 proog128 commented Mar 8, 2022

This PR adds a node definition and implementation of the glTF PBR material with the following extensions:

  • KHR_materials_transmission
  • KHR_materials_specular
  • KHR_materials_ior
  • KHR_materials_sheen
  • KHR_materials_clearcoat
  • KHR_materials_volume

For extensive discussion and test models see KhronosGroup#1.

Known issues

No thin-walled support yet. In glTF, the material can either be thin-walled (thickness = 0) or volumetric (thickness > 0). A volumetric material has refraction. The implementation in this PR only supports the volumetric case, i.e., the node graph ends with a surface node. The alternative would be thin_surface, but this seems to be not supported by MaterialXView. Besides, I wasn't able to figure out how to switch between surface and thin_surface based on an input.

Occlusion is ignored. There is no concept of occlusion in MaterialX.

Thickness is ignored. There is no concept of a thickness map in MaterialX. Nevertheless, the parameter could be used to switch between thin-walled and volumetric.

Potential future additions

Add node definition and implementation for texture fetches. Most material parameters in glTF have a "factor" and a "texture". For example, there is baseColorFactor that modulates baseColorTexture, i.e., baseColor = baseColorFactor * texture(baseColorTexture, uv). Normal maps, on the other hand, have a scale: normal = normalize(texture(normalTexture) * 2 - 1) * vec3(scale, scale, 1). By using these nodes for texture fetches implementations can ensure compatibility to glTF.

Add support for KHR_materials_emissive_strength. Waiting for the extension to be ratified. This makes it possible to use values for emission that are greater than 1.

Add support for KHR_materials_iridescence. Waiting for the extension to be ratified. Layers a thin_film node on top of the specular BSDF.

@jstone-lucasfilm
Copy link
Member

jstone-lucasfilm commented Mar 13, 2022

This looks fantastic, thanks @proog128. For the new folder containing example materials in the gltf_pbr shading model, would it be acceptable to use the name GltfPbr for consistency with our other folders of example materials, or would this reduce clarity? The name of the gltf_pbr shading node is fine as it stands, and this proposed change would only affect the name of the new examples folder.

@proog128
Copy link
Contributor Author

The folder is renamed to GltfPbr. Thanks for your feedback!

@fire
Copy link
Contributor

fire commented Mar 14, 2022

Given one is trying to test MaterialX gltf principled pbr property mapping is there a unit test-able check for validity when compared against a reference photo.

The idea would be to provide a feature test gltf MaterialX and an associated obj similar to https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/SpecularTest

@emackey
Copy link

emackey commented Mar 14, 2022

@fire It looks like @kwokcb is doing something with tests, for example: kwokcb@882c7de

@jstone-lucasfilm jstone-lucasfilm merged commit b58042e into AcademySoftwareFoundation:main Mar 14, 2022
@emackey emackey deleted the gltf branch March 14, 2022 23:26
Michaelredaa pushed a commit to Michaelredaa/MaterialX that referenced this pull request Oct 21, 2023
This PR adds a node definition and implementation of the glTF PBR material with the following extensions:
* KHR_materials_transmission
* KHR_materials_specular
* KHR_materials_ior
* KHR_materials_sheen
* KHR_materials_clearcoat
* KHR_materials_volume
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants