Skip to content

Commit 54d014f

Browse files
rparrettameknite
authored andcommitted
Combine visibility queries in check_visibility_system (bevyengine#10196)
# Objective Alternative to bevyengine#7310 ## Solution Implemented the suggestion from bevyengine#7310 (comment) 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 bevyengine#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.
1 parent fe7203f commit 54d014f

File tree

1 file changed

+17
-45
lines changed
  • crates/bevy_render/src/view/visibility

1 file changed

+17
-45
lines changed

crates/bevy_render/src/view/visibility/mod.rs

Lines changed: 17 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -390,19 +390,10 @@ pub fn check_visibility(
390390
&InheritedVisibility,
391391
&mut ViewVisibility,
392392
Option<&RenderLayers>,
393-
&Aabb,
393+
Option<&Aabb>,
394394
&GlobalTransform,
395395
Has<NoFrustumCulling>,
396396
)>,
397-
mut visible_no_aabb_query: Query<
398-
(
399-
Entity,
400-
&InheritedVisibility,
401-
&mut ViewVisibility,
402-
Option<&RenderLayers>,
403-
),
404-
Without<Aabb>,
405-
>,
406397
) {
407398
for (mut visible_entities, frustum, maybe_view_mask) in &mut view_query {
408399
let view_mask = maybe_view_mask.copied().unwrap_or_default();
@@ -414,7 +405,7 @@ pub fn check_visibility(
414405
inherited_visibility,
415406
mut view_visibility,
416407
maybe_entity_mask,
417-
model_aabb,
408+
maybe_model_aabb,
418409
transform,
419410
no_frustum_culling,
420411
) = query_item;
@@ -430,20 +421,22 @@ pub fn check_visibility(
430421
return;
431422
}
432423

433-
// If we have an aabb and transform, do frustum culling
424+
// If we have an aabb, do frustum culling
434425
if !no_frustum_culling {
435-
let model = transform.affine();
436-
let model_sphere = Sphere {
437-
center: model.transform_point3a(model_aabb.center),
438-
radius: transform.radius_vec3a(model_aabb.half_extents),
439-
};
440-
// Do quick sphere-based frustum culling
441-
if !frustum.intersects_sphere(&model_sphere, false) {
442-
return;
443-
}
444-
// If we have an aabb, do aabb-based frustum culling
445-
if !frustum.intersects_obb(model_aabb, &model, true, false) {
446-
return;
426+
if let Some(model_aabb) = maybe_model_aabb {
427+
let model = transform.affine();
428+
let model_sphere = Sphere {
429+
center: model.transform_point3a(model_aabb.center),
430+
radius: transform.radius_vec3a(model_aabb.half_extents),
431+
};
432+
// Do quick sphere-based frustum culling
433+
if !frustum.intersects_sphere(&model_sphere, false) {
434+
return;
435+
}
436+
// Do aabb-based frustum culling
437+
if !frustum.intersects_obb(model_aabb, &model, true, false) {
438+
return;
439+
}
447440
}
448441
}
449442

@@ -454,27 +447,6 @@ pub fn check_visibility(
454447
cell.set(queue);
455448
});
456449

457-
visible_no_aabb_query.par_iter_mut().for_each(|query_item| {
458-
let (entity, inherited_visibility, mut view_visibility, maybe_entity_mask) = query_item;
459-
460-
// Skip computing visibility for entities that are configured to be hidden.
461-
// `ViewVisibility` has already been reset in `reset_view_visibility`.
462-
if !inherited_visibility.get() {
463-
return;
464-
}
465-
466-
let entity_mask = maybe_entity_mask.copied().unwrap_or_default();
467-
if !view_mask.intersects(&entity_mask) {
468-
return;
469-
}
470-
471-
view_visibility.set();
472-
let cell = thread_queues.get_or_default();
473-
let mut queue = cell.take();
474-
queue.push(entity);
475-
cell.set(queue);
476-
});
477-
478450
for cell in &mut thread_queues {
479451
visible_entities.entities.append(cell.get_mut());
480452
}

0 commit comments

Comments
 (0)