Fix GLTF bind matrix remapping#5782
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the GLTF skin import path to remap inverse bind matrices in a joint-local way (instead of applying a world-space skeleton transform directly), improving rig parity between GLB and DAE imports when bind poses differ from the viewer skeleton. It also adds ignores for locally generated build/log output folders.
Changes:
- Precomputes rotated bind matrices in viewer space and remaps each joint’s bind matrix using a per-joint rest→bind delta applied onto the viewer override rest matrix.
- Adds helper APIs to centralize GLTF→viewer coordinate rotation for bind/rest computations.
- Updates
.gitignoreto exclude additional generated folders.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
indra/newview/gltf/llgltfloader.h |
Declares new helper methods for coordinate conversion and bind-matrix remapping. |
indra/newview/gltf/llgltfloader.cpp |
Implements joint-local bind remapping (computeViewerBindMatrix) and uses it during skin processing. |
.gitignore |
Adds ignore rules for local build/install/log directories. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (bind_iter == rotated_bind_matrices.end() || !node_data.mIsOverrideValid) | ||
| { | ||
| converted_bind_matrices[gltf_node_index] = node_data.mOverrideRestMatrix; | ||
| return node_data.mOverrideRestMatrix; | ||
| } | ||
|
|
There was a problem hiding this comment.
Looks like copilot got the right idea here. mOverrideRestMatrix isn't initialized by default so code should either check it, assert or have other sort of 'don't run before buildOverrideMatrix' security.
But it does look like mOverrideRestMatrix would be initialized because buildOverrideMatrix gets called first for every joint.
| // skeleton override and bind-pose calculations. Keeping this conversion in | ||
| // one helper ensures bind and rest matrices use the same rotation path, | ||
| // including the optional XY rotation applied for compatible uploads. |
|
|
|
Thank you. |
Description
Fixes GLTF inverse bind matrix remapping for skeletons whose bind pose does not exactly match the viewer skeleton. This makes it possible to import GLB or DAE files created from the same model with identical vertex-level results. Edges may still vary depending on how each exporter calculates triangulation.
The previous code applied a per-joint world-space skeleton transform directly to each bind matrix. When a source parent joint carries bind-pose rotation that the viewer parent does not, that rotation can be baked into child inverse bind matrices, causing visible child joint twists.
This change remaps bind matrices by preserving each joint's bind-pose delta from its own GLTF rest matrix, then applying that local delta to the viewer override rest matrix. It also centralizes GLTF-to-viewer coordinate conversion for rest and bind matrix calculations.
Also adds
.gitignoreentries for generated local build folders:/.autobuild-installables//.logs/Related Issues
Issue Link: relates to https://feedback.secondlife.com/bug-reports/p/gltf-inverse-bind-matrix-remapping-bind-poses-not-fully-supported
Checklist
Please ensure the following before requesting review:
Additional Notes
1.) This is a bind-posed model in Blender with A-pose as rest pose:
2.) This is how the SL preViewer displays the model with this fix applied:
3.) This is how the official SL preViewer displays the same model without the fix:
4.) Here are the DAE and GLB files, both exported from the same A-posed mesh:
glb_and_dae.zip
Important: When both files are loaded with the patched viewer, both meshes are identical at vertex level. Edges may differ due to different triangulation between GLB and DAE exports.
Local build verification:
llgltfloader.cppwas compiled through the Xcodesecondlife-bintarget inRelease.-Walland-Werror.llgltfloader.cpp.Disclaimer
This patch fixes the issue in our tests, but there may be a better approach. Please feel free to adjust the implementation as needed.