[rmodels] Fix glTF skinning when joints have non-joint parent nodes - #5876
Conversation
Some glTF exporters (notably wow.export, but also various other DCC pipelines) place skin joints under intermediate non-joint transform nodes that carry part of the bind-pose offset. raylib's existing LoadBoneInfoGLTF and LoadModelAnimationsGLTF only inspected a joint's immediate parent and only sampled joint-local TRS, so any transform stored on an intermediate non-joint ancestor was silently dropped, producing exploded or stretched meshes at runtime. Two surgical changes: LoadBoneInfoGLTF: walk the parent chain past any non-joint ancestors when looking up parentIndex, instead of comparing only against node.parent. Joints whose direct parent is a non-joint were previously treated as skeleton roots. LoadModelAnimationsGLTF: precompute a per-joint extOffset matrix that bakes in the static TRS contribution of any intermediate non-joint nodes between the joint and its nearest joint ancestor. Apply it to each frame's joint TRS before BuildPoseFromParentJoints so the per-frame world transforms match the bind-pose world transforms (LoadGLTF already used cgltf_node_transform_world for bindPose, so this aligns the two code paths). The replaced root-only worldTransform adjustment is a strict subset of the new per-joint extOffset machinery, so it has been removed. Spec-compliant files (the six skeletal-skinning .glb examples shipped with raylib) render bit-identically before and after; previously broken files (e.g. wow.export's babyoctopus.gltf) now match the reference rendering from f3d, the Khronos sample viewer, and three.js.
|
@cseelhoff Please, could you provide some affected model for testing? |
|
Just started to test it. |
|
The fix worked for the cow. I will try the rest. |
|
Works nice, great work. |
| bool isJoint = false; | ||
| for (int jj = 0; jj < jointCount; jj++) | ||
| { | ||
| if (skin.joints[jj] == n) { isJoint = true; break; } |
| if (skin.joints[jj] == n) { isJoint = true; break; } | ||
| } | ||
| if (isJoint) break; | ||
| Matrix nm = MatrixMultiply(MatrixMultiply( |
There was a problem hiding this comment.
Formatting or try to write this more readable.
|
@kimkulling Thanks for the review! Just approveed the CodeQL checks running. |
[rmodels] Address review feedback on glTF joint offset handling Resolve the open review points raised on PR raysan5#5876 for the per-joint extOffset precompute in LoadModelAnimationsGLTF. * NULL check: validate the extOffset RL_MALLOC result before use, which was previously missing. * Fail-fast handling: detect the allocation failure at its source and abort animation loading (log a warning, free resources, return NULL) instead of propagating a NULL pointer into the per-frame loop. * Brace formatting: expand the collapsed one-line joint-match check (if (...) { isJoint = true; break; }) into raylib's standard Allman brace style. * Readability: break the deeply nested MatrixMultiply(MatrixMultiply(...)) into named nodeScale/nodeRotation/nodeTranslation/nodeTransform locals, mirroring the existing S/R/T composition later in the function. * Spacing/line breaks: add blank lines within the precompute block to match the surrounding code style.
|
|
@cseelhoff thanks for the improvement and thanks @kimkulling for the review! Merging! |
[rmodels] Fix glTF skinning when joints have non-joint parent nodes
Summary
Two surgical fixes in
src/rmodels.cthat correct glTF skinning for fileswhere the skin's joints are nested under intermediate non-joint transform
nodes that carry part of the bind-pose offset. Common in exports from
wow.export, and seen in some Blender/Maya/Houdini pipelines that wrap an
armature in a parent "rig" or "offset" empty.
Both spec-compliant files (every skeletal
.glbshipped with raylib'sexamples) and previously broken files now render correctly.
The bug
A glTF skin lists which nodes are joints. The skeleton hierarchy that
raylib reconstructs from that list assumes each joint's parent in
skin.joints[]equals its node-tree parent. Many real-world filesviolate this: a joint can have a non-joint ancestor (a plain transform
node) sitting between it and the next joint up the chain. That
intermediate node's TRS is part of the model's bind pose and must be
applied to the joint's animation frames; otherwise the per-frame world
transform diverges from the bind world transform, the inverse-bind
multiplication in
UpdateModelAnimationBoneMatricesproducesnon-identity skin matrices at rest, and the mesh explodes.
LoadGLTFalready handles this correctly when buildingbindPosebecause it calls
cgltf_node_transform_world(joint)per joint, whichwalks the full ancestor chain. The two animation-side functions did not.
Repro file: babyoctopus.gltf from a wow.export of a World of Warcraft
model. Stock raylib renders a stretched/inverted mesh; f3d, the Khronos
sample viewer, and three.js all render it correctly. Screenshots
attached.
The fix
1.
LoadBoneInfoGLTFReplace the single-step parent comparison with a loop that walks
node.parent->parent->parent...until it either hits a node listed inskin.joints[]or runs off the scene root. Joints whose direct parentis a non-joint were previously assigned
parent = -1(treated asskeleton roots), corrupting the chain used by
BuildPoseFromParentJoints.2.
LoadModelAnimationsGLTFPrecompute once per skin a
Matrix extOffset[k]per joint that bakes inthe composed TRS of every intermediate non-joint node between joint
kand its nearest joint ancestor. Then, in the per-frame loop, compose the
joint's local TRS into a matrix, multiply by
extOffset[k], andMatrixDecomposeback to TRS before storing inkeyframePoses. Theresult is the joint's transform expressed relative to its skeleton
parent, which is what
BuildPoseFromParentJointsexpects.This also subsumes the previous root-only
worldTransformadjustmentblock — the new
extOffset[0]covers it for the root joint andcorrectly extends the same logic to every joint. The dead variables and
the root patch-up are removed.
Diff size
+49 / -29 lines in one file. No new files, no API changes, no behaviour
changes for files that don't have intermediate non-joint nodes.
Validation
Built on Linux with cmake + gcc, no warnings.
Regression sweep across every skeletal
.glbin raylib's examples(
robot.glb,greenman.glb,raylib_logo_3d.glb,old_car_new.glb,plane.glb,shaders/robot.glb): pixel-identical output before/afterunder fixed-camera screenshot comparison. Any noise was sub-0.0002 and
traced to HUD framerate-counter antialiasing, not 3D output.
Visual verification of the broken file (babyoctopus.gltf): now matches
f3d, the Khronos sample viewer, and three.js.
Known limitation (pre-existing, not addressed by this PR)
If a non-joint ancestor uses a 4×4
matrixinstead of TRS(
node->has_matrix == 1in cgltf), this patch'sextOffsetbuilderreads
n->scale/n->rotation/n->translationdirectly and ignoresmatrix. This matches the rest ofrmodels.c(nohas_matrixreferences; joint TRS reader behaves the same way). Properly handling
matrix-form nodes would be a separate, file-wide change.
Checklist
rmodels.c