Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LOD system does not recognize when camera enters AABB (solution provided) #88273

Closed
Tracked by #57416
roalyr opened this issue Feb 13, 2024 · 3 comments
Closed
Tracked by #57416

Comments

@roalyr
Copy link

roalyr commented Feb 13, 2024

Tested versions

4.x as of 2d0ee20

System information

Any platform

Issue description

In the LOD system:

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

Both distances are always positive values, so the condition that checks if camera is inside AABB:

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

Never triggers.

Video: https://www.youtube.com/watch?v=j_rp_k5rDPA

Solution

To fix the issue - supplement distance_min with a sign, for example: roalyr@afbdb54
Video: https://www.youtube.com/watch?v=LF21eYlb44Y

I am not opening a PR because I am not sure if this approach is the best.

Steps to reproduce

Set LOD bias to a really low value (0.1) and move camera inside AABB of a mesh, rotate the view.

Minimal reproduction project (MRP)

Irrelevant

@roalyr
Copy link
Author

roalyr commented Feb 13, 2024

Also

} else if (distance_max <= 0.0) {
    distance = -distance_max;
}

needs to be addressed in the same fashion, because distance_max doesn't get negative either.

@roalyr
Copy link
Author

roalyr commented Feb 13, 2024

In fact, it is questionable why distance_max is used at all. Why not origin?

@clayjohn
Copy link
Member

clayjohn commented May 23, 2024

Fixed by #79590 or #92232

@clayjohn clayjohn added this to the 4.3 milestone May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants