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

Internal rewrite #99

Closed
23 tasks done
bitshifter opened this issue Nov 18, 2020 · 1 comment · Fixed by #117
Closed
23 tasks done

Internal rewrite #99

bitshifter opened this issue Nov 18, 2020 · 1 comment · Fixed by #117
Assignees
Labels
enhancement New feature or request

Comments

@bitshifter
Copy link
Owner

bitshifter commented Nov 18, 2020

I'm opening this for visibility, particularly for #85 since internal changes will require rework for any one with active glam forks.

The primary design goals of glam have always been having a simple user facing API and trying to get the best performance for scalar math by using SIMD under the hood where possible. Part of keeping things simple was limiting glam to only supporting f32, since the majority of the time I've only needed f32. Inside glam I have also opted to keep things simple and accepted to a bit of code duplication to avoid complicating the implementation with generics, traits and macros.

It has been a pleasant surprise that other people have liked and adopted the crate. Other people, it turns out, sometimes want different things from me. For example f64 or i32 support. Or support for different architectures like WASM or Neon. Long term I think supporting these things is going to tip the scales away from code duplication in favour of a more complex implementation to reduce the development and maintenance burden on myself and contributors. I do not wish to burden users with additional complexity so the public API should remain the same.

This refactor should help with:

  • Support f64 and i32 #70
  • Vec SIMD types are not understood by rust-analyzer without a workaround #95
  • Adding support for other SIMD architectures (although this could also be achieved by using the packed_simd or wide crates.
  • Possible Vec2 optimisations - that is internally loading into SIMD, performing the operation and storing back to the Vec2 (see pathfinder_geometry)
  • Possibly exposing a low level API similar to DirectX math that can be used to write high level APIs that are different to glam

At a high level the idea is to add storage types and implement operations on those storage types. Similar to how DirectXMath has its XMVECTOR type and functions that perform Vector3, Vector4, Quaternion operations on the one type. It's not a friendly API for users but it makes sense internally. My idea is the make traits for these methods and implement them for the different storage types that I want to support, e.g. struct XYZ<T> { x: T, y: T, z: T} or __m128 could both implement a Vector3 trait. Using traits does make it slightly easier to support f32, f64, i32 and so on without having to add duplicate implementations. The high level types like Vec3, or DVec3 can be defined using a single macro that specifies the scalar type.

I have a work in progress branch here https://github.com/bitshifter/glam-rs/tree/refactor.

Things to do:

  • f64 types - DVec2, DVec3, DVec4, DQuat, DMat2, DMat3, DMat4
  • i32 types - IVec2, IVec3, IVec4
  • u32 types - UVec2, UVec3, UVec4
  • bool types - BVec2, BVec3, BVec4 - mask type can just be used when SIMD is available
  • Check performance
  • Check documentation makes sense
    • Document internal architecture
    • Improve module level module docs (maybe split types by module)
    • Document Deref usage
    • Document as cast methods
  • Check debugging experience (VS Code)
    • Using map functions obsfucates implementation a bit
    • Need to check matrix debugging.
  • Tests for const macros
  • Swizzles for all types
  • Casts between different types
  • Support optional features (serde, mint, rand) for the new types
  • Remove build.rs
  • Check with rust-gpu
  • Check with bevy
  • Check with macroquad (can't - it's using an older version of glam)
  • Check build times
    • Make num-traits optional to reduce build times
@bitshifter
Copy link
Owner Author

bitshifter commented Dec 2, 2020

For SPIRV I need to try out #[repr(transparent)] on the outer type and #[repr(simd)] on the inner type to make sure it works. If not then SPIRV will require #[repr(simd)] on the outer type which would necessitate a rethink of how I'm doing this.

bitshifter added a commit that referenced this issue Jan 12, 2021
Add support for `f64`, `i32`, `u32` primitive types.

Fixes #70.
Fixes #99.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant