From 28060f3180743ce5bef4ada594c51347ccf6c406 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 18 Sep 2023 16:55:24 +0100 Subject: [PATCH] invert face culling for negatively scaled gltf nodes (#8859) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective according to [khronos](https://github.com/KhronosGroup/glTF/issues/1697), gltf nodes with inverted scales should invert the winding order of the mesh data. this is to allow negative scale to be used for mirrored geometry. ## Solution in the gltf loader, create a separate material with `cull_mode` set to `Face::Front` when the node scale is negative. note/alternatives: this applies for nodes where the scale is negative at gltf import time. that seems like enough for the mentioned use case of mirrored geometry. it doesn't help when scales dynamically go negative at runtime, but you can always set double sided in that case. i don't think there's any practical difference between using front-face culling and setting a clockwise winding order explicitly, but winding order is supported by wgpu so we could add the field to StandardMaterial/StandardMaterialKey and set it directly on the pipeline descriptor if there's a reason to. it wouldn't help with dynamic scale adjustments anyway, and would still require a separate material. fixes #4738, probably fixes #7901. --------- Co-authored-by: François --- crates/bevy_gltf/src/loader.rs | 35 +++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 64d17305b3e04..069e1e6ed513f 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -296,7 +296,7 @@ async fn load_gltf<'a, 'b, 'c>( let mut named_materials = HashMap::default(); // NOTE: materials must be loaded after textures because image load() calls will happen before load_with_settings, preventing is_srgb from being set properly for material in gltf.materials() { - let handle = load_material(&material, load_context); + let handle = load_material(&material, load_context, false); if let Some(name) = material.name() { named_materials.insert(name.to_string(), handle.clone()); } @@ -504,6 +504,7 @@ async fn load_gltf<'a, 'b, 'c>( &mut node_index_to_entity_map, &mut entity_to_skin_index_map, &mut active_camera_found, + &Transform::default(), ); if result.is_err() { err = Some(result); @@ -665,8 +666,12 @@ async fn load_image<'a, 'b>( } /// Loads a glTF material as a bevy [`StandardMaterial`] and returns it. -fn load_material(material: &Material, load_context: &mut LoadContext) -> Handle { - let material_label = material_label(material); +fn load_material( + material: &Material, + load_context: &mut LoadContext, + is_scale_inverted: bool, +) -> Handle { + let material_label = material_label(material, is_scale_inverted); load_context.labeled_asset_scope(material_label, |load_context| { let pbr = material.pbr_metallic_roughness(); @@ -712,6 +717,8 @@ fn load_material(material: &Material, load_context: &mut LoadContext) -> Handle< double_sided: material.double_sided(), cull_mode: if material.double_sided() { None + } else if is_scale_inverted { + Some(Face::Front) } else { Some(Face::Back) }, @@ -734,10 +741,19 @@ fn load_node( node_index_to_entity_map: &mut HashMap, entity_to_skin_index_map: &mut HashMap, active_camera_found: &mut bool, + parent_transform: &Transform, ) -> Result<(), GltfError> { let transform = gltf_node.transform(); let mut gltf_error = None; let transform = Transform::from_matrix(Mat4::from_cols_array_2d(&transform.matrix())); + let world_transform = *parent_transform * transform; + // according to https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#instantiation, + // if the determinant of the transform is negative we must invert the winding order of + // triangles in meshes on the node. + // instead we equivalently test if the global scale is inverted by checking if the number + // of negative scale factors is odd. if so we will assign a copy of the material with face + // culling inverted, rather than modifying the mesh data directly. + let is_scale_inverted = world_transform.scale.is_negative_bitmask().count_ones() & 1 == 1; let mut node = world_builder.spawn(SpatialBundle::from(transform)); node.insert(node_name(gltf_node)); @@ -812,13 +828,14 @@ fn load_node( // append primitives for primitive in mesh.primitives() { let material = primitive.material(); - let material_label = material_label(&material); + let material_label = material_label(&material, is_scale_inverted); // This will make sure we load the default material now since it would not have been // added when iterating over all the gltf materials (since the default material is // not explicitly listed in the gltf). + // It also ensures an inverted scale copy is instantiated if required. if !load_context.has_labeled_asset(&material_label) { - load_material(&material, load_context); + load_material(&material, load_context, is_scale_inverted); } let primitive_label = primitive_label(&mesh, &primitive); @@ -948,6 +965,7 @@ fn load_node( node_index_to_entity_map, entity_to_skin_index_map, active_camera_found, + &world_transform, ) { gltf_error = Some(err); return; @@ -990,9 +1008,12 @@ fn morph_targets_label(mesh: &gltf::Mesh, primitive: &Primitive) -> String { } /// Returns the label for the `material`. -fn material_label(material: &gltf::Material) -> String { +fn material_label(material: &gltf::Material, is_scale_inverted: bool) -> String { if let Some(index) = material.index() { - format!("Material{index}") + format!( + "Material{index}{}", + if is_scale_inverted { " (inverted)" } else { "" } + ) } else { "MaterialDefault".to_string() }