Extra Cull Margin triggers worst LOD

[lyuma]

Godot version

4.0.beta3 (not in 4.0.beta2)

Unlike #67772, this is not a double-precision build

System information

Windows 10, Nvidia

Issue description

Since 4.0 beta3, using high values for Extra Cull Margin will cause the LODs to break. This bug was not present on 4.0 beta2

Screenshot of Extra Cull Margin=16384 on beta3

Screenshot on beta2:

CC @Zylann Unrelated to previous bug with import and double precision builds. This is not affected by importer, and the same binary scene will look different on beta2 and beta3; or depending on extra cull margin.

CC @TechnoPorg the regression may be your PR #67307 - I haven't done a full bisect yet but it is between beta2 and beta3

Steps to reproduce

1. Open the attached project.
2. Check if the mesh looks broken (LODs)
3. Set Extra Cull Margin to 0, and then 16384, on the Body mesh to see how it affects rendering.

Minimal reproduction project

ExtraCullLODTest.zip

[lyuma]

Possibly also some relation to #67479
it's curious that both rendering issues related to LODs showed up in beta3

[clayjohn]

Indeed this is caused by #67307 Distance is now calculated as an absolute value rather than a directional value so the LODing code fails to detect when the camera is inside the AABB and instead gives an LOD distance for how far to the edge of the AABB.

It seems though that the "correct" behaviour may not be much better for your use-case. The "correct" behaviour would means that your model would never use any LOD except for the highest resolution as the camera will always be inside the model's AABB.

The bug is here:

godot/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

Lines 948 to 949 in 2b505b7

	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);

and here:

godot/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp

Lines 1815 to 1816 in 2b505b7

	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);

distance_min and distance_max need to be calculated in a way that preserves direction so that we can detect when the camera is inside the AABB. That, or we can just replace:

godot/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

Line 953 in 2b505b7

if (distance_min * distance_max < 0.0) {

with: inst->transformed_aabb.has_point(p_render_data->scene_data->cam_transform.origin)

[fire]

@clayjohn Perhaps a boolean for lod disable would be the "expected" option on the mesh instance 3d

[TechnoPorg]

Should LOD calculations factor in Extra Cull Margin? I don't think it affects the visible level of detail, so maybe only the base mesh AABB could be used.
That still doesn't solve the issue of when the camera is in the AABB, though.

[Calinou]

Should LOD calculations factor in Extra Cull Margin?

I'd say Extra Cull Margin shouldn't affect LOD. Extra Cull Margin is typically only used to workaround rendering issues with objects being culled too early, so it shouldn't change how things look in practice.

[lyuma]

This LOD bug can also be triggered by setting a large w, h and d in the mesh renderer Custom AABB box.

Reproduction steps:

1. Use reproduction project from BoneMap retarget on FBX files or FBX2glTF conversions cause broken LODs #76892 ( https://github.com/godotengine/godot/files/11437148/BoneMap_non_skinned.zip )
2. Enable editable children on the _rotated or _original versions
3. Find Body mesh
4. Set Custom AABB to have x=0,y=0,z=0 and w=1000,h=1000,d=1000
5. The model will get a distorted LOD as in the above screenshot.

Any reason not to check for inst->transformed_aabb.has_point(p_render_data->scene_data->cam_transform.origin) ? that seems like a pretty safe fix for now

[lyuma]

This bug has multiple causes so it is not fuly fixed yet. It seems to depend on the camera direction but being inside a mesh with large AABB (either using extra cull margin or using custom AABB or simply having a huge mesh) godot still seeems to use thewromg lod. Using a non equal size helps repro, like 100,100,5

[briansemrau]

The distance LOD selection appears to be much better with the following change. Not sure it's 100% correct and I haven't tested it much.

index 069b43203f..38b803d67c 100644
--- a/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp
+++ b/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp
@@ -944,23 +944,14 @@ void RenderForwardClustered::_fill_render_list(RenderListType p_render_list, con
                        // LOD

                        if (p_render_data->scene_data->screen_mesh_lod_threshold > 0.0 && mesh_storage->mesh_surface_has_lod(surf->surface)) {
-                               // 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));
+                               Vector3 forward = p_render_data->scene_data->cam_transform.basis.get_column(Vector3::AXIS_Z);
+                               Vector3 center = inst->transformed_aabb.get_center();
+                               Vector3 support = inst->transformed_aabb.get_support(-forward);
+                               real_t radius = (support - center).length();
+                               float distance = p_render_data->scene_data->cam_transform.origin.distance_to(center) - radius;

-                               // 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);
-
-                               float distance = 0.0;
-
-                               if (distance_min * distance_max < 0.0) {
-                                       //crossing plane
+                               if (distance < 0.0) {
                                        distance = 0.0;
-                               } else if (distance_min >= 0.0) {
-                                       distance = distance_min;
-                               } else if (distance_max <= 0.0) {
-                                       distance = -distance_max;
                                }

                                if (p_render_data->scene_data->cam_orthogonal) {

anyone feel free to PR it if I don't find the time

[fire]

I tested @briansemrau 's patch on the original model and it seem to in error. Looking into it.

Going to sleep on this and test it more. https://github.com/V-Sekai/godot/tree/worst-lod-distances

[lyuma]

@briansemrau Would it be possible to explain a little intuition of how your proposed changes work?

Another thing. Fire was asking me a question. Do we want:

1. if inside aabb pick the best quality
2. scale quality by aabb origin

Intuitively speaking, I can't think of a reason why a mesh would ever use any LOD other than best quality, if within the AABB. but perhaps we could assert this and ignore the AABB. If the logic of meshopt is correct, it should never choose a lower quality within the AABB, because you may be staring at one specific polygon in the mesh.

Without doing a per-polygon check, there's no way to to know if you are looking at an important polygon, so no matter what logic is used, it must never use a lower quality if within the AABB

[briansemrau]

The previous code (and the version before) seemed to be checking multiple aabb supports, which intuitively doesn't make much sense to me. We want the signed distance to the AABB surface, and I'm not sure what the intent of that code is.

I simplified that logic so that the distance to the AABB is measured by the distance to the origin, minus the "radius" of the AABB towards the camera.
By calculating this way, when the camera is inside the AABB the distance is negative, showing the highest quality mesh.

My use case for this is to show a multimesh spanning a large area. It makes no sense for a tree to suddenly look lower quality while you're walking right by one just because you're in the AABB. Once inside the AABB we can't make any assumptions about the model because it's just an approximation.

[fire]

Grass multimesh

Use case is one grass multi mesh that covers the entire map. Being in the AABB should not fail

Culling extent to the maxmium extent

1. Current case is picks the most decimated lod
2. Proposals here say once the camera is inside of the AABB it should pick the LOD with the most indices
3. My proposal is to avoid disabling lods on avatar globally. The rendering system will still attempt to lod the avatar and not pick the highest index count lod.

Conclusions

We have a test case for avatars culling extent to the maxmium extent. @briansemrau can you provide a grass [tree?] multi-mesh test case?

[YuriSizov]

Fixed by #79590.