Skip to content

Commit 50176fc

Browse files
committed
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-component using that names refers to. Refer to #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-component 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.
1 parent a58a846 commit 50176fc

File tree

2 files changed

+118
-1
lines changed

2 files changed

+118
-1
lines changed

src/bit-systems/mixer-animatable.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const mixerExitQuery = exitQuery(mixerQuery);
1010

1111
export function mixerAnimatableSystem(world: HubsWorld): void {
1212
initializeEnterQuery(world).forEach(eid => {
13-
if (entityExists(world, eid)) {
13+
if (!entityExists(world, eid)) {
1414
console.warn("Skipping nonexistant entity."); // TODO Why does this happen?
1515
return;
1616
}

src/components/gltf-model-plus.js

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,122 @@ class GLTFMozTextureRGBE {
670670
}
671671
}
672672

673+
// Mozilla Hubs loop-animation component has a problem in the spec.
674+
// The loop-animation component can refer to glTF.animations with
675+
// animation names but the glTF specification allows non-unique names
676+
// in glTF.animations so if there are multiple glTF.animations that
677+
// have the same name no one can know what glTF.animations a
678+
// loop-component using that names refers to.
679+
// This plugin converts animation names in the component to
680+
// animation indices to avoid the problem.
681+
// The old loop-animation component handler (without bitECS) seems
682+
// to assume that multiple glTF.animations that have the same name
683+
// have the same animation data and loop-component refers to the
684+
// first glTF.animation of the ones having the same name. This
685+
// plugin follows the assumption for the compatibility.
686+
// Refer to https://github.com/mozilla/hubs/pull/6153 for details.
687+
// TODO: Deprecate the loop-animation animation reference with name
688+
class GLTFHubsLoopAnimationComponent {
689+
constructor(parser) {
690+
this.parser = parser;
691+
this.name = "MOZ_hubs_components.loop-animation";
692+
}
693+
694+
beforeRoot() {
695+
const json = this.parser.json;
696+
697+
if (json.animations === undefined) {
698+
return;
699+
}
700+
701+
// TODO: Optimize if needed
702+
const findAnimation = (name) => {
703+
for (let animationIndex = 0; animationIndex < json.animations.length; animationIndex++) {
704+
const animationDef = json.animations[animationIndex];
705+
if (animationDef.name === name) {
706+
return animationIndex;
707+
}
708+
}
709+
return null;
710+
};
711+
712+
// TODO: Optimize if needed
713+
const collectNodeIndices = (nodeIndex, nodeIndices) => {
714+
nodeIndices.add(nodeIndex);
715+
const nodeDef = json.nodes[nodeIndex];
716+
717+
if (nodeDef.children !== undefined) {
718+
for (const child of nodeDef.children) {
719+
collectNodeIndices(child, nodeIndices);
720+
}
721+
}
722+
723+
return nodeIndices;
724+
};
725+
726+
const clonedAnimations = [];
727+
728+
for (let nodeIndex = 0; nodeIndex < json.nodes.length; nodeIndex++) {
729+
const nodeDef = json.nodes[nodeIndex];
730+
731+
if (nodeDef.extensions?.MOZ_hubs_components?.["loop-animation"] !== undefined) {
732+
const extensionDef = nodeDef.extensions.MOZ_hubs_components["loop-animation"];
733+
734+
if (extensionDef.clip === undefined || extensionDef.clip === '') {
735+
continue;
736+
}
737+
738+
// Converts .clip (name based) to .activeClipIndices (index based).
739+
// Assumes that .activeClipIndices is undefined
740+
// if .clip is defined
741+
742+
const clipNames = extensionDef.clip.split(',');
743+
const activeClipIndices = [];
744+
const nodeIndices = collectNodeIndices(nodeIndex, new Set());
745+
746+
for (const clipName of clipNames) {
747+
const animationIndex = findAnimation(clipName);
748+
749+
if (animationIndex === null) {
750+
continue;
751+
}
752+
753+
const clonedAnimation = structuredClone(json.animations[animationIndex]);
754+
let updated = false;
755+
756+
for (const channel of clonedAnimation.channels) {
757+
if (channel.target.node !== undefined && !nodeIndices.has(channel.target.node)) {
758+
// The old loop-animation handler (without bitECS) seems to retarget
759+
// the loop-animation component root node if traget node is unfound
760+
// under the loop-animation component root node.
761+
// Here follows it for the compatibility, not sure if it's the best approach.
762+
// Another approach may be to find a glTF.animation that is likely for
763+
// this node. It may be guessed with target node.
764+
channel.target.node = nodeIndex;
765+
updated = true;
766+
}
767+
}
768+
769+
if (updated) {
770+
// Retargetted so need to add a new glTF.animation.
771+
activeClipIndices.push(json.animations.length + clonedAnimations.length);
772+
clonedAnimations.push(clonedAnimation);
773+
} else {
774+
activeClipIndices.push(animationIndex);
775+
}
776+
}
777+
778+
extensionDef.activeClipIndices = activeClipIndices;
779+
delete extensionDef.clip;
780+
}
781+
}
782+
783+
for (const animation of clonedAnimations) {
784+
json.animations.push(animation);
785+
}
786+
}
787+
}
788+
673789
export async function loadGLTF(src, contentType, onProgress, jsonPreprocessor) {
674790
let gltfUrl = src;
675791
let fileMap;
@@ -689,6 +805,7 @@ export async function loadGLTF(src, contentType, onProgress, jsonPreprocessor) {
689805
.register(parser => new GLTFHubsLightMapExtension(parser))
690806
.register(parser => new GLTFHubsTextureBasisExtension(parser))
691807
.register(parser => new GLTFMozTextureRGBE(parser, new RGBELoader().setDataType(THREE.HalfFloatType)))
808+
.register(parser => new GLTFHubsLoopAnimationComponent(parser))
692809
.register(
693810
parser =>
694811
new GLTFLodExtension(parser, {

0 commit comments

Comments
 (0)