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 specular-glossiness workflow and some gui for tweaking values #2

Merged
merged 3 commits into from
Nov 16, 2016
Merged

Add specular-glossiness workflow and some gui for tweaking values #2

merged 3 commits into from
Nov 16, 2016

Conversation

bghgary
Copy link

@bghgary bghgary commented Nov 10, 2016

This change provides an example of the specular-glossiness workflow and make it easy to see how the shaders calculation differ between the two. It also adds dat.gui for tweaking some properties on the fly.

Changing between metallic-roughness and specular-glossiness workflow makes no difference to the rendered result. The difference is in the underlying parameters being passed to the shader and how the math is calculated.

The following parameters were added:
workflow (switches between specular-glossiness and metallic-roughness)
albedo
opacity
reflectivity (0.16 * reflectivity^2 = dielectric f0 value) (see mrdoob/three.js#8505 (comment))

The current PBR proposal does not include reflectivity, but this comment (KhronosGroup#696 (comment)) seems to indicate that we want to add this.

See here for a live view: https://bghgary.github.io/glTF/add-specular-glossiness-workflow/

@mlimper
Copy link
Collaborator

mlimper commented Nov 15, 2016

Thanks a lot for this PR!

The reflectivity, or in general a way to use different F0 values, is something that was already requested at some point. For now, I think it is awesome to have things like this in the example, so everybody can check what these values mean. The discussion will greatly benefit from this.

Just one question:

If a reflectivity value different than zero is used, the result changes between the two workflows. Is this intended?

@bghgary
Copy link
Author

bghgary commented Nov 15, 2016

No, this is not intended. I will take a look.

Update: Found it. Thanks for catching this! 👍

@mlimper mlimper merged commit 6ca13dd into tsturm:master Nov 16, 2016
@mlimper
Copy link
Collaborator

mlimper commented Nov 16, 2016

Thanks! :-)

@bghgary bghgary deleted the master branch May 1, 2017 20:23
tsturm pushed a commit that referenced this pull request Oct 18, 2021
Revert "Added Datakit CrossCad/Ware SDK"
tsturm pushed a commit that referenced this pull request Oct 18, 2021
Update to latest Khronos master
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.

2 participants