From 81c7cb65274bf2a952a97fb32bc153a3c71ba5c1 Mon Sep 17 00:00:00 2001 From: Caleb Seelhoff Date: Fri, 29 May 2026 12:17:04 -0400 Subject: [PATCH] [rmodels] Fix glTF skinning when joints have non-joint parent nodes (#5876) * [rmodels] Fix glTF skinning when joints have non-joint parent nodes 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. * Resolve review: NULL-check joint offset allocation, fail fast [rmodels] Address review feedback on glTF joint offset handling Resolve the open review points raised on PR #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. --- src/rmodels.c | 100 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 29 deletions(-) diff --git a/src/rmodels.c b/src/rmodels.c index eb7f30cb8..eeebc3d2b 100644 --- a/src/rmodels.c +++ b/src/rmodels.c @@ -5375,16 +5375,18 @@ static BoneInfo *LoadBoneInfoGLTF(cgltf_skin skin, int *boneCount) cgltf_node node = *skin.joints[i]; if (node.name != NULL) strncpy(bones[i].name, node.name, sizeof(bones[i].name) - 1); - // Find parent bone index + // Find parent bone index by walking up the node tree past any + // non-joint ancestors (intermediate transform nodes used by some + // DCC exporters), until we hit a node that is also in skin.joints. int parentIndex = -1; - - for (unsigned int j = 0; j < skin.joints_count; j++) + cgltf_node *ancestor = node.parent; + while (ancestor != NULL && parentIndex == -1) { - if (skin.joints[j] == node.parent) + for (unsigned int j = 0; j < skin.joints_count; j++) { - parentIndex = (int)j; - break; + if (skin.joints[j] == ancestor) { parentIndex = (int)j; break; } } + if (parentIndex == -1) ancestor = ancestor->parent; } bones[i].parent = parentIndex; @@ -6515,20 +6517,58 @@ static ModelAnimation *LoadModelAnimationsGLTF(const char *fileName, int *animCo if (data->skins_count > 0) { cgltf_skin skin = data->skins[0]; + + // Precompute, per joint, the static transform contributed by any + // intermediate non-joint nodes between the joint and its nearest + // joint ancestor. This handles exporters (e.g. wow.export) that + // store bone offsets on dummy parent nodes rather than on the + // joints themselves. Depends only on the skin, not the animation. + int jointCount = (int)skin.joints_count; + Matrix *extOffset = (Matrix *)RL_MALLOC(jointCount*sizeof(Matrix)); + + if (extOffset == NULL) + { + // Allocation failed: abort animation loading at the source rather than + // propagating a NULL pointer into the per-frame transform loop below + TRACELOG(LOG_WARNING, "MODEL: [%s] Failed to allocate joint offset buffer", fileName); + cgltf_free(data); + UnloadFileData(fileData); + *animCount = 0; + return NULL; + } + *animCount = (int)data->animations_count; animations = (ModelAnimation *)RL_CALLOC(data->animations_count, sizeof(ModelAnimation)); - Transform worldTransform = { 0 }; - cgltf_float cgltf_worldTransform[16] = { 0 }; - cgltf_node *node = skin.joints[0]; - cgltf_node_transform_world(node->parent, cgltf_worldTransform); - Matrix worldMatrix = { - cgltf_worldTransform[0], cgltf_worldTransform[4], cgltf_worldTransform[8], cgltf_worldTransform[12], - cgltf_worldTransform[1], cgltf_worldTransform[5], cgltf_worldTransform[9], cgltf_worldTransform[13], - cgltf_worldTransform[2], cgltf_worldTransform[6], cgltf_worldTransform[10], cgltf_worldTransform[14], - cgltf_worldTransform[3], cgltf_worldTransform[7], cgltf_worldTransform[11], cgltf_worldTransform[15] - }; - MatrixDecompose(worldMatrix, &worldTransform.translation, &worldTransform.rotation, &worldTransform.scale); + for (int k = 0; k < jointCount; k++) + { + extOffset[k] = MatrixIdentity(); + cgltf_node *n = skin.joints[k]->parent; + + while (n != NULL) + { + bool isJoint = false; + for (int jj = 0; jj < jointCount; jj++) + { + if (skin.joints[jj] == n) + { + isJoint = true; + break; + } + } + + if (isJoint) break; + + // Compose the intermediate node's local TRS (scale, then rotation, then translation) + Matrix nodeScale = MatrixScale(n->scale[0], n->scale[1], n->scale[2]); + Matrix nodeRotation = QuaternionToMatrix((Quaternion){ n->rotation[0], n->rotation[1], n->rotation[2], n->rotation[3] }); + Matrix nodeTranslation = MatrixTranslate(n->translation[0], n->translation[1], n->translation[2]); + Matrix nodeTransform = MatrixMultiply(MatrixMultiply(nodeScale, nodeRotation), nodeTranslation); + + extOffset[k] = MatrixMultiply(extOffset[k], nodeTransform); + n = n->parent; + } + } for (unsigned int a = 0; a < data->animations_count; a++) { @@ -6637,19 +6677,19 @@ static ModelAnimation *LoadModelAnimationsGLTF(const char *fileName, int *animCo } } - animations[a].keyframePoses[j][k] = (Transform){ - .translation = translation, - .rotation = rotation, - .scale = scale - }; - } + // Compose joint local TRS, then prepend the static + // intermediate non-joint offsets so the final TRS is + // expressed relative to the joint's skeleton parent. + Matrix S = MatrixScale(scale.x, scale.y, scale.z); + Matrix R = QuaternionToMatrix(rotation); + Matrix T = MatrixTranslate(translation.x, translation.y, translation.z); + Matrix jointLocal = MatrixMultiply(MatrixMultiply(S, R), T); + Matrix combined = MatrixMultiply(jointLocal, extOffset[k]); - Transform *root = &animations[a].keyframePoses[j][0]; - root->rotation = QuaternionMultiply(worldTransform.rotation, root->rotation); - root->scale = Vector3Multiply(root->scale, worldTransform.scale); - root->translation = Vector3Multiply(root->translation, worldTransform.scale); - root->translation = Vector3RotateByQuaternion(root->translation, worldTransform.rotation); - root->translation = Vector3Add(root->translation, worldTransform.translation); + Transform tr; + MatrixDecompose(combined, &tr.translation, &tr.rotation, &tr.scale); + animations[a].keyframePoses[j][k] = tr; + } BuildPoseFromParentJoints(bones, animations[a].boneCount, animations[a].keyframePoses[j]); } @@ -6658,6 +6698,8 @@ static ModelAnimation *LoadModelAnimationsGLTF(const char *fileName, int *animCo RL_FREE(boneChannels); RL_FREE(bones); } + + RL_FREE(extOffset); } if (data->skins_count > 1)