dimforge / dimforge/nalgebra

Add RotationQuaternion wrapper around Unit<Quaternion>

Open
#591 1 comment 1 reaction 0 assignees View on GitHub
Dominant language
Rust
Stars
4.8k
Forks
565
PR merge metrics
No merged PRs in 30d

Description

From https://github.com/rustsim/nalgebra/pull/584#issuecomment-485100760:

> (...) I'm starting to feel like my mistake in the first place was to not create a wrapper type like `struct RotationQuaternion(Unit>)` that would indeed assume that its internal `q` is the same as its `-q`.
>
> There are at least two other benefits of doing that:
>
> 1. It will be more obvious for newcomers to understand that `RotationQuaternion` is a rotation represented as a quaternion. (We could also rename the current `Rotation` type to `RotationMatrix` to be explicit.)
>
> 2. It will prevent a tricky mistake. Currently if you write `-q * v` where `q` is a unit-quaternion and `v` a vector, it will compile and be equal to `q * v` which is misleading considering one could expect the result to be `-(q * v)`. With a wrapper type that does not implement `Neg`, the `-q * v` expression will simply not compile.
>
>
> One downside is that this will be a breaking change.
>

The list of things to do:

> 1. Replace the `type UnitQuaternion = Unit>` by `struct RotationQuaternion(pub Unit>)`.
>
> 2. Replace `UnitQuaternion` by `RotationQuaternion` everywhere.
>
> 3. On all methods of the new `RotationQuaternion` the `self.` will have to be replaced by `self.0` to account for the struct wrapping.
>
> 4. Add a custom implementation of `Hash` that returns the same hash for both `self.0` and `-self.0`.
>
> 5. Add a `impl From> for RotationQuaternion`.

This should also fix #583.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.