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.

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

Add hair_chiang_bsdf closure definition from MaterialX #1891

Merged

Conversation

fpsunflower
Copy link
Contributor

Description

This just adds a closure definition so the MaterialX project can work on their OSL backend bindings for this new BSDF.

Tests

We don't have an implementation of this closure (nor would be it be easy to provide since testrender doesn't support hair fibers and this model won't work well on triangulated hair. We'll have to rely on MaterialX and renderers implementing this closure for testing.

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format v17 before submitting, I definitely will look at
    the CI test that runs clang-format and fix anything that it highlights as
    being nonconforming.

@fpsunflower
Copy link
Contributor Author

@jstone-lucasfilm for vis.

@fpsunflower
Copy link
Contributor Author

Related MaterialX issue for posterity: AcademySoftwareFoundation/MaterialX#1968

float azimuthal_roughness_TRT,
float cuticle_angle,
color absorption_coefficient
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this have a BUILTIN tag here like the others?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed. Updated.

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM

@lgritz lgritz merged commit 416410d into AcademySoftwareFoundation:main Nov 13, 2024
21 of 23 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