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

KTX2Loader #18490

Merged
merged 2 commits into from
Jul 2, 2020
Merged

KTX2Loader #18490

merged 2 commits into from
Jul 2, 2020

Conversation

donmccurdy
Copy link
Collaborator

@donmccurdy donmccurdy commented Jan 27, 2020

Implements a basic loader for KTX 2.0. The KTX format includes a lot of features that aren't covered here (cubemaps, 3D textures, HDR, lots of hardware-specific compressed texture formats), but this PR supports only Basis Universal 2D textures. Both low- and high-quality Basis variants are supported: ETC1S and UASTC.

KTX2 — or at least this subset of it — is a prerequisite to support for Basis Universal in GLTFLoader. There's a fair bit more work to do here (see the "To do" section in the KTX2Loader JSDoc). Until more of that work is done, I'd probably skip adding documentation or a more flashy demo — we'll polish this up, but "caveat emptor" until then.

@mrdoob mrdoob added this to the rXXX milestone Jan 27, 2020
@MarkCallow
Copy link

There is no option for ETC2. I don't know if this is a THREE issue or just a mistake in this code. If THREE.RGB_ETC1_Format really only supports ETC1 then that is not good. There is a WebGL_compressed_texture_etc extension for ETC2_EAC.

If an RGBA texture is transcoded to RGB_ETC1 then, obviously, they'll be no alpha channel. The right thing to do in this case, assuming THREE fails to support WebGL_compressed_texture_etc, is to create 2 ETC RGB textures, 1 with the RGB components and 1 with the alpha then have the shader read and apply the alpha from the second texture. To do this the loader will need to call transcodeImage twice, once with the transcodeAlphaToOpaqueFormats parameter set to true.

@donmccurdy
Copy link
Collaborator Author

ETC2 coming up: #18581.

MarkCallow pushed a commit to KhronosGroup/KTX-Software that referenced this pull request Feb 10, 2020
* Import KTX2Loader from threejs PR.

- Imports KTX2Loader from open PR (mrdoob/three.js#18490) to consolidate development.
- Adds an import map shim, to properly link KTX2Loader to the threejs CDN.
- Removes requestIdleCallback invocation, so demo can be tested on Safari.

* Remove KTX2Loader.

* Add comment to explain import map shim

* Include ETC2 in format labels.
MarkCallow pushed a commit to MarkCallow/KTX-Software that referenced this pull request Feb 29, 2020
* Import KTX2Loader from threejs PR.
- Imports KTX2Loader from open PR (mrdoob/three.js#18490) to consolidate development.
- Adds an import map shim, to properly link KTX2Loader to the threejs CDN.
- Removes requestIdleCallback invocation, so demo can be tested on Safari.
* Remove KTX2Loader.
* Add comment to explain import map shim
* Include ETC2 in format labels.
@MarkCallow
Copy link

@donmccurdy when I try to run the llt-three test in KTX-Software now I am getting

SyntaxError: import not found: RGBA_ETC2_EAC_Format

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Mar 18, 2020

@MarkCallow the various three@0.113.2 references in the example will need to be updated to three@0.114.0 for ETC2 EAC. I can do that here, but can't necessarily keep both repositories updated regularly while developing this PR.

@MarkCallow
Copy link

Thanks @donmccurdy. I've fixed it.

@donmccurdy donmccurdy marked this pull request as ready for review July 1, 2020 21:45
@donmccurdy
Copy link
Collaborator Author

I think this should be ready for review, and perhaps inclusion in r119. There's more work to do, but I'd like to get the basics covered so we can begin testing this with GLTFLoader.

@MarkCallow
Copy link

By all means review but this should not be released until it supports zstd inflation. zstd is an important part of getting UASTC textures to a more reasonable transmission size. toktx is fully capable of encoding such textures.

@polarathene
Copy link

this should not be released until it supports zstd inflation.

Is it that important to hold back a release though? A future update could bring that support no?

I'd like to use the cubemap support KTX has with basis packed texture(single file with each face separate texture included), but I don't think that should hold back the release.

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Jul 2, 2020

@MarkCallow Note that we'll support gzip out of the gate — the web browser handles that automatically in the HTTP request layer, so the user just needs to gzip the .ktx2 file. I realize zstd gives better compression, and I'd like to support both, but UASTC should be pretty usable even without it.

@mrdoob mrdoob modified the milestones: rXXX, r119 Jul 2, 2020
@mrdoob mrdoob merged commit 921c23e into mrdoob:dev Jul 2, 2020
@mrdoob
Copy link
Owner

mrdoob commented Jul 2, 2020

Thanks!

@donmccurdy
Copy link
Collaborator Author

@MarkCallow FYI, we cut a release at the end of each month — so KTX2Loader will be available by August 1st.

@donmccurdy donmccurdy deleted the feat-ktx2loader branch July 2, 2020 01:47
@MarkCallow
Copy link

MarkCallow commented Jul 2, 2020

I view use of zstd as a basic way of making a KTX file with a UASTC payload. If people start making KTX files without zstd and using external gzip instead, they are likely to continue doing so. In other words I don't want them getting into bad habits. External gzip means that random access to the mip levels and thus streaming is not possible.

@polarathene, cubemap support is different in that extends the types of texture structure supported. It's absence is not leaving out a fundamental building block. Limiting people to 2D textures today isn't going to develop bad habits that will affect making of cubemaps in future.

The main issue with a zstd decoder is making a JS wrapper so KTX2Loader.js can call it. We already have a single file decoder of c code compilable with Emscripten that does the real work. You can find it at https://github.com/KhronosGroup/KTX-Software/blob/master/lib/zstddeclib.c. I have not made a wrapper because I am not a JS expert. It seems eminently doable within 1 month. Only 4 functions are needed: ZSTD_createDCtx, ZSTD_decompressDCtx, ZSTD_isError and ZSTD_freeDCtx.

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Jul 6, 2020

I disagree with the argument that external gzip would a bad habit. Even once we have zstd support, gzip has some advantages — on the web, at least — that make zlib vs gzip a more nuanced question than just X% size difference and random access to mip levels. We'll support zstd as soon as we have an implementation, but I do see other issues — like non-blocking transcoding — as more urgent. We could disable UASTC support entirely and KTX2Loader would remain a very useful addition to the next release: our users are already hacking .basis files (with ETC1S) into glTF and we need to provide a future-proof alternative.

I have not made a wrapper because I am not a JS expert.

Unfortunately there seem to be very few people comfortable enough with both C/C++ and JS to write these wrappers. The current state of WASM tooling (e.g. how do you distribute it in package repositories?) is a headache for JS developers too, I'm afraid. 😇 I see a handful of zstd implementations already on npm, but none look very promising and we may need to create our own.

@arpu
Copy link

arpu commented Jul 6, 2020

for zstd the is this official emscript example https://github.com/facebook/zstd/tree/dev/contrib/single_file_libs/examples

for me as js dev i would like to see the KTX2Loader with basis without UASTC support ( smaller wasm builds)

@MarkCallow
Copy link

MarkCallow commented Jul 6, 2020

for zstd the is this official emscript example https://github.com/facebook/zstd/tree/dev/contrib/single_file_libs/examples

Unfortunately it does not have a JS wrapper. The c code in emscripten.c calls the decompressor as well as doing all the GL drawing. My libktx.js wrapper uses the same single file decoder which it also calls from c code. The JS call is to create a ktxTexture object from the incoming data. During this the decoder is called to inflate the data.

for me as js dev i would like to see the KTX2Loader with basis without UASTC support ( smaller wasm builds)

We need to try building with and without UASTC support before making any such decision. The bulk of the code is tables, some of which are shared. It would also be possible to either download just the necessary tables once the file format is known or to generate them on the fly. So please no knee jerk reactions that we have to get rid of UASTC support to make things small. Study is needed first.

@MarkCallow
Copy link

The current state of WASM tooling (e.g. how do you distribute it in package repositories?) is a headache for JS developers too, I'm afraid.

Why is that? Only a .js and a .wasm file need to be distributed. Can't the package repositories handle .wasm files?

😇 I see a handful of zstd implementations already on npm, but none look very promising and we may need to create our own.

Are these pure JS or do they use zstddeclib.c? In what ways do they not seem promising?

@donmccurdy
Copy link
Collaborator Author

Why is that? Only a .js and a .wasm file need to be distributed. Can't the package repositories handle .wasm files?

No, JS package repositories don't handle .wasm files properly. Hopefully they will, with time. For now developers have to copy the WASM and JS files into their applications manually, because the build tools for repositories like npm cannot. We give users instructions on how to do that, but it becomes a burden when you consider all of the additional files that a glTF loader might need:

  • draco_decoder.js
  • draco_decoder.wasm
  • basis_transcoder.js
  • basis_transcoder.wasm
  • zstd_decoder.js
  • zstd_decoder.wasm

Users who want to support older browsers (with plain JS instead of WASM) will need even more files. Each file is downloaded by the web application individually. We've discussed this a bit on the forum, more focused on Draco than KTX, but the same issues apply. We basically have to invent best practice as we go, with WASM. 😕

Are these pure JS or do they use zstddeclib.c? In what ways do they not seem promising?

Most were written for Node.js environments, which means they use native addons instead of WASM. The one browser-compatible package I spotted was a 3MB library, which is clearly too large. Based on the comments from you and zeux I'm confident we can do better than that, but it does increase the scope here.

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

Successfully merging this pull request may close these issues.

6 participants