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

Vectorize BSDFContext #731

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Vectorize BSDFContext #731

wants to merge 1 commit into from

Conversation

Speierers
Copy link
Member

Description

This PR adds vectorization to the BSDFContext datastructure passed to the BSDF routines (e.g. eval(), sample(), pdf()). This allows the callee to specify the BSDF components and event types to be sampled/evaluated on a per-lane basis. This is very useful for implementing non-conventional integrators for the vectorized variants of Mitsuba (e.g. light path expression, ...).

This PR adapts the code of all BSDFs to use masking logic to handle the non-scalar conditions based on the BSDF context object.

Testing

On top of the current test suite which already tests the activation/de-activation of the BSDF components using the BSDFConext, I added a simple test in test_dielectric.py to test it in a vectorized way.

@Speierers Speierers force-pushed the vectorize_bsdfcontext branch 4 times, most recently from 6bd8773 to 2808967 Compare May 13, 2023 00:30
@Speierers Speierers requested a review from njroussel May 13, 2023 00:34
@wjakob
Copy link
Member

wjakob commented May 15, 2023

Just a quick note (also for Nicolas): I am a little skeptical about this change and the cost/complexity that it adds. Let's discuss further after the SIGGRAPH Asia deadline.

@Speierers
Copy link
Member Author

Pinging @wjakob and @njroussel for discussing this PR further.

@njroussel
Copy link
Member

njroussel commented Aug 18, 2023

Thanks for the reminder 😄

I don't have any strong opinions on this. I personally don't feel like this adds a whole lot of additional complexity to the renderer.

The only reason why I wouldn't want to merge this PR is because we don't have an actual supported use case for it (yet - maybe LPEs). However, there have been discussions/threads of people needing more "specialization" per ray and this would be a step towards that.

Can you tell us more about what your initial thoughts were on this PR @wjakob?

@Speierers
Copy link
Member Author

After discussing with @njroussel , it was decided not to merge this PR into master due to the invasive nature of the changes and limited clarity on the use case for the vectorized BSDFContext datastructure. We'll keep the PR open for reference and potential future discussions on merging it into master if the need arises.

@wjakob wjakob force-pushed the master branch 3 times, most recently from 3f3b8d0 to 1bdea6e Compare October 12, 2023 19:49
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.

3 participants