Skip to content

Commit cbce57a

Browse files
committed
Rework extract_meshes
* 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.
1 parent f1aae38 commit cbce57a

File tree

1 file changed

+43
-71
lines changed

1 file changed

+43
-71
lines changed

crates/bevy_pbr/src/render/mesh.rs

+43-71
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use bevy_app::Plugin;
77
use bevy_asset::{load_internal_asset, Assets, Handle, HandleUntyped};
88
use bevy_ecs::{
99
prelude::*,
10+
query::QueryItem,
1011
system::{lifetimeless::*, SystemParamItem},
1112
};
1213
use bevy_math::{Mat4, Vec2};
@@ -93,81 +94,52 @@ bitflags::bitflags! {
9394
}
9495
}
9596

97+
pub type ExtractMeshQuery = (
98+
Entity,
99+
&'static ComputedVisibility,
100+
&'static GlobalTransform,
101+
&'static Handle<Mesh>,
102+
Option<&'static NotShadowReceiver>,
103+
);
104+
type ExtractMeshItem<'w, 's> = QueryItem<'w, 's, ExtractMeshQuery>;
105+
96106
pub fn extract_meshes(
97107
mut commands: Commands,
98-
mut previous_caster_len: Local<usize>,
99-
mut previous_not_caster_len: Local<usize>,
100-
caster_query: Query<
101-
(
102-
Entity,
103-
&ComputedVisibility,
104-
&GlobalTransform,
105-
&Handle<Mesh>,
106-
Option<&NotShadowReceiver>,
107-
),
108-
Without<NotShadowCaster>,
109-
>,
110-
not_caster_query: Query<
111-
(
112-
Entity,
113-
&ComputedVisibility,
114-
&GlobalTransform,
115-
&Handle<Mesh>,
116-
Option<&NotShadowReceiver>,
117-
),
118-
With<NotShadowCaster>,
119-
>,
108+
mut prev_len_caster: Local<usize>,
109+
mut prev_len_not: Local<usize>,
110+
caster_query: Query<ExtractMeshQuery, Without<NotShadowCaster>>,
111+
not_caster_query: Query<ExtractMeshQuery, With<NotShadowCaster>>,
120112
) {
121-
let mut caster_values = Vec::with_capacity(*previous_caster_len);
122-
for (entity, computed_visibility, transform, handle, not_receiver) in caster_query.iter() {
123-
if !computed_visibility.is_visible {
124-
continue;
125-
}
113+
let mesh_bundle = |(entity, _, transform, handle, not_receiver): ExtractMeshItem| {
126114
let transform = transform.compute_matrix();
127-
caster_values.push((
128-
entity,
129-
(
130-
handle.clone_weak(),
131-
MeshUniform {
132-
flags: if not_receiver.is_some() {
133-
MeshFlags::empty().bits
134-
} else {
135-
MeshFlags::SHADOW_RECEIVER.bits
136-
},
137-
transform,
138-
inverse_transpose_model: transform.inverse().transpose(),
139-
},
140-
),
141-
));
142-
}
143-
*previous_caster_len = caster_values.len();
144-
commands.insert_or_spawn_batch(caster_values);
145-
146-
let mut not_caster_values = Vec::with_capacity(*previous_not_caster_len);
147-
for (entity, computed_visibility, transform, mesh, not_receiver) in not_caster_query.iter() {
148-
if !computed_visibility.is_visible {
149-
continue;
150-
}
151-
let transform = transform.compute_matrix();
152-
not_caster_values.push((
153-
entity,
154-
(
155-
mesh.clone_weak(),
156-
MeshUniform {
157-
flags: if not_receiver.is_some() {
158-
MeshFlags::empty().bits
159-
} else {
160-
MeshFlags::SHADOW_RECEIVER.bits
161-
},
162-
transform,
163-
inverse_transpose_model: transform.inverse().transpose(),
164-
},
165-
NotShadowCaster,
166-
),
167-
));
168-
}
169-
*previous_not_caster_len = not_caster_values.len();
170-
commands.insert_or_spawn_batch(not_caster_values);
115+
let flags = if not_receiver.is_some() {
116+
MeshFlags::empty().bits
117+
} else {
118+
MeshFlags::SHADOW_RECEIVER.bits
119+
};
120+
let uniform = MeshUniform {
121+
flags,
122+
transform,
123+
inverse_transpose_model: transform.inverse().transpose(),
124+
};
125+
(entity, (handle.clone_weak(), uniform))
126+
};
127+
let with_marker = |(entity, (handle, uniform))| (entity, (handle, uniform, NotShadowCaster));
128+
let is_visible = |(_, vis, ..): &ExtractMeshItem| vis.is_visible;
129+
let mut caster_cmds = Vec::with_capacity(*prev_len_caster);
130+
let mut not_caster_cmds = Vec::with_capacity(*prev_len_not);
131+
caster_cmds.extend(caster_query.iter().filter(is_visible).map(mesh_bundle));
132+
not_caster_cmds.extend(
133+
not_caster_query
134+
.iter()
135+
.filter(is_visible)
136+
.map(mesh_bundle)
137+
.map(with_marker),
138+
);
139+
*prev_len_caster = caster_cmds.len();
140+
*prev_len_not = not_caster_cmds.len();
141+
commands.insert_or_spawn_batch(caster_cmds);
142+
commands.insert_or_spawn_batch(not_caster_cmds);
171143
}
172144

173145
#[derive(Debug, Default)]

0 commit comments

Comments
 (0)