Skip to content

Clean up is_frustum projection code#116752

Merged
Repiteo merged 1 commit into
godotengine:masterfrom
BastiaanOlij:cleanup_is_frustum
Mar 4, 2026
Merged

Clean up is_frustum projection code#116752
Repiteo merged 1 commit into
godotengine:masterfrom
BastiaanOlij:cleanup_is_frustum

Conversation

@BastiaanOlij
Copy link
Copy Markdown
Contributor

Cleanup projection code so we don't need our frustum override in the renderer.

As I'm working through a bunch of projection changes I ran into the hacky workarounds we introduced in the renderer around the "Frustum" camera mode. It always irked me that we had to do these things as they didn't make sense.

What I think happened here is that as we migrated code over from Godot 3, we were actually dealing with an issue that we were incorrectly applying projection[3][2] in our unprojection code.

This was never visible because for all projection matrices except the "newly" introduced Frustum mode, and when stereo rendering, projection[3][2] is ALWAYS zero. With frustum mode we have the ugly hacks, for stereo rendering we use full unprojection and skip the faulty code.

I did a bunch of cleanup to ensure both compatibility and RD renderers use the proper unproject formulas for our sky rendering (the only place we applied the hack) and that we properly and consistently use our flip_y settings so we don't have to make weird changes to the projection matrices to make this work.

Could use some help testing this all out to make sure I haven't overlooked a place, but I've tested all different camera modes, checking if the sky still renders properly, making sure reflections of the sky are still correctly oriented, and checking if reflection probes still render properly and all looks good.

@BastiaanOlij BastiaanOlij marked this pull request as ready for review February 25, 2026 10:16
@BastiaanOlij BastiaanOlij requested a review from a team as a code owner February 25, 2026 10:16
@BastiaanOlij
Copy link
Copy Markdown
Contributor Author

cc @dsnopek @Calinou

@AThousandShips AThousandShips changed the title Cleanup is_frustum projection code Clean up is_frustum projection code Feb 25, 2026
@@ -195,7 +195,7 @@ void main() {
#else
cube_normal.z = -1.0;
cube_normal.x = (uv_interp.x + projection.x) / projection.y;
Copy link
Copy Markdown
Contributor Author

@BastiaanOlij BastiaanOlij Feb 25, 2026

Choose a reason for hiding this comment

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

One small remark here, and also for the same calculation in the render device version of this shader. We're doing an addition here which is counter intuitive because for un-projecting you would expect to subtract.

But we have to remember that originally we project our vertex by doing + z * projection[2][0] and we are looking in the z- direction. So we end up with + -1.0 * projection[2][0] which means we need to do - -1.0 * projection[2][0] to un-project. Hence + projection[2][0]...

Also very counter intuitive is the order in which we've stored our projection matrix entries into our vector, yes projection.x stores projection[2][0] while projection.y stores projection[0][0].

Just interesting info to have for prosperity :)

Copy link
Copy Markdown
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 on the 3D platformer demo (with camera mode changed to frustum), it works as expected. I double-checked the sky orientation is correct on all renderers.

Code looks good to me.

@Repiteo Repiteo merged commit 4296a0c into godotengine:master Mar 4, 2026
20 checks passed
@Repiteo
Copy link
Copy Markdown
Contributor

Repiteo commented Mar 4, 2026

Thanks!

@BastiaanOlij BastiaanOlij deleted the cleanup_is_frustum branch March 6, 2026 02:19
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.

3 participants