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>
This commit is contained in:
		
							parent
							
								
									aca7fc1854
								
							
						
					
					
						commit
						48ac955afd
					
				@ -268,7 +268,9 @@ async fn load_gltf<'a, 'b>(
 | 
				
			|||||||
                mesh.set_indices(Some(Indices::U32(indices.into_u32().collect())));
 | 
					                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();
 | 
					                let vertex_count_before = mesh.count_vertices();
 | 
				
			||||||
                mesh.duplicate_vertices();
 | 
					                mesh.duplicate_vertices();
 | 
				
			||||||
                mesh.compute_flat_normals();
 | 
					                mesh.compute_flat_normals();
 | 
				
			||||||
 | 
				
			|||||||
@ -252,23 +252,16 @@ impl Mesh {
 | 
				
			|||||||
    ///
 | 
					    ///
 | 
				
			||||||
    /// This can dramatically increase the vertex count, so make sure this is what you want.
 | 
					    /// This can dramatically increase the vertex count, so make sure this is what you want.
 | 
				
			||||||
    /// Does nothing if no [Indices] are set.
 | 
					    /// Does nothing if no [Indices] are set.
 | 
				
			||||||
    ///
 | 
					 | 
				
			||||||
    /// # Panics
 | 
					 | 
				
			||||||
    /// If the mesh has any other topology than [`PrimitiveTopology::TriangleList`].
 | 
					 | 
				
			||||||
    pub fn duplicate_vertices(&mut self) {
 | 
					    pub fn duplicate_vertices(&mut self) {
 | 
				
			||||||
        fn duplicate<T: Copy>(values: &[T], indices: impl Iterator<Item = usize>) -> Vec<T> {
 | 
					        fn duplicate<T: Copy>(values: &[T], indices: impl Iterator<Item = usize>) -> Vec<T> {
 | 
				
			||||||
            indices.map(|i| values[i]).collect()
 | 
					            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() {
 | 
					        let indices = match self.indices.take() {
 | 
				
			||||||
            Some(indices) => indices,
 | 
					            Some(indices) => indices,
 | 
				
			||||||
            None => return,
 | 
					            None => return,
 | 
				
			||||||
        };
 | 
					        };
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        for attributes in self.attributes.values_mut() {
 | 
					        for attributes in self.attributes.values_mut() {
 | 
				
			||||||
            let indices = indices.iter();
 | 
					            let indices = indices.iter();
 | 
				
			||||||
            match &mut attributes.values {
 | 
					            match &mut attributes.values {
 | 
				
			||||||
@ -307,11 +300,17 @@ impl Mesh {
 | 
				
			|||||||
    /// Calculates the [`Mesh::ATTRIBUTE_NORMAL`] of a mesh.
 | 
					    /// Calculates the [`Mesh::ATTRIBUTE_NORMAL`] of a mesh.
 | 
				
			||||||
    ///
 | 
					    ///
 | 
				
			||||||
    /// # Panics
 | 
					    /// # 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.
 | 
					    /// Consider calling [`Mesh::duplicate_vertices`] or export your mesh with normal attributes.
 | 
				
			||||||
    pub fn compute_flat_normals(&mut self) {
 | 
					    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!(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
 | 
					        let positions = self
 | 
				
			||||||
            .attribute(Mesh::ATTRIBUTE_POSITION)
 | 
					            .attribute(Mesh::ATTRIBUTE_POSITION)
 | 
				
			||||||
            .unwrap()
 | 
					            .unwrap()
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
		Reference in New Issue
	
	Block a user