There are also a lot of compiler warnings from bc7enc_rdo dependency. These should be straightforward to address directly in copied files from bc7enc_rdo.

All committers have signed the CLA.

@walcht, Thank you for this. Can you view the build logs?

One issue I notice immediately is that there are no changes to command_create.cpp. This too needs updating to support BCn encoding.

Regarding RDO and multi-threading, the UASTC encoder also has separate multi-threading options for the encoder and for the RDO step. You can follow the same model.

If we need to fix warnings in the encoder I suggest forking the encoder then incorporating the fork here by way of git subrepo. That will make it easier to potentially contribute fixes back upstream. If you are agreeable I can make the fork and provide instructions for how to incorporate it in your workarea.

Re. SIMD and ISPC, be careful how you support this. We need to support building and running on arm64 processors. Also compile flags to enable SSE or other SIMD options are not compatible with straightforward use of universal build tool chains, which is why this project does not do universal builds. Better is use of compiler pre-defined macros and run-time queries to discover what the software is being compiled for and running on. However since we aren't doing universal builds there is no need to obsess over this last detail.

@MarkCallow - Concerning the command_create.cpp, I am adding BCn encoding for it currently (somehow forgot it) - will also check if I have missed any other commands.

Concerning the build/CI logs: they are mostly failing because of bc7enc_rdo compiler warnings which should be suppressed. They will also fail because I haven't re-generated the golden files yet for ktx CTS (e.g., ktx encode --help output).

If you are agreeable I can make the fork and provide instructions for how to incorporate it in your workarea

Please do so (as far as I understood, this will be forked under the KhronosGroup and any updates here will be pushed there via the git subrepo command). Until that is done, I will keep edit the bc7enc_rdo sub-folder until the CI passes.

Concerning SIMD and ISPC: I think it makes sense to leave this for another PR, do you agree? (reasoning is this: I have to get the basics working properly, add proper testsuite that covers all supported BCn formats, etc. Once that is done, I can open another PR for SIMD performance improvements). Or I will leave this to very end (last in TODO list above).

Updates about CI jobs:

