Simplify and fix Projection's getter functions

[Flarkk]

Fixes #90427
Fixes #93683

This PR simplifies and fixes a few functions of struct Projection :

• Fix get_aspect() and get_lod_multiplier() with camera in `PROJECTION_FRUSTUM mode. The latter now has a uniform behavior no matter the camera mode (perspective, frustum and orthographic). Besides it was broken in frustum mode, it also used to decimate meshes twice slower in orthographic than in perspective mode.
• Fix and document get_pixels_per_meter(). Note : this function has a clumsy definition and is not used anywhere. It's a good candidate for deprecation.
• Optimize get_z_far(), get_z_near(), get_viewport_half_extents(), get_far_plane_half_extents() and get_fov()
• Add (more) unit test to the above functions

Test project for LOD multiplier issues : lod.zip

[Flarkk]

Replaced atan(a / b) by atan2(a, b). Thanks @Ivorforce

[Ivorforce]

This PR is very interesting to me, considering it could eliminate a large number of computations in the rendering pipeline.
However, I'm also a bit concerned about the reduction in complexity. I think you're making a few assumptions that may not be true in nonstandard setups.

May I ask, how did you arrive at the simplifications in this PR?

[Ivorforce]

Just as an example, I think this would yield incorrect values for non-perspective projection matrices (e.g. orthographic).

[Flarkk]

The far plane's homogeneous coordinate (Vector4) is row[3] - row[2] (see previous implementation).
Zfar is the intercept of the plane, namely its w component after normalization.

I'm making one assumption here that is the far plane is perpendicular to Z (i.e. both x and y equal 0 in the above vector) which is the case in all current projection modes¹.

Under those conditions, normalizing the plane is just doing w / abs(z), which is exacty what's done in the new implementation.

I made sure all unit tests still pass (and added new ones to cover all projection modes).
Of course it's not a guarantee that the code is bug free, but I'm pretty confident on this one.
If you can add more tests to cover the edge cases you may have in mind, please do.

Footnotes

1. x and y may not be 0 anymore once we introduce oblique clipping planes. A specific code path might be needed then to perform a full normalization. That being said, Zfar kind of looses all its meaning with oblique planes, so I'm not even sure it would be relevant at all. ↩

[Flarkk]

I think you're making a few assumptions that may not be true in nonstandard setups.

May I ask, how did you arrive at the simplifications in this PR?

Sure thing. Here we are :

Assumptions

There are 3 general assumptions, and only really 1 new :

1. the matrix is a projection matrix and not an arbitrary transformation - already taken in previous implementation
2. near and far planes are rectangles - already taken in previous implementation
3. near and far planes are orthogonal to z-axis - new assumption, allows simplying get_z_near() and get_z_far()

All 3 are always true with our current projection modes (ORTHOGONAL, PERSPECTIVE, FRUSTUM).
Note that no.3 may not be true anymore when we introduce oblique near / far planes. However with oblique planes the notions of znear and zfar become themselves nonsensical, so it may not be an issue (if it turns out to be, a specific code path can be added).

Rationale

• get_z_near() and get_z_far() : see my comment
• get_aspect() : the matrix diagonal's values Dx and Dy are by definition k / extent.x (resp. k / extent.y) with extent.xy the near plane rectangle's size. X:Y aspect is just the ratio Dy / Dx. As a side benefit this release the assumption that the frustum is symmetrical (which is not the case in FRUSTUM mode) and solves Camera3D.get_camera_projection().get_aspect() returns unexpected results for PROJECTION_FRUSTUM #90427
• get_viewport_half_extents() and get_far_plane_half_extents() : we're looking for the vector Vector4(x, y, -[zfar or znear], 1) whose transformed clip-space coordinate is Vector4(w, w, ±w, w). Assuming a symetrical frustum (if not these functions are irrelevant) this yields x = w / Dx and y = w / Dy with w = -[zfar or znear] * m[2][3] + 1 * m[3][3] as the direct consequence of the matrix multiplication.
• get_fov() : angle between center and right (resp. left) bound of the view frustum can be seen as the atan of viewport's right (resp. left) distance over near distance. I kept the two code paths as it saves one atan2 for symmetrical frustums (most common case by far)
• get_lod_multiplier() : the current implementation returns 1.0f / (zn / width) in PERSPECTIVE mode, which can be straightforwardly simplified into 2 / Dx. In ORTHOGONAL mode however, it currently returns get_viewport_half_extents() which is 1 / Dx. I chose to harmonize to the former, which means meshes now decimate at the same pace visually in all projection modes, and fixes Geometry gets strongly visually modified (simplified) when Camera3D's x frustum_offset is decreased below a certain negative number #93683 by the fact we don't need a specific path for FRUSTUM anymore
• get_pixels_per_meter() : I couldn't get what it is supposed to return (code and documentation are inconsistent, and the function is not used anywhere). I chose to return the most sensical thing to me, which is the user input in pixels divided by the viewport's width measured in meters on the near plane. When used with the viewport's pixel width as an input, it returns the viewport's resolution in pixels per meter

[Ivorforce]

Sure thing. Here we are :
[lots of great explanations]

Awesome, thank you for the detailed breakdown!
Unfortunately, this PR is too far outside my own expertise to be confident to give you an approval, but your explanations seem sensible to me (and code wise, it's good too!).

I do have one request: Please document assumptions you make (or alternatively, those you specifically do not make) in the respective functions. It's a bit of an unfair ask, since they haven't been documented so far, but I think it's essential to keep the code integrity intact for maintainers and future contributors.

Here's a rough example of what I mean:

real_t Projection::get_z_far() const {
	// NOTE: This assumes that:
	// - The far plane is a rectangle orthogonal to z (incorrect for oblique clipping planes, which are not supported).
	// - The matrix is a valid projection matrix.
 	return -(columns[3][3] - columns[3][2]) / (columns[2][2] - columns[2][3]);
}

This might be a bit verbose, based on the overarching assumptions of the Projection class itself, so feel free to tweak the granularity of the statements. But I do think they have to be there in some form, to protect it against regressions :)

[Flarkk]

@Ivorforce just took a stab at it - see my last push.
I took this opportunity to also improve 2 minor things :

• removed the need for a negation in get_z_far() by incorporating the minus sign in the denominator
• changed is_orthogonal() to return columns[2][3] == 0.0 instead of testing whether columns[3][3] == 1.0. This is slightly more generic as the latter could return true with some interpolated perspective/orthogonal matrix setups which have both columns[2][3] != 0 && columns[3][3] != 0 (see Custom Camera3D projection : a practical approach godot-proposals#11436 for more background). These are not part of the currently allowed camera setups though, so this makes no difference right now.

[Ivorforce]

Awesome!! This es exactly what I was looking for.
Somebody should probably double check the math, and test functionality in bigger projects - but between your comments, explanations and unit tests i feel comfortable giving this a thumbs up. If we merge it early in the 4.5 cycle there should be enough time to verify everything works as expected.
Great job!