Skip to content

Commit

Permalink
Validate vertex attribute format on insert (bevyengine#5259)
Browse files Browse the repository at this point in the history
# Objective

- Validate the format of the values with the expected attribute format.
- Currently, if you pass the wrong format, it will crash somewhere unrelated with a very cryptic error message, so it's really hard to debug for beginners.

## Solution

- Compare the format and panic when unexpected format is passed

## Note

- I used a separate `error!()` for a human friendly message because the panic message is very noisy and hard to parse for beginners. I don't mind changing this to only a panic if people prefer that.
- This could potentially be something that runs only in debug mode, but I don't think inserting attributes is done often enough for this to be an issue.


Co-authored-by: Charles <IceSentry@users.noreply.github.com>
  • Loading branch information
2 people authored and james7132 committed Oct 28, 2022
1 parent ce20f48 commit b0cce41
Showing 1 changed file with 30 additions and 8 deletions.
38 changes: 30 additions & 8 deletions crates/bevy_render/src/mesh/mesh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use bevy_derive::EnumVariantMeta;
use bevy_ecs::system::{lifetimeless::SRes, SystemParamItem};
use bevy_math::*;
use bevy_reflect::TypeUuid;
use bevy_utils::Hashed;
use bevy_utils::{tracing::error, Hashed};
use std::{collections::BTreeMap, hash::Hash, iter::FusedIterator};
use thiserror::Error;
use wgpu::{
Expand Down Expand Up @@ -101,19 +101,28 @@ impl Mesh {

/// Sets the data for a vertex attribute (position, normal etc.). The name will
/// often be one of the associated constants such as [`Mesh::ATTRIBUTE_POSITION`].
///
/// # Panics
/// Panics when the format of the values does not match the attribute's format.
#[inline]
pub fn insert_attribute(
&mut self,
attribute: MeshVertexAttribute,
values: impl Into<VertexAttributeValues>,
) {
self.attributes.insert(
attribute.id,
MeshAttributeData {
attribute,
values: values.into(),
},
);
let values: VertexAttributeValues = values.into();

let values_format = VertexFormat::from(&values);
if values_format != attribute.format {
error!(
"Invalid attribute format for {}. Given format is {:?} but expected {:?}",
attribute.name, values_format, attribute.format
);
panic!("Failed to insert attribute");
}

self.attributes
.insert(attribute.id, MeshAttributeData { attribute, values });
}

/// Removes the data for a vertex attribute
Expand Down Expand Up @@ -994,3 +1003,16 @@ fn generate_tangents_for_mesh(mesh: &Mesh) -> Result<Vec<[f32; 4]>, GenerateTang

Ok(mikktspace_mesh.tangents)
}

#[cfg(test)]
mod tests {
use super::Mesh;
use wgpu::PrimitiveTopology;

#[test]
#[should_panic]
fn panic_invalid_format() {
let mut mesh = Mesh::new(PrimitiveTopology::TriangleList);
mesh.insert_attribute(Mesh::ATTRIBUTE_UV_0, vec![[0.0, 0.0, 0.0]]);
}
}

0 comments on commit b0cce41

Please sign in to comment.