• reuse lint: I don't know why it is not ignoring external/bc7enc_rdo but I have added the SPDX comments in these files.
• MingW CI: failing because of some linkage error with bc7enc_rdo (undefined reference to rgbx::). I have to test this on my Windows machine to see what I am doing wrong/missing.
• MacOS (arm64) compiling bc7enc_rdo fails. I have no idea why (I don't have a MacOS machine so I will be testing this CI on my fork).
• Windows CI: probably some NSIS issue (I don't think I have introduced anything to make it fail)
• Other failures are still due to unused-variables/functions that I don't get locally (have addressed all that were mentioned in the CI logs).

@walcht,

You will need to rebase to or merge current main to get the fix for the NSIS issue on Windows.

I have created a fork of https://github.com/richgel999/bc7enc_rdo. To incorporate it in your add-BCn-decoder branch do the following in the repo root directory:

git subrepo clone https://github.com/KhronosGroup/bc7enc_rdo.git external/bc7enc_rdo -b changes_for_ktx

You can make changes in this subdirectory, as you are now, and when everything is working I can push the changes to the fork.

Concerning SIMD and ISPC: I think it makes sense to leave this for another PR

I agree. I wanted to make you aware that arm64 is a build target.

Re. reuse, you have to add an entry to REUSE.toml to get external/bc7enc_rdo ignored. It is better to do that than add SPDX comments to all the files. The entry in REUSE.toml will have to mention a license. Use the MIT license option.

Here are a few high level points.

• I want to use the bc7f encoder once it is integrated into the bc7enc_rdo repo.
• Re. decoding, currently the ETC and ASTC decoders have different APIs. The former has a non-public unpack function for single blocks. This is called from the GL and Vulkan upload functions, if the device does not support ETC, to convert the data before it is uploaded. The latter has a call that decodes and converts an entire ktxTexture2 object. I want to have both for all decoders: ASTC, BCn and ETC and the transcoders as both have their uses. It was a mistake not to have provided an unpack function and updated the upload functions when the ASTC decoder was added.
• I would like BC6H support to be included in this or a later PR.
• I will be away for 5 days from May 15th and not be able to approve workflow runs during that time. Please plan accordingly.

Re the macOS build failure, because the output from the Xcode build is so voluminous it is run through a script, xcpretty, to prettify it. On a past CI service, without this, the logs exceeded the maximum allowed. Recently, for reasons I have yet to investigate, it has started swallowing compile errors. You can turn it off by editing scripts/install_macos.sh and commenting out the line that installs xcpretty.

@MarkCallow

I want to use the bc7f encoder once it is integrated into the bc7enc_rdo repo.

Agree. I wasn't initially aware of this. Apparently bc7f is significantly faster that the bc7 encoder used here (also the added benefit of being continuously maintained). I will integrate this right now since this seems to be straightforward (bc7enc_rdo also seems a bit not-longer-maintained so l think it's better to just integrate it now rather than waiting for it to be integrated into bc7enc_rdo repo).

Concerning the decoding API: I added BCn decoders for VkUpload/GLUpload as a TODO (will follow same API as in etcunpack). Might also open a PR to add it for ASTC since I have already spent some time getting familiar with this code base.

I would like BC6H support to be included in this or a later PR.

Will add it in this PR. Will add it at the very end though since I have to finalize current formats.

I will be away for 5 days ...

I will try to address CI issues now (It is fine if I incrementally commit here to see if certain jobs pass?).

The integrated basisu_transcoder is a bit outdated (actually, significantly) and doesn't contain bc7f. So for the moment I will just use bc7enc_rdo's BC7 encoder until the fork at (https://github.com/KhronosGroup/basis_universal/tree/fixes_for_ktx_v5_0) is updated to a commit that includes the bc7f namespace.

I was not aware that bc7f is included in basisu_transcoder. By pure coincidence I have just completed integration of Basis Universal release 2.1.0. See the update_basisu_to_2_1_0 branch. We have the v5.0.0 release in flight. I've made a v5.0.0-rc1 release while we wait for some external (to KTX-Software) pieces to be put in place. My plan was to wait until we made the v5.0.0 release before merging this branch. If you retarget this PR and your branch to update_basisu_to_2_1_0 you can start working on bc7f now.

Neither the KTX-Software code nor our golden files required any updates for our extensive test suite to pass with BU 2.1.0. I am therefore amenable to merging it now but will have to discuss within the Khronos WG and can't make any promises.

The single image decoders used by GLUpload/VkUpload should be exposed in the library API but must be independent of the ktxTexture* classes. Software reading a KTX file incrementally will find them useful.

Regarding the current ETC decoder, please note that it does not have a recognized open source license so might not be suitable for your OIIO work.

@MarkCallow - please don't approve the CI workflow yet (it will fail because I haven't updated the golden test files yet).

• I have added initial RDO postprocessing step support for BC1, BC3, and BC7 (only relying on ert.h/cpp without having to use bc7enc_rdo utils.h/.cpp and rdo_bc_encoder.h/cpp because these used OpenMP and it's just better that we only depend on the actual core RDO function in ert.h/cpp).
• Also fixed the encoder in case input texture is not multiple of 4 (still have to absolutely verify this in all cases - this was reading beyond std::vector's size before this commit...).
• No ultrasmooth block support yet (added to TODO list).
• Added initial RDO params for BCnParams in ktx.h (some detailed description for fields I understand).

Will add bc7f once I finish RDO post processing.

I have just merged PR #1170 so you will now find bc7f in main. Please remove external/bc7enc_rdo and adapt this to bc7f. I hope that not too much of your work to date will have been wasted.

One other thing re. bcn_codec.cpp, we need to be able to build with just the decoder when building libktx_read. astc_codec.cpp is one file because of pieces needed by both encoder and decoder so it has ifdefs to allow building only the decoder parts. Unless there is substantial common code you can consider making separate source files for encode and decode. If you do not, then add similar ifdefs.

Windows CI is failing because of some NSIS issue...

I have triggered a re-run. The NSIS install has worked on the first job this time. Fingers crossed.

Windows failed again because of NSIS.

MingW is failing because of something I really don't understand:

D:/a/KTX-Software/KTX-Software/external/bc7enc_rdo/rgbcx.cpp:3074:29: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 3074 |         pPixels[i * stride] = ((alpha_block >> (4 * i)) & 0x0F) * 17;
      |         ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I don't see anything problematic with my code (and as usual I ran all tests + DSANITIZE=undefined/address with Debug targets). Do you have any ideas why this is occuring?

Original code:

    auto alpha_block = *reinterpret_cast<const uint64_t*>(pBlock_bits);
    for (int i = 0; i < 16; ++i)
        pPixels[i * stride] = ((alpha_block >> (4 * i)) & 0x0F) * 17;

This is most probably a GCC bug/false positive.
This states that it is a bug for gcc 13.2.0 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106757.
The version used in MingW is 15.2.0. I will just set -Wno-stringop-overflow.

Thanks for pointing it out. Please add these in a separate PR. This one is complicated enough already.

Somehow I only read this now after I have added the ASTC tests (+ missing decodeAstc() method) here. Apologies. If you want me to revert them, say so and I will do it + open separate PR.

Update: Android/MacOS/WebCI are failing because of the -Wno-stringop-overflow I added. I will fix this later. I already triggered tons of CI runs ...

I don't see anything problematic with my code (and as usual I ran all tests + DSANITIZE=undefined/address with Debug targets). Do you have any ideas why this is occuring?

No I do not. It keeps occurring in the Binomial code and I have never been able to track down the cause and I failed to find the bug you have linked. Thanks for that. It says it is fixed in gcc-14 though I am sure I have seen similar issues with gcc-16 and in this case it is with gcc-15.

What changed in the code between it working and getting this error? Or was the compiler version updated in the runners?

I failed to find the bug you have linked. Thanks for that.

Please put the bug URL in the comment where you are setting -Wno-stringop-overflow.

Or was the compiler version updated in the runners?

No, it was certainly not.

What changed in the code between it working and getting this error?

I refactored the alpha unpacking code section in BC2 to a separate function which then got inlined by the compiler (only with optimizations enabled) which caused the GCC bug (i.e. it only occurs when inlining functions with pointer-access loops).

Some weird non-determinism is occurring in BC6HU (as stated before, either ktx diff is doing something weird or the encoder itself is non-deterministic). As you can see, in latest CIs, there are no issues with BC6HU tests. However, with some CIs before it (on Windows), there has been some failing BC6HU tests.

Anyway, even if CIs pass, please don't merge this yet until I figure out the issue (I'm somewhat confident it has to do with the half floating point conversions).

This Windows 10 ClangCL is really frustrating. It just randomly fails and only occurs on Windows 10 with ClangCL compiler and only on x64 (never seen it occur on Arm).

Since you are familiar with Basis Universal codebase, do you know of any non-determinism issues with BC6HU HDR encoders?

This might related to the non-determinism issue within BasisU that you mentioned earlier in this PR... I might do a deep dive into the BC6HU encoder...

We are aware of two non-determinism issues. The first is that multi-threaded RDO run results differ from run to run. (It was a long time ago now so I can't be sure, but as the issues cropped up when running automated tests the number of threads was the same each run.) The second issue was non-determinism across platforms in the ETC1S/BasisLZ encoder. We believe this was caused by use of std::unordered map whose implement varies from platform to platform. See issue #60 in the Basis Universal repo. It was supposedly fixed though the issue is still open.

What is the failure, differing images or something else? If the former, can't you use ktxdiff with tolerance for the comparison?

I can only Windows 11 on ARM so I can't see the issue myself.

What is the failure, differing images or something else? If the former, can't you use ktxdiff with tolerance for the comparison?

ktxdiff fails, sometimes, and passes other times on the same platform/compiler/compiler options: ClangCL 19.1.5.
The test simple encodes then decodes and compares with original input. The test passes if ktxdiff succeeds with default tolerance which it does on all my machines but sometimes doesn't on Windows 2022 ClangCL CIs.

For instance, both of these CIs have the exact same code with regards to HDR:

1. this fails: https://github.com/KhronosGroup/KTX-Software/actions/runs/34260205148/job/102175963632?pr=1167#step:21:1503
2. this passes: https://github.com/KhronosGroup/KTX-Software/actions/runs/34077740812/job/101607063997#step:21:443

I re-run the tests a dozen of times or so, on my Windows 10 machine, using ClangCL 16.0.5 and I couldn't replicate this locally.

I do expect the output to not be deterministic between different compilers/platforms, but this is occuring whithin the same compiler, same platform, and same build options which is really unexpected.

I will re-trigger the CIs so that I can get the resulting file + ktxdiff output (CI currently only uploads CTS files on failure and this is part of the core texturetests).

I got the CIs to upload files generated by failing texturetests tests and the BC6HU files are here:
https://github.com/KhronosGroup/KTX-Software/actions/runs/34661344514/artifacts/10287209659

The ktxdiff output is the following:

ktxdiff: Mismatching image data (diff: 780; lhs[11]=780; rhs[11]=0): level 0, face 0, layer 0, depth 0, pixel 3, component 2 between
          Expected: C:\Users\RUNNER~1\AppData\Local\Temp\encode_rgb16_sfloat_to_bc6hu_then_decode_original.ktx2 and
          Received: C:\Users\RUNNER~1\AppData\Local\Temp\encode_rgb16_sfloat_to_bc6hu_then_decode_decoded.ktx2

I managed to reproduce the ktxdiff output on my machine (on the files above) => the issue is probably somewhere in the encoder or how my code calls BC6HU.

I tried to encode the original file above and the following assertion fails:

.\build\Debug\ktx.exe encode --format BC6H_UFLOAT_BLOCK encode_rgb16_sfloat_to_bc6hu_then_decode_original.ktx2 bc6hu.ktx2
Assertion failed: pos == private->_levelIndex[level].byteOffset + baseOffset, file C:/Users/walid/KTX-Software/lib/src/writer2.c, line 563

=> This was some weird issue on my Windows 10 trying to copy same build config as MinGW (just ignore this comment)

I think the source of the problem is that I am setting the component type wrongly to a GLubyte. This is how I wrote the BC6HU tests:

class ktxTexture2_BCnEncodeDecodeTestRGB16_SFLOAT
    : public ktxTexture2BCnEncodeDecodeTestBase<GLubyte, 3, GL_RGB16F> {};

class ktxTexture2_BCnEncodeDecodeTestRGBA16_SFLOAT
    : public ktxTexture2BCnEncodeDecodeTestBase<GLubyte, 4, GL_RGBA16F> {};

// ...

// BC6HU:
//    - VK_FORMAT_R16G16B16_SFLOAT
//    - VK_FORMAT_R16G16B16A16_SFLOAT
TEST_F(ktxTexture2_BCnEncodeDecodeTestRGB16_SFLOAT, encode_rgb16_sfloat_to_bc6hu_then_decode) { runTest(KTX_BCN_COMPRESSION_BC6HU, false); }
TEST_F(ktxTexture2_BCnEncodeDecodeTestRGBA16_SFLOAT, encode_rgba16_sfloat_to_bc6hu_then_decode) { runTest(KTX_BCN_COMPRESSION_BC6HU, false); }

Now my question is: why wasn't this caught on any of my systems (even with ASan and UBsan enabled) and why that assert simply doesn't occur for my system locally? (of course, with Debug builds).

I think this is pretty much done. As I stated before, that issue with BC6HU was purely because I set the component size wrongly in the tests. It would be nice if you can re-trigger the workflow just to be absolutely certain.

Since this is still waiting for 5.0.0 release, I already moved to finishing the D3D12/D3D11 PRs (these will add Direc3D texture upload functionalities like those for Vulkan/OpenGL).

Congratulations on finding the BC6HU test problem. My deepest apologies for the writing that bug.

I look forward to seeing the Direct3D texture upload function PR.

@walcht, I have asked Rich Geldreich, the author of bc7f, about sRGB encoding. He tells me that for speed, bc7f uses purely "RGB(A) squared distance metrics" so it is agnostic of color space. He says the PSNR is "typically so high (well above 40dB) that using other metrics does not add much."

He also tells me that there is now a plain c++ port of the bc7e.ispc encoder in the repo which is "also a full and more powerful (but slower) encoder and it supports perceptual metrics." He says that this can be used if the bc7f quality isn't high enough.

Is the bc7f quality high enough?

I look forward to seeing the Direct3D texture upload function PR.

As it is likely to be useful in this effort I want to let you know that the format queries addition I have waiting on a branch locally includes a function to get the DXGI format for a ktxTexture* object. The switch is generated from the KTX specification database. If it will indeed be useful, let me know and we discuss how to proceed.

Is the bc7f quality high enough?

I will compare them both on many sets of images and see which one is better (will also benchmark the timings). Since I'm not that experienced with textures, I will just upload the results in another comment here and you can decide which one you think is more adequate for this PR.

As it is likely to be useful in this effort I want to let you know that the format queries addition I have waiting on a branch locally includes a function to get the DXGI format for a ktxTexture* object. The switch is generated from the KTX specification database. If it will indeed be useful, let me know and we discuss how to proceed.

Isn't this the same Vulkan -> DXGI switch that I used in my DDS PR? Or is this about an OpenGL -> DXGI switch?
On a sidenote, I'm still going through the official Direct3D12 samples and trying to understand them. I can push an early PR but I want to understand what should be exposed and replicate the Vulkan upload functions as much as possible.

I will compare them both on many sets of images and see which one is better (will also benchmark the timings). Since I'm not that experienced with textures, I will just upload the results in another comment here and you can decide which one you think is more adequate for this PR.

Sounds like the right approach but I am concerned about uploading a large number of images to a comment in this PR, which already takes a long time to load in the browser. Please find an alternative.

Isn't this the same Vulkan -> DXGI switch that I used in my DDS PR? Or is this about an OpenGL -> DXGI switch?

It is the inverse of what you created for the DDS PR. Based on the same spec. database you used, my WIP will add new public API functions that return the equivalent DXGI and MTLPixelFormat formats for both ktxTexture1 (i.e. mapping from OpenGL) and ktxTexture2 (i.e. mapping from Vulkan) objects. You will need this to get the DXGI format to use for the upload. The WIP also adds source files with the declarations for the DXGI and MTLPixelFormats needed so that the generated switches can be compiled. I expect the DXGI enum declaration will be useful for compiling the uploader too.

On a sidenote, I'm still going through the official Direct3D12 samples and trying to understand them. I can push an early PR but I want to understand what should be exposed and replicate the Vulkan upload functions as much as possible.

This is an excellent approach. There is no need to spend time making an early PR.