Math::round icorrectly rounds big values

[Shatur]

Godot version:
6dad72d

OS/device including version:
Any

Issue description:

Math::round returns inf for DBL_MAX because it adds 0.5 to the passed value that causes overflow for maximum value:

godot/core/math/math_funcs.h

Line 278 in 365ab88

static _ALWAYS_INLINE_ double round(double p_val) { return (p_val >= 0) ? Math::floor(p_val + 0.5) : -Math::floor(-p_val + 0.5); }

Steps to reproduce:

double value = Math::round(DBL_MAX); // value will be inf

Minimal reproduction project:

Just add the following check:

CHECK(Math::is_equal_approx(Math::round(DBL_MAX), DBL_MAX));

to round/floor/ceil test case in this PR: #48721

[lawnjelly]

I suspect there is a problem in your test. When I tried debugging:

	double d = Math::round(DBL_MAX);
	double d2 = Math::round(-DBL_MAX);

	float f = Math::round(FLT_MAX);
	float f2 = Math::round(-FLT_MAX);

In the code, the results after the round are the same, i.e. DBL_MAX is the same rounded.
I found the same in plain c++, adding 0.5 to DBL_MAX gives the same value (due to lack of precision), same with -DBL_MAX, same with floats.

What could cause an INF is if double is being truncated to float somewhere in the test.

The other possibility is different behaviour of your CPU (maybe using SSE or rounding mode), but a problem with the test seems more likely.

[lawnjelly]

And this correctly returns true on my machine:

bool equal = Math::is_equal_approx(Math::round(DBL_MAX), DBL_MAX);

What OS / CPU are you trying this on? And can you replicate it in c++?

Curiously, on 3.x at least, is_equal_approx appears to take real_t as an argument, which I'm not sure is right. There should probably be separate is_equal_approx functions for float and for double, and none for real_t (it will select the correct type). I'll have a look at the code for Godot 4 and see if there's anything suspicious there.

The is_equal_approx also uses real_t in Godot 4. This may the cause of the problem, if not, it does not look correct. The fact it is 'working' at all with doubles may be an accident. If you compare two doubles outside float range with is_equal_approx when real_t is defined as float, you would think it would truncate to float (giving INF), thus compare INF to INF...

There may even be some compiler quirks as to how this is handled, particularly with inline functions. See here for an interesting discussion:
https://stackoverflow.com/questions/17588419/when-a-float-variable-goes-out-of-the-float-limits-what-happens

[Shatur]

Oh, sorry, I a little overlooked. You are right Math::is_equal_approx(Math::round(DBL_MAX), DBL_MAX) works correctly. I just had an issue with Math::is_equal_approx and real_t that you already mentioned in #48876.

But Math::round not works correctly too. Let's consider the following example:

const double value = 9007199254740991;

const double round1 = std::round(value) << std::endl; // 9007199254740991.0
const double round2 = Math::round(value) << std::endl; // 9007199254740992.0

The fix suggested for Math::round by @Yozzaxia1311 solves the problem:

return (p_val >= 0) ? Math::ceil(p_val - 0.5) : Math::floor(p_val + 0.5);

Tested on Linux with GCC 11.1.0.

[aaronfranke]

There is another problem caused by the implementation of round doing this: When double drops to only integer precision, adding 0.5 will round it to the nearest sum of powers of two (in this case, the nearest even number).

	var x = 4503599627370497.0
	print(x)
	print(round(x))
	print(floor(x))

Output:

4503599627370497
4503599627370498
4503599627370497

[Shatur]

@aaronfranke, I was ahead of you by a couple of seconds :D
Yes, this is what I talking about.

[aaronfranke]

@Shatur95 Can you test if #48887 fixes this bug for you?

[Shatur]

@aaronfranke, yes, thank you!