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

Expose Luau 3-vectors support #41

Merged
merged 1 commit into from
Jan 15, 2024
Merged

Conversation

nurpax
Copy link
Contributor

@nurpax nurpax commented Jan 7, 2024

The Luau VM supports native f32 3-vectors so that typical linear algebra operations are fast in game code.

Supports both 3-vectors and 4-vectors.

@nurpax
Copy link
Contributor Author

nurpax commented Jan 10, 2024

This change is pending for #38 to go in. Once that's done, I'll clean this up and it should become pretty straightforward.

@nurpax nurpax force-pushed the luau-vector branch 2 times, most recently from a66d254 to c2f7954 Compare January 12, 2024 22:54
src/luau.cpp Outdated Show resolved Hide resolved
@nurpax
Copy link
Contributor Author

nurpax commented Jan 12, 2024

@natecraddock phew, this should be more or less ready too. It's worth taking a look as you might have some opinions about it.

It got a little tricky with the addition of built-time choice for vector length (.xyz or .xyzw). It's pretty cool that Zig conditional compilation can support a choice like this so well.

I abandoned the use of @Vector(3, f32) types for passing the float vectors around. It just gets a little messy, slices work better. Most likely will compile to sufficiently efficient code anyway.

build.zig Outdated Show resolved Hide resolved
src/libluau.zig Outdated Show resolved Hide resolved
src/luau.cpp Outdated Show resolved Hide resolved
src/tests.zig Show resolved Hide resolved
@natecraddock natecraddock added this to the 0.3.0 milestone Jan 13, 2024
@nurpax
Copy link
Contributor Author

nurpax commented Jan 13, 2024

PR updated based on your insightful code review, @natecraddock!

Copy link
Owner

@natecraddock natecraddock left a comment

Choose a reason for hiding this comment

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

Other than one small comment I left, everything looks great! Do you still consider this WIP?

src/libluau.zig Outdated
} else if (luau_vector_size == 4) {
return [_]f32{ r[0], r[1], r[2], r[3] };
} else {
@compileError("luau_vector_size must be 3 or 4");
Copy link
Owner

Choose a reason for hiding this comment

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

I think this compile error can be removed now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this to

            switch (luau_vector_size) {
                3 => return [_]f32{ r[0], r[1], r[2] },
                4 => return [_]f32{ r[0], r[1], r[2], r[3] },
                else => @compileError("invalid luau_vector size - should not happen"),
            }

IMO it reads better with the @compileError as that documents better that only 3 or 4 are legal here.

@nurpax
Copy link
Contributor Author

nurpax commented Jan 14, 2024

Other than one small comment I left, everything looks great! Do you still consider this WIP?

Good to go now!

@nurpax nurpax changed the title WIP: Expose Luau 3-vectors support Expose Luau 3-vectors support Jan 14, 2024
The Luau VM supports native f32 3- or 4-vectors so that typical
linear algebra operations are fast in game code.

Both the 3- and 4-vector flavors are supported.  Use
-Dluau_vector_size=N to choose which.  This must be configured
at build time, as the native Luau VM must be re-compiled for this
setting.
@nurpax
Copy link
Contributor Author

nurpax commented Jan 14, 2024

Added a tiny test case change to document use of vectors better. Still good to go.

@natecraddock natecraddock merged commit d63c450 into natecraddock:main Jan 15, 2024
3 checks passed
@nurpax nurpax deleted the luau-vector branch January 17, 2024 16:34
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