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 not working properly in stereo #86996

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

BastiaanOlij
Copy link
Contributor

@BastiaanOlij BastiaanOlij commented Jan 9, 2024

Fix typo that caused #86987

Bugsquad edit:

@BastiaanOlij BastiaanOlij added topic:rendering topic:xr regression topic:3d cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jan 9, 2024
@BastiaanOlij BastiaanOlij added this to the 4.3 milestone Jan 9, 2024
@BastiaanOlij BastiaanOlij self-assigned this Jan 9, 2024
@BastiaanOlij BastiaanOlij requested a review from a team as a code owner January 9, 2024 12:18
@BastiaanOlij BastiaanOlij linked an issue Jan 9, 2024 that may be closed by this pull request
@akien-mga akien-mga added the bug label Jan 9, 2024
@akien-mga
Copy link
Member

Remember to use closing keywords in the OP :)
Fixed it already.

@akien-mga
Copy link
Member

There are a lot more cases still using MULTIVIEW, are the rest intended?

$ rg "if.* MULTIVIEW"
servers/rendering/renderer_rd/shaders/effects/copy_to_fb.glsl
7:#ifdef MULTIVIEW
25:#ifdef MULTIVIEW
44:#ifdef MULTIVIEW
65:#ifdef MULTIVIEW
94:#ifdef MULTIVIEW
100:#ifdef MULTIVIEW
111:#endif /* MULTIVIEW */
132:#ifdef MULTIVIEW
168:#ifdef MULTIVIEW
180:#endif /* MULTIVIEW */

servers/rendering/renderer_rd/shaders/effects/tonemap.glsl
7:#ifdef MULTIVIEW
27:#ifdef MULTIVIEW
47:#ifdef MULTIVIEW
128:#ifdef MULTIVIEW
189:#ifdef MULTIVIEW
276:#ifdef MULTIVIEW
367:#ifdef MULTIVIEW
402:#ifdef MULTIVIEW

servers/rendering/renderer_rd/shaders/effects/vrs.glsl
7:#ifdef MULTIVIEW
16:#ifdef MULTIVIEW
26:#ifdef MULTIVIEW
37:#ifdef MULTIVIEW
46:#ifdef MULTIVIEW
52:#endif /* MULTIVIEW */
57:#ifdef MULTIVIEW
63:#ifdef MULTIVIEW
78:#endif /* MULTIVIEW */

servers/rendering/renderer_rd/shaders/effects/specular_merge.glsl
33:#ifdef MULTIVIEW

@clayjohn
Copy link
Member

clayjohn commented Jan 9, 2024

MULTIVIEW

Yep, those all seem to use #define MULTIVIEW

@akien-mga
Copy link
Member

Shouldn't we unify those so they all use the same name?

@clayjohn
Copy link
Member

clayjohn commented Jan 9, 2024

Shouldn't we unify those so they all use the same name?

We could, there would be no harm in doing it

@dsnopek
Copy link
Contributor

dsnopek commented Jan 9, 2024

The compatibility renderer consistently uses USE_MULTIVIEW. It'd be nice to unify all the shader code on the same define.

@BastiaanOlij
Copy link
Contributor Author

Remember to use closing keywords in the OP :) Fixed it already.

Thanks, I always forget the syntax, so I just add it manually on the right side.

@BastiaanOlij
Copy link
Contributor Author

And yes, I agree that we should unify these, thats probably how the bug got introduced. But I suggest we do that in a separate PR as a mistake will break stuff.

@akien-mga akien-mga merged commit a40a134 into godotengine:master Jan 10, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 25, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.2.

@BastiaanOlij BastiaanOlij deleted the fix_stereo_ssr branch February 27, 2024 23:41
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.

SSR is broken in stereo rendering
5 participants