-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Refactor check_light_mesh_visibility for performance #1 #13905
Conversation
This reverts commit 6ce8c81.
check_light_mesh_visibility | ||
( | ||
check_dir_light_mesh_visibility, | ||
check_point_light_mesh_visibility, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these systems need to be ordered relative to each other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not be necessary. The main reason for splitting the system is that in subsequent PRs, we hope they can be automatically parallelized by the bevy's scheduler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've checked this over, and the refactoring / splitting appears to be done correctly. The core idea for why this should be done also makes sense to me :)
# Objective - Second part of #13900 - based on #13905 ## Solution - check_dir_light_mesh_visibility defers setting the entity's `ViewVisibility `so that Bevy can schedule it to run in parallel with `check_point_light_mesh_visibility`. - Reduce HashMap lookups for directional light checking as much as possible - Use `par_iter `to parallelize the checking process within each system. --------- Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
Objective
Solution
check_light_mesh_visibility
intocheck_dir_light_mesh_visibility
andcheck_point_light_mesh_visibility
for better review