Make Transform::rotate_axis
and Transform::rotate_local_axis
use Dir3
#12986
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Related to #12981
Presently, there is a footgun where we allow non-normalized vectors to be passed in the
axis
parameters ofTransform::rotate_axis
andTransform::rotate_local_axis
. These methods invokeQuat::from_axis_angle
which expects the vector to be normalized. This PR aims to address this.Solution
Require
Dir3
-valuedaxis
parameters for these functions so that the vector's normalization can be enforced at type-level.Migration Guide
All calls to
Transform::rotate_axis
andTransform::rotate_local_axis
will need to be updated to use aDir3
for theaxis
parameter rather than aVec3
. For a general input, this means callingDir3::new
and handling theResult
, but if the previous vector is already known to be normalized,Dir3::new_unchecked
can be called instead. Note that literals likeVec3::X
also have correspondingDir3
literals; e.g.Dir3::X
,Dir3::NEG_Y
and so on.Discussion
This axis input is unambigiously a direction instead of a vector, and that should probably be reflected and enforced by the function signature. In previous cases where we used, e.g.,
impl TryInto<Dir3>
, the associated methods already implemented (and required!) additional fall-back logic, since the input is conceptually more complicated than merely specifying an axis. In this case, I think it's fairly cut-and-dry, and I'm pretty sure these methods just predate our direction types.