math: refactor Vec and Mat mixin structs #1420

2025-05-09T08:57:59Z

spectre256 commented

2025-05-09 08:57:59 +00:00

(Migrated from github.com)

This change refactors the vector and matrix mixin structs to instead share a single generic struct. This allows for more generalized functions and less boilerplate. It also adds a new implementation of swizzle that is more flexible, more suitable for generalized vectors, and supports different axis names.

New mixin approach

This change moves all vector and matrix functions to their respective generic structs (e.g. Vec2.dot, Vec3.dot, Vec4.dot => VecN.dot). Functions that are specific to certain versions of the struct (e.g. Mat4x4.projection2D) have also been moved to the generic struct. This means that the public API for these functions does not change at all; users can still call Mat4x4.projection2D(.{ ... }) like normal.

Somewhat confusingly, this means that users can also refer to this function as Mat2x2.projection2D, but if they attempt to use the result as a Mat2x2 instead of a Mat4x4, there will of course be a compile error. I think this is a reasonable sacrifice compared to the benefits of this change.

Pros	Cons
No more `pub const x = Shared.x` boilerplate	Multiple ways to refer to the same declaration
Functions like `transpose`, `scale`, `translate`, etc. can be merged into one implementation	Minor API changes to functions like `init`
We get implementations of generic functions on other matrix types for free
Easier to maintain because there is only one implementation to make changes to

New swizzle implementation

The current implementation is somewhat awkward to use and lacks the flexibility that swizzling in shaders feature. For example, using the current implementation, the input vector type must be the same as the output, e.g. you can't write const v: Vec2 = myVec4.swizzle(.x, .y);. You are also locked into using xyz to as your axis names. This is inconvenient when working with vectors of rgba values or texture coordinates. Finally, due to the current implementation of mixin structs, swizzle can't be used on higher dimensional vectors.

My implementation solves all of these problems. Instead of an array of enum values, it takes a comptime []const u8 which represents a list of the vector components to use. It accounts for returning vectors of a different size than the input, it allows you to use "xyzw", "rgba", and "stpq" depending on the application. It also has the bonus of being slightly faster to type (myVec.swizzle("yzw") instead of myVec.swizzle(.y, .z, .w)).

P.S. I know it's probably bad to include this in the same PR, but making these changes separately would result in wasted work. If you would like me to move it into a new PR, I will do that.

Public API changes

Swizzle changes arguments from enum to string
VectorComponent enum removed
init functions for vector and matrix structs take slices instead of the exact number of arguments (the reexports in main remain the same, however)
Vec.fromInt removed from generic struct (the reexports in main remain the same, however)
Mat.Vec changed to Mat.MinVec
By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

This change refactors the vector and matrix mixin structs to instead share a single generic struct. This allows for more generalized functions and less boilerplate. It also adds a new implementation of swizzle that is more flexible, more suitable for generalized vectors, and supports different axis names. ## New mixin approach This change moves all vector and matrix functions to their respective generic structs (e.g. `Vec2.dot`, `Vec3.dot`, `Vec4.dot` => `VecN.dot`). Functions that are specific to certain versions of the struct (e.g. `Mat4x4.projection2D`) have also been moved to the generic struct. This means that the public API for these functions does not change at all; users can still call `Mat4x4.projection2D(.{ ... })` like normal. Somewhat confusingly, this means that users can also refer to this function as `Mat2x2.projection2D`, but if they attempt to use the result as a `Mat2x2` instead of a `Mat4x4`, there will of course be a compile error. I think this is a reasonable sacrifice compared to the benefits of this change. | Pros | Cons | |--------|---------| | No more `pub const x = Shared.x` boilerplate | Multiple ways to refer to the same declaration | | Functions like `transpose`, `scale`, `translate`, etc. can be merged into one implementation | Minor API changes to functions like `init` | | We get implementations of generic functions on other matrix types for free | | | Easier to maintain because there is only one implementation to make changes to | | ## New swizzle implementation The current implementation is somewhat awkward to use and lacks the flexibility that swizzling in shaders feature. For example, using the current implementation, the input vector type must be the same as the output, e.g. you can't write `const v: Vec2 = myVec4.swizzle(.x, .y);`. You are also locked into using xyz to as your axis names. This is inconvenient when working with vectors of rgba values or texture coordinates. Finally, due to the current implementation of mixin structs, swizzle can't be used on higher dimensional vectors. My implementation solves all of these problems. Instead of an array of enum values, it takes a `comptime []const u8` which represents a list of the vector components to use. It accounts for returning vectors of a different size than the input, it allows you to use "xyzw", "rgba", and "stpq" depending on the application. It also has the bonus of being slightly faster to type (`myVec.swizzle("yzw")` instead of `myVec.swizzle(.y, .z, .w)`). P.S. I know it's probably bad to include this in the same PR, but making these changes separately would result in wasted work. If you would like me to move it into a new PR, I will do that. ## Public API changes - Swizzle changes arguments from enum to string - VectorComponent enum removed - `init` functions for vector and matrix structs take slices instead of the exact number of arguments (the reexports in main remain the same, however) - `Vec.fromInt` removed from generic struct (the reexports in main remain the same, however) - Mat.Vec changed to Mat.MinVec - [x] By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

GalaxyShard (Migrated from github.com) reviewed

2025-05-09 08:57:59 +00:00

GalaxyShard (Migrated from github.com) reviewed

2025-05-17 17:24:38 +00:00

