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 SSR orientation issues when using orthogonal camera #94184

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

mertkasar
Copy link
Contributor

Negative view space reconstruction was causing reversed screen space reflection rotations when using the orthogonal camera. The fix should not affect perspective.

@mertkasar mertkasar requested a review from a team as a code owner July 10, 2024 19:03
@clayjohn clayjohn added the bug label Jul 10, 2024
@Calinou Calinou added this to the 4.3 milestone Jul 10, 2024
@clayjohn
Copy link
Member

As discussed on RocketChat. Feel free to force-push to this PR with your additional fix to ensure that the reflections don't use perspective projection. I am happy either way!

@mertkasar
Copy link
Contributor Author

mertkasar commented Jul 12, 2024

Regarding the second commit:

SSR shader were using a view direction value calculated from the view space when raymarching, which produces perspective on the reflection with lower camera z-depths. Using a constant -z view direction solves this issue.

Screenshots and an MRP can be found here: #79002 (comment)

@mertkasar mertkasar requested a review from clayjohn July 12, 2024 17:16
@mertkasar mertkasar changed the title Fix reversed SSR rotations when using orthogonal camera Fix SSR orientation issues when using orthogonal camera Jul 12, 2024
@clayjohn
Copy link
Member

Looks great! The final step before merging is to squash all the commits together so that the whole PR only contains 1 big commit with all your changes. We like to merge one commit at a time to keep the git history clean and navigable.

If you don't know how to do that, we have a helpful tutorial in the official documentation https://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#the-interactive-rebase

@mertkasar
Copy link
Contributor Author

Oh, I see, that's why you mentioned force pushing 😅 On it.

- Use negative clip space values to fix reversed rotations in reflections
- Use constant -z view vector when raymarching to fix perspective in reflections
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you for the quick fix

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.

Before

ssr_ortho_before.mp4

After

ssr_ortho_after.mp4

@akien-mga akien-mga merged commit c2375d0 into godotengine:master Jul 17, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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.

Vulkan: Screen space reflections weirdly rotated with orthogonal cameras
4 participants