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 ReflectionProbe Cull Mask not working on Mobile rendering method #93175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Jun 14, 2024

I've also tested Reflection Mask on all rendering methods and can confirm it still works as intended with this fix.

Testing project: gd4MRP-MobileReflectionProbeMasks.zip

Preview

Before After
image image

@Calinou Calinou added bug topic:rendering topic:3d cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jun 14, 2024
@Calinou Calinou added this to the 4.3 milestone Jun 14, 2024
@Calinou Calinou requested a review from a team as a code owner June 14, 2024 20:24
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.

ReflectionProbes should be masked before calling the shader. Since ReflectionProbes are linked against Meshes, we can just avoid linking them if the mask does not overlap.

This way we can properly cull objects that don't fit in the mask without adding a run time cost in the shader

@Calinou
Copy link
Member Author

Calinou commented Jun 25, 2024

ReflectionProbes should be masked before calling the shader. Since ReflectionProbes are linked against Meshes, we can just avoid linking them if the mask does not overlap.

Is this change also needed in Forward+ and Compatibility?

@clayjohn
Copy link
Member

ReflectionProbes should be masked before calling the shader. Since ReflectionProbes are linked against Meshes, we can just avoid linking them if the mask does not overlap.

Is this change also needed in Forward+ and Compatibility?

It should be done in RendererSceneCull, so it would apply to Mobile and Compatibility, the Forward+ renderer has to do masking in the shader

@clayjohn clayjohn modified the milestones: 4.3, 4.4 Jul 24, 2024
@AThousandShips AThousandShips added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:rendering topic:3d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ReflectionProbe Reflection Masks have no effect on Mobile renderer
3 participants