Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple DRACOLoader instances to load multiple GLB files fails on iOS #22445

Open
manmohanbishnoi opened this issue Aug 27, 2021 · 6 comments
Open
Assignees

Comments

@manmohanbishnoi
Copy link

Describe the bug

When trying to load multiple GLB files that are DRACO compressed the DRACOLoader crashes sometimes. (no errors in console)

To Reproduce

Steps to reproduce the behavior:

  1. Create an array with file names for GLB files
  2. Create new instance of DRACOLoaer for when loading each file
  3. Refresh a few times.

I noticed that sometimes just one file loaded, sometimes 2 but as per my code below they were not added to the scene because DRACOLoader for the remaining file did not finish.
Additionally, which file(s) failed to load was also random from the array.
At random page refresh sometimes it was able to load all three GLB files and they were properly added to the screen.

Fix: @manthrax suggested to initialize a DRACOLoader instance in global state and reuse that in loadModelFile() instead of creating a new DRACOLoader instance for each file.

Code

// this array holds file names to load
const ModelList = [
    {
      fileName: "file1.glb",
    },
    {
      fileName: "file2.glb",
    },
    {
      fileName: "file3.glb",
    },
  ];
var loadedModels = []; // used to store scenes for later manipulation

loadModelFile = (fileName, path) => {
    return new Promise((resolve) => {
      let dracoLoader = new DRACOLoader();
      dracoLoader.setDecoderPath("assets/js/libs/draco/gltf/");
      let loader = new GLTFLoader();

      loader.setDRACOLoader(dracoLoader);

      loader.load(`${path}${fileName}`, (gltf) => {
        console.info("GLTF file load complete");

        gltf.scene.rotateY(this.modelRot * THREE.Math.DEG2RAD);
        gltf.scene.scale.set(this.modelScale, this.modelScale, this.modelScale);

        gltf.scene.traverse((child) => {
          if (child.isMesh) {
            console.log(`Material: ${child.material.name}`);

            // this HDR image is already loaded
            child.material.envMap = hdrBackground;
            child.frustumCulled = false;
          }
        });
        console.log(`DONE ${fileName}`);

        resolve({ fileName: fileName, gltf: gltf });
      });
    });
  };

  load3DModels = (list, destination, path = "assets/3d/") => {
    let promises = [];

    for (let j in list) {
      let mt = list[j];

      promises.push(this.loadModelFile(mt.fileName, path));
    }

    return Promise.all(promises).then((result) => {
      console.log("All 3D files loaded: ", result);

      for (let r in result) {
        let res = result[r];
        console.log(res);

        // save in associative array for easy manipulation
        destination[res.fileName] = res.gltf;
      }

      return new Promise((resolve) => {
        resolve(destination);
      });
    });
  };

loadHDRTexture = (filename) => {
    return new Promise((resolve) => {
      new RGBELoader()
        .setDataType(THREE.UnsignedByteType)
        .setPath("assets/3d/textures/envMaps/")
        .load(filename, (texture) => {
          resolve(texture);
        });
    });
  };

loadHDRTexture(HDRIMap).then((hdrTexture) => {
     hdrCubeRenderTarget =
        pmremGenerator.fromEquirectangular(hdrTexture);
        hdrBackground =hdrCubeRenderTarget.texture; // we use this HDRi later
        // load 3D models
        load3DModels(ModelList, loadedModels);.then(() => {
        animate(); // start render loop
      });
    });

Expected behavior

All GLB files should have loaded.

Platform:

  • Device: Mobile
  • OS: iOS
  • Browser: Chrome, Safari (I did not test on others)
  • Three.js version: r132
@donmccurdy
Copy link
Collaborator

I think we should refactor DRACOLoader to use examples/jsm/utils/WorkerPool.js, this problem may have something to do with allocating too many Web Workers or installing the Draco library multiple times... A shared worker pool could help with that.

It's very likely that KTX2Loader has a similar issue, and would need the same fix.

@gkjohnson
Copy link
Collaborator

