Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Initial translation from open_pbr_surface to standard_surface #1949

Merged
merged 2 commits into from
Jul 30, 2024
Merged

Initial translation from open_pbr_surface to standard_surface #1949

merged 2 commits into from
Jul 30, 2024

Conversation

mkuo-lucasfilm
Copy link
Contributor

  1. Authored an initial pass at the translation from OpenPBR to standard surface
  • This would help to support OpenPBR in applications using earlier versions of MaterialX, which only have access to Standard Surface, but may want to participate in the OpenPBR ecosystem before they upgrade.

@mkuo-lucasfilm
Initial translation from 'open_pbr_surface' to 'standard_surface'
9666f82 
1. Authored an initial pass at the translation from OpenPBR to standard surface
- This would help to support OpenPBR in applications using earlier versions of MaterialX, which only have access to Standard Surface, but may want to participate in the OpenPBR ecosystem before they upgrade.
@kwokcb
Copy link
Contributor

kwokcb commented Jul 24, 2024 •
edited
Loading

Hi @mkuo-lucasfilm ,
I brought this up with @jstone-lucasfilm but wanted to ask about a couple of things.

  1. First is if "chaining" translators will work now that you have actual nodes.
    There are already std-surface to glTF and USDPreviewSurface. The question is, with this new translator can a user connect a OpenPBR -> std-surface translator and then connect that to a std-surface -> X translator ?

    I know it's all node name based so not sure if it's actually possible. It would be better to allow translations in 1 step. For instance there is a bake that occurs for each step I believe so a 2-step translation may not be equivalent to single step one.

  2. A second item is that as the version of OpenPBR / or std-surface changes since these translators are purely name based (without version information) how will this be handled ? e.g. even now how to you translate to the 2 versions of std surface?

Thanks.

@jstone-lucasfilm
Copy link
Member

Good questions, @kwokcb, and I can provide some initial thoughts:

  1. We haven't yet tested the chaining of shader translation graphs on our side, but my guess is that we'd encounter minor issues in the shader translation code that would need to be addressed, after which this pathway could be tested in production situations.
  2. We'll definitely need to resolve this in the future, and for now, the source and destination shaders are required to be at their default versions. The simplest solution would be a convention for incorporating version numbers in translation node names, but it seems worthwhile to see if we can come up with a more elegant solution.

@jstone-lucasfilm jstone-lucasfilm changed the title Initial translation from 'open_pbr_surface' to 'standard_surface' Initial translation from open_pbr_surface to standard_surface Jul 30, 2024
@jstone-lucasfilm
Minor comment update
ea2fe7f 
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
jstone-lucasfilm
jstone-lucasfilm approved these changes Jul 30, 2024
View reviewed changes
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.

This looks good to me, thanks @mkuo-lucasfilm!

@jstone-lucasfilm jstone-lucasfilm merged commit 55b52ec into AcademySoftwareFoundation:main Jul 30, 2024
34 checks passed
JGamache-autodesk added a commit to autodesk-forks/MaterialX that referenced this pull request Nov 5, 2024
@JGamache-autodesk
Coat layer should affect emission in USD translation (AcademySoftware…
4a3df18 
…Foundation#2087)

The standard_surface coat parameters are currently affecting the baseColor of UsdPreviewSurface. It should also affect emission color.

Initial translation from `open_pbr_surface` to `standard_surface` (AcademySoftwareFoundation#1949)

- Authored an initial pass at the translation from OpenPBR to standard surface.
- This would help to support OpenPBR in applications using earlier versions of MaterialX, which only have access to Standard Surface, but may want to participate in the OpenPBR ecosystem before they upgrade.

Update `standard_surface` to `open_pbr` translation graph   (AcademySoftwareFoundation#1956)

Replacing coat_affect_roughness with a constant value, since a coating always affects roughness in OpenPBR and is not dependent on the value of coat_darkening.

Improved `open_pbr_surface` to `standard_surface` translation (AcademySoftwareFoundation#1958)

Integrated the logic of coat_darkening from OpenPBR into the calculations of base_color and subsurface_color.

Initial translation from `standard_surface` to `open_pbr_surface` (AcademySoftwareFoundation#1934)

1. Authored an initial pass at the translation graph between standard surface and OpenPBR
2. Added units to standard surface thin film thickness documentation

Update documentation of OpenPBR and Standard Surface translations (AcademySoftwareFoundation#1963)

Clarified documentation where the motivation for the conversion might not be clear - both for the translation graph from OpenPBR to Standard Surface, and the translation graph from Standard Surface to OpenPBR.

Decrement translation graphs minor version (backport)

Consider subsurface color in dielectric albedo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jstone-lucasfilm jstone-lucasfilm jstone-lucasfilm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mkuo-lucasfilm @kwokcb @jstone-lucasfilm