feat(ktx): add initial ktx input/output support.

[walcht]

Description

Fixes #2329. Add initial KTX2 format support (closest similar plugin/format in OIIO is DDS).

This KTX plugin support obviously nullifies the benefits of using KTX in the
first place. That being said, this plugin is still useful so that end users
don't have to convert back and forth between KTX <-> supported format (e.g., PNG).
It is also useful to convert to and from KTX2 format using OIIO.

An example usecase would be Blender and its glTf import/export plugin.
(see KhronosGroup/glTF-Blender-IO#1896).

Ideally, at some point in the future, OIIO may introduce a new API to accomodate
texture formats that are mainly used for fast texture uploads to GPUs.

For details on KTX2 and what this PR supports and what it doesn't (alongside a
description of current limitations), see src/ktx.imageio/README.md (copied parts
of it at the end of this).

Tests:

Added initial testsuite but only for the input (i.e., info_command) part of the plugin.
Ideally, after fixing CI issues, the tests will be adjusted for ktxouput.

Checklist:

• I have read the guidelines on contributions and code review procedures.
• I have read the Policy on AI Coding Assistants
  => Haven't used any AI coding assistant tools in any capacity whatsoever.
• I have updated the documentation if my PR adds features or changes
  behavior.
• I am sure that this PR's changes are tested in the testsuite.
• I have run and passed the testsuite in CI before submitting the
  PR, by pushing the changes to my fork and seeing that the automated CI
  passed there.
• My code follows the prevailing code style of this project and I
  fixed any problems reported by the clang-format CI test.
• If I added or modified a public C++ API call, I have also amended the
  corresponding Python bindings. If altering ImageBufAlgo functions, I also
  exposed the new functionality as oiiotool options.
  => No new API is introduced. This is just a plugin.

The sections below are copied from src/ktx.imageio/README.md. These will be updated
as this PR progresses.

Supported Encoders/Decoders

• Supported/Tested texture kinds:

  • SINGLE_TEXTURE_1D
  • SINGLE_TEXTURE_2D
  • SINGLE_TEXTURE_3D
  • CUBEMAP_TEXTURE
  • ARRAY_TEXTURE_1D
  • ARRAY_TEXTURE_2D
  • ARRAY_TEXTURE_CUBEMAP
    [ ] ARRAY_TEXTURE_3D (not planned)

• Supported/Tested raw VkFormats (decoder + encoder):

  • VK_FORMAT_R8_UNORM
  • VK_FORMAT_R8G8_[SRGB|UNORM]
  • VK_FORMAT_R8G8B8_[SRGB|UNORM]
  • VK_FORMAT_R8G8B8A8_[SRGB|UNORM]
  • VK_FORMAT_R16G16B16_SFLOAT
  • VK_FORMAT_ASTC_NxM_SRGB_BLOCK (all block dimensions)
  • VK_FORMAT_ASTC_NxM_SFLOAT_BLOCK (all block dimensions)

• Block-compressed formats (decoder + encoder):

  • ASTC
  • [ ] BCn (waiting on libktx BCn support PR merge here: Add BCn encoders/decoders with RDO support KhronosGroup/KTX-Software#1167) => add BCn support in a separate PR whenever libktx 5.0.0 is officially released and my BCn PR is merged.

• Basis Universal schemes (encoder + decoder):

  • UASTC
  • ETC1S

• Supercompression schemes (decompressor + compressor):

  • ZLIB
  • ZSTD

Dependencies (only one - libktx)

• libktx: for general KTX@ format support (loading of KTX2 files, transcoding
  support, supercompression decompression support, etc.).

  • Commit hash: see OpenImageIO/src/cmake/build_Ktx.cmake
  • License: Many subresources. All permissive except lib/etcdec.cxx's license (ETC is not used in PR).

This PR should only be merged after the release of libktx 5.0.0.

Resources

• KTX2 Specs: https://registry.khronos.org/KTX/specs/2.0/ktxspec.v2.html
• Official Implementation (KTX-Software): https://github.com/KhronosGroup/KTX-Software
• Basis Universal Supercompression Implementation (used by libktx): https://github.com/BinomialLLC/basis_universal
• Comparing-BCn-texture-decoders: https://aras-p.info/blog/2022/06/23/Comparing-BCn-texture-decoders/

[walcht]

Update: I'm working on integrating BCn decoders/encoders directly within libktx so that this PR gets significantly simpler (e.g., from 12_000 lines most of which are from dependencies to circa 2_000).

[walcht]

Update2: significantly simplified the whole PR. Now only dependency is libktx (since I offloaded most work to libktx via a PR that will be probably merged soon).

Can you please not approve workflows unless I explicitly request a workflow run? (to avoid filling the actions log with CIs that I know will certainly fail).

[lgritz]

Can you please not approve workflows unless I explicitly request a workflow run? (to avoid filling the actions log with CIs that I know will certainly fail).

Sure, in this case I can do so, but only because you are a a first-time contributor to this project, so admins have to approve workflow runs on the project's main account. Once you have any PR accepted and merged, the workflows will run automatically.

Here is the general solution that you or others can do, in this situation and after you no longer need admin approval:

Let's say you have pushed to your fork's branch "mypr" and submitted a PR to the project from that branch. (Aside: I strongly recommend working in topic branches, and never submitting a PR from your "main".)

After submitting the PR, you realize you need to iterate (you want to add things, or CI is not passing and you need to make repairs, or you have review comments to address), but you don't want to do a series of many many pushes as you go, each of which will trigger a CI run on the main branch as well as sending email alerts to everybody "watching" this project, since pushing to your branch is technically a PR update.

Instead, push to a different branch!

git push origin mypr:test

This pushes your fork's "mypr" branch to your fork in a "test" branch. This will run the CI on your fork in your account (allowing you to see if CI passes for the changes you've made), but since your "test" branch is not associated with the PR, it will not send the rest of us alerts and will not trigger a CI run on the project's main repo account. Then when you finally have it right and are ready for the rest of us to see it, you can git push origin mypr to update "mypr", which the system will know is the branch you are making the PR from.

[walcht]

Question about older CMake version that is failing some CIs and icpx:

1. Some CIs require a relatively old version of CMake (3.18.3) but libktx (the library through which KTX2 support is achieved) requires CMake >= 3.23. (e.g., CI oldest clang11/C++17 py3.9 exr3.1 ocio2.3 / c++17 py3.9).
   In case I may not be able to lower libktx's minimum CMake version (e.g., request refused or simply not possible/requires too many changes) => I couldn't lower CMake version simply because libktx dependencies also require CMake >= 3.20. How should I proceed with this? => Solved. Used older libktx version (4.3.2) for these CIs.

