search

gltf-rs / mikktspace

Rust MikkTSpace implementation
Apache License 2.0
32 stars 20 forks source link

forking and publishing the repo using glam instead of nalgebra #20

Closed jakobhellermann closed 3 years ago

jakobhellermann commented 3 years ago

I am currently trying to generate tangents on the fly for the bevy engine, and to avoid pulling in the nalgebra crate, I'd like to publish a fork of this crate using glam (and of course clearly credit you in the readme). Is this something you'd be okay with?

Thank you very much for creating this library.

jakobhellermann commented 3 years ago

Alternatively I can create a PR which lets you select the vector library used via a feature.

alteous commented 3 years ago

Hi, I'm glad you are finding the library useful. The feature idea seems reasonable to me, as long as the conditional compilation can be kept to a minimum.

jakobhellermann commented 3 years ago

@alteous I've made a PR which uses conditional compilation in #21. The difference is mostly that nalgebra uses references instead of by-value.

So there are three possibilities:

  1. conditional compilation for using glam or nalgebra
  2. switch to glam
  3. fork and switch to glam

Since the difference is only internal and doesn't affect the public API, I think 2. is the best option because it has the least maintaining overhead and glam compiles way faster than nalgebra.

If you agree I'd be happy to make a PR switching to glam which will make the library compile faster for projects not using nalgebra, otherwise let me know if you'd want to merge #21.

alteous commented 3 years ago

Option 2 might help your particular situation but will likely introduce the same dependency problem for other people. It's similar to an issue that was raised in gltf (https://github.com/gltf-rs/gltf/issues/197) a while ago, which now uses nalgebra internally. Since the majority of users of gltf will use this crate as well, I'd rather not explore this option.

If you want to fork the repo that's fine by me from an authoring persective; however, it may not be wise to fragment the crates.io repository with margingly different crates. It will also require you to pull changes from the main repository if you want updates.

Therefore, from my perspective, option 1 is the least worst option. Your PR looks quite reasonable so as long as nalgebra remains the default then I'm happy to merge it.

jakobhellermann commented 3 years ago

Done in #21, thanks for merging :slightly_smiling_face: