LOD selection implementation is wrong

[briansemrau]

Godot version

4.2.dev

System information

n/a

Issue description

Issue

I've just come back to work on a project and I'm certain the LOD selection code is wrong after #67307
I've witnessed LOD selection artifacts even after #79590, and looking at the code changes from #67307 the implementation doesn't make sense.

Example of broken LOD selection viewing a MultiMeshInstance3D:
Viewed from one angle:

And with camera rotated just barely, clearly the wrong LOD is used:

The problematic code:
https://github.com/godotengine/godot/blame/26c4644b388afb775c0563e7f8d70a3215c1216b/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp#L947-L967

if (!inst->transformed_aabb.has_point(p_render_data->scene_data->cam_transform.origin)) {
	// Get the LOD support points on the mesh AABB.
	Vector3 lod_support_min = inst->transformed_aabb.get_support(p_render_data->scene_data->cam_transform.basis.get_column(Vector3::AXIS_Z));
	Vector3 lod_support_max = inst->transformed_aabb.get_support(-p_render_data->scene_data->cam_transform.basis.get_column(Vector3::AXIS_Z));

	// Get the distances to those points on the AABB from the camera origin.
	float distance_min = (float)p_render_data->scene_data->cam_transform.origin.distance_to(lod_support_min);
	float distance_max = (float)p_render_data->scene_data->cam_transform.origin.distance_to(lod_support_max);

	if (distance_min * distance_max < 0.0) {
		//crossing plane
		distance = 0.0;
	} else if (distance_min >= 0.0) {
		distance = distance_min;
	} else if (distance_max <= 0.0) {
		distance = -distance_max;
	}
}

Implementation flaws

First of all, plainly using vector distance to support points will not work.

There are illogical situations where distance_min is greater than distance_max:

Additionally, distance_min/max will never be negative when calculated by Vector3::distance_to. Therefore the final distance will always be equal to distance_min, which can be arbitrarily far from the camera for long AABBs despite the mesh being so close to the camera.

I recommend reverting this code (I have a branch prepared) before trying to implement a better solution. A better solution may be harder to attain than you would expect (or prove me wrong).

Discussion notes

The purpose of LOD level selection

An ideal LOD selection should maintain optimal visual quality for all parts of the mesh (enforcing projected triangle size * lod_bias < 1 pixel). Therefore, you should locate the worst-case geometry on the mesh (the position closest to the camera view plane) and calculate the LOD based on the distance.
(This tracks with the orthogonal case which assigns distance = 1.0 where the projected size is constant with respect to distance.)

Identifying the actual problems

The discussion of whether LOD levels should be determined by radial distance or view-normal scalar projection #54885 is motivated by two core problems:

1. a need for frustum culling (if not already implemented)
2. LOD popping in/out with camera rotation

Solving problem 1 involves a different system than LODs, and problem 2 only exists if you decide to change lod_bias. The documentation suggests modifying the bias for performance, but I believe other approaches should take priority (mesh optimization, base model quality, etc.), otherwise LOD popping is completely unavoidable without implementing an independent solution. Even if rotational consistency is achieved, LODs will still pop in/out as the camera translates. This is the wrong place to solve the problem.

Possible approach for rotationally-consistent LOD level selection

If we need the marginal quality improvements from achieving rotational consistency, some notes:

1. Calculate shortest distance between AABB surface and the camera near plane (this is possibly non-trivial)
2. distance *= 1.0 / cos(camera FOV angle from screen center to corner) to make sure that LOD is high enough to ensure projected triangle size * lod_bias < 1 pixel is enforced in the corners of the screen

Steps to reproduce

Minimal reproduction project

[mrjustaguy]

Yeah, I think that PR needs to be reverted, it also totally broke Shadow rendering LOD logic

[briansemrau]

it also totally broke Shadow rendering LOD logic

Something I didn't work out is how it actually broke anything related to shadows. It appears to be independent at a glance. So I'd keep your eyes out even if my PR gets merged.

[mrjustaguy]

oh it was that PR.. There's nothing else that messed with the logic between the 2 versions I've tested where the regression occured (4.0 Beta 2 and 3) - #78664