-
-
Notifications
You must be signed in to change notification settings - Fork 22k
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
Fix Occlusion Culling Buffer occasionally getting corrupted. #98758
Conversation
bcc968d
to
0f0e240
Compare
Tested and seems to work fine. Didn't look in detail at why the math broke down in the original case, but the half extents uses 3 plane intersections which should be fine with orthogonal cameras. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, it works as expected.
occlusion_culling_orthogonal.mp4
Perspective culling still works too.
Thanks! |
When using OpenXR the new code produces different locations compared to the old code: before left_corner_world (-6.465447, 1.908369, -0.349659) without OpenXR the numbers are much closer, but still not the same as the old code (maybe due to rounding error accumulation?): |
td.pixel_corner = p_cam_transform.xform(Vector3(-viewport_half.x, -viewport_half.y, -p_cam_projection.get_z_near())); | ||
Vector3 top_corner_world = p_cam_transform.xform(Vector3(-viewport_half.x, viewport_half.y, -p_cam_projection.get_z_near())); | ||
Vector3 left_corner_world = p_cam_transform.xform(Vector3(viewport_half.x, -viewport_half.y, -p_cam_projection.get_z_near())); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has already been refactored the right way by #99974
Fixes #98756
The issue was caused by an inverse matrix operation (
p_cam_projection.inverse()
) failing due to a determinant evaluating to 0 under certain circumstances. The underlying implementation usesMath::is_zero_approx()
, which checks for values less than1e-5
. Using1e-7
also fixed the issue but could have unintended consequences, as both inverse() and is_zero_approx() are used throughout the codebase. For reference, a previous discussion can be found at #58507.The provided solution sidesteps this issue by simplifying the math to avoid using the inverse operation altogether.