glam-rs
glam-rs copied to clipboard
Add wasm-bindgen feature
See issue #292 and discussion #277
This adds a wasm-bindgen feature that decorates structs for wasm/JS bindings, and adds a wasm-only constructor.
There are a couple points worth discussing/considering before merging this:
- It's a feature, instead of
target_arch = "wasm32"because not everyone compiling for WASM needs to access the types from JS const fncannot be bound inwasm_bindgen, so I crated a separatenew(calledwasm_bindgen_ctor) function just for the wasm constructor- Additionally, we can't (probably shouldn't) just make the existing
newnon-const, as other const factory functions use it transitively - The function is only added when the
wasm-bindgenfeature is active, it won't pollute auto-complete et. al
- Additionally, we can't (probably shouldn't) just make the existing
- I chose not to bind all the methods on types, though many could be bound
- This decision was mostly out of laziness; I didn't want to bind them all if there isn't interest in this PR anyway
- This will however add a lot of extra
#[wasm_bindgen(skip)]decorators
- SSE2 is exclusive with
wasm_bindgen - I couldn't figure out why it was still mad when binding an affine, so I skipped it
I need this for my project, but I can use my own fork if this PR doesn't fit well with your direction. No worries.
The code looks simple enough but I'm not very familiar with bindgen so I have a few questions
It's a feature, instead of target_arch = "wasm32" because not everyone compiling for WASM needs to access the types from JS
Is there any benefit to enabling this for other architectures?
const fn cannot be bound in wasm_bindgen, so I crated a separate new (called wasm_bindgen_ctor) function just for the wasm constructor
Does this mean none of the const fn methods can be used? That seems like it would be quite limiting?
SSE2 is exclusive with wasm_bindgen
This doesn't appear to be consistent, I see that it's true for matrices but it looked like sse2 implementations of Quat, Vec3A and Vec4 still have bindgen enabled. Why did it need to be excluded on matrices?
I need this for my project, but I can use my own fork if this PR doesn't fit well with your direction. No worries.
It has been asked for before so it seems reasonable to consider it. I don't really have a good grasp on how it works though, how is this used in your project?
In addition to gaining some understanding of a new feature to accept a PR, I also need to be able to test it, so I'll know if I managed to break it through other changes. Is there a way this can be tested?
@bitshifter
Is there any benefit to enabling this for other architectures?
Almost certainly no. But while wasm_bindgen implies target_arch = "wasm32", the other way around isn't true. In fact my guess would be that the vast majority of people compiling for WASM (to run on the web, or in some other WASM runtime) won't want to access glam types from JS. wasm-bindgen is just that, it generated bindings to marshal JS calls to WASM Rust, and the other way around. If you don't need to access glam types from JS, them just compiling to WASM is all you need.
Does this mean none of the const fn methods can be used? That seems like it would be quite limiting?
Correct. But... shims can be enabled when the feature is active that expose those functions to JS, just as I did with new and wasm_bindgen_ctor. It's a pain, but remember what wasm-bindgen is going. It's generating a marshal layer between a compiled unmanaged language and a GCed JS runtime. const fn has no meaning to JS.
This doesn't appear to be consistent, I see that it's true for matrices but it looked like sse2 implementations of Quat, Vec3A and Vec4 still have bindgen enabled. Why did it need to be excluded on matrices?
That's entirely possible, I just fused off things until the compiler was happy.
...how is this used in your project?
My project allows you to paint N and P type silicon on an infinite substrate. The core of it is written in Rust and compiled to WASM. It uses WebGL (soon to be WebGPU) to do rendering. I'm building a website around it now, and need to generate binding in-depth to minipulate things. For example, my camera type leans heavily on glam for all the space transforms. For example, this line wouldn't compile without Vec2 itself being bindgen'ed already. And it's much easier to expose the glam types directly then to wrap everything in a newtype, where operators would have to be re-implemented.
Is there a way this can be tested?
I don't think testing makes any logical sense beyond 'does it compile' 🤷 Anything worth testing would be a bug in wasm_bindgen and thus beyond the responsibility of glam to test for.
If you're happy with the direction of this PR I'll finish off the SSE stuff, clean things up and fix the lint errors?
Also totally unrelated, thank you for Glam! You rock. I used a lot of nalgebra back in the Amethyst days, and when Bevy chose Glam I never looked back. Nalgebra might the the right pick for mathematicians, but the generics were pure hell. Glam is a joy to work with, it's my go-to on so many projects.
Almost certainly no. But while wasm_bindgen implies target_arch = "wasm32", the other way around isn't true. In fact my guess would be that the vast majority of people compiling for WASM (to run on the web, or in some other WASM runtime) won't want to access glam types from JS. wasm-bindgen is just that, it generated bindings to marshal JS calls to WASM Rust, and the other way around. If you don't need to access glam types from JS, them just compiling to WASM is all you need.
I think having it on a feature makes sense, but perhaps it would be better to constrain it to wasm32 as well for now if that is sufficient for your needs? E.g. #[cfg_attr(all(feature = "wasm-bindgen", wasm_bindgen, target_arch = "wasm32"))]
It's generating a marshal layer between a compiled unmanaged language and a GCed JS runtime. const fn has no meaning to JS.
OK, so you effectively need to expose the types to JS but don't actually use them from there? That makes sense then.
I don't think testing makes any logical sense beyond 'does it compile'
In that case perhaps compiling with the feature enabled in wasm32 would probably be sufficient to know it's not broken?
Perhaps adding a step to build with this enabled to https://github.com/bitshifter/glam-rs/blob/main/build_and_test_wasm32_firefox.sh or https://github.com/bitshifter/glam-rs/blob/main/build_and_test_wasm32_chrome.sh would suffice? Would there be any way to confirm that types were exposed to bindgen. For example if there was some refectoring of glam and the binggen stuff was inadvertently removed from a type that's the kind of thing that would be good to catch.
You project looks pretty interesting btw, I'll definitely try it out once the website is ready!
For the lint failure, in this case it's fine to add a #[allow(clippy::too_many_arguments)]
OK, so you effectively need to expose the types to JS but don't actually use them from there? That makes sense then.
Close. I do actually use the types from JS. The editor controls are implemented in TypeScript, because I want to allow for people making new tools as plugins. Even if those are written in a WASM language, until WASI ships you still need a JS bridge to get from WASM to WASM module.
In that case perhaps compiling with the feature enabled in wasm32 would probably be sufficient to know it's not broken?
Yep! Let me poke around your tests a bit today, and see what I can come up with. Wasm bindgen can generate TypeScript types, so an 'example usage' TS file could be checked with tsc to ensure the correct bindings / signatures are exposed. In theory anyway.
Chiming in here because I recently added support for wasmtime's WIT components in glamour.
I'm not sure I see the benefit to exposing host glam functions to a WASM guest. The overhead of calling back and forth makes it a not-great solution.
But what does make sense is to support passing values, so that glam can potentially be used by both the host and the guest with zero overhead (modulo potentially some alignment adjustment). This is trivial to support because glam already supports bytemuck.
(The WIT component support in glamour does not actually leverage this, but just hooks into wasmtime's Lower/Lift traits in a way that should result in a simple memcpy in/out of guest memory. See the tests in that repo for how that looks from a user's perspective.)
@simonask
I'm not sure I see the benefit to exposing host glam functions to a WASM guest. The overhead of calling back and forth makes it a not-great solution.
I was going to argue that it lent some ergonomics to the calling JS code (I don't need to find a JS lib to do common ops, even if the performance is terrible) but I don't actually have a single use case for doing so. Every glam fn I would want to call (apart from new) I've wrapped in other rust logic before exposing to JS. So perhaps binding methods is a solution in search of a problem 🤷♂️ Binding the structs themselves is super useful, as are the fields, as you noted.
Also: sorry for the lack of progress. I had my first kiddo, and free-time... well it doesn't really exist right now lol.
Ah congrats!