From 968f05174b02ea874b68b2f2d657a10c5d7b0082 Mon Sep 17 00:00:00 2001 From: adithramachandran1 Date: Sat, 11 May 2024 20:22:33 -0400 Subject: [PATCH] Revert "Support calculating normals for indexed meshes" (#12716) # Objective - Refactor the changes merged in #11654 to compute flat normals for indexed meshes instead of smooth normals. ## Solution - Partially revert the changes in #11654 to compute flat normals for both indexed and unindexed meshes in `compute_flat_normals` - Create a new method, `compute_smooth_normals`, that computes smooth normals for indexed meshes - Create a new method, `compute_normals`, that computes smooth normals for indexed meshes and flat normals for unindexed meshes by default. Use this new method instead of `compute_flat_normals`. ## Testing - Run the example with and without the changes to ensure that the results are identical. --- crates/bevy_render/src/mesh/mesh/mod.rs | 168 +++++++++++++++++------- 1 file changed, 122 insertions(+), 46 deletions(-) diff --git a/crates/bevy_render/src/mesh/mesh/mod.rs b/crates/bevy_render/src/mesh/mesh/mod.rs index 90e23fbcd8849..a58556d827d97 100644 --- a/crates/bevy_render/src/mesh/mesh/mod.rs +++ b/crates/bevy_render/src/mesh/mesh/mod.rs @@ -544,15 +544,44 @@ impl Mesh { } /// Calculates the [`Mesh::ATTRIBUTE_NORMAL`] of a mesh. + /// If the mesh is indexed, this defaults to smooth normals. Otherwise, it defaults to flat + /// normals. /// /// # Panics /// Panics if [`Mesh::ATTRIBUTE_POSITION`] is not of type `float3`. /// Panics if the mesh has any other topology than [`PrimitiveTopology::TriangleList`]. /// - /// FIXME: The should handle more cases since this is called as a part of gltf + /// FIXME: This should handle more cases since this is called as a part of gltf + /// mesh loading where we can't really blame users for loading meshes that might + /// not conform to the limitations here! + pub fn compute_normals(&mut self) { + assert!( + matches!(self.primitive_topology, PrimitiveTopology::TriangleList), + "`compute_normals` can only work on `TriangleList`s" + ); + if self.indices().is_none() { + self.compute_flat_normals(); + } else { + self.compute_smooth_normals(); + } + } + + /// Calculates the [`Mesh::ATTRIBUTE_NORMAL`] of a mesh. + /// + /// # Panics + /// Panics if [`Indices`] are set or [`Mesh::ATTRIBUTE_POSITION`] is not of type `float3`. + /// Panics if the mesh has any other topology than [`PrimitiveTopology::TriangleList`]. + /// Consider calling [`Mesh::duplicate_vertices`] or exporting your mesh with normal + /// attributes. + /// + /// FIXME: This should handle more cases since this is called as a part of gltf /// mesh loading where we can't really blame users for loading meshes that might /// not conform to the limitations here! pub fn compute_flat_normals(&mut self) { + assert!( + self.indices().is_none(), + "`compute_flat_normals` can't work on indexed geometry. Consider calling either `Mesh::compute_smooth_normals` or `Mesh::duplicate_vertices` followed by `Mesh::compute_flat_normals`." + ); assert!( matches!(self.primitive_topology, PrimitiveTopology::TriangleList), "`compute_flat_normals` can only work on `TriangleList`s" @@ -564,67 +593,114 @@ impl Mesh { .as_float3() .expect("`Mesh::ATTRIBUTE_POSITION` vertex attributes should be of type `float3`"); - match self.indices() { - Some(indices) => { - let mut count: usize = 0; - let mut corners = [0_usize; 3]; - let mut normals = vec![[0.0f32; 3]; positions.len()]; - let mut adjacency_counts = vec![0_usize; positions.len()]; - - for i in indices.iter() { - corners[count % 3] = i; - count += 1; - if count % 3 == 0 { - let normal = face_normal( - positions[corners[0]], - positions[corners[1]], - positions[corners[2]], - ); - for corner in corners { - normals[corner] = - (Vec3::from(normal) + Vec3::from(normals[corner])).into(); - adjacency_counts[corner] += 1; - } - } - } + let normals: Vec<_> = positions + .chunks_exact(3) + .map(|p| face_normal(p[0], p[1], p[2])) + .flat_map(|normal| [normal; 3]) + .collect(); - // average (smooth) normals for shared vertices... - // TODO: support different methods of weighting the average - for i in 0..normals.len() { - let count = adjacency_counts[i]; - if count > 0 { - normals[i] = (Vec3::from(normals[i]) / (count as f32)).normalize().into(); - } - } + self.insert_attribute(Mesh::ATTRIBUTE_NORMAL, normals); + } - self.insert_attribute(Mesh::ATTRIBUTE_NORMAL, normals); - } - None => { - let normals: Vec<_> = positions - .chunks_exact(3) - .map(|p| face_normal(p[0], p[1], p[2])) - .flat_map(|normal| [normal; 3]) - .collect(); - - self.insert_attribute(Mesh::ATTRIBUTE_NORMAL, normals); + /// Calculates the [`Mesh::ATTRIBUTE_NORMAL`] of an indexed mesh, smoothing normals for shared + /// vertices. + /// + /// # Panics + /// Panics if [`Mesh::ATTRIBUTE_POSITION`] is not of type `float3`. + /// Panics if the mesh has any other topology than [`PrimitiveTopology::TriangleList`]. + /// Panics if the mesh does not have indices defined. + /// + /// FIXME: This should handle more cases since this is called as a part of gltf + /// mesh loading where we can't really blame users for loading meshes that might + /// not conform to the limitations here! + pub fn compute_smooth_normals(&mut self) { + assert!( + matches!(self.primitive_topology, PrimitiveTopology::TriangleList), + "`compute_smooth_normals` can only work on `TriangleList`s" + ); + assert!( + self.indices().is_some(), + "`compute_smooth_normals` can only work on indexed meshes" + ); + + let positions = self + .attribute(Mesh::ATTRIBUTE_POSITION) + .unwrap() + .as_float3() + .expect("`Mesh::ATTRIBUTE_POSITION` vertex attributes should be of type `float3`"); + + let mut normals = vec![Vec3::ZERO; positions.len()]; + let mut adjacency_counts = vec![0_usize; positions.len()]; + + self.indices() + .unwrap() + .iter() + .collect::>() + .chunks_exact(3) + .for_each(|face| { + let [a, b, c] = [face[0], face[1], face[2]]; + let normal = Vec3::from(face_normal(positions[a], positions[b], positions[c])); + [a, b, c].iter().for_each(|pos| { + normals[*pos] += normal; + adjacency_counts[*pos] += 1; + }); + }); + + // average (smooth) normals for shared vertices... + // TODO: support different methods of weighting the average + for i in 0..normals.len() { + let count = adjacency_counts[i]; + if count > 0 { + normals[i] = (normals[i] / (count as f32)).normalize(); } } + + self.insert_attribute(Mesh::ATTRIBUTE_NORMAL, normals); + } + + /// Consumes the mesh and returns a mesh with calculated [`Mesh::ATTRIBUTE_NORMAL`]. + /// If the mesh is indexed, this defaults to smooth normals. Otherwise, it defaults to flat + /// normals. + /// + /// (Alternatively, you can use [`Mesh::compute_normals`] to mutate an existing mesh in-place) + /// + /// # Panics + /// Panics if [`Mesh::ATTRIBUTE_POSITION`] is not of type `float3`. + /// Panics if the mesh has any other topology than [`PrimitiveTopology::TriangleList`]. + #[must_use] + pub fn with_computed_normals(mut self) -> Self { + self.compute_normals(); + self } /// Consumes the mesh and returns a mesh with calculated [`Mesh::ATTRIBUTE_NORMAL`]. /// - /// (Alternatively, you can use [`Mesh::with_computed_flat_normals`] to mutate an existing mesh in-place) + /// (Alternatively, you can use [`Mesh::compute_flat_normals`] to mutate an existing mesh in-place) /// /// # Panics - /// Panics if [`Indices`] are set or [`Mesh::ATTRIBUTE_POSITION`] is not of type `float3` or - /// if the mesh has any other topology than [`PrimitiveTopology::TriangleList`]. - /// Consider calling [`Mesh::with_duplicated_vertices`] or export your mesh with normal attributes. + /// Panics if [`Mesh::ATTRIBUTE_POSITION`] is not of type `float3`. + /// Panics if the mesh has any other topology than [`PrimitiveTopology::TriangleList`]. + /// Panics if the mesh has indices defined #[must_use] pub fn with_computed_flat_normals(mut self) -> Self { self.compute_flat_normals(); self } + /// Consumes the mesh and returns a mesh with calculated [`Mesh::ATTRIBUTE_NORMAL`]. + /// + /// (Alternatively, you can use [`Mesh::compute_smooth_normals`] to mutate an existing mesh in-place) + /// + /// # Panics + /// Panics if [`Mesh::ATTRIBUTE_POSITION`] is not of type `float3`. + /// Panics if the mesh has any other topology than [`PrimitiveTopology::TriangleList`]. + /// Panics if the mesh does not have indices defined. + #[must_use] + pub fn with_computed_smooth_normals(mut self) -> Self { + self.compute_smooth_normals(); + self + } + /// Generate tangents for the mesh using the `mikktspace` algorithm. /// /// Sets the [`Mesh::ATTRIBUTE_TANGENT`] attribute if successful.