Skip to content

Commit

Permalink
Rework extract_meshes (bevyengine#4240)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
nicopap authored and ItsDoot committed Feb 1, 2023
1 parent edf98d2 commit f105b2b
Showing 1 changed file with 33 additions and 69 deletions.
102 changes: 33 additions & 69 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down

0 comments on commit f105b2b

Please sign in to comment.