Skip to content

Commit

Permalink
Fix Mesh allocator bug and reduce Mesh data copies by two (#15566)
Browse files Browse the repository at this point in the history
# Objective

- First step towards #15558

## Solution

- Rename `get_vertex_buffer_data` to `create_packed_vertex_buffer_data`
to make it clear that it is not "free" and actually allocates
- Compute length analytically for preallocation instead of creating the
buffer to get its length and immediately discard it
- Use existing vertex attribute size calculation method to reduce code
duplication
- Fix a bug where mesh index data was being replaced by unnecessarily
newly created mesh vertex data in some cases
- Overall reduces mesh copies by two. We still have plenty to go, but
these were the easy ones.

## Testing

- I ran 3d_scene, lighting, and many_cubes, they look fine.
- Benchmarks would be nice, but this is very obviously a win in perf and
correctness.

---

## Migration Guide

- `Mesh::create_packed_vertex_buffer_data` has been renamed
`Mesh::create_packed_vertex_buffer_data` to reflect the fact that it
copies data and allocates.

## Showcase

- look mom, less copies
  • Loading branch information
atlv24 authored Oct 1, 2024
1 parent 1df8238 commit 6465e3b
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 14 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/meshlet/from_mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl MeshletMesh {
let indices = validate_input_mesh(mesh)?;

// Split the mesh into an initial list of meshlets (LOD 0)
let vertex_buffer = mesh.get_vertex_buffer_data();
let vertex_buffer = mesh.create_packed_vertex_buffer_data();
let vertex_stride = mesh.get_vertex_size() as usize;
let vertices = VertexDataAdapter::new(&vertex_buffer, vertex_stride, 0).unwrap();
let mut meshlets = compute_meshlets(&indices, &vertices);
Expand Down
9 changes: 3 additions & 6 deletions crates/bevy_render/src/mesh/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl MeshAllocator {
if self.general_vertex_slabs_supported {
self.allocate(
mesh_id,
mesh.get_vertex_buffer_data().len() as u64,
mesh.get_vertex_size() * mesh.count_vertices() as u64,
vertex_element_layout,
&mut slabs_to_grow,
mesh_allocator_settings,
Expand Down Expand Up @@ -474,12 +474,11 @@ impl MeshAllocator {
let Some(&slab_id) = self.mesh_id_to_vertex_slab.get(mesh_id) else {
return;
};
let vertex_data = mesh.get_vertex_buffer_data();
let vertex_data = mesh.create_packed_vertex_buffer_data();

// Call the generic function.
self.copy_element_data(
mesh_id,
mesh,
&vertex_data,
BufferUsages::VERTEX,
slab_id,
Expand Down Expand Up @@ -507,7 +506,6 @@ impl MeshAllocator {
// Call the generic function.
self.copy_element_data(
mesh_id,
mesh,
index_data,
BufferUsages::INDEX,
slab_id,
Expand All @@ -521,7 +519,6 @@ impl MeshAllocator {
fn copy_element_data(
&mut self,
mesh_id: &AssetId<Mesh>,
mesh: &Mesh,
data: &[u8],
buffer_usages: BufferUsages,
slab_id: SlabId,
Expand Down Expand Up @@ -567,7 +564,7 @@ impl MeshAllocator {
slab_id,
buffer_usages_to_str(buffer_usages)
)),
contents: &mesh.get_vertex_buffer_data(),
contents: data,
usage: buffer_usages | BufferUsages::COPY_DST,
},
));
Expand Down
9 changes: 2 additions & 7 deletions crates/bevy_render/src/mesh/mesh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,13 +458,8 @@ impl Mesh {
///
/// If the vertex attributes have different lengths, they are all truncated to
/// the length of the smallest.
pub fn get_vertex_buffer_data(&self) -> Vec<u8> {
let mut vertex_size = 0;
for attribute_data in self.attributes.values() {
let vertex_format = attribute_data.attribute.format;
vertex_size += vertex_format.get_size() as usize;
}

pub fn create_packed_vertex_buffer_data(&self) -> Vec<u8> {
let vertex_size = self.get_vertex_size() as usize;
let vertex_count = self.count_vertices();
let mut attributes_interleaved_buffer = vec![0; vertex_count * vertex_size];
// bundle into interleaved buffers
Expand Down

0 comments on commit 6465e3b

Please sign in to comment.