search

rustgd / cgmath

A linear algebra and mathematics library for computer graphics.
https://docs.rs/cgmath
Apache License 2.0
1.12k stars 155 forks source link

Consider documenting `impl Rotation::rotate_vector for Quaternion` as only valid for normalized quaternions #479

Closed tangmi closed 5 years ago

tangmi commented 5 years ago

It seems that the rotate_vector implementation for Quaternion doesn't care that self is normalized or not.

I believe that rotating a vector with a quaternion only makes sense for unit quaternions. Would it be a good idea to document Quaternion::rotate_vector and possibly put an debug_assert!(self.is_unit()) so users don't get strange behavior trying to use Rotation<Point3<S>> when self doesn't validly represent a rotation?

Cheers!

kvark commented 5 years ago

I think adding an assertion could trigger a few unintended failures on the user side, i.e when the quaternion is produced by some computation that is mathematically supposed get us a normalized quaternion but numerically ends up failing the assertion by some margin. It would be better to let the users just accumulate that minor error instead of hitting a sudden assert.

Documenting it clearly would be though!