2. ICPX compiler is not supported by libktx (haven't tried compiling it with it yet but it is definitely not supported as per the main CMakeList.txt of libktx). How should I proceed here? => Solved. I have just followed libuhrd and overridden C/C++ compilers for libktx in ICX CI.

These are the only* remaining CI issues.

*: there is another one that is Intel-MacOS only but there is a PR already to fix it in libktx.

All CI issues are now solved. Will push updates here when sufficient testing is done.

[lgritz]

Hi, I'd like to revive this so that if possible, we can include it in the 3.2 release.

Two problems currently standing in our way:

1. You are failing CI in some cases. Look like only the wheel workflow, and only on Mac.
2. Since you started working on this, we have converted the main docs from rst to md (and renamed builtinplugins.rst to builtinplugins.md in the process), and that makes you patch now have merge conflicts. So can you please rebase onto the current main, and I think that will necessitate your re-applying the builtinplugins docs changes to the new location/name of the file and fixing up any formatting.

[jessey-git]

Dropping a note here either for now/this PR or for subsequent work after this lands.

Users will need a way, through the ImageSpec attributes, to control the KTX writer so that it performs the following in some cases:

ktxTexture2_SetPrimaries(..., KHR_DF_PRIMARIES_UNSPECIFIED);

Users need to do this for "data"/"non-color" textures like normal maps or similar. I don't know all the nuances around if that call is valid for all formats inside the KTX container but another implementation I'm working with at the moment is doing so at least for the basisu family of formats I think. It has this comment:

/* `ktxTexture2_CompressBasisEx()` always writes BT709 primaries, regardless of the source
 * VkFormat. KHR_texture_basisu requires UNSPECIFIED primaries for non-color data (normal
 * maps, etc.), so force it back for `is_data` textures. */

[zachlewis]

interesting... thanks, Jesse -- noted. When colorInteropID or oiio:ColorSpace is set to data, the ktx writer must explicitly set the primaries to KHR_DF_PRIMARIES_UNSPECIFIED... at least for the basisu family.

[walcht]

Some updates:

1. The failing wheels are because of ASTC (dependency of libktx) being built using AVX2 which is not supported on Intel-based MacOS'es. This should be fixed now (after my latest commit). Can you please trigger the workflows?
2. The docs are now updated (note: I haven't built them yet locally).
3. There were some issues with my write_tiles() and write_tile() implementations for writing volume and cubemap textures (segfaults and other issues). These are now fixed.
4. I'm confident with the ktxinput part of this PR. The writer part still lacks testing (I don't know how to setup oiiotool to write volume/cubemaps). I'm doing extensive testing in a separate repo instead: https://github.com/walcht/ktx-oiio-examples
5. Apropos the note about KTX primaries; I looked into libktx' ktxTexture2_CompressBasisEx() and I just didn't find anything about the: ktxTexture2_CompressBasisEx() always writes BT709 primaries. It just writes the same primaries as the ones that were set to its provided KtxTexture2 object. Also, looking at ktx create source code, if the input normal map is in PNG format then it defaults to KHR_DF_PRIMARIES_BT709 regardless of the vkformat. I will test whether there is a difference in output basisu-encoded file if the primaries are set to KHR_DF_PRIMARIES_UNSPECIFIED.

Since I have time this week, I will focus a lot on extensive testing on my other repo of the KTX2 writer part (against 100s of libktx's CTS test files). I will also focus on colorspaces since I still only tested/cared about sRGB/linear...

[walcht]

CIs were failing because I used the attribute compression to configure the compression/codec to be used. Apparently, this attribute is used by OIIO and is getting set to "zip" which is invalid for KTX2 (I thought it wasn't used by OIIO).

This is now reverted to ktx:compression. My fork's CI is passing as can be seen here.

[lgritz]

CIs were failing because I used the attribute compression to configure the compression/codec to be used. Apparently, this attribute is used by OIIO and is getting set to "zip" which is invalid for KTX2 (I thought it wasn't used by OIIO).

This is now reverted to ktx:compression. My fork's CI is passing as can be seen here.

No, it was correct all along. You should use "compression" like all the other plugins do.
If you have to pass "ktx:compression" that is format-specific, it means it will never get set by most software that is trying to use OIIO as a single format-agnostic API (its core mission).

The correct behavior when receiving an unsupported/invalid compression is just to silently use a suitable default that is appropriate or typical for the file format.

Do you know where in the software it is setting it to "zip"? Maybe the problem is that this caller is more opinionated than it should be?

[walcht]

Do you know where in the software it is setting it to "zip"? Maybe the problem is that this caller is more opinionated than it should be?

The problem is occurring in testsuite/src/test_sourceprovenance.py as can be seen here by the previously failing CI.

Before I re-revert this and use compression attribute (and look in detail where the zip is set), there is major choice that has to be taken with KTX2.

KTX2 supports two kinds of compressions: the first layer is either a GPU-block-compressed format or a latent Basis Universal coder (the current attribute for this is ktx:compression). This one doesn't support zip, it only supports: uastc, etc1s, astc, etc. The second layer (which is referred to as super compression) supports zip and zstd (typical lossless compression schemes). This second layer of compression is setup using the attribute ktx:super_compression_scheme (long name, I know...).

The question now is: what should the compression attribute refer to? The first layer that expects uastc, etc1s, etc. or the second super-compression layer that expects zip and zstd?

Last note: please do not merge this until I do extensive ktxoutput testing (1000s on libktx CTS testsuite) on my second repo. I had ktxoutput segfaults before that I couldn't capture using the testsuite in testsuite/ktx/.

I will try to finalize all of this this week (alongside some libktx fixes).

[lgritz]

The problem is occurring in testsuite/src/test_sourceprovenance.py as can be seen here by the previously failing CI.

That test is checking that certain internal hint attributes that are not meant to be written to files are properly suppressed, by all file writers. It makes a small TIFF file, reads it, then writes to a file of each format that supports arbitrary metadata, to ensure that it really is suppressed.

The TIFF file it reads has "zip" compression, so of course that's what it will contain when it writes the ktx file.

This is indicative of what will happen any time you do a plain OIIO-based copy of a file, such as

oiiotool input.tif -o output.ktx

This is precisely the reason why I gave you the advice above. You have to ignore unknown compression formats, because they might have come from other file types with compression methods you don't support in ktx.

The only reason that test is failing is that the ktx output is having a hard error when encountering the "compression" value that came from the input file. Substitute in a reasonable default in that case, without generating an error message.

The question now is: what should the compression attribute refer to? The first layer that expects uastc, etc1s, etc. or the second super-compression layer that expects zip and zstd?

If it were me, I would choose the following design:

• Use the standard "compression" to mean super-compression, since it supports types typically found in other file formats.
• Accept a "ktx:blockcompression" for the first layer (it doesn't have to be exactly that, you pick the name that will make sense to ktx users).
• If no blockcompression is specified (as it won't in a copying example like the above), default to something reasonble.

[walcht]

Concerning the discussion about KHR_DF_PRIMARIES_UNSPECIFIED, I got the following reply from libktx' main maintainer:

The comment is incorrect. ktxTexture2_CompressBasisEx does not “write... BT709 primaries”. It only writes the primaries field of the result texture’s DFD to make it the same as that of the input texture. If it is somehow changing the primaries, that is a bug. The only thing the underlying compressor cares about is “perceptual” or not. That setting is derived from the input texture's DFD’s transfer function field not the primaries. Your final sentence is also incorrect. It has a typo "non-data” and shows confusion between KHR_texture_basisu and Basis Universal. The former is the glTF extension for using Basis compressed KTX v2 textures. It does indeed require UNSPECIFIED for non-color data. The latter is the compressor used by ktxTexture2_CompressBasisEx.

[jessey-git]

Yes, the code I was reviewing is related to glTF processing. I'll loop in @julienduroure in case he wants to clarify further, but I think the initial request still stands. Users will need a way, through the ImageSpec attributes, to control the KTX writer so that it sets those primaries to unspecified when requested.

[walcht]

While testing with -DSANITIZE=address,undefined, ktx, png, and couple of other tests fail with the following:

-/usr/lib/gcc/x86_64-linux-gnu/13/include/xmmintrin.h:969:8: runtime error: store to misaligned address 0x73bf2fd912ca for type 'float', which requires 4 byte alignment
-0x73bf2fd912ca: note: pointer points here
- ff ff  ff ff 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
-              ^
 Comparing "../oiio-images/png/oiio-logo-no-alpha.png" and "oiio-logo-no-alpha.png"

Has anyone encountered this before? If so, is it a false positive? (so that I know whether to spend time debugging this).

I built OIIO as such:

cmake -S . -B build/ -DCMAKE_BUILD_TYPE=Debug -DOIIO_DOWNLOAD_MISSING_TESTDATA=ON \
-DOpenImageIO_BUILD_MISSING_DEPS=required -DUSE_PYTHON=1 -DSANITIZE=address,undefined

[lgritz]

While testing with -DSANITIZE=address,undefined, ktx, png, and couple of other tests fail with the following:

-/usr/lib/gcc/x86_64-linux-gnu/13/include/xmmintrin.h:969:8: runtime error: store to misaligned address 0x73bf2fd912ca for type 'float', which requires 4 byte alignment
-0x73bf2fd912ca: note: pointer points here
- ff ff  ff ff 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
-              ^
 Comparing "../oiio-images/png/oiio-logo-no-alpha.png" and "oiio-logo-no-alpha.png"

Has anyone encountered this before? If so, is it a false positive? (so that I know whether to spend time debugging this).

In memory, types generally need to be aligned, e.g., a 4-byte float needs to start at a memory address divisible by 4, but a 1-byte char can be on any address. There are times when we are reading part of a file into memory as a "blob" (a bunch of values hidden behind a char*), and we are walking our way through that blob with a char* and then if you naively copy the next 4 bytes out like float f = *(float*)unaligned_blob_ptr;, that could be an alignment problem. What you actually need there is memcpy.

This can also happen in less obvious ways, like my_struct_t *m = (my_struct_t*)unligned_blob_ptr, which seems fine at that site, but then much much later if you are accessing m->my_float, that will be an alignment error. Again, the really safe solution is to instead do

my_struct_t m;
memcpy(&m, unaligned_blob_ptr, sizeof(my_struct_t);
// ... using m is safe now ...

What you need to do is repeat the command (still with the sanitizer-compiled version) run in gdb or lldb, catch the error, and walk up the stack until you can see the line in OIIO code where it is happening and which variables are involved.

[brechtvl]

Not a complete review by any means, but some hopefully straightforward things to fix.

[brechtvl]

Suggested change

	if (m_tex->numDimensions == 2) {
	else if (m_tex->numDimensions == 2) {

[brechtvl]

Suggested change

	create_info.numFaces = (m_spec.tile_width > 1 && m_basedepth <= 1) ? 6 : 1;
	create_info.numFaces = m_is_cubemap ? 6 : 1;

m_spec comes from the last subimage, which could be the 1x1 mip level I think. Simpler to check m_is_cubemap which was already determined earlier.

[brechtvl]

Suggested change

	case 4: return srgb_colorspace ? VK_FORMAT_B8G8R8A8_SRGB : normalized ? VK_FORMAT_B8G8R8_UNORM : VK_FORMAT_B8G8R8A8_UINT;
	case 4: return srgb_colorspace ? VK_FORMAT_B8G8R8A8_SRGB : normalized ? VK_FORMAT_B8G8R8A8_UNORM : VK_FORMAT_B8G8R8A8_UINT;

[brechtvl]

Suggested change

	// Copyright Co tributors to the OpenImageIO project.
	// Copyright Contributors to the OpenImageIO project.

[brechtvl]

Suggested change

	if (vallen <= 1 || cval[vallen - 1] != '\0') {
	if (vallen <= 1 || vallen > 4 || cval[vallen - 1] != '\0') {

Better to also not crash in case it happens anyway, rather than just assert in debug builds.

[brechtvl]

Suggested change

	ENABLEVAR ENABLE_KTX
	FOUNDVAR Ktx_FOUND ENABLEVAR ENABLE_KTX

To properly disable test if not found.

[brechtvl]

Don't use floating point math for computing mip levels due to rounding errors. 8192x8192 already gives the wrong number (13 instead of 14).

Should be possible to copy integer math code from elsewhere in OIIO to do this.

[brechtvl]

This ioproxy is in the middle between various validations, I think it should be near the end of this function.

[brechtvl]

Here it's loading the image data before check_open validates the dimensions, which defeats the purpose of limits checked there.

[brechtvl]

m_normalized defaults to false if ktx:normalized is not specified. But when converting from most (all?) other image file formats, they are meant to be normalized to 0..1 I think.

Also, ktxinput.cpp doesn't set the ktx:normalized attribute so it doesn't roundtrip.

[julienduroure]

Yes, the code I was reviewing is related to glTF processing. I'll loop in @julienduroure in case he wants to clarify further, but I think the initial request still stands. Users will need a way, through the ImageSpec attributes, to control the KTX writer so that it sets those primaries to unspecified when requested.

Seems my comment on the code you referenced is incorrect. Thanks to point it out.

If I understand correctly, it should change from:

* `ktxTexture2_CompressBasisEx()` always writes BT709 primaries, regardless of the source
* VkFormat. KHR_texture_basisu requires UNSPECIFIED primaries for non-color data (normal
* maps, etc.), so force it back for `is_data` textures. */

to

/* `ktxTexture2_Create` always writes BT709 primaries, regardless of the source
* VkFormat. KHR_texture_basisu requires UNSPECIFIED primaries for non-color data (normal
* maps, etc.), so force it back for `is_data` textures. */

Is that OK?