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 swizzle variants of with_X methods #584

Merged
merged 7 commits into from
Nov 13, 2024

Conversation

tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Nov 10, 2024

Example

Vec3::Y.with_xz(Vec2::ONE) // == Vec3::ONE

Notes

  • Do we want set_X swizzles also?

  • One complication is it would be good to support Vec3 and Vec3A as the thing vector which will probably make the trait slightly more complex.

    Quote from the linked comment. Trivial to solve by taking impl Into<{{ vec3_t }}> though that would allow [f32, f32, f32], etc. as well.

@bitshifter
Copy link
Owner

Ah right, I guess I didn't see a need to port the swizzle test generation to tera when I migrated everything else. This is the old swizzlegen code before it was deleted https://github.com/bitshifter/glam-rs/blob/88e4538695cf1babddec1a9778510790cd1d7671/swizzlegen/src/main.rs

@bitshifter
Copy link
Owner

Rather than adding it to tera I've restore enough of the old code to generate swizzle tests.

@bitshifter
Copy link
Owner

https://github.com/bitshifter/glam-rs/tree/main/swizzlegen

@tim-blackbird
Copy link
Contributor Author

The tests use the values 11, 12 and 13 for the right hand side X, Y and Z components respectively.
This hopefully makes verifying correctness easy enough :)

// Examples
assert_eq!(v.with_xyz(rhs3), vec4(11_f32, 12_f32, 13_f32, 4_f32));

assert_eq!(v.with_zx(rhs2), vec4(12_f32, 2_f32, 11_f32, 4_f32));

@bitshifter
Copy link
Owner

I haven't reviewed every generated line but hopefully I've looked at enough to check things look correct AFAICT.

It took a little while to understand exactly what this does (having not used this in shaders) but it looks like it matches the https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.4.60.pdf swizzle assignments.

The testing approach makes sense to me.

For set, it can be added later if there is a desire for it.

I think into Into<{{ vec3_t }}> would be more permissive than I would like. It could also be done by splitting the swizzle trait up https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fbc9b6031e9224e27c286d256101d412, but I'll leave that for now, it complicates things and the benefits would be very niche.

Thanks for this, it's a pretty fiddly thing to implement.

@bitshifter bitshifter merged commit c83f129 into bitshifter:main Nov 13, 2024
18 checks passed
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