Commit 4f2d5669 authored by bsheedy's avatar bsheedy Committed by Commit Bot

Fix AngleBetweenVectorsInDegrees NAN

Fixes potential NANs from gfx::AngleBetweenVectorsInDegrees and
gfx::ClockwiseAngleBetweenVectorsInDegrees when the given vectors are
very close together by clamping the value passed to acos. When
unclamped, the value passed to acos could be slightly outside [-1, 1]
due to floating point precision.

Bug: 918734
Change-Id: I55b75b21581d25134fdc3e8e6db80892607a807c
Reviewed-on: https://chromium-review.googlesource.com/c/1394086Reviewed-by: default avatarIan Vollick <vollick@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619780}
parent cc02a0cd
...@@ -86,8 +86,11 @@ Vector3dF ScaleVector3d(const Vector3dF& v, ...@@ -86,8 +86,11 @@ Vector3dF ScaleVector3d(const Vector3dF& v,
float AngleBetweenVectorsInDegrees(const gfx::Vector3dF& base, float AngleBetweenVectorsInDegrees(const gfx::Vector3dF& base,
const gfx::Vector3dF& other) { const gfx::Vector3dF& other) {
return gfx::RadToDeg( // Clamp the resulting value to prevent potential NANs from floating point
std::acos(gfx::DotProduct(base, other) / base.Length() / other.Length())); // precision issues.
return gfx::RadToDeg(std::acos(fmax(
fmin(gfx::DotProduct(base, other) / base.Length() / other.Length(), 1.f),
-1.f)));
} }
float ClockwiseAngleBetweenVectorsInDegrees(const gfx::Vector3dF& base, float ClockwiseAngleBetweenVectorsInDegrees(const gfx::Vector3dF& base,
......
...@@ -270,14 +270,16 @@ TEST(Vector3dTest, AngleBetweenVectorsInDegress) { ...@@ -270,14 +270,16 @@ TEST(Vector3dTest, AngleBetweenVectorsInDegress) {
float expected; float expected;
gfx::Vector3dF input1; gfx::Vector3dF input1;
gfx::Vector3dF input2; gfx::Vector3dF input2;
} tests[] = { } tests[] = {{0, gfx::Vector3dF(0, 1, 0), gfx::Vector3dF(0, 1, 0)},
{0, gfx::Vector3dF(0, 1, 0), gfx::Vector3dF(0, 1, 0)}, {90, gfx::Vector3dF(0, 1, 0), gfx::Vector3dF(0, 0, 1)},
{90, gfx::Vector3dF(0, 1, 0), gfx::Vector3dF(0, 0, 1)}, {45, gfx::Vector3dF(0, 1, 0),
{45, gfx::Vector3dF(0, 0.70710678188f, 0.70710678188f)},
gfx::Vector3dF(0, 1, 0), {180, gfx::Vector3dF(0, 1, 0), gfx::Vector3dF(0, -1, 0)},
gfx::Vector3dF(0, 0.70710678188f, 0.70710678188f)}, // Two vectors that are sufficiently close enough together to
{180, gfx::Vector3dF(0, 1, 0), gfx::Vector3dF(0, -1, 0)}, // trigger an issue that produces NANs if the value passed to
}; // acos is not clamped due to floating point precision.
{0, gfx::Vector3dF(0, -0.990842f, -0.003177f),
gfx::Vector3dF(0, -0.999995f, -0.003124f)}};
for (size_t i = 0; i < base::size(tests); ++i) { for (size_t i = 0; i < base::size(tests); ++i) {
float actual = float actual =
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment