Skip to content

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

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
clayjohn:RD-mobile
Jan 5, 2023
Merged

Use proper indices for lights, decals, and reflection probes in mobile scene shader#70929
akien-mga merged 1 commit into
godotengine:masterfrom
clayjohn:RD-mobile

Conversation

@clayjohn

@clayjohn clayjohn commented Jan 4, 2023

Copy link
Copy Markdown
Member

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

@akien-mga akien-mga left a comment

Copy link
Copy Markdown
Member

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?

@BastiaanOlij BastiaanOlij left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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