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

KHR_materials_clearcoat extension #1756

Merged
merged 9 commits into from
Feb 27, 2020
Merged

Conversation

emackey
Copy link
Member

@emackey emackey commented Feb 7, 2020

This is a continuation of #1740. I wasn't able to push updates to UX3D's repo, so I've moved the branch here to the main repo.

The changes here add an Overview and remove the TODOs. @abwood and I also fine-tuned some descriptions of parameters, just for clarification (it will not require any changes to existing implementations).

Alex and I also considered @proog128 comments in #1740 (comment), but until the Appendix B changes are ready to merge, it does not yet make sense to reference them here. Once Appendix B is updated, the descriptions here that reference Appendix B will also need updating as described in the comment.

@lexaknyazev
Copy link
Member

Nit: the spec should mention what is the normal map for clearcoat layer when clearcoatNormalTexture is undefined.

@donmccurdy donmccurdy added this to the PBR Next milestone Feb 12, 2020
@MiiBond
Copy link
Contributor

MiiBond commented Feb 13, 2020

I'm sure that this was probably discussed a long time ago but there are other properties that sometimes appear in clear coat parameterizations like anisotropy, ior and tint. I certainly understand the desire to keep our extension simple, both for ease of implementation and for cross-renderer compatibility but I wonder if we're limiting this extension a bit too much. Tint, for example, would be very easy for renders to support.
Ultimately though, it's usage that should drive our decision so perhaps we should reach out to, say, the 3D Commerce group and see what their interest in clear coat tinting is. If there are virtually no real-world products/assets that need tint, then perhaps we can confidently disregard it.

@emackey
Copy link
Member Author

emackey commented Feb 13, 2020

@lexaknyazev Done.

@MiiBond Thanks. I'm concerned about feature-creep here. I'd like to keep this extension compatible with existing renderers that have a simple clear-coat function. Perhaps there could be a more advanced KHR_materials_coating or KHR_materials_layered type of extension to handle these more exotic terms? As it stands, this extension is relatively low-hanging fruit, and already has working implementations in BabylonJS and other engines.

@MiiBond
Copy link
Contributor

MiiBond commented Feb 19, 2020

Yeah, that's totally fair. An additional extension at a later date is probably the best way to handle additional parameters that are in use by some runtimes and not others.

@donmccurdy
Copy link
Contributor

Draft implementation in Blender: KhronosGroup/glTF-Blender-IO#931

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants