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 radiance for sky in GLES stereo rendering #86018

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

BastiaanOlij
Copy link
Contributor

This took me waaaaaaaaaaaaaaaaaaaaaaay too long to figure out.

In OpenGL everything is output upside down compared to Vulkan, that means that our culling order is reversed, see

	if (!flip_y) {
		// If we're rendering right-side up, then we need to change the winding order.
		glFrontFace(GL_CW);
	}

in RasterizerSceneGLES3::render_scene.

However, when we output to OpenXR (or webXR), the compositor expects us to deliver images with the same orientation and thus we change things up and render things "upside down", and only flip the image on output to screen. So we keep to the same culling order.

The problem then is that our sky radiance map update code runs after we do this optional flip. This logic renders a full screen triangle to perform a full screen effect. The order of the vertices of this triangle is defined on the assumption we are doing our flip.

In normal mode, we render our triangle and thus our radiance map is rendered.
In XR mode we cull our triangle and thus our radiance map remains black.

The simple answer is to disable culling as we don't care for culling in this case.

This PR cleans up a few related issues and code that assumes state is retained when it's not.

Fixes #84598

@BastiaanOlij BastiaanOlij added bug topic:rendering topic:xr cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Dec 11, 2023
@BastiaanOlij BastiaanOlij added this to the 4.x milestone Dec 11, 2023
@BastiaanOlij BastiaanOlij self-assigned this Dec 11, 2023
@BastiaanOlij BastiaanOlij marked this pull request as ready for review December 11, 2023 03:34
@BastiaanOlij BastiaanOlij requested a review from a team as a code owner December 11, 2023 03:34
@decacis
Copy link
Contributor

decacis commented Dec 11, 2023

It's working fine now!

Master This PR
Killifish-Ripe-Composed Gossamerwingedbutterfly-Unwilling-Distant

Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Great catch! The code changes look good to me, but probably should be reviewed by someone who knows rendering a little better than me :-)

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!

@clayjohn clayjohn modified the milestones: 4.x, 4.3 Dec 12, 2023
@akien-mga akien-mga merged commit 5088cd8 into godotengine:master Dec 13, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@BastiaanOlij BastiaanOlij deleted the fix_gles_stereo_sky branch December 15, 2023 09:53
@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 24, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.2.

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

Can't be cherry-picked for 4.1 because it depends on changes from #77496.

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.

OpenGL: Sky contribution fails in VR
7 participants