Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - calculate flat normals for mesh if missing #1808

Closed
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/bevy_gltf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ bevy_render = { path = "../bevy_render", version = "0.5.0" }
bevy_transform = { path = "../bevy_transform", version = "0.5.0" }
bevy_math = { path = "../bevy_math", version = "0.5.0" }
bevy_scene = { path = "../bevy_scene", version = "0.5.0" }
bevy_log = { path = "../bevy_log", version = "0.5.0" }

# other
gltf = { version = "0.15.2", default-features = false, features = ["utils", "names", "KHR_materials_unlit"] }
Expand Down
16 changes: 16 additions & 0 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,22 @@ async fn load_gltf<'a, 'b>(
mesh.set_indices(Some(Indices::U32(indices.into_u32().collect())));
};

if mesh.attribute(Mesh::ATTRIBUTE_NORMAL).is_none() {
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::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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like we have dupe checks here

bevy_log::debug!(
"Missing vertex normals in indexed geometry, computing them as flat."
);
}
}

let mesh = load_context.set_labeled_asset(&primitive_label, LoadedAsset::new(mesh));
primitives.push(super::GltfPrimitive {
mesh,
Expand Down
112 changes: 112 additions & 0 deletions crates/bevy_render/src/mesh/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,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.
Expand Down Expand Up @@ -254,6 +261,29 @@ pub enum Indices {
U32(Vec<u32>),
}

impl Indices {
fn iter(&self) -> impl Iterator<Item = usize> + '_ {
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<Self::Item> {
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 {
Expand Down Expand Up @@ -431,6 +461,88 @@ impl Mesh {

attributes_interleaved_buffer
}

/// 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<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.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::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),
}
}
}

/// Calculates the [`Mesh::ATTRIBUTE_NORMAL`] of a mesh.
///
/// Panics if [`Indices`] are set.
/// Consider calling [Mesh::duplicate_vertices] or export your mesh with normal attributes.
pub fn compute_flat_normals(&mut self) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normals should also be normalized/unit length.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, normals should be actually normal 😄

if self.indices().is_some() {
panic!("`compute_flat_normals` can't work on indexed geometry. Consider calling `Mesh::duplicate_vertices`.");
}

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).normalize().into()
}

fn remove_resource_save(
Expand Down