AcademySoftwareFoundation / OpenPBR

Specification and reference implementation for the OpenPBR Surface shading model
Apache License 2.0
425 stars 18 forks source link

Implement coat_ior_level and specular_ior_level in MaterialX #101

Closed portsmouth closed 11 months ago

portsmouth commented 11 months ago

This was a bit complicated to do in nodes..

I assumed the external/ambient IOR outside the coat is 1, since it seems there is no support for nested dielectrics in MaterialX yet.

jstone-lucasfilm commented 11 months ago

@portsmouth There's a long history of discussion of exterior IOR in MaterialX, and I'll add just two starting links here, which can be used to catch up with the latest thinking from the OSL and MaterialX teams:

https://academysoftwarefdn.slack.com/archives/C0230LWBE2X/p1638976312321500?thread_ts=1638906990.320000&channel=C0230LWBE2X&message_ts=1638976312.321500 https://academysoftwarefdn.slack.com/archives/C03LJBJ43SR/p1659091382475249?thread_ts=1658782550.911829&cid=C03LJBJ43SR

portsmouth commented 11 months ago

@portsmouth There's a long history of discussion of exterior IOR in MaterialX, and I'll add just two starting links here, which can be used to catch up with the latest thinking from the OSL and MaterialX teams:

OK thanks. I take it the current state is that there is no facility for querying the external IOR though, correct? (In which case, the proposed code is as close as we can get to the stated formulas in OpenPBR).

portsmouth commented 11 months ago

@jstone-lucasfilm any problem if we merge this?

