Skip to content

Apply subsurface_bsdf weight in MDL and add tests #2264

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

Conversation

krohmerNV
Copy link
Contributor

This PR is mostly about the added tests materials.
Attached is a comparison between GLSL, OSL, and MDL.

  • MDL should be correct as far as I can tell. SSS is implemented using a random walk in the path tracer (which will be added to Add MDL Compiler dependency for [genmdl] tests #2254)
  • For the GLSL implementation the radius seems to not have a big impact which might be okay due to the real-time approximation. But we are wondering about the max(mfp, 0.1) here . This means at least 10cm radius if the scene is modelled in meters, right?
  • OSL matches GLSL but I haven't checked the source.

Screenshot_7-3-2025_153024_

Copy link

linux-foundation-easycla bot commented Mar 7, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

krohmerNV added a commit to krohmerNV/MaterialX that referenced this pull request Mar 11, 2025
krohmerNV added a commit to krohmerNV/MaterialX that referenced this pull request Apr 22, 2025
@jstone-lucasfilm jstone-lucasfilm changed the title MDL: apply subsurface_bsdf weight in MDL implementation and add tests Apply subsurface_bsdf weight in MDL and add tests Jun 15, 2025
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look like good changes to me, thanks @krohmerNV.

Our render support for subsurface_bsdf in GLSL and OSL testrender are very early, so I wouldn't yet expect a match with the more mature implementation in MDL, but these new tests should help to align future implementations across all languages.

@jstone-lucasfilm jstone-lucasfilm merged commit c4e35e9 into AcademySoftwareFoundation:main Jun 15, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants