mosra / magnum-plugins

Plugins for the Magnum C++11 graphics engine
https://magnum.graphics/
Other
98 stars 59 forks source link

KTX2 + Basis Universal #110

Closed pezcode closed 2 years ago

pezcode commented 3 years ago

I had time to kill so I spent an afternoon investigating how to hook up KtxImporter with Zstandard supercompression and the latest Basis Universal formats. Mainly writing this down so I don't forget again, but also happy to get some input.

Zstandard

This was rather trivial to add to KtxImporter, but might be unneeded (more below).

Updating basisu

Updating basis_universal to 1.15 for BasisImporter worked fairly quickly, too, with two caveats:

Forwarding Basis-compressed KTX2 to BasisImporter

This one gave me a bit of a headache. I naively assumed BasisImporter could handle the raw data inside KTX2 level data, but currently it only decodes .basis containers. There are ways to use the lowlevel transcoders, but for BasisLZ (Basis ETC1 + LZ supercompression) this is completely insane because you need to validate and decompress the global LZ data and somehow pass that through to BasisImporter.

The far easier solution here is to detect Basis data in KTXImporter early and then directly send the entire file data to BasisImporter. Basisu 1.15 can decode KTX2 and handles all the BasisLZ supercompression stuff for us. BasisImporter then only needs a few minor changes to use ktx2_transcoder next to the existing basisu_transcoder (which only supports .basis containers).

Another effect would be that basisu handles all Zstandard decoding, so that might not have to be handled in KtxImporter, unless it's commonly used outside of Basis-compressed KTX files. Needs some investigation.

mosra commented 3 years ago

Thanks a lot for this!

Updating basisu

Last time I tried updating I got some rather nasty crashes (or ASan failures? don't remember), which is why I put this on the backlog and never looked at it again. If you don't get a crash with 1.15, that's great, the problems solved themselves it seems. Updating the thresholds is completely fine, the images I tested with are extremely small and rather busy so large differences are expected.

Some other tasks that come to mind -- not essential to enabling proper Basis support for glTF, but good to eventually have done as well:

Forwarding Basis-compressed KTX2 to BasisImporter

I naively assumed BasisImporter could handle the raw data inside KTX2 level data

Yep, my original thought was about doing this manually as well. Since the KTX import is completely under our control and the spec is clear, passing the data to an external tool should be doable -- if the external tool provided a usable interface to consume such data. Which it doesn't, and given the state how feature requests and code contributions are dealt with in the Basis project, I think we shouldn't waste time with this at all. Sigh.

The far easier solution here is to detect Basis data in KtxImporter early and then directly send the entire file data to BasisImporter

That's what I think should be done as well, yes. And as you say, it deals nicely with the compression lib dependency as well. What I'm wondering about is the implications on binary size on the web:

Additionally, I need Basis KTX support to handle 2D array images, but since the original .basis files supported that, I hope their KTX reader/writer has that too.

Zstandard

Apart from what Basis handles on its own, I don't have an immediate need for zstd-compressed KTX files and there isn't many KTX files in the wild, so I wouldn't deal with this for now. Yes, the proposed KHR_texture_ktx glTF extension that I want to make use of requires zstd, but since it's still at a proposal stage, there aren't any real glTF+KTX files either, and so I think I could take the liberty of allowing uncompressed files as well.

[Sorry, this is a rather long reply already, but here goes a dump of my research --]

From a brief look at the KTX spec, they don't seem to be doing anything special with the data before sending them to the compressor, which leads me to think that there would be very little size difference between a KTX file with internal zstd compression and a KTX file compressed with a generic zstd library externally. In contrast to various filtering OpenEXR is doing before sending channel data to the compressor it feels like a missed opportunity to me -- and the processing used in OpenEXR isn't anything that would be too complex to be included in the KTX spec.

Given the above, and without dealing with arbitrary external KTX files, I'm thinking KTX files without an internal compression are actually a better storage option:

[-- end of the dump.]

Here I don't see any actionable items so far -- unless priorities change, first place where I could see zstd being used is for the filesystem abstraction lib in mosra/corrade#39, and there I would prefer an external dependency, no bundling. 600 kB is quite a lot -- that's the size of json.hpp we now happily made obsolete with #107 :)

pezcode commented 3 years ago

I got some rather nasty crashes (or ASan failures? don't remember)

Good point, I'll check again with ASan

UASTC support [...] I hope it's just about adding some extra formats and (de)compression options, not a whole new API altogether

From a quick glance that's done transparently by the transcoder, and the encoder only needs to flip a single bool to get UASTC output.

raw printf() calls that I couldn't override/redirect in any way

That's still there, with no way to override it. It has a macro BASISU_DEVEL_ERROR but it always defines that, so no way to redirect it even in source builds.

I need Basis KTX support to handle 2D array images

2D array images are supported: https://github.com/BinomialLLC/basis_universal/blob/master/transcoder/basisu_transcoder.cpp#L16803

mosra commented 3 years ago

Oh, forgot to reply on this part:

keeping compatability with old versions (mainly the prehistoric 1.11 on vcpkg) is a little trickier

In most packages (Arch, Homebrew, and even vcpkg) I'm directly fetching a particular commit from the Basis repo and building against that. Because Basis is such a janky project and it has no real buildsystem nor anybody cared to create any packages for any system, I think it's fine if you just upgrade to whatever tag/version/? is latest without keeping backwards compatibility -- the majority of people who's using the packages won't notice anything as the package transparently fetches the new Basis version, and people using Magnum as a subproject will just update the Basis submodule if they discover a breakage.

I went through this compatibility pain for SPIR-V Tools and glslang because those are essential projects and package managers have a ton of different versions against all of which it should compile. And there it's a hell because neither really has a version define, so it was a ton of try_compile() misery.

But Basis is a rather exotic thing (especially given that there are no packages for it at all whatsoever), so unless the compatibility is just a matter of a few trivial #ifs against some version define in some header (which, given the project history, I seriously doubt), it's a waste of time.

mosra commented 2 years ago

Done with #112 and #113 :tada: