From 0689d9cc46ded0b5493ec6a5af063ecf377e415d Mon Sep 17 00:00:00 2001 From: 0b5vr <0b5vr@0b5vr.com> Date: Tue, 14 Jan 2025 18:08:03 +0900 Subject: [PATCH 1/2] feat: Fix combineSkeletons There is a model that uses the same skinIndex accessor but not the same skinWeight accessors between meshes This commit updates the implementation of attributeUsedIndexSetMap to use both skinIndex and skinWeight accesstors The same skinIndex accessor could also be used between different bone sets Clone the bone set if the skinIndex accessor is the same but the bone set is different --- .../src/VRMUtils/combineSkeletons.ts | 102 +++++++++++++++--- 1 file changed, 87 insertions(+), 15 deletions(-) diff --git a/packages/three-vrm/src/VRMUtils/combineSkeletons.ts b/packages/three-vrm/src/VRMUtils/combineSkeletons.ts index fcc3887d4..cf996c4b4 100644 --- a/packages/three-vrm/src/VRMUtils/combineSkeletons.ts +++ b/packages/three-vrm/src/VRMUtils/combineSkeletons.ts @@ -12,13 +12,21 @@ export function combineSkeletons(root: THREE.Object3D): void { const skinnedMeshes = collectSkinnedMeshes(root); // List all used skin indices for each skin index attribute - const attributeUsedIndexSetMap = new Map>(); + /** A map: skin index attribute -> skin weight attribute -> used index set */ + const attributeUsedIndexSetMap = new Map< + THREE.BufferAttribute | THREE.InterleavedBufferAttribute, + Map> + >(); for (const mesh of skinnedMeshes) { const geometry = mesh.geometry; + const skinIndexAttr = geometry.getAttribute('skinIndex'); + const skinIndexMap = attributeUsedIndexSetMap.get(skinIndexAttr) ?? new Map(); + attributeUsedIndexSetMap.set(skinIndexAttr, skinIndexMap); + const skinWeightAttr = geometry.getAttribute('skinWeight'); const usedIndicesSet = listUsedIndices(skinIndexAttr, skinWeightAttr); - attributeUsedIndexSetMap.set(skinIndexAttr, usedIndicesSet); + skinIndexMap.set(skinWeightAttr, usedIndicesSet); } // List all bones and boneInverses for each meshes @@ -63,18 +71,16 @@ export function combineSkeletons(root: THREE.Object3D): void { const newBoneInverses = Array.from(boneInverseMap.values()); const newSkeleton = new THREE.Skeleton(newBones, newBoneInverses); - const attributeProcessedSet = new Set(); - - for (const mesh of meshes) { - const attribute = mesh.geometry.getAttribute('skinIndex'); + // collect skin index attributes and corresponding bone arrays + const skinIndexBonesPairSet = collectSkinIndexAttrs(meshes); - if (!attributeProcessedSet.has(attribute)) { - // remap skin index attribute - remapSkinIndexAttribute(attribute, mesh.skeleton.bones, newBones); - attributeProcessedSet.add(attribute); - } + // remap skin index attribute + for (const [skinIndexAttr, bones] of skinIndexBonesPairSet) { + remapSkinIndexAttribute(skinIndexAttr, bones, newBones); + } - // bind the new skeleton to the mesh + // bind the new skeleton to the meshes + for (const mesh of meshes) { mesh.bind(newSkeleton, new THREE.Matrix4()); } } @@ -132,7 +138,10 @@ function listUsedIndices( */ function listUsedBones( mesh: THREE.SkinnedMesh, - attributeUsedIndexSetMap: Map>, + attributeUsedIndexSetMap: Map< + THREE.BufferAttribute | THREE.InterleavedBufferAttribute, + Map> + >, ): Map { const boneInverseMap = new Map(); @@ -140,10 +149,14 @@ function listUsedBones( const geometry = mesh.geometry; const skinIndexAttr = geometry.getAttribute('skinIndex'); - const usedIndicesSet = attributeUsedIndexSetMap.get(skinIndexAttr); + const skinWeightAttr = geometry.getAttribute('skinWeight'); + const skinIndexMap = attributeUsedIndexSetMap.get(skinIndexAttr); + const usedIndicesSet = skinIndexMap?.get(skinWeightAttr); if (!usedIndicesSet) { - throw new Error('Unreachable. attributeUsedIndexSetMap does not know the skin index attribute'); + throw new Error( + 'Unreachable. attributeUsedIndexSetMap does not know the skin index attribute or the skin weight attribute.', + ); } for (const index of usedIndicesSet) { @@ -222,3 +235,62 @@ function matrixEquals(a: THREE.Matrix4, b: THREE.Matrix4, tolerance?: number) { return true; } + +/** + * Check if the contents of two arrays are equal. + */ +function arrayEquals(a: T[], b: T[]): boolean { + if (a.length !== b.length) { + return false; + } + + return a.every((v, i) => v === b[i]); +} + +/** + * Collect skin index attributes and corresponding bone arrays from the given skinned meshes. + * If a skin index attribute is shared among different bone sets, clone the attribute. + */ +function collectSkinIndexAttrs( + meshes: Iterable, +): Set<[THREE.BufferAttribute | THREE.InterleavedBufferAttribute, THREE.Bone[]]> { + const skinIndexBonesPairSet = new Set<[THREE.BufferAttribute | THREE.InterleavedBufferAttribute, THREE.Bone[]]>(); + + // Collect skin index attributes + // skinIndex attribute might be shared among different bone sets + // If there are multiple bone sets that share the same skinIndex attribute, clone the attribute + const skinIndexNewSkinIndexBonesMapMap = new Map< + THREE.BufferAttribute | THREE.InterleavedBufferAttribute, + Map + >(); + + for (const mesh of meshes) { + let skinIndexAttr = mesh.geometry.getAttribute('skinIndex'); + + // Get or create a map for the skin index attribute + let newSkinIndexBonesMap = skinIndexNewSkinIndexBonesMapMap.get(skinIndexAttr); + if (newSkinIndexBonesMap == null) { + // Create a new map for the skin index attribute and register the bone array + newSkinIndexBonesMap = new Map(); + skinIndexNewSkinIndexBonesMapMap.set(skinIndexAttr, newSkinIndexBonesMap); + newSkinIndexBonesMap.set(skinIndexAttr, mesh.skeleton.bones); + skinIndexBonesPairSet.add([skinIndexAttr, mesh.skeleton.bones]); + continue; + } + + // Check if the bone set is already registered + for (const bones of newSkinIndexBonesMap.values()) { + if (arrayEquals(bones, mesh.skeleton.bones)) { + continue; + } + } + + // There is no matching bone set, so clone the skin index attribute + skinIndexAttr = skinIndexAttr.clone(); + mesh.geometry.setAttribute('skinIndex', skinIndexAttr); + newSkinIndexBonesMap.set(skinIndexAttr, mesh.skeleton.bones); + skinIndexBonesPairSet.add([skinIndexAttr, mesh.skeleton.bones]); + } + + return skinIndexBonesPairSet; +} From 902e6d83a4048a1e7a6bf4dfdb51252d7f8eb456 Mon Sep 17 00:00:00 2001 From: 0b5vr <0b5vr@0b5vr.com> Date: Wed, 15 Jan 2025 17:35:43 +0900 Subject: [PATCH 2/2] fix: Fix combineSkeletons, collectSkinIndexAttrs The continue statement was not working as intended I also forgot to set the new attribute when we found an already registered skin index attribute I had to change eslint `@typescript-eslint/naming-convention` rule in order to use `_` for parameter name See: https://typescript-eslint.io/rules/naming-convention/ --- .eslintrc.json | 2 +- .../src/VRMUtils/combineSkeletons.ts | 22 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index e348252ca..d709b5a12 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -27,7 +27,7 @@ "format": ["camelCase"] }, { - "selector": "variable", + "selector": "variableLike", "format": ["camelCase", "UPPER_CASE"], "leadingUnderscore": "allow" }, diff --git a/packages/three-vrm/src/VRMUtils/combineSkeletons.ts b/packages/three-vrm/src/VRMUtils/combineSkeletons.ts index cf996c4b4..bde4c9490 100644 --- a/packages/three-vrm/src/VRMUtils/combineSkeletons.ts +++ b/packages/three-vrm/src/VRMUtils/combineSkeletons.ts @@ -265,7 +265,7 @@ function collectSkinIndexAttrs( >(); for (const mesh of meshes) { - let skinIndexAttr = mesh.geometry.getAttribute('skinIndex'); + const skinIndexAttr = mesh.geometry.getAttribute('skinIndex'); // Get or create a map for the skin index attribute let newSkinIndexBonesMap = skinIndexNewSkinIndexBonesMapMap.get(skinIndexAttr); @@ -279,17 +279,19 @@ function collectSkinIndexAttrs( } // Check if the bone set is already registered - for (const bones of newSkinIndexBonesMap.values()) { - if (arrayEquals(bones, mesh.skeleton.bones)) { - continue; - } + // If the bone set is already registered, reuse the skin index attribute + let newSkinIndexAttr = Array.from(newSkinIndexBonesMap).find(([_, bones]) => + arrayEquals(bones, mesh.skeleton.bones), + )?.[0]; + + // If there is no matching bone set, clone the skin index attribute + if (newSkinIndexAttr == null) { + newSkinIndexAttr = skinIndexAttr.clone(); + newSkinIndexBonesMap.set(newSkinIndexAttr, mesh.skeleton.bones); + skinIndexBonesPairSet.add([newSkinIndexAttr, mesh.skeleton.bones]); } - // There is no matching bone set, so clone the skin index attribute - skinIndexAttr = skinIndexAttr.clone(); - mesh.geometry.setAttribute('skinIndex', skinIndexAttr); - newSkinIndexBonesMap.set(skinIndexAttr, mesh.skeleton.bones); - skinIndexBonesPairSet.add([skinIndexAttr, mesh.skeleton.bones]); + mesh.geometry.setAttribute('skinIndex', newSkinIndexAttr); } return skinIndexBonesPairSet;