From f964ee1e3ae2fc28ab2e7586b581271fb973e913 Mon Sep 17 00:00:00 2001 From: Pnoenix <65498290+Pnoenix@users.noreply.github.com> Date: Wed, 16 Jul 2025 22:37:58 +0200 Subject: [PATCH] Expand MergeMeshError to include IncompatiblePrimitiveTopology variant (#18561) # Objective Fix #18546 by adding a variant to `MergeMeshError`, for incompatible primitive topologies. ## Solution Made `MergeMeshError` into an enum with two variants; `IncompatibleVertexAttributes` and `IncompatiblePrimitiveTopology`. Added an if statement in `Mesh::merge` to check if the `primitive_topology` field of `self` matches `other`. Also renamed `MergeMeshError` to `MeshMergeError` to align with the two other `MeshSomethingError`'s. ## Testing Didn't do any. --- crates/bevy_mesh/src/mesh.rs | 42 +++++++++++++++---- .../rework_merge_mesh_error.md | 10 +++++ 2 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 release-content/migration-guides/rework_merge_mesh_error.md diff --git a/crates/bevy_mesh/src/mesh.rs b/crates/bevy_mesh/src/mesh.rs index 3492788c4b..eabc7621ca 100644 --- a/crates/bevy_mesh/src/mesh.rs +++ b/crates/bevy_mesh/src/mesh.rs @@ -826,11 +826,26 @@ impl Mesh { /// /// # Errors /// - /// 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) -> Result<(), MergeMeshError> { + /// If any of the following conditions are not met, this function errors: + /// * All of the vertex attributes that have the same attribute id, must also + /// have the same attribute type. + /// For example two attributes with the same id, but where one is a + /// [`VertexAttributeValues::Float32`] and the other is a + /// [`VertexAttributeValues::Float32x3`], would be invalid. + /// * Both meshes must have the same primitive topology. + pub fn merge(&mut self, other: &Mesh) -> Result<(), MeshMergeError> { use VertexAttributeValues::*; + // Check if the meshes `primitive_topology` field is the same, + // as if that is not the case, the resulting mesh could (and most likely would) + // be invalid. + if self.primitive_topology != other.primitive_topology { + return Err(MeshMergeError::IncompatiblePrimitiveTopology { + self_primitive_topology: self.primitive_topology, + other_primitive_topology: other.primitive_topology, + }); + } + // The indices of `other` should start after the last vertex of `self`. let index_offset = self.count_vertices(); @@ -871,7 +886,7 @@ impl Mesh { (Uint8x4(vec1), Uint8x4(vec2)) => vec1.extend(vec2), (Unorm8x4(vec1), Unorm8x4(vec2)) => vec1.extend(vec2), _ => { - return Err(MergeMeshError { + return Err(MeshMergeError::IncompatibleVertexAttributes { self_attribute: *attribute, other_attribute: other .attribute_data(attribute.id) @@ -1391,10 +1406,21 @@ impl MeshDeserializer { /// 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, +pub enum MeshMergeError { + #[error("Incompatible vertex attribute types: {} and {}", self_attribute.name, other_attribute.map(|a| a.name).unwrap_or("None"))] + IncompatibleVertexAttributes { + self_attribute: MeshVertexAttribute, + other_attribute: Option, + }, + #[error( + "Incompatible primitive topologies: {:?} and {:?}", + self_primitive_topology, + other_primitive_topology + )] + IncompatiblePrimitiveTopology { + self_primitive_topology: PrimitiveTopology, + other_primitive_topology: PrimitiveTopology, + }, } #[cfg(test)] diff --git a/release-content/migration-guides/rework_merge_mesh_error.md b/release-content/migration-guides/rework_merge_mesh_error.md new file mode 100644 index 0000000000..ab611710fb --- /dev/null +++ b/release-content/migration-guides/rework_merge_mesh_error.md @@ -0,0 +1,10 @@ +--- +title: Rework `MergeMeshError` +pull_requests: [18561] +--- + +`MergeMeshError` was reworked to account for the possibility of the meshes being merged having two different `PrimitiveTopology`'s, and was renamed to `MeshMergeError` to align with the naming of other mesh errors. + +- Users will need to rename `MergeMeshError` to `MeshMergeError` +- When handling `MergeMeshError` (now `MeshMergeError`), users will need to account for the new `IncompatiblePrimitiveTopology` variant, as it has been changed from a struct to an enum +- `Mesh::merge` now returns `Result<(), MeshMergeError>` instead of the previous `Result<(), MergeMeshError>`