invert face culling for negatively scaled gltf nodes #8859
Conversation
robtfm
added
A-Rendering
|
tested and seems fine with examples from https://github.com/KhronosGroup/glTF-Asset-Generator/blob/master/Output/Positive/Node_NegativeScale/README.md. i had to generate indices to get the normal maps to load (#8862) |
| ) -> 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; |
There was a problem hiding this comment.
Transform * Transform is a lossy operation. I'm concerned the determinant might end up incorrect in some transform hierarchies.
I suggest passing a GlobalTransform around instead to be certain.
There was a problem hiding this comment.
i'm happy to do this. it'll be slower, but this isn't a hot code path.
in the gltf spec it says that you can either use a translation + scale + rotation, or a matrix which must decompose into those parts, and explicitly cannot skew or shear.
since we only care about the scale, and the scales are just Vec3-multiplied at each step, i'm pretty sure this is safe. i guess i could pass the scale down as a Vec3 instead of the whole transform if that would be clearer / avoid potential future misuse.
There was a problem hiding this comment.
in the gltf spec it says that you can either use a translation + scale + rotation, or a matrix which must decompose into those parts, and explicitly cannot skew or shear.
I think that implementation note is referring purely to the local transform stored in the gltf format, but the note is unfortunately not making that very clear.
since we only care about the scale, and the scales are just Vec3-multiplied at each step, i'm pretty sure this is safe.
I don't have the math intuition to know whether the loss of skew and shear still keeps the determinant the same 🤷
There was a problem hiding this comment.
I think that implementation note is referring purely to the local transform stored in the gltf format, but the note is unfortunately not making that very clear.
yes, but that's all we're dealing with here. at this point the gltf isn't part of the world heirarchy, we just initialize the transform to IDENTITY at the gltf root node.
Co-authored-by: François <mockersf@gmail.com>
mockersf
added
the
S-Ready-For-Final-Review
# 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>
Objective
according to khronos, 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_modeset toFace::Frontwhen 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.