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

Use proper indices for lights, decals, and reflection probes in mobile scene shader #70929

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Jan 4, 2023

Fixes: #70280
Fixes: #70231

Because the logic in these loops reads from ***_indices before incrementing, we are incrementing for the next iteration not the current iteration. So we need to move to our second ***_indices variable during the 4th iteration not during the 5th

By incrementing in the 4th iteration, we were always reading from the 0th index on the 5th iteration which often contains a thing (decal, probe, or light) that shouldn't be visible from this mesh.

@clayjohn clayjohn added this to the 4.0 milestone Jan 4, 2023
@clayjohn clayjohn requested a review from BastiaanOlij January 4, 2023 22:08
@clayjohn clayjohn requested a review from a team as a code owner January 4, 2023 22:08
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Wouldn't this warrant an explanatory comment to make the magic value explcit?

Copy link
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.

lgtm

@akien-mga akien-mga merged commit a5e43da into godotengine:master Jan 5, 2023
@akien-mga
Copy link
Member

Thanks!

@clayjohn clayjohn deleted the RD-mobile branch May 2, 2023 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants