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

Fix to occlusion culling where all math is based on Euclidean distance. #98257

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

Rudolph-B
Copy link
Contributor

@Rudolph-B Rudolph-B commented Oct 17, 2024

Alternative solution to #94210. Main discussion can be found here #97712

The occlusion culler uses a point to compare with the depth mipmap to determine if an object is occluded or not. The occlusion culler currently chooses the point with the shortest Euclidean distance to the camera. While not entirely inaccurate, it can drift when compared to the actual closest point to the camera in terms of depth.

This inaccuracy increases when an object's AABB is skewed (relative to the camera perspective) and partially occluded by other objects, as seen in the MRP in #94210 and the 2D example below.

Screenshot from 2024-10-01 21-37-13

There were two possible solutions to this problem, and this PR implements the first solution.

Possible solutions:

  1. Convert the entire occlusion culling algorithm to use Euclidean distance.
  2. Select the closest point based on depth. (Fix occlusion culling by using depth instead of Euclidean distance when selecting the closest point #97712)

This PR converts the depthmap used for occlusion culling to euclidean distance. Additionally logic is then added for an orthogonal camera.

bugsquad edit: Fixes #94210

Copy link
Member

@Calinou Calinou left a 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. The overocclusion issue from #94210 no longer occurs, yet the level of occlusion remains similar as seen in the Overdraw debug draw mode.

However, in the MRP from #94210, no occlusion culling seems to occur with an orthogonal camera in the editor (using the Overdraw debug draw mode for a preview). This occurs in both master and this PR, so the issue is likely somewhere else:

image

Either way, this resolves the original issue, so it could be merged now already.

@lawnjelly
Copy link
Member

Although I was initially thinking #97712 would be simpler, having had a look at that PR last night, there are a number of gotchas with that method that are unresolved. This PR on the other hand ends up being simpler than expected, and sidesteps these gotchas, so on balance I now would be thinking of trying this method first.

Of course we will only know snags after some trial in the wild.

@Rudolph-B Rudolph-B marked this pull request as ready for review October 19, 2024 04:34
@Rudolph-B Rudolph-B requested a review from a team as a code owner October 19, 2024 04:34
@Rudolph-B Rudolph-B changed the title Alternative fix to occlusion culling where all math is based on Euclidean distance. Fix to occlusion culling where all math is based on Euclidean distance. Oct 19, 2024
@clayjohn clayjohn modified the milestones: 4.x, 4.4 Oct 25, 2024
@clayjohn
Copy link
Member

Looks good to me. I also tested locally and can confirm that the overocclusion issues in the MRP are fixed and there is no noticeable increase in false negatives (from eyeballing).

Let's go ahead with this PR and see if things improve in the wild.

@clayjohn clayjohn merged commit 78a4e63 into godotengine:master Oct 25, 2024
20 checks passed
@clayjohn
Copy link
Member

Thank you! And great work everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants