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 mobile and gl_compatibility renderers sky_transform operations #69636

Merged

Conversation

Malcolmnixon
Copy link
Contributor

@Malcolmnixon Malcolmnixon commented Dec 6, 2022

This pull request fixes issue #69630 by performing the sky_transform operations in the same order as performed for the forward_plus renderer. These changes are the same transform-fix that was performed in #61859, but applied in the new mobile renderer code.

Bugsquad edit:

@Malcolmnixon Malcolmnixon requested a review from a team as a code owner December 6, 2022 04:06
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 good, change will be needed in the OpenGL renderer as well

sky_transform = p_transform.basis * sky_transform;

… the same order as the forward_plus renderer.

Update rasterizer_scene_gles3.cpp

Apply sky_transform order fix to the gles3 renderer.
@Malcolmnixon
Copy link
Contributor Author

As requested I applied the sky_transform order fix to the gles3 renderer. Visual inspection shows it has the same transform-ordering bug, but I cannot confirm whether the fix worked because the sky just shows as gray when I switch to the gl_compatibility renderer.

@akien-mga akien-mga merged commit bd290ad into godotengine:master Dec 6, 2022
@akien-mga
Copy link
Member

akien-mga commented Dec 6, 2022

Thanks! And congrats for your first merged Godot contribution 🎉 (in this repo at least ;))

@akien-mga akien-mga changed the title Fix mobile renderer sky_transform operations Fix mobile and gl_compatibility renderers sky_transform operations Dec 9, 2022
@Malcolmnixon Malcolmnixon deleted the vulkan-mobile-sky-matrix branch December 10, 2022 14:26
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.

Godot 4 Sky rotation applied in wrong order
4 participants