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

PBR Metallic-Roughness Extension Overview #1738

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

proog128
Copy link
Contributor

This pull request adds a section to the glTF 2.0 specification that summarizes all extensions of the PBR metallic-roughness material:

@donmccurdy donmccurdy added this to the PBR Next milestone Jan 16, 2020
@UX3D-nopper UX3D-nopper added the needs discussion Issue or PR requires working group discussion to resolve. label Feb 27, 2020
@proog128 proog128 mentioned this pull request Mar 20, 2020
@proog128 proog128 mentioned this pull request May 20, 2020
@proog128 proog128 mentioned this pull request Jul 1, 2020
@@ -1175,6 +1175,12 @@ The following equations show how to calculate bidirectional reflectance distribu

All implementations should use the same calculations for the BRDF inputs. Implementations of the BRDF itself can vary based on device performance and resource constraints. See [Appendix B](#appendix-b-brdf-implementation) for more details on the BRDF calculations.

#### Extensions

glTF 2.0 defines several optional extensions that make the metallic-roughness material more flexible. The following image shows the complete material including all extensions. The default values for parameters introduced in extensions are chosen in a way that they do not affect the material if the extension is not used.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this section makes sense in the core spec. glTF 2.0 does not define optional extensions. These extensions are based on glTF 2.0, but glTF 2.0 itself does not define them. Maybe this should go in the extensions README?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. Only thing that bothers me a bit is the fact that the information about materials is already scattered across so many places (material chapter, appendix b, several extension readmes), and with this we would introduce yet another place. I probably wouldn't have guessed that the glTF Extension Registry contains information about material semantics, but maybe it's just me :-). Maybe rephrasing to something like "Several optional extensions provide more flexibility to the metallic-roughness material. ....." could be an alternative?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll be okay if we figure out a way to say it such that it's some kind of note (or appendix?) to the spec. The way you have it originally doesn't sound right to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Issue or PR requires working group discussion to resolve.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants