Added initial draft for OSL closures #614
Conversation
|
This is looking very good! |
@fpsunflower Yes absolutely. I probably should have made this PR directly to OSL in the first place.. But as soon as this initial design has been stabilized I will make a proper PR to the OSL repo (and I think we're getting close). Then we can finalize it there and anyone who wants to contribute can start adding implementations in testrender. I'll make sure to merge this PR first, saving all discussions here, and will link to this from the PR in OSL so all conversation history is preserved. |
…ectric_bsdf, generalized_schlick_bsdf and conductor_bsdf
| // \param priority Priority of this medium (for nested dielectrics). | ||
| // \param label Optional string parameter to name this component. For use in AOVs / LPEs. | ||
| // | ||
| closure color medium_vdf(color albedo, float transmission_depth, color transmission_color, float anisotropy, float ior, int priority) BUILTIN; |
There was a problem hiding this comment.
@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.
Hm, it seems that anisotropic_vdf is basically specifying a (homogeneous) volumetric medium, and medium_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 one vdf for the volume properties (maybe using the transmission parameterization, which is nicer than raw extinction), which you pass into the full medium_vdf (I think I would have just called it medium..) to define the general case where the volume is embedded in dielectric?
There was a problem hiding this comment.
To my mind anisotropic_vdf is a closure for general participating media (both homogenous and heterogeneous), whereas media_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 have subsurface_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 make media_vdf harder to implement and more confusing to use for OSL programmers as it allows plugging any closure into this input.
| // Do we just derive it from f0? For an artist it seems hard to control refractions that way. | ||
| // @Jonathan Stone you mentioned Lucasfilm (Naty Hoffman) had some feedback and ideas for how to handle this? | ||
| // | ||
| closure color generalized_schlick_bsdf(normal N, vector U, color reflection_tint, color transmission_tint, float roughness_x, float roughness_y, color f0, color f90, float exponent, string distribution) BUILTIN; |
There was a problem hiding this comment.
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.
@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.
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.
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.
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.
"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.
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.
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.
This was discussed in one of the threads marked as resolved, here: #614 (comment) |
|
A PR with these changes has now been made to OSL master, here: AcademySoftwareFoundation/OpenShadingLanguage#1371 Please continue the discussion there if there are outstanding questions. When that PR has been merged you are all very welcome to contribute to the reference implementation of the closures in OSL testrender. @jstone-lucasfilm I suggest we merge this PR in now, so all conversation history is saved here. |
This PR adds closure declarations for the MaterialX PBS nodes. A discussion of their design has been ongoing in a thread at the MaterialX GitHub repo. The full history of this discussion can be found here: AcademySoftwareFoundation/MaterialX#614 Signed-off-by: Niklas Harrysson <niklas.harrysson@gmail.com>
* Improvements to viewer property editor This changelist implements improvements to the viewer property editor, updating its display of shading models and ungrouped inputs to v1.38 conventions. * Added initial draft for OSL closures (AcademySoftwareFoundation#614) This changelist contains the initial draft and discussion for a set of new OSL closures to represent the MaterialX PBS library. Discussion and development on this project continues in the OSL repository at AcademySoftwareFoundation/OpenShadingLanguage#1371. * Remove legacy texture This changelist removes a legacy texture that was referenced only in unit tests, replacing it with the standard grid texture. * Fix color asTuple methods This changelist fixes the asTuple methods for the Color3 and Color4 classes in Python. These methods are an older mechanism for casting vector/color instances to Python tuples, and the current recommended approach is to directly cast vector/color instances to tuple type. * Remove invalid interfacename attribute This changelist removes an invalid interfacename attribute from the nodegraph_nodegraph example, allowing improvements to document validation logic in the future. * Add Element::getTreeIndex method This changelist adds the Element::getTreeIndex method, which returns the breadth-first index of an element within the document tree. * Improved support for compound nodegraphs. * Clarify opacity logic in Standard Surface This changelist clarifies the opacity logic in the Standard Surface nodegraph, replacing a <swizzle> node containing an unused "param" element with the equivalent channel-specific connection. * Improvements to shader translation and baking - Add support for graph outputs with multiple bindings in shader translation. - Remove extra dot nodes that were created in shader translation, as they're no longer needed in MaterialX v1.38. - Simplify the handling of world-space nodes in shader translation and texture baking. - Rename MaterialX::connectsToNodeOfCategory to MaterialX::connectsToWorldSpaceNode for clarity. * Improvements to viewer commands. * Patch. viewer. * Cleanup. * Missed patch. Co-authored-by: Jonathan Stone <jstone@lucasfilm.com> Co-authored-by: Niklas Harrysson <niklas.harrysson@gmail.com>
This changelist contains the initial draft and discussion for a set of new OSL closures to represent the MaterialX PBS library. Discussion and development on this project continues in the OSL repository at AcademySoftwareFoundation/OpenShadingLanguage#1371.
A first draft for adding OSL closures to represent the MaterialX PBS library in OSL.
This PR is created to be a forum for discussion. When the proposal is finalized a real PR should be made to https://github.com/AcademySoftwareFoundation/OpenShadingLanguage instead.