Use better math constants #1447

illwieckz · 2024-11-20T16:44:01Z

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 · 2024-11-20T16:45:05Z

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

sweet235 · 2024-11-20T17:23:39Z

#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 · 2024-11-20T17:40:27Z

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 · 2024-11-20T17:42:33Z

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 · 2024-11-22T03:42:43Z

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 · 2024-11-24T00:18:44Z

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 · 2024-11-25T00:00:54Z

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 · 2024-11-25T00:05:13Z

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 · 2024-11-25T00:06:49Z

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 · 2024-11-25T03:12:48Z

src/common/math/Constant.h

+	constexpr float divpi_4_f     = .785398163f; // π÷4        M_PI_4f
+	constexpr float divpi_90_f    = .034906585f; // π÷90
+	constexpr float divpi_180_f   = .017453292f; // π÷180
+	constexpr float divpi_360_f   = .008726646f; // π÷360


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

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 · 2024-11-25T03:16:29Z

src/common/math/Constant.h

+// Generated with tools/math-constant/math-constant.py
+// Do not modify!
+
+#ifndef COMMON_CONSTANT_H_


lacks "math" component

I did cargo cult from src/common/math/Vector.h that also lacks the MATH component, now fixed for constant.h.

slipher · 2024-11-25T03:24:20Z

src/engine/renderer/tr_model_md3.cpp

@@ -218,8 +218,8 @@ bool R_LoadMD3( model_t *mod, int lod, const void *buffer, const char *modName )
 			v->xyz[ 1 ] = LittleShort( md3xyz->xyz[ 1 ] ) * MD3_XYZ_SCALE;
 			v->xyz[ 2 ] = LittleShort( md3xyz->xyz[ 2 ] ) * MD3_XYZ_SCALE;

-			lat = (latLng >> 8) * (2.0f * M_PI / 255.0f);
-			lng = (latLng & 255) * (2.0f * M_PI / 255.0f);
+			lat = (latLng >> 8) * (Math::mul2_pi_f / 255.0f);


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

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

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 · 2024-12-06T15:04:17Z

I created the API_CHANGES.md file.

illwieckz mentioned this pull request Nov 20, 2024

Use better math constants Unvanquished/Unvanquished#3200

Open

illwieckz added 7 commits November 24, 2024 08:24

fixup: generate math contants

4d79894

more

76f2a8b

more

f364d5f

more

8d86d2f

more

001dc7b

more

c697cdc

DegToRad

b1bf52e

illwieckz force-pushed the illwieckz/constants/sync branch from 9b53faf to dd52bb4 Compare November 24, 2024 23:40

slipher reviewed Nov 25, 2024

View reviewed changes

illwieckz added 4 commits November 25, 2024 23:25

fixup: constant generation: do not strip precision

7add964

fixup: constant generation: better header id

f6f1c65

fixup: ordering

b870042

Add API_CHANGES.md file

0e0f8e3

illwieckz force-pushed the illwieckz/constants/sync branch from c8e5882 to 64b4977 Compare December 6, 2024 15:03

illwieckz added 2 commits December 6, 2024 16:25

deprecate

53c745f

deprecate

8291807

illwieckz force-pushed the illwieckz/constants/sync branch from 64b4977 to 8291807 Compare December 6, 2024 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use better math constants #1447

Use better math constants #1447

illwieckz commented Nov 20, 2024 •

edited

Loading

illwieckz commented Nov 20, 2024

sweet235 commented Nov 20, 2024

slipher commented Nov 20, 2024

slipher commented Nov 20, 2024

slipher commented Nov 22, 2024

illwieckz commented Nov 24, 2024 •

edited

Loading

illwieckz commented Nov 25, 2024 •

edited

Loading

illwieckz commented Nov 25, 2024

illwieckz commented Nov 25, 2024 •

edited

Loading

slipher Nov 25, 2024

illwieckz Nov 25, 2024

slipher Nov 25, 2024

illwieckz Nov 25, 2024

slipher Nov 25, 2024

illwieckz Nov 25, 2024

slipher Nov 26, 2024

illwieckz commented Dec 6, 2024

Use better math constants #1447

Are you sure you want to change the base?

Use better math constants #1447

Conversation

illwieckz commented Nov 20, 2024 • edited Loading

illwieckz commented Nov 20, 2024

sweet235 commented Nov 20, 2024

slipher commented Nov 20, 2024

slipher commented Nov 20, 2024

slipher commented Nov 22, 2024

illwieckz commented Nov 24, 2024 • edited Loading

illwieckz commented Nov 25, 2024 • edited Loading

illwieckz commented Nov 25, 2024

illwieckz commented Nov 25, 2024 • edited Loading

slipher Nov 25, 2024

Choose a reason for hiding this comment

illwieckz Nov 25, 2024

Choose a reason for hiding this comment

slipher Nov 25, 2024

Choose a reason for hiding this comment

illwieckz Nov 25, 2024

Choose a reason for hiding this comment

slipher Nov 25, 2024

Choose a reason for hiding this comment

illwieckz Nov 25, 2024

Choose a reason for hiding this comment

slipher Nov 26, 2024

Choose a reason for hiding this comment

illwieckz commented Dec 6, 2024

illwieckz commented Nov 20, 2024 •

edited

Loading

illwieckz commented Nov 24, 2024 •

edited

Loading

illwieckz commented Nov 25, 2024 •

edited

Loading

illwieckz commented Nov 25, 2024 •

edited

Loading