From 055ad124d1860a0065979a2ded09464bdc8e5fda Mon Sep 17 00:00:00 2001 From: Takahiro Date: Fri, 21 Jul 2023 19:47:25 -0700 Subject: [PATCH] Fix bitECS loop-animation **Problem** loop-animation with bitECS may not work correctly if multiple loop-animation components use the same animation name to refer to glTF.animation(s). **Root issue** Mozilla Hubs loop-animation component has a problem in the spec. A loop-animation component can refer to glTF.animations with animation names but the glTF specification allows non-unique names in glTF.animations so if there are multiple glTF.animations that have the same name no one can know what glTF.animations a loop-animation using that names refers to. Refer to https://github.com/mozilla/hubs/pull/6153 for details. **Solution** Preprocesses glTF content before parsing to avoid the problem by adding a glTF loader plugin that converts name based animation reference in loop-animation component to index based. **Note** The old loop-animation component handler (without bitECS) seems to assume that multiple glTF.animations that have the same name have the same animation data and loop-animation refers to the first glTF.animation of the ones having the same name. The plugin follows the assumption for the compatibility, not sure if it is the best approach. With this commit we may remove name based animation reference processing codes from the loop-animation handlers but until we are confident for the change we may keep them. **Change** - Add a glTF loader plugin to convert name based animation reference in loop-animation component to index based. --- src/bit-systems/mixer-animatable.ts | 2 +- src/components/gltf-model-plus.js | 117 ++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/bit-systems/mixer-animatable.ts b/src/bit-systems/mixer-animatable.ts index 8609a277a1..48711af0eb 100644 --- a/src/bit-systems/mixer-animatable.ts +++ b/src/bit-systems/mixer-animatable.ts @@ -10,7 +10,7 @@ const mixerExitQuery = exitQuery(mixerQuery); export function mixerAnimatableSystem(world: HubsWorld): void { initializeEnterQuery(world).forEach(eid => { - if (entityExists(world, eid)) { + if (!entityExists(world, eid)) { console.warn("Skipping nonexistant entity."); // TODO Why does this happen? return; } diff --git a/src/components/gltf-model-plus.js b/src/components/gltf-model-plus.js index 426b480839..e2bec4d5bd 100644 --- a/src/components/gltf-model-plus.js +++ b/src/components/gltf-model-plus.js @@ -670,6 +670,122 @@ class GLTFMozTextureRGBE { } } +// Mozilla Hubs loop-animation component has a problem in the spec. +// The loop-animation component can refer to glTF.animations with +// animation names but the glTF specification allows non-unique names +// in glTF.animations so if there are multiple glTF.animations that +// have the same name no one can know what glTF.animations a +// loop-component using that names refers to. +// This plugin converts animation names in the component to +// animation indices to avoid the problem. +// The old loop-animation component handler (without bitECS) seems +// to assume that multiple glTF.animations that have the same name +// have the same animation data and loop-component refers to the +// first glTF.animation of the ones having the same name. This +// plugin follows the assumption for the compatibility. +// Refer to https://github.com/mozilla/hubs/pull/6153 for details. +// TODO: Deprecate the loop-animation animation reference with name +class GLTFHubsLoopAnimationComponent { + constructor(parser) { + this.parser = parser; + this.name = "MOZ_hubs_components.loop-animation"; + } + + beforeRoot() { + const json = this.parser.json; + + if (json.animations === undefined) { + return; + } + + // TODO: Optimize if needed + const findAnimation = name => { + for (let animationIndex = 0; animationIndex < json.animations.length; animationIndex++) { + const animationDef = json.animations[animationIndex]; + if (animationDef.name === name) { + return animationIndex; + } + } + return null; + }; + + // TODO: Optimize if needed + const collectNodeIndices = (nodeIndex, nodeIndices) => { + nodeIndices.add(nodeIndex); + const nodeDef = json.nodes[nodeIndex]; + + if (nodeDef.children !== undefined) { + for (const child of nodeDef.children) { + collectNodeIndices(child, nodeIndices); + } + } + + return nodeIndices; + }; + + const clonedAnimations = []; + + for (let nodeIndex = 0; nodeIndex < json.nodes.length; nodeIndex++) { + const nodeDef = json.nodes[nodeIndex]; + + if (nodeDef.extensions?.MOZ_hubs_components?.["loop-animation"] !== undefined) { + const extensionDef = nodeDef.extensions.MOZ_hubs_components["loop-animation"]; + + if (extensionDef.clip === undefined || extensionDef.clip === "") { + continue; + } + + // Converts .clip (name based) to .activeClipIndices (index based). + // Assumes that .activeClipIndices is undefined + // if .clip is defined + + const clipNames = extensionDef.clip.split(","); + const activeClipIndices = []; + const nodeIndices = collectNodeIndices(nodeIndex, new Set()); + + for (const clipName of clipNames) { + const animationIndex = findAnimation(clipName); + + if (animationIndex === null) { + continue; + } + + const clonedAnimation = structuredClone(json.animations[animationIndex]); + let updated = false; + + for (const channel of clonedAnimation.channels) { + if (channel.target.node !== undefined && !nodeIndices.has(channel.target.node)) { + // The old loop-animation handler (without bitECS) seems to retarget + // the loop-animation component root node if traget node is unfound + // under the loop-animation component root node. + // Here follows it for the compatibility, not sure if it's the best approach. + // Another approach may be to find a glTF.animation that is likely for + // this node. It may be guessed with target node. + channel.target.node = nodeIndex; + updated = true; + } + } + + if (updated) { + // Retargetted so need to add a new glTF.animation. + activeClipIndices.push(json.animations.length + clonedAnimations.length); + clonedAnimations.push(clonedAnimation); + } else { + activeClipIndices.push(animationIndex); + } + } + + extensionDef.activeClipIndices = activeClipIndices; + delete extensionDef.clip; + } + } + + for (const animation of clonedAnimations) { + json.animations.push(animation); + } + } +} + export async function loadGLTF(src, contentType, onProgress, jsonPreprocessor) { let gltfUrl = src; let fileMap; @@ -689,6 +805,7 @@ export async function loadGLTF(src, contentType, onProgress, jsonPreprocessor) { .register(parser => new GLTFHubsLightMapExtension(parser)) .register(parser => new GLTFHubsTextureBasisExtension(parser)) .register(parser => new GLTFMozTextureRGBE(parser, new RGBELoader().setDataType(THREE.HalfFloatType))) + .register(parser => new GLTFHubsLoopAnimationComponent(parser)) .register( parser => new GLTFLodExtension(parser, {