Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Added initial draft for OSL closures #614
Added initial draft for OSL closures #614
Changes from all commits
1bd3a42
7c3c875
743f74b
33c22e5
a8073dd
9b6129c
a932213
ad87d93
cec87b6
78e913a
f250013
7791e3f
84ed703
22acb3f
9eaeb43
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
How is external IOR / IOR layering tracked? If it's done behind the scenes by passing the IOR ratio to the closure, that may not be enough information for the generalized Schlick BSDF closure.
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.
@nhoffman-lucasfilm That's a great question. I believe this will be tracked through the use of the medium_vdf closure that describes the medium which the light travels through before hitting a new interface. The medium_vdf has an
ior
parameter that is an absolute IOR value for the medium.For the primary intersection I guess air/vacuum is assumed outside. I'm not sure how it's handled for situations where the camera starts inside, like a camera under water, but I suppose a media closure could be attached to the camera.
For cases where BSDFs are layered without the use of media in-between I think all the needed information should be available still, since IOR could be derived from the parameters on each BSDF.
But maybe people from Imageworks, Adsk/Arnold or other teams that have implemented OSL closures in practice can fill in more details here?
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.
We attach medium data to every ray (if present). If a camera is inside some medium we find the object and evaluate the shader to grab what you're calling medium_vdf here. We also compute relative IOR outside the shader. Basically we intercept the BSDF creation from closure data and override the IOR parameters.
May be worth mentioning that we have an optional parameter to all bsdfs called "force_eta" (float from 0 to 1). This allows the shader, and the artist, to override the dynamic IOR. For instance, if the look changes too much for them when underwater, they can tone down the change.
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.
We track the IOR of the bulk medium as the ray transmits through dielectric boundaries, via the usual priority system approach. The interior bulk medium IOR is taken from the specular lobe in the standard surface model (i.e. this lobe effectively defines the IOR of the bulk). We don't take into account IOR jumps between the various layers in the model though (e.g the coat BSDF "above" the specular layer, is not aware of the specular IOR), which is probably a reasonable approach considering the closure mixture model is a somewhat ad-hoc representation of the layered structure anyway.
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.
We don't currently handle the "camera under water" situation, unfortunately. Though it seems that determining this is a matter for the renderer to handle, e.g. by casting a ray to infinity and tracking the media.
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.
"We also compute relative IOR outside the shader. Basically we intercept the BSDF creation from closure data and override the IOR parameters" - in other words the ratio (external_ior / bulk_ior) is passed into the closure's "ior" parameter? In that case, for the generalized_schlick_bsdf closure a different parameter adjustment would be needed, adjusting f0 using something like the equations from slides 107-108 or slide 111 in https://blog.selfshadow.com/publications/s2020-shading-course/hoffman/s2020_pbs_hoffman_slides.pdf (a similar adjustment might need to be made for the f90 parameter - I've spent a little bit of time on that but don't have a formula for that one ready yet). Would this kind of closure-specific parameter adjustment be feasible, or is does that logic need to be agnostic to which closure is used?
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.
Related question - is there a way to determine the reflection's "sidedness" inside the closure? When using IOR as a parameter, internal vs. external reflection can be easily determined from the relative IOR - if it is greater or small than 1. But 1/eta and eta produce the same value for f0, so an extra bit (literally) of information is needed to disambiguate those two cases for the generalized_schlick_bsdf closure.
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.
Not overriding the closure parameter itself, but the BSDF internal object that gets created on the renderer side. We do some translation for non-ior parametrizations, but we haven't polished that a lot, it just works. Also, this doesn't affect internal BSDF layering, we don't track stuff there.
About sidedness, our convention: The shader always puts the IOR of the front surface regardless of what side is being rendered. The implementation is responsible for doing 1/eta if on the backside. We did this to remove the burden from the shaders, but other people might have other opinions.
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.
@portsmouth @aconty With the use of optional params for ior and priority, as well as the alternative of using transmission_depth/transmission_color to control the extinction coefficient, I felt it was justified to instead break this out to a separate VDF for homogenous media. Let me know what you think?
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.
Yeah, that makes sense.
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.
Hm, it seems that
anisotropic_vdf
is basically specifying a (homogeneous) volumetric medium, andmedium_vdf
is doing that plus specifying IOR and priority (of the "embedding" dielectric medium), except with a different way of parameterizing the extinction coefficient.. Wouldn't it be easier to just have onevdf
for the volume properties (maybe using the transmission parameterization, which is nicer than raw extinction), which you pass into the fullmedium_vdf
(I think I would have just called itmedium
..) to define the general case where the volume is embedded in dielectric?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.
To my mind
anisotropic_vdf
is a closure for general participating media (both homogenous and heterogeneous), whereasmedia_vdf
is a specialization for homogenous dielectric media. So a specialization for a common case, where it more efficiently can handle strictly homogenous media, as well as extra logic for priority etc. with nested dielectrics. It's similar to how we havesubsurface_bssrdf
as a separate closure specialized for that use case.I suppose we could change the parameterization of
anisotropic_vdf
to use the transmission parameterization instead of raw extinction here as well, if that makes sense for this more general case?However I'm not sure about removing these parameters from
media_vdf
to replace them with a closure input (for plugging in an anisotropic_vdf, if I understood your proposal correctly). To my mind this would makemedia_vdf
harder to implement and more confusing to use for OSL programmers as it allows plugging any closure into this input.