-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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_subsurface #1928
KHR_materials_subsurface #1928
Conversation
Is it possible to add this one to Extension List in main repo? |
@Reon90 — if you mean this list in the extension registry, note that it has several sections, including Khronos extensions that are (a) already ratified, or (b) in progress and actively moving toward ratification. At the moment, this PR is just a PR and nothing more. I'm guessing that your comment is meant as a +1 for supporting SSS in glTF in the future? If so, noted! If not, please let us know if you have any other preferences or concerns here. 🙂 |
(Related: I think something like #1970 may help to communicate the status of PRs more clearly) |
Looking at the Khronos Sample Viewer implementation for transmission and volume, I was wondering what the shader code would look like just for the SSS portion of Volume. Our team wants to implement SSS and I wanted to setup support for this extension so they could see the results, but I'm not sure how to detangle the SSS code from the volume code in 'ibl.glsl' located here: https://github.com/KhronosGroup/glTF-Sample-Viewer/blob/master/source/Renderer/shaders/ibl.glsl Could someone help? For these type of extensions have the sample shader source is really, really useful for us devs. |
"properties": { | ||
"scatterDistance": { | ||
"type": "number", | ||
"minimum": 0.0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scatterDistance should declare the minimum as an exclusiveMinimum
. See the schema for KHR_materials_volume
Replace pseudo equations with latex math Wording
Apologies in advance if this goes into bike-shedding territory, but to me the |
I've no strong opinion here. Agreed that KHR_materials_sss naming feels somewhat inconsistent. On the other hand, the term SSS is almost being used like a name in itself, it's short and certainly triggers the right associations for many people. |
No strong opinion from me either, but I notice the name "scatter" is already in the parameters (
|
Hello all! Just to chip in if I may 😄 In Unreal, we also use the shortcut Additionally, we also allow the |
@aidinabedi, thanks for the input. We explicitely defined the current extension proposal without texturable parameters. With the new glTF PBR extensions we decided to strictly separate surface from volume effects. Texturing volumetric parameters with a surface texture will change the results of the volume evaluation based on the volume entry point. This may be perfectly fine for a rasterizer that uses a surface-based approximation to volume integration, e.g. diffusion approximation. But for path-tracers with bi-directional integration methods such behavior will destroy reciprocity. So you end up with inconsitent results for different rendering techniques, which is something we were trying to prevent by design. We recently added a Not saying that the parameterization is set in stone though, we're still discussing alternatives back and forth. If you have example scenarios and you're allowed to share, we' re more than happy to look at them! |
Further discussion should take place in #2453, not here. Closing. |
The subsurface scattering part of KHR_materials_volume extracted to its own extension. Please note, this extension drops the parameterization via
subsurfaceColor
in favor of a parameterization which is semantically more detached from the absorption definition in KHR_materials_volume. New parameters arescatterColor
andscatterDistance
, from which we can directly derive the scattering coefficient as defined in the readme.