Skip to content

Commit

Permalink
Revert "Support calculating normals for indexed meshes" (#12716) and …
Browse files Browse the repository at this point in the history
…add support for calculating smooth normals (#13333)

# Objective

- Refactor the changes merged in #11654 to compute flat normals for
indexed meshes instead of smooth normals.
- Fixes #12716 

## 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.
  • Loading branch information
adithramachandran committed May 16, 2024
1 parent 1efa578 commit 65e62ba
Showing 1 changed file with 122 additions and 46 deletions.
168 changes: 122 additions & 46 deletions crates/bevy_render/src/mesh/mesh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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::<Vec<usize>>()
.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.
Expand Down

0 comments on commit 65e62ba

Please sign in to comment.