Skip to content

Commit

Permalink
invert face culling for negatively scaled gltf nodes (bevyengine#8859)
Browse files Browse the repository at this point in the history
# Objective

according to
[khronos](KhronosGroup/glTF#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 bevyengine#4738, probably fixes bevyengine#7901.

---------

Co-authored-by: François <mockersf@gmail.com>
  • Loading branch information
2 people authored and Ray Redondo committed Jan 9, 2024
1 parent 835fc96 commit 628661d
Showing 1 changed file with 28 additions and 7 deletions.
35 changes: 28 additions & 7 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<StandardMaterial> {
let material_label = material_label(material);
fn load_material(
material: &Material,
load_context: &mut LoadContext,
is_scale_inverted: bool,
) -> Handle<StandardMaterial> {
let material_label = material_label(material, is_scale_inverted);
load_context.labeled_asset_scope(material_label, |load_context| {
let pbr = material.pbr_metallic_roughness();

Expand Down Expand Up @@ -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)
},
Expand All @@ -734,10 +741,19 @@ fn load_node(
node_index_to_entity_map: &mut HashMap<usize, Entity>,
entity_to_skin_index_map: &mut HashMap<Entity, usize>,
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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()
}
Expand Down

0 comments on commit 628661d

Please sign in to comment.