Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Directly extract joints into SkinnedMeshJoints #6833

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 21 additions & 31 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,6 @@ pub fn extract_meshes(
commands.insert_or_spawn_batch(not_caster_commands);
}

#[derive(Resource, Debug, Default)]
pub struct ExtractedJoints {
pub buffer: Vec<Mat4>,
}

#[derive(Component)]
pub struct SkinnedMeshJoints {
pub index: u32,
Expand All @@ -188,19 +183,22 @@ impl SkinnedMeshJoints {
skin: &SkinnedMesh,
inverse_bindposes: &Assets<SkinnedMeshInverseBindposes>,
joints: &Query<&GlobalTransform>,
buffer: &mut Vec<Mat4>,
buffer: &mut BufferVec<Mat4>,
) -> Option<Self> {
let inverse_bindposes = inverse_bindposes.get(&skin.inverse_bindposes)?;
let bindposes = inverse_bindposes.iter();
let skin_joints = skin.joints.iter();
let start = buffer.len();
for (inverse_bindpose, joint) in bindposes.zip(skin_joints).take(MAX_JOINTS) {
if let Ok(joint) = joints.get(*joint) {
buffer.push(joint.affine() * *inverse_bindpose);
} else {
buffer.truncate(start);
return None;
}
let target = start + skin.joints.len().min(MAX_JOINTS);
buffer.extend(
joints
.iter_many(&skin.joints)
.zip(inverse_bindposes.iter())
.map(|(joint, bindpose)| joint.affine() * *bindpose),
);
// iter_many will skip any failed fetches. This will cause it to assign the wrong bones,
// so just bail by truncating to the start.
if buffer.len() != target {
james7132 marked this conversation as resolved.
Show resolved Hide resolved
buffer.truncate(start);
return None;
}

// Pad to 256 byte alignment
Expand All @@ -221,13 +219,13 @@ impl SkinnedMeshJoints {
pub fn extract_skinned_meshes(
mut commands: Commands,
mut previous_len: Local<usize>,
mut previous_joint_len: Local<usize>,
mut uniform: ResMut<SkinnedMeshUniform>,
query: Extract<Query<(Entity, &ComputedVisibility, &SkinnedMesh)>>,
inverse_bindposes: Extract<Res<Assets<SkinnedMeshInverseBindposes>>>,
joint_query: Extract<Query<&GlobalTransform>>,
) {
uniform.buffer.clear();
let mut values = Vec::with_capacity(*previous_len);
let mut joints = Vec::with_capacity(*previous_joint_len);
let mut last_start = 0;

for (entity, computed_visibility, skin) in &query {
Expand All @@ -236,21 +234,19 @@ pub fn extract_skinned_meshes(
}
// PERF: This can be expensive, can we move this to prepare?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment still relevant/would it be possible to move it to prepare?

Copy link
Member Author

@james7132 james7132 Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did test this and omitting this computation dropped many_foxes by about 80us on my machine (250us->170us). However this does mean we need have to extract the inverse bindposes and move the compute to prepare them in Prepare. This basically eliminates the win we have in Prepare. It's probably that the memory bandwidth is the bottleneck here and not the actual computation: copy/move less and you'll see the bigger gains.

This might have the benefit of moving it out of the hotspot blocking both main and render worlds, but it incurs a heavier cost in render. Not sure if this is worth it.

if let Some(skinned_joints) =
SkinnedMeshJoints::build(skin, &inverse_bindposes, &joint_query, &mut joints)
SkinnedMeshJoints::build(skin, &inverse_bindposes, &joint_query, &mut uniform.buffer)
{
last_start = last_start.max(skinned_joints.index as usize);
values.push((entity, skinned_joints.to_buffer_index()));
}
}

// Pad out the buffer to ensure that there's enough space for bindings
while joints.len() - last_start < MAX_JOINTS {
joints.push(Mat4::ZERO);
while uniform.buffer.len() - last_start < MAX_JOINTS {
uniform.buffer.push(Mat4::ZERO);
}

*previous_len = values.len();
*previous_joint_len = joints.len();
commands.insert_resource(ExtractedJoints { buffer: joints });
commands.insert_or_spawn_batch(values);
}

Expand Down Expand Up @@ -779,20 +775,14 @@ impl Default for SkinnedMeshUniform {
pub fn prepare_skinned_meshes(
render_device: Res<RenderDevice>,
render_queue: Res<RenderQueue>,
extracted_joints: Res<ExtractedJoints>,
mut skinned_mesh_uniform: ResMut<SkinnedMeshUniform>,
) {
if extracted_joints.buffer.is_empty() {
if skinned_mesh_uniform.buffer.is_empty() {
return;
}

skinned_mesh_uniform.buffer.clear();
skinned_mesh_uniform
.buffer
.reserve(extracted_joints.buffer.len(), &render_device);
for joint in &extracted_joints.buffer {
skinned_mesh_uniform.buffer.push(*joint);
}
let len = skinned_mesh_uniform.buffer.len();
skinned_mesh_uniform.buffer.reserve(len, &render_device);
skinned_mesh_uniform
.buffer
.write_buffer(&render_device, &render_queue);
Expand Down
11 changes: 11 additions & 0 deletions crates/bevy_render/src/render_resource/buffer_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,18 @@ impl<T: Pod> BufferVec<T> {
}
}

pub fn truncate(&mut self, len: usize) {
self.values.truncate(len);
}

pub fn clear(&mut self) {
self.values.clear();
}
}

impl<T: Pod> Extend<T> for BufferVec<T> {
#[inline]
fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I) {
self.values.extend(iter);
}
}