Fix light culling mask behavior in Mobile and Compat renderers#98266
Conversation
There was a problem hiding this comment.
Tested locally, it works as expected in Mobile and Compatibility.
However, the solution used in Mobile is probably not the correct one, as it'd leave performance on the table since lights would not be culled on the CPU before being sent to the GPU. See comments in #86105, as it's the same fix as applied there (except it's applied in two locations in that PR).
I suggest looking at moving the culling to the CPU side in Mobile, as is already done in Compatibility in your PR. If you can't find a way to do this, then I suggest removing the Mobile changes so the Compatibility changes can be merged first.
|
Oh I see! I'll look for a while and hope i find the correct fix but if not ill just do as you said and try again only with the compatibility fix. ty for the directions! |
|
I think in both cases the check should be inside of |
|
(I really should have consulted this page again before making the changes, I think I added the culling logic for Mobile just before the pairing call but I'll revise it if need be.) Shifted culling back to CPU so that the light instances indices wont be sent to the buffer at all. Also covers both spot and omni lights in Mobile now. |
There was a problem hiding this comment.
Tested locally, it works as expected in Compatibility and Mobile.
This should be good to merge after squashing the 2 commits together; see PR workflow for instructions.
a8f304d to
ac907f8
Compare
There was a problem hiding this comment.
If the light is already culled in _scene_cull() then this check is unnecessary right?
There was a problem hiding this comment.
Yes. I skipped the check and it works just fine. (sry for that!)
ac907f8 to
fcea158
Compare
clayjohn
left a comment
There was a problem hiding this comment.
Looks great now! Thank you for investigating further.
|
Thanks! |
Fixes #94643. Added logic which checks the light's culling mask settings before proceeding with lighting calculations to wherever not present (namely the Forward-Mobile and Compatibility renderers). Just a simple port of the one already used in the Forward-Plus renderer.
Pictures for comparison:
MRP:
Compatibility:Forward+:
Forward-Mobile:
Fixed:
Compatibility:Forward+:
Forward-Mobile:
(Also fixes the bug in editor mode too)