src/math/mat.zig

					
				@ -286,39 +36,33 @@ pub fn Mat4x4(

				        ///

Unless a Mat3x4 is constructed with MatNxN(4, 3), rather than MatNxN(3, 4), wouldn't rows be 3 and cols be 4 in this example?

This would also mean Mat3x4.ColVec == Vec3, not Vec4 like stated in a comment above, since there are 3 rows which corresponds to 3 elements per column (the logic/code is correct, the comment seems wrong).

Unless a Mat3x4 is constructed with MatNxN(4, 3), rather than MatNxN(3, 4), wouldn't `rows` be 3 and `cols` be 4 in this example? This would also mean Mat3x4.ColVec == Vec3, not Vec4 like stated in a comment above, since there are 3 rows which corresponds to 3 elements per column (the logic/code is correct, the comment seems wrong).

src/math/mat.zig

					
				@ -331,129 +75,230 @@ pub fn Mat4x4(

				        /// You would write it with the same visual layout:

        /// Constructs a 3D matrix which rotates around the Y axis by `angle_radians`.

Looks like there's been a typo here.

```suggestion /// Constructs a 3D matrix which rotates around the Y axis by `angle_radians`. ``` Looks like there's been a typo here.

spectre256 (Migrated from github.com) reviewed

2025-05-17 17:49:39 +00:00

src/math/mat.zig

					
				@ -286,39 +36,33 @@ pub fn Mat4x4(

				        ///

Good catch, thanks!

spectre256 (Migrated from github.com) reviewed

2025-05-17 17:50:00 +00:00

src/math/mat.zig

					
				@ -331,129 +75,230 @@ pub fn Mat4x4(

				        /// You would write it with the same visual layout:

Fixed 👍

Fixed :+1:

GalaxyShard (Migrated from github.com) reviewed

2025-05-17 19:39:14 +00:00

GalaxyShard (Migrated from github.com) left a comment

Fair disclaimer, these are just my opinions as someone who isn't an active contributor to this project, so take them with a grain of salt.

src/math/main.zig

					
				@ -49,71 +49,92 @@ const ray = @import("ray.zig");

				/// Public namespaces

nitpick: it would usually be called MxN in linear algebra since NxN may imply it's a square matrix.

src/math/mat.zig

					
				@ -6,269 +6,19 @@ const math = mach.math;

				const vec = @import("vec.zig");

In linear algebra, the convention is that m usually comes first and means rows. In this case it's flipped around; m is last and is columns. I might suggest naming them rows and cols (then remove the other definitions of rows/cols) to simplify things.

In linear algebra, the convention is that `m` usually comes first and means rows. In this case it's flipped around; `m` is last and is columns. I might suggest naming them `rows` and `cols` (then remove the other definitions of rows/cols) to simplify things.

src/math/mat.zig

					
				@ -286,39 +36,33 @@ pub fn Mat4x4(

				        ///

Does this type make sense for matrices without equal numbers of rows and columns?

If this definition of Vec is kept, maybe it should be renamed to MinVec to go alongside RowVec and ColVec, although this would create a breaking change since Vec would no longer exist

Does this type make sense for matrices without equal numbers of rows and columns? If this definition of `Vec` is kept, maybe it should be renamed to `MinVec` to go alongside `RowVec` and `ColVec`, although this would create a breaking change since `Vec` would no longer exist

This name isn't all that descriptive to me, although I'm not sure what else to call it. Maybe MinVecMinusOne?

This name isn't all that descriptive to me, although I'm not sure what else to call it. Maybe `MinVecMinusOne`?

spectre256 (Migrated from github.com) reviewed

2025-05-18 06:16:52 +00:00

src/math/mat.zig

					
				@ -286,39 +36,33 @@ pub fn Mat4x4(

				        ///

Sure, that works. I wasn't sure what to name it either

spectre256 (Migrated from github.com) reviewed

2025-05-18 06:18:06 +00:00

src/math/mat.zig

					
				@ -6,269 +6,19 @@ const math = mach.math;

				const vec = @import("vec.zig");

Fixed, thanks! I decided to change the names of the generic structs (VecN and MatMxN) to the same names as the reexports in main (Vec and Mat) for the sake of consistency.

spectre256 (Migrated from github.com) reviewed

2025-05-18 06:18:15 +00:00

src/math/mat.zig

					
				@ -286,39 +36,33 @@ pub fn Mat4x4(

				        ///

Since the Mat type is not necessarily square anymore, maybe it just doesn't make sense to have this type at all? Those using Vec should replace that usage with RowVec or ColVec as appropriate.

spectre256 (Migrated from github.com) reviewed

2025-05-18 06:19:35 +00:00

src/math/main.zig

					
				@ -49,71 +49,92 @@ const ray = @import("ray.zig");

				/// Public namespaces

Good point, thanks. I changed the names of the reexports in main accordingly. I decided to change VecN and MatMxN to Vec and Mat, since those are the names of their reexports in main.

spectre256 commented

2025-05-18 06:21:51 +00:00

(Migrated from github.com)

I noticed a minor inconsistency: Vec takes a parameter named length and the resulting struct has a member named n, but Mat takes parameters m and n but has members rows and cols. Maybe one of these should be switched to match the other (e.g. change Vec to take a parameter n and have a member length)?

I noticed a minor inconsistency: Vec takes a parameter named `length` and the resulting struct has a member named `n`, but Mat takes parameters `m` and `n` but has members `rows` and `cols`. Maybe one of these should be switched to match the other (e.g. change Vec to take a parameter `n` and have a member `length`)?

This pull request is broken due to missing fork information.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin spectre256/mixins:spectre256/mixins

git switch spectre256/mixins

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff spectre256/mixins

git switch spectre256/mixins

git rebase main

git switch main

git merge --ff-only spectre256/mixins

git switch spectre256/mixins

git rebase main

git switch main

git merge --no-ff spectre256/mixins

git switch main

git merge --squash spectre256/mixins