BasisTextureLoader: Next steps

[donmccurdy]

Initial support for .basis textures added in https://github.com/mrdoob/three.js/pull/16522. This issue tracks remaining cleanup and planned enhancements. I'm not working on these yet, and will update this issue when I begin, so PRs are welcome in the meantime:

• [x] Clean up remaining TODOs in the code
• [x] Apply eslint fixes
• [x] Add documentation
• [x] Add example
• [x] Add setMaxWorkers() method
• [x] Support mipmaps
• [x] Fix mipmap support in iOS (working at https://github.com/mrdoob/three.js/pull/21131)
• [x] Return texture synchronously from load() (added at https://github.com/mrdoob/three.js/pull/21131)
• [x] Support alpha
• [x] Add ES module
• [ ] ~Recompile Basis transcoder with bootstrapping suggested by @austinEng~
• [ ] ~Support user-configurable transcode output format~

[takahirox]

I haven't looked into the code in detail yet but could we synchronously return texture from load as other texture loaders do?

I think it's good for the consistency. And if we don't synchronously return texture user needs to call material.needsUpdate = true in callback (if already started animation loop).

var material = new XXXMaterial();
textureLoader.load(..., function (texture) {
  material.map = texture;
   // .map is from null to non-null.
   // User needs to call material.needsUpdate = true here if already started animation loop
   // because whether material.map is null or not affects the final shader code.
  material.needsUpdate = true;
});

[donmccurdy]

I haven't checked yet to confirm that works with THREE.CompressedTexture, but if so I agree that would be best. 👍

[donmccurdy]

Other cleanup: the properties assigned to the texture are a little arbitrary (left over from Basis demo), like flipY=false. And there's an unused startTime variable in the worker.

[takahirox]

If I'm right mipmaps doesn't seem to be supported. Transcoder doesn't support? Or loader hasn't just implemented mipmaps support yet?

[donmccurdy]

A .basis file can contain multiple mipmap levels, yes. I think the transcoder supports it already, but I haven't tested that. BasisTextureLoader doesn't support it yet.

We should update to the new/smaller version of the transcoder, too: https://github.com/BinomialLLC/basis_universal/pull/7 Done.

[austinEng]

Yup, the transcoder should support it. You pass the mip level as the levelIndex to transcodeImage. https://github.com/BinomialLLC/basis_universal/blob/master/webgl/transcoder/basis_wrappers.cpp#L197

[takahirox]

Thanks for your explanations.

And if there is any other functionalities the loader hasn't supported yet, but transcoder does, you are aware of, please add to TODO list. We can help implementation.

[takahirox]

Made an example. #16553 It may be too simple, please feel free to enhance/replace later if it's merged.

[donmccurdy]

On other features the transcoder has but THREE.BasisTextureLoader does not, a key difference is that the transcoder can output many additional formats:

https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/BasisTextureLoader.js#L264-L273

I don't honestly know when to use all of these. I expect that the user will sometimes want control of that, and in other cases a loader (e.g. GLTFLoader) will make the decision based on the purpose of the texture. For example, it might choose a different compressed format for material.map (base color) than for material.aoMap (ambient occlusion).

The most obvious way to support this would be to add an alternative to detectSupport( renderer ):

// Let loader decide, based on device capabilities:
loader.detectSupport( renderer );

// Or, choose a particular format:
loader.setFormat( THREE.BasisTextureLoader.BASIS_FORMAT.cTFBC4 );

This has a potential problem – if I'm loading multiple textures, we might not want to transcode them all to the same format. We could create multiple loaders, but then it's harder to reuse existing Web Workers (which is important). We could pass a format into the load() method, but then it's not backward compatible with TextureLoader. I guess the best thing might be to ensure that doing this...

loader.setFormat( THREE.BasisTextureLoader.BASIS_FORMAT.cTFBC4 );
var fooTex = loader.load( 'foo.basis' );

loader.setFormat( THREE.BasisTextureLoader.BASIS_FORMAT.cTFBC1 );
var barTex = loader.load( 'bar.basis' );

... will always apply the right format to each texture, even though decoding is asynchronous.

[donmccurdy]

Relevant: http://www.reedbeta.com/blog/understanding-bcn-texture-compression-formats/#comparison-table

[donmccurdy]

Another note, just to keep this tracked somewhere: the JS wrapper in examples/js/libs/basis contains a minor change from the version in the Basis repository. The first declaration (var Module) is replaced with just Module to enable the lazy initialization used in a Web Worker. This can probably be done differently, either by compiling the transcoder with different flags or via https://github.com/BinomialLLC/basis_universal/issues/21.

[zeux]

Should BasisTextureLoader work in tandem with glTF? I've tried manually encoding textures to .basis format and adding BasisTextureLoader as a loader like this:

var basisLoader = new THREE.BasisTextureLoader();
basisLoader.setTranscoderPath( 'basis/' );
basisLoader.detectSupport( renderer );

THREE.Loader.Handlers.add( /\.basis$/i, basisLoader );

But the textures are not rendered properly, and the console has the following output:

[.WebGL-000002B68031CEF0]RENDER WARNING: texture bound to texture unit 1 is not renderable. It maybe non-power-of-2 and have incompatible texture filtering.

[arpu]

same Problem as @zeux describe

[zeux]

I debugged this and this happens because:

1. glTF loader usually sets mag filtering to LinearMipMapLinearFilter
2. BasisTextureLoader doesn't support mipmaps
3. webGL requires mip chains to be complete :-/

[donmccurdy]

@zeux Not officially supported – the core glTF spec allows only JPEG and PNG textures. But an extension for Basis (via KTX2 wrapper) is the mechanism by which we plan to add compressed texture support to the format. See https://github.com/KhronosGroup/glTF/pull/1612 for draft extensions (only the first two are relevant here) and feel free to add feedback.

That said, the lack of mipmap support in BasisTextureLoader is purely because we haven’t gotten to it yet. The transcoder itself should support that as far as I know.

[zeux]

Submitted PR to fix mipmap support - with this as long as you convert textures with -mipmap option, glTF loader just works as long as you add the loader as listed above, at least on desktop. I wasn't able to get it to run on mobile (iPhone or Android), but threejs.org example with the spinning cube doesn't work on iPhone either so that might be a separate issue.

[donmccurdy]

I wasn't able to get it to run on mobile (iPhone or Android)

From the Basis docs –

For example, on iOS, you can only use square power of 2 texture dimensions for PVRTC1, and there's nothing Basis can do for you today that works around this limitation. (We will be supporting the ability to trancode smaller non-pow2 textures into larger power of 2 PVRTC1 textures soon.)

We've used a 512x768 texture in this demo, and should probably replace it with something that fits that constraint.

[zeux]

Ok - that makes sense. FWIW the Android phone I was testing on has a whole host of issues with multiple WebGL demos, not just Basis texture related - a different Android phone runs just fine. So yeah it's probably just the power-of-two restriction that's problematic on iOS.

[donmccurdy]

Various updates to BasisTextureLoader coming in https://github.com/mrdoob/three.js/pull/16675.

[donmccurdy]

We should probably also think through how to support alpha... the Basis documentation goes into some detail on the options (https://github.com/BinomialLLC/basis_universal/#how-to-use-the-system) but for some devices this involves multiple transcoding outputs:

ETC1 only devices/API's: Transcode to two ETC1 textures and sample them in a shader. You can either use one ETC1 texture that's twice as high, or two separate ETC1 textures.

So far the API matches TextureLoader, which would need to change (or have an alternative API) to support returning multiple transcoded outputs from a single .basis texture.

[donmccurdy]

Changing to a square power-of-two texture in https://github.com/mrdoob/three.js/pull/16686 fixes the demo on iOS, but mipmaps aren't working:

INVALID_VALUE: compressedTexImage2D: length of ArrayBufferView is not correct for dimensions.

Do we need to do anything particular for mipmaps with PVRTC?

[zeux]

Is that happening for one of the last few mips?

[donmccurdy]

I didn't debug closely enough to identify which mips, but the error is printed three times, and the last three did all have the same buffer size. 🤔

[zeux]

I suspect that Basis doesn't compute the size (in bytes) of last few mips correctly. For PVRTC1 4bpp, dimensions of blocks are rounded to 8, so 4x4, 2x2 and 1x1 should be the same size as 8x8. I think Basis transcoder rounds to 4x4. All images of sizes 8x8, 4x4, 2x2, 1x1 must take 32 bytes; I think for 4x4 and below Basis assumes it's just 1 4x4 block that's 8 bytes and gives you 8 bytes instead of 32. @richgel999

[zeux]

The formula for computing image sizes is here: https://www.khronos.org/registry/OpenGL/extensions/IMG/IMG_texture_compression_pvrtc.txt:

   For PVRTC 4BPP formats the imageSize is calculated as:
     ( max(width, 8) * max(height, 8) * 4 + 7) / 8

[zeux]

This workaround (in the worker just before calling transcode) seems to work for me. This needs to be fixed in the transcoder of course, but it's easy to validate this in JS:

var mipWidth = basisFile.getImageWidth( 0, mip );
var mipHeight = basisFile.getImageHeight( 0, mip );
var mipSize = basisFile.getImageTranscodedSizeInBytes( 0, mip, config.format );

if ( config.pvrtcSupported ) {

    // Basis incorrectly computes mip sizes for PVRTC, let's fix them up using the spec:
    // https://www.khronos.org/registry/OpenGL/extensions/IMG/IMG_texture_compression_pvrtc.txt
    mipSize = Math.floor((Math.max(mipWidth, 8) * Math.max(mipHeight, 8) * 4 + 7) / 8);

}

var dst = new Uint8Array( mipSize );

[richgel999]

Thanks! I'll fix this ASAP.

[richgel999]

The PVRTC1 mipmap size bug should be fixed. We fixed the transcoder so it’ll clear the extra bytes on small (less than 8 pixel wide/tall) mips. And we fixed the wrapper to return the correct sizes. Please let me know if there are any issues and I’ll fix them ASAP.

[Ben-Mack]

@donmccurdy Could you add "Synchronously return texture from load method" to the todo list? (as takahirox suggested above)

[donmccurdy]

@Ben-Mack Added. Note that because textures with an alpha channel may transcode to multiple textures, and we won't know that synchronously, we'll need a different API for it.

[donmccurdy]

@richgel999 Thank you! I'm having trouble rebuilding the WASM transcoder, as https://github.com/BinomialLLC/basis_universal/commit/ab722fa2e18536f9a1d5f33814f3088232446d52 only updated webgl/transcoder/basis_wrappers.cpp. Compiling on macOS:

$ emcmake cmake ../
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/donmccurdy/Documents/Projects/basis_universal/webgl/transcoder/build

$ make
[ 33%] Linking CXX executable basis_transcoder.js
Traceback (most recent call last):
  File "/Users/donmccurdy/Documents/Projects/emsdk/emscripten/1.37.22/em++", line 16, in <module>
    emcc.run()
  File "/Users/donmccurdy/Documents/Projects/emsdk/emscripten/1.37.22/emcc.py", line 882, in run
    exec 'shared.Settings.' + key + ' = ' + value in globals(), locals()
  File "<string>", line 1, in <module>
NameError: name 'emmalloc' is not defined
make[2]: *** [basis_transcoder.js] Error 1
make[1]: *** [CMakeFiles/basis_transcoder.js.dir/all] Error 2
make: *** [all] Error 2

In theory we should be able to update the transcoder and replace the current PavingStones.basis texture with this one, which includes mipmaps:

PavingStones.basis.zip

EDIT: Oops, this might be related to https://github.com/BinomialLLC/basis_universal/pull/27.

[zeux]

@donmccurdy I believe this requires https://github.com/BinomialLLC/basis_universal/pull/27 to be merged, hopefully this can happen soon. Also your emscripten version may predate the existence of emmalloc, the files that are currently part of three.js were built with 1.38.31 I think.

[Ben-Mack]

This error occurred while I try to load multiple basis files in the same time:

Uncaught (in promise) DOMException: Failed to execute 'postMessage' on 'Worker': ArrayBuffer at index 0 is already neutered.

To reproduce, just call load method 2 times like:

loader.load( 'textures/compressed/PavingStones.basis');
loader.load( 'textures/compressed/PavingStones.basis');

[donmccurdy]

@Ben-Mack It's probably still worth fixing, but that error is only because you're loading the exact same texture twice, and the loader reuses an ArrayBuffer that can't be transferred to two workers at the same time. The code below runs, at the expense of doing twice the work:

loader.load( 'textures/compressed/PavingStones.basis?v=1' );
loader.load( 'textures/compressed/PavingStones.basis?v=2' );

[Makio64]

@donmccurdy For the support of alpha what about something like :

load(...) // as usual
//then :  
loader.getRGBTexture() //return by the load function as usual
loader.getAlphaTexture() //can be use as alphaMap

An option to support alpha might also be add on the model of mipmap: loader.generateAlpha = true //default

[donmccurdy]

@Makio64 Something like that! Most threejs loaders are not stateful (a single loader could be working on multiple things in parallel) so maybe:

const [ map ]           = loader.loadRGB( 'foo.basis', { ...options } );
const [ map, alphaMap ] = loader.loadRGBA( 'bar.basis', { ...options } );

^In both cases, but especially alpha, I think there may be enough different ways to do things to require some configuration on the method.

Or we could just go asynchronous on the new methods, instead:

loader.loadRGBA( 'bar.basis', { ...options }, ( map, alphaMap ) => {
  // ...
} );

[Makio64]

The asynchronous solution look more easy to implement with the worker and align with the others threejs's loaders.

[mozg4D]

I'm only able to load textures with resolution 768 by 512 or 384 by 256 etc. any other resolution fails to load with Three.js and BasisTextureLoader with warning: "texture bound to texture unit 0 is not renderable. It maybe non-power-of-2 and have incompatible texture filtering."

[donmccurdy]

@mozg4D please see the Basis documentation, in particular power-of-two textures are required in iOS. If the documentation doesn't match the behavior you're seeing, please file a new bug. It's also possible that the texture filtering selected is incompatible, in which case we'd still need a demo and a new bug would be helpful. Thanks!

[Ben-Mack]

@donmccurdy Will Basis alpha support be released soon?

[nh2]

I think I found a bug: On iOS only, there's a weird "texture glowing" effect across texture-geometry edges: #17597

Has anybody else encountered this?

[shrekshao]

I think on iOS it will likely use PVRTC. Does this happen in an earlier version (r108)? Maybe you could try file an issue in https://github.com/BinomialLLC/basis_universal?

[nh2]

Does this happen in an earlier version (r108)?

The bug I filed is for r108.

Maybe you could try file an issue in https://github.com/BinomialLLC/basis_universal?

Was just in the process of doing so: https://github.com/BinomialLLC/basis_universal/issues/78

[richgel999]

I think I found a bug: On iOS only, there's a weird "texture glowing" effect across texture-geometry edges: #17597

I replied on the basis_universal github. We'll grab the texture and see what's happening. Most likely, it's an artifact related to not setting the wrap/clamp addressing transcode flag correctly, or an artifact caused by our real-time PVRTC1 encoder. If either is the problem, there are usually workarounds. We can also increase PVRTC1 quality, at the cost of more transcode CPU time/memory.

[nh2]

I've posted an update on https://github.com/BinomialLLC/basis_universal/issues/78#issuecomment-536159690 -- screenshots included there.

It shows the wrapping-around problem with a non-spinning cube.

[nh2]

I found another bug (but most likely unrelated): https://github.com/mrdoob/three.js/pull/17546#commitcomment-35275564

[nh2]

I found another bug (but most likely unrelated): #17546 (comment)

Fixed in #17622.

[supermoos]

In terms of mipmaps, is there a way to properly load basis texture files (referenced inside a .gltf file) without having to embed the mipmaps in the .basis file?

I can get it to load proplery when I generate the .basis file with -mipmap, however this adds a lot of filesize to the .basis file - but when I generate a .basis file without -mipmap option, it simply shows as black in the browser with threejs.

[JohannesDeml]

In terms of mipmaps, is there a way to properly load basis texture files (referenced inside a .gltf file) without having to embed the mipmaps in the .basis file?

I can get it to load proplery when I generate the .basis file with -mipmap, however this adds a lot of filesize to the .basis file - but when I generate a .basis file without -mipmap option, it simply shows as black in the browser with threejs.

For now, you can disable mipmaps to make the texture show: https://discourse.threejs.org/t/compressed-texture-workflow-gltf-basis/10039/12?u=johannesdeml https://github.com/JohannesDeml/three.js/commit/909d9cc6dc9192f398df7455f52b7e71e3bf61e2

That of course is no support for mipmaps, but if your goal is to just show textures, that might be an easy solution for you.

[supermoos]

That doesn't seem to be working anymore, and it looks like the BasisTextureLoader now also has similar code (setting the min/magFilter = LinearFilter) when only 1 mipmap is detected (https://github.com/mrdoob/three.js/blob/e66e86901abd84ffc260fea9665170631a2b0493/examples/js/loaders/BasisTextureLoader.js#L170-L171) - which is what basisu without the -mipmap option generates. However it's still black.