Add BCn encoders/decoders with RDO support by walcht · Pull Request #1167 · KhronosGroup/KTX-Software

walcht · 2026-05-07T14:04:58Z

As discussed in #1159: ktxTexture2_CompressBCn and ktxTexture2_DecodeBCn are introduced in this PR to allow libktx users/consumers to encode/decode BCn textures from/to raw decompressed formats.

https://github.com/richgel999/bc7enc_rdo does not support BC6HU/BC6HS encoding/decoding and also no BC2 (this format is essentially dead since BC3 replaces it).

Please feel free to give feedback, edit, and nitpick as much as possible.

Some context: I am adding KTX2 support to OIIO (PR: AcademySoftwareFoundation/OpenImageIO#5185) and having libktx encode/decode BCn formats significantly simplifies things (also ETC encoding/decoding which I can also open a PR for - if approved).

I haven't updated the KTX-Software-CTS with the added BCn test files.

Once this is finalized, this will fix #587.

Current TODOs:

Note1: no LLMs/AI coding tools were used in any capacity whatsoever in writing or aiding in the writing of this PR.
Note2: I am an individual contributor (main reason I am contributing here is to add support for KTX2 in Blender).

Edit: TODO list edits

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

*Add encode/decode tests (that use both CompressBCn/DecodeBCn) *Add BCn ktx2 test files (transcoded from tests/resources/ktx2/color_grid_uastc_zstd_5.ktx2) *Cleanup BCn test fixtures *Remove `std::cout` statement Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht · 2026-05-07T14:13:14Z

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.

*Add BC1, BC3, BC4, BC5, and BC7 encoding support to "ktx encode" command. *Cleanup ktxBCnParams and add BC1/BC3 quality and mode params. *Add docstrings/documentation to newly added enums/structs in ktx.h. Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

… add-BCn-decoder

CLAassistant · 2026-05-08T07:17:33Z

All committers have signed the CLA.

MarkCallow · 2026-05-09T08:35:46Z

@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.

walcht · 2026-05-10T01:49:42Z

@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).

*Suppress bc7enc_rdo warnings like unused-variables, memset'ing a non-trivial class (in this case the class is obviously trivial hence a void* cast is used to suppress this warning). Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

*Some CIs report further unused variables/functions that are not reported when building locally. Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht · 2026-05-10T06:03:57Z

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).

MarkCallow · 2026-05-10T06:52:39Z

@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.

MarkCallow · 2026-05-10T07:00:38Z

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.

*Add BCn encoder support for `ktx create` command. *Add missing tests for `ktx encode`, `ktx extract`, and `ktx create`. *Expose BC1/BC3 approximation mode option to ktx CLIs *Misc cleanups (still early-stage PR) Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht · 2026-05-10T12:28:27Z

@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?).

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

…Group/bc7enc_rdo.git external/bc7enc_rdo subrepo: subdir: "external/bc7enc_rdo" merged: "dbe416d2" upstream: origin: "https://github.com/KhronosGroup/bc7enc_rdo.git" branch: "changes_for_ktx" commit: "dbe416d2" git-subrepo: version: "0.4.9" origin: "https://github.com/ingydotnet/git-subrepo" commit: "5e0f401"

*Before this commit, bc7enc_rdo dependency was manually copied to external/bc7enc_rdo directory (only needed files were copied). This was not ideal for a lot of reasons (mainly that we are introducing changes that may be streamed back to the original repo and having a subrepo/submodule is better suited for that than manually copying dependency files). *Add bc7enc_rdo to REUSE.toml with MIT license. *Git ignore compile_commands.json file (used by clangd LSP) Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

… add-BCn-decoder

walcht · 2026-05-10T13:38:10Z

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.

MarkCallow · 2026-05-11T11:39:17Z

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.

*Add initial RDO post processing step for BC1, BC3, and BC7 but without ultrasmooth blocks support see: https://richg42.blogspot.com/2021/02/updated-bc7encrdo-with-improved-smooth.html *Fix encoder in case input texture does not have multiple-of-4 dimensions. This was reading beyond std::vector size before this commit. *Add initial RDO params to BCnParams with verbose explanation/description comments. *Misc refactoring/adjustments. Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht · 2026-05-14T07:46:28Z

@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.

MarkCallow · 2026-05-14T08:24:14Z

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.

MarkCallow · 2026-08-04T11:36:55Z

I cloned the basis repo and searched for bc7g and I couldn't find anything.

I thought I saw mention in one of the announcements. I could be wrong. Release notes are reportedly at https://github.com/BinomialLLC/basis_universal/wiki/Release-Notes.

MarkCallow · 2026-08-04T13:33:47Z

Before I forget, I want to fix UASTC multi-threaded RDO in the same way you fixed the bc7 RDO. Have you done that yet? If not, it's fine to later. It should be a separate PR from this.

walcht · 2026-08-04T14:00:36Z

I haven't touched the UASTC multi-threading RDO yet (because, as you stated, I wanted it to be a separate PR).

I will fix it in another PR (I think it should be straigh-forward; it's all about how work is dispatched to threads).

MarkCallow

37 comments including some change requests. I have now reviewed everything to date.

Thanks for this impressive effort on this codec.

MarkCallow · 2026-07-06T09:46:30Z

 option( LIBKTX_FEATURE_VK_UPLOAD "Enable Vulkan texture upload." ON )
 option( LIBKTX_FEATURE_GL_UPLOAD "Enable OpenGL texture upload." ON )
 option( LIBKTX_FEATURE_ETC_UNPACK "ETC decoding support." ON )
+option( LIBKTX_FEATURE_BCN_DECODER "BCn decoding support." ON )


As long as it is done before this is released, another PR is okay.

As for how the library is expected to be used, my preference is to provide a consistent set of functionality in the library. The ETC_UNPACK option was put in primarily because of pushback about the weird license for the ETC unpacker.

MarkCallow · 2026-08-05T07:40:01Z

+    }
+
+    // Can't encode from BCn to ASTC
+    if ((model == KHR_DF_MODEL_BC1A || model == KHR_DF_MODEL_BC3 || model == KHR_DF_MODEL_BC4 ||


This is duplicating lines 273:286 except for the suggestion to use ktx extract.

If we do make a suggestion for ktx extract it should be offered for any similar conversion attempt (ASTC to ASTC, ASTC ro BCn, etc.) - preferably without duplicating the message each time - and it must include a warning that decoding and re-encoding will result in a quality loss.

This is still a duplicate. Now, lines 285:288. Delete these.

walcht · 2026-08-06T23:28:39Z

Thank you for the detailed review (I will push the commits that address this probably tomorrow).

There is a very important note:

The key here is that sRGB is an encoding of colors. If you are doing any calculation based on the color values, you must decode first, perform the calculation then re-encode.

I really thought, initially, that I should do this (after all, almost always when one reads sRGB, it should be converted to linear, do math, then convert back).

The thing is, the original repo bc7enc_rdo doesn't do this (there is no sRGB->linear conversions at all) and simply performs encoding on sRGB directly. Also, the encode_bc? functions expects unsigned char per channel so converting to linear from sRGB means loosing information. I think these functions expect sRGB inputs and they don't do any internal conversions (I'll double verify again if there is any gamma applied and update this comment but I'm almost certain there isn't).

MarkCallow · 2026-08-07T03:37:55Z

The thing is, the original repo bc7enc_rdo doesn't do this (there is no sRGB->linear conversions at all) and simply performs encoding on sRGB directly.

The key parameter in basis_universal is called m_perceptual and I see it is used by basis_bc7enc.{cpp,h}. It does not actually cause a decode and re-encode, it modifies the way the encoder works in things like computing the color difference. I haven't looked to see if RDO for BasisLZ and XUASTC use it. The encoders definitely do. If those RDO functions do not then perhaps RDO is done in a way that makes it not necessary.

There are two other Basis compress parameters with srgb in their names. You can ignore them. One is used when generating mipmaps and we don't gen our mipmaps via the BasisU compressor. The other tells it to mark the output file, .basis or .ktx2, as sRGB. We don't use its file writing features either as, among other things, it does not support 3d textures.

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht · 2026-08-09T14:43:36Z

Update on this:

Before I forget, I want to fix UASTC multi-threaded RDO in the same way you fixed the bc7 RDO. Have you done that yet? If not, it's fine to later. It should be a separate PR from this.

I spent some hours understanding the RDO code and apparently I was initially wrong about this being fixed and I think there is just no way to make multi-threaded RDO deterministic (i.e., running ST vs. MT yields same results).
The fix (which I figured right now that is not actually a fix) just happened to work on the input test images which were very simple, 16x16, grayscale images. That fix just made it less likely for non-determinism to occur but definitely doesn't solve the problem with actual, real-world images.

Why? Because running RDO on the whole image (single-threaded mode) means that each block N depends on previously RDO'ed M blocks (with M influenced by the window/dict size). In MT mode, image is divided into regions (one region per thread) and RDO is ran on these separately.

I'm currently updating my guide, the no-multithreading parameter description, and a bunch of other things to reflect this.

I also see that a lot of improvements can be done on the RDO function but I will postpone these to after when this PR gets merged.

If you want, I can open a PR to add a comment and documentation about this with UASTC RDO (or any other RDO that is used).

MarkCallow · 2026-08-10T05:56:55Z

Thank you for your study.

Why? Because running RDO on the whole image (single-threaded mode) means that each block N depends on previously RDO'ed M blocks (with M influenced by the window/dict size). In MT mode, image is divided into regions (one region per thread) and RDO is ran on these separately.

The problem we originally observed was that the MT results were non-deterministic between runs. It wasn't investigated; there is still an open issue on the basis_universal repo. Do you think with the changes you have made MT will at least be deterministic between runs?

We can certainly live with ST and MT giving different results provided we explain it in the docs. An obvious question is which produces the result with the least entropy (i.e. which will deflate best with zstd): ST or MT?

Is the dictionary shared between the threads and would that affect the result.

I'm currently updating my guide, the no-multithreading parameter description, and a bunch of other things to reflect this.

I also see that a lot of improvements can be done on the RDO function but I will postpone these to after when this PR gets merged.

Sounds good. When you have time, please give me a summary of the possible improvements.

If you want, I can open a PR to add a comment and documentation about this with UASTC RDO (or any other RDO that is used).

That will be terrific. I'd like to get this in for v5.0.0. It would have to cover BasisLZ, UASTC LDR 4x4 and UASTC HDR 6x6 Intermediate.

walcht · 2026-08-13T17:07:10Z

Make this an active cross reference to the documentation for the struct.

I just couldn't figure out how to make Doxygen generate a reference inside a C-code comment that is isn't supposed to consume...

MarkCallow · 2026-08-14T11:17:49Z

Make this an active cross reference to the documentation for the struct.

I just couldn't figure out how to make Doxygen generate a reference inside a C-code comment that is isn't supposed to consume...

This is a markdown page being processed by Doxygen. According to https://www.doxygen.nl/manual/markdown.html to link a documented entity you can use

[The link text](@ref MyClass)

MarkCallow · 2026-08-17T09:20:56Z

Regarding --normal_mode, one of my Khronos colleagues has said "If anything, BC3 normal maps that use grayscale-alpha encoding should be a thing of a distant past. There's no reason to use those over BC5." Therefore I suggest that --normal_mode should only support BC5.

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht · 2026-08-20T13:50:20Z

It is determined by the transfer function in the DFD of the incoming ktx2 file, isn't it? Color model is something different.

I changed it to this:

UNORM vs. SRGB is determined depending on the original BCn VkFormat.

I determine whether the decompressed format is SRGB vs. UNORM depending on the set BCn VkFormat (I don't look at the DFD since I assume if BCn is set to SRGB then so is the DFD's TF => otherwise an invalid KTX2 file, right?).

In normal use a library should not print messages to stderr or stdout because it has no idea if the application is GUI or command line and no idea how the application is using those file descriptors. Use for debugging is fine.

It would be acceptable if we can find a way to provide the warning to the application to let it handle it. Perhaps a return code that says the encoder generated warnings and a query to retrieve the warning message or messages.

I think it makes sense to do this in a separate PR after this gets merged, do you agree?

We should follow typical usage. Allow/require --skip-kvd before the first none option argument. Also print usage for any option arguments that are not skip-kvd. Currently this is assuming a fifth argument is skip_kvd.

I used cxxopts in ktxdiff. It's just better that having to manually parse the CLI arguments/options.

Still a couple of comments that I haven't addressed yet.

~~CIs will probably fail because of cxxopts dependency setup. Since tests are always added, I will always add it.~~

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht · 2026-08-21T01:33:33Z

We must investigate why. It is intended to be supported but the tolerance is given as a percentage of the minimum of the two values being compared. Could this also be the input being "cleaned" before encoding? Please send me an example and I will investigate.

The two files that causes ktxdiff to fail are:
rgb16_sfloat_files.zip

And this is the output I get:

ktxdiff: Mismatching image data (diff: 6.4373016e-06): level 0, face 0, layer 0, depth 0, pixel 196, component 0 between
          Expected: /home/walcht/KTX-Software/tests/resources/ktx2/rgb16_sfloat_input_for_bc6hu.ktx2 and
          Received: build/Testing/Temporary/GeneratedFilesOnFailure/decoded_rgb16_sfloat_bc6hu.ktx2

The problem is obvious when looking at this code sample in compareSFloat16:

        const auto diff = std::abs(lhs[i] - rhs[i]);
        const auto absMin = std::min(std::abs(lhs[1]), std::abs(rhs[1]));
        if (diff > tolerance * absMin)
            return CompareResult{false, diff, i, i * element_size};

If lhs[i] or rhs[i] is 0, then the if statement will always enter regardless of how high the tolerance is set to. A quick fix is to add an epsilon but a whole different approach to computing the diff for unnormalized values is probably better.

There is also a mistake here at compareSFloat32 (1 should be i):

        const auto absMin = std::min(std::abs(lhs[1]), std::abs(rhs[1]));

Should I fix it here or in another issue + PR?

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht · 2026-08-24T22:50:54Z

All review comments are addressed (and some more docs issues are fixed)

The only remaining thing:

[...] There's no reason to use those over BC5." Therefore I suggest that --normal_mode should only support BC5.

So, if I understood correctly and since BC5 encoder (and all other BCn encoders) don't have a normal-map-specific parameters (such as angular error), this will only disallow RDO (i.e., will generate warning when supplied with --bcn-rdo), right?

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

MarkCallow · 2026-08-25T09:56:33Z

Starting to review the latest changes now. Sorry for the delay. I've been distracted with other work. As you may have seen I made a v5.0.0-rc2 with all your wonderful fixes.

The only remaining thing:

[...] There's no reason to use those over BC5." Therefore I suggest that --normal_mode should only support BC5.

So, if I understood correctly and since BC5 encoder (and all other BCn encoders) don't have a normal-map-specific parameters (such as angular error), this will only disallow RDO (i.e., will generate warning when supplied with --bcn-rdo), right?

--normal-mode most importantly creates a 2-component normal map. Adjusting encoder parameters is secondary. Yes it is likely a good idea to disable RDO as the "distortion" it uses is based about perception of colors. It would be nice to run some tests though to see if the effects are truly detrimental to normals since RDO can offer a large reduction in the deflated (LZ-compressed) size of the texture. Unfortunately I don't have any tests.

MarkCallow

I still have to re-review the stuff in tools/ktx and texturetests.cc. I decided to post my comments so far from the re-review, as I've accumulated a reasonable number, mostly straightforward. This way, you can get started addressing them now, if you have the time.

MarkCallow · 2026-08-25T10:05:53Z

    KTX_BCN_COMPRESSION_BC3     = 4,
+        /*!< BC3 compression. Encodes a 4x4 RGBA LDR block into 16 bytes.
+           RGB block is encoded using BC1 into 8 bytes. Alpha is encoded
+           separately into 8 bytes.


What is the difference between "sharply" (BC2) and "separately"?

MarkCallow · 2026-08-26T08:42:31Z

+        "received-ktx2", "Received KTX2 file", cxxopts::value<std::string>())(
+        "tolerance,t",
+        "For normalized formats tolerance is the normalized absolute value of the acceptable "
+        "difference (inclusive). For unnormalized formats it is the fraction of the minimum of the "


This is not true for the unorm8 comparison. The code is normalizing the values then applying the tolerance as the absolute value of the acceptable difference. Fix either the code or the documentation.

MarkCallow · 2026-08-26T08:50:12Z

    for (std::size_t i = 0; i < count; ++i) {
        const auto diff = std::abs(lhs[i] - rhs[i]);
-        const auto absMin = std::min(std::abs(lhs[1]), std::abs(rhs[1]));
+        const auto absMin = std::min(std::abs(lhs[i]), std::abs(rhs[i]));


This still suffers from the problem you pointed out that if one of the values is 0, the difference will appear greater than the tolerance. It needs the epsilon trick you have put in compareSFloat16.

MarkCallow · 2026-08-26T08:52:56Z

+ * If an inf value is encountered in any of the two inputs the comparison for
+ * that index is ignored.
+ *
+ * nan and inf values are ignored because HDR usually encoders do not even


s/HDR usually encoders/HDR encoders usually/

MarkCallow · 2026-08-26T08:57:21Z

    for (std::size_t i = 0; i < count; ++i) {
-        const auto lhsFloat = imageio::half_to_float(lhs[i]);
-        const auto rhsFloat = imageio::half_to_float(rhs[i]);
+        // TODO: make sure to revert this or replace imageio's half_to_float with Basis Universal's version


"revert ... or replace"? Do you mean revert and replace?

Why? Is there a problem with imageio's half_to_float? I'd prefer to fix that, if there is.

MarkCallow · 2026-08-26T09:47:54Z

+(i.e., BC1, BC3, BC4, BC5, BC6HU, BC6HS, or BC7) texture using
+ktxTexture2::ktxTexture2\_CompressBCnEx.
+
+You can also use Basis Universal's transcode utilities to transcode UASTC (or


Reference that you can transcode UASTC to BCn withktxTexture2_TranscodeBasis rather than "basis Universal's transcode utilities."

MarkCallow · 2026-08-26T09:49:43Z

+
+You can also use Basis Universal's transcode utilities to transcode UASTC (or
+other supported codecs) to BCn but the difference here is that this directly
+encodes to target BCn format without any intermediate steps (i.e., transcoding)


s/to target/to a target/

MarkCallow · 2026-08-26T09:50:43Z

+This example is kept as simple as possible. There are a lot of other parameters
+that are only activated depending on the target BCn scheme and whether rate
+distortion optimization (RDO) is enabled. The discussion about RDO parameter
+details is too involved for this example (see member parameters descriptions


Make an @ref to the struct.

MarkCallow · 2026-08-26T11:42:41Z

+
+~~~~~~~~~~~~~~~~{.c}
+#include <ktx.h>
+#include <vulkan/vulkan_core.h>         // From your Vulkan SDK


Just #include <vulkan/vulkan.h> which is the standard. There isn't much else in it except for #include <vulkan/vulkan_core.h>. Please change the other examples in this file too.

MarkCallow · 2026-08-26T11:48:33Z

+src = 0;            // Open the file for the baseLevel image, slice 0 and
+                    // read it into memory.
+srcSize = 0;        // Query size of one image slice from file
+


Add comment saying open a file for each mip level or generate levels from the baseLevel image and query the size of the level image. Probably the comment should be in the loop to promote using less memory than if all images are read or generated first.

MarkCallow

This completes my re-review after the latest changes.

MarkCallow · 2026-08-27T08:02:29Z

+
+        switch (bcn) {
+          case KTX_BCN_COMPRESSION_BC1:
+            original = tmpDir / format("encode_rgb8_{}_to_bc1{}_then_decode_original.ktx2", isSRGB ? "srgb" : "unorm", rdo_str);


Do the same with isSRGB as you've done with rdo.

MarkCallow · 2026-08-27T08:08:51Z

+        }
+        khr_df_model_e model = static_cast<khr_df_model_e>(KHR_DFDVAL(pBdb, MODEL));
+        // TODO: check that model is correctly set
+        // EXPECT_EQ(model, color);


Please implement this. You can set the expected model in the switch statement above that selects compressedFormat.

Note that this commented-out line is comparing model and color!

MarkCallow · 2026-08-27T08:18:47Z

+TEST_F(ktxTexture2_BCnEncodeDecodeTestRGB8_SRGB, encode_rgb8_srgb_to_bc1_then_decode) { runTest(KTX_BCN_COMPRESSION_BC1, false); }
+TEST_F(ktxTexture2_BCnEncodeDecodeTestRGB8_SRGB, encode_rgb8_srgb_to_bc1_rdo_then_decode) { runTest(KTX_BCN_COMPRESSION_BC1, true); }
+
+// TODO: add encode-then-decode test for BC2 once BC2 encoder is implemented


What is the plan for BC2?

MarkCallow · 2026-08-27T08:40:35Z

+};
+
+/*
+ * @~English


My mistake. Sorry. This should be internal not private - which has the same meaning as c++'s private.

MarkCallow · 2026-08-27T08:40:55Z

+//************************************************************************
+
+/**
+ * @~English


internal here too.

MarkCallow · 2026-08-27T09:20:27Z

+    {
+      // Can't directly encode from BCn to BCn
+      if (options.encodeBCn)
+        fatal_usage("Encoding from BCn format {} to another BCn format {} is not supported. " +


Make indent 4 spaces here and after next if.

MarkCallow · 2026-08-27T09:22:40Z

+    }
+
+    // Can't encode from BCn to ASTC
+    if ((model == KHR_DF_MODEL_BC1A || model == KHR_DF_MODEL_BC3 || model == KHR_DF_MODEL_BC4 ||


This is still a duplicate. Now, lines 285:288. Delete these.

MarkCallow · 2026-08-27T09:32:21Z

+        <dt>\--bcn-quality &lt;level&gt;</dt>
+        <dd>The quality level configures the quality-performance tradeoff for
+            BC1, BC3, and BC7 encoders. Can also be set via the following
+            presets: fastest, faster, fast, medium, thorough, exhaustive.


If you do not use the presets, what is the value range you can use?

MarkCallow · 2026-08-27T09:37:07Z

+        <dd>Set BCn RDO quality scalar to the specified value. Lower values
+            yield higher quality/larger supercompressed files, higher values
+            yield lower quality/smaller supercompressed files. A good range to
+            try is [.25,10]. For normal maps a good range is [.25,.75]. The full


This seems to answer the question about RDO and --normal-mode. The user has to choose whether to use RDO or not.normal_mode should not override their choice.

In the ETC1S/BasisLZ codec the default settings include RDO. --normal-mode disables it in that case. The user can override by using the explicit RDO options.

MarkCallow · 2026-08-27T09:39:28Z

+            kBCnQuality,
+            "The quality level configures the quality-performance tradeoff for BC1, BC3, and BC7 "
+            "encoders. Can also be set via the following presets: fastest, faster, fast, medium, "
+            "thorough, exhaustive."


Same question I asked in the Doxygen comment.

walcht and others added 4 commits May 5, 2026 17:01

Add initial BC1 decoder

306b578

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

Add BC1, BC3, BC4, BC5, and BC7 decoding support to ktx extract command

12ab662

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

Merge branch 'main' into add-BCn-decoder

67facb3

walcht marked this pull request as draft May 7, 2026 14:11

walcht added 2 commits May 8, 2026 09:13

Merge branch 'add-BCn-decoder' of github.com:walcht/KTX-Software into…

6848b82

… add-BCn-decoder

walcht added 2 commits May 10, 2026 05:44

Fix/Suppress unused variables/functions compilier warnings

4a6580d

*Some CIs report further unused variables/functions that are not reported when building locally. Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht added 2 commits May 10, 2026 12:31

Update CTS submodule commit pointer

bf153ca

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht and others added 6 commits May 10, 2026 14:30

Update CTS submodule

b2eec1b

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

Temporary pre-integration of bc7enc_rdo as a 'subrepo'

68bb0de

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

Merge branch 'KhronosGroup:main' into add-BCn-decoder

236cc4d

Merge branch 'add-BCn-decoder' of github.com:walcht/KTX-Software into…

389fa06

… add-BCn-decoder

MarkCallow reviewed Aug 5, 2026

View reviewed changes

Comment thread tools/ktx/command_extract.cpp Outdated

Merge branch 'main' into add-BCn-decoder

cf217ca

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht added 4 commits August 20, 2026 14:01

address review comments

4171dbc

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

Merge branch 'main' into add-BCn-decoder

7294128

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

Update CTS submodule commit pointer

cd4c591

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

feat: use cxxopts for ktxdiff CLI

e2b8ed4

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

fix: add missing cxxopts to tests CMakeLists

3e7dc51

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht added 4 commits August 21, 2026 04:19

fix: ktxdiff SFLOAT16 diff issues with small values

cdd008b

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

fix: erroneously computed expectedDstByteLength

eae29b6

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

feat: add --ignore-signed option to ktxdiff

3109d47

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

fix docs; add RGBA16 input for BC6HU; fix typos

5c37393

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

walcht added 2 commits August 25, 2026 02:01

add BC6HU RGBA16 cleanup preprocessing step

33710e7

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

attempt to address nan/inf issues for FLOAT16 in ktxdiff

a4fc3e9

Signed-off-by: Walid Chtioui <walid.chtioui.main@gmail.com>

MarkCallow reviewed Aug 26, 2026

View reviewed changes

MarkCallow reviewed Aug 27, 2026

View reviewed changes

Uh oh!

Conversation

walcht commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

walcht commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarkCallow commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

walcht commented May 10, 2026

Uh oh!

walcht commented May 10, 2026

Uh oh!

MarkCallow commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarkCallow commented May 10, 2026

Uh oh!

walcht commented May 10, 2026

Uh oh!

walcht commented May 10, 2026

Uh oh!

MarkCallow commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

walcht commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarkCallow commented May 14, 2026

Uh oh!

MarkCallow commented Aug 4, 2026

Uh oh!

MarkCallow commented Aug 4, 2026

Uh oh!

walcht commented Aug 4, 2026

Uh oh!

MarkCallow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

walcht commented Aug 6, 2026

Uh oh!

MarkCallow commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

walcht commented Aug 9, 2026

Uh oh!

MarkCallow commented Aug 10, 2026

Uh oh!

walcht commented Aug 13, 2026

Uh oh!

MarkCallow commented Aug 14, 2026

Uh oh!

MarkCallow commented Aug 17, 2026

Uh oh!

walcht commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

walcht commented Aug 21, 2026

Uh oh!

walcht commented Aug 24, 2026

walcht commented May 7, 2026 •

edited

Loading

walcht commented May 7, 2026 •

edited

Loading

CLAassistant commented May 8, 2026 •

edited

Loading

MarkCallow commented May 9, 2026 •

edited

Loading

MarkCallow commented May 10, 2026 •

edited

Loading

MarkCallow commented May 11, 2026 •

edited

Loading

walcht commented May 14, 2026 •

edited

Loading

MarkCallow commented Aug 7, 2026 •

edited

Loading

walcht commented Aug 20, 2026 •

edited

Loading

MarkCallow left a comment •

edited

Loading