feat: inital matrix support#25
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds initial matrix support to the wgsl-rs library, enabling the use of 2x2, 3x3, and 4x4 matrices (Mat2, Mat3, Mat4) with f32 elements, matching WGSL's matrix type constraints.
Key Changes:
- Extended the
IsScalartrait with Matrix2, Matrix3, and Matrix4 associated types - Implemented matrix parsing for both generic (
Mat2<f32>) and aliased (Mat2f) syntax - Added WGSL code generation for matrix types outputting
mat{N}x{N}<f32>ormat{N}x{N}fformat
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/wgsl-rs/src/std.rs | Adds Matrix associated types to IsScalar trait, implements Mat2/Mat3/Mat4 generic types with f32 aliases (Mat2f/Mat3f/Mat4f) and column-wise constructors |
| crates/wgsl-rs-macros/src/parse.rs | Adds Matrix enum variant, implements split_as_mat helper function, extends type parsing to handle matrix types with validation for f32-only support, includes comprehensive test coverage |
| crates/wgsl-rs-macros/src/code_gen/formatter.rs | Implements WGSL code generation for Matrix types outputting proper mat{N}x{N} syntax |
| crates/example/src/main.rs | Demonstrates matrix usage with example constants (identity, rotation, and scaling matrices) and a matrix_vertex function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const ROTATION_2D: Mat3f = mat3x3f( | ||
| vec3f(1.0, 0.0, 0.0), | ||
| vec3f(0.0, 1.0, 0.0), | ||
| vec3f(0.0, 0.0, 1.0), | ||
| ); |
There was a problem hiding this comment.
The comment says "3x3 matrix constant" and the variable is named ROTATION_2D, but this is actually a 3x3 identity matrix, not a 2D rotation matrix. A 2D rotation matrix would typically have cosine and sine values. Consider either renaming this to IDENTITY_3X3 or changing the matrix values to represent an actual 2D rotation (e.g., using cos/sin values).
| let (_mat, n_suffix) = s.split_once("Mat")?; | ||
| (n_suffix.len() == 2).then_some(())?; | ||
| let split = n_suffix.split_at(1); |
There was a problem hiding this comment.
The variable is named n_suffix here but in the analogous split_as_vec function at line 237, it's called n_prefix. Both variables serve the same purpose (holding the characters after "Mat"/"Vec"), so they should be named consistently. Consider renaming this to n_prefix to match the vector parsing convention, or rename both to a more descriptive name like remainder.
| let (_mat, n_suffix) = s.split_once("Mat")?; | |
| (n_suffix.len() == 2).then_some(())?; | |
| let split = n_suffix.split_at(1); | |
| let (_mat, n_prefix) = s.split_once("Mat")?; | |
| (n_prefix.len() == 2).then_some(())?; | |
| let split = n_prefix.split_at(1); |
| ident: ident.clone(), | ||
| scalar: None, | ||
| } | ||
| } else if let Some((n, suffix)) = split_as_mat(other) { |
There was a problem hiding this comment.
The destructured variable is named suffix here, but in the analogous vector parsing code at line 294, it's called prefix. Since both represent the scalar type indicator ('f', 'i', 'u', 'b'), they should be named consistently. Consider renaming this to prefix to match the vector parsing pattern.
schell
force-pushed
the
feat/matrices
branch
from
ec20da1 to
c71c89c
Compare
* feat: inital matrix support * N-vectors and NxM-matrices * matrix multiplication * fmt * PR fixup * use nightly for cargo fmt action
No description provided.