(As an aside, I think it's pretty cumbersome having to work directly with XML like this, to implement what amounts to a trivial piece of code, that turns into a mess in XML. Having to manually implement something in XML nodes is quite awful and error prone. It would be much easier if the model could be defined e.g. as a script in a real programming language, e.g. OSL.

Or maybe it's possible to have some mathematical calculations done as code, and have the code referenced or embedded by the XML which is reserved for the high level structure. Alternatively, have some way to generate the XML from code).

jstone-lucasfilm commented 11 months ago

@portsmouth On the manual editing of text documents, I'll just note that there are some fantastic graph editors for MaterialX BSDF graphs in the industry, with Houdini/Solaris being one of my favorites.

As you might imagine, I'm a massive fan of Open Shading Language as well, and we've done some great collaborations with their team under the ASWF, but I'm not quite as convinced OSL would be the right domain in which to implement a language-independent definition of an open shading model, since cross-compilation from OSL to GLSL/MSL/MDL/Karma is not nearly as straightforward as shader generation from a MaterialX graph.

portsmouth commented 11 months ago

Just saying, implementing image

as the below, is quite awful:

    <divide name="specular_to_coat_ior_ratio" type="float">
      <input name="in1" type="float" interfacename="specular_ior" />
      <input name="in2" type="float" nodename="modulated_coat_ior" />
    </divide>
    <subtract name="specular_ior_minus_one" type="float">
      <input name="in1" type="float" nodename="specular_to_coat_ior_ratio" />
      <input name="in2" type="float" value="1.0" />
    </subtract>
    <add name="one_plus_specular_ior" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="specular_to_coat_ior_ratio" />
    </add>
    <divide name="specular_ior_to_F0_sqrt" type="float">
      <input name="in1" type="float" nodename="specular_ior_minus_one" />
      <input name="in2" type="float" nodename="one_plus_specular_ior" />
    </divide>
    <multiply name="specular_ior_to_F0" type="float">
      <input name="in1" type="float" nodename="specular_ior_to_F0_sqrt" />
      <input name="in2" type="float" nodename="specular_ior_to_F0_sqrt" />
    </multiply>
    <divide name="half_over_specular_F0" type="float">
      <input name="in1" type="float" value="0.5" />
      <input name="in2" type="float" nodename="specular_ior_to_F0" />
    </divide>
    <min name="specular_ior_level_upper_bound" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="half_over_specular_F0" />
    </min>
    <clamp name="specular_ior_level_clamped" type="float">
      <input name="in" type="float" interfacename="specular_ior_level" />
      <input name="low" type="float" value="0.0" />
      <input name="high" type="float" nodename="specular_ior_level_upper_bound" />
    </clamp>
    <multiply name="modulated_specular_reflectivity1" type="float">
      <input name="in1" type="float" nodename="specular_ior_level_clamped" />
      <input name="in2" type="float" nodename="specular_ior_to_F0" />
    </multiply>
    <multiply name="modulated_specular_reflectivity2" type="float">
      <input name="in1" type="float" value="2.0" />
      <input name="in2" type="float" nodename="modulated_specular_reflectivity1" />
    </multiply>
    <sqrt name="sqrt_modulated_specular_reflectivity" type="float">
      <in name="in" type="float" nodename="modulated_specular_reflectivity2" />
    </sqrt>
    <sign name="sign_specular_ior_minus_one" type="float">
      <in name="in" type="float" nodename="specular_ior_minus_one" />
    </sign>
    <multiply name="modulated_specular_reflectivity3" type="float">
      <input name="in1" type="float" nodename="sign_specular_ior_minus_one" />
      <input name="in2" type="float" nodename="sqrt_modulated_specular_reflectivity" />
    </multiply>
    <subtract name="one_minus_modulated_specular_reflectivity3" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="modulated_specular_reflectivity3" />
    </subtract>
    <add name="one_plus_modulated_specular_reflectivity3" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="modulated_specular_reflectivity3" />
    </add>
    <divide name="modulated_specular_ior_ratio" type="float">
      <input name="in1" type="float" nodename="one_plus_modulated_specular_reflectivity3" />
      <input name="in2" type="float" nodename="one_minus_modulated_specular_reflectivity3" />
    </divide>
    <multiply name="modulated_specular_ior" type="float">
      <input name="in1" type="float" nodename="modulated_specular_ior_ratio" />
      <input name="in2" type="float" nodename="modulated_coat_ior" />
    </multiply>

and it would still be awful even if you did it with visual nodes and noodles..

portsmouth commented 11 months ago

Discussion about the difficulty of implementing it aside, is it good to merge?

jstone-lucasfilm commented 11 months ago

@portsmouth For sure, there are plenty of cases where shading language code is more compact and clear than visual graphs, and we'd love to collaborate with teams that have ideas for further modularizing the BSDF functionality in MaterialX, allowing some of these common patterns to be implemented more succinctly in the graph context.

So far, this hasn't stopped developers from implementing some very complex BSDF concepts in MaterialX, including the open graphs for Standard Surface, glTF PBR, UsdPreviewSurface, and MaterialX Lama, but there's always room for improvement!

jstone-lucasfilm commented 11 months ago

I'll need to do a deeper dive into this specific PR to give a proper review of the changes, and I'll take a closer look when I have a chance!

portsmouth commented 11 months ago

I'll need to do a deeper dive into this specific PR to give a proper review of the changes, and I'll take a closer look when I have a chance!

OK, though bear in mind it's simply implementing the formulas from the spec which determine the IOR of the coat and specular dielectric, so nothing very complicated.

jstone-lucasfilm commented 11 months ago

This looks promising, @portsmouth, but there seems to be a logic bug that breaks the look of coated OpenPBR materials such as Carpaint.

Here's the look of an OpenPBR Carpaint material in the main branch:

OpenPBR_Carpaint_MainBranch

And here's the look of the same material with PR 101:

OpenPBR_Carpaint_PR101

Just to provide additional options for testing and validation, I've heard that Arnold 7.2.4 now supports custom MaterialX node definitions through the PXR_MTLX_STDLIB_SEARCH_PATHS environment, which should allow you to test this graph directly in Arnold:

https://help.autodesk.com/view/ARNOL/ENU/?guid=arnold_core_7240_html

portsmouth commented 11 months ago

Here's the look of an OpenPBR Carpaint material in the main branch:

Can you paste the parameters for this?

jstone-lucasfilm commented 11 months ago

Sure, here is the OpenPBR Carpaint material that I have been using for testing so far:

open_pbr_carpaint.zip

Once the shading model becomes public, I'd like to create a wider set of example materials that we can include with the repository, potentially adding render testing and validation for each commit.

For now, though, I'm just manually testing each pull request by rendering this simple material in MaterialXView and MaterialXGraphEditor.

portsmouth commented 11 months ago

there seems to be a logic bug that breaks the look of coated OpenPBR materials such as Carpaint.

Fixed the bug (I assume) in abc24e96fef2d65d32cdbf992aa51ef91c88c3a9, just needed to swap fg, bg of a mix node.

jstone-lucasfilm commented 11 months ago

Alas, this doesn't seem to fully address the logic issue, and I see the same visual change between the main branch and this PR:

OpenPBR Carpaint (Main Branch): OpenPBR_Carpaint_MainBranch

OpenPBR Carpaint (PR 101): OpenPBR_Carpaint_PR101

Would it be practical to validate this change in Arnold, so that we have both validation of the new graph and an example render to provide with the PR?

portsmouth commented 11 months ago

Works now I think:

image

portsmouth commented 11 months ago

By the way, if the coat IOR matches the specular IOR, then now the specular reflection disappears. Also, if the specular IOR is lower than the coat IOR, then the specular Fresnel exhibits TIR beyond a certain grazing angle. These are physically correct, but we might want to discuss what implications it has for the choice of the default IORs for example.

jstone-lucasfilm commented 11 months ago

@portsmouth This latest version is looking better, though I wanted to point out some significant visual differences between this implementation of OpenPBR and the previous one.

It could well be that these visual differences are intentional, but the OpenPBR Carpaint material seems to lose its anisotropic highlights and has hazier reflections at grazing angles in PR 101:

OpenPBR Carpaint (Main Branch): OpenPBR_Carpaint_MainBranch

OpenPBR Carpaint (PR 101): OpenPBR_Carpaint_PR101

Subjectively, the render from the main branch looks more like "carpaint" to me, but then it's quite possible that we expect artists to define their carpaint materials differently in OpenPBR than they would have done in shading models like Standard Surface.

Let me know what your thoughts are!

theblackunknown commented 11 months ago

(As an aside, I think it's pretty cumbersome having to work directly with XML like this, to implement what amounts to a trivial piece of code, that turns into a mess in XML. Having to manually implement something in XML nodes is quite awful and error prone. It would be much easier if the model could be defined e.g. as a script in a real programming language, e.g. OSL.

Or maybe it's possible to have some mathematical calculations done as code, and have the code referenced or embedded by the XML which is reserved for the high level structure. Alternatively, have some way to generate the XML from code).

As someone who wrote a manual MaterialX XML code, I can relate by I have not found a tool which fits my needs yet. I know we did try using a MaterialX Houdini graph in past with @virtualzavie but if I remember correctly it was really hard to bring back as a structured and readable .mtlx file. Maybe things are easier with MaterialXGraphEditor or LookDevX but I have not tested those yet.

theblackunknown commented 11 months ago

One nice thing to keep a MaterialX "module" - I denote the open_pbr_surface.mtlx here as a container of shading definitions - structured and readable is to introduce addition nodegraph to abstract some complex implementations. This can even have the benefit of being easily re-used elsewhere.

Here is a verbatim copy of what I did in our internal Adobe Standard Material MaterialX implementation:

  <nodedef name="ND_apply_specular_level_to_ior" node="apply_specular_level_to_ior" nodegroup="convert" doc="Scales reflectance linearly between IOR 1.0 and max_ior according to specular_level (works above and below 1.0 max).">
    <input name="eta_t_over_eta_i" type="float" />
    <input name="specular_level"   type="float" />
    <output name="out" type="float" />
  </nodedef>

  <nodegraph name="NG_f0_from_ior" nodedef="ND_f0_from_ior">
    <subtract name="ior_minus_one" type="float">
      <input name="in1" type="float" interfacename="eta_t_over_eta_i" />
      <input name="in2" type="float" value="1.0" />
    </subtract>
    <add name="ior_plus_one" type="float">
      <input name="in1" type="float" interfacename="eta_t_over_eta_i" />
      <input name="in2" type="float" value="1.0" />
    </add>
    <divide name="ratio" type="float">
      <input name="in1" type="float" nodename="ior_minus_one" />
      <input name="in2" type="float" nodename="ior_plus_one" />
    </divide>
    <multiply name="squaring" type="float">
      <input name="in1" type="float" nodename="ratio" />
      <input name="in2" type="float" nodename="ratio" />
    </multiply>
    <output name="out" type="float" nodename="squaring" />
  </nodegraph>

  <nodegraph name="NG_ior_from_f0" nodedef="ND_ior_from_f0">
    <sqrt name="sqrt_f0" type="float">
      <input name="in" type="float" interfacename="f0" />
    </sqrt>
    <add name="one_plus_sqrt_f0" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="sqrt_f0" />
    </add>
    <subtract name="one_minus_sqrt_f0" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="sqrt_f0" />
    </subtract>
    <divide name="ratio" type="float">
      <input name="in1" type="float" nodename="one_plus_sqrt_f0" />
      <input name="in2" type="float" nodename="one_minus_sqrt_f0" />
    </divide>
    <output name="out" type="float" nodename="ratio" />
  </nodegraph>

  <nodegraph name="NG_apply_specular_level_to_ior" nodedef="ND_apply_specular_level_to_ior">
    <f0_from_ior name="unscaled_f0" type="float">
      <input name="eta_t_over_eta_i" type="float" interfacename="eta_t_over_eta_i" />
    </f0_from_ior>
    <multiply name="_scaled_f0" type="float">
      <input name="in1" type="float" interfacename="specular_level" />
      <input name="in2" type="float" value="2" />
    </multiply>
    <multiply name="scaled_f0" type="float">
      <input name="in1" type="float" nodename="unscaled_f0" />
      <input name="in2" type="float" nodename="_scaled_f0" />
    </multiply>
    <min name="clamped_scaled_f0" type="float">
      <input name="in1" type="float" nodename="scaled_f0" />
      <input name="in2" type="float" value="0.9999" />
    </min>
    <ior_from_f0 name="external_ior" type="float">
      <input name="f0" type="float" nodename="clamped_scaled_f0" />
    </ior_from_f0>
    <output name="out" type="float" nodename="external_ior" />
  </nodegraph>
theblackunknown commented 11 months ago

Then you can simply use it like this

  <nodegraph name="NG_adobe_standard_material" nodedef="ND_adobe_standard_material">

    <!-- ... -->

    <apply_specular_level_to_ior name="specular_ior_after_specular_level" type="float">
      <input name="eta_t_over_eta_i" type="float" interfacename="specular_ior" />
      <input name="specular_level"   type="float" interfacename="specular_level" />
    </apply_specular_level_to_ior>

    <!-- ... -->

    <dielectric_bsdf name="specular_reflection_lobe" type="BSDF">
      <input name="tint"         type="color3"  interfacename="specular_edge_color" />
      <input name="ior"          type="float"   nodename="specular_ior_after_specular_level" />
      <input name="roughness"    type="vector2" nodename="rotated_anisotropic_alpha" />
      <input name="normal"       type="vector3" nodename="scaled_base_normal" />
      <input name="tangent"      type="vector3" nodename="rotated_tangent" />
      <input name="distribution" type="string"  value="ggx" />
      <input name="scatter_mode" type="string"  value="R" />
    </dielectric_bsdf>

    <!-- ... -->

  </nodegraph>
portsmouth commented 11 months ago

Thanks Andrea! Yeah we can definitely refactor later to simplify the XML code a bit using such constructs.

portsmouth commented 11 months ago

@portsmouth This latest version is looking better, though I wanted to point out some significant visual differences between this implementation of OpenPBR and the previous one.

Subjectively, the render from the main branch looks more like "carpaint" to me, but then it's quite possible that we expect artists to define their carpaint materials differently in OpenPBR than they would have done in shading models like Standard Surface.

Let me know what your thoughts are!

Yes indeed I think the parameters will need to be tweaked somewhat differently in OpenPBR, as it's more physically correct now. If the clearcoat on a car really had the same (or close) IOR as the base dielectric, then there would be no visible Fresnel reflection from the base.

So we can get roughly the same look, either by making the specular IOR higher (as here) or lower than the coat IOR:

image

theblackunknown commented 11 months ago

Just saying, implementing image

as the below, is quite awful:

    <divide name="specular_to_coat_ior_ratio" type="float">
      <input name="in1" type="float" interfacename="specular_ior" />
      <input name="in2" type="float" nodename="modulated_coat_ior" />
    </divide>
    <subtract name="specular_ior_minus_one" type="float">
      <input name="in1" type="float" nodename="specular_to_coat_ior_ratio" />
      <input name="in2" type="float" value="1.0" />
    </subtract>
    <add name="one_plus_specular_ior" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="specular_to_coat_ior_ratio" />
    </add>
    <divide name="specular_ior_to_F0_sqrt" type="float">
      <input name="in1" type="float" nodename="specular_ior_minus_one" />
      <input name="in2" type="float" nodename="one_plus_specular_ior" />
    </divide>
    <multiply name="specular_ior_to_F0" type="float">
      <input name="in1" type="float" nodename="specular_ior_to_F0_sqrt" />
      <input name="in2" type="float" nodename="specular_ior_to_F0_sqrt" />
    </multiply>
    <divide name="half_over_specular_F0" type="float">
      <input name="in1" type="float" value="0.5" />
      <input name="in2" type="float" nodename="specular_ior_to_F0" />
    </divide>
    <min name="specular_ior_level_upper_bound" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="half_over_specular_F0" />
    </min>
    <clamp name="specular_ior_level_clamped" type="float">
      <input name="in" type="float" interfacename="specular_ior_level" />
      <input name="low" type="float" value="0.0" />
      <input name="high" type="float" nodename="specular_ior_level_upper_bound" />
    </clamp>
    <multiply name="modulated_specular_reflectivity1" type="float">
      <input name="in1" type="float" nodename="specular_ior_level_clamped" />
      <input name="in2" type="float" nodename="specular_ior_to_F0" />
    </multiply>
    <multiply name="modulated_specular_reflectivity2" type="float">
      <input name="in1" type="float" value="2.0" />
      <input name="in2" type="float" nodename="modulated_specular_reflectivity1" />
    </multiply>
    <sqrt name="sqrt_modulated_specular_reflectivity" type="float">
      <in name="in" type="float" nodename="modulated_specular_reflectivity2" />
    </sqrt>
    <sign name="sign_specular_ior_minus_one" type="float">
      <in name="in" type="float" nodename="specular_ior_minus_one" />
    </sign>
    <multiply name="modulated_specular_reflectivity3" type="float">
      <input name="in1" type="float" nodename="sign_specular_ior_minus_one" />
      <input name="in2" type="float" nodename="sqrt_modulated_specular_reflectivity" />
    </multiply>
    <subtract name="one_minus_modulated_specular_reflectivity3" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="modulated_specular_reflectivity3" />
    </subtract>
    <add name="one_plus_modulated_specular_reflectivity3" type="float">
      <input name="in1" type="float" value="1.0" />
      <input name="in2" type="float" nodename="modulated_specular_reflectivity3" />
    </add>
    <divide name="modulated_specular_ior_ratio" type="float">
      <input name="in1" type="float" nodename="one_plus_modulated_specular_reflectivity3" />
      <input name="in2" type="float" nodename="one_minus_modulated_specular_reflectivity3" />
    </divide>
    <multiply name="modulated_specular_ior" type="float">
      <input name="in1" type="float" nodename="modulated_specular_ior_ratio" />
      <input name="in2" type="float" nodename="modulated_coat_ior" />
    </multiply>

and it would still be awful even if you did it with visual nodes and noodles..

I do not think we have the same formula to solve this in our Adobe Standard Material implementation, I would like to double check this with Peter. I do not see any flow in your math but jsut curious why we approached this differently.

Feel free to assess if this is best to put this topic on hold for this week release, or if we can merge your current implementation and follow up later.

theblackunknown commented 11 months ago

Happy to approve it now or tomorrow before our meeting if you would like to merge it rather sooner than later.

portsmouth commented 11 months ago

I do not see any flow in your math but jsut curious why we approached this differently.

I haven't checked in detail, but it looks quite similar to the math in your code. The only added things are the sign operations, and the clamping. Which seem to be necessary if you work out the inversion of (22).

image

theblackunknown commented 11 months ago

Sorry for the misclick ! I actually wanted to comment that I'll approve it based on your last comment on case you want to merge it before tomorrow meeting ! I can spend a bit of time tomorrow testing the new implementation too compared to the main branch if this is useful, but it seems that you have already covered quite a few things.

jstone-lucasfilm commented 11 months ago

This is great additional context, thanks @theblackunknown, and the modular approach that you recommend for building MaterialX graphs is a good one that we often use.

@portsmouth Your note about the need for a different IOR for the clearcoat makes sense, though it makes me wonder whether the default coat_ior ought to differ from the default specular_ior, so that artists get the behavior they expect when increasing the coat strength.

Although physical correctness is one important goal for OpenPBR, intuitive artist controls is certainly another!

portsmouth commented 11 months ago

@portsmouth Your note about the need for a different IOR for the clearcoat makes sense, though it makes me wonder whether the default coat_ior ought to differ from the default specular_ior, so that artists get the behavior they expect when increasing the coat strength. Although physical correctness is one important goal for OpenPBR, intuitive artist controls is certainly another!

It does differ in the OpenPBR spec -- which was not reflected in the MaterialX implementation, but I've fixed it up.

It may not differ enough though. I'd suggest we make the default coat IOR lower than the spec IOR, so e.g. 1.3 versus 1.6. Then the spec will show more prominently through the coat by default (while spec is dimmed by the coat as well, so there's a better balance).

image

theblackunknown commented 11 months ago

I'd suggest we make the default coat IOR lower than the spec IOR, so e.g. 1.3 versus 1.6. Then the spec will show more prominently through the coat by default (while spec is dimmed by the coat as well, so there's a better balance).

I do not have strong opinions on choosing good defaults as long as those two IORs are at different values.

One pitfall I want us to avoid is too choose defaults because we may have a bias towards making car paint looks good, is it the case here ? Defaults are just defaults in my opinion, artists will change parameter values to achieve the desired looks depending on the appearance they want to achieve.

jstone-lucasfilm commented 11 months ago

@theblackunknown Let's move forward with this change, since it aligns our reference implementation with the specification text, though I still harbor some concerns about the new coat parametrization being less intuitive from an artistic perspective (e.g. the visual impact of adjusting the coat slider). Let's see how artists feel when they create OpenPBR assets in production, and we can always evolve our parametrization over time.