Use better math constants

[illwieckz]

We have this:

#define M_SQRT2 1.414213562f
#define M_ROOT3 1.732050808f

So we do:

daemon/src/engine/renderer/tr_shade_calc.cpp
498:	float scale = 1.0 / M_SQRT2;

src/cgame/cg_marks.cpp
262:		if( CG_CullPointAndRadius( origin, M_SQRT2 * radius ) )

src/sgame/sg_cmds.cpp
1944:	const float AS_OVER_RT3 = ( ALIENSENSE_RANGE * 0.5f ) / M_ROOT3;

src/sgame/components/SpikerComponent.cpp
97:		float bboxEdge     = (1.0f / M_ROOT3) * bboxDiameter; // Assumes a cube.
98:		float hitEdge      = bboxEdge + ((1.0f / M_ROOT3) * ma->size); // Add half missile edge.

I suggest we get this:

#define M_SQRT2  1.414213562f
#define M_RSQRT2 0.707106781f
#define M_SQRT3  1.732050808f
#define M_RSQRT3 0.577350269f

So we can do instead:

daemon/src/engine/renderer/tr_shade_calc.cpp
498:	float scale = M_RSQRT2;

src/cgame/cg_marks.cpp
262:		if( CG_CullPointAndRadius( origin, M_SQRT2 * radius ) )

src/sgame/sg_cmds.cpp
1944:	const float AS_OVER_RT3 = ( ALIENSENSE_RANGE * 0.5f ) * M_RSQRT3;

src/sgame/components/SpikerComponent.cpp
97:		float bboxEdge     = M_RSQRT3 * bboxDiameter; // Assumes a cube.
98:		float hitEdge      = bboxEdge + (M_RSQRT3 * ma->size); // Add half missile edge.

See also:

• Use better math constants Unvanquished/Unvanquished#3200

[illwieckz]

We can discuss the opportunity to use more precision if we want to.

[sweet235]

#define M_SQRT2 1.414213562f

The accuracy is a little unusual. float has 24 binary digits, that is about 7 decimal digits. I would have used 8 digits, like in 1.4142136f.

[slipher]

Let's use names that are not the same as the C ones so we don't have to deal with ifdef bullshit. How about Math::sqrt2_f (float version) and Math::sqrt2_d (double version). They can be constexpr instead of macros.

[slipher]

The accuracy is a little unusual. float has 24 binary digits, that is about 7 decimal digits. I would have used 8 digits, like in 1.4142136f.

For a random float value 7 digits is often enough, but some need up to 9 digits to get back the float value which is the closest to the number you want to represent.

[slipher]

Besides annoying ifdef stuff, using the C library names is also bad because of the type mismatch. The C library ones are expected to be floats. So a 3rd-party library using these could get our definitions and end up with a value with the wrong type and less precision than expected. Conversely, when the C library does provide them we will end up using double instead of float in our own code when we didn't want to.

[illwieckz]

so we don't have to deal with ifdef bullshit.

Oh, right, M_SQRT2 is defined in /usr/include/math.h, I guess the ifdef thing was a QVM workaround…

[illwieckz]

I added some Math::<something> constants.

I wrote a generator for them in Python using mpmath. It can generate constants for float, double and long double. For now I only enabled float and double, as it was suggested to do float and double variants, though it looks like only float ones are used.

For completeness I added one constant for every one defined by GNU or std::numbers, so a few of them may be unused.

I added some constants that weren't defined in either GNU or std::numbers but look to be useful to us.

I also turned DEG2RAD and RAD2DEG macros into Math::DegToRad() and Math::RadToDeg() functions.

I actually wonder if all x×π÷180 and x×180÷π we can find in the code are actually undocumented instances of them, and we may replace them by usage of these functions to make the code more auto-documented.

There is no one usage of any M_* defines like M_PI or M_SQRT2 in our src/ directories anymore.

For the naming scheme, I followed the way it was done in std::numbers: <function><value>, like sqrt2, sqrtpi, etc. and for 1/something I did but inv_<something as well, like inv_sqrtpi or inv_pi.

The exception is Γe which is named egamma while it is the value of gamma(e), because that's how std::numbers did it.

For constants from functions that have more than one argument (something std::numbers doesn't have but GNU have) I did <function><value1>_<value2>, so for example 180÷π is div180_pi but π÷180 is divp_180i, and 2÷√π (named M_2_SQRTPI by GNU) is div2_sqrtpi.

As suggested, I added a _f and _d suffix for the type-specific names. So for example we have Math::pi_f and Math::pi_s. This differs from std::numbers and GNU in the way they both only use a suffix and without underscore when it's not a float, so basically M_PIf and M_PI and std::numbers:pif and std::numbers:pi.

I did some search for hardcoded constant values, and I replaced all the ln(2) hardcoded as 0.6931472f with Math::ln2_f. Maybe other values can be found and rewritten as well.

[illwieckz]

There was at least two comments saying to include headers in specific order to avoid getting in troubles with M_PI and third-party libraries, I guess getting rid of any M_PI name in our code base fixes that.

[illwieckz]

The generator script is not run at build time, as we only need to run it when we add a new named constant. The one adding such new named constant can run the script and commit the regenerated header.

[slipher]

The ones less than 0.1 seem to have less precision. You should do like %.9g not %.9f

[illwieckz]

It's me who actually stripped that to have nice aligned columns! 🤦‍♀️️ I forgot I could aligne the numbers to the right. Now fixed.

[slipher]

Would rather use 2.0f * Math::pi_f. "mul2_pi" is incomprehensible

[illwieckz]

Unfortunately I cannot do Math::2pi_f. 😭️🤣️

[slipher]

I think the ones that are just a power of 2 times some other constant should be removed. Especially if they are not in any of the other math libraries.

[illwieckz]

I'm fine with those names, mul2_pi just means mul(2, pi) so 2π, like div2_pi means div(2, pi) so 2/π, this is consistant. It matches prefixed writing and function calling (which uses a prefixed convention by the way).

Also, this constant is used very often, and some math libraries like GLM also provides a dedicated symbol for it as well, named glm::two_pi().

That GLM naming looks interesting at first glance but it quickly goes out of hands, for example with inv(mul(2, pi) so 1/2π, I named it inv_mul2_pi but GLM named one_over_two_pi…

I'm not surprised GLM doesn't define a constant for π/360, as what I named divpi_360, GLM would name it pi_over_three_hundred_and_sixty… 🤦‍♀️️

[illwieckz]

We also see that GLM naming convention is following infix convention, which is also calling for trouble.

So I'm fine with the convention I propose, it's the best I could find: better than GLM, and very close to standard C++ names.

I'm also fine with those constants being defined to begin with, they are common enough for other math libraries to define them and define more than what GCC and the C++ standard do. Actually GLM define some constants I haven't defined, like 4/π or 3/2π…

[necessarily-equal]

For those I would just use normal multiplication and delete the 2*π constants :S

[necessarily-equal]

For those I would just use normal multiplication and delete the 2*π constants. Only downside is that in a few case one would want to put parethesis if after a divide sign, but that should be rare and easy enough. And for performance concerns I'm really not worried about it, see my main comment

[slipher]

That GLM naming looks interesting at first glance but it quickly goes out of hands, for example with inv(mul(2, pi) so 1/2π, I named it inv_mul2_pi but GLM named one_over_two_pi…

The GLM naming is better since it follows mathematical terminology, so I would actually be able to tell what it means without having to go read the source.

Actually GLM define some constants I haven't defined, like 4/π or 3/2π…

WTF

[illwieckz]

I created the API_CHANGES.md file.

[illwieckz]

While making use of the new constants, the compiler type check insulted me because of type mismatches with double hardcoded values, that reminds me may have to check for all the useless double hardcoded values (like 3.0 instead of 3.0f) that may turn whole operations from float to double for nothing.

[necessarily-equal]

I've checked the code somewhat quickly, that's ok with me. I would would argue that this is a bit overkill though ^_^' Especially for the constants that do things like 2/3: since we enable agressive optimisations (that are not allowed in IEE-754) that allow reordering the operations. So gcc can already turn y = x * 2.0f / 3.0f into y = x * (2.0f / 3.0f), and calculate that constant at compile-time (clang also, and we give /fp:fast to MSVC which is assume does it also. By the way gcc likewise turns division by a constant into a multiplication (with our optimisation flags). So don't worry about it too much really

I also did change plenty of constants to floats back in the day. I think those are helpful. Good that you did catch some more.