From 48ac955afd10a5b73380851eb535062154782030 Mon Sep 17 00:00:00 2001
From: devil ira <JustTheCoolDude@gmail.com>
Date: Thu, 31 Mar 2022 20:43:01 +0000
Subject: [PATCH] Fix loading non-TriangleList meshes without normals in gltf
 loader (#4376)

# Objective
Make it so that loading in a mesh without normals that is not a `TriangleList` succeeds.

## Solution
Flat normals can only be calculated on a mesh made of triangles.
Check whether the mesh is a `TriangleList` before trying to compute missing normals.

## Additional changes
The panic condition in `duplicate_vertices` did not make sense to me. I moved it to `compute_flat_normals` where the algorithm would produce incorrect results if the mesh is not a `TriangleList`.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
---
 crates/bevy_gltf/src/loader.rs          |  4 +++-
 crates/bevy_render/src/mesh/mesh/mod.rs | 17 ++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs
index 2f9066069d..8b45290719 100644
--- a/crates/bevy_gltf/src/loader.rs
+++ b/crates/bevy_gltf/src/loader.rs
@@ -268,7 +268,9 @@ async fn load_gltf<'a, 'b>(
                 mesh.set_indices(Some(Indices::U32(indices.into_u32().collect())));
             };
 
-            if mesh.attribute(Mesh::ATTRIBUTE_NORMAL).is_none() {
+            if mesh.attribute(Mesh::ATTRIBUTE_NORMAL).is_none()
+                && matches!(mesh.primitive_topology(), PrimitiveTopology::TriangleList)
+            {
                 let vertex_count_before = mesh.count_vertices();
                 mesh.duplicate_vertices();
                 mesh.compute_flat_normals();
diff --git a/crates/bevy_render/src/mesh/mesh/mod.rs b/crates/bevy_render/src/mesh/mesh/mod.rs
index babf3c2530..73488a60af 100644
--- a/crates/bevy_render/src/mesh/mesh/mod.rs
+++ b/crates/bevy_render/src/mesh/mesh/mod.rs
@@ -252,23 +252,16 @@ impl Mesh {
     ///
     /// This can dramatically increase the vertex count, so make sure this is what you want.
     /// Does nothing if no [Indices] are set.
-    ///
-    /// # Panics
-    /// If the mesh has any other topology than [`PrimitiveTopology::TriangleList`].
     pub fn duplicate_vertices(&mut self) {
         fn duplicate<T: Copy>(values: &[T], indices: impl Iterator<Item = usize>) -> Vec<T> {
             indices.map(|i| values[i]).collect()
         }
 
-        assert!(
-            matches!(self.primitive_topology, PrimitiveTopology::TriangleList),
-            "can only duplicate vertices for `TriangleList`s"
-        );
-
         let indices = match self.indices.take() {
             Some(indices) => indices,
             None => return,
         };
+
         for attributes in self.attributes.values_mut() {
             let indices = indices.iter();
             match &mut attributes.values {
@@ -307,11 +300,17 @@ impl Mesh {
     /// 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 [`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::duplicate_vertices`] or export your mesh with normal attributes.
     pub fn compute_flat_normals(&mut self) {
         assert!(self.indices().is_none(), "`compute_flat_normals` can't work on indexed geometry. Consider calling `Mesh::duplicate_vertices`.");
 
+        assert!(
+            matches!(self.primitive_topology, PrimitiveTopology::TriangleList),
+            "`compute_flat_normals` can only work on `TriangleList`s"
+        );
+
         let positions = self
             .attribute(Mesh::ATTRIBUTE_POSITION)
             .unwrap()