Skip to content

Fix incorrect projection matrix for GI#108628

Closed
retrotails wants to merge 1 commit into
godotengine:masterfrom
retrotails:fix_proj
Closed

Fix incorrect projection matrix for GI#108628
retrotails wants to merge 1 commit into
godotengine:masterfrom
retrotails:fix_proj

Conversation

@retrotails
Copy link
Copy Markdown
Contributor

@retrotails retrotails commented Jul 15, 2025

so I think this problem was a lot simpler than I thought. it looks like when these lines were written originally, the column order was backwards. [0][2] and [1][2] are both always 0.0 in my testing, unused elements of the matrix as far as I can tell, while [2][0] and [2][1] appear to be the intended ones.

note that this fixes voxel GI and SDFGI, but not screen space effects. thus, this is only a partial fix for #108508. these exact same lines also exist in ss_effects.cpp, and applying the same fix here does fix the X axis, but it does not fix the Y axis, so I'm leaving it as-is for someone else to tackle, because I think at least SSR might need more work than just this projection fixed in order to support an offset frustum.

@retrotails retrotails requested a review from a team as a code owner July 15, 2025 09:13
@Calinou Calinou added this to the 4.6 milestone Jul 16, 2025
@Repiteo Repiteo modified the milestones: 4.6, 4.x Jan 26, 2026
Copy link
Copy Markdown
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

Ran into this while investigating #117303 and this also fixes that issue when frustum camera mode is used.

There is one more related bug around this when stereo render is enabled and it would be great if you could include it (else I'm fixing this as part of a bigger PR of mine), could you change this line in gi.glsl:

pos.z = texelFetch(sampler2D(depth_buffer, linear_sampler), screen_pos, 0).r * 2.0 - 1.0;

to:

pos.z = texelFetch(sampler2D(depth_buffer, linear_sampler), screen_pos, 0).r;

@BastiaanOlij
Copy link
Copy Markdown
Contributor

Ok, doing some more testing on this I found additional issues. One clue that likely explains why your fix didn't work properly for the other screenspace effects is that our matrix unproject code results in an inverted-y.

In the GI code you'll find that this is only applied after reconstruct_position is called:

	vec3 vertex = reconstruct_position(pos);
	vertex.y = -vertex.y;

Unfortunately this is incorrect for stereo rendering. Whomever changed this though it would be enough to remove the y-flip from our projections depth correction, but sadly all sorts of funky things go wrong when you do this.

I'll raise a PR on top of yours with my fixes that make SDFGI work in all scenarios I've tested.

@akien-mga
Copy link
Copy Markdown
Member

Superseded by #117619. Thanks for the contribution!

@akien-mga akien-mga closed this Mar 23, 2026
@akien-mga akien-mga removed this from the 4.x milestone Mar 23, 2026
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