In #20973 the docs were updated to recommend not creating a new DracoLoader per model loaded because the wasm seems to be loaded and instantiated multiple times. Is there a reason to create multiple loaders?

@manthrax
Copy link
Contributor

Consider a module that may not have access to the loader created by the hosting framework, or incidentally created by some other opaque module in the application. Your only option is to allocate one, but if anyone else has already created one, this problem could come up. Also, the gltfloader can be configured in different ways upon construction. Seems like there needs to be some mechanism to mediate these concerns.

Additionally, DRACO can be used standalone or by attaching it to GLTFLoader.. but this problem seems to imply that only one usage will be allowed?

I've also seen a few users attempt to use GLTFLoader for a draco encoded gltf, and then give up when it doesn't work, and then switching to using FBX or similar.. but really they just needed the extra step of making a DRACOLoader.
I also wouldn't mind of GLTFLoader just wrapped the loading of DRACOLoader (and even BasisTexture loader?) as a config flag.
It already does this implicitly for TextureLoader...

I have seen some related threads that seem to imply that there is already some direct Basis texture support in GLTFLoader?
I've also seen apps that are using GLTFLoader+DRACO, and then separately loading textures via BasisTextureLoader. Would love if the blender exporter could export embedded basis textures out of the box?

Have things progressed beyond the discussion here: https://discourse.threejs.org/t/compressed-texture-workflow-gltf-basis/10039/20 ?

@donmccurdy
Copy link
Collaborator

donmccurdy commented Aug 29, 2021

Is there a reason to create multiple loaders?

@gkjohnson I don't think there's much reason an application couldn't reuse a singleton DRACOLoader instance, but the fact that it's necessary (and has rare, unpredictable bugs if you don't) is not intuitive, and most other loaders don't have this restriction. I think ideally multiple instances would 'just work' with a shared WorkerPool instance, and it'd be beneficial to refactor DRACOLoader to use WorkerPool anyway.

I also wouldn't mind of GLTFLoader just wrapped the loading of DRACOLoader (and even BasisTexture loader?) as a config flag.

@manthrax We could wrap the construction of the loaders perhaps, but not the WASM binaries they rely on since the user has to provide a URL path to those... so in any case the user needs to do some configuration, and currently that config happens on DRACOLoader. Hopefully the error messages shown in these cases are clear about what to do, I understand sometimes users don't look at console output but it's hard to do much more in this case.

I have seen some related threads that seem to imply that there is already some direct Basis texture support in GLTFLoader?

Yes, KTX2Loader (for .ktx2 textures) is what you'll want here. It's Basis Univeral compression in a standardized container format, similar to DDS or KTX 1.0, whereas .basis-extension files are more like the raw texture data and not as portable. But it's the same compression. The glTF compression example uses both KTX2/Basis compression and Meshopt compression. Either can be applied to existing models using gltfpack or gltf-transform, or existing textures can be compressed to KTX 2.0 with the basisu or toktx CLI tools.

Would love if the blender exporter could export embedded basis textures out of the box?

Not sure if this is officially planned... it probably would need to go into Blender itself, since Python addons can't provide native code. On the other hand it could be added to Renaud Rohlinger's Blender scripts, which enhance Blender's glTF addon with some glTF-Transform functionality... it doesn't expose KTX2 compression yet but that seems like a minor addition if it already bundles glTF-Transform. I'm curious if that would be a good workflow — KTX2/Basis compression sometimes needs a bit of visual QA to get tuned right for a particular texture (artist guide).

@donmccurdy
Copy link
Collaborator

Any preferences here on the question in #20958 (comment)?

@manthrax
Copy link
Contributor

manthrax commented Oct 2, 2021

No strong opinion here.. I can see the trickyness inherent in sharing a workerpool betw different instances of the loaders. Seems easiest for each loader instance to fire up its own workers.. That way it works if used naively.. you don't have to worry about the versioning problem you mentioned.. and if you, as a user are worried about the worker overhead, you can reuse a single loaders, to avoid incurring the overhead of multiple workers. Hopefully this is actually related to what you were asking? :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants