From 326e183961dc09bcca8b53fa2138efc080aa5338 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Thu, 11 Jun 2026 15:41:16 +0800 Subject: [PATCH 01/17] fix(loader): split shaderlab gltf fixes --- .../gltf/extensions/GLTFExtensionSchema.ts | 5 +- .../loader/src/gltf/parser/GLTFSceneParser.ts | 47 +-------- .../src/gltf/parser/GLTFSkinParser.test.ts | 44 +++++++++ .../loader/src/gltf/parser/GLTFSkinParser.ts | 53 ++++++---- tests/src/loader/GLTFLoader.test.ts | 97 +++++++++++++------ 5 files changed, 154 insertions(+), 92 deletions(-) create mode 100644 packages/loader/src/gltf/parser/GLTFSkinParser.test.ts diff --git a/packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts b/packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts index 522876e53b..126d3cab2b 100644 --- a/packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts +++ b/packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts @@ -1,5 +1,4 @@ import type { IMaterialNormalTextureInfo, ITextureInfo } from "../GLTFSchema"; -import type { RefItem } from "../../schema/CommonSchema"; /** * Interfaces from the KHR_lights_punctual extension @@ -153,7 +152,9 @@ export interface IEXTMeshoptCompressionSchema { filter?: "NONE" | "OCTAHEDRAL" | "QUATERNION" | "EXPONENTIAL"; } -export interface IGalaceanMaterialRemap extends RefItem { +export interface IGalaceanMaterialRemap { + url: string; + key?: string; isClone?: boolean; } diff --git a/packages/loader/src/gltf/parser/GLTFSceneParser.ts b/packages/loader/src/gltf/parser/GLTFSceneParser.ts index 5ee981b1d3..553bf177f2 100644 --- a/packages/loader/src/gltf/parser/GLTFSceneParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSceneParser.ts @@ -38,6 +38,7 @@ export class GLTFSceneParser extends GLTFParser { sceneRoot.addChild(context.get(GLTFParserType.Entity, sceneNodes[i])); } + (glTFResource._sceneRoots ||= [])[index] = sceneRoot; if (isDefaultScene) { glTFResource._defaultSceneRoot = sceneRoot; } @@ -195,49 +196,9 @@ export class GLTFSceneParser extends GLTFParser { if (rootBoneIndex !== -1) { BoundingBox.transform(mesh.bounds, inverseBindMatrices[rootBoneIndex], skinnedMeshRenderer.localBounds); } else { - // Root bone is not in joints list, we can only compute approximate inverse bind matrix - // Average all root bone's children inverse bind matrix - const approximateBindMatrix = new Matrix(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0); - let subRootBoneCount = this._computeApproximateBindMatrix( - bones, - inverseBindMatrices, - rootBone, - approximateBindMatrix - ); - - if (subRootBoneCount !== 0) { - Matrix.multiplyScalar(approximateBindMatrix, 1.0 / subRootBoneCount, approximateBindMatrix); - BoundingBox.transform(mesh.bounds, approximateBindMatrix, skinnedMeshRenderer.localBounds); - } else { - skinnedMeshRenderer.localBounds.copyFrom(mesh.bounds); - } - } - } - - private _computeApproximateBindMatrix( - jointEntities: ReadonlyArray, - inverseBindMatrices: Matrix[], - rootEntity: Entity, - approximateBindMatrix: Matrix - ): number { - let subRootBoneCount = 0; - const children = rootEntity.children; - for (let i = 0, n = children.length; i < n; i++) { - const rootChild = children[i]; - const index = jointEntities.indexOf(rootChild); - if (index !== -1) { - Matrix.add(approximateBindMatrix, inverseBindMatrices[index], approximateBindMatrix); - subRootBoneCount++; - } else { - subRootBoneCount += this._computeApproximateBindMatrix( - jointEntities, - inverseBindMatrices, - rootChild, - approximateBindMatrix - ); - } + const inverseRootBoneWorld = new Matrix(); + Matrix.invert(rootBone.transform.worldMatrix, inverseRootBoneWorld); + BoundingBox.transform(mesh.bounds, inverseRootBoneWorld, skinnedMeshRenderer.localBounds); } - - return subRootBoneCount; } } diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts new file mode 100644 index 0000000000..bb5714b847 --- /dev/null +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; + +describe("GLTFSkinParser rootBone resolution", () => { + async function createParser(): Promise { + (globalThis as any).window = { AudioContext: undefined, TextMetrics: undefined }; + const { GLTFSkinParser } = await import("./GLTFSkinParser"); + return new GLTFSkinParser(); + } + + it("includes skinned mesh nodes when resolving missing skin.skeleton", async () => { + const parser = await createParser(); + const sceneRoot = { name: "GLTF_ROOT" }; + const meshRoot = { name: "Character_Man", parent: sceneRoot }; + const hips = { name: "mixamorig:Hips", parent: sceneRoot }; + const spine = { name: "mixamorig:Spine", parent: hips }; + + const rootBone = (parser as any)._findSkinRootBoneByLCA( + 0, + [1, 2], + [meshRoot, hips, spine], + [{ name: "Character_Man", skin: 0 }, { name: "mixamorig:Hips" }, { name: "mixamorig:Spine" }] + ); + + expect(rootBone).toBe(sceneRoot); + }); + + it("does not promote to the scene wrapper for unrelated top-level siblings", async () => { + const parser = await createParser(); + const sceneRoot = { name: "GLTF_ROOT" }; + const characterRoot = { name: "Character_Root", parent: sceneRoot }; + const mesh = { name: "Character_Mesh", parent: characterRoot }; + const hips = { name: "mixamorig:Hips", parent: characterRoot }; + const light = { name: "Light", parent: sceneRoot }; + + const rootBone = (parser as any)._findSkinRootBoneByLCA( + 0, + [3], + [characterRoot, mesh, light, hips], + [{ name: "Character_Root" }, { name: "Character_Mesh", skin: 0 }, { name: "Light" }, { name: "mixamorig:Hips" }] + ); + + expect(rootBone).toBe(characterRoot); + }); +}); diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.ts index e8959b4b70..3f11f487e5 100644 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.ts @@ -34,20 +34,17 @@ export class GLTFSkinParser extends GLTFParser { } skin.bones = bones; - // Get skeleton — when `skin.skeleton` is absent, resolve via joints' LCA - // LCA falls back to the GLTF_ROOT wrapper only when joints span multiple top-level scene nodes + // Get skeleton if (skeleton !== undefined) { const rootBone = entities[skeleton]; - if (!rootBone) { - throw `Skin skeleton index ${skeleton} is out of range.`; - } skin.rootBone = rootBone; } else { - const rootBone = this._findSkeletonRootBone(joints, entities); - if (!rootBone) { + const rootBone = this._findSkinRootBoneByLCA(index, joints, entities, glTF.nodes); + if (rootBone) { + skin.rootBone = rootBone; + } else { throw "Failed to find skeleton root bone."; } - skin.rootBone = rootBone; } return skin; @@ -56,32 +53,50 @@ export class GLTFSkinParser extends GLTFParser { return AssetPromise.resolve(skinPromise); } - /** - * Resolve the skeleton rootBone as the lowest common ancestor of the joints' parent chains. - * Returns null when joints share no common ancestor. - */ - private _findSkeletonRootBone(joints: number[], entities: Entity[]): Entity | null { - const paths = >{}; - for (const index of joints) { + private _findSkinRootBoneByLCA( + skinIndex: number, + joints: number[], + entities: Entity[], + nodes: Array<{ skin?: number }> = [] + ): Entity | null { + const nodeIndices = joints.slice(); + for (let i = 0, n = nodes.length; i < n; i++) { + if (nodes[i]?.skin === skinIndex) { + nodeIndices.push(i); + } + } + + return this._findRootBoneByLCA(nodeIndices, entities); + } + + private _findRootBoneByLCA(nodeIndices: number[], entities: Entity[]): Entity | null { + const paths: Entity[][] = []; + for (const index of nodeIndices) { const path = new Array(); let entity = entities[index]; while (entity) { path.unshift(entity); entity = entity.parent; } - paths[index] = path; + if (path.length) { + paths.push(path); + } + } + + if (!paths.length) { + return null; } let rootNode: Entity | null = null; for (let i = 0; ; i++) { - let path = paths[joints[0]]; + let path = paths[0]; if (i >= path.length) { return rootNode; } const entity = path[i]; - for (let j = 1, m = joints.length; j < m; j++) { - path = paths[joints[j]]; + for (let j = 1, m = paths.length; j < m; j++) { + path = paths[j]; if (i >= path.length || entity !== path[i]) { return rootNode; } diff --git a/tests/src/loader/GLTFLoader.test.ts b/tests/src/loader/GLTFLoader.test.ts index fc166a5a77..164dd788aa 100644 --- a/tests/src/loader/GLTFLoader.test.ts +++ b/tests/src/loader/GLTFLoader.test.ts @@ -26,7 +26,7 @@ import { registerGLTFParser } from "@galacean/engine-loader"; import { Color } from "@galacean/engine-math"; -import { WebGLEngine } from "@galacean/engine"; +import { WebGLEngine } from "@galacean/engine-rhi-webgl"; import { describe, beforeAll, afterAll, expect, it } from "vitest"; let engine: WebGLEngine; @@ -40,7 +40,7 @@ beforeAll(async function () { class GLTFCustomJSONParser extends GLTFParser { parse(context: GLTFParserContext) { if (context.glTFResource.url.endsWith("testSkinRoot.gltf")) { - context.buffers = [new ArrayBuffer(192)]; + context.buffers = [new ArrayBuffer(128)]; return Promise.resolve({ asset: { version: "2.0" @@ -53,7 +53,8 @@ beforeAll(async function () { ], nodes: [ { - name: "Character_Man" + name: "Character_Man", + skin: 0 }, { name: "mixamorig:Hips", @@ -66,8 +67,7 @@ beforeAll(async function () { skins: [ { inverseBindMatrices: 0, - // Joints span both top-level scene roots: Character_Man (0) and Hips (1)/Spine (2). - joints: [0, 1, 2] + joints: [1, 2] } ], accessors: [ @@ -75,7 +75,7 @@ beforeAll(async function () { bufferView: 0, byteOffset: 0, componentType: 5126, - count: 3, + count: 2, type: "MAT4" } ], @@ -83,19 +83,26 @@ beforeAll(async function () { { buffer: 0, byteOffset: 0, - byteLength: 192 + byteLength: 128 } ], buffers: [ { - byteLength: 192 + byteLength: 128 } ] }); } - if (context.glTFResource.url.endsWith("testSingleSkeleton.gltf")) { - context.buffers = [new ArrayBuffer(128)]; + if (context.glTFResource.url.endsWith("testSkinRootBounds.gltf")) { + const buffer = new ArrayBuffer(152); + const floats = new Float32Array(buffer); + // Inverse bind matrices for Hips and Spine. Their bind pose world x is + // Character_Group(3) + Hips(10), so inverse bind translates by -13. + floats.set([1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, -13, 0, 0, 1], 0); + floats.set([1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, -13, 0, 0, 1], 16); + floats.set([9, -1, -1, 11, 1, 1], 32); + context.buffers = [buffer]; return Promise.resolve({ asset: { version: "2.0" @@ -103,27 +110,45 @@ beforeAll(async function () { scene: 0, scenes: [ { - // Two top-level roots: a character skeleton and an unrelated sibling (e.g., a light). - nodes: [0, 2] + nodes: [0] } ], nodes: [ { - name: "Character_Root", - children: [1] + name: "Character_Group", + translation: [3, 0, 0], + children: [1, 2] }, { - name: "mixamorig:Hips" + name: "Character_Man", + mesh: 0, + skin: 0 }, { - name: "Light" + name: "mixamorig:Hips", + translation: [10, 0, 0], + children: [3] + }, + { + name: "mixamorig:Spine" } ], skins: [ { inverseBindMatrices: 0, - // All joints converge to a single top-level root (Character_Root). - joints: [0, 1] + joints: [2, 3] + } + ], + meshes: [ + { + primitives: [ + { + attributes: { + POSITION: 1 + }, + mode: 4 + } + ] } ], accessors: [ @@ -133,6 +158,15 @@ beforeAll(async function () { componentType: 5126, count: 2, type: "MAT4" + }, + { + bufferView: 1, + byteOffset: 0, + componentType: 5126, + count: 2, + type: "VEC3", + min: [9, -1, -1], + max: [11, 1, 1] } ], bufferViews: [ @@ -140,11 +174,16 @@ beforeAll(async function () { buffer: 0, byteOffset: 0, byteLength: 128 + }, + { + buffer: 0, + byteOffset: 128, + byteLength: 24 } ], buffers: [ { - byteLength: 128 + byteLength: 152 } ] }); @@ -665,19 +704,21 @@ describe("glTF scene root structure", function () { expect(skins[0].rootBone).to.equal(defaultSceneRoot); }); - it("Multi-root scenes whose joints converge to a single top-level root should not use the scene wrapper", async () => { + it("Skinned mesh bounds should stay in rootBone space when inferred rootBone is outside joints", async () => { const glTFResource: GLTFResource = await engine.resourceManager.load({ type: AssetType.GLTF, - url: "mock/path/testSingleSkeleton.gltf" + url: "mock/path/testSkinRootBounds.gltf" }); const { defaultSceneRoot, skins } = glTFResource; - - expect(defaultSceneRoot.name).to.equal("GLTF_ROOT"); - // Scene has two top-level roots, but all joints converge to "Character_Root". - expect(defaultSceneRoot.children.length).to.equal(2); - expect(skins[0].rootBone).to.not.equal(defaultSceneRoot); - // rootBone should be inside the Character_Root subtree (LCA = Character_Root). - expect(skins[0].rootBone.name).to.equal("Character_Root"); + const characterGroup = defaultSceneRoot.children[0]; + const characterMesh = characterGroup.children[0]; + const renderer = characterMesh.getComponent(SkinnedMeshRenderer); + + expect(skins[0].rootBone).to.equal(characterGroup); + expect(renderer.localBounds.min.x).to.be.closeTo(6, 1e-5); + expect(renderer.localBounds.max.x).to.be.closeTo(8, 1e-5); + expect(renderer.bounds.min.x).to.be.closeTo(9, 1e-5); + expect(renderer.bounds.max.x).to.be.closeTo(11, 1e-5); }); }); From 227278bb26c3f9094b396725b9e6ea9b43be36ab Mon Sep 17 00:00:00 2001 From: luzhuang Date: Thu, 11 Jun 2026 16:11:13 +0800 Subject: [PATCH 02/17] fix(loader): keep material remap ref schema shared --- packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts b/packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts index 126d3cab2b..522876e53b 100644 --- a/packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts +++ b/packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts @@ -1,4 +1,5 @@ import type { IMaterialNormalTextureInfo, ITextureInfo } from "../GLTFSchema"; +import type { RefItem } from "../../schema/CommonSchema"; /** * Interfaces from the KHR_lights_punctual extension @@ -152,9 +153,7 @@ export interface IEXTMeshoptCompressionSchema { filter?: "NONE" | "OCTAHEDRAL" | "QUATERNION" | "EXPONENTIAL"; } -export interface IGalaceanMaterialRemap { - url: string; - key?: string; +export interface IGalaceanMaterialRemap extends RefItem { isClone?: boolean; } From 010262e7c247ac8028deac30100d0516dee10419 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Thu, 11 Jun 2026 16:17:16 +0800 Subject: [PATCH 03/17] fix(loader): tighten gltf skin root cleanup --- packages/loader/src/gltf/parser/GLTFSceneParser.ts | 1 - packages/loader/src/gltf/parser/GLTFSkinParser.ts | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/loader/src/gltf/parser/GLTFSceneParser.ts b/packages/loader/src/gltf/parser/GLTFSceneParser.ts index 553bf177f2..39fe207491 100644 --- a/packages/loader/src/gltf/parser/GLTFSceneParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSceneParser.ts @@ -38,7 +38,6 @@ export class GLTFSceneParser extends GLTFParser { sceneRoot.addChild(context.get(GLTFParserType.Entity, sceneNodes[i])); } - (glTFResource._sceneRoots ||= [])[index] = sceneRoot; if (isDefaultScene) { glTFResource._defaultSceneRoot = sceneRoot; } diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.ts index 3f11f487e5..5bbc6881cb 100644 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.ts @@ -37,6 +37,9 @@ export class GLTFSkinParser extends GLTFParser { // Get skeleton if (skeleton !== undefined) { const rootBone = entities[skeleton]; + if (!rootBone) { + throw `Skin skeleton index ${skeleton} is out of range.`; + } skin.rootBone = rootBone; } else { const rootBone = this._findSkinRootBoneByLCA(index, joints, entities, glTF.nodes); From 884b85726aba4e3e10fb948563f24064039a48f6 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Thu, 11 Jun 2026 16:25:03 +0800 Subject: [PATCH 04/17] test(loader): tighten gltf skin root coverage --- tests/src/loader/GLTFLoader.test.ts | 70 ++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/tests/src/loader/GLTFLoader.test.ts b/tests/src/loader/GLTFLoader.test.ts index 164dd788aa..bcbf29d575 100644 --- a/tests/src/loader/GLTFLoader.test.ts +++ b/tests/src/loader/GLTFLoader.test.ts @@ -26,7 +26,7 @@ import { registerGLTFParser } from "@galacean/engine-loader"; import { Color } from "@galacean/engine-math"; -import { WebGLEngine } from "@galacean/engine-rhi-webgl"; +import { WebGLEngine } from "@galacean/engine"; import { describe, beforeAll, afterAll, expect, it } from "vitest"; let engine: WebGLEngine; @@ -94,6 +94,60 @@ beforeAll(async function () { }); } + if (context.glTFResource.url.endsWith("testSingleSkeleton.gltf")) { + context.buffers = [new ArrayBuffer(128)]; + return Promise.resolve({ + asset: { + version: "2.0" + }, + scene: 0, + scenes: [ + { + nodes: [0, 2] + } + ], + nodes: [ + { + name: "Character_Root", + children: [1] + }, + { + name: "mixamorig:Hips" + }, + { + name: "Light" + } + ], + skins: [ + { + inverseBindMatrices: 0, + joints: [0, 1] + } + ], + accessors: [ + { + bufferView: 0, + byteOffset: 0, + componentType: 5126, + count: 2, + type: "MAT4" + } + ], + bufferViews: [ + { + buffer: 0, + byteOffset: 0, + byteLength: 128 + } + ], + buffers: [ + { + byteLength: 128 + } + ] + }); + } + if (context.glTFResource.url.endsWith("testSkinRootBounds.gltf")) { const buffer = new ArrayBuffer(152); const floats = new Float32Array(buffer); @@ -704,6 +758,19 @@ describe("glTF scene root structure", function () { expect(skins[0].rootBone).to.equal(defaultSceneRoot); }); + it("Multi-root scenes whose joints converge to a single top-level root should not use the scene wrapper", async () => { + const glTFResource: GLTFResource = await engine.resourceManager.load({ + type: AssetType.GLTF, + url: "mock/path/testSingleSkeleton.gltf" + }); + const { defaultSceneRoot, skins } = glTFResource; + + expect(defaultSceneRoot.name).to.equal("GLTF_ROOT"); + expect(defaultSceneRoot.children.length).to.equal(2); + expect(skins[0].rootBone).to.not.equal(defaultSceneRoot); + expect(skins[0].rootBone.name).to.equal("Character_Root"); + }); + it("Skinned mesh bounds should stay in rootBone space when inferred rootBone is outside joints", async () => { const glTFResource: GLTFResource = await engine.resourceManager.load({ type: AssetType.GLTF, @@ -715,6 +782,7 @@ describe("glTF scene root structure", function () { const renderer = characterMesh.getComponent(SkinnedMeshRenderer); expect(skins[0].rootBone).to.equal(characterGroup); + expect(renderer).to.exist; expect(renderer.localBounds.min.x).to.be.closeTo(6, 1e-5); expect(renderer.localBounds.max.x).to.be.closeTo(8, 1e-5); expect(renderer.bounds.min.x).to.be.closeTo(9, 1e-5); From 02542af8869699b1588bb38ed63e73904fdf19d5 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Thu, 11 Jun 2026 16:57:10 +0800 Subject: [PATCH 05/17] fix(loader): infer gltf skin root from joints only --- ...2026-06-11-gltf-skin-rootbone-visual-ci.md | 44 +++++++++++++++++++ .../src/gltf/parser/GLTFSkinParser.test.ts | 18 ++------ .../loader/src/gltf/parser/GLTFSkinParser.ts | 18 ++------ tests/src/loader/GLTFLoader.test.ts | 8 ++-- 4 files changed, 56 insertions(+), 32 deletions(-) create mode 100644 notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md diff --git a/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md b/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md new file mode 100644 index 0000000000..b7dff915fc --- /dev/null +++ b/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md @@ -0,0 +1,44 @@ +# glTF skin rootBone visual CI regression + +## Symptom + +PR #3027 failed e2e visual checks in Animator, Shadow, Camera, and Material cases with tiny image diffs. +Local reproduction on `fix/gltf-loader-shaderlab-split` initially failed because `odiff-bin` postinstall had not linked the binary; after `pnpm rebuild odiff-bin`, the same visual diffs reproduced. + +Representative local failures before the fix: + +- `Animator/additive`: `0.0015625%` +- `Animator/crossfade`: `0.00489583333333%` +- `Animator/play`: `0.0028125%` +- `Animator/playBackWards`: `0.00104166666667%` +- `Animator/stateMachine`: `0.00375%` +- `Shadow/basic`: `0.0132291666667%` +- `Camera/opaqueTexture`: `0.00135416666667%` + +## Root Cause + +`GLTFSkinParser` started including nodes that reference a skin (`node.skin === skinIndex`) when inferring `skin.rootBone` for glTF skins with no explicit `skin.skeleton`. + +For the shared Mixamo GLB used by the failing e2e cases: + +- `skin.skeleton` is absent. +- joints-only LCA is `mixamorig:Hips`. +- joints plus skinned mesh nodes LCA is `Armature`. +- `Armature` has scale `0.01`; the skinned mesh nodes are siblings of `mixamorig:Hips`. + +`Skin.rootBone` is not only a bounds hint. Setting it changes `SkinnedMeshRenderer._transformEntity`, feeds renderer transform matrices, and changes `Skin._updateSkinMatrices()` through `rootBone.getInvModelMatrix()`. Promoting rootBone from `mixamorig:Hips` to `Armature` changed the rendered coordinate space enough to trigger strict visual snapshots. + +## Fix + +Infer missing `skin.skeleton` from joints only. Nodes that use the skin are mesh owners, not skeleton joints, and must not participate in skeleton root inference. + +Keep the separate bounds behavior for explicit root bones outside the joint list: `GLTFSceneParser` still transforms mesh bounds into explicit rootBone space. + +## Verification + +- `pnpm vitest run tests/src/loader/GLTFLoader.test.ts --testNamePattern "Multi-root skins without skeleton"` failed before the fix with `skins[0].rootBone === defaultSceneRoot`. +- `npm run build` passed. +- `pnpm vitest run packages/loader/src/gltf/parser/GLTFSkinParser.test.ts` passed. +- `pnpm vitest run tests/src/loader/GLTFLoader.test.ts --testNamePattern "Multi-root skins without skeleton|Skinned mesh bounds"` passed. +- `CI=true PLAYWRIGHT_FORCE_TTY=1 npx playwright test --grep "(Animator.*(additive|crossfade|play$|playBackWards|stateMachine$)|Material.*unlit|Shadow.*basic|Camera.*opaqueTexture)" --reporter=list,github` passed: 8/8. + diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts index bb5714b847..d882b98174 100644 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts @@ -7,21 +7,16 @@ describe("GLTFSkinParser rootBone resolution", () => { return new GLTFSkinParser(); } - it("includes skinned mesh nodes when resolving missing skin.skeleton", async () => { + it("ignores skinned mesh nodes when resolving missing skin.skeleton", async () => { const parser = await createParser(); const sceneRoot = { name: "GLTF_ROOT" }; const meshRoot = { name: "Character_Man", parent: sceneRoot }; const hips = { name: "mixamorig:Hips", parent: sceneRoot }; const spine = { name: "mixamorig:Spine", parent: hips }; - const rootBone = (parser as any)._findSkinRootBoneByLCA( - 0, - [1, 2], - [meshRoot, hips, spine], - [{ name: "Character_Man", skin: 0 }, { name: "mixamorig:Hips" }, { name: "mixamorig:Spine" }] - ); + const rootBone = (parser as any)._findSkinRootBoneByLCA([1, 2], [meshRoot, hips, spine]); - expect(rootBone).toBe(sceneRoot); + expect(rootBone).toBe(hips); }); it("does not promote to the scene wrapper for unrelated top-level siblings", async () => { @@ -32,12 +27,7 @@ describe("GLTFSkinParser rootBone resolution", () => { const hips = { name: "mixamorig:Hips", parent: characterRoot }; const light = { name: "Light", parent: sceneRoot }; - const rootBone = (parser as any)._findSkinRootBoneByLCA( - 0, - [3], - [characterRoot, mesh, light, hips], - [{ name: "Character_Root" }, { name: "Character_Mesh", skin: 0 }, { name: "Light" }, { name: "mixamorig:Hips" }] - ); + const rootBone = (parser as any)._findSkinRootBoneByLCA([0, 3], [characterRoot, mesh, light, hips]); expect(rootBone).toBe(characterRoot); }); diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.ts index 5bbc6881cb..ce2fcc2036 100644 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.ts @@ -42,7 +42,7 @@ export class GLTFSkinParser extends GLTFParser { } skin.rootBone = rootBone; } else { - const rootBone = this._findSkinRootBoneByLCA(index, joints, entities, glTF.nodes); + const rootBone = this._findSkinRootBoneByLCA(joints, entities); if (rootBone) { skin.rootBone = rootBone; } else { @@ -56,20 +56,8 @@ export class GLTFSkinParser extends GLTFParser { return AssetPromise.resolve(skinPromise); } - private _findSkinRootBoneByLCA( - skinIndex: number, - joints: number[], - entities: Entity[], - nodes: Array<{ skin?: number }> = [] - ): Entity | null { - const nodeIndices = joints.slice(); - for (let i = 0, n = nodes.length; i < n; i++) { - if (nodes[i]?.skin === skinIndex) { - nodeIndices.push(i); - } - } - - return this._findRootBoneByLCA(nodeIndices, entities); + private _findSkinRootBoneByLCA(joints: number[], entities: Entity[]): Entity | null { + return this._findRootBoneByLCA(joints, entities); } private _findRootBoneByLCA(nodeIndices: number[], entities: Entity[]): Entity | null { diff --git a/tests/src/loader/GLTFLoader.test.ts b/tests/src/loader/GLTFLoader.test.ts index bcbf29d575..7d235b24d6 100644 --- a/tests/src/loader/GLTFLoader.test.ts +++ b/tests/src/loader/GLTFLoader.test.ts @@ -190,6 +190,7 @@ beforeAll(async function () { skins: [ { inverseBindMatrices: 0, + skeleton: 0, joints: [2, 3] } ], @@ -746,7 +747,7 @@ describe("glTF scene root structure", function () { expect(defaultSceneRoot.children[0].name).to.equal("entity1"); }); - it("Multi-root skins without skeleton should use the scene wrapper as rootBone", async () => { + it("Multi-root skins without skeleton should use the joints LCA as rootBone", async () => { const glTFResource: GLTFResource = await engine.resourceManager.load({ type: AssetType.GLTF, url: "mock/path/testSkinRoot.gltf" @@ -755,7 +756,8 @@ describe("glTF scene root structure", function () { expect(defaultSceneRoot.name).to.equal("GLTF_ROOT"); expect(defaultSceneRoot.children.length).to.equal(2); - expect(skins[0].rootBone).to.equal(defaultSceneRoot); + expect(skins[0].rootBone).to.not.equal(defaultSceneRoot); + expect(skins[0].rootBone.name).to.equal("mixamorig:Hips"); }); it("Multi-root scenes whose joints converge to a single top-level root should not use the scene wrapper", async () => { @@ -771,7 +773,7 @@ describe("glTF scene root structure", function () { expect(skins[0].rootBone.name).to.equal("Character_Root"); }); - it("Skinned mesh bounds should stay in rootBone space when inferred rootBone is outside joints", async () => { + it("Skinned mesh bounds should stay in rootBone space when explicit rootBone is outside joints", async () => { const glTFResource: GLTFResource = await engine.resourceManager.load({ type: AssetType.GLTF, url: "mock/path/testSkinRootBounds.gltf" From 0c3b250f41a8b3e1d125b93f2452ed282e306396 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Thu, 11 Jun 2026 17:27:22 +0800 Subject: [PATCH 06/17] test(loader): keep gltf skin tests in loader suite --- ...2026-06-11-gltf-skin-rootbone-visual-ci.md | 2 -- .../src/gltf/parser/GLTFSkinParser.test.ts | 34 ------------------- tests/src/loader/GLTFLoader.test.ts | 3 +- 3 files changed, 1 insertion(+), 38 deletions(-) delete mode 100644 packages/loader/src/gltf/parser/GLTFSkinParser.test.ts diff --git a/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md b/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md index b7dff915fc..d21b4a284d 100644 --- a/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md +++ b/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md @@ -38,7 +38,5 @@ Keep the separate bounds behavior for explicit root bones outside the joint list - `pnpm vitest run tests/src/loader/GLTFLoader.test.ts --testNamePattern "Multi-root skins without skeleton"` failed before the fix with `skins[0].rootBone === defaultSceneRoot`. - `npm run build` passed. -- `pnpm vitest run packages/loader/src/gltf/parser/GLTFSkinParser.test.ts` passed. - `pnpm vitest run tests/src/loader/GLTFLoader.test.ts --testNamePattern "Multi-root skins without skeleton|Skinned mesh bounds"` passed. - `CI=true PLAYWRIGHT_FORCE_TTY=1 npx playwright test --grep "(Animator.*(additive|crossfade|play$|playBackWards|stateMachine$)|Material.*unlit|Shadow.*basic|Camera.*opaqueTexture)" --reporter=list,github` passed: 8/8. - diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts deleted file mode 100644 index d882b98174..0000000000 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { describe, expect, it } from "vitest"; - -describe("GLTFSkinParser rootBone resolution", () => { - async function createParser(): Promise { - (globalThis as any).window = { AudioContext: undefined, TextMetrics: undefined }; - const { GLTFSkinParser } = await import("./GLTFSkinParser"); - return new GLTFSkinParser(); - } - - it("ignores skinned mesh nodes when resolving missing skin.skeleton", async () => { - const parser = await createParser(); - const sceneRoot = { name: "GLTF_ROOT" }; - const meshRoot = { name: "Character_Man", parent: sceneRoot }; - const hips = { name: "mixamorig:Hips", parent: sceneRoot }; - const spine = { name: "mixamorig:Spine", parent: hips }; - - const rootBone = (parser as any)._findSkinRootBoneByLCA([1, 2], [meshRoot, hips, spine]); - - expect(rootBone).toBe(hips); - }); - - it("does not promote to the scene wrapper for unrelated top-level siblings", async () => { - const parser = await createParser(); - const sceneRoot = { name: "GLTF_ROOT" }; - const characterRoot = { name: "Character_Root", parent: sceneRoot }; - const mesh = { name: "Character_Mesh", parent: characterRoot }; - const hips = { name: "mixamorig:Hips", parent: characterRoot }; - const light = { name: "Light", parent: sceneRoot }; - - const rootBone = (parser as any)._findSkinRootBoneByLCA([0, 3], [characterRoot, mesh, light, hips]); - - expect(rootBone).toBe(characterRoot); - }); -}); diff --git a/tests/src/loader/GLTFLoader.test.ts b/tests/src/loader/GLTFLoader.test.ts index 7d235b24d6..f59982fa22 100644 --- a/tests/src/loader/GLTFLoader.test.ts +++ b/tests/src/loader/GLTFLoader.test.ts @@ -53,8 +53,7 @@ beforeAll(async function () { ], nodes: [ { - name: "Character_Man", - skin: 0 + name: "Character_Man" }, { name: "mixamorig:Hips", From 56cf6ba1d39c6b2c19c99ff64bb3468b12a066a0 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Thu, 11 Jun 2026 17:35:57 +0800 Subject: [PATCH 07/17] test(loader): cover gltf skin parser outside src --- ...2026-06-11-gltf-skin-rootbone-visual-ci.md | 1 + packages/loader/tests/GLTFSkinParser.test.ts | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 packages/loader/tests/GLTFSkinParser.test.ts diff --git a/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md b/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md index d21b4a284d..7a8264e83e 100644 --- a/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md +++ b/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md @@ -38,5 +38,6 @@ Keep the separate bounds behavior for explicit root bones outside the joint list - `pnpm vitest run tests/src/loader/GLTFLoader.test.ts --testNamePattern "Multi-root skins without skeleton"` failed before the fix with `skins[0].rootBone === defaultSceneRoot`. - `npm run build` passed. +- `pnpm vitest run packages/loader/tests/GLTFSkinParser.test.ts` passed. - `pnpm vitest run tests/src/loader/GLTFLoader.test.ts --testNamePattern "Multi-root skins without skeleton|Skinned mesh bounds"` passed. - `CI=true PLAYWRIGHT_FORCE_TTY=1 npx playwright test --grep "(Animator.*(additive|crossfade|play$|playBackWards|stateMachine$)|Material.*unlit|Shadow.*basic|Camera.*opaqueTexture)" --reporter=list,github` passed: 8/8. diff --git a/packages/loader/tests/GLTFSkinParser.test.ts b/packages/loader/tests/GLTFSkinParser.test.ts new file mode 100644 index 0000000000..84efa8e96a --- /dev/null +++ b/packages/loader/tests/GLTFSkinParser.test.ts @@ -0,0 +1,48 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +describe("GLTFSkinParser rootBone resolution", () => { + let originalWindow: any; + + beforeEach(() => { + originalWindow = (globalThis as any).window; + (globalThis as any).window = { AudioContext: undefined, TextMetrics: undefined }; + }); + + afterEach(() => { + if (originalWindow === undefined) { + delete (globalThis as any).window; + } else { + (globalThis as any).window = originalWindow; + } + }); + + async function createParser(): Promise { + const { GLTFSkinParser } = await import("../src/gltf/parser/GLTFSkinParser"); + return new GLTFSkinParser(); + } + + it("ignores skinned mesh nodes when resolving missing skin.skeleton", async () => { + const parser = await createParser(); + const sceneRoot = { name: "GLTF_ROOT" }; + const meshRoot = { name: "Character_Man", parent: sceneRoot }; + const hips = { name: "mixamorig:Hips", parent: sceneRoot }; + const spine = { name: "mixamorig:Spine", parent: hips }; + + const rootBone = (parser as any)._findSkinRootBoneByLCA([1, 2], [meshRoot, hips, spine]); + + expect(rootBone).toBe(hips); + }); + + it("does not promote to the scene wrapper for unrelated top-level siblings", async () => { + const parser = await createParser(); + const sceneRoot = { name: "GLTF_ROOT" }; + const characterRoot = { name: "Character_Root", parent: sceneRoot }; + const mesh = { name: "Character_Mesh", parent: characterRoot }; + const hips = { name: "mixamorig:Hips", parent: characterRoot }; + const light = { name: "Light", parent: sceneRoot }; + + const rootBone = (parser as any)._findSkinRootBoneByLCA([0, 3], [characterRoot, mesh, light, hips]); + + expect(rootBone).toBe(characterRoot); + }); +}); From d0bb6067076ead84fc83658e73757aef9ee6edfe Mon Sep 17 00:00:00 2001 From: luzhuang Date: Thu, 11 Jun 2026 21:06:48 +0800 Subject: [PATCH 08/17] docs(loader): clarify gltf skin root fallback --- packages/loader/src/gltf/parser/GLTFSceneParser.ts | 2 ++ packages/loader/src/gltf/parser/GLTFSkinParser.ts | 4 +++- tests/src/loader/GLTFLoader.test.ts | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/loader/src/gltf/parser/GLTFSceneParser.ts b/packages/loader/src/gltf/parser/GLTFSceneParser.ts index 39fe207491..d9ebbd1383 100644 --- a/packages/loader/src/gltf/parser/GLTFSceneParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSceneParser.ts @@ -195,6 +195,8 @@ export class GLTFSceneParser extends GLTFParser { if (rootBoneIndex !== -1) { BoundingBox.transform(mesh.bounds, inverseBindMatrices[rootBoneIndex], skinnedMeshRenderer.localBounds); } else { + // rootBone is outside the joints list, so no inverse bind matrix exists + // for it; map mesh bounds into rootBone space with the loaded transform const inverseRootBoneWorld = new Matrix(); Matrix.invert(rootBone.transform.worldMatrix, inverseRootBoneWorld); BoundingBox.transform(mesh.bounds, inverseRootBoneWorld, skinnedMeshRenderer.localBounds); diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.ts index ce2fcc2036..d6a04a9859 100644 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.ts @@ -34,7 +34,8 @@ export class GLTFSkinParser extends GLTFParser { } skin.bones = bones; - // Get skeleton + // Respect explicit skeleton root. If missing, infer only from the joint + // hierarchy; mesh nodes using the skin are owners, not skeleton roots if (skeleton !== undefined) { const rootBone = entities[skeleton]; if (!rootBone) { @@ -60,6 +61,7 @@ export class GLTFSkinParser extends GLTFParser { return this._findRootBoneByLCA(joints, entities); } + /** Resolves missing skin.skeleton from the joints' lowest common ancestor. */ private _findRootBoneByLCA(nodeIndices: number[], entities: Entity[]): Entity | null { const paths: Entity[][] = []; for (const index of nodeIndices) { diff --git a/tests/src/loader/GLTFLoader.test.ts b/tests/src/loader/GLTFLoader.test.ts index f59982fa22..f7e27594b7 100644 --- a/tests/src/loader/GLTFLoader.test.ts +++ b/tests/src/loader/GLTFLoader.test.ts @@ -150,8 +150,8 @@ beforeAll(async function () { if (context.glTFResource.url.endsWith("testSkinRootBounds.gltf")) { const buffer = new ArrayBuffer(152); const floats = new Float32Array(buffer); - // Inverse bind matrices for Hips and Spine. Their bind pose world x is - // Character_Group(3) + Hips(10), so inverse bind translates by -13. + // Inverse bind matrices for Hips and Spine; their bind pose world x is + // Character_Group(3) + Hips(10), so inverse bind translates by -13 floats.set([1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, -13, 0, 0, 1], 0); floats.set([1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, -13, 0, 0, 1], 16); floats.set([9, -1, -1, 11, 1, 1], 32); From 68915d6be2c14b6d5f11c783f212bb8e71867dd6 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Fri, 12 Jun 2026 10:28:26 +0800 Subject: [PATCH 09/17] refactor(loader): clean gltf skin root fallback --- ...2026-06-11-gltf-skin-rootbone-visual-ci.md | 43 ----------------- .../loader/src/gltf/parser/GLTFSkinParser.ts | 19 ++------ packages/loader/tests/GLTFSkinParser.test.ts | 48 ------------------- 3 files changed, 4 insertions(+), 106 deletions(-) delete mode 100644 notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md delete mode 100644 packages/loader/tests/GLTFSkinParser.test.ts diff --git a/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md b/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md deleted file mode 100644 index 7a8264e83e..0000000000 --- a/notes/loader/2026-06-11-gltf-skin-rootbone-visual-ci.md +++ /dev/null @@ -1,43 +0,0 @@ -# glTF skin rootBone visual CI regression - -## Symptom - -PR #3027 failed e2e visual checks in Animator, Shadow, Camera, and Material cases with tiny image diffs. -Local reproduction on `fix/gltf-loader-shaderlab-split` initially failed because `odiff-bin` postinstall had not linked the binary; after `pnpm rebuild odiff-bin`, the same visual diffs reproduced. - -Representative local failures before the fix: - -- `Animator/additive`: `0.0015625%` -- `Animator/crossfade`: `0.00489583333333%` -- `Animator/play`: `0.0028125%` -- `Animator/playBackWards`: `0.00104166666667%` -- `Animator/stateMachine`: `0.00375%` -- `Shadow/basic`: `0.0132291666667%` -- `Camera/opaqueTexture`: `0.00135416666667%` - -## Root Cause - -`GLTFSkinParser` started including nodes that reference a skin (`node.skin === skinIndex`) when inferring `skin.rootBone` for glTF skins with no explicit `skin.skeleton`. - -For the shared Mixamo GLB used by the failing e2e cases: - -- `skin.skeleton` is absent. -- joints-only LCA is `mixamorig:Hips`. -- joints plus skinned mesh nodes LCA is `Armature`. -- `Armature` has scale `0.01`; the skinned mesh nodes are siblings of `mixamorig:Hips`. - -`Skin.rootBone` is not only a bounds hint. Setting it changes `SkinnedMeshRenderer._transformEntity`, feeds renderer transform matrices, and changes `Skin._updateSkinMatrices()` through `rootBone.getInvModelMatrix()`. Promoting rootBone from `mixamorig:Hips` to `Armature` changed the rendered coordinate space enough to trigger strict visual snapshots. - -## Fix - -Infer missing `skin.skeleton` from joints only. Nodes that use the skin are mesh owners, not skeleton joints, and must not participate in skeleton root inference. - -Keep the separate bounds behavior for explicit root bones outside the joint list: `GLTFSceneParser` still transforms mesh bounds into explicit rootBone space. - -## Verification - -- `pnpm vitest run tests/src/loader/GLTFLoader.test.ts --testNamePattern "Multi-root skins without skeleton"` failed before the fix with `skins[0].rootBone === defaultSceneRoot`. -- `npm run build` passed. -- `pnpm vitest run packages/loader/tests/GLTFSkinParser.test.ts` passed. -- `pnpm vitest run tests/src/loader/GLTFLoader.test.ts --testNamePattern "Multi-root skins without skeleton|Skinned mesh bounds"` passed. -- `CI=true PLAYWRIGHT_FORCE_TTY=1 npx playwright test --grep "(Animator.*(additive|crossfade|play$|playBackWards|stateMachine$)|Material.*unlit|Shadow.*basic|Camera.*opaqueTexture)" --reporter=list,github` passed: 8/8. diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.ts index d6a04a9859..840c014cc8 100644 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.ts @@ -57,28 +57,17 @@ export class GLTFSkinParser extends GLTFParser { return AssetPromise.resolve(skinPromise); } - private _findSkinRootBoneByLCA(joints: number[], entities: Entity[]): Entity | null { - return this._findRootBoneByLCA(joints, entities); - } - /** Resolves missing skin.skeleton from the joints' lowest common ancestor. */ - private _findRootBoneByLCA(nodeIndices: number[], entities: Entity[]): Entity | null { - const paths: Entity[][] = []; - for (const index of nodeIndices) { + private _findSkinRootBoneByLCA(joints: number[], entities: Entity[]): Entity | null { + const paths = joints.map((index) => { const path = new Array(); let entity = entities[index]; while (entity) { path.unshift(entity); entity = entity.parent; } - if (path.length) { - paths.push(path); - } - } - - if (!paths.length) { - return null; - } + return path; + }); let rootNode: Entity | null = null; for (let i = 0; ; i++) { diff --git a/packages/loader/tests/GLTFSkinParser.test.ts b/packages/loader/tests/GLTFSkinParser.test.ts deleted file mode 100644 index 84efa8e96a..0000000000 --- a/packages/loader/tests/GLTFSkinParser.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it } from "vitest"; - -describe("GLTFSkinParser rootBone resolution", () => { - let originalWindow: any; - - beforeEach(() => { - originalWindow = (globalThis as any).window; - (globalThis as any).window = { AudioContext: undefined, TextMetrics: undefined }; - }); - - afterEach(() => { - if (originalWindow === undefined) { - delete (globalThis as any).window; - } else { - (globalThis as any).window = originalWindow; - } - }); - - async function createParser(): Promise { - const { GLTFSkinParser } = await import("../src/gltf/parser/GLTFSkinParser"); - return new GLTFSkinParser(); - } - - it("ignores skinned mesh nodes when resolving missing skin.skeleton", async () => { - const parser = await createParser(); - const sceneRoot = { name: "GLTF_ROOT" }; - const meshRoot = { name: "Character_Man", parent: sceneRoot }; - const hips = { name: "mixamorig:Hips", parent: sceneRoot }; - const spine = { name: "mixamorig:Spine", parent: hips }; - - const rootBone = (parser as any)._findSkinRootBoneByLCA([1, 2], [meshRoot, hips, spine]); - - expect(rootBone).toBe(hips); - }); - - it("does not promote to the scene wrapper for unrelated top-level siblings", async () => { - const parser = await createParser(); - const sceneRoot = { name: "GLTF_ROOT" }; - const characterRoot = { name: "Character_Root", parent: sceneRoot }; - const mesh = { name: "Character_Mesh", parent: characterRoot }; - const hips = { name: "mixamorig:Hips", parent: characterRoot }; - const light = { name: "Light", parent: sceneRoot }; - - const rootBone = (parser as any)._findSkinRootBoneByLCA([0, 3], [characterRoot, mesh, light, hips]); - - expect(rootBone).toBe(characterRoot); - }); -}); From da98cdaa8a3e5a9fe3c0b0fb5cc19d1083320def Mon Sep 17 00:00:00 2001 From: luzhuang Date: Fri, 12 Jun 2026 11:43:34 +0800 Subject: [PATCH 10/17] chore: ignore local notes --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index f6049b3640..c692c405e6 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,7 @@ tsconfig.tsbuildinfo api docs/api docs/plans +/notes/ yarn.lock e2e/videos/* e2e/screenshots/* @@ -47,4 +48,4 @@ CLAUDE.md # For bison generated files used by ShaderLab *.tab.c -*.output \ No newline at end of file +*.output From d08cbaa9f2e1b03da9be4cd4fe21a9669ba67346 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Fri, 12 Jun 2026 11:52:15 +0800 Subject: [PATCH 11/17] refactor(loader): reduce gltf skin patch surface --- .../loader/src/gltf/parser/GLTFSceneParser.ts | 5 +---- .../loader/src/gltf/parser/GLTFSkinParser.ts | 18 +++++++++--------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/loader/src/gltf/parser/GLTFSceneParser.ts b/packages/loader/src/gltf/parser/GLTFSceneParser.ts index d9ebbd1383..5a360734a4 100644 --- a/packages/loader/src/gltf/parser/GLTFSceneParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSceneParser.ts @@ -195,10 +195,7 @@ export class GLTFSceneParser extends GLTFParser { if (rootBoneIndex !== -1) { BoundingBox.transform(mesh.bounds, inverseBindMatrices[rootBoneIndex], skinnedMeshRenderer.localBounds); } else { - // rootBone is outside the joints list, so no inverse bind matrix exists - // for it; map mesh bounds into rootBone space with the loaded transform - const inverseRootBoneWorld = new Matrix(); - Matrix.invert(rootBone.transform.worldMatrix, inverseRootBoneWorld); + const inverseRootBoneWorld = rootBone.transform.worldMatrix.clone().invert(); // rootBone can be outside skin.joints BoundingBox.transform(mesh.bounds, inverseRootBoneWorld, skinnedMeshRenderer.localBounds); } } diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.ts index 840c014cc8..d5bf283084 100644 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.ts @@ -44,11 +44,10 @@ export class GLTFSkinParser extends GLTFParser { skin.rootBone = rootBone; } else { const rootBone = this._findSkinRootBoneByLCA(joints, entities); - if (rootBone) { - skin.rootBone = rootBone; - } else { + if (!rootBone) { throw "Failed to find skeleton root bone."; } + skin.rootBone = rootBone; } return skin; @@ -59,26 +58,27 @@ export class GLTFSkinParser extends GLTFParser { /** Resolves missing skin.skeleton from the joints' lowest common ancestor. */ private _findSkinRootBoneByLCA(joints: number[], entities: Entity[]): Entity | null { - const paths = joints.map((index) => { + const paths = >{}; + for (const index of joints) { const path = new Array(); let entity = entities[index]; while (entity) { path.unshift(entity); entity = entity.parent; } - return path; - }); + paths[index] = path; + } let rootNode: Entity | null = null; for (let i = 0; ; i++) { - let path = paths[0]; + let path = paths[joints[0]]; if (i >= path.length) { return rootNode; } const entity = path[i]; - for (let j = 1, m = paths.length; j < m; j++) { - path = paths[j]; + for (let j = 1, m = joints.length; j < m; j++) { + path = paths[joints[j]]; if (i >= path.length || entity !== path[i]) { return rootNode; } From cc82ad9e347ed28559beeb131da4b2faea5d9733 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Fri, 12 Jun 2026 12:02:55 +0800 Subject: [PATCH 12/17] refactor(loader): drop redundant skin parser diff --- packages/loader/src/gltf/parser/GLTFSkinParser.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/loader/src/gltf/parser/GLTFSkinParser.ts b/packages/loader/src/gltf/parser/GLTFSkinParser.ts index d5bf283084..e8959b4b70 100644 --- a/packages/loader/src/gltf/parser/GLTFSkinParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSkinParser.ts @@ -34,8 +34,8 @@ export class GLTFSkinParser extends GLTFParser { } skin.bones = bones; - // Respect explicit skeleton root. If missing, infer only from the joint - // hierarchy; mesh nodes using the skin are owners, not skeleton roots + // Get skeleton — when `skin.skeleton` is absent, resolve via joints' LCA + // LCA falls back to the GLTF_ROOT wrapper only when joints span multiple top-level scene nodes if (skeleton !== undefined) { const rootBone = entities[skeleton]; if (!rootBone) { @@ -43,7 +43,7 @@ export class GLTFSkinParser extends GLTFParser { } skin.rootBone = rootBone; } else { - const rootBone = this._findSkinRootBoneByLCA(joints, entities); + const rootBone = this._findSkeletonRootBone(joints, entities); if (!rootBone) { throw "Failed to find skeleton root bone."; } @@ -56,8 +56,11 @@ export class GLTFSkinParser extends GLTFParser { return AssetPromise.resolve(skinPromise); } - /** Resolves missing skin.skeleton from the joints' lowest common ancestor. */ - private _findSkinRootBoneByLCA(joints: number[], entities: Entity[]): Entity | null { + /** + * Resolve the skeleton rootBone as the lowest common ancestor of the joints' parent chains. + * Returns null when joints share no common ancestor. + */ + private _findSkeletonRootBone(joints: number[], entities: Entity[]): Entity | null { const paths = >{}; for (const index of joints) { const path = new Array(); From 4f8bd8f98071d542a32c66e7267587c0c2704410 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Fri, 12 Jun 2026 12:13:35 +0800 Subject: [PATCH 13/17] test(loader): cover explicit gltf skin root bounds --- tests/src/loader/GLTFLoader.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/src/loader/GLTFLoader.test.ts b/tests/src/loader/GLTFLoader.test.ts index f7e27594b7..cf105fbf6e 100644 --- a/tests/src/loader/GLTFLoader.test.ts +++ b/tests/src/loader/GLTFLoader.test.ts @@ -21,6 +21,7 @@ import { GLTFParserContext, GLTFParserType, GLTFResource, + GLTFSceneParser, GLTFSchemaParser, registerGLTFExtension, registerGLTFParser @@ -784,6 +785,13 @@ describe("glTF scene root structure", function () { expect(skins[0].rootBone).to.equal(characterGroup); expect(renderer).to.exist; + (new GLTFSceneParser() as any)._computeLocalBounds( + renderer, + renderer.mesh, + skins[0].bones, + skins[0].rootBone, + skins[0].inverseBindMatrices + ); expect(renderer.localBounds.min.x).to.be.closeTo(6, 1e-5); expect(renderer.localBounds.max.x).to.be.closeTo(8, 1e-5); expect(renderer.bounds.min.x).to.be.closeTo(9, 1e-5); From 2e8696958328a698f05f4e759ba58edfebe1fbd7 Mon Sep 17 00:00:00 2001 From: luzhuang Date: Fri, 12 Jun 2026 12:19:08 +0800 Subject: [PATCH 14/17] test(loader): keep gltf bounds coverage public --- tests/src/loader/GLTFLoader.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/src/loader/GLTFLoader.test.ts b/tests/src/loader/GLTFLoader.test.ts index cf105fbf6e..f7e27594b7 100644 --- a/tests/src/loader/GLTFLoader.test.ts +++ b/tests/src/loader/GLTFLoader.test.ts @@ -21,7 +21,6 @@ import { GLTFParserContext, GLTFParserType, GLTFResource, - GLTFSceneParser, GLTFSchemaParser, registerGLTFExtension, registerGLTFParser @@ -785,13 +784,6 @@ describe("glTF scene root structure", function () { expect(skins[0].rootBone).to.equal(characterGroup); expect(renderer).to.exist; - (new GLTFSceneParser() as any)._computeLocalBounds( - renderer, - renderer.mesh, - skins[0].bones, - skins[0].rootBone, - skins[0].inverseBindMatrices - ); expect(renderer.localBounds.min.x).to.be.closeTo(6, 1e-5); expect(renderer.localBounds.max.x).to.be.closeTo(8, 1e-5); expect(renderer.bounds.min.x).to.be.closeTo(9, 1e-5); From fdae6cf08be369c5ea88a56e063e5bc3662a7f2c Mon Sep 17 00:00:00 2001 From: luzhuang Date: Fri, 12 Jun 2026 15:09:26 +0800 Subject: [PATCH 15/17] test(loader): cover scaled gltf skin root bounds --- tests/src/loader/GLTFLoader.test.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/src/loader/GLTFLoader.test.ts b/tests/src/loader/GLTFLoader.test.ts index f7e27594b7..85cceaa5d7 100644 --- a/tests/src/loader/GLTFLoader.test.ts +++ b/tests/src/loader/GLTFLoader.test.ts @@ -150,11 +150,11 @@ beforeAll(async function () { if (context.glTFResource.url.endsWith("testSkinRootBounds.gltf")) { const buffer = new ArrayBuffer(152); const floats = new Float32Array(buffer); - // Inverse bind matrices for Hips and Spine; their bind pose world x is - // Character_Group(3) + Hips(10), so inverse bind translates by -13 - floats.set([1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, -13, 0, 0, 1], 0); - floats.set([1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, -13, 0, 0, 1], 16); - floats.set([9, -1, -1, 11, 1, 1], 32); + // Inverse bind matrices for Hips and Spine; Character_Group scales by + // (2, 3, 1), then offsets Hips by 10 on x, so Hips world x is 23. + floats.set([0.5, 0, 0, 0, 0, 1 / 3, 0, 0, 0, 0, 1, 0, -11.5, 0, 0, 1], 0); + floats.set([0.5, 0, 0, 0, 0, 1 / 3, 0, 0, 0, 0, 1, 0, -11.5, 0, 0, 1], 16); + floats.set([9, -3, -1, 11, 3, 1], 32); context.buffers = [buffer]; return Promise.resolve({ asset: { @@ -170,6 +170,7 @@ beforeAll(async function () { { name: "Character_Group", translation: [3, 0, 0], + scale: [2, 3, 1], children: [1, 2] }, { @@ -219,8 +220,8 @@ beforeAll(async function () { componentType: 5126, count: 2, type: "VEC3", - min: [9, -1, -1], - max: [11, 1, 1] + min: [9, -3, -1], + max: [11, 3, 1] } ], bufferViews: [ @@ -772,7 +773,7 @@ describe("glTF scene root structure", function () { expect(skins[0].rootBone.name).to.equal("Character_Root"); }); - it("Skinned mesh bounds should stay in rootBone space when explicit rootBone is outside joints", async () => { + it("Skinned mesh bounds should stay in scaled rootBone space when explicit rootBone is outside joints", async () => { const glTFResource: GLTFResource = await engine.resourceManager.load({ type: AssetType.GLTF, url: "mock/path/testSkinRootBounds.gltf" @@ -784,10 +785,14 @@ describe("glTF scene root structure", function () { expect(skins[0].rootBone).to.equal(characterGroup); expect(renderer).to.exist; - expect(renderer.localBounds.min.x).to.be.closeTo(6, 1e-5); - expect(renderer.localBounds.max.x).to.be.closeTo(8, 1e-5); + expect(renderer.localBounds.min.x).to.be.closeTo(3, 1e-5); + expect(renderer.localBounds.max.x).to.be.closeTo(4, 1e-5); + expect(renderer.localBounds.min.y).to.be.closeTo(-1, 1e-5); + expect(renderer.localBounds.max.y).to.be.closeTo(1, 1e-5); expect(renderer.bounds.min.x).to.be.closeTo(9, 1e-5); expect(renderer.bounds.max.x).to.be.closeTo(11, 1e-5); + expect(renderer.bounds.min.y).to.be.closeTo(-3, 1e-5); + expect(renderer.bounds.max.y).to.be.closeTo(3, 1e-5); }); }); From 12757c378ebb0934e7f9c95b8e5eeab1f320c672 Mon Sep 17 00:00:00 2001 From: GuoLei1990 Date: Sat, 13 Jun 2026 20:31:36 +0800 Subject: [PATCH 16/17] chore: keep .gitignore out of this PR --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index c692c405e6..f6049b3640 100644 --- a/.gitignore +++ b/.gitignore @@ -27,7 +27,6 @@ tsconfig.tsbuildinfo api docs/api docs/plans -/notes/ yarn.lock e2e/videos/* e2e/screenshots/* @@ -48,4 +47,4 @@ CLAUDE.md # For bison generated files used by ShaderLab *.tab.c -*.output +*.output \ No newline at end of file From c3f279dfc88bf4f301ab32e11dc6b941261de0ed Mon Sep 17 00:00:00 2001 From: GuoLei1990 Date: Sun, 14 Jun 2026 01:05:25 +0800 Subject: [PATCH 17/17] refactor(loader): reuse static matrix for skin bounds inverse --- packages/loader/src/gltf/parser/GLTFSceneParser.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/loader/src/gltf/parser/GLTFSceneParser.ts b/packages/loader/src/gltf/parser/GLTFSceneParser.ts index 5a360734a4..a21528e1ae 100644 --- a/packages/loader/src/gltf/parser/GLTFSceneParser.ts +++ b/packages/loader/src/gltf/parser/GLTFSceneParser.ts @@ -18,6 +18,8 @@ import { GLTFParserContext, GLTFParserType, registerGLTFParser } from "./GLTFPar @registerGLTFParser(GLTFParserType.Scene) export class GLTFSceneParser extends GLTFParser { + private static _tempMatrix: Matrix = new Matrix(); + parse(context: GLTFParserContext, index: number): AssetPromise { const { glTF: { scenes, scene = 0 }, @@ -195,7 +197,9 @@ export class GLTFSceneParser extends GLTFParser { if (rootBoneIndex !== -1) { BoundingBox.transform(mesh.bounds, inverseBindMatrices[rootBoneIndex], skinnedMeshRenderer.localBounds); } else { - const inverseRootBoneWorld = rootBone.transform.worldMatrix.clone().invert(); // rootBone can be outside skin.joints + // rootBone can be outside skin.joints, so it has no inverse bind matrix + const inverseRootBoneWorld = GLTFSceneParser._tempMatrix; + Matrix.invert(rootBone.transform.worldMatrix, inverseRootBoneWorld); BoundingBox.transform(mesh.bounds, inverseRootBoneWorld, skinnedMeshRenderer.localBounds); } }