Skip to content

Commit

Permalink
GLTF loader: handle warning NODE_SKINNED_MESH_WITHOUT_SKIN (#9360)
Browse files Browse the repository at this point in the history
# Objective

- According to the GLTF spec, it should not be possible to have a non
skinned mesh on a skinned node
> When the node contains skin, all mesh.primitives MUST contain JOINTS_0
and WEIGHTS_0 attributes
>
https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#reference-node
- However, the reverse (a skinned mesh on a non skinned node) is just a
warning, see `NODE_SKINNED_MESH_WITHOUT_SKIN` in
https://github.com/KhronosGroup/glTF-Validator/blob/main/ISSUES.md#linkerror
- This causes a crash in Bevy because the bind group layout is made from
the mesh which is skinned, but filled from the entity which is not
```
thread '<unnamed>' panicked at 'wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `<CommandBuffer-(0, 5, Metal)>`
    In a set_bind_group command
      note: bind group = `<BindGroup-(27, 1, Metal)>`
    Bind group 2 expects 2 dynamic offsets. However 1 dynamic offset were provided.
```
- Blender can export GLTF files with this kind of issues

## Solution

- When a skinned mesh is only used on non skinned nodes, ignore skinned
information from the mesh and warn the user (this is what three.js is
doing)
- When a skinned mesh is used on both skinned and non skinned nodes, log
an error
  • Loading branch information
mockersf authored Oct 13, 2023
1 parent 8b88887 commit 9290674
Showing 1 changed file with 25 additions and 2 deletions.
27 changes: 25 additions & 2 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use bevy_core::Name;
use bevy_core_pipeline::prelude::Camera3dBundle;
use bevy_ecs::{entity::Entity, world::World};
use bevy_hierarchy::{BuildWorldChildren, WorldChildBuilder};
use bevy_log::warn;
use bevy_log::{error, warn};
use bevy_math::{Mat4, Vec3};
use bevy_pbr::{
AlphaMode, DirectionalLight, DirectionalLightBundle, PbrBundle, PointLight, PointLightBundle,
Expand Down Expand Up @@ -36,7 +36,7 @@ use gltf::{
accessor::Iter,
mesh::{util::ReadIndices, Mode},
texture::{MagFilter, MinFilter, WrappingMode},
Material, Node, Primitive,
Material, Node, Primitive, Semantic,
};
use serde::Deserialize;
use std::{
Expand Down Expand Up @@ -329,6 +329,17 @@ async fn load_gltf<'a, 'b, 'c>(

let mut meshes = vec![];
let mut named_meshes = HashMap::default();
let mut meshes_on_skinned_nodes = HashSet::default();
let mut meshes_on_non_skinned_nodes = HashSet::default();
for gltf_node in gltf.nodes() {
if gltf_node.skin().is_some() {
if let Some(mesh) = gltf_node.mesh() {
meshes_on_skinned_nodes.insert(mesh.index());
}
} else if let Some(mesh) = gltf_node.mesh() {
meshes_on_non_skinned_nodes.insert(mesh.index());
}
}
for gltf_mesh in gltf.meshes() {
let mut primitives = vec![];
for primitive in gltf_mesh.primitives() {
Expand All @@ -339,6 +350,18 @@ async fn load_gltf<'a, 'b, 'c>(

// Read vertex attributes
for (semantic, accessor) in primitive.attributes() {
if [Semantic::Joints(0), Semantic::Weights(0)].contains(&semantic) {
if !meshes_on_skinned_nodes.contains(&gltf_mesh.index()) {
warn!(
"Ignoring attribute {:?} for skinned mesh {:?} used on non skinned nodes (NODE_SKINNED_MESH_WITHOUT_SKIN)",
semantic,
primitive_label
);
continue;
} else if meshes_on_non_skinned_nodes.contains(&gltf_mesh.index()) {
error!("Skinned mesh {:?} used on both skinned and non skin nodes, this is likely to cause an error (NODE_SKINNED_MESH_WITHOUT_SKIN)", primitive_label);
}
}
match convert_attribute(
semantic,
accessor,
Expand Down

0 comments on commit 9290674

Please sign in to comment.