Combine visibility queries in check_visibility_system (#10196)

# Objective

Alternative to #7310

## Solution

Implemented the suggestion from
https://github.com/bevyengine/bevy/pull/7310#discussion_r1083356655

I am guessing that these were originally split as an optimization, but I
am not sure since I believe the original author of the code is the one
speculating about combining them up there.

## Benchmarks

I ran three benchmarks to compare main, this PR, and the approach from
#7310
([updated](https://github.com/rparrett/bevy/commits/rebased-parallel-check-visibility)
to the same commit on main).

This seems to perform slightly better than main in scenarios where most
entities have AABBs, and a bit worse when they don't (`many_lights`).
That seems to make sense to me.

Either way, the difference is ~-20 microseconds in the more common
scenarios or ~+100 microseconds in the less common scenario. I would
speculate that this might perform **very slightly** worse in
single-threaded scenarios.

Benches were run in release mode for 2000 frames while capturing a trace
with tracy.

| bench | commit | check_visibility_system mean μs |
| -- | -- | -- |
| many_cubes | main | 929.5 |
| many_cubes | this | 914.0 |
| many_cubes | 7310 | 1003.5 |
| | |
| many_foxes | main | 191.6 |
| many_foxes | this | 173.2 |
| many_foxes | 7310 | 167.9 |
| | |
| many_lights | main | 619.3 |
| many_lights | this | 703.7 |
| many_lights | 7310 | 842.5 |

## Notes

Technically this behaves slightly differently -- prior to this PR, view
visibility was determined even for entities without `GlobalTransform`. I
don't think this has any practical impact though.

IMO, I don't think we need to do this. But I opened a PR because it
seemed like the handiest way to share the code / benchmarks.

## TODO

I have done some rudimentary testing with the examples above, but I can
do some screenshot diffing if it seems like we want to do this.
This commit is contained in:
Rob Parrett 2023-11-02 15:06:38 -07:00 committed by GitHub
parent 0dfb6cf89b
commit 09c2090c15
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -390,19 +390,10 @@ pub fn check_visibility(
&InheritedVisibility,
&mut ViewVisibility,
Option<&RenderLayers>,
&Aabb,
Option<&Aabb>,
&GlobalTransform,
Has<NoFrustumCulling>,
)>,
mut visible_no_aabb_query: Query<
(
Entity,
&InheritedVisibility,
&mut ViewVisibility,
Option<&RenderLayers>,
),
Without<Aabb>,
>,
) {
for (mut visible_entities, frustum, maybe_view_mask) in &mut view_query {
let view_mask = maybe_view_mask.copied().unwrap_or_default();
@ -414,7 +405,7 @@ pub fn check_visibility(
inherited_visibility,
mut view_visibility,
maybe_entity_mask,
model_aabb,
maybe_model_aabb,
transform,
no_frustum_culling,
) = query_item;
@ -430,42 +421,23 @@ pub fn check_visibility(
return;
}
// If we have an aabb and transform, do frustum culling
// If we have an aabb, do frustum culling
if !no_frustum_culling {
let model = transform.affine();
let model_sphere = Sphere {
center: model.transform_point3a(model_aabb.center),
radius: transform.radius_vec3a(model_aabb.half_extents),
};
// Do quick sphere-based frustum culling
if !frustum.intersects_sphere(&model_sphere, false) {
return;
if let Some(model_aabb) = maybe_model_aabb {
let model = transform.affine();
let model_sphere = Sphere {
center: model.transform_point3a(model_aabb.center),
radius: transform.radius_vec3a(model_aabb.half_extents),
};
// Do quick sphere-based frustum culling
if !frustum.intersects_sphere(&model_sphere, false) {
return;
}
// Do aabb-based frustum culling
if !frustum.intersects_obb(model_aabb, &model, true, false) {
return;
}
}
// If we have an aabb, do aabb-based frustum culling
if !frustum.intersects_obb(model_aabb, &model, true, false) {
return;
}
}
view_visibility.set();
let cell = thread_queues.get_or_default();
let mut queue = cell.take();
queue.push(entity);
cell.set(queue);
});
visible_no_aabb_query.par_iter_mut().for_each(|query_item| {
let (entity, inherited_visibility, mut view_visibility, maybe_entity_mask) = query_item;
// Skip computing visibility for entities that are configured to be hidden.
// `ViewVisibility` has already been reset in `reset_view_visibility`.
if !inherited_visibility.get() {
return;
}
let entity_mask = maybe_entity_mask.copied().unwrap_or_default();
if !view_mask.intersects(&entity_mask) {
return;
}
view_visibility.set();