From 6f8e632848a252ef3ce1989ac02272d4ccf7a4ea Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Tue, 9 May 2023 13:27:09 +0100 Subject: [PATCH] Make acos and asin safe A common bug with using acos and asin is that input outside -1 to 1 range will result in Nan output. This can occur due to floating point error in the input. The standard solution is to provide safe_acos function with clamped input. For Godot it may make more sense to make the standard functions safe. --- core/math/basis.cpp | 1 + core/math/math_funcs.h | 10 ++++++---- core/math/quat.cpp | 4 +++- modules/gdscript/doc_classes/@GDScript.xml | 4 ++-- scene/3d/path.cpp | 4 +++- scene/animation/skeleton_ik.cpp | 3 ++- scene/resources/sky.cpp | 6 ++++-- 7 files changed, 21 insertions(+), 11 deletions(-) diff --git a/core/math/basis.cpp b/core/math/basis.cpp index a33e030b841..48fd33ed433 100644 --- a/core/math/basis.cpp +++ b/core/math/basis.cpp @@ -925,6 +925,7 @@ void Basis::get_axis_angle(Vector3 &r_axis, real_t &r_angle) const { // as we have reached here there are no singularities so we can handle normally real_t s = Math::sqrt((elements[1][2] - elements[2][1]) * (elements[1][2] - elements[2][1]) + (elements[2][0] - elements[0][2]) * (elements[2][0] - elements[0][2]) + (elements[0][1] - elements[1][0]) * (elements[0][1] - elements[1][0])); // s=|axis||sin(angle)|, used to normalise + // acos does clamping. angle = Math::acos((elements[0][0] + elements[1][1] + elements[2][2] - 1) / 2); if (angle < 0) { s = -s; diff --git a/core/math/math_funcs.h b/core/math/math_funcs.h index 482986f27fc..557ed0564a9 100644 --- a/core/math/math_funcs.h +++ b/core/math/math_funcs.h @@ -74,11 +74,13 @@ public: static _ALWAYS_INLINE_ double tanh(double p_x) { return ::tanh(p_x); } static _ALWAYS_INLINE_ float tanh(float p_x) { return ::tanhf(p_x); } - static _ALWAYS_INLINE_ double asin(double p_x) { return ::asin(p_x); } - static _ALWAYS_INLINE_ float asin(float p_x) { return ::asinf(p_x); } + // Always does clamping so always safe to use. + static _ALWAYS_INLINE_ double asin(double p_x) { return p_x < -1 ? (-Math_PI / 2) : (p_x > 1 ? (Math_PI / 2) : ::asin(p_x)); } + static _ALWAYS_INLINE_ float asin(float p_x) { return p_x < -1 ? (-Math_PI / 2) : (p_x > 1 ? (Math_PI / 2) : ::asinf(p_x)); } - static _ALWAYS_INLINE_ double acos(double p_x) { return ::acos(p_x); } - static _ALWAYS_INLINE_ float acos(float p_x) { return ::acosf(p_x); } + // Always does clamping so always safe to use. + static _ALWAYS_INLINE_ double acos(double p_x) { return p_x < -1 ? Math_PI : (p_x > 1 ? 0 : ::acos(p_x)); } + static _ALWAYS_INLINE_ float acos(float p_x) { return p_x < -1 ? Math_PI : (p_x > 1 ? 0 : ::acosf(p_x)); } static _ALWAYS_INLINE_ double atan(double p_x) { return ::atan(p_x); } static _ALWAYS_INLINE_ float atan(float p_x) { return ::atanf(p_x); } diff --git a/core/math/quat.cpp b/core/math/quat.cpp index 37e5369faac..600c60e1f1c 100644 --- a/core/math/quat.cpp +++ b/core/math/quat.cpp @@ -35,7 +35,9 @@ real_t Quat::angle_to(const Quat &p_to) const { real_t d = dot(p_to); - return Math::acos(CLAMP(d * d * 2 - 1, -1, 1)); + + // acos does clamping. + return Math::acos(d * d * 2 - 1); } // set_euler_xyz expects a vector containing the Euler angles in the format diff --git a/modules/gdscript/doc_classes/@GDScript.xml b/modules/gdscript/doc_classes/@GDScript.xml index fd382f44258..2d7aed1f8fd 100644 --- a/modules/gdscript/doc_classes/@GDScript.xml +++ b/modules/gdscript/doc_classes/@GDScript.xml @@ -52,7 +52,7 @@ - Returns the arc cosine of [code]s[/code] in radians. Use to get the angle of cosine [code]s[/code]. [code]s[/code] must be between [code]-1.0[/code] and [code]1.0[/code] (inclusive), otherwise, [method acos] will return [constant NAN]. + Returns the arc cosine of [code]s[/code] in radians. Use to get the angle of cosine [code]s[/code]. [code]s[/code] will be clamped between [code]-1.0[/code] and [code]1.0[/code] (inclusive), in order to prevent [method acos] from returning [constant NAN]. [codeblock] # c is 0.523599 or 30 degrees if converted with rad2deg(s) c = acos(0.866025) @@ -63,7 +63,7 @@ - Returns the arc sine of [code]s[/code] in radians. Use to get the angle of sine [code]s[/code]. [code]s[/code] must be between [code]-1.0[/code] and [code]1.0[/code] (inclusive), otherwise, [method asin] will return [constant NAN]. + Returns the arc sine of [code]s[/code] in radians. Use to get the angle of sine [code]s[/code]. [code]s[/code] will be clamped between [code]-1.0[/code] and [code]1.0[/code] (inclusive), in order to prevent [method asin] from returning [constant NAN]. [codeblock] # s is 0.523599 or 30 degrees if converted with rad2deg(s) s = asin(0.5) diff --git a/scene/3d/path.cpp b/scene/3d/path.cpp index 2370104a325..ddbdcb2c5f1 100644 --- a/scene/3d/path.cpp +++ b/scene/3d/path.cpp @@ -175,7 +175,9 @@ void PathFollow::_update_transform(bool p_update_xyz_rot) { Vector3 axis = t_prev.cross(t_cur); float dot = t_prev.dot(t_cur); - float angle = Math::acos(CLAMP(dot, -1, 1)); + + // acos does clamping. + float angle = Math::acos(dot); if (likely(!Math::is_zero_approx(angle))) { if (rotation_mode == ROTATION_Y) { diff --git a/scene/animation/skeleton_ik.cpp b/scene/animation/skeleton_ik.cpp index 954ec66a845..22561d1d7be 100644 --- a/scene/animation/skeleton_ik.cpp +++ b/scene/animation/skeleton_ik.cpp @@ -296,7 +296,8 @@ void FabrikInverseKinematic::solve(Task *p_task, real_t blending_delta, bool ove const Vector3 rot_axis(initial_ori.cross(ci->current_ori).normalized()); if (rot_axis[0] != 0 && rot_axis[1] != 0 && rot_axis[2] != 0) { - const real_t rot_angle(Math::acos(CLAMP(initial_ori.dot(ci->current_ori), -1, 1))); + // acos does clamping. + const real_t rot_angle(Math::acos(initial_ori.dot(ci->current_ori))); new_bone_pose.basis.rotate(rot_axis, rot_angle); } diff --git a/scene/resources/sky.cpp b/scene/resources/sky.cpp index 52bd36a3f8b..9862bc71860 100644 --- a/scene/resources/sky.cpp +++ b/scene/resources/sky.cpp @@ -173,7 +173,8 @@ Ref ProceduralSky::_generate_sky() { normal.normalize(); - float v_angle = Math::acos(CLAMP(normal.y, -1.0, 1.0)); + // acos does clamping. + float v_angle = Math::acos(normal.y); Color color; @@ -192,7 +193,8 @@ Ref ProceduralSky::_generate_sky() { color.g *= sky_energy; color.b *= sky_energy; - float sun_angle = Math::rad2deg(Math::acos(CLAMP(sun.dot(normal), -1.0, 1.0))); + // acos does clamping. + float sun_angle = Math::rad2deg(Math::acos(sun.dot(normal))); if (sun_angle < sun_angle_min) { color = color.blend(sun_linear);