Fix handling of small magnitude quaternions #1022
Merged
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.
Hello again! I contributed to the project last year in #946. I have finally updated the version of glm in my project, and I noticed that there is still actually something afoul with small-angle quaternions.
The special case handling in
glm::qua::pow
is specifically intended for the case when the magnitude is null. Even for magnitudes smaller than epsilon, the common formula should be used. Because comparing a floating-point value by equality triggers a warning through the-Weveverything
setting of clang, it must be selectively disabled for the condition.I initially did so in a921b51 but, as you can see, the pipeline failed because of
-Wfloat-equal
, and I fixed it too quickly by using a comparison to epsilon like in other parts of the project.In passing,
glm::epsilon
is defined inext/scalar_constants.inl
asstd::numeric_limits<T>::epsilon
. According to cppreference.com, it “returns the difference between 1.0 and the next representable value of the given floating-point type”. But, for small values ofx
, there are many floating-point values betweenx
andx + epsilon
. Dawson's series on floating-point numbers seems to agree with me. Also, for values ofx
larger than 2.0, comparing to epsilon is the same as simple equality. So it is possible that other floating-point discrepancies have crept up somewhere else due to comparing to epsilon.Tell me if this fix is acceptable (for information, I am actually using the version with
if (VectorMagnitude == 0) {
if (< std::numeric_limits<T>::min()) {
in my project, and it works perfectly).