Core: Reimplement math structs as constexpr

[Repiteo]

Supersedes #91089

In constrast to the above commit having constexpr as a sort of bonus addition from initializer lists, this commit focuses squarely on incorporating a constexpr foundation for various math structs. Because making everything about these structs constexpr would be extremely noisy & make micromanaging the more explicit changes a headache, this instead implements the concept for constructors and operators (and any constructor-helper funcs).

The most obvious change revolves around operator[] for structs with union members. This is because, to my knowledge, constexpr environments cannot dynamically change an active union member. Because construction values are explicitly assigned to named members, the usual approach of calling to the array union member will simply fail. This can be worked around by explicitly checking for the named values via a switch statement first.

constexpr Vector3 vec3 = Vector3();

// Normal environment, any extraction method works
const real_t val_name = vec3.x;
const real_t val_coord = vec3.coord[0];
const real_t val_index = vec3[0];

// Constexpr environment
constexpr real_t const_name = vec3.x; // Works, struct containing `x` is active union member.
constexpr real_t const_coord = vec3.coord[0]; // Error, accessing inactive union member.
constexpr real_t const_index = vec3[0]; // Error with old implementation, works on PR. See below:

// Old implementation
_FORCE_INLINE_ real_t &operator[](int p_axis) const {
	DEV_ASSERT((unsigned int)p_axis < 3);
	return coord[p_axis]; // Tries to access inactive member, fails.
}

// New implementation
constexpr const real_t &Vector3::operator[](size_t p_axis) const {
	switch (p_axis) {
		case AXIS_X: // Explicitly parse for valid values, assign to named equivalents.
			return x;
		case AXIS_Y:
			return y;
		case AXIS_Z:
			return z;
		default: // Only remaining values are oob, will be logged as such.
			return coord[p_axis];
	}
}

Some other aspects of the code that were adjusted are:

• Removed redundant variables from codebase: As I learned in Core: Utilize initalizer lists in Math constructors #91089, making these structs constexpr was enough to make GCC "aware" of unused instances of these variables. These came up as actual warnings now, which made GHA builds fail, so removing them were the only non-math changes in this PR.
• Index args for operator[] now use size_t: Works around a bizarre error on GHA where it was parsing certain values as invalid (Error: ./core/math/vector3.h:209:23: error: array subscript [0, 2] is outside array bounds of 'real_t [3]' {aka 'float [3]'} [-Werror=array-bounds]). Couldn't replicate locally, so it's possible this is something particular to older GCC versions? Either way, the errors no longer occurred when switching int to size_t as the array index, which is probably the more modern approach anyway. It still recognizes actual oob access from arguments, so nothing was lost in translation.
• Operator/Constructor sorting now consistent: A ton of the files were all over the place in terms of where they positioned their constructors & operators. Because I was already shuffling & somewhat reformatting their contents, it was the perfect opportunity to get them relatively organized.
• Assignment operators return self instead of void where applicable: Implementation was inconsistent across the different math structs on if they should return self or void, with no real rhyme or reason to the decisions; as part of their reshuffling, I opted for the former. I'm fully aware the repo avoids the practice of x = y = z syntax, but this matches built-in/stdlib functionality & doesn't actually change any existing code; it's only "supported" in that it's functional.
• Removed redundant self construct/assign operators where applicable: The few structs that were doing this weren't doing anything special with them, so they are able to be wholly substituted for their built-in equivalents.
• Removed Projection destructor: A user-defined destructor prevents becoming constexpr & the destructor was already empty.
• Constant variables made constexpr: Namespace variables were converted to inline constexpr and class variables were converted to static constexpr. Only applied to the files I was already adjusting.
• Converted relevant tests to constexpr: Almost every test with a simple math struct variable has been converted to constexpr, and checks with all-constexpr logic were converted to static_assert.

[clayjohn]

You've done a great job explaining the technical considerations you faced when implementing this PR. Can you also provide an explanation about why these changes are beneficial? At face value, this looks like a stylistic change that ended up necessitating some functional changes that may have performance tradeoffs and may impact compatibility with older compiler versions (in addition to introducing risk throughout the engine).

If performance is a factor, can you post the results of the benchmarking that you did while making this change? If the benefit is stylistic, can you explain why the benefit outweighs the cost?

[fire]

Style improvements

One point of favour constexpr allows the compiler to remove dead code (unused variables) from the constructors.

This conversion also removed duplicated methods.

[clayjohn]

One point of favour constexpr allows the compiler to remove dead code (unused variables) from the constructors.

This conversion also removed duplicated methods.

The compiler does that anyway, see #91089 (comment)

[fire]

Yes, that's why I'm approaching it from an improved style point of view and not performance.

Like the principle of don't repeat yourself. https://en.wikipedia.org/wiki/Don%27t_repeat_yourself

[Repiteo]

The primary functional benefits are those inherent to constexpr: the ability to evaluate variables at compile-time. Relatedly, benefits from constexpr aren't inherently applied to non-constexpr environments, so practically speaking this won't change the current codebase's performance. However, this does open the door for constexpr to be assigned to variables of these types; benchmarking will only have a real impact once variables start implementing this. Older compiler versions shouldn't be a concern so long as they have proper C++17 support; after #91833 bumped the GCC minimum version to 9, I don't think any current minimums feature breaking regressions with C++17, but more testing of those minimum versions couldn't hurt.

[clayjohn]

In that case, I'm not sure what to say. We can't really justify changing 1700 lines of deeply core code for no benefit. The ability to evaluate variables at compile-time is really nice for development since it helps you catch type errors while writing code. But you are applying this to code that has already been written, tested, and used in production for years. Effectively creating a risk of introducing new bugs into a code that works in the name of making the code easier to write.

To be perfectly blunt, I think this falls into the "if its not broken, don't fix it" territory.

If there were some other benefit to this change, then it would be a different story. So perhaps it makes sense to look into those performance benefits that we could take advantage of and form a picture of whether that is something worth pursuing.

[Repiteo]

I'm used to separating foundational implementation from codebase integration in PRs, but it sounds like that's not viable here. One obvious option would be the ability to convert several runtime tests to static_assert equivalents, catching breaking changes immediately:

TEST_CASE("[AABB] Constructor methods") {
	const AABB aabb = AABB(Vector3(-1.5, 2, -2.5), Vector3(4, 5, 6));
	const AABB aabb_copy = AABB(aabb);

	CHECK_MESSAGE(
			aabb == aabb_copy,
			"AABBs created with the same dimensions but by different methods should be equal.");
}

namespace AABBConstructorMethods {
inline constexpr AABB aabb = AABB(Vector3(-1.5, 2, -2.5), Vector3(4, 5, 6));
inline constexpr AABB aabb_copy = AABB(aabb);

static_assert(
		aabb == aabb_copy,
		"AABBs created with the same dimensions but by different methods should be equal.");
} //namespace AABBConstructorMethods

[AThousandShips]

This should be an error instead IMO, the dev assert is there to make the error explicit and reliable, now it becomes a contextual segfault

[Repiteo]

Those error macros don't play nice with constexpr environments out of the gate, but an early catch would still be nice in some form. Can't use consteval nor std::is_constant_evaluated in C++17, but __builtin_is_constant_evaluated should do the trick.

[Repiteo]

The scope of this is much greater than I expected, even after trying to trim things down. Gonna be converting this to a draft while I work on piecemealing the concepts across other PRs, starting with #92059.