From c7898b0c36d2b70f31bea0dafc92d88852370d76 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 3 Apr 2021 16:52:22 +0200 Subject: [PATCH 1/6] calculate flat normals for mesh if missing --- crates/bevy_gltf/Cargo.toml | 1 + crates/bevy_gltf/src/loader.rs | 6 ++ crates/bevy_render/src/mesh/mesh.rs | 96 +++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+) diff --git a/crates/bevy_gltf/Cargo.toml b/crates/bevy_gltf/Cargo.toml index 5e8d1ad72b480..846d86a2c7b50 100644 --- a/crates/bevy_gltf/Cargo.toml +++ b/crates/bevy_gltf/Cargo.toml @@ -24,6 +24,7 @@ bevy_render = { path = "../bevy_render", version = "0.4.0" } bevy_transform = { path = "../bevy_transform", version = "0.4.0" } bevy_math = { path = "../bevy_math", version = "0.4.0" } bevy_scene = { path = "../bevy_scene", version = "0.4.0" } +bevy_log = { path = "../bevy_log", version = "0.4.0" } # other gltf = { version = "0.15.2", default-features = false, features = ["utils", "names", "KHR_materials_unlit"] } diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 25da841eb3664..3b1566509aaa9 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -146,6 +146,12 @@ async fn load_gltf<'a, 'b>( mesh.set_indices(Some(Indices::U32(indices.into_u32().collect()))); }; + if mesh.attribute(Mesh::ATTRIBUTE_NORMAL).is_none() { + bevy_log::info!("missing normals, computing them as flat"); + mesh.duplicate_vertices(); + mesh.compute_flat_normals(); + } + let mesh = load_context.set_labeled_asset(&primitive_label, LoadedAsset::new(mesh)); primitives.push(super::GltfPrimitive { mesh, diff --git a/crates/bevy_render/src/mesh/mesh.rs b/crates/bevy_render/src/mesh/mesh.rs index aece08891876f..6c2c5454ac0fc 100644 --- a/crates/bevy_render/src/mesh/mesh.rs +++ b/crates/bevy_render/src/mesh/mesh.rs @@ -65,6 +65,13 @@ impl VertexAttributeValues { self.len() == 0 } + fn as_float3(&self) -> Option<&[[f32; 3]]> { + match self { + VertexAttributeValues::Float3(values) => Some(values), + _ => None, + } + } + // TODO: add vertex format as parameter here and perform type conversions /// Flattens the VertexAttributeArray into a sequence of bytes. This is /// useful for serialization and sending to the GPU. @@ -194,6 +201,29 @@ pub enum Indices { U32(Vec), } +impl Indices { + fn iter(&self) -> impl Iterator + '_ { + match self { + Indices::U16(vec) => IndicesIter::U16(vec.iter()), + Indices::U32(vec) => IndicesIter::U32(vec.iter()), + } + } +} +enum IndicesIter<'a> { + U16(std::slice::Iter<'a, u16>), + U32(std::slice::Iter<'a, u32>), +} +impl Iterator for IndicesIter<'_> { + type Item = usize; + + fn next(&mut self) -> Option { + match self { + IndicesIter::U16(iter) => iter.next().map(|val| *val as usize), + IndicesIter::U32(iter) => iter.next().map(|val| *val as usize), + } + } +} + impl From<&Indices> for IndexFormat { fn from(indices: &Indices) -> Self { match indices { @@ -369,6 +399,72 @@ impl Mesh { attributes_interleaved_buffer } + + /// Duplicates the vertex attributes so that no vertices are shared. + /// + /// Does nothing if no [Indices] are set. + pub fn duplicate_vertices(&mut self) { + fn duplicate(values: &[T], indices: impl Iterator) -> Vec { + 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.iter_mut() { + let indices = indices.iter(); + match attributes { + VertexAttributeValues::Float(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Int(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Uint(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Float2(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Int2(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Uint2(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Float3(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Int3(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Uint3(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Int4(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Uint4(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Float4(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Uchar4Norm(vec) => *vec = duplicate(&vec, indices), + } + } + } + + /// Calculates the [`Mesh::ATTRIBUTE_NORMAL`] of a mesh. + /// This [duplicates the vertices](Mesh::duplicate_vertices), so any [`Indices`] will be gone if set. + pub fn compute_flat_normals(&mut self) { + if self.indices().is_some() { + self.duplicate_vertices(); + self.compute_flat_normals(); + return; + } + + let positions = self + .attribute(Mesh::ATTRIBUTE_POSITION) + .unwrap() + .as_float3() + .expect("`Mesh::ATTRIBUTE_POSITION` vertex attributes should be of type `float3`"); + + let normals: Vec<_> = positions + .chunks_exact(3) + .map(|p| face_normal(p[0], p[1], p[2])) + .flat_map(|normal| std::array::IntoIter::new([normal, normal, normal])) + .collect(); + + self.set_attribute(Mesh::ATTRIBUTE_NORMAL, normals); + } +} + +fn face_normal(a: [f32; 3], b: [f32; 3], c: [f32; 3]) -> [f32; 3] { + let (a, b, c) = (Vec3::from(a), Vec3::from(b), Vec3::from(c)); + (b - a).cross(c - a).into() } fn remove_resource_save( From 5ca2020a83ccc21959f69a8b1fc315b1b8fbf42f Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sat, 3 Apr 2021 22:04:45 +0200 Subject: [PATCH 2/6] more detailed warning when computing normals also panic in compute_flat_normals if indices are set. That way there won't be no unexpected vertex buffer size increases. --- crates/bevy_gltf/src/loader.rs | 7 ++++++- crates/bevy_render/src/mesh/mesh.rs | 9 +++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 3b1566509aaa9..930ae155af379 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -147,9 +147,14 @@ async fn load_gltf<'a, 'b>( }; if mesh.attribute(Mesh::ATTRIBUTE_NORMAL).is_none() { - bevy_log::info!("missing normals, computing them as flat"); + let vertex_count_before = mesh.count_vertices(); mesh.duplicate_vertices(); mesh.compute_flat_normals(); + let vertex_count_after = mesh.count_vertices(); + + if vertex_count_before != vertex_count_after { + bevy_log::warn!("Missing vertex normals in indexed geometry, computing them as flat. Vertex count increased from {} to {}", vertex_count_before, vertex_count_after); + } } let mesh = load_context.set_labeled_asset(&primitive_label, LoadedAsset::new(mesh)); diff --git a/crates/bevy_render/src/mesh/mesh.rs b/crates/bevy_render/src/mesh/mesh.rs index 6c2c5454ac0fc..efa81b4d94131 100644 --- a/crates/bevy_render/src/mesh/mesh.rs +++ b/crates/bevy_render/src/mesh/mesh.rs @@ -402,6 +402,7 @@ impl Mesh { /// Duplicates the vertex attributes so that no vertices are shared. /// + /// This can dramatically increase the vertex count, so make sure this is what you want. /// Does nothing if no [Indices] are set. pub fn duplicate_vertices(&mut self) { fn duplicate(values: &[T], indices: impl Iterator) -> Vec { @@ -438,12 +439,12 @@ impl Mesh { } /// Calculates the [`Mesh::ATTRIBUTE_NORMAL`] of a mesh. - /// This [duplicates the vertices](Mesh::duplicate_vertices), so any [`Indices`] will be gone if set. + /// + /// Panics if [`Indices`] are set. + /// Consider calling [Mesh::duplicate_vertices] or export your mesh with normal attributes. pub fn compute_flat_normals(&mut self) { if self.indices().is_some() { - self.duplicate_vertices(); - self.compute_flat_normals(); - return; + panic!("`compute_flat_normals` can't work on indexed geometry. Consider calling `Mesh::duplicate_vertices`."); } let positions = self From 9534a2de46b614f44a066df16f2b29c98b2a3013 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 6 Apr 2021 20:17:43 +0200 Subject: [PATCH 3/6] normalize normals (duh) --- crates/bevy_render/src/mesh/mesh.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_render/src/mesh/mesh.rs b/crates/bevy_render/src/mesh/mesh.rs index efa81b4d94131..77ef66ee78aa5 100644 --- a/crates/bevy_render/src/mesh/mesh.rs +++ b/crates/bevy_render/src/mesh/mesh.rs @@ -465,7 +465,7 @@ impl Mesh { fn face_normal(a: [f32; 3], b: [f32; 3], c: [f32; 3]) -> [f32; 3] { let (a, b, c) = (Vec3::from(a), Vec3::from(b), Vec3::from(c)); - (b - a).cross(c - a).into() + (b - a).cross(c - a).normalize().into() } fn remove_resource_save( From d8f57cf647c3e5b0f590d533dfb1606244bb6522 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 15 Apr 2021 21:08:41 +0200 Subject: [PATCH 4/6] add remaining VertexAttributesValues --- crates/bevy_render/src/mesh/mesh.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/bevy_render/src/mesh/mesh.rs b/crates/bevy_render/src/mesh/mesh.rs index 3eb628aa71a0f..8ce04a7ba9a60 100644 --- a/crates/bevy_render/src/mesh/mesh.rs +++ b/crates/bevy_render/src/mesh/mesh.rs @@ -495,6 +495,21 @@ impl Mesh { VertexAttributeValues::Int4(vec) => *vec = duplicate(&vec, indices), VertexAttributeValues::Uint4(vec) => *vec = duplicate(&vec, indices), VertexAttributeValues::Float4(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Short2(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Short2Norm(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Ushort2(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Ushort2Norm(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Short4(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Short4Norm(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Ushort4(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Ushort4Norm(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Char2(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Char2Norm(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Uchar2(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Uchar2Norm(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Char4(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Char4Norm(vec) => *vec = duplicate(&vec, indices), + VertexAttributeValues::Uchar4(vec) => *vec = duplicate(&vec, indices), VertexAttributeValues::Uchar4Norm(vec) => *vec = duplicate(&vec, indices), } } From dfc9907a5814c7a8f1634ec29c56998575288649 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 15 Apr 2021 21:09:37 +0200 Subject: [PATCH 5/6] debug instead of warn, log always --- crates/bevy_gltf/src/loader.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 443c3db9fec68..7f8aae10414ba 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -160,7 +160,12 @@ async fn load_gltf<'a, 'b>( let vertex_count_after = mesh.count_vertices(); if vertex_count_before != vertex_count_after { - bevy_log::warn!("Missing vertex normals in indexed geometry, computing them as flat. Vertex count increased from {} to {}", vertex_count_before, vertex_count_after); + bevy_log::debug!("Missing vertex normals in indexed geometry, computing them as flat. Vertex count increased from {} to {}", vertex_count_before, vertex_count_after); + } + if vertex_count_before != vertex_count_after { + bevy_log::debug!( + "Missing vertex normals in indexed geometry, computing them as flat." + ); } } From ed0da67bd78679c8cae7a9856c9c9e5abab325c9 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Thu, 15 Apr 2021 22:40:00 +0200 Subject: [PATCH 6/6] fix duplicate if --- crates/bevy_gltf/src/loader.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 7f8aae10414ba..3f4947e63772f 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -161,8 +161,7 @@ async fn load_gltf<'a, 'b>( if vertex_count_before != vertex_count_after { bevy_log::debug!("Missing vertex normals in indexed geometry, computing them as flat. Vertex count increased from {} to {}", vertex_count_before, vertex_count_after); - } - if vertex_count_before != vertex_count_after { + } else { bevy_log::debug!( "Missing vertex normals in indexed geometry, computing them as flat." );