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

Add Transform::{look_at_rh, look_at_lh} and deprecate look_at (continued) #514

Open elrnv opened 4 years ago

elrnv commented 4 years ago

This is a continuation of #508 with the following additions:

These changes make look_at and look_to functions consistent in meaning throughout the crate (with exception of deprecated functions).

kvark commented 3 years ago

Could somebody sum up the status of this PR? Is there a consensus, and what's left to do?

elrnv commented 3 years ago

IIRC this PR aims to get consistent naming for _to and _at variants of the look_ functions on top of #508.

It does this to a certain extent without committing to major changes to the Rotation and Transform APIs. In particular, this PR changes look_at to look_to in Rotation without the _lh or _rh suffixes for now in hopes of properly resolving the overlap between the Rotation and Transform APIs in another PR since I don't have the time at the moment to dwell on this. The remaining decision I think is whether to roll back the changes to the Rotation trait or keep it as is since it has better consistency for the time being, and it's not clear that the next PR will come any time soon.

The other point to resolve is whether Matrix2 needs _lh and _rh variants for the look_to function. In my opinion it does not, but I am more than happy to either revert that change or do something else there.

508 leaves the look_to and look_at function variants in an inconsistent state to (I think) avoid breaking the APIs of things like Rotation and Matrix2 temporarily before a more appropriate solution is found. Given the cadence of this crate, I think the changes in this PR are appropriate to get some consistency albeit not necessarily with respect to _lh and _rh variants of the look_ functions.

thatcomputerguy0101 commented 2 months ago

I believe the current implementation of look_at/to in Matrix2 is suited for 2d spritework, where in order for a sprite to maintain an "upwards" orientation while looking in the opposite direction, it must mirror itself. It is a related, but different operation than in the Matrix3 and Matrix4 implementations. Even with that in mind, I would still favor renaming it to look_to instead of look_at.