pmndrs / three-stdlib

📚 Stand-alone library of threejs examples designed to run without transpilation in node & browser
https://npmjs.com/three-stdlib
MIT License
676 stars 110 forks source link

GLTFLoader does not load external ktx2 textures #361

Closed catalin-enache closed 2 months ago

catalin-enache commented 2 months ago

Problem description:

Loading this GLTF asset https://github.com/KhronosGroup/glTF-Sample-Assets/tree/main/Models/AnisotropyBarnLamp/glTF-KTX-BasisU which has external ktx2 textures fails with error: THREE.GLTFLoader: Couldn't load texture.
This is raise by GLTFLoader at line 2658

Relevant code:

I'm doing all the required setup:

ktxLoader = new KTX2Loader()
  .setTranscoderPath( 'jsm/libs/basis/' )
  .detectSupport( renderer );

const loader = new GLTFLoader().setPath( 'models/gltf/' );
loader.setKTX2Loader( ktx2Loader );
loader.setMeshoptDecoder( MeshoptDecoder );
loader.load( 'AnisotropyBarnLamp.gltf', function ( gltf ) {
  scene.add( gltf.scene );
});

Suggested solution:

Looking through the code I found that ktx2Loader is not used inside the GLTFParser even it is sent in options from GLTFLoader

It seems to be fixed with a bit of a patch inside GLTFParser. If we change these lines from:

    if (typeof createImageBitmap === 'undefined' || isSafari || (isFirefox && firefoxVersion < 98)) {
      this.textureLoader = new TextureLoader(this.options.manager);
    } else {
      this.textureLoader = new ImageBitmapLoader(this.options.manager);
    }

to:

    if (this.options.ktx2Loader) {
      this.textureLoader = this.options.ktx2Loader;
    } else if (typeof createImageBitmap === 'undefined' || isSafari || (isFirefox && firefoxVersion < 98)) {
      this.textureLoader = new TextureLoader(this.options.manager);
    } else {
      this.textureLoader = new ImageBitmapLoader(this.options.manager);
    }

the external ktx2 textures will be loaded. I'm not sure if this is the right approach to fix it though.

There is a sandbox that works when using suggested patch: https://codesandbox.io/p/sandbox/young-waterfall-22pkx4?file=%2Fsrc%2Findex.js%3A8%2C61 Please play with different imports (original/patched) (also note that it takes long to load the asset): image

catalin-enache commented 2 months ago

Just realised that the fix only works with gltf having ktx2 textures but will make it crash if there are other kinds of textures. We need to also check for the file extension. A quick finding would be something like:

const isKTX2 = json.images?.some((img) => img.uri?.split('.').pop() === 'ktx2'); // assuming all images are the same type
if (isKTX2 && this.options.ktx2Loader) {
      this.textureLoader = this.options.ktx2Loader;
    } else if (typeof createImageBitmap === 'undefined' || isSafari || (isFirefox && firefoxVersion < 98)) {
      this.textureLoader = new TextureLoader(this.options.manager);
    } else {
      this.textureLoader = new ImageBitmapLoader(this.options.manager);
    }
CodyJasonBennett commented 2 months ago

Does this also affect the upstream GLTFLoader in three? We can consult there as well and backport progress.

catalin-enache commented 2 months ago

Ah sorry, I was so sure that I created the issue in main ThreeJS repo :) I realise now that I was raising it in the three-stdlib repo. I'll report it there too. Thanks for feedback !

catalin-enache commented 2 months ago

I just reported it in ThreeJs repo (https://github.com/mrdoob/three.js/issues/28258), I will close it here. Thank you !

catalin-enache commented 2 months ago

It seems the issue was with the official gltf asset not declaring required extensions.