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 Occlusion culling jitter #99941

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

Flarkk
Copy link
Contributor

@Flarkk Flarkk commented Dec 2, 2024

Jitter is applied the wrong way to the projection matrix, with 2 noticeable impacts :

  • the jitter magnitude depends on znear (more jitter when smaller)
  • no noticeable jitter with orthographic projection

This PR applies the jitter correction in clip space (left side matrix product) instead of adding the offset to the matrix' 4th column (which is not even a linear transformation, but vaguely resembles a translation in view space with scaling screwed up). This new implementation is aligned with the way jitter is applied for TAA in scene rendering and sky.

The new jitter magnitude is tuned to roughly match the former behavior with the default znear (0.05).

Test project : occlusion_jitter.zip

Captures below are from the Editor, in Display debug mode Occlusion Culling Buffer :

Before After
znear 0.5 before_05 after
znear 0.05 (default) before after
znear 0.01 before_001 after
ortho before_ortho after_ortho

@Flarkk Flarkk requested a review from a team as a code owner December 2, 2024 20:20
@clayjohn clayjohn requested a review from lawnjelly December 2, 2024 20:49
@clayjohn clayjohn added this to the 4.x milestone Dec 2, 2024
@lawnjelly
Copy link
Member

lawnjelly commented Dec 3, 2024

Oh yup I'm sure this is an improvement, I originally eyeballed it and took the jitter approach from elsewhere. 😁
Will try and take a look.

Yes it looks like your approach is much better way to apply the jitter. I probably would have noticed but I'm 99% working on 3.x.

It may even be able to be improved more if you notice anything else, I didn't spend too long on it, it was essentially a wild hunch that turned out to broadly work to solve the problem. In particular the jittering seems to affect the pixels at the edge more than the centre, maybe we are jittering with rotation and actually translating the camera could be an option too.

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Dec 3, 2024
@Flarkk
Copy link
Contributor Author

Flarkk commented Dec 3, 2024

Thanks for the feedback @lawnjelly.

Indeed there is more jitter at the edges than at the center. I figured out another approach with much more uniform results that consists in jittering the near plane corners instead of the whole matrix (only those are ultimately passed to embree in the end anyway).

It may be the subject of another PR. Will ping you there if needed.

@Repiteo Repiteo merged commit 87515ae into godotengine:master Dec 3, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 3, 2024

Thanks!

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

Successfully merging this pull request may close these issues.

5 participants