Skip to content

Commit

Permalink
Mesh::merge to return a Result (#17475)
Browse files Browse the repository at this point in the history
# Objective

Make `Mesh::merge` more resilient to use.

Currently, it's difficult to make sure `Mesh::merge` will not panic
(we'd have to check if all attributes are compatible).

- I'd appreciate it for utility function to convert different mesh
representations such as:
dimforge/bevy_rapier#628.

## Solution

- Make `Mesh::merge` return a `Result`.

## Testing

- It builds

## Migration Guide

- `Mesh::merge` now returns a `Result<(), MeshMergeError>`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Greeble <166992735+greeble-dev@users.noreply.github.com>
Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
  • Loading branch information
4 people authored Jan 23, 2025
1 parent 9387fcf commit fd2afee
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 13 deletions.
41 changes: 31 additions & 10 deletions crates/bevy_mesh/src/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use bevy_image::Image;
use bevy_math::{primitives::Triangle3d, *};
use bevy_reflect::Reflect;
use bytemuck::cast_slice;
use thiserror::Error;
use tracing::warn;
use wgpu_types::{VertexAttribute, VertexFormat, VertexStepMode};

Expand Down Expand Up @@ -280,7 +281,7 @@ impl Mesh {
self.attributes.contains_key(&id.into())
}

/// Retrieves the data currently set to the vertex attribute with the specified `name`.
/// Retrieves the data currently set to the vertex attribute with the specified [`MeshVertexAttributeId`].
#[inline]
pub fn attribute(
&self,
Expand All @@ -289,6 +290,15 @@ impl Mesh {
self.attributes.get(&id.into()).map(|data| &data.values)
}

/// Retrieves the full data currently set to the vertex attribute with the specified [`MeshVertexAttributeId`].
#[inline]
pub(crate) fn attribute_data(
&self,
id: impl Into<MeshVertexAttributeId>,
) -> Option<&MeshAttributeData> {
self.attributes.get(&id.into())
}

/// Retrieves the data currently set to the vertex attribute with the specified `name` mutably.
#[inline]
pub fn attribute_mut(
Expand Down Expand Up @@ -788,19 +798,18 @@ impl Mesh {
///
/// `Aabb` of entities with modified mesh are not updated automatically.
///
/// # Panics
/// # Errors
///
/// Panics if the vertex attribute values of `other` are incompatible with `self`.
/// Returns [`Err(MergeMeshError)`](MergeMeshError) if the vertex attribute values of `other` are incompatible with `self`.
/// For example, [`VertexAttributeValues::Float32`] is incompatible with [`VertexAttributeValues::Float32x3`].
pub fn merge(&mut self, other: &Mesh) {
pub fn merge(&mut self, other: &Mesh) -> Result<(), MergeMeshError> {
use VertexAttributeValues::*;

// The indices of `other` should start after the last vertex of `self`.
let index_offset = self.count_vertices();

// Extend attributes of `self` with attributes of `other`.
for (attribute, values) in self.attributes_mut() {
let enum_variant_name = values.enum_variant_name();
if let Some(other_values) = other.attribute(attribute.id) {
#[expect(
clippy::match_same_arms,
Expand Down Expand Up @@ -835,11 +844,14 @@ impl Mesh {
(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()
),
_ => {
return Err(MergeMeshError {
self_attribute: *attribute,
other_attribute: other
.attribute_data(attribute.id)
.map(|data| data.attribute),
})
}
}
}
}
Expand All @@ -848,6 +860,7 @@ impl Mesh {
if let (Some(indices), Some(other_indices)) = (self.indices_mut(), other.indices()) {
indices.extend(other_indices.iter().map(|i| (i + index_offset) as u32));
}
Ok(())
}

/// Transforms the vertex positions, normals, and tangents of the mesh by the given [`Transform`].
Expand Down Expand Up @@ -1213,6 +1226,14 @@ impl core::ops::Mul<Mesh> for Transform {
}
}

/// Error that can occur when calling [`Mesh::merge`].
#[derive(Error, Debug, Clone)]
#[error("Incompatible vertex attribute types {} and {}", self_attribute.name, other_attribute.map(|a| a.name).unwrap_or("None"))]
pub struct MergeMeshError {
pub self_attribute: MeshVertexAttribute,
pub other_attribute: Option<MeshVertexAttribute>,
}

#[cfg(test)]
mod tests {
use super::Mesh;
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_mesh/src/primitives/extrusion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ where

// An extrusion of depth 0 does not need a mantel
if self.half_depth == 0. {
front_face.merge(&back_face);
front_face.merge(&back_face).unwrap();
return front_face;
}

Expand Down Expand Up @@ -408,8 +408,8 @@ where
.with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, uvs)
};

front_face.merge(&back_face);
front_face.merge(&mantel);
front_face.merge(&back_face).unwrap();
front_face.merge(&mantel).unwrap();
front_face
}
}
Expand Down

0 comments on commit fd2afee

Please sign in to comment.