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

rpv: Fixes and improvements #240

Merged
merged 4 commits into from
Jun 13, 2022
Merged

rpv: Fixes and improvements #240

merged 4 commits into from
Jun 13, 2022

Conversation

leroyvn
Copy link
Member

@leroyvn leroyvn commented Jun 13, 2022

Description

This PR fixes and upgrades the RPV BSDF plugin. Changes are as follows:

  • sample() is fixed: The missing pdf division has been added. A corresponding unit test was added, as well as a plugin evaluation helper in a new test_tools.plugin module.
  • A few more fixes and improvements were made (constructor, string repr, tests).
  • The RPVBSDF test suite was overhauled (code deduplication).
  • A system test checking that degenerating the RPV BRDF into a Lambertian BRDF yields the expected result.

Checklist

  • The code follows the relevant coding guidelines
  • The code generates no new warnings
  • The code is appropriately documented
  • The code is tested to prove its function
  • The feature branch is rebased on the current state of the main branch
  • I updated the change log if relevant
  • I give permission that the Eradiate project may redistribute my contributions under the terms of its license

This commit fixes and upgrades the RPV BSDF plugin. Changes are as follows:

* sample() is fixed: A missing pdf division has been added. A 
  corresponding unit test was added, as well as a plugin evaluation 
  helper in a new `test_tools.plugin` module.

* A few more fixes and improvements were made (constructor, string repr,
  tests).
@leroyvn leroyvn added bug 🐛 Something isn't working enhancement 🦾 New feature or request labels Jun 13, 2022
@leroyvn leroyvn requested a review from wint3ria June 13, 2022 13:05
@leroyvn
Copy link
Member Author

leroyvn commented Jun 13, 2022

@wint3ria please tell me if this impacts your workflow. The eval() method is unchanged, so I'd be surprised if this would make a difference for you.

@leroyvn
Copy link
Member Author

leroyvn commented Jun 13, 2022

@nollety I added tests derived from #235 and took the opportunity to deduplicate a lot of the RPV test code. Please take a look, comments are welcome.

@leroyvn leroyvn requested a review from nollety June 13, 2022 15:08
@leroyvn leroyvn mentioned this pull request Jun 13, 2022
7 tasks
@leroyvn
Copy link
Member Author

leroyvn commented Jun 13, 2022

@nollety I'll merge, we'll adjust the test code later if you wish.

@leroyvn leroyvn merged commit 56014f9 into eradiate:main Jun 13, 2022
@leroyvn leroyvn deleted the fix_rpv branch June 13, 2022 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working enhancement 🦾 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant