glTF MSFT_lod extension support #5713

takahirox · 2022-09-13T02:53:24Z

This PR add glTF MSFT_lod extension support.

Demo video: https://twitter.com/superhoge/status/1567656038480093184

It allows LOD in Hubs, and it supports progressive loading that first loads only the lowest levels and then progressively load the higher levels on demand if an asset has MSFT_lod extension.

Expected benefits from LOD + Progressive loading:

Better runtime performance because of the lower quality models for further objects
Shorter response time from the asset loading because the asset becomes visible if only the lowest levels are ready
Save network usage because of loading only the needed levels

Note that currently to get the benefits 2. and 3. the assets need to be .gltf/glb + separated bin/texture files for each level. Because Three.js doesn't support HTTP Range Requests yet. If an assets consists of a single bundled .glb file Three.js GLTFLoader first loads the entire contents.

I'm discussing with Three.js devs about whether we can add HTTP Range Requests to Three.js. If they accept it or we apply my HTTP Range request patches we will be able to get the benefits of 2. and 3. even for single bundled glb without changing our code.

takahirox · 2022-10-07T21:51:54Z

Regarding Three.js,

They seem to feel positive for allowing HTTP range requests in FileLoader mrdoob/three.js#24580

but not really in GLTFLoader mrdoob/three.js#24506

However, I confirmed HTTP range requests + GLTFLoader is doable with its Plugin system mrdoob/three.js#24506 (comment)

I'm not sure when they will merge FileLoader HTTP range requests support PR. So I'm thinking of cherry-picking it to our Three.js fork so far and then writing GLTFLoader HTTP range request plugin.

Update:

I made a branch Three.js branch https://github.com/MozillaReality/three.js/tree/hubs-patches-141-range-requests that cherry-picked the FileLoader PR to hubs-patches-141 and Hubs refers to it in this PR.

takahirox · 2022-10-13T18:17:21Z

src/components/gltf-model-plus.js

@@ -11,6 +11,8 @@ import { RGBELoader } from "three/examples/jsm/loaders/RGBELoader";
 import { BasisTextureLoader } from "three/examples/jsm/loaders/BasisTextureLoader";
 import { DRACOLoader } from "three/examples/jsm/loaders/DRACOLoader";
 import { GLTFLoader } from "three/examples/jsm/loaders/GLTFLoader";
+import GLBRangeRequests from "three-gltf-extensions/loaders/GLB_range_requests/GLB_range_requests";


Note for reviews:

This is a GLTFLoader plugin for HTTP range requests. It does

checks if a file a url referes to is GLB and the server supports HTTP range requests

if so it first loads only json chunk and partially and lazily loads bin chunk content with HTTP range requests

https://github.com/takahirox/three-gltf-extensions/blob/main/loaders/GLB_range_requests/GLB_range_requests.js

takahirox · 2022-10-13T18:20:50Z

src/components/gltf-model-plus.js

@@ -11,6 +11,8 @@ import { RGBELoader } from "three/examples/jsm/loaders/RGBELoader";
 import { BasisTextureLoader } from "three/examples/jsm/loaders/BasisTextureLoader";
 import { DRACOLoader } from "three/examples/jsm/loaders/DRACOLoader";
 import { GLTFLoader } from "three/examples/jsm/loaders/GLTFLoader";
+import GLBRangeRequests from "three-gltf-extensions/loaders/GLB_range_requests/GLB_range_requests";
+import GLTFLodExtension from "three-gltf-extensions/loaders/MSFT_lod/MSFT_lod";


Note for reviews:

This is a GLTFLoader plugin for glTF MSFT_lod extension

https://github.com/takahirox/three-gltf-extensions/tree/main/loaders/MSFT_lod

The plugin requires mrdoob/three.js#24580 in Three.js

The plugin supports the LOD progressive loading

First load only the lowest level

Set up THREE.LOD. Clone the lowest level and set it to all the levels.

Set onBeforeRender for higher levels

In onBeforeRender it invokes the downloading of that level object and replace it with the cloned lowest level object

takahirox · 2022-10-13T19:37:32Z

src/components/gltf-model-plus.js

 let ktxLoader;
 let dracoLoader;

 class GLTFHubsPlugin {
  constructor(parser, jsonPreprocessor) {
    this.parser = parser;
    this.jsonPreprocessor = jsonPreprocessor;
+    this.name = 'MOZ_hubs_plugin';


Note for reviewers: A plugin must have a name

takahirox · 2022-10-13T20:34:15Z

src/utils/three-utils.js

@@ -165,6 +165,8 @@ export function cloneObject3D(source, preserveUUIDs) {
      return;
    }

+    clonedNode.onBeforeRender = sourceNode.onBeforeRender;


Note for reviewers: Progressive loading is fired in onBeforeRender. So it should be copied in cloning.

takahirox · 2022-10-13T22:45:51Z

src/components/gltf-model-plus.js

+    .register(parser => new GLTFMozTextureRGBE(parser, new RGBELoader().setDataType(THREE.HalfFloatType)))
+    .register(parser => new GLTFLodExtension(parser, {
+      loadingMode: 'progressive',
+      onLoadMesh: (lod, mesh, level, lowestLevel) => {


Note for reviewers: onLoadMesh is a callback that is fired when a new level is progressively loaded.

netpro2k

Looking pretty good.

The LOD stuff is not very concerning since its opt in and most files wont be using it so we can refine it over time. I am slightly uneasy about how components will be handled and the fact that we will by dynamically adding new objects to the scene at sort of arbitrary times, but I suspect it wont be much of an issue in practice.

The range request stuff is s bit more concerning since the common case of loading a glb file without LODs will require 5 requests (3 with the changes I suggested). We might want to profile this one some more or wait until we have a workflow that makes LODs the common case.

netpro2k · 2022-10-18T00:35:50Z

src/components/gltf-model-plus.js

@@ -441,8 +450,7 @@ class GLTFHubsPlugin {
      // GLTFLoader sets matrixAutoUpdate on animated objects, we want to keep the defaults
      // @TODO: Should this be fixed in the gltf loader?
      object.matrixAutoUpdate = THREE.Object3D.DefaultMatrixAutoUpdate;
-      const materialQuality = window.APP.store.state.preferences.materialQualitySetting;
-      updateMaterials(object, material => convertStandardMaterial(material, materialQuality));
+      convertStandardMaterialsIfNeeded(object);


Probably not part of this PR since it was already like this, but the fact you had to do this both here and for each LOD level points to the fact that this should really be done as part of creating the materials initially in the GLTFLoader, probably via an additional extension.

netpro2k · 2022-10-18T00:37:28Z

src/components/gltf-model-plus.js

+        // So some post-loading processings done in gltf-model-plus and media-loader
+        // need to be done here now.
+
+        // Nothing to do if this is the lowest level mesh.


Because the lowest level was already part of the scene graph returned initially by GLTFLoader's load right?

Yes, the lowest levels already be in the scene graph when GLTFLoader onLoad callback is fired so the post-processing here isn't needed.

src/components/gltf-model-plus.js

netpro2k · 2022-10-18T03:11:34Z

src/components/gltf-model-plus.js

+
+        convertStandardMaterialsIfNeeded(mesh);
+
+        // A hacky solution. media-loader and media-utils make a material clone


Yeah I really don't like how we are patching those materials. It has lead to lots of confusing bugs where things hold onto stale references of the pre-patched materials. We should think about this as part of new gltf loading stuff.

netpro2k · 2022-10-18T03:13:02Z

src/components/gltf-model-plus.js

@@ -676,7 +732,7 @@ export async function loadGLTF(src, contentType, onProgress, jsonPreprocessor) {
  }

  return new Promise((resolve, reject) => {
-    gltfLoader.load(gltfUrl, resolve, onProgress, reject);
+    GLBRangeRequests.load(gltfUrl, gltfLoader, resolve, onProgress, reject);


It's kind of odd that we have to use this static load function like this. Not sure its better than us just patching our GLTFLoader since we are bypassing the original load function anyway so we might miss upstream changes and have to think about upgrades closely anyway.

See #5713 (comment)

It's a kind of a helper function. We may copy/paste the code to gltf-model-plus.

I want to go as is because the range requests feature is behind the query strings now. I may revisit if it'll be enabled by default.

netpro2k · 2022-10-18T03:14:34Z

src/components/gltf-model-plus.js

@@ -11,6 +11,8 @@ import { RGBELoader } from "three/examples/jsm/loaders/RGBELoader";
 import { BasisTextureLoader } from "three/examples/jsm/loaders/BasisTextureLoader";
 import { DRACOLoader } from "three/examples/jsm/loaders/DRACOLoader";
 import { GLTFLoader } from "three/examples/jsm/loaders/GLTFLoader";
+import GLBRangeRequests from "three-gltf-extensions/loaders/GLB_range_requests/GLB_range_requests";
+import GLTFLodExtension from "three-gltf-extensions/loaders/MSFT_lod/MSFT_lod";


No good way to comment upstream since the there are multiple closed PRs so just including links here:

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L224
Based on your comments upstream I agree that you should be able to handle the material LOD case with just node LODs (making other node LOD levels just reference a different material), so shouldn't we prefer node rather than material in the case both exist?

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L291
Shouldn't `onLoadMesh`` be called here as well like we do above for the material LOD case?

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L120
This LOD swapping with dummy objects in onBeforeRender feels slightly hacky but seems like it should work. I mostly don't like that its happening at a sort of unpredictable part in the frame since its both in onBeforeRender but also a .then. I wonder if we can think through a way to do this as system instead. Maybe after we have transitioned gltf loading to bitecs fully we can revisit.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L115
I assume (though I don't actually see it specifically outlined in the spec) that "coverage" is a 0-1 % of the screen that rendering the object will cover when this LOD should be used. It's not really clear how you would translate that to distance as its actually going to change at runtime depending on the camera properties. In practice just using a distance seems fine for Hubs but I don't think it would be to spec. Like the previous one maybe this is something we would handle in an "LOD system".

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L12
After this getCurrentLevel could be pointing at the level that was just removed. If it was one of the middle levels I guess it would still be accurate since it would just be pointing at the next level but if the removed level was the last it would be pointing at an invalid index.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L159
Hmm yeah I think the issue is coming into play here because THREE.LOD is at the node level. We almost want something like an LODMaterial that would do similar swapping of materials based on distanced. Though honeslty I am a bit confused what the usecase for material LODs is. Seems like any case could be handled by node LODs.

And some general notes:
When I was originally thinking about LODs I assumed that the different LOD levels would only be pointing at different meshes (like what was proposed here KhronosGroup/glTF#1045 (comment)), so only the Geometry in THREE would need switching. It seems MSFT_LOD switches at the Node level, which means technically different LODs could have different sets of Hubs components. We would need to account for that in the "progressive" and "any" case since not all the component data would be in the scene graph when its first processed in gtlf-model-plus. Maybe we just say this is unsupported for now. If it seems useful we can implement it as part of rewriting gltf loading as we move away from aframe rather than making this more complex only to get rid of it later.

Related. It might end up being a bit of an issue for some components that they will now be "on" the THREE.LOD instead of a THREE.Mesh. (well until we fully migrate away from aframe they are "on" the THREE.Group which aframe creates, but .getObject3D("mesh") will now be a THREE.LOD)

Also just realized the additional layer of the THREE.LOD node may cause issues with animations, but maybe THREE already accounts for this.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L224
Based on your comments upstream I agree that you should be able to handle the material LOD case with just node LODs (making other node LOD levels just reference a different material), so shouldn't we prefer node rather than material in the case both exist?

The reason why I give the priority material LOD over node LOD is just the implementation can be simpler due to GLTFLoader plugin API and design. No other big reasons.

And the behavior of the case where the both node and material LODs are defined is not clarified in the spec. Personally I think the spec shouldn't allow the both LOD definitions and/or the spec may drop material LOD support for simplicity as suggested in

KhronosGroup/glTF#1952

I'm not sure if we really want to take effort for the case where both node and material LODs are defined because I'm not sure if there are many assets having the both. So, I want to go as is for now and think more later if needed.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L291
Shouldn't `onLoadMesh`` be called here as well like we do above for the material LOD case?

Good catch, I updated the plugin. I will upgrade the package when I will resolve all the issues.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/MSFT_lod/MSFT_lod.js#L12
After this getCurrentLevel could be pointing at the level that was just removed. If it was one of the middle levels I guess it would still be accurate since it would just be pointing at the next level but if the removed level was the last it would be pointing at an invalid index.

removeLevel() is called only right before LOD.addLevel() call and getCurrentLevel is re-calculated in the renderer in the next frame. So I don't think it is a big deal.

When I was originally thinking about LODs I assumed that the different LOD levels would only be pointing at different meshes (like what was proposed here KhronosGroup/glTF#1045 (comment)), so only the Geometry in THREE would need switching.

Honestly I agree with that comment. I think LOD extension should allow only LOD meshes in a node like

node: [ { mesh: 0, extensions: { EXT_lod: { indices: [ // mesh indices 1, 2 ] } } } ]

The current asset with MSFT_lod spec and its handler can be more complex than it really needs to be.

I'm thinking to suggest it to MSFT_lod to simplify, or propose a new simpler LOD extension like EXT_mesh_lod.

By the way, Mesh is a set of geometry and material. Even if the LOD extension allows only mesh LODs in a node, switching only Geometry in Three.js isn't good enough, switching Mesh is needed.

It seems MSFT_LOD switches at the Node level, which means technically different LODs could have different sets of Hubs components. We would need to account for that in the "progressive" and "any" case since not all the component data would be in the scene graph when its first processed in gtlf-model-plus. Maybe we just say this is unsupported for now. If it seems useful we can implement it as part of rewriting gltf loading as we move away from aframe rather than making this more complex only to get rid of it later.

Related. It might end up being a bit of an issue for some components that they will now be "on" the THREE.LOD instead of a THREE.Mesh. (well until we fully migrate away from aframe they are "on" the THREE.Group which aframe creates, but .getObject3D("mesh") will now be a THREE.LOD)

Unspoorting Hubs components for LOD for now and rethink it when rewriting our gltf loading and our scene graph structure sounds good to me.

Also just realized the additional layer of the THREE.LOD node may cause issues with animations, but maybe THREE already accounts for this.

Mesh objects are placed under LOD and key frame animation clip is bound to LOD object so keyframe position/rotation/scale animation shouldn't be a problem because they are propagated to child objects world matrices.

But morph animation may be problematic. Key frame morph animation clip needs to be bound to Mesh object, not LOD object. Animation clip binding is done in the first parse, so for lazily loaded Mesh objects the binding needs to be done when they are loaded.

And no guarantee that all levels have the same morph set. Probably the LOD specification should state that all lavels have the same morph set with the same order mainly for animation? This also pushes me to propose a simpler LOD extension.

Perhaps, unsupporting key frame morph for LOD for now?

By the way, Mesh is a set of geometry and material. Even if the LOD extension allows only mesh LODs in a node, switching only Geometry in Three.js isn't good enough, switching Mesh is needed.

Ah right. For some reason I thought material was on node in GLTF but its not. We will definitely have to figure out what to do about components. I guess in the new world this isn't so bad since components are associated with an eid, and an eid is also associated with an Object3D, but you should not hold onto a reference to that Object3D ever, so it should be fine for us to swap it for LODs. We actually do this here https://github.com/mozilla/hubs/blob/38d5cbfd21a3bf0ce8d3ddc01132e5c702f9b72b/src/bit-systems/video-system.ts#L79 though we are still a bit unsure if its a good idea.

Mesh objects are placed under LOD and key frame animation clip is bound to LOD object so keyframe position/rotation/scale animation shouldn't be a problem because they are propagated to child objects world matrices.

Right but if an animation targets a child of something that becomes an Three.LOD won't it be an issue because there is now an extra parent LOD -> Obj A -> Obj B. When we added the THREE.Group nodes for AFRAME i think we also had to rewrite animations to target.

Right but if an animation targets a child of something that becomes an Three.LOD won't it be an issue because there is now an extra parent LOD -> Obj A -> Obj B.

What are Obj A and B in this context? If they are different level objects, the both are right under THREE.LOD.

netpro2k · 2022-10-18T03:16:21Z

src/components/gltf-model-plus.js

@@ -11,6 +11,8 @@ import { RGBELoader } from "three/examples/jsm/loaders/RGBELoader";
 import { BasisTextureLoader } from "three/examples/jsm/loaders/BasisTextureLoader";
 import { DRACOLoader } from "three/examples/jsm/loaders/DRACOLoader";
 import { GLTFLoader } from "three/examples/jsm/loaders/GLTFLoader";
+import GLBRangeRequests from "three-gltf-extensions/loaders/GLB_range_requests/GLB_range_requests";


https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L32
We should probably fetch the whole binary header here since we are going to need it anyway. The json chunk is also required to be first so we might as well also get its size/type too. So we can request bytes 0-20 in 1 request and then return the header, and first chunk length. This avoids 2 extra requests from the way its currently implemetned.

We should also check the byte length returned. If its larger than expected the server probably ignored the range header. In that case it would be ideal if we could just pass the buffer off to the normal loading flow so that we didn't need to download the whole file again.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L104
If we do the initial request to grab the size of the first chunk too this code gets simpler and no longer needs to be a loop. We just loadPartially the first chunk. The vinChunkOffset will just be the first chunk length + 12. If its the same as the header.length we know there is no binary chunk.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L121
Hmm not sure we should reject in this case. Its valid to have a glb with no bin, and we would not want to use a different loader just for that case (which we wouldn't know ahead of time).

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L124
The way we are using GLBRangRequests right now this would mean we cant load anything with EXT_meshopt_compression at all anymore

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L67
Dynamically registering this extension seems odd.. I guess there is no way for GLTFLoader plugins to override the initial load so we had to use this static load function instead?

Slightly concerned about increasing the number of requests in the (very common) case that the glb does not have any LODs that need progressive loading. I think my suggestions knock the baseline down to 2 requests instead of 4 but that's still an extra request.

We should also check the byte length returned. If its larger than expected the server probably ignored the range header.

FileLoader invokes onError if the server doesn't support HTTP range requests. And if the plugin catches the error it loads a glTF file in the conditional way (without range requests).

mrdoob/three.js#24580

so that we didn't need to download the whole file again.

I assume browser's disk cache works

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L124
The way we are using GLBRangRequests right now this would mean we cant load anything with EXT_meshopt_compression at all anymore

Ah, I was going to

rejects if EXT_meshopt_compression is found

and then loads the asset with regular gltfLoader.load()

but I have wrongly implemented. I will update the implementation.

So that the combination of progressive loading + EXT_meshopt_compression is impossible but EXT_meshopt_compression can still be used. progressive loading + EXT_meshopt_compression is a future work for me.

Update: done

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L121
Hmm not sure we should reject in this case. Its valid to have a glb with no bin, and we would not want to use a different loader just for that case (which we wouldn't know ahead of time).

Makes sense to me. Updated.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L32
We should probably fetch the whole binary header here since we are going to need it anyway. The json chunk is also required to be first so we might as well also get its size/type too. So we can request bytes 0-20 in 1 request and then return the header, and first chunk length. This avoids 2 extra requests from the way its currently implemetned.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L104
If we do the initial request to grab the size of the first chunk too this code gets simpler and no longer needs to be a loop. We just loadPartially the first chunk. The vinChunkOffset will just be the first chunk length + 12. If its the same as the header.length we know there is no binary chunk.

Sounds good to me. Updated.

https://github.com/takahirox/three-gltf-extensions/blob/62288914fbafb2500e500b0209b33d4f3a96cfac/loaders/GLB_range_requests/GLB_range_requests.js#L67
Dynamically registering this extension seems odd.. I guess there is no way for GLTFLoader plugins to override the initial load so we had to use this static load function instead?

Your guess is right. I couldn't come up with any other ways.

Added a patch for avoiding duplicated full range content request in case of server doesn't support range requests.

takahirox/three-gltf-extensions#90

takahirox · 2022-10-20T19:08:01Z

This discussion may be too long now. My opinion in summary.

Regarding LOD:

The LOD handling in this PR doesn't great fit to the current Hubs Client design because the followings are not expected in the current design

Dynamic loading at arbitrary time
.getObject3D("mesh") will be mapped to LOD object, not Mesh

So, some Hubs component/features (eg. morph animation, skinning animation) may not work for LOD.

We are working on Hubs Client refactoring with bitecs. When we will finish, probably we will rewrite the LOD hanlder to a system to let LOD handling more finely fit to the new Hubs Client design.

I don't think we should update the current Hubs Client design for LOD. Instead we should focus on the refactoring and we may ask users to accept the limitation for LOD for now.

Regarding Range request:

We might want to profile this one some more or wait until we have a workflow that makes LODs the common case.

The change has an effect to all the glTF asset loadings. Honestly I feel a bit scared for the functionality and performance impact. Starting with progressive loading feature behind the flag for now? Test a lot and then enable it if it looks ok.

Regarding MSFT_lod extension spec:

While working on the handler, I found some problems in the spec. I will give the feedback to them or propose a new simpler extension. I hope we can switch to the new one at some point. Probably the handler will not need to change much.

takahirox · 2022-10-21T00:04:31Z

package.json

@@ -131,8 +131,9 @@
    "screenfull": "^4.0.1",
    "sdp-transform": "^2.14.1",
    "semver": "^7.3.2",
-    "three": "github:mozillareality/three.js#c40ae76f114346b33616f72edf43b4eef26d338d",
+    "three": "github:mozillareality/three.js#0404718c9b3a502ac66408683b2d793a80813a91",


Note for reviewers:

Range request requires this Three.js PR that has not been merged yet.

mrdoob/three.js#24580

I made a branch hubs-patches-141-range-requests from hubs-patches-141, did cherry-pick to hubs-patches-141-range-requests, and let Hubs Client refer to it.

https://github.com/MozillaReality/three.js/tree/hubs-patches-141
https://github.com/MozillaReality/three.js/tree/hubs-patches-141-range-requests

Another option is applying the change to hubs-patches-141.

I think we should create a PR against hubs-patches-141, merge it, then do a interactive rebase to make the build. Same as for other patches we have made. Once this lands in upstream and we update three we can remove it.

OK, I sent a PR to our Three.js fork.

Hubs-Foundation/three.js#68

netpro2k · 2022-10-21T01:02:14Z

Added an additional suggestion here takahirox/three-gltf-extensions@3db115a - In the common case the code you have now is the same. If it gets an invalid glb file (where the JSON chunk doesn't come first) your code would handle it, but I think its cleaner (and more correct) not to.

takahirox · 2022-11-01T20:44:42Z

Merging. We can improve LOD handling when we will refactor glTF loading path and systems.

takahirox force-pushed the GLTFMSFTLOD branch from b56cc3b to c46bb37 Compare September 13, 2022 02:54

takahirox force-pushed the GLTFMSFTLOD branch from c46bb37 to 51d853d Compare October 4, 2022 00:41

takahirox force-pushed the GLTFMSFTLOD branch from 51d853d to 260b50e Compare October 12, 2022 23:29

takahirox requested review from netpro2k and johnshaughnessy October 12, 2022 23:31

takahirox had a problem deploying to hc-bio October 12, 2022 23:44 Failure

takahirox commented Oct 13, 2022

View reviewed changes

netpro2k reviewed Oct 18, 2022

View reviewed changes

takahirox had a problem deploying to hc-bio October 18, 2022 21:57 Failure

takahirox force-pushed the GLTFMSFTLOD branch from 3a775a5 to 08e667e Compare October 20, 2022 04:43

takahirox had a problem deploying to hc-bio October 20, 2022 05:01 Failure

takahirox had a problem deploying to hc-bio October 21, 2022 00:00 Failure

takahirox commented Oct 21, 2022

View reviewed changes

takahirox mentioned this pull request Oct 25, 2022

[HUBS] FileLoader: HTTP Range requests support Hubs-Foundation/three.js#68

Merged

takahirox added 3 commits November 1, 2022 13:09

glTF MSFT_lod extension support

2b4585d

Fix a wrong method name

c8fdf13

Hide Range requests feature behind qsTruthy("rangerequests")

d2d115d

takahirox force-pushed the GLTFMSFTLOD branch from 8eee2cb to d2d115d Compare November 1, 2022 20:11

takahirox had a problem deploying to hc-bio November 1, 2022 20:27 Failure

takahirox merged commit ca43d8d into master Nov 1, 2022

takahirox deleted the GLTFMSFTLOD branch November 1, 2022 20:46


		convertStandardMaterialsIfNeeded(mesh);

		// A hacky solution. media-loader and media-utils make a material clone

glTF MSFT_lod extension support #5713

glTF MSFT_lod extension support #5713

Conversation

takahirox commented Sep 13, 2022 • edited Loading

takahirox commented Oct 7, 2022 • edited Loading

Choose a reason for hiding this comment

takahirox Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netpro2k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netpro2k Oct 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirox Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

takahirox Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netpro2k Oct 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirox Oct 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirox Oct 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirox commented Oct 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netpro2k commented Oct 21, 2022

takahirox commented Nov 1, 2022

takahirox commented Sep 13, 2022 •

edited

Loading

takahirox commented Oct 7, 2022 •

edited

Loading

takahirox Oct 13, 2022 •

edited

Loading

netpro2k Oct 18, 2022 •

edited

Loading

takahirox Oct 19, 2022 •

edited

Loading

takahirox Oct 19, 2022 •

edited

Loading

netpro2k Oct 18, 2022 •

edited

Loading

takahirox Oct 20, 2022 •

edited

Loading

takahirox Oct 20, 2022 •

edited

Loading

takahirox commented Oct 20, 2022 •

edited

Loading