Take the rest pose into account when computing the AABBs of skinned meshes in glTF files. #21845
+266
−19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
meshes in glTF files. In glTF, the joints of a skinned mesh aren't required to have identity transforms for the rest pose. In particular, Maya likes to place skinned meshes at the origin and then uses joint transforms to move them into place. At present, the Bevy glTF loader naively assumes that the minimum and maximum values of the `POSITION` accessor suffice to determine the bounding box of a mesh, but for skinned meshes with non-identity joint transforms this is not true. This could cause Bevy to apply incorrect `Aabb` components to skinned meshes, which would in turn cause those meshes to be incorrectly frustum culled and disappear. This PR fixes the issue by constructing the AABBs manually for skinned meshes in glTF files. When computing these AABBs, this patch takes the rest pose into account, fixing the issue. For non-skinned meshes, this patch makes Bevy continue to use the precomputed minimum and maximum values of the `POSITION` accessor, as this is safe. Note that this patch *doesn't* fix all possible causes of incorrect AABBs. In particular, animation of skins and morph targets can still cause meshes to extend outside their AABBs and be incorrectly culled. The [`bevy_mod_skinned_aabb`] plugin can compute per-joint AABBs that remain correct in the presence of animation, at some CPU cost. Alternately, developers may wish to manually extend AABBs for skinned meshes as necessary to include all possible animations by modifying the automatically-generated `Aabb` component, or even remove the `Aabb` component altogether. Additionally, this patch doesn't handle the case in which a mesh and joints are manually constructed outside of glTF. In this case, the `bevy_camera::visibility::calculate_bounds` system will generate an incorrect AABB for the mesh. I intentionally left that out of this patch, because regenerating AABBs on CPU whenever a joint is updated would be slow; [`bevy_mod_skinned_aabb`] would be a better approach. Besides, constructing skinned meshes programmatically is rare, and glTF is much more commonly used in practice. For comparison, Unreal and Godot use a technique similar to [`bevy_mod_skinned_aabb`] to generate AABBs. Unity can either use that technique or, by default, simply widens the AABB to encompass not only the rest pose but also all animations in the imported FBX file. We could implement that if desired in a follow-up. This PR obsoletes bevyengine#21787, which removed `Aabb` components entirely for skinned meshes. The current patch is a more aggressive approach that, while not foolproof, strictly improves the situation in common cases while maintaining automatic frustum culling support for skinned meshes in general. [`bevy_mod_skinned_aabb`]: https://github.com/greeble-dev/bevy_mod_skinned_aabb
This was referenced Nov 15, 2025
pcwalton
added
S-Needs-Review
Contributor
Author
|
#21837 would be great as an alternative approach to this. There's room to support both IMO. |
Contributor
Author
|
The failure looks unrelated and this should be good to review. |
greeble-dev
approved these changes
Nov 15, 2025
Contributor
greeble-dev
left a comment
greeble-dev
left a comment
There was a problem hiding this comment.
Clicking approve as it seems like a useful option and I couldn't spot any problems - just a couple of optional suggestions. Is a pity that the code is glTF specific when it could be useful elsewhere, but I don't see a practical alternative in the short-term.
If this was the only PR related to skinned mesh bounds then I'd suggest the new behavior should be optional, and arguably not the default. However, #21837 adds all the plumbing for an option and proposes a different default. So I'd suggest:
Comment on lines
+2097
to
+2107
| // Make sure to compute joint matrix * position * weight, | ||
| // not joint matrix * weight * position, as the latter would | ||
| // require Bevy to multiply every element of the matrix by | ||
| // the weight (i.e. weighting the joint would use 12 | ||
| // multiplies instead of 3). | ||
| let [i0, i1, i2, i3] = joint_indices; | ||
| let [w0, w1, w2, w3] = joint_weights; | ||
| position = joint_matrices[i0 as usize].transform_point3(position) * w0 | ||
| + joint_matrices[i1 as usize].transform_point3(position) * w1 | ||
| + joint_matrices[i2 as usize].transform_point3(position) * w2 | ||
| + joint_matrices[i3 as usize].transform_point3(position) * w3; |
Contributor
There was a problem hiding this comment.
There's a third option: weight and sum the matrices and then do a single matrix * position. This is fairly similar if the target has FMA, and somewhat faster if that target doesn't have FMA. Don't think it's worth changing the code now though.
| } | ||
| } | ||
|
|
||
| /// A helper method that computex the global transform for a node and its |
Contributor
There was a problem hiding this comment.
Suggested change
| /// A helper method that computex the global transform for a node and its | |
| /// A helper method that computes the global transform for a node and its |
Comment on lines
+2025
to
+2027
| /// If the given node doesn't represent a skinned mesh, returns an empty | ||
| /// vector. | ||
| fn compute_joint_matrices_for_node(&mut self, gltf_node: &Node) -> Vec<Affine3A> { |
Contributor
There was a problem hiding this comment.
Suggested change
| /// If the given node doesn't represent a skinned mesh, returns an empty | |
| /// vector. | |
| fn compute_joint_matrices_for_node(&mut self, gltf_node: &Node) -> Vec<Affine3A> { | |
| /// If the given node doesn't represent a skinned mesh, returns `None`. | |
| fn compute_joint_matrices_for_node(&mut self, gltf_node: &Node) -> Option<Vec<Affine3A>> { |
Changing to an Option would add complication, but maybe worth it to clarify the non-skinned case?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In glTF, the joints of a skinned mesh aren't required to have identity transforms for the rest pose. In particular, Maya likes to place skinned meshes at the origin and then uses joint transforms to move them into place. At present, the Bevy glTF loader naively assumes that the minimum and maximum values of the
POSITIONaccessor suffice to determine the bounding box of a mesh, but for skinned meshes with non-identity joint transforms this is not true. This could cause Bevy to apply incorrectAabbcomponents to skinned meshes, which would in turn cause those meshes to be incorrectly frustum culled and disappear.This PR fixes the issue by constructing the AABBs manually for skinned meshes in glTF files. When computing these AABBs, this patch takes the rest pose into account, fixing the issue. For non-skinned meshes, this patch makes Bevy continue to use the precomputed minimum and maximum values of the
POSITIONaccessor, as this is safe.Note that this patch doesn't fix all possible causes of incorrect AABBs. In particular, animation of skins and morph targets can still cause meshes to extend outside their AABBs and be incorrectly culled. The
bevy_mod_skinned_aabbplugin can compute per-joint AABBs that remain correct in the presence of animation, at some CPU cost. Alternately, developers may wish to manually extend AABBs for skinned meshes as necessary to include all possible animations by modifying the automatically-generatedAabbcomponent, or even remove theAabbcomponent altogether.Additionally, this patch doesn't handle the case in which a mesh and joints are manually constructed outside of glTF. In this case, the
bevy_camera::visibility::calculate_boundssystem will generate an incorrect AABB for the mesh. I intentionally left that out of this patch, because regenerating AABBs on CPU whenever a joint is updated would be slow;bevy_mod_skinned_aabbwould be a better approach. Besides, constructing skinned meshes programmatically is rare, and glTF is much more commonly used in practice.For comparison, Unreal and Godot use a technique similar to
bevy_mod_skinned_aabbto generate AABBs. Unity can either use that technique or, by default, simply widens the AABB to encompass not only the rest pose but also all animations in the imported FBX file. We could implement that if desired in a follow-up.This PR obsoletes #21787, which removed
Aabbcomponents entirely for skinned meshes. The current patch is a more aggressive approach that, while not foolproof, strictly improves the situation in common cases while maintaining automatic frustum culling support for skinned meshes in general.