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 some furnace tests for some of the MaterialX closures #1542

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

fpsunflower
Copy link
Contributor

This is to quantify the energy loss/gain of the individual closures. I've also added a way for testrender to show the results of the get_albedo() method directly, for comparison with furnace tests.

I've also added a rational fit for the albedo of sheen, which is taken from the MaterialX project. It doesn't look like an exact match, but it should be good enough for now.

@fpsunflower fpsunflower changed the title Add some furnace tests for the MaterialX closures Add some furnace tests for some of the MaterialX closures Jul 27, 2022
@lgritz
Copy link
Collaborator

lgritz commented Aug 1, 2022

This is failing across the board, but only the render-mx-layer test. Which is odd because that test was not modified directly by this PR. And it's not just a speckle or occasional LSB errors. The whole thing has a visible intensity shift.

Do you think that's because of the change to get_albedo?

I'm about to test whether it's sufficient to just update that one ref file without even having to adjust the comparison thresholds (i.e., if it's just different now, but not experiencing significant per-platform variation).

@fpsunflower
Copy link
Contributor Author

The mx-layer test was added in a previous PR, but was not actually being run. I added it to the list of tests to run but didn't re-generate the image yet.

@lgritz
Copy link
Collaborator

lgritz commented Aug 1, 2022

It looks like you just need to update that one ref image and all will be fine.

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, pending updating the render-mx-layer reference image.

@fpsunflower fpsunflower force-pushed the mx-furnace branch 4 times, most recently from 255a827 to 2a9a011 Compare August 1, 2022 18:46
@lgritz
Copy link
Collaborator

lgritz commented Aug 1, 2022

The one remaining that fails is the Debug build, and at least some of the test were failing simply by timing out -- some of the render tests take a godawful long time when not optimized. Like that touch-up patch I added earlier today, we may just want to bump up the ctest timeout value for the individual tests higher in debug mode.

@lgritz
Copy link
Collaborator

lgritz commented Aug 1, 2022

I'm tempted to merge this and just bump the timeouts for those failing tests individually, as a separate PR. Shall I do that, or do you really want to do that yourself, @fpsunflower ?

@fpsunflower
Copy link
Contributor Author

Yeah I'm fine with ignoring the timeouts, but there is one legit failure in debug mode -- an assert is being triggered. I was just able to repro locally, so let me try to sort that out first ...

Implement get_albedo for the sheen closure, based on fit derived by the MaterialX team

Add optional mode to testrender to visualize the result of the get_albedo method. This can be useful for validation of the get_albedo method.

Signed-off-by: Chris Kulla <[email protected]>
@fpsunflower fpsunflower merged commit 23ea064 into AcademySoftwareFoundation:main Aug 1, 2022
lgritz pushed a commit to lgritz/OpenShadingLanguage that referenced this pull request Aug 2, 2022
)

Implement get_albedo for the sheen closure, based on fit derived by the MaterialX team

Add optional mode to testrender to visualize the result of the get_albedo method. This can be useful for validation of the get_albedo method.

Signed-off-by: Chris Kulla <[email protected]>
lgritz pushed a commit to lgritz/OpenShadingLanguage that referenced this pull request Aug 2, 2022
)

Implement get_albedo for the sheen closure, based on fit derived by the MaterialX team

Add optional mode to testrender to visualize the result of the get_albedo method. This can be useful for validation of the get_albedo method.

Signed-off-by: Chris Kulla <[email protected]>
chellmuth pushed a commit to chellmuth/OpenShadingLanguage that referenced this pull request Sep 6, 2024
Relevant changes:

  * GPU string fix -- needed missing cast (AcademySoftwareFoundation#1553)
  * Fix typo error that prevented correct typecheck of ternary (AcademySoftwareFoundation#1552)
  * testrender now supports the standard MaterialX closure:
  * testrender improvements: modernize sampler (AcademySoftwareFoundation#1534), switch to cone
    tracing (AcademySoftwareFoundation#1543), support MaterialX closures (AcademySoftwareFoundation#1533, AcademySoftwareFoundation#1536, AcademySoftwareFoundation#1537,
    AcademySoftwareFoundation#1538, AcademySoftwareFoundation#1541, AcademySoftwareFoundation#1542, AcademySoftwareFoundation#1547)
  * Bump LLVM to 14.0
  * Help reduce PTX nondeterminism (AcademySoftwareFoundation#1570)

See merge request spi/dev/3rd-party/osl-feedstock!30
@fpsunflower fpsunflower deleted the mx-furnace branch October 28, 2024 03:43
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