forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Rework extract_meshes (#4240)

Browse Source 
* Cleanup redundant code
* Use a type alias to make sure the `caster_query` and
  `not_caster_query` really do the same thing and access the same things

**Objective**

Cleanup code that would otherwise be difficult to understand

**Solution**

* `extract_meshes` had two for loops which are functionally identical,
  just copy-pasted code. I extracted the common code between the two
  and put them into an anonymous function.
* I flattened the tuple literal for the bundle batch, it looks much
  less nested and the code is much more readable as a result.
* The parameters of `extract_meshes` were also very daunting, but they
  turned out to be the same query repeated twice. I extracted the query
  into a type alias.

EDIT: I reworked the PR to **not do anything breaking**, and keep the old allocation behavior. Removing the memorized length was clearly a performance loss, so I kept it.
This commit is contained in:
Nicola Papale 2022-07-04 12:44:23 +00:00
parent af48c10b3e
commit 288765930f
1 changed files with 33 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
crates/bevy_pbr/src/render/mesh.rs
View File

@ -116,79 +116,43 @@ bitflags::bitflags! {
pub fn extract_meshes(
mut commands: Commands,
mut previous_caster_len: Local<usize>,
mut previous_not_caster_len: Local<usize>,
caster_query: Query<
(
Entity,
&ComputedVisibility,
&GlobalTransform,
&Handle<Mesh>,
Option<&NotShadowReceiver>,
),
Without<NotShadowCaster>,
>,
not_caster_query: Query<
(
Entity,
&ComputedVisibility,
&GlobalTransform,
&Handle<Mesh>,
Option<&NotShadowReceiver>,
),
With<NotShadowCaster>,
>,
mut prev_caster_commands_len: Local<usize>,
mut prev_not_caster_commands_len: Local<usize>,
meshes_query: Query<(
Entity,
&ComputedVisibility,
&GlobalTransform,
&Handle<Mesh>,
Option<With<NotShadowReceiver>>,
Option<With<NotShadowCaster>>,
)>,
) {
let mut caster_values = Vec::with_capacity(*previous_caster_len);
for (entity, computed_visibility, transform, handle, not_receiver) in caster_query.iter() {
if !computed_visibility.is_visible {
continue;
}
let transform = transform.compute_matrix();
caster_values.push((
entity,
(
handle.clone_weak(),
MeshUniform {
flags: if not_receiver.is_some() {
MeshFlags::empty().bits
} else {
MeshFlags::SHADOW_RECEIVER.bits
},
transform,
inverse_transpose_model: transform.inverse().transpose(),
},
),
));
}
*previous_caster_len = caster_values.len();
commands.insert_or_spawn_batch(caster_values);
let mut caster_commands = Vec::with_capacity(*prev_caster_commands_len);
let mut not_caster_commands = Vec::with_capacity(*prev_not_caster_commands_len);
let visible_meshes = meshes_query.iter().filter(|(_, vis, ..)| vis.is_visible);
let mut not_caster_values = Vec::with_capacity(*previous_not_caster_len);
for (entity, computed_visibility, transform, mesh, not_receiver) in not_caster_query.iter() {
if !computed_visibility.is_visible {
continue;
}
for (entity, _, transform, handle, not_receiver, not_caster) in visible_meshes {
let transform = transform.compute_matrix();
not_caster_values.push((
entity,
(
mesh.clone_weak(),
MeshUniform {
flags: if not_receiver.is_some() {
MeshFlags::empty().bits
} else {
MeshFlags::SHADOW_RECEIVER.bits
},
transform,
inverse_transpose_model: transform.inverse().transpose(),
},
NotShadowCaster,
),
));
let shadow_receiver_flags = if not_receiver.is_some() {
MeshFlags::empty().bits
} else {
MeshFlags::SHADOW_RECEIVER.bits
};
let uniform = MeshUniform {
flags: shadow_receiver_flags,
transform,
inverse_transpose_model: transform.inverse().transpose(),
};
if not_caster.is_some() {
not_caster_commands.push((entity, (handle.clone_weak(), uniform, NotShadowCaster)));
} else {
caster_commands.push((entity, (handle.clone_weak(), uniform)));
}
}
*previous_not_caster_len = not_caster_values.len();
commands.insert_or_spawn_batch(not_caster_values);
*prev_caster_commands_len = caster_commands.len();
*prev_not_caster_commands_len = not_caster_commands.len();
commands.insert_or_spawn_batch(caster_commands);
commands.insert_or_spawn_batch(not_caster_commands);
}
#[derive(Debug, Default)]