Bump into the latest glTF loader #4198
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ import { getCustomGLTFParserURLResolver } from "../utils/media-url-utils"; | |
| import { promisifyWorker } from "../utils/promisify-worker.js"; | ||
| import { MeshBVH, acceleratedRaycast } from "three-mesh-bvh"; | ||
| import { disposeNode, cloneObject3D } from "../utils/three-utils"; | ||
| import { KTX2Loader } from "three/examples/jsm/loaders/KTX2Loader"; | ||
|
|
||
| THREE.Mesh.prototype.raycast = acceleratedRaycast; | ||
|
|
@@ -341,127 +340,221 @@ function runMigration(version, json) { | |
| } | ||
| } | ||
|
|
||
| let ktxLoader; | ||
|
|
||
| class GLTFHubsPlugin { | ||
| constructor(parser, jsonPreprocessor, fileMap) { | ||
| this.parser = parser; | ||
| this.jsonPreprocessor = jsonPreprocessor; | ||
| this.fileMap = fileMap; | ||
|
|
||
| // The latest GLTFLoader doesn't use ImageBitmapLoader for Firefox | ||
| // because the latest ImageBitmapLoader passes an option parameter | ||
| // to createImageBitmap() but Firefox createImageBitmap() fails | ||
| // if an option parameter is passed (known bug and already reported to bugzilla). | ||
| // But our r111 based ImageBitmapLoader doesn't pass the option parameter yet | ||
| // so we can use ImageBitmapLoader even for Firefox. | ||
| // When we replace our Three.js fork with the latest official one | ||
| // we need to revisit this workaround. | ||
| if (!parser.textureLoader.isImageBitmapLoader && typeof createImageBitmap !== undefined) { | ||
| parser.textureLoader = new THREE.ImageBitmapLoader(parser.options.manager); | ||
|
||
| } | ||
| } | ||
|
|
||
| beforeRoot() { | ||
| const parser = this.parser; | ||
| const jsonPreprocessor = this.jsonPreprocessor; | ||
|
|
||
| // | ||
| if (jsonPreprocessor) { | ||
| parser.json = jsonPreprocessor(parser.json); | ||
| } | ||
|
|
||
| // | ||
| let version = 0; | ||
| if ( | ||
|
Contributor
There was a problem hiding this comment. Not particularly important but eventually we might want this to be its own MozHubsComponetnsExtension that just handles the components extension and not the other weird hubs specific workarounds. Doesn't really matter too much now since the components stuff is all still very tightly integrated into aframe and the rest of Hubs.
Contributor
Author
There was a problem hiding this comment. Added a comment so far and would like to think of the cleaning up later. |
||
| parser.json.extensions && | ||
| parser.json.extensions.MOZ_hubs_components && | ||
| parser.json.extensions.MOZ_hubs_components.hasOwnProperty("version") | ||
| ) { | ||
| version = parser.json.extensions.MOZ_hubs_components.version; | ||
| } | ||
| runMigration(version, parser.json); | ||
|
|
||
| // Note: Here may be rewritten with the one with parser.associations | ||
| const nodes = parser.json.nodes; | ||
| if (nodes) { | ||
| for (let i = 0; i < nodes.length; i++) { | ||
| const node = nodes[i]; | ||
|
|
||
| if (!node.extras) { | ||
| node.extras = {}; | ||
| } | ||
|
|
||
| node.extras.gltfIndex = i; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| afterRoot(gltf) { | ||
| gltf.scene.traverse(object => { | ||
| // GLTFLoader sets matrixAutoUpdate on animated objects, we want to keep the defaults | ||
| // @TODO: Should this be fixed in the gltf loader? | ||
|
Contributor
There was a problem hiding this comment. I know this was just copied over from our existing code, but it still does seem like the GLTFLoader should be respecting this default.
Contributor
Author
There was a problem hiding this comment. I think |
||
| object.matrixAutoUpdate = THREE.Object3D.DefaultMatrixAutoUpdate; | ||
| const materialQuality = window.APP.store.materialQualitySetting; | ||
| object.material = mapMaterials(object, material => convertStandardMaterial(material, materialQuality)); | ||
| }); | ||
|
|
||
| // | ||
| gltf.scene.traverse(object => { | ||
| if (!object.material) { | ||
| return; | ||
| } | ||
| const materials = Array.isArray(object.material) ? object.material : [object.material]; | ||
| // @TODO: Handle more efficiently | ||
|
||
| for (const material of materials) { | ||
| for (const key in material) { | ||
| const prop = material[key]; | ||
| if (prop && prop.isTexture && !prop.onUpdate) { | ||
| prop.onUpdate = () => { | ||
| // Delete texture data once it has been uploaded to the GPU | ||
| prop.image.close && prop.image.close(); | ||
| delete prop.image; | ||
| }; | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| // Replace animation target node name with the node uuid. | ||
|
Contributor
There was a problem hiding this comment. As we have discussed before, it still seems a bit weird that this isn't just the default behavior since GLTF does not guarantee unique names... This is definitely preferable to maintaining a fork, but would like to get to the bottom of why this isn't always desirable. |
||
| // I assume track name is 'nodename.property'. | ||
| if (gltf.animations) { | ||
| for (const animation of gltf.animations) { | ||
| for (const track of animation.tracks) { | ||
| const parsedPath = THREE.PropertyBinding.parseTrackName(track.name); | ||
| for (const scene of gltf.scenes) { | ||
| const node = THREE.PropertyBinding.findNode(scene, parsedPath.nodeName); | ||
| if (node) { | ||
| track.name = track.name.replace(/^[^.]*\./, node.uuid + "."); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // | ||
| if (this.fileMap) { | ||
takahirox marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // The GLTF is now cached as a THREE object, we can get rid of the original blobs | ||
|
||
| Object.keys(this.fileMap).forEach(URL.revokeObjectURL); | ||
| } | ||
|
|
||
| // | ||
| gltf.scene.animations = gltf.animations; | ||
| } | ||
| } | ||
|
|
||
| class GLTFHubsLightMapExtension { | ||
| constructor(parser) { | ||
| this.parser = parser; | ||
| this.name = "MOZ_lightmap"; | ||
| } | ||
|
|
||
| // @TODO: Ideally we should use extendMaterialParams hook. | ||
| // But the current official glTF loader doesn't fire extendMaterialParams | ||
| // hook for unlit and specular-glossiness materials. | ||
| // So using loadMaterial hook as workaround so far. | ||
| // Cons is loadMaterial hook is fired as _invokeOne so | ||
| // if other plugins defining loadMaterial is registered | ||
| // there is a chance that this light map extension handler isn't called. | ||
|
Contributor
There was a problem hiding this comment. Since we have control over the loader this shouldnt really be an issue in practice, but we will need to be careful when doing upgrades that we don't accidentally break it by some plugin being introduced upstream. Hopefully this workaround doesn't last too long anyway. |
||
| // The glTF loader should be updated to remove the limitation. | ||
|
Contributor
There was a problem hiding this comment. Hmm yeah we should discuss. Figuring out ow multiple extensions handling the same thing should interact is tricky, but I could definitely see cases where there are multiple layers of extensions for loading a single thing. |
||
| loadMaterial(materialIndex) { | ||
| const parser = this.parser; | ||
| const json = parser.json; | ||
| const materialDef = json.materials[materialIndex]; | ||
|
|
||
| if (!materialDef.extensions || !materialDef.extensions[this.name]) { | ||
| return null; | ||
| } | ||
|
|
||
| const extensionDef = materialDef.extensions[this.name]; | ||
|
|
||
| const pending = []; | ||
|
|
||
| pending.push(parser.loadMaterial(materialIndex)); | ||
| pending.push(parser.getDependency("texture", extensionDef.index)); | ||
|
|
||
| return Promise.all(pending).then(results => { | ||
| const material = results[0]; | ||
| const lightMap = results[1]; | ||
| material.lightMap = lightMap; | ||
| material.lightMapIntensity = extensionDef.intensity !== undefined ? extensionDef.intensity : 1; | ||
| return material; | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| class GLTFHubsTextureBasisExtension { | ||
| constructor(parser) { | ||
| this.parser = parser; | ||
| this.name = "MOZ_HUBS_texture_basis"; | ||
| } | ||
|
|
||
| loadTexture(textureIndex) { | ||
| const parser = this.parser; | ||
| const json = parser.json; | ||
| const textureDef = json.textures[textureIndex]; | ||
|
|
||
| if (!textureDef.extensions || !textureDef.extensions[this.name]) { | ||
| return null; | ||
| } | ||
|
|
||
| if (!parser.options.ktx2Loader) { | ||
| // @TODO: Display warning (only if the extension is in extensionsRequired)? | ||
|
Contributor
There was a problem hiding this comment. In practice we will always be providing a ktx2loader so if this happens its certainly a bug... We probably don't want to fail silently.
Contributor
Author
There was a problem hiding this comment. OK, I will add warning here |
||
| return null; | ||
| } | ||
|
|
||
| console.warn(`The ${this.name} extension is deprecated, you should use KHR_texture_basisu instead.`); | ||
|
|
||
| const extensionDef = textureDef.extensions[this.name]; | ||
| const source = json.images[extensionDef.source]; | ||
| const loader = parser.options.ktx2Loader.basisLoader; | ||
|
|
||
| return parser.loadTextureImage(textureIndex, source, loader); | ||
| } | ||
| } | ||
|
|
||
| export async function loadGLTF(src, contentType, onProgress, jsonPreprocessor) { | ||
| let gltfUrl = src; | ||
| let fileMap; | ||
|
|
||
| if (contentType && (contentType.includes("model/gltf+zip") || contentType.includes("application/x-zip-compressed"))) { | ||
| fileMap = await extractZipFile(gltfUrl); | ||
| gltfUrl = fileMap["scene.gtlf"]; | ||
| } | ||
|
|
||
| const loadingManager = new THREE.LoadingManager(); | ||
| loadingManager.setURLModifier(getCustomGLTFParserURLResolver(gltfUrl)); | ||
| const gltfLoader = new THREE.GLTFLoader(loadingManager); | ||
| gltfLoader | ||
| .register(parser => new GLTFHubsPlugin(parser, jsonPreprocessor, fileMap)) | ||
| .register(parser => new GLTFHubsLightMapExtension(parser)) | ||
| .register(parser => new GLTFHubsTextureBasisExtension(parser)); | ||
|
|
||
| // TODO some models are loaded before the renderer exists. This is likely things like the camera tool and loading cube. | ||
| // They don't currently use KTX textures but if they did this would be an issue. Fixing this is hard but is part of | ||
| // "taking control of the render loop" which is something we want to tackle for many reasons. | ||
| if (!ktxLoader && AFRAME && AFRAME.scenes && AFRAME.scenes[0]) { | ||
| ktxLoader = new KTX2Loader(loadingManager).detectSupport(AFRAME.scenes[0].renderer); | ||
| } | ||
|
|
||
| if (ktxLoader) { | ||
| gltfLoader.setKTX2Loader(ktxLoader); | ||
| } | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| gltfLoader.load(gltfUrl, resolve, onProgress, reject); | ||
| }); | ||
| } | ||
|
|
||
| export async function loadModel(src, contentType = null, useCache = false, jsonPreprocessor = null) { | ||
|
|
@@ -569,7 +662,7 @@ AFRAME.registerComponent("gltf-model-plus", { | |
| // If we had inflated something already before, clean that up | ||
| this.disposeLastInflatedEl(); | ||
|
|
||
| this.model = gltf.scene; | ||
|
Contributor
There was a problem hiding this comment. Is this guaranteed to be set now? I think the reason we had it originally is that not GLTFs are not required to have a default scene (though almost all I have encountered in practice do).
Contributor
Author
There was a problem hiding this comment. Yes, it's guaranteed in the latest |
||
|
|
||
| if (this.data.batch) { | ||
| this.el.sceneEl.systems["hubs-systems"].batchManagerSystem.addObject(this.model); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Hubs-Foundation/three.js#57 is merged, I would update this line back to
masterin this PR or follow up PR.package-lock.jsonis same.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I usually do it just before merging, and I leave a [ ] todo in the PR description to remind me