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 basic logic operations to Vec*Mask types. #9

Closed
cessen opened this issue Jul 22, 2019 · 6 comments
Closed

Add basic logic operations to Vec*Mask types. #9

cessen opened this issue Jul 22, 2019 · 6 comments

Comments

@cessen
Copy link
Contributor

cessen commented Jul 22, 2019

I would like to implement or, and, and not operators for the various Vec*Mask types.

My use-case:

In my path tracer I accumulate the results of many ray/box intersection tests via an or operation, as part of a ray streaming architecture. Rays are tested against four bounding boxes at once via SIMD, and the results from many rays are accumulated via the SSE _mm_or_ps() intrinsic.

In my specific case, I only need the or operation on Vec4Mask. However, or, and, and not seem like they would be generally useful for the mask types, given that they essentially represent boolean values.

Alternative

The same logical result can be achieved with glam already by first converting to a bitmask and using integer operations. But that adds an additional conversion operation in a tight loop, instead of doing the conversion once at the end of the accumulation. Having these operators available directly on the mask types allows writing more efficient code.

@bitshifter
Copy link
Owner

Sounds good! Ideally please provide a test for each new type method and a scalar fallback implementation that passes the test (cargo test --features scalar-math).

@cessen
Copy link
Contributor Author

cessen commented Jul 22, 2019

Will do! Thank you!

For writing tests I'll need a way to construct new arbitrary Vec4Masks. Since all of the tests are outside of the crate at the moment, that means it will need to be a public method. Should I add that too, or would you prefer me to keep the tests for this private inside their respective modules instead? IMO having a new(bool, bool, bool, bool) public method seems reasonable anyway, but I don't have a concrete use-case for it.

@bitshifter
Copy link
Owner

You could create arbitrary masks indirectly by doing comparisons, e.g. Vec4::new(1.0, 0.0, 1.0, 0.0).cmpgt(Vec4::zero()). That said, I can't think of a reason to not add a Vec4Mask::new so feel free to add one.

@cessen
Copy link
Contributor Author

cessen commented Jul 22, 2019

Okay, thanks! I added a Vec4Mask::new in my most recent PR, adding the tests for Vec*Mask.

@bitshifter
Copy link
Owner

@cessen I guess this can be closed now?

@cessen
Copy link
Contributor Author

cessen commented Jul 22, 2019

Ah, yes! Implemented in PR #12. Yay! \o/

@cessen cessen closed this as completed Jul 22, 2019
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

No branches or pull requests

2 participants