-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
BasisTextureLoader: Next steps #16524
Comments
I haven't looked into the code in detail yet but could we synchronously return I think it's good for the consistency. And if we don't synchronously return texture user needs to call 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;
}); |
I haven't checked yet to confirm that works with |
Other cleanup: the properties assigned to the texture are a little arbitrary (left over from Basis demo), like |
If I'm right mipmaps doesn't seem to be supported. Transcoder doesn't support? Or loader hasn't just implemented mipmaps support yet? |
A
|
Yup, the transcoder should support it. You pass the mip level as the |
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. |
Made an example. #16553 It may be too simple, please feel free to enhance/replace later if it's merged. |
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 The most obvious way to support this would be to add an alternative to // 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 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. |
Another note, just to keep this tracked somewhere: the JS wrapper in |
Should BasisTextureLoader work in tandem with glTF? I've tried manually encoding textures to .basis format and adding BasisTextureLoader as a loader like this:
But the textures are not rendered properly, and the console has the following output:
|
same Problem as @zeux describe |
I debugged this and this happens because:
|
@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 KhronosGroup/glTF#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. |
Submitted PR to fix mipmap support - with this as long as you convert textures with |
From the Basis docs –
We've used a 512x768 texture in this demo, and should probably replace it with something that fits that constraint. |
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. |
Various updates to BasisTextureLoader coming in #16675. |
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:
So far the API matches |
Changing to a square power-of-two texture in #16686 fixes the demo on iOS, but mipmaps aren't working:
Do we need to do anything particular for mipmaps with PVRTC? |
Is that happening for one of the last few mips? |
@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 ) => {
// ...
} ); |
The asynchronous solution look more easy to implement with the worker and align with the others threejs's loaders. |
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: |
@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! |
@donmccurdy Will Basis alpha support be released soon? |
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? |
I think on iOS it will likely use PVRTC. Does this happen in an earlier version (r108)? |
The bug I filed is for r108.
Was just in the process of doing so: BinomialLLC/basis_universal#78 |
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. |
I've posted an update on BinomialLLC/basis_universal#78 (comment) -- screenshots included there. It shows the wrapping-around problem with a non-spinning cube. |
I found another bug (but most likely unrelated): #17546 (comment) |
Fixed in #17622. |
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 |
For now, you can disable mipmaps to make the texture show: 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. |
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 ( three.js/examples/js/loaders/BasisTextureLoader.js Lines 170 to 171 in e66e869
basisu without the -mipmap option generates. However it's still black.
|
Can you share the file? I was doing this as recently as last week, although it was Basis in a And just to confirm – you're aware that you can't generate these mipmaps at runtime, and would have to make do with the interpolation constraints involved? |
Sure, and thanks for taking a look!
Yes, I figured that out too, a bit of a shame since much of the the gain I saw in lower filesize of .basis compared to a compressed jpg is then lost - I know the GPU memory gain is still there, but I was mostly focused on download/transfer size of the texture. |
Is this always the case when using basis instead of jpg/png? What about a use-case of packing 6 textures as facelist for a cube map into a single basis file as the format supports/mentions this on the README. Is PMREM generator useless here and the basis file should have mipmaps for each texture generated? What about providing this packed texture data for use as a cubemap in ThreeJS? Normally you'd pass args for each individual texture? (I haven't looked into this texture loader support yet to see if a basis file with multiple textures is possible) An alternative perhaps is via KTX2 which might more suitable for providing a facelist packed basis file to be used as a cubemap? One final question about this usage as you were discussing handling of alpha, these textures could be encoded as RGBE or RGBM(ThreeJS has support for M7 and M16). I haven't investigated how well those compress with basis, but they can have similar issues as normal maps have been known to. The alpha channel data is fairly important to have supported there, I'm not sure what compressed texture format they'll be transcoded to, some may produce pretty poor results as the linked article below addresses. ARM wrote an article about issues with ASTC and RGBM for example. Unity engine as the article mentions uses RGBM5, which should in most cases cover a suitable HDR range, the lower multiplier presumably would produce less compression quality issues than a higher multiplier constant if the data fits within the range limits. |
Sorry, I don't know the answer to these questions. You may have better luck on the Basis Universal or KTX GitHub repositories. I do know that KTX2 is designed to support cubemaps with Basis payloads. |
I raised them here as they seemed relevant cases for using the ThreeJS support with being discussed here. Basis can store all 6 textures for a cubemap in a single basis file. KTX2 with Basis support can cater to better supporting cubemaps with multiple textures but a single file afaik. ThreeJS being able to handle it in future is unclear. Perhaps 6 different basis files need to be provided, or gain KTX2 with basis loader support. RGBM5 would need a separate PR, it'd just be RGBM7 or RGBM16 that currently exist, or a variant that takes a uniform to adjust the multiplier value. The main important part there is the alpha needs to be handled appropriately to compress right, so being able to have some control on that as discussed earlier in this issue is another example of where such would be useful, similar to Normal Maps and their Linear/non-colour encoding, which may also have the format split into two possible textures(from single colour RGB for X, and alpha for Y in the basis file) depending on compression support. |
Three.js currently supports Basis Universal. A new Basis format, UASTC, was released just a few weeks ago. I do not think Basis supported cubemaps in a single file prior to that. Pull requests would be welcome. A KTX2 loader is in progress, with #18490. |
Older versions of the README for Basis(prior to UASTC, but still present in current README) shows mention of the multiple texture packing into single basis file feature:
I would not know where to start, nor do I have the spare time at present. Perhaps once this issue progresses along to completion and the KTX2 loader is ready, the compressed/packed cubemap support could be addressed. If I have spare time by then, I'd be happy to try contribute a PR :) |
Hi all, I've got an intermittent issue on iOS (aawww maaaan!) [EDIT] just got the same issue on Safari on Mac OS, still intermittent |
@igghera Sounds like this may be a bug, especially if it's happening on the official example. Do you mind opening a new issue for this? Thanks! |
How might one go about adding support for 2D array textures in Changing the
Then, for video textures, it'd probably need a different implementation. Rather than transcoding in batch at once, you'd keep the basis file handle open and have some way to request the next frame. |
See #19676 for more on compressed array textures. |
I don't think there is anything left to do from the checklist in this issue, after #21131. Rather than keep this issue open indefinitely let's close it and start new issues for future bugs or requests. |
Initial support for .basis textures added in #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:
setMaxWorkers()
methodload()
(added at BasisTextureLoader: Refactor #21131)Recompile Basis transcoder with bootstrapping suggested by @austinEngSupport user-configurable transcode output formatThe text was updated successfully, but these errors were encountered: