From b5b6d3a8ec722719edd3f260e789315b87322b4c Mon Sep 17 00:00:00 2001 From: Aaron Franke Date: Thu, 20 May 2021 06:04:41 -0400 Subject: [PATCH] Make is_equal_approx have explicit float and double versions --- core/math/basis.cpp | 2 +- core/math/math_funcs.h | 34 ++++++++++++++++--- core/math/quat.cpp | 2 +- core/math/vector2.cpp | 2 +- core/math/vector3.h | 2 +- .../plugins/sprite_frames_editor_plugin.cpp | 2 +- servers/text_server.cpp | 2 +- tests/test_color.h | 6 ++-- tests/test_curve.h | 28 +++++++-------- tests/test_expression.h | 24 ++++++------- tests/test_json.h | 4 +-- tests/test_rect2.h | 10 +++--- 12 files changed, 72 insertions(+), 46 deletions(-) diff --git a/core/math/basis.cpp b/core/math/basis.cpp index 50299902eb2..037378b9d7a 100644 --- a/core/math/basis.cpp +++ b/core/math/basis.cpp @@ -109,7 +109,7 @@ bool Basis::is_diagonal() const { } bool Basis::is_rotation() const { - return Math::is_equal_approx(determinant(), 1, UNIT_EPSILON) && is_orthogonal(); + return Math::is_equal_approx(determinant(), 1, (real_t)UNIT_EPSILON) && is_orthogonal(); } #ifdef MATH_CHECKS diff --git a/core/math/math_funcs.h b/core/math/math_funcs.h index c0d7649b656..40234f6ae51 100644 --- a/core/math/math_funcs.h +++ b/core/math/math_funcs.h @@ -311,20 +311,20 @@ public: static float random(float from, float to); static int random(int from, int to); - static _ALWAYS_INLINE_ bool is_equal_approx(real_t a, real_t b) { + static _ALWAYS_INLINE_ bool is_equal_approx(float a, float b) { // Check for exact equality first, required to handle "infinity" values. if (a == b) { return true; } // Then check for approximate equality. - real_t tolerance = CMP_EPSILON * abs(a); + float tolerance = CMP_EPSILON * abs(a); if (tolerance < CMP_EPSILON) { tolerance = CMP_EPSILON; } return abs(a - b) < tolerance; } - static _ALWAYS_INLINE_ bool is_equal_approx(real_t a, real_t b, real_t tolerance) { + static _ALWAYS_INLINE_ bool is_equal_approx(float a, float b, float tolerance) { // Check for exact equality first, required to handle "infinity" values. if (a == b) { return true; @@ -333,7 +333,33 @@ public: return abs(a - b) < tolerance; } - static _ALWAYS_INLINE_ bool is_zero_approx(real_t s) { + static _ALWAYS_INLINE_ bool is_zero_approx(float s) { + return abs(s) < CMP_EPSILON; + } + + static _ALWAYS_INLINE_ bool is_equal_approx(double a, double b) { + // Check for exact equality first, required to handle "infinity" values. + if (a == b) { + return true; + } + // Then check for approximate equality. + double tolerance = CMP_EPSILON * abs(a); + if (tolerance < CMP_EPSILON) { + tolerance = CMP_EPSILON; + } + return abs(a - b) < tolerance; + } + + static _ALWAYS_INLINE_ bool is_equal_approx(double a, double b, double tolerance) { + // Check for exact equality first, required to handle "infinity" values. + if (a == b) { + return true; + } + // Then check for approximate equality. + return abs(a - b) < tolerance; + } + + static _ALWAYS_INLINE_ bool is_zero_approx(double s) { return abs(s) < CMP_EPSILON; } diff --git a/core/math/quat.cpp b/core/math/quat.cpp index 6f13e040270..3982a0b993b 100644 --- a/core/math/quat.cpp +++ b/core/math/quat.cpp @@ -87,7 +87,7 @@ Quat Quat::normalized() const { } bool Quat::is_normalized() const { - return Math::is_equal_approx(length_squared(), 1.0, UNIT_EPSILON); //use less epsilon + return Math::is_equal_approx(length_squared(), 1, (real_t)UNIT_EPSILON); //use less epsilon } Quat Quat::inverse() const { diff --git a/core/math/vector2.cpp b/core/math/vector2.cpp index 5129ed336ef..46a08b53ab9 100644 --- a/core/math/vector2.cpp +++ b/core/math/vector2.cpp @@ -59,7 +59,7 @@ Vector2 Vector2::normalized() const { bool Vector2::is_normalized() const { // use length_squared() instead of length() to avoid sqrt(), makes it more stringent. - return Math::is_equal_approx(length_squared(), 1.0, UNIT_EPSILON); + return Math::is_equal_approx(length_squared(), 1, (real_t)UNIT_EPSILON); } real_t Vector2::distance_to(const Vector2 &p_vector2) const { diff --git a/core/math/vector3.h b/core/math/vector3.h index b47c3cc9163..adfc52566f1 100644 --- a/core/math/vector3.h +++ b/core/math/vector3.h @@ -423,7 +423,7 @@ Vector3 Vector3::normalized() const { bool Vector3::is_normalized() const { // use length_squared() instead of length() to avoid sqrt(), makes it more stringent. - return Math::is_equal_approx(length_squared(), 1.0, UNIT_EPSILON); + return Math::is_equal_approx(length_squared(), 1, (real_t)UNIT_EPSILON); } Vector3 Vector3::inverse() const { diff --git a/editor/plugins/sprite_frames_editor_plugin.cpp b/editor/plugins/sprite_frames_editor_plugin.cpp index 476e9a072bf..551d1c027ab 100644 --- a/editor/plugins/sprite_frames_editor_plugin.cpp +++ b/editor/plugins/sprite_frames_editor_plugin.cpp @@ -513,7 +513,7 @@ void SpriteFramesEditor::_animation_select() { if (frames->has_animation(edited_anim)) { double value = anim_speed->get_line_edit()->get_text().to_float(); - if (!Math::is_equal_approx(value, frames->get_animation_speed(edited_anim))) { + if (!Math::is_equal_approx(value, (double)frames->get_animation_speed(edited_anim))) { _animation_fps_changed(value); } } diff --git a/servers/text_server.cpp b/servers/text_server.cpp index 8b035652911..daed612b028 100644 --- a/servers/text_server.cpp +++ b/servers/text_server.cpp @@ -987,7 +987,7 @@ Vector TextServer::shaped_text_get_selection(RID p_shaped, int p_start, while (i < ranges.size()) { int j = i + 1; while (j < ranges.size()) { - if (Math::is_equal_approx(ranges[i].y, ranges[j].x, UNIT_EPSILON)) { + if (Math::is_equal_approx(ranges[i].y, ranges[j].x, (real_t)UNIT_EPSILON)) { ranges.write[i].y = ranges[j].y; ranges.remove(j); continue; diff --git a/tests/test_color.h b/tests/test_color.h index eb8d7dcbd46..ad4a7cd3f2a 100644 --- a/tests/test_color.h +++ b/tests/test_color.h @@ -101,13 +101,13 @@ TEST_CASE("[Color] Reading methods") { const Color dark_blue = Color(0, 0, 0.5, 0.4); CHECK_MESSAGE( - Math::is_equal_approx(dark_blue.get_h(), 240 / 360.0), + Math::is_equal_approx(dark_blue.get_h(), 240.0f / 360.0f), "The returned HSV hue should match the expected value."); CHECK_MESSAGE( - Math::is_equal_approx(dark_blue.get_s(), 1), + Math::is_equal_approx(dark_blue.get_s(), 1.0f), "The returned HSV saturation should match the expected value."); CHECK_MESSAGE( - Math::is_equal_approx(dark_blue.get_v(), 0.5), + Math::is_equal_approx(dark_blue.get_v(), 0.5f), "The returned HSV value should match the expected value."); } diff --git a/tests/test_curve.h b/tests/test_curve.h index 019941a7ce1..3055cfd97be 100644 --- a/tests/test_curve.h +++ b/tests/test_curve.h @@ -83,13 +83,13 @@ TEST_CASE("[Curve] Custom curve with free tangents") { Math::is_equal_approx(curve->interpolate(-0.1), 0), "Custom free curve should return the expected value at offset 0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate(0.1), 0.352), + Math::is_equal_approx(curve->interpolate(0.1), (real_t)0.352), "Custom free curve should return the expected value at offset 0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate(0.4), 0.352), + Math::is_equal_approx(curve->interpolate(0.4), (real_t)0.352), "Custom free curve should return the expected value at offset 0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate(0.7), 0.896), + Math::is_equal_approx(curve->interpolate(0.7), (real_t)0.896), "Custom free curve should return the expected value at offset 0.1."); CHECK_MESSAGE( Math::is_equal_approx(curve->interpolate(1), 1), @@ -102,13 +102,13 @@ TEST_CASE("[Curve] Custom curve with free tangents") { Math::is_equal_approx(curve->interpolate_baked(-0.1), 0), "Custom free curve should return the expected baked value at offset 0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate_baked(0.1), 0.352), + Math::is_equal_approx(curve->interpolate_baked(0.1), (real_t)0.352), "Custom free curve should return the expected baked value at offset 0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate_baked(0.4), 0.352), + Math::is_equal_approx(curve->interpolate_baked(0.4), (real_t)0.352), "Custom free curve should return the expected baked value at offset 0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate_baked(0.7), 0.896), + Math::is_equal_approx(curve->interpolate_baked(0.7), (real_t)0.896), "Custom free curve should return the expected baked value at offset 0.1."); CHECK_MESSAGE( Math::is_equal_approx(curve->interpolate_baked(1), 1), @@ -172,13 +172,13 @@ TEST_CASE("[Curve] Custom curve with linear tangents") { Math::is_equal_approx(curve->interpolate(-0.1), 0), "Custom linear curve should return the expected value at offset -0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate(0.1), 0.4), + Math::is_equal_approx(curve->interpolate(0.1), (real_t)0.4), "Custom linear curve should return the expected value at offset 0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate(0.4), 0.4), + Math::is_equal_approx(curve->interpolate(0.4), (real_t)0.4), "Custom linear curve should return the expected value at offset 0.4."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate(0.7), 0.8), + Math::is_equal_approx(curve->interpolate(0.7), (real_t)0.8), "Custom linear curve should return the expected value at offset 0.7."); CHECK_MESSAGE( Math::is_equal_approx(curve->interpolate(1), 1), @@ -191,13 +191,13 @@ TEST_CASE("[Curve] Custom curve with linear tangents") { Math::is_equal_approx(curve->interpolate_baked(-0.1), 0), "Custom linear curve should return the expected baked value at offset -0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate_baked(0.1), 0.4), + Math::is_equal_approx(curve->interpolate_baked(0.1), (real_t)0.4), "Custom linear curve should return the expected baked value at offset 0.1."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate_baked(0.4), 0.4), + Math::is_equal_approx(curve->interpolate_baked(0.4), (real_t)0.4), "Custom linear curve should return the expected baked value at offset 0.4."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate_baked(0.7), 0.8), + Math::is_equal_approx(curve->interpolate_baked(0.7), (real_t)0.8), "Custom linear curve should return the expected baked value at offset 0.7."); CHECK_MESSAGE( Math::is_equal_approx(curve->interpolate_baked(1), 1), @@ -210,10 +210,10 @@ TEST_CASE("[Curve] Custom curve with linear tangents") { curve->remove_point(10); ERR_PRINT_ON; CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate(0.7), 0.8), + Math::is_equal_approx(curve->interpolate(0.7), (real_t)0.8), "Custom free curve should return the expected value at offset 0.7 after removing point at invalid index 10."); CHECK_MESSAGE( - Math::is_equal_approx(curve->interpolate_baked(0.7), 0.8), + Math::is_equal_approx(curve->interpolate_baked(0.7), (real_t)0.8), "Custom free curve should return the expected baked value at offset 0.7 after removing point at invalid index 10."); } } // namespace TestCurve diff --git a/tests/test_expression.h b/tests/test_expression.h index 0ef60d1a19e..cb1d29389f0 100644 --- a/tests/test_expression.h +++ b/tests/test_expression.h @@ -83,42 +83,42 @@ TEST_CASE("[Expression] Floating-point arithmetic") { expression.parse("-123.456") == OK, "Float identity should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), -123.456), + Math::is_equal_approx(double(expression.execute()), -123.456), "Float identity should return the expected result."); CHECK_MESSAGE( expression.parse("2.0 + 3.0") == OK, "Float addition should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 5), + Math::is_equal_approx(double(expression.execute()), 5), "Float addition should return the expected result."); CHECK_MESSAGE( expression.parse("3.0 / 10") == OK, "Float / integer division should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 0.3), + Math::is_equal_approx(double(expression.execute()), 0.3), "Float / integer division should return the expected result."); CHECK_MESSAGE( expression.parse("3 / 10.0") == OK, "Basic integer / float division should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 0.3), + Math::is_equal_approx(double(expression.execute()), 0.3), "Basic integer / float division should return the expected result."); CHECK_MESSAGE( expression.parse("3.0 / 10.0") == OK, "Float / float division should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 0.3), + Math::is_equal_approx(double(expression.execute()), 0.3), "Float / float division should return the expected result."); CHECK_MESSAGE( expression.parse("2.5 * (6.0 + 14.25) / 2.0 - 5.12345") == OK, "Float multiplication-addition-subtraction-division should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 20.18905), + Math::is_equal_approx(double(expression.execute()), 20.18905), "Float multiplication-addition-subtraction-division should return the expected result."); } @@ -129,7 +129,7 @@ TEST_CASE("[Expression] Scientific notation") { expression.parse("2.e5") == OK, "The expression should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 200'000), + Math::is_equal_approx(double(expression.execute()), 200'000), "The expression should return the expected result."); // The middle "e" is ignored here. @@ -137,14 +137,14 @@ TEST_CASE("[Expression] Scientific notation") { expression.parse("2e5") == OK, "The expression should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 25), + Math::is_equal_approx(double(expression.execute()), 25), "The expression should return the expected result."); CHECK_MESSAGE( expression.parse("2e.5") == OK, "The expression should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 2), + Math::is_equal_approx(double(expression.execute()), 2), "The expression should return the expected result."); } @@ -176,14 +176,14 @@ TEST_CASE("[Expression] Built-in functions") { expression.parse("snapped(sin(0.5), 0.01)") == OK, "The expression should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(float(expression.execute()), 0.48), + Math::is_equal_approx(double(expression.execute()), 0.48), "`snapped(sin(0.5), 0.01)` should return the expected result."); CHECK_MESSAGE( expression.parse("pow(2.0, -2500)") == OK, "The expression should parse successfully."); CHECK_MESSAGE( - Math::is_zero_approx(float(expression.execute())), + Math::is_zero_approx(double(expression.execute())), "`pow(2.0, -2500)` should return the expected result (asymptotically zero)."); } @@ -410,7 +410,7 @@ TEST_CASE("[Expression] Unusual expressions") { "The expression should parse successfully."); ERR_PRINT_OFF; CHECK_MESSAGE( - Math::is_inf(float(expression.execute())), + Math::is_inf(double(expression.execute())), "`-25.4 / 0` should return inf."); ERR_PRINT_ON; diff --git a/tests/test_json.h b/tests/test_json.h index e652a8fced4..f1cb4799dc8 100644 --- a/tests/test_json.h +++ b/tests/test_json.h @@ -80,7 +80,7 @@ TEST_CASE("[JSON] Parsing single data types") { err_line == 0, "Parsing an integer number as JSON should parse successfully."); CHECK_MESSAGE( - Math::is_equal_approx(result, 123'456), + (int)result == 123'456, "Parsing an integer number as JSON should return the expected value."); json.parse("0.123456", result, err_str, err_line); @@ -155,7 +155,7 @@ TEST_CASE("[JSON] Parsing objects (dictionaries)") { dictionary["bugs"] == Variant(), "The parsed JSON should contain the expected values."); CHECK_MESSAGE( - Math::is_equal_approx(Dictionary(dictionary["apples"])["blue"], -20), + (int)Dictionary(dictionary["apples"])["blue"] == -20, "The parsed JSON should contain the expected values."); CHECK_MESSAGE( dictionary["empty_object"].hash() == Dictionary().hash(), diff --git a/tests/test_rect2.h b/tests/test_rect2.h index b94a8b7d05b..f9ef5758c79 100644 --- a/tests/test_rect2.h +++ b/tests/test_rect2.h @@ -312,19 +312,19 @@ TEST_CASE("[Rect2i] Basic setters") { TEST_CASE("[Rect2i] Area getters") { CHECK_MESSAGE( - Math::is_equal_approx(Rect2i(0, 100, 1280, 720).get_area(), 921'600), + Rect2i(0, 100, 1280, 720).get_area() == 921'600, "get_area() should return the expected value."); CHECK_MESSAGE( - Math::is_equal_approx(Rect2i(0, 100, -1280, -720).get_area(), 921'600), + Rect2i(0, 100, -1280, -720).get_area() == 921'600, "get_area() should return the expected value."); CHECK_MESSAGE( - Math::is_equal_approx(Rect2i(0, 100, 1280, -720).get_area(), -921'600), + Rect2i(0, 100, 1280, -720).get_area() == -921'600, "get_area() should return the expected value."); CHECK_MESSAGE( - Math::is_equal_approx(Rect2i(0, 100, -1280, 720).get_area(), -921'600), + Rect2i(0, 100, -1280, 720).get_area() == -921'600, "get_area() should return the expected value."); CHECK_MESSAGE( - Math::is_zero_approx(Rect2i(0, 100, 0, 720).get_area()), + Rect2i(0, 100, 0, 720).get_area() == 0, "get_area() should return the expected value."); CHECK_MESSAGE(