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 ScalarOrVector<S> trait #1030

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BeastLe9enD
Copy link
Contributor

@BeastLe9enD BeastLe9enD commented Apr 7, 2023

This PR adds the ScalarOrVector<S> trait which represents types that can be either a scalar or a vector!

Motivation:
I am currently adding support for subgroup operations, and for example OpGroupNonUniformIAdds result type must be scalar or vector of integer type, so it would be useful to have this trait.

@BeastLe9enD BeastLe9enD changed the title Add vector-or-scalar traits Add ScalarOrVector<S> trait Apr 7, 2023
type Scalar;

/// The dimension of the vector, or 1 if it is a scalar
const DIM: usize;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use std::num::NonZeroUsize here?

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, theoretically you could do it, but I am not sure if it is currently supported in Rust-Gpu.

@oisyn
Copy link
Contributor

oisyn commented Apr 12, 2023

I like the general idea but if rust-gpu doesn't use it internally I'm not really seeing why it should add these things, rather than putting them in a separate crate for people to use?

@BeastLe9enD
Copy link
Contributor Author

@oisyn Yea the thing is, my plan is to add support for subgroup operations after this PR has been merged, and there are some ops like OpGroupNonUniformIAdds for which this trait can be really useful.

@oisyn
Copy link
Contributor

oisyn commented Apr 17, 2023

Right! I would suggest adding this trait as part of that work.

Comment on lines +58 to +78
impl_vector_or_scalar! {
for bool, 2: unsafe impl ScalarOrVector for glam::BVec2;
for bool, 3: unsafe impl ScalarOrVector for glam::BVec3;
for bool, 4: unsafe impl ScalarOrVector for glam::BVec4;

for f32, 2: unsafe impl ScalarOrVector for glam::Vec2;
for f32, 3: unsafe impl ScalarOrVector for glam::Vec3;
for f32, 4: unsafe impl ScalarOrVector for glam::Vec4;

for f64, 2: unsafe impl ScalarOrVector for glam::DVec2;
for f64, 3: unsafe impl ScalarOrVector for glam::DVec3;
for f64, 4: unsafe impl ScalarOrVector for glam::DVec4;

for u32, 2: unsafe impl ScalarOrVector for glam::UVec2;
for u32, 3: unsafe impl ScalarOrVector for glam::UVec3;
for u32, 4: unsafe impl ScalarOrVector for glam::UVec4;

for i32, 2: unsafe impl ScalarOrVector for glam::IVec2;
for i32, 3: unsafe impl ScalarOrVector for glam::IVec3;
for i32, 4: unsafe impl ScalarOrVector for glam::IVec4;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nicer to make this one per line, like i32: IVec2 => 2, IVec3 => 3, IVec4 => 4;
Also, you can make the macro implement both Vector and ScalarOrVector at the same time.

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.

None yet

4 participants