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

Add Mesh::merge #11456

Merged
merged 5 commits into from
Feb 12, 2024
Merged
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
79 changes: 79 additions & 0 deletions crates/bevy_render/src/mesh/mesh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,85 @@ impl Mesh {
Ok(self)
}

/// Merges the [`Mesh`] data of `other` with `self`. The attributes and indices of `other` will be appended to `self`.
///
/// Note that attributes of `other` that don't exist on `self` will be ignored.
///
/// # Panics
///
/// Panics if the vertex attribute values of `other` are incompatible with `self`.
/// For example, [`VertexAttributeValues::Float32`] is incompatible with [`VertexAttributeValues::Float32x3`].
#[allow(clippy::match_same_arms)]
pub fn merge(&mut self, other: Mesh) {
use VertexAttributeValues::*;

// The indices of `other` should start after the last vertex of `self`.
let index_offset = self
.attribute(Mesh::ATTRIBUTE_POSITION)
.get_or_insert(&VertexAttributeValues::Float32x3(Vec::default()))
.len();

// Extend attributes of `self` with attributes of `other`.
for (id, values) in self.attributes_mut() {
let enum_variant_name = values.enum_variant_name();
if let Some(other_values) = other.attribute(id) {
match (values, other_values) {
Comment on lines +613 to +615
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I can see, if self and other's attributes don't match up:

  • if self has an attribute but other doesn't, it is kept (note that this means future's self.count_vertices() will report the old vertices count);
  • if other has an attribute but self doesn't, it is silently ignored.

Which could be kinda surprising. I don't see much better ways to handle this (other than error), in both cases it should be documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, I just added a short mention in the doc comment that attributes of other that don't exist on self will be ignored

(Float32(vec1), Float32(vec2)) => vec1.extend(vec2),
(Sint32(vec1), Sint32(vec2)) => vec1.extend(vec2),
(Uint32(vec1), Uint32(vec2)) => vec1.extend(vec2),
(Float32x2(vec1), Float32x2(vec2)) => vec1.extend(vec2),
(Sint32x2(vec1), Sint32x2(vec2)) => vec1.extend(vec2),
(Uint32x2(vec1), Uint32x2(vec2)) => vec1.extend(vec2),
(Float32x3(vec1), Float32x3(vec2)) => vec1.extend(vec2),
(Sint32x3(vec1), Sint32x3(vec2)) => vec1.extend(vec2),
(Uint32x3(vec1), Uint32x3(vec2)) => vec1.extend(vec2),
(Sint32x4(vec1), Sint32x4(vec2)) => vec1.extend(vec2),
(Uint32x4(vec1), Uint32x4(vec2)) => vec1.extend(vec2),
(Float32x4(vec1), Float32x4(vec2)) => vec1.extend(vec2),
(Sint16x2(vec1), Sint16x2(vec2)) => vec1.extend(vec2),
(Snorm16x2(vec1), Snorm16x2(vec2)) => vec1.extend(vec2),
(Uint16x2(vec1), Uint16x2(vec2)) => vec1.extend(vec2),
(Unorm16x2(vec1), Unorm16x2(vec2)) => vec1.extend(vec2),
(Sint16x4(vec1), Sint16x4(vec2)) => vec1.extend(vec2),
(Snorm16x4(vec1), Snorm16x4(vec2)) => vec1.extend(vec2),
(Uint16x4(vec1), Uint16x4(vec2)) => vec1.extend(vec2),
(Unorm16x4(vec1), Unorm16x4(vec2)) => vec1.extend(vec2),
(Sint8x2(vec1), Sint8x2(vec2)) => vec1.extend(vec2),
(Snorm8x2(vec1), Snorm8x2(vec2)) => vec1.extend(vec2),
(Uint8x2(vec1), Uint8x2(vec2)) => vec1.extend(vec2),
(Unorm8x2(vec1), Unorm8x2(vec2)) => vec1.extend(vec2),
(Sint8x4(vec1), Sint8x4(vec2)) => vec1.extend(vec2),
(Snorm8x4(vec1), Snorm8x4(vec2)) => vec1.extend(vec2),
(Uint8x4(vec1), Uint8x4(vec2)) => vec1.extend(vec2),
(Unorm8x4(vec1), Unorm8x4(vec2)) => vec1.extend(vec2),
_ => panic!(
"Incompatible vertex attribute types {} and {}",
enum_variant_name,
other_values.enum_variant_name()
),
Comment on lines +644 to +648
Copy link
Contributor Author

@Jondolf Jondolf Jan 21, 2024

Choose a reason for hiding this comment

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

We might want to return some error instead? I'm not sure how common it would be to reach this panic since I'd expect e.g. position attributes to almost always be Float32x3

}
}
}

// Extend indices of `self` with indices of `other`.
if let (Some(indices), Some(other_indices)) = (self.indices_mut(), other.indices()) {
match (indices, other_indices) {
(Indices::U16(i1), Indices::U16(i2)) => {
i1.extend(i2.iter().map(|i| *i + index_offset as u16));
}
(Indices::U32(i1), Indices::U32(i2)) => {
i1.extend(i2.iter().map(|i| *i + index_offset as u32));
}
(Indices::U16(i1), Indices::U32(i2)) => {
i1.extend(i2.iter().map(|i| *i as u16 + index_offset as u16));
}
(Indices::U32(i1), Indices::U16(i2)) => {
i1.extend(i2.iter().map(|i| *i as u32 + index_offset as u32));
}
}
}
}

/// Transforms the vertex positions, normals, and tangents of the mesh by the given [`Transform`].
pub fn transformed_by(mut self, transform: Transform) -> Self {
self.transform_by(transform);
Expand Down