Skip to content

Apply thin-film inputs to Standard Surface transmission - #3086

Open
agetroortega wants to merge 5 commits into
AcademySoftwareFoundation:mainfrom
agetroortega:issue-3025-thin-film-bindings
Open

agetroortega wants to merge 5 commits into
AcademySoftwareFoundation:mainfrom
agetroortega:issue-3025-thin-film-bindings

Conversation

@agetroortega

Copy link
Copy Markdown

Brief

Fixes #3025

  • Added thin-film inputs to transmission_bsdf node
  • Added a regression test for them
  • Updated relevant documentation

Testing

  • Clean build on MacOS
  • Green test suite

Manual testing

2 renders of a sphere with the following test material, the only difference is thin_film_thickness with a value of 0 and 550. Also confirmed that this input had no effect before this PR.

Result

thin_film_thickness=0 thin_film_thickness=550
no-film with-film
No thin-film Iridescence visible on sphere

Material

<?xml version="1.0"?>
<materialx version="1.39" colorspace="lin_rec709">
  <standard_surface name="SR_glass" type="surfaceshader">
    <input name="base" type="float" value="0" />
    <input name="specular" type="float" value="0" />
    <input name="specular_color" type="color3" value="1, 1, 1" />
    <input name="specular_roughness" type="float" value="0" />
    <input name="specular_IOR" type="float" value="3.5" />
    <input name="transmission" type="float" value="1" />
    <input name="transmission_color" type="color3" value="1, 1, 1" />
    <input name="transmission_depth" type="float" value="0" />
    <input name="transmission_scatter" type="color3" value="0, 0, 0" />
    <input name="transmission_scatter_anisotropy" type="float" value="0" />
    <input name="transmission_dispersion" type="float" value="0" />
    <input name="transmission_extra_roughness" type="float" value="0" />
    <input name="opacity" type="color3" value="1, 1, 1" colorspace="lin_rec709" />
    <input name="thin_film_thickness" type="float" value="0" />
    <input name="thin_film_IOR" type="float" value="1.5" />
  </standard_surface>
  <surfacematerial name="Glass" type="material">
    <input name="surfaceshader" type="surfaceshader" nodename="SR_glass" />
  </surfacematerial>
</materialx>

Render command

build/bin/MaterialXView \
--material "transmission-thin-film.mtlx" \
--mesh "resources/Geometry/sphere.obj" \
--envRad "resources/Lights/san_giuseppe_bridge.hdr" \
--drawEnvironment true \
--envSampleCount 64 \
--captureFilename "thin-film.png"

Agent Assistance

Agents used for codebase inspection and debugging.
Code written and tested manually, like in the olden days.

Model: gpt-5.6-sol
Harnesses: Codex and Cursor

@jstone-lucasfilm

Copy link
Copy Markdown
Member

Thanks for taking this on, @agetroortega! This correctly implements the fix proposed in #3025, and the two new connections on transmission_bsdf mirror the existing wiring of the specular and metal lobes, so Standard Surface now presents a single thin-film interface whose reflection and transmission are complementary.

I see one issue that's important to address, along with a suggestion on the unit test:

  1. On the MDL side, the changes in this PR won't currently have any effect. In mx_dielectric_bsdf, the bsdf_R and bsdf_RT variants wrap their microfacet lobes in df::thin_film, but the transmit-only bsdf_T variant does not, so a Standard Surface glass with thin film would be iridescent in our hardware shading languages and unchanged in MDL. This is a pre-existing issue in the MDL node rather than anything introduced here, but this PR is what will make it visible to users, so it would be ideal to resolve it here as well. I'd suggest wrapping the bsdf_T layer in df::thin_film in the same manner as bsdf_RT, along the lines of the following:

    bsdf bsdf_T = df::weighted_layer(
        weight: mxp_weight,
        layer: df::thin_film(
            thickness: mxp_thinfilm_thickness,
            ior: color(coatIor),
            base: df::microfacet_ggx_smith_bsdf(
                roughness_u: mxp_roughness.x,
                roughness_v: mxp_roughness.y,
                tint: mxp_tint * mxp_top_weight,
                multiscatter_tint: mxp_tint * mxp_top_weight,
                tangent_u: mxp_tangent,
                mode: df::scatter_transmit,
                backscatter: df::backscatter_none)),
        normal: mxp_normal);
    

    The function is defined independently in pbrlib_1_6.mdl, pbrlib_1_9.mdl, and pbrlib_1_11.mdl, with the other versioned modules re-exporting from those three, so the same change would be needed in each. The 1.6 and 1.9 versions predate the backscatter parameter, so that line would be omitted there.

  2. On the unit test, the new case in Node.cpp checks for specific node names and interface connections within the data library, so it would fail on any future restructuring of the Standard Surface graph, as we did in Optimizations to Standard Surface graph #2483, without telling us anything about rendered behavior. I'd suggest replacing it with a material in our render test suite. None of our current test materials exercise dielectric_bsdf thin-film in transmission mode, and a small document under resources/Materials/TestSuite/pbrlib/surfaceshader would provide coverage of this path across all of our shading languages, including the MDL fix above. Something like standard_surface_thin_film_transmission.mtlx with base and specular at 0.0, transmission at 1.0, specular_roughness at 0.0, and a thin_film_thickness of 550 would work well, and it could follow the pattern of the neighboring open_pbr_thin_walled_subsurface.mtlx.

One smaller note: the comment above the specular reflection lobe in the functional definition reads "with optional thin-film", and it would be worth giving the transmission lobe's comment the same treatment now that both lobes carry it.

Overall this looks like a great first contribution, and with the refinements above, I believe it should be ready to merge.

- Added thin-film inputs to transmission_bsdf node
- Added regression test for it
- Updated documentation
- Wrapped the bsdf_T variant of mx_dielectric_bsdf definition with df:thin_film, similar to bsdf_R and bsdf_RT variants.
- Small comment update in the PBR specification.
@agetroortega
agetroortega force-pushed the issue-3025-thin-film-bindings branch from c82eca4 to 6084a2e Compare October 16, 2026 16:00
@agetroortega

Copy link
Copy Markdown
Author

@jstone-lucasfilm thank you for the review! Apologies for the delay as well, I was only able to jump back on this PR now.

I added 2 new commits addressing your comments.
Let me know if there are any changes and if you prefer changes squashed into a single commit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Thin-film inputs are not applied to specular transmission in Standard Surface

2 participants