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.
This commit is contained in:
parent
713acc2e6d
commit
f964ee1e3a
@ -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<MeshVertexAttribute>,
|
||||
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<MeshVertexAttribute>,
|
||||
},
|
||||
#[error(
|
||||
"Incompatible primitive topologies: {:?} and {:?}",
|
||||
self_primitive_topology,
|
||||
other_primitive_topology
|
||||
)]
|
||||
IncompatiblePrimitiveTopology {
|
||||
self_primitive_topology: PrimitiveTopology,
|
||||
other_primitive_topology: PrimitiveTopology,
|
||||
},
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
10
release-content/migration-guides/rework_merge_mesh_error.md
Normal file
10
release-content/migration-guides/rework_merge_mesh_error.md
Normal file
@ -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>`
|
Loading…
Reference in New Issue